Risco Alarm : Keypad shown, even if code arm/disarm = false

home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.

https://www.home-assistant.io

Apache License 2.0

72.4k stars 30.3k forks source link

Risco Alarm : Keypad shown, even if code arm/disarm = false #39297

Closed pergolafabio closed 4 years ago

pergolafabio commented 4 years ago

The problem

The lovelace alarm panel is showing a keypad when both options code arm required & code disarm required are false (not checked in config flow) , it should be hidden...

Environment

Home Assistant Core release with the issue: 114.3
Last working Home Assistant Core release (if known): None
Operating environment (OS/Container/Supervised/Core): HassOS
Integration causing this issue: Risco Alarm, not yet released, scheduled for 115.0
Link to integration documentation on our website: This PR is giving issues https://github.com/home-assistant/core/pull/39177 & https://github.com/home-assistant/core/pull/39154

Traceback/Error logs

No Logs

Additional information

@OnFreund , i created an issue for this :-)

thnx for looking into it!

pergolafabio commented 4 years ago

Some extra info

https://github.com/home-assistant/frontend/pull/2379

OnFreund commented 4 years ago

This isn't a bug, but rather the intended behavior. The system should not force a non-privileged UI to expose whether a pin code is required (the options flow exposes it, but it's an admin UI). An potential alternative could be to add configuration to the UI, and allow the admin to decide to hide the keypad, but that's not for me to decide.

OnFreund commented 4 years ago

As for Discord, I'm @on

pergolafabio commented 4 years ago

Ah , strange, cause a week ago, when I tested your first integration, it didn't display the code keypad... Also the mqtt version isn't showing it :)

OnFreund commented 4 years ago

That version was a temp version that didn't support codes at all, so no information was leaked by not showing the keypad. The MQTT version needs to support different types of alarms, including some where the pin code is text rather than numbers, so is not really a comparison. I quickly glanced through the other alarm integrations (excluding the generic ones like mqtt, template, etc...), and couldn't find any other integration that allows the degree of control that the Risco integration does (4 different options), so there isn't really a good comparison. From a security perspective, though, this seems like the right thing to do, when the difference in usability is minimal.

pergolafabio commented 4 years ago

Ok, no problem, if it bothers me, then I can still use the template alarm... I don't see those pending times anyway...

It was just strange to me, never saw the keypad before, always thought that the code_required functions should hide the keypad, if no codes were used... Cause I can now arm/disarm the alarm, without entering anything on the keypad

probot-home-assistant[bot] commented 4 years ago

risco documentation risco source _{^{(message by IssueLinks)}}

OnFreund commented 4 years ago

@pergolafabio Initially you wanted pin code support. Why aren't you using it? :)

Anyway, this issue can be closed. Let's continue the discussion on Discord.

pergolafabio commented 4 years ago

yeah, indeed i wanted it, but only for diarming ... to arm, there is no need to enter a pin :-)

OnFreund commented 4 years ago

Well, if you're using the pin to disarm, you definitely need the keypad in the UI then...