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

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

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

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: