Return 401 Unauthorized when using json/url encoded auth fails (#5844)

When authentication using JSON payload or URL encoded payload fails,
use the generic HTTP response code 401 Unauthorized instead of 400
Bad Request.

This is a more appropriate response code for authentication errors
and is consistent with the behavior of other authentication methods.

Author: agners

supervisor/api/auth.py

@@ -92,13 +92,18 @@ async def auth(self, request: web.Request) -> bool:
         # Json
         if request.headers.get(CONTENT_TYPE) == CONTENT_TYPE_JSON:
             data = await request.json(loads=json_loads)
-            return await self._process_dict(request, addon, data)
+            if not await self._process_dict(request, addon, data):
+                raise HTTPUnauthorized()
+            return True
 
         # URL encoded
         if request.headers.get(CONTENT_TYPE) == CONTENT_TYPE_URL:
             data = await request.post()
-            return await self._process_dict(request, addon, data)
+            if not await self._process_dict(request, addon, data):
+                raise HTTPUnauthorized()
+            return True
 
+        # Advertise Basic authentication by default
         raise HTTPUnauthorized(headers=REALM_HEADER)
 
     @api_process

tests/api/test_auth.py

@@ -3,6 +3,7 @@
 from datetime import UTC, datetime, timedelta
 from unittest.mock import AsyncMock, MagicMock, patch
 
+from aiohttp.hdrs import WWW_AUTHENTICATE
 from aiohttp.test_utils import TestClient
 import pytest
 
@@ -166,8 +167,8 @@ async def test_auth_json_invalid_credentials(
     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
+    assert WWW_AUTHENTICATE not in resp.headers
+    assert resp.status == 401
 
 
 @pytest.mark.parametrize("api_client", [TEST_ADDON_SLUG], indirect=True)
@@ -213,8 +214,8 @@ async def test_auth_urlencoded_failure(
         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
+    assert WWW_AUTHENTICATE not in resp.headers
+    assert resp.status == 401
 
 
 @pytest.mark.parametrize("api_client", [TEST_ADDON_SLUG], indirect=True)
@@ -225,7 +226,7 @@ async def test_auth_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 "Basic realm" in resp.headers[WWW_AUTHENTICATE]
     assert resp.status == 401