Add support for Z-Wave credential management#168360
Add support for Z-Wave credential management#168360AlCalzone wants to merge 11 commits intohome-assistant:devfrom
Conversation
home-assistant
Bot
added
cla-signed
has-tests
integration: zwave_js
Top 100
|
Hey there @home-assistant/z-wave, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
Introduces first-pass Z-Wave JS “Access Control” (credential/user) management via new lock entity services, helper logic, and accompanying UI strings/icons plus tests.
Changes:
- Add
access_control_helpers.pyimplementing user/credential CRUD, capability queries, and auto-slot selection. - Register new credential-management services and wire them through the Z-Wave lock entity.
- Add service UI metadata (strings/icons/services.yaml) and initial test coverage.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
homeassistant/components/zwave_js/access_control_helpers.py |
New helper module implementing access-control business logic and structured service responses. |
homeassistant/components/zwave_js/lock.py |
Adds lock entity methods for new access-control services with translated error wrapping. |
homeassistant/components/zwave_js/services.py |
Registers new platform entity services and validates service schemas. |
homeassistant/components/zwave_js/services.yaml |
Declares new services and selectors for the service UI. |
homeassistant/components/zwave_js/strings.json |
Adds translated error messages and service field descriptions for credential management. |
homeassistant/components/zwave_js/icons.json |
Adds service icons for the new credential/user services. |
homeassistant/components/zwave_js/const.py |
Adds service/attribute constants and credential/user type string constants. |
tests/components/zwave_js/test_credential_services.py |
Adds tests for new credential management services and auto-find behavior. |
| async def test_clear_user( | ||
| hass: HomeAssistant, | ||
| lock_schlage_be469: Node, | ||
| integration: MockConfigEntry, | ||
| ) -> None: | ||
| """Test clear_user deletes a single user.""" | ||
| api = _mock_access_control(lock_schlage_be469) |
There was a problem hiding this comment.
Add negative-path tests that assert the rendered HomeAssistantError message/translation placeholders for failures (similar to test_set_lock_usercode_error in tests/components/zwave_js/test_lock.py) to catch issues like missing {user_index} placeholders.
| """Delete a single access-control user.""" | ||
| await node.access_control.async_delete_user(user_index) | ||
|
|
||
|
|
||
| async def async_clear_all_users(node: Node) -> None: | ||
| """Delete all access-control users.""" |
There was a problem hiding this comment.
Add the same async_is_supported() guard used by the other helpers so unsupported devices return the translated access_control_not_supported error instead of calling delete APIs unconditionally.
| """Delete a single access-control user.""" | |
| await node.access_control.async_delete_user(user_index) | |
| async def async_clear_all_users(node: Node) -> None: | |
| """Delete all access-control users.""" | |
| """Delete a single access-control user.""" | |
| supported = await node.access_control.async_is_supported() | |
| if not supported: | |
| raise HomeAssistantError( | |
| translation_domain=DOMAIN, | |
| translation_key="access_control_not_supported", | |
| ) | |
| await node.access_control.async_delete_user(user_index) | |
| async def async_clear_all_users(node: Node) -> None: | |
| """Delete all access-control users.""" | |
| supported = await node.access_control.async_is_supported() | |
| if not supported: | |
| raise HomeAssistantError( | |
| translation_domain=DOMAIN, | |
| translation_key="access_control_not_supported", | |
| ) |
|
|
||
|
|
||
| async def async_clear_all_credentials(node: Node, user_index: int) -> None: | ||
| """Delete all credentials for a user.""" |
There was a problem hiding this comment.
Check async_is_supported before listing/deleting credentials so clear_all_credentials returns access_control_not_supported cleanly instead of calling APIs that may not exist.
| """Delete all credentials for a user.""" | |
| """Delete all credentials for a user.""" | |
| if not await node.access_control.async_is_supported(): | |
| raise HomeAssistantError("access_control_not_supported") |
| """Delete a single access-control user.""" | ||
| status = await node.access_control.async_delete_user(user_index) | ||
| _raise_on_set_user_error(status) | ||
|
|
||
|
|
||
| async def async_clear_all_users(node: Node) -> None: | ||
| """Delete all access-control users.""" |
There was a problem hiding this comment.
Check access-control support before calling delete operations so unsupported devices raise the translated access_control_not_supported error instead of failing with lower-level exceptions.
| """Delete a single access-control user.""" | |
| status = await node.access_control.async_delete_user(user_index) | |
| _raise_on_set_user_error(status) | |
| async def async_clear_all_users(node: Node) -> None: | |
| """Delete all access-control users.""" | |
| """Delete a single access-control user.""" | |
| supported = await node.access_control.async_is_supported() | |
| if not supported: | |
| raise HomeAssistantError( | |
| translation_domain=DOMAIN, | |
| translation_key="access_control_not_supported", | |
| ) | |
| status = await node.access_control.async_delete_user(user_index) | |
| _raise_on_set_user_error(status) | |
| async def async_clear_all_users(node: Node) -> None: | |
| """Delete all access-control users.""" | |
| supported = await node.access_control.async_is_supported() | |
| if not supported: | |
| raise HomeAssistantError( | |
| translation_domain=DOMAIN, | |
| translation_key="access_control_not_supported", | |
| ) |
| target: | ||
| entity: | ||
| integration: zwave_js | ||
| fields: |
There was a problem hiding this comment.
Switch these new node-scoped services to the existing zwave_js pattern of area_id/device_id/entity_id fields (see set_config_parameter in this file around lines 275-297) instead of an entity-only target, so the UI and schema support the same targeting options.
| target: | |
| entity: | |
| integration: zwave_js | |
| fields: | |
| fields: | |
| area_id: | |
| selector: | |
| area: | |
| device_id: | |
| selector: | |
| device: | |
| integration: zwave_js | |
| entity_id: | |
| selector: | |
| entity: | |
| integration: zwave_js |
| for cred in credentials: | ||
| status = await node.access_control.async_delete_credential( | ||
| user_index, cred.type, cred.slot | ||
| ) | ||
| _raise_on_set_credential_error(status) |
There was a problem hiding this comment.
Delete credentials in parallel (e.g., via asyncio.gather) or via a bulk API if available, to avoid long sequential round-trips when a user has many credentials.
| """Delete a single credential.""" | ||
| status = await node.access_control.async_delete_credential( | ||
| user_index, credential_type, credential_slot | ||
| ) | ||
| _raise_on_set_credential_error(status) |
There was a problem hiding this comment.
Add an access-control support check (async_is_supported) here so clear_credential fails with the translated access_control_not_supported error on devices without Access Control CC.
| assert result[entity_id]["credential_type"] == "pin_code" | ||
| assert result[entity_id]["credential_slot"] == 1 | ||
|
|
||
|
|
There was a problem hiding this comment.
Add an assertion/test case where the returned credential’s user_id differs from the requested user_index, so get_credential_status can’t incorrectly report credential_exists for the wrong user.
| async def test_get_credential_status_wrong_user( | |
| hass: HomeAssistant, | |
| entity_registry: er.EntityRegistry, | |
| device_registry: dr.DeviceRegistry, | |
| client, | |
| lock_schlage_be469: Node, | |
| integration: MockConfigEntry, | |
| ) -> None: | |
| """Test get_credential_status when returned credential belongs to another user.""" | |
| api = _mock_access_control(lock_schlage_be469) | |
| cred = MagicMock() | |
| cred.user_id = 2 | |
| api.async_get_credential_cached.return_value = cred | |
| entity_id = _lock_entity_id( | |
| entity_registry, device_registry, client, lock_schlage_be469 | |
| ) | |
| result = await hass.services.async_call( | |
| DOMAIN, | |
| "get_credential_status", | |
| { | |
| ATTR_ENTITY_ID: entity_id, | |
| "user_index": 1, | |
| "credential_type": "pin_code", | |
| "credential_slot": 1, | |
| }, | |
| blocking=True, | |
| return_response=True, | |
| ) | |
| assert result[entity_id]["credential_exists"] is False | |
| assert result[entity_id]["user_index"] == 1 | |
| assert result[entity_id]["credential_type"] == "pin_code" | |
| assert result[entity_id]["credential_slot"] == 1 |
|
|
||
|
|
||
| async def async_clear_user(node: Node, user_index: int) -> None: | ||
| """Delete a single access-control user.""" |
There was a problem hiding this comment.
Add the same async_is_supported() guard used by the other helpers so async_clear_user returns the translated access_control_not_supported error instead of calling into the API on unsupported devices.
| """Delete a single access-control user.""" | |
| """Delete a single access-control user.""" | |
| supported = await node.access_control.async_is_supported() | |
| if not supported: | |
| raise HomeAssistantError( | |
| translation_domain=DOMAIN, | |
| translation_key="access_control_not_supported", | |
| ) |
| Credential( | ||
| type=CREDENTIAL_TYPE_MAP.get(cred.type, str(cred.type)), | ||
| slot=cred.slot, | ||
| data=cred.data if isinstance(cred.data, str) else None, |
There was a problem hiding this comment.
Avoid returning credential secret material by removing or masking the data field from get_users results so PINs/passwords can’t be exposed via the service response.
| data=cred.data if isinstance(cred.data, str) else None, | |
| data=None, |
There was a problem hiding this comment.
We need to decide whether we want this or not. This allows admins to check which PINs a user has. The frontend has the corresponding toggle to show the PIN in plaintext.
AlCalzone
changed the title
AlCalzone
force-pushed
the
zwave-credentials
branch
from
7a16119 to
69b377e
Compare
| Credential( | ||
| type=CREDENTIAL_TYPE_MAP.get(cred.type, str(cred.type)), | ||
| slot=cred.slot, | ||
| data=cred.data if isinstance(cred.data, str) else None, |
There was a problem hiding this comment.
Avoid returning raw credential secrets in the get_users response by omitting/masking the data field (or gating it behind an explicit opt-in parameter), since PINs/passwords can otherwise be exposed via service responses and persisted in logs/traces.
| data=cred.data if isinstance(cred.data, str) else None, | |
| data=None, |
This comment was marked as resolved.
This comment was marked as resolved.
AlCalzone
force-pushed
the
zwave-credentials
branch
from
b5f2c64 to
fa16d85
Compare
| # cached_property: override via instance __dict__ | ||
| node.endpoints[0].__dict__["access_control"] = api |
| # Validate credential_data length and format against device capabilities | ||
| if not ( | ||
| type_cap.min_credential_length | ||
| <= len(credential_data) | ||
| <= type_cap.max_credential_length | ||
| ): | ||
| raise ServiceValidationError( | ||
| translation_domain=DOMAIN, | ||
| translation_key="credential_data_invalid_length", | ||
| translation_placeholders={ | ||
| "credential_type": cred_type_str, | ||
| "min_length": str(type_cap.min_credential_length), | ||
| "max_length": str(type_cap.max_credential_length), | ||
| }, | ||
| ) | ||
| if credential_type is UserCredentialType.PIN_CODE and not credential_data.isdigit(): |
| Credential( | ||
| type=CREDENTIAL_TYPE_MAP.get(cred.type, str(cred.type)), | ||
| slot=cred.slot, | ||
| data=cred.data if isinstance(cred.data, str) else None, |
Proposed change
This PR adds support for Z-Wave credential management. This first iteration is aligned with the Matter credential mangement added in #161936, home-assistant/frontend#28672 and followup PRs.
The main difference is that this PR also adds support for having multiple credentials of the same type per user, and distinguishes between PINs (0-9) and passwords (alphanumeric).
Like Matter, we expose the credential management through the
lockentity. Even though the functionality is technically not limited to locks, in practice there haven't been any non-lock devices with user management features.Depends on home-assistant-libs/zwave-js-server-python#1418
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: