-
-
Notifications
You must be signed in to change notification settings - Fork 31.5k
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
Add permission checking to all RainMachine services #22399
Add permission checking to all RainMachine services #22399
Conversation
b4ee99b
to
390cac3
Compare
b97c594
to
e8c0fe2
Compare
@@ -128,6 +131,44 @@ | |||
}, extra=vol.ALLOW_EXTRA) | |||
|
|||
|
|||
def _check_valid_user(hass): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we make this a helper decorator? There's nothing here that is specific to rainmachine. It could be used on any domain that has entity registry support that needs to do the same check, if we pass in domain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, love that idea. I'll submit a PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bachya and then maybe also update your implementation here so that you do something like this:
check_valid_user = GEN_FUNC(hass, DOMAIN)
@check_valid_user
async def handle_bla(call):
That way the decorator is re-used instead of re-generated each time.
…2399) * Add permission checking to all RainMachine services * Linting * Some initial work * Owner comments * Test in place (I think) * Linting * Update conftest.py
Description:
This PR implements user permission checking on all RainMachine services (per the "Can I Have User Permissions?" blog post).
Related issue (if applicable): N/A
Pull request in home-assistant.io with documentation (if applicable): N/A
Example entry for
configuration.yaml
(if applicable):Checklist:
tox
. Your PR cannot be merged unless tests pass