XaF/qolsysgw

Suggestion for improvement of ha_check_user_code documentation

Closed this issue · 3 comments

This documentation is confusing because ha_check_user_code cannot be set if code_disarm_required and code_arm_required are both false. If ha_check_user_code is set to false and the code_disarm_required and code_arm_required is false, the disarm will fail silently (as will arm I think if code is required).

ha_check_user_code: whether or not the check of the user code should be done in the Home Assistant user interface. If set to false, the code will never be checked in home assistant even if required for arming, disarming or triggering the alarm, and will always be sent to Qolsys Gateway for checking. If set to true and the code is required for one of the actions, if will be checked by the Home Assistant user interface, and never sent through the network to Qolsys Gateway (a session token is shared to Home Assistant by Qolsys Gateway to make sure it is receiving control messages from the right instance). Note that in case of issues of checking a code in Qolsys Gateway, things will simply fail silently. Requires panel_user_code to be set, will silently revert to false otherwise. Defaults to true.

I suggest changing it to:

ha_check_user_code: whether or not the check of the user code should be done in the Home Assistant user interface. this should only be set if code_disarm_required or code_arm_required is set to true If set to false, the code will never be checked in home assistant even if required for arming, disarming or triggering the alarm, and will always be sent to Qolsys Gateway for checking. If set to true and the code is required for one of the actions, if will be checked by the Home Assistant user interface, and never sent through the network to Qolsys Gateway (a session token is shared to Home Assistant by Qolsys Gateway to make sure it is receiving control messages from the right instance). Note that in case of issues of checking a code in Qolsys Gateway, things will simply fail silently. Requires panel_user_code to be set, will silently revert to false otherwise. Defaults to true.

Alternatively, I guess the code might be considered broken and should ignore the ha_check_user_code if the _required flags are false.

XaF commented

I started looking at this and was able to reproduce the failure - I'm hesitating between a change in documentation / configuration check (to avoid issues happening, fail early and loudly) and a different handling in the code.

By itself, ha_check_user_code means that HA is sole-responsible for checking if the user has the right to perform the action, and the gateway will not handle the check. If set to false, it means the gateway "needs to check" that the action can be performed (if having an available valid code, or relying on the panel by simply forwarding to it).

When setting code_disarm_required to false, home assistant does not fetch any code from the user, and did not receive any code from the gateway either. Meaning if ha_check_user_code is false, given the gateway does not simply trust home assistant, it tries to check the code (which is not there), and thus fails. The failing thus "makes sense" even if the UX of that failing is not great.

This leads back to my hesitation:

  • This is a configuration "incompatibility" issue in some way: does what we try to say with ha_check_user_code set to false still make sense? Maybe it does if we have secure_arm or code_arm_required or code_trigger_required set to true, so maybe failing at configuration would just make things more confusing
  • Does changing the behavior when receiving a control potentially open a security risk? Currently disarm forces the code requirement to true because we "DO" need a code (whether configured in the gateway configuration or coming from the user), we can't simply ignore the flag if the required flag is false, because even if the required flag is false, we might need a code (because none is configured, or because secure_arm is enabled on the partition), so that requires to be handled carefully.

I'll think a bit more about it, but I do believe something needs to be done about that. #17 and #54 definitely show that this is confusing, and it shouldn't be!

Even an error message rather than failing silently would help here.

I would expect that if ha_check_user_code is false and code_disarm_required is false and panel_code is set, then disarming should work.

It's weird to me that this isn't teh case.