Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR consolidating multiple fixes as provided by users across 3 "Issue" threads #102

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

edrikk
Copy link

@edrikk edrikk commented Feb 25, 2025

@ajschmidt8
As requested, creating a PR which pulls together changes from:

#93
#99
and
#92

I'm not an expert here but I can say that I've been running these changes for months (and with the updates requirements.txt for weeks) without issues on my setup.

In short:

  • Fixes - Socket IO ACL not setup. Keypad not working. stuck in Please Wait Loading
  • Fixes - Broken custom notifications
  • Update to support Flask-login 0.5.0
  • Updates requirements.txt to pull latest modules (including Flask-login 0.5.0 which is required)

As provided by star-glider in issue:

nutechsoftware#99

The custom notification option is broken; the notify_data dictionary is always empty. There's quite a bit of code that works over this dictionary (lines 1182 through 1204 in types.py), but none of it runs because custom_values is blank. I haven't dug in deeply enough to really understand what's going on here; it's way more code than exists in the other notification methods, and I'm not sure why.

I ran into this trying to set up Slack notifications (JSON POST) and threw in a quick patch to get it working. Basically, I just skip over all of the inactive code, create a dictionary directly in a manner similar to the other notification methods, and then reference that in the _do_post method call. Since the project isn't being maintained, I don't think it's worth the time to submit a pull request (and do the associated work to figure out why there's all that additional code), but I figured I'd drop my patch in here just in case it helps anyone.

Two notes: 1) the modification to line 1144 was just me troubleshooting; it's not relevant to the fix but is kind of a nice-to-have; 2) the new "message_string" variable includes a version of the "Subject" option from the email notification, but I just harcoded in the message instead of modifying the web forms to offer it as a front-end-selectable option, mainly because I'm lazy. Presumably, if you're applying this patch, you can just type in whatever you want there or just set it equal to the 'text' variable to eliminate it entirely.

I know the project is essentially abandoned, but it's a great addition to these old (and very reliable) ADEMCO systems. As long as you're running it behind a firewall with no ports exposed, there's really not much risk in running an old image with so little attack surface, and while old it works just perfectly (kind of like my 20-year-old alarm system)!
…t Loading

From jimmyhchan

nutechsoftware#93

I have manually setup an raspbian image on a raspberrypi 3 and have most of everything working with a customized version of the docker file.

Unfortunately, the virtual keypad was not working. The symptoms is the same as outlined here https://www.alarmdecoder.com/forums/viewtopic.php?f=3&t=1167&p=4267&hilit=keypad+loading#p4267
Steps to repro:

    setup the webapp
    go through the setup flow
    navigate to the keypad page
    BUG:
    Keypad only shows Please Wait Loading...
    Clicks on any button causes the log

 ERROR: default_error_handler: method_access_denied, You do not have access to method "on_keypress" (endpoint=/alarmdecoder, msg_id=None) [in /usr/local/lib/python2.7/dist-packages/socketio/virtsocket.py:51]

Root cause

I debugged a bit and found the root cause is user_id is None in the recv_connect method which calls add_acl_method:
https://github.com/nutechsoftware/alarmdecoder-webapp/blob/master/ad2web/decoder.py#L829

I could not find anything which sets the user_id for the session. Perhaps a dependency api changed.
Fix/workaround

I have a fix using  user_is_authenticated(current_user) instead. Happy to create a real PR if someone can confirm this is the desired approach.
Changed 'user_id' to '_user_id' to support changes in Flask-login 0.5.0 - requires requirements.txt update to Flask-login == 0.5.0
Update the latest compatible version of most modules.
@ajschmidt8
Copy link
Collaborator

thanks, I will try to review this this weekend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants