Home-Assistant/supervisor
1
0
Fork 0
mirror of https://github.com/home-assistant/supervisor.git synced 2026-04-02 00:07:16 +01:00
Code Activity

Unify Core user handling with HomeAssistantUser model (#6558)

Browse Source 
* Unify Core user listing with HomeAssistantUser model

Replace the ingress-specific IngressSessionDataUser with a general
HomeAssistantUser dataclass that models the Core config/auth/list WS
response. This deduplicates the WS call (previously in both auth.py
and module.py) into a single HomeAssistant.list_users() method.

- Add HomeAssistantUser dataclass with fields matching Core's user API
- Remove get_users() and its unnecessary 5-minute Job throttle
- Auth and ingress consumers both use HomeAssistant.list_users()
- Auth API endpoint uses typed attribute access instead of dict keys
- Migrate session serialization from legacy "displayname" to "name"
- Accept both keys in schema/deserialization for backwards compat
- Add test for loading persisted sessions with legacy displayname key

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Tighten list_users() to trust Core's auth/list contract

Core's config/auth/list WS command always returns a list, never None.
Replace the silent `if not raw: return []` (which also swallowed empty
lists) with an assert, remove the dead AuthListUsersNoneResponseError
exception class, and document the HomeAssistantWSError contract in the
docstring.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Remove | None from async_send_command return type

The WebSocket result is always set from data["result"] in _receive_json,
never explicitly to None. Remove the misleading | None from the return
type of both WSClient and HomeAssistantWebSocket async_send_command, and
drop the now-unnecessary assert in list_users.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Use HomeAssistantWSConnectionError in _ensure_connected

_ensure_connected and connect_with_auth raise on connection-level
failures, so use the more specific HomeAssistantWSConnectionError
instead of the broad HomeAssistantWSError. This allows callers to
distinguish connection errors from Core API errors (e.g. unsuccessful
WebSocket command responses). Also document that _ensure_connected can
propagate HomeAssistantAuthError from ensure_access_token.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Remove user list cache from _find_user_by_id

Drop the _list_of_users cache to avoid stale auth data in ingress
session creation. The method now fetches users fresh each time and
returns None on any API error instead of serving potentially outdated
cached results.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Stefan Agner
2026-02-17 18:31:08 +01:00
committed by GitHub
parent 09a4e9d5a2
commit 3147d080a2
13 changed files with 144 additions and 155 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
supervisor/api/auth.py
View File

@@ -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
]
}

27
supervisor/api/ingress.py
View File

@@ -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():

15
supervisor/auth.py
View File

@@ -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."""

74
supervisor/const.py
View File

@@ -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 = [

12
supervisor/exceptions.py
View File

@@ -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."""

28
supervisor/homeassistant/module.py
View File

@@ -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]

15
supervisor/homeassistant/websocket.py
View File

@@ -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

5
supervisor/validate.py
View File

@@ -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),
}
)
}

46
tests/api/test_auth.py
View File

@@ -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(

4
tests/api/test_ingress.py
View File

@@ -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(

9
tests/homeassistant/test_module.py
View File

@@ -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):

6
tests/homeassistant/test_websocket.py
View File

@@ -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()

44
tests/test_ingress.py
View File

@@ -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()