From 9470f44840104eb48c8d1c451abc77d31614b6c8 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Fri, 25 Apr 2025 15:17:25 +0200 Subject: [PATCH] Improve /auth API request sanitation (#5843) * Add basic test coverage for /auth API * Check /auth API is called from an add-on Currently the /auth API is only available for add-ons. Return 403 for calls not originating from an add-on. * Handle bad json in auth API Use the API specific JSON load helper which raises an APIError. This causes the API to return a 400 error instead of a 500 error when the JSON is invalid. * Avoid redefining name 'mock_check_login' * Update tests/api/test_auth.py --- supervisor/api/auth.py | 5 +- tests/api/test_auth.py | 131 ++++++++++++++++++++ tests/fixtures/addons/local/ssh/config.yaml | 1 + 3 files changed, 134 insertions(+), 3 deletions(-) diff --git a/supervisor/api/auth.py b/supervisor/api/auth.py index 2d0dd46b2..313bcb659 100644 --- a/supervisor/api/auth.py +++ b/supervisor/api/auth.py @@ -14,7 +14,6 @@ from ..addons.addon import Addon from ..const import ATTR_NAME, ATTR_PASSWORD, ATTR_USERNAME, REQUEST_FROM from ..coresys import CoreSysAttributes from ..exceptions import APIForbidden -from ..utils.json import json_loads from .const import ( ATTR_GROUP_IDS, ATTR_IS_ACTIVE, @@ -24,7 +23,7 @@ from .const import ( CONTENT_TYPE_JSON, CONTENT_TYPE_URL, ) -from .utils import api_process, api_validate +from .utils import api_process, api_validate, json_loads _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -68,7 +67,7 @@ class APIAuth(CoreSysAttributes): """Process login request.""" addon = request[REQUEST_FROM] - if not addon.access_auth_api: + if not isinstance(addon, Addon) or not addon.access_auth_api: raise APIForbidden("Can't use Home Assistant auth!") # BasicAuth diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py index 6acbb12d8..8b651067e 100644 --- a/tests/api/test_auth.py +++ b/tests/api/test_auth.py @@ -6,8 +6,11 @@ from unittest.mock import AsyncMock, patch from aiohttp.test_utils import TestClient import pytest +from supervisor.addons.addon import Addon from supervisor.coresys import CoreSys +from tests.const import TEST_ADDON_SLUG + LIST_USERS_RESPONSE = [ { "id": "a1d90e114a3b4da4a487fe327918dcef", @@ -67,6 +70,13 @@ LIST_USERS_RESPONSE = [ ] +@pytest.fixture(name="mock_check_login") +def fixture_mock_check_login(coresys: CoreSys): + """Patch sys_auth.check_login.""" + with patch.object(coresys.auth, "check_login", new_callable=AsyncMock) as mock: + yield mock + + async def test_password_reset( api_client: TestClient, coresys: CoreSys, caplog: pytest.LogCaptureFixture ): @@ -106,3 +116,124 @@ async def test_list_users( "group_ids": ["system-admin"], }, ] + + +@pytest.mark.parametrize("api_client", [TEST_ADDON_SLUG], indirect=True) +async def test_auth_json_success( + api_client: TestClient, mock_check_login: AsyncMock, install_addon_ssh: Addon +): + """Test successful JSON auth.""" + mock_check_login.return_value = True + resp = await api_client.post("/auth", json={"username": "test", "password": "pass"}) + assert resp.status == 200 + + +@pytest.mark.parametrize("api_client", [TEST_ADDON_SLUG], indirect=True) +async def test_auth_json_invalid_credentials( + api_client: TestClient, mock_check_login: AsyncMock, install_addon_ssh: Addon +): + """Test failed JSON auth due to invalid credentials.""" + mock_check_login.return_value = False + resp = await api_client.post( + "/auth", json={"username": "test", "password": "wrong"} + ) + # Do we really want the API to return 400 here? + assert resp.status == 400 + + +@pytest.mark.parametrize("api_client", [TEST_ADDON_SLUG], indirect=True) +async def test_auth_json_empty_body(api_client: TestClient, install_addon_ssh: Addon): + """Test JSON auth with empty body.""" + resp = await api_client.post( + "/auth", data="", headers={"Content-Type": "application/json"} + ) + assert resp.status == 400 + + +@pytest.mark.parametrize("api_client", [TEST_ADDON_SLUG], indirect=True) +async def test_auth_json_invalid_json(api_client: TestClient, install_addon_ssh: Addon): + """Test JSON auth with malformed JSON.""" + resp = await api_client.post( + "/auth", data="{not json}", headers={"Content-Type": "application/json"} + ) + assert resp.status == 400 + + +@pytest.mark.parametrize("api_client", [TEST_ADDON_SLUG], indirect=True) +async def test_auth_urlencoded_success( + api_client: TestClient, mock_check_login: AsyncMock, install_addon_ssh: Addon +): + """Test successful URL-encoded auth.""" + mock_check_login.return_value = True + resp = await api_client.post( + "/auth", + data="username=test&password=pass", + headers={"Content-Type": "application/x-www-form-urlencoded"}, + ) + assert resp.status == 200 + + +@pytest.mark.parametrize("api_client", [TEST_ADDON_SLUG], indirect=True) +async def test_auth_urlencoded_failure( + api_client: TestClient, mock_check_login: AsyncMock, install_addon_ssh: Addon +): + """Test URL-encoded auth with invalid credentials.""" + mock_check_login.return_value = False + resp = await api_client.post( + "/auth", + data="username=test&password=fail", + headers={"Content-Type": "application/x-www-form-urlencoded"}, + ) + # Do we really want the API to return 400 here? + assert resp.status == 400 + + +@pytest.mark.parametrize("api_client", [TEST_ADDON_SLUG], indirect=True) +async def test_auth_unsupported_content_type( + api_client: TestClient, install_addon_ssh: Addon +): + """Test auth with unsupported content type.""" + resp = await api_client.post( + "/auth", data="something", headers={"Content-Type": "text/plain"} + ) + # This probably should be 400 here for better consistency + assert resp.status == 401 + + +@pytest.mark.parametrize("api_client", [TEST_ADDON_SLUG], indirect=True) +async def test_auth_basic_auth( + api_client: TestClient, mock_check_login: AsyncMock, install_addon_ssh: Addon +): + """Test auth with BasicAuth header.""" + mock_check_login.return_value = True + resp = await api_client.post( + "/auth", headers={"Authorization": "Basic dGVzdDpwYXNz"} + ) + assert resp.status == 200 + + +@pytest.mark.parametrize("api_client", [TEST_ADDON_SLUG], indirect=True) +async def test_auth_basic_auth_failure( + api_client: TestClient, mock_check_login: AsyncMock, install_addon_ssh: Addon +): + """Test auth with BasicAuth header and failure.""" + mock_check_login.return_value = False + resp = await api_client.post( + "/auth", headers={"Authorization": "Basic dGVzdDpwYXNz"} + ) + assert resp.status == 401 + + +@pytest.mark.parametrize("api_client", ["local_example"], indirect=True) +async def test_auth_addon_no_auth_access( + api_client: TestClient, install_addon_example: Addon +): + """Test auth where add-on is not allowed to access auth API.""" + resp = await api_client.post("/auth", json={"username": "test", "password": "pass"}) + assert resp.status == 403 + + +async def test_non_addon_token_no_auth_access(api_client: TestClient): + """Test auth where add-on is not allowed to access auth API.""" + resp = await api_client.post("/auth", json={"username": "test", "password": "pass"}) + assert resp.status == 403 diff --git a/tests/fixtures/addons/local/ssh/config.yaml b/tests/fixtures/addons/local/ssh/config.yaml index 38dac441c..1d001ebd2 100644 --- a/tests/fixtures/addons/local/ssh/config.yaml +++ b/tests/fixtures/addons/local/ssh/config.yaml @@ -18,6 +18,7 @@ panel_title: Terminal hassio_api: true hassio_role: manager homeassistant_api: true +auth_api: true audio: true uart: true ports: