search

myhomeiot / DahuaVTO

Control Dahua VTO/VTH devices from Home Assistant
GNU General Public License v3.0
158 stars 19 forks source link

Use async_register instead of async_register_admin_service #7

Closed Azelphur closed 3 years ago

Azelphur commented 3 years ago

With the use of async_register_admin_service, only administrator accounts can use these features. I bumped into this issue where non-admin users were unable to trigger door unlock.

This patch resolves that issue, but may not be the best way of doing it. If the use of async_register_admin_service was intentional, perhaps some way to allow non admin users access would be handy.

myhomeiot commented 3 years ago

Thanks for contribution!

If I add additional configuration param for example admin_only_services and by default it's will be true it's will be more useful?

Azelphur commented 3 years ago

Sure, that would solve it for me.

Edit: I spotted this it talks about a new permissions system and calls on integration developers to read up on how to check permissions I'll look into it when I have some time and see if there's anything relevant to this PR in there.

Azelphur commented 3 years ago

Hi, have updated my PR, I think this is the correct way to handle permissions, it's documented here and can be configured as detailed here

This should allow users to have the permissions however they want, in theory :)

myhomeiot commented 3 years ago

@azelphur thanks for information! I check Home Assistant Core code and add user permission checks in the same way how they did it. I was check it according article which you points me, and it's works! ;-) Please update to the lattest version and let's me know if it's works for you. Thanks in advance.

Azelphur commented 3 years ago

Doesn't work for me, tried running:

service: dahua_vto.open_door
data:
  entity_id: sensor.doorbell
  channel: 1
  short_number: HA

and nothing happens. Nothing in the logs either. Done a little debugging, it seems DahuaVTO.async_open_door never gets called.

myhomeiot commented 3 years ago

Very strange, check it again, works. sensor.doorbell in OK state and has deviceType and serialNumber attributes? Do you change user group permissions or you did test under user with admin rights?

Azelphur commented 3 years ago

sensor.doorbell has state "OK" and attributes:

Tested with both admin and normal user, in both cases nothing happens. If I revert your most recent commit and restart home assistant, it works (only as an admin user)

myhomeiot commented 3 years ago

Very strange, which version of Home Assistant do you have? I test it with 2021.9, just updates integration using HACS and all works as with previous version.

Can you try to revert to 1.0.4 using HACS (or uninstall) and update to 1.0.5 using HACS?

Azelphur commented 3 years ago

I'm using 2021.9.2, for reference I run home assistant (core only), using docker, and I have added this component manually (no HACS).

If it helps, I'm on the official home assistant discord, we could chat there and try and debug it.

Azelphur commented 3 years ago

This is resolved in the most recent release, thanks again :)