diff --git a/supervisor/api/auth.py b/supervisor/api/auth.py index b68336384..9b194054b 100644 --- a/supervisor/api/auth.py +++ b/supervisor/api/auth.py @@ -127,14 +127,14 @@ class APIAuth(CoreSysAttributes): return { ATTR_USERS: [ { - ATTR_USERNAME: user[ATTR_USERNAME], - ATTR_NAME: user[ATTR_NAME], - ATTR_IS_OWNER: user[ATTR_IS_OWNER], - ATTR_IS_ACTIVE: user[ATTR_IS_ACTIVE], - ATTR_LOCAL_ONLY: user[ATTR_LOCAL_ONLY], - ATTR_GROUP_IDS: user[ATTR_GROUP_IDS], + ATTR_USERNAME: user.username, + ATTR_NAME: user.name, + ATTR_IS_OWNER: user.is_owner, + ATTR_IS_ACTIVE: user.is_active, + ATTR_LOCAL_ONLY: user.local_only, + ATTR_GROUP_IDS: user.group_ids, } for user in await self.sys_auth.list_users() - if user[ATTR_USERNAME] + if user.username ] } diff --git a/supervisor/api/ingress.py b/supervisor/api/ingress.py index bf2d56ffe..378e74d64 100644 --- a/supervisor/api/ingress.py +++ b/supervisor/api/ingress.py @@ -29,8 +29,8 @@ from ..const import ( HEADER_REMOTE_USER_NAME, HEADER_TOKEN, HEADER_TOKEN_OLD, + HomeAssistantUser, IngressSessionData, - IngressSessionDataUser, ) from ..coresys import CoreSysAttributes from ..exceptions import HomeAssistantAPIError @@ -75,12 +75,6 @@ def status_code_must_be_empty_body(code: int) -> bool: class APIIngress(CoreSysAttributes): """Ingress view to handle add-on webui routing.""" - _list_of_users: list[IngressSessionDataUser] - - def __init__(self) -> None: - """Initialize APIIngress.""" - self._list_of_users = [] - def _extract_addon(self, request: web.Request) -> Addon: """Return addon, throw an exception it it doesn't exist.""" token = request.match_info["token"] @@ -306,20 +300,15 @@ class APIIngress(CoreSysAttributes): return response - async def _find_user_by_id(self, user_id: str) -> IngressSessionDataUser | None: + async def _find_user_by_id(self, user_id: str) -> HomeAssistantUser | None: """Find user object by the user's ID.""" try: - list_of_users = await self.sys_homeassistant.get_users() - except (HomeAssistantAPIError, TypeError) as err: - _LOGGER.error( - "%s error occurred while requesting list of users: %s", type(err), err - ) + users = await self.sys_homeassistant.list_users() + except HomeAssistantAPIError as err: + _LOGGER.warning("Could not fetch list of users: %s", err) return None - if list_of_users is not None: - self._list_of_users = list_of_users - - return next((user for user in self._list_of_users if user.id == user_id), None) + return next((user for user in users if user.id == user_id), None) def _init_header( @@ -332,8 +321,8 @@ def _init_header( headers[HEADER_REMOTE_USER_ID] = session_data.user.id if session_data.user.username is not None: headers[HEADER_REMOTE_USER_NAME] = session_data.user.username - if session_data.user.display_name is not None: - headers[HEADER_REMOTE_USER_DISPLAY_NAME] = session_data.user.display_name + if session_data.user.name is not None: + headers[HEADER_REMOTE_USER_DISPLAY_NAME] = session_data.user.name # filter flags for name, value in request.headers.items(): diff --git a/supervisor/auth.py b/supervisor/auth.py index 5116cc0bd..815e179dd 100644 --- a/supervisor/auth.py +++ b/supervisor/auth.py @@ -6,13 +6,12 @@ import logging from typing import Any, TypedDict, cast from .addons.addon import Addon -from .const import ATTR_PASSWORD, ATTR_TYPE, ATTR_USERNAME, FILE_HASSIO_AUTH +from .const import ATTR_PASSWORD, ATTR_USERNAME, FILE_HASSIO_AUTH, HomeAssistantUser from .coresys import CoreSys, CoreSysAttributes from .exceptions import ( AuthHomeAssistantAPIValidationError, AuthInvalidNonStringValueError, AuthListUsersError, - AuthListUsersNoneResponseError, AuthPasswordResetError, HomeAssistantAPIError, HomeAssistantWSError, @@ -157,22 +156,14 @@ class Auth(FileConfiguration, CoreSysAttributes): raise AuthPasswordResetError(user=username) - async def list_users(self) -> list[dict[str, Any]]: + async def list_users(self) -> list[HomeAssistantUser]: """List users on the Home Assistant instance.""" try: - users: ( - list[dict[str, Any]] | None - ) = await self.sys_homeassistant.websocket.async_send_command( - {ATTR_TYPE: "config/auth/list"} - ) + return await self.sys_homeassistant.list_users() except HomeAssistantWSError as err: _LOGGER.error("Can't request listing users on Home Assistant: %s", err) raise AuthListUsersError() from err - if users is not None: - return users - raise AuthListUsersNoneResponseError(_LOGGER.error) - @staticmethod def _rehash(value: str, salt2: str = "") -> str: """Rehash a value.""" diff --git a/supervisor/const.py b/supervisor/const.py index ba1f85104..7bd8a3c5b 100644 --- a/supervisor/const.py +++ b/supervisor/const.py @@ -1,11 +1,12 @@ """Constants file for Supervisor.""" +from collections.abc import Mapping from dataclasses import dataclass from enum import StrEnum from ipaddress import IPv4Network, IPv6Network from pathlib import Path from sys import version_info as systemversion -from typing import NotRequired, Self, TypedDict +from typing import Any, NotRequired, Self, TypedDict from aiohttp import __version__ as aiohttpversion @@ -536,60 +537,77 @@ class CpuArch(StrEnum): AMD64 = "amd64" -class IngressSessionDataUserDict(TypedDict): - """Response object for ingress session user.""" - - id: str - username: NotRequired[str | None] - # Name is an alias for displayname, only one should be used - displayname: NotRequired[str | None] - name: NotRequired[str | None] - - @dataclass -class IngressSessionDataUser: - """Format of an IngressSessionDataUser object.""" +class HomeAssistantUser: + """A Home Assistant Core user. + + Incomplete model — Core's User object has additional fields + (credentials, refresh_tokens, etc.) that are not represented here. + Only fields used by the Supervisor are included. + """ id: str - display_name: str | None = None username: str | None = None - - def to_dict(self) -> IngressSessionDataUserDict: - """Get dictionary representation.""" - return IngressSessionDataUserDict( - id=self.id, displayname=self.display_name, username=self.username - ) + name: str | None = None + is_owner: bool = False + is_active: bool = False + local_only: bool = False + system_generated: bool = False + group_ids: list[str] | None = None @classmethod - def from_dict(cls, data: IngressSessionDataUserDict) -> Self: + def from_dict(cls, data: Mapping[str, Any]) -> Self: """Return object from dictionary representation.""" return cls( id=data["id"], - display_name=data.get("displayname") or data.get("name"), username=data.get("username"), + # "displayname" is a legacy key from old ingress session data + name=data.get("name") or data.get("displayname"), + is_owner=data.get("is_owner", False), + is_active=data.get("is_active", False), + local_only=data.get("local_only", False), + system_generated=data.get("system_generated", False), + group_ids=data.get("group_ids"), ) +class IngressSessionDataUserDict(TypedDict): + """Serialization format for user data stored in ingress sessions. + + Legacy data may contain "displayname" instead of "name". + """ + + id: str + username: NotRequired[str | None] + name: NotRequired[str | None] + + class IngressSessionDataDict(TypedDict): - """Response object for ingress session data.""" + """Serialization format for ingress session data.""" user: IngressSessionDataUserDict @dataclass class IngressSessionData: - """Format of an IngressSessionData object.""" + """Ingress session data attached to a session token.""" - user: IngressSessionDataUser + user: HomeAssistantUser def to_dict(self) -> IngressSessionDataDict: """Get dictionary representation.""" - return IngressSessionDataDict(user=self.user.to_dict()) + return IngressSessionDataDict( + user=IngressSessionDataUserDict( + id=self.user.id, + name=self.user.name, + username=self.user.username, + ) + ) @classmethod - def from_dict(cls, data: IngressSessionDataDict) -> Self: + def from_dict(cls, data: Mapping[str, Any]) -> Self: """Return object from dictionary representation.""" - return cls(user=IngressSessionDataUser.from_dict(data["user"])) + return cls(user=HomeAssistantUser.from_dict(data["user"])) STARTING_STATES = [ diff --git a/supervisor/exceptions.py b/supervisor/exceptions.py index 7d3186488..d458da93b 100644 --- a/supervisor/exceptions.py +++ b/supervisor/exceptions.py @@ -620,18 +620,6 @@ class AuthListUsersError(AuthError, APIUnknownSupervisorError): message_template = "Can't request listing users on Home Assistant" -class AuthListUsersNoneResponseError(AuthError, APIInternalServerError): - """Auth error if listing users returned invalid None response.""" - - error_key = "auth_list_users_none_response_error" - message_template = "Home Assistant returned invalid response of `{none}` instead of a list of users. Check Home Assistant logs for details (check with `{logs_command}`)" - extra_fields = {"none": "None", "logs_command": "ha core logs"} - - def __init__(self, logger: Callable[..., None] | None = None) -> None: - """Initialize exception.""" - super().__init__(None, logger) - - class AuthInvalidNonStringValueError(AuthError, APIUnauthorized): """Auth error if something besides a string provided as username or password.""" diff --git a/supervisor/homeassistant/module.py b/supervisor/homeassistant/module.py index b8f51e205..6d6a0f6ad 100644 --- a/supervisor/homeassistant/module.py +++ b/supervisor/homeassistant/module.py @@ -1,7 +1,6 @@ """Home Assistant control object.""" import asyncio -from datetime import timedelta import errno from ipaddress import IPv4Address import logging @@ -35,8 +34,7 @@ from ..const import ( ATTR_WATCHDOG, FILE_HASSIO_HOMEASSISTANT, BusEvent, - IngressSessionDataUser, - IngressSessionDataUserDict, + HomeAssistantUser, ) from ..coresys import CoreSys, CoreSysAttributes from ..exceptions import ( @@ -47,7 +45,6 @@ from ..exceptions import ( ) from ..hardware.const import PolicyGroup from ..hardware.data import Device -from ..jobs.const import JobConcurrency, JobThrottle from ..jobs.decorator import Job from ..resolution.const import UnhealthyReason from ..utils import remove_folder, remove_folder_with_excludes @@ -570,21 +567,12 @@ class HomeAssistant(FileConfiguration, CoreSysAttributes): if attr in data: self._data[attr] = data[attr] - @Job( - name="home_assistant_get_users", - throttle_period=timedelta(minutes=5), - internal=True, - concurrency=JobConcurrency.QUEUE, - throttle=JobThrottle.THROTTLE, - ) - async def get_users(self) -> list[IngressSessionDataUser]: - """Get list of all configured users.""" - list_of_users: ( - list[IngressSessionDataUserDict] | None - ) = await self.sys_homeassistant.websocket.async_send_command( + async def list_users(self) -> list[HomeAssistantUser]: + """Fetch list of all users from Home Assistant Core via WebSocket. + + Raises HomeAssistantWSError on WebSocket connection/communication failure. + """ + raw: list[dict[str, Any]] = await self.websocket.async_send_command( {ATTR_TYPE: "config/auth/list"} ) - - if list_of_users: - return [IngressSessionDataUser.from_dict(data) for data in list_of_users] - return [] + return [HomeAssistantUser.from_dict(data) for data in raw] diff --git a/supervisor/homeassistant/websocket.py b/supervisor/homeassistant/websocket.py index 21b200e7e..eab4d5061 100644 --- a/supervisor/homeassistant/websocket.py +++ b/supervisor/homeassistant/websocket.py @@ -65,7 +65,7 @@ class WSClient: if not self._client.closed: await self._client.close() - async def async_send_command(self, message: dict[str, Any]) -> T | None: + async def async_send_command(self, message: dict[str, Any]) -> T: """Send a websocket message, and return the response.""" self._message_id += 1 message["id"] = self._message_id @@ -146,7 +146,7 @@ class WSClient: try: client = await session.ws_connect(url, ssl=False) except aiohttp.client_exceptions.ClientConnectorError: - raise HomeAssistantWSError("Can't connect") from None + raise HomeAssistantWSConnectionError("Can't connect") from None hello_message = await client.receive_json() @@ -200,10 +200,11 @@ class HomeAssistantWebSocket(CoreSysAttributes): async def _ensure_connected(self) -> None: """Ensure WebSocket connection is ready. - Raises HomeAssistantWSError if unable to connect. + Raises HomeAssistantWSConnectionError if unable to connect. + Raises HomeAssistantAuthError if authentication with Core fails. """ if self.sys_core.state in CLOSING_STATES: - raise HomeAssistantWSError( + raise HomeAssistantWSConnectionError( "WebSocket not available, system is shutting down" ) @@ -211,7 +212,7 @@ class HomeAssistantWebSocket(CoreSysAttributes): # If we are already connected, we can avoid the check_api_state call # since it makes a new socket connection and we already have one. if not connected and not await self.sys_homeassistant.api.check_api_state(): - raise HomeAssistantWSError( + raise HomeAssistantWSConnectionError( "Can't connect to Home Assistant Core WebSocket, the API is not reachable" ) @@ -251,10 +252,10 @@ class HomeAssistantWebSocket(CoreSysAttributes): await self._client.close() self._client = None - async def async_send_command(self, message: dict[str, Any]) -> T | None: + async def async_send_command(self, message: dict[str, Any]) -> T: """Send a command and return the response. - Raises HomeAssistantWSError if unable to connect to Home Assistant Core. + Raises HomeAssistantWSError on WebSocket connection or communication failure. """ await self._ensure_connected() # _ensure_connected guarantees self._client is set diff --git a/supervisor/validate.py b/supervisor/validate.py index 1030d3e06..ed41c4944 100644 --- a/supervisor/validate.py +++ b/supervisor/validate.py @@ -31,6 +31,7 @@ from .const import ( ATTR_LOGGING, ATTR_MTU, ATTR_MULTICAST, + ATTR_NAME, ATTR_OBSERVER, ATTR_OTA, ATTR_PASSWORD, @@ -206,7 +207,9 @@ SCHEMA_SESSION_DATA = vol.Schema( { vol.Required(ATTR_ID): str, vol.Required(ATTR_USERNAME, default=None): vol.Maybe(str), - vol.Required(ATTR_DISPLAYNAME, default=None): vol.Maybe(str), + vol.Required(ATTR_NAME, default=None): vol.Maybe(str), + # Legacy key, replaced by ATTR_NAME + vol.Optional(ATTR_DISPLAYNAME): vol.Maybe(str), } ) } diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py index e224e410f..7d83cf62a 100644 --- a/tests/api/test_auth.py +++ b/tests/api/test_auth.py @@ -1,7 +1,6 @@ """Test auth API.""" from datetime import UTC, datetime, timedelta -from typing import Any from unittest.mock import AsyncMock, MagicMock, patch from aiohttp.hdrs import WWW_AUTHENTICATE @@ -169,46 +168,25 @@ async def test_list_users( ] -@pytest.mark.parametrize( - ("send_command_mock", "error_response", "expected_log"), - [ - ( - AsyncMock(return_value=None), - { - "result": "error", - "message": "Home Assistant returned invalid response of `None` instead of a list of users. Check Home Assistant logs for details (check with `ha core logs`)", - "error_key": "auth_list_users_none_response_error", - "extra_fields": {"none": "None", "logs_command": "ha core logs"}, - }, - "Home Assistant returned invalid response of `None` instead of a list of users. Check Home Assistant logs for details (check with `ha core logs`)", - ), - ( - AsyncMock(side_effect=HomeAssistantWSError("fail")), - { - "result": "error", - "message": "Can't request listing users on Home Assistant. Check supervisor logs for details (check with 'ha supervisor logs')", - "error_key": "auth_list_users_error", - "extra_fields": {"logs_command": "ha supervisor logs"}, - }, - "Can't request listing users on Home Assistant: fail", - ), - ], -) -async def test_list_users_failure( +async def test_list_users_ws_error( api_client: TestClient, ha_ws_client: AsyncMock, caplog: pytest.LogCaptureFixture, - send_command_mock: AsyncMock, - error_response: dict[str, Any], - expected_log: str, ): - """Test failure listing users via API.""" - ha_ws_client.async_send_command = send_command_mock + """Test WS error when listing users via API.""" + ha_ws_client.async_send_command = AsyncMock( + side_effect=HomeAssistantWSError("fail") + ) resp = await api_client.get("/auth/list") assert resp.status == 500 result = await resp.json() - assert result == error_response - assert expected_log in caplog.text + assert result == { + "result": "error", + "message": "Can't request listing users on Home Assistant. Check supervisor logs for details (check with 'ha supervisor logs')", + "error_key": "auth_list_users_error", + "extra_fields": {"logs_command": "ha supervisor logs"}, + } + assert "Can't request listing users on Home Assistant: fail" in caplog.text @pytest.mark.parametrize( diff --git a/tests/api/test_ingress.py b/tests/api/test_ingress.py index f3007879e..8724ce0c8 100644 --- a/tests/api/test_ingress.py +++ b/tests/api/test_ingress.py @@ -99,9 +99,7 @@ async def test_validate_session_with_user_id( assert session in coresys.ingress.sessions_data assert coresys.ingress.get_session_data(session).user.id == "some-id" assert coresys.ingress.get_session_data(session).user.username == "sn" - assert ( - coresys.ingress.get_session_data(session).user.display_name == "Some Name" - ) + assert coresys.ingress.get_session_data(session).user.name == "Some Name" async def test_ingress_proxy_no_content_type_for_empty_body_responses( diff --git a/tests/homeassistant/test_module.py b/tests/homeassistant/test_module.py index 5459899e3..dec68cb87 100644 --- a/tests/homeassistant/test_module.py +++ b/tests/homeassistant/test_module.py @@ -58,12 +58,11 @@ async def test_load( assert ha_ws_client.async_send_command.call_args_list[0][0][0] == {"lorem": "ipsum"} -async def test_get_users_none(coresys: CoreSys, ha_ws_client: AsyncMock): - """Test get users returning none does not fail.""" +async def test_list_users_none(coresys: CoreSys, ha_ws_client: AsyncMock): + """Test list users raises on unexpected None response from Core.""" ha_ws_client.async_send_command.return_value = None - assert ( - await coresys.homeassistant.get_users.__wrapped__(coresys.homeassistant) == [] - ) + with pytest.raises(TypeError): + await coresys.homeassistant.list_users() async def test_write_pulse_error(coresys: CoreSys, caplog: pytest.LogCaptureFixture): diff --git a/tests/homeassistant/test_websocket.py b/tests/homeassistant/test_websocket.py index 2fc2d6c04..acc582095 100644 --- a/tests/homeassistant/test_websocket.py +++ b/tests/homeassistant/test_websocket.py @@ -8,7 +8,7 @@ import pytest from supervisor.const import CoreState from supervisor.coresys import CoreSys -from supervisor.exceptions import HomeAssistantWSError +from supervisor.exceptions import HomeAssistantWSConnectionError from supervisor.homeassistant.const import WSEvent, WSType @@ -81,7 +81,7 @@ async def test_send_command_core_not_reachable( ha_ws_client.connected = False with ( patch.object(coresys.homeassistant.api, "check_api_state", return_value=False), - pytest.raises(HomeAssistantWSError, match="not reachable"), + pytest.raises(HomeAssistantWSConnectionError, match="not reachable"), ): await coresys.homeassistant.websocket.async_send_command({"type": "test"}) @@ -102,7 +102,7 @@ async def test_fire_and_forget_core_not_reachable( async def test_send_command_during_shutdown(coresys: CoreSys, ha_ws_client: AsyncMock): """Test async_send_command raises during shutdown.""" await coresys.core.set_state(CoreState.SHUTDOWN) - with pytest.raises(HomeAssistantWSError, match="shutting down"): + with pytest.raises(HomeAssistantWSConnectionError, match="shutting down"): await coresys.homeassistant.websocket.async_send_command({"type": "test"}) ha_ws_client.async_send_command.assert_not_called() diff --git a/tests/test_ingress.py b/tests/test_ingress.py index bd4ca331a..3bc9eccd5 100644 --- a/tests/test_ingress.py +++ b/tests/test_ingress.py @@ -1,10 +1,11 @@ """Test ingress.""" from datetime import timedelta +import json from pathlib import Path from unittest.mock import ANY, patch -from supervisor.const import IngressSessionData, IngressSessionDataUser +from supervisor.const import HomeAssistantUser, IngressSessionData from supervisor.coresys import CoreSys from supervisor.ingress import Ingress from supervisor.utils.dt import utc_from_timestamp @@ -34,7 +35,7 @@ def test_session_handling(coresys: CoreSys): def test_session_handling_with_session_data(coresys: CoreSys): """Create and test session.""" session = coresys.ingress.create_session( - IngressSessionData(IngressSessionDataUser("some-id")) + IngressSessionData(HomeAssistantUser("some-id")) ) assert session @@ -76,7 +77,7 @@ async def test_ingress_save_data(coresys: CoreSys, tmp_supervisor_data: Path): with patch("supervisor.ingress.FILE_HASSIO_INGRESS", new=config_file): ingress = await Ingress(coresys).load_config() session = ingress.create_session( - IngressSessionData(IngressSessionDataUser("123", "Test", "test")) + IngressSessionData(HomeAssistantUser("123", name="Test", username="test")) ) await ingress.save_data() @@ -87,12 +88,47 @@ async def test_ingress_save_data(coresys: CoreSys, tmp_supervisor_data: Path): assert await coresys.run_in_executor(get_config) == { "session": {session: ANY}, "session_data": { - session: {"user": {"id": "123", "displayname": "Test", "username": "test"}} + session: {"user": {"id": "123", "name": "Test", "username": "test"}} }, "ports": {}, } +async def test_ingress_load_legacy_displayname( + coresys: CoreSys, tmp_supervisor_data: Path +): + """Test loading session data with legacy 'displayname' key.""" + config_file = tmp_supervisor_data / "ingress.json" + session_token = "a" * 128 + + config_file.write_text( + json.dumps( + { + "session": {session_token: 9999999999.0}, + "session_data": { + session_token: { + "user": { + "id": "456", + "displayname": "Legacy Name", + "username": "legacy", + } + } + }, + "ports": {}, + } + ) + ) + + with patch("supervisor.ingress.FILE_HASSIO_INGRESS", new=config_file): + ingress = await Ingress(coresys).load_config() + + session_data = ingress.get_session_data(session_token) + assert session_data is not None + assert session_data.user.id == "456" + assert session_data.user.name == "Legacy Name" + assert session_data.user.username == "legacy" + + async def test_ingress_reload_ignore_none_data(coresys: CoreSys): """Test reloading ingress does not add None for session data and create errors.""" session = coresys.ingress.create_session()