Add user enable and disable actions#1187
Conversation
📝 WalkthroughWalkthroughAdds two Home Assistant services, ChangesUser Management Services
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@custom_components/spook/ectoplasms/homeassistant/services/enable_user.py`:
- Around line 26-33: The async_handle_service handler currently enables users
from call.data["user_id"] without checking for system accounts; update
async_handle_service to fetch each user via hass.auth.async_get_user, then if
user is None raise HomeAssistantError as before, but also check
user.system_generated and either raise HomeAssistantError (or skip with a logged
warning) when true to prevent modifying internal HA service accounts, only
calling hass.auth.async_update_user(user, is_active=True) for
non-system_generated users.
In `@documentation/actions.md`:
- Around line 395-405: Add the missing MyST anchor label for the "User: Disable"
heading so the cross-reference from the "User: Enable" section resolves;
specifically, insert the anchor `(user-disable)=` immediately before the `##
User: Disable` heading (the pair of headings referenced are the `## User:
Disable` and `## User: Enable` entries) so the `[](`#user-disable`)` link in the
enable section points to the correct target.
In `@documentation/users.md`:
- Around line 56-62: The Jinja2 template snippet contains invalid constructs for
Home Assistant templates—specifically using await and
hass.auth.async_get_users()—so remove the entire code block that iterates with
`{% for user in (await hass.auth.async_get_users()) %}` and replace it with
either (a) no template block (delete the snippet) or (b) a clearly labelled
alternative that uses the REST API or a backend Python script to list users
(e.g., note “Use Home Assistant REST API or a Python script to fetch users;
templates cannot call hass.auth or use await”). Ensure the replacement text
explicitly states templates cannot access hass or async functions and points
readers to the REST API/Python script option.
|
|
Made changes requested by CodeRabbit, also tested 
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@custom_components/spook/ectoplasms/homeassistant/services/enable_user.py`:
- Around line 26-36: The service currently only blocks system-generated accounts
in async_handle_service (uses hass.auth.async_get_user and
hass.auth.async_update_user); add a guard that also prevents acting on the owner
account by checking the user's owner flag (e.g., user.is_owner) and raising a
HomeAssistantError with a clear message like "Cannot enable the owner account"
(and similarly "Cannot disable the owner account" in disable_user.py) before
calling async_update_user so both enable_user.py and disable_user.py
consistently protect the owner.
In `@documentation/users.md`:
- Around line 54-62: The template example under "Developer Tools → Template" is
misleading because it iterates states.person and therefore only shows
person-linked users and will yield state.attributes.user_id as None for person
entities without linked users; update the copy to either (A) qualify the
sentence that it only lists user IDs for person entities (mentioning
states.person and state.attributes.user_id) and warn that standalone
auth/API/integration users are omitted, or (B) remove the template block
entirely and instead rely on the previously described UI path; modify the
introductory sentence on Line 54 accordingly to reflect the chosen approach.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1187 +/- ##
==========================================
+ Coverage 61.86% 62.38% +0.51%
==========================================
Files 121 123 +2
Lines 3084 3126 +42
Branches 396 402 +6
==========================================
+ Hits 1908 1950 +42
Misses 1128 1128
Partials 48 48 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Actionable comments posted: 0 |
frenckatron
changed the title
homeassistant.disable_user + homeassistant.enable_user
frenckatron
left a comment
frenckatron
left a comment
There was a problem hiding this comment.
Reviewed the implementation and the follow-up test coverage.
The services correctly resolve users through the auth manager, reject missing and system-generated users, and delegate activation state changes through the existing Home Assistant auth APIs. The added tests cover enable, disable, multiple user IDs, missing users, and system-generated users.
I also re-ran the targeted validation locally:
uv run pytest tests/ectoplasms/homeassistant/services/test_user.py -quv run ruff check custom_components/spook/ectoplasms/homeassistant/services/disable_user.py custom_components/spook/ectoplasms/homeassistant/services/enable_user.py tests/ectoplasms/homeassistant/services/test_user.pyuv run ruff format --check custom_components/spook/ectoplasms/homeassistant/services/disable_user.py custom_components/spook/ectoplasms/homeassistant/services/enable_user.py tests/ectoplasms/homeassistant/services/test_user.py
All passed. Looks good to me.
../Frenckatron
The robot version of Frenck.
Blogging my personal ramblings at frenck.dev
4 participants
Description
These options allow you to enable/disable users, i.e. toggle this setting:
Motivation and Context
I have home assistant in my car, it can open my gate. I don't want someone to steal my car and be able to press the open gate button... so I lock the account and unlock it based on certain automations.
How has this been tested?
I tested on my own environment and the user switches between Active enabled and disabled.
Screenshots (if appropriate):
Types of changes
Checklist