From 7a4a282405cc748ae78224fc06892f7ee8a21353 Mon Sep 17 00:00:00 2001 From: Mike Degatano Date: Wed, 11 Dec 2024 19:24:28 +0000 Subject: [PATCH] Return 404 for all APIs when resource not found --- supervisor/api/discovery.py | 4 +- supervisor/api/docker.py | 4 ++ supervisor/api/mounts.py | 27 ++++++++------ supervisor/api/store.py | 32 +++++++--------- tests/api/test_addons.py | 2 +- tests/api/test_discovery.py | 12 ++++++ tests/api/test_docker.py | 11 +++++- tests/api/test_jobs.py | 8 ++-- tests/api/test_network.py | 4 +- tests/api/test_resolution.py | 12 +++--- tests/api/test_services.py | 10 ++--- tests/api/test_store.py | 72 ++++++++++++++++++++++++++++++++++++ 12 files changed, 148 insertions(+), 50 deletions(-) diff --git a/supervisor/api/discovery.py b/supervisor/api/discovery.py index 26cbc9bd68c..c3a994db59a 100644 --- a/supervisor/api/discovery.py +++ b/supervisor/api/discovery.py @@ -16,7 +16,7 @@ AddonState, ) from ..coresys import CoreSysAttributes -from ..exceptions import APIError, APIForbidden +from ..exceptions import APIForbidden, APINotFound from .utils import api_process, api_validate, require_home_assistant _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -36,7 +36,7 @@ def _extract_message(self, request): """Extract discovery message from URL.""" message = self.sys_discovery.get(request.match_info.get("uuid")) if not message: - raise APIError("Discovery message not found") + raise APINotFound("Discovery message not found") return message @api_process diff --git a/supervisor/api/docker.py b/supervisor/api/docker.py index 642ad1e468b..dfa6c849e9c 100644 --- a/supervisor/api/docker.py +++ b/supervisor/api/docker.py @@ -16,6 +16,7 @@ ATTR_VERSION, ) from ..coresys import CoreSysAttributes +from ..exceptions import APINotFound from .utils import api_process, api_validate _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -58,6 +59,9 @@ async def create_registry(self, request: web.Request): async def remove_registry(self, request: web.Request): """Delete a docker registry.""" hostname = request.match_info.get(ATTR_HOSTNAME) + if hostname not in self.sys_docker.config.registries: + raise APINotFound(f"Hostname {hostname} does not exist in registries") + del self.sys_docker.config.registries[hostname] self.sys_docker.config.save_data() diff --git a/supervisor/api/mounts.py b/supervisor/api/mounts.py index b00f15781f0..8567580f8e0 100644 --- a/supervisor/api/mounts.py +++ b/supervisor/api/mounts.py @@ -7,7 +7,7 @@ from ..const import ATTR_NAME, ATTR_STATE from ..coresys import CoreSysAttributes -from ..exceptions import APIError +from ..exceptions import APIError, APINotFound from ..mounts.const import ATTR_DEFAULT_BACKUP_MOUNT, MountUsage from ..mounts.mount import Mount from ..mounts.validate import SCHEMA_MOUNT_CONFIG @@ -24,6 +24,13 @@ class APIMounts(CoreSysAttributes): """Handle REST API for mounting options.""" + def _extract_mount(self, request: web.Request) -> Mount: + """Extract mount from request or raise.""" + name = request.match_info.get("mount") + if name not in self.sys_mounts: + raise APINotFound(f"No mount exists with name {name}") + return self.sys_mounts.get(name) + @api_process async def info(self, request: web.Request) -> dict[str, Any]: """Return MountManager info.""" @@ -85,15 +92,13 @@ async def create_mount(self, request: web.Request) -> None: @api_process async def update_mount(self, request: web.Request) -> None: """Update an existing mount in supervisor.""" - name = request.match_info.get("mount") + current = self._extract_mount(request) name_schema = vol.Schema( - {vol.Optional(ATTR_NAME, default=name): name}, extra=vol.ALLOW_EXTRA + {vol.Optional(ATTR_NAME, default=current.name): current.name}, + extra=vol.ALLOW_EXTRA, ) body = await api_validate(vol.All(name_schema, SCHEMA_MOUNT_CONFIG), request) - if name not in self.sys_mounts: - raise APIError(f"No mount exists with name {name}") - mount = Mount.from_dict(self.coresys, body) await self.sys_mounts.create_mount(mount) @@ -110,8 +115,8 @@ async def update_mount(self, request: web.Request) -> None: @api_process async def delete_mount(self, request: web.Request) -> None: """Delete an existing mount in supervisor.""" - name = request.match_info.get("mount") - mount = await self.sys_mounts.remove_mount(name) + current = self._extract_mount(request) + mount = await self.sys_mounts.remove_mount(current.name) # If it was a backup mount, reload backups if mount.usage == MountUsage.BACKUP: @@ -122,9 +127,9 @@ async def delete_mount(self, request: web.Request) -> None: @api_process async def reload_mount(self, request: web.Request) -> None: """Reload an existing mount in supervisor.""" - name = request.match_info.get("mount") - await self.sys_mounts.reload_mount(name) + mount = self._extract_mount(request) + await self.sys_mounts.reload_mount(mount.name) # If it's a backup mount, reload backups - if self.sys_mounts.get(name).usage == MountUsage.BACKUP: + if mount.usage == MountUsage.BACKUP: self.sys_create_task(self.sys_backups.reload()) diff --git a/supervisor/api/store.py b/supervisor/api/store.py index 4dc4e2747bf..29792eaa113 100644 --- a/supervisor/api/store.py +++ b/supervisor/api/store.py @@ -51,7 +51,7 @@ REQUEST_FROM, ) from ..coresys import CoreSysAttributes -from ..exceptions import APIError, APIForbidden +from ..exceptions import APIError, APIForbidden, APINotFound from ..store.addon import AddonStore from ..store.repository import Repository from ..store.validate import validate_repository @@ -74,31 +74,27 @@ class APIStore(CoreSysAttributes): def _extract_addon(self, request: web.Request, installed=False) -> AnyAddon: """Return add-on, throw an exception it it doesn't exist.""" addon_slug: str = request.match_info.get("addon") - addon_version: str = request.match_info.get("version", "latest") - - if installed: - addon = self.sys_addons.local.get(addon_slug) - if addon is None or not addon.is_installed: - raise APIError(f"Addon {addon_slug} is not installed") - else: - addon = self.sys_addons.store.get(addon_slug) - - if not addon: - raise APIError( - f"Addon {addon_slug} with version {addon_version} does not exist in the store" - ) + if not (addon := self.sys_addons.get(addon_slug)): + raise APINotFound(f"Addon {addon_slug} does not exist in the store") + + if installed and not addon.is_installed: + raise APIError(f"Addon {addon_slug} is not installed") + + if not installed: + return addon.store_addon return addon def _extract_repository(self, request: web.Request) -> Repository: """Return repository, throw an exception it it doesn't exist.""" repository_slug: str = request.match_info.get("repository") - repository = self.sys_store.get(repository_slug) - if not repository: - raise APIError(f"Repository {repository_slug} does not exist in the store") + if repository_slug not in self.sys_store.repositories: + raise APINotFound( + f"Repository {repository_slug} does not exist in the store" + ) - return repository + return self.sys_store.get(repository_slug) def _generate_addon_information( self, addon: AddonStore, extended: bool = False diff --git a/tests/api/test_addons.py b/tests/api/test_addons.py index 70c048e140a..b27e40dca88 100644 --- a/tests/api/test_addons.py +++ b/tests/api/test_addons.py @@ -83,7 +83,7 @@ async def test_api_addon_logs_not_installed(api_client: TestClient): """Test error is returned for non-existing add-on.""" resp = await api_client.get("/addons/hic_sunt_leones/logs") - assert resp.status == 400 + assert resp.status == 404 assert resp.content_type == "text/plain" content = await resp.text() assert content == "Addon hic_sunt_leones does not exist" diff --git a/tests/api/test_discovery.py b/tests/api/test_discovery.py index 01295859b24..32e83ff83bc 100644 --- a/tests/api/test_discovery.py +++ b/tests/api/test_discovery.py @@ -138,3 +138,15 @@ async def test_api_invalid_discovery(api_client: TestClient, install_addon_ssh: resp = await api_client.post("/discovery", json={"service": "test", "config": None}) assert resp.status == 400 + + +@pytest.mark.parametrize( + ("method", "url"), + [("get", "/discovery/bad"), ("delete", "/discovery/bad")], +) +async def test_discovery_not_found(api_client: TestClient, method: str, url: str): + """Test discovery not found error.""" + resp = await api_client.request(method, url) + assert resp.status == 404 + resp = await resp.json() + assert resp["message"] == "Discovery message not found" diff --git a/tests/api/test_docker.py b/tests/api/test_docker.py index 040a958d46e..1cf6ec1c99a 100644 --- a/tests/api/test_docker.py +++ b/tests/api/test_docker.py @@ -1,10 +1,11 @@ """Test Docker API.""" +from aiohttp.test_utils import TestClient import pytest @pytest.mark.asyncio -async def test_api_docker_info(api_client): +async def test_api_docker_info(api_client: TestClient): """Test docker info api.""" resp = await api_client.get("/docker/info") result = await resp.json() @@ -12,3 +13,11 @@ async def test_api_docker_info(api_client): assert result["data"]["logging"] == "journald" assert result["data"]["storage"] == "overlay2" assert result["data"]["version"] == "1.0.0" + + +async def test_registry_not_found(api_client: TestClient): + """Test registry not found error.""" + resp = await api_client.delete("/docker/registries/bad") + assert resp.status == 404 + body = await resp.json() + assert body["message"] == "Hostname bad does not exist in registries" diff --git a/tests/api/test_jobs.py b/tests/api/test_jobs.py index 02aacf82e2b..298f350d911 100644 --- a/tests/api/test_jobs.py +++ b/tests/api/test_jobs.py @@ -214,18 +214,18 @@ async def test_job_manual_cleanup(self) -> None: # Confirm it no longer exists resp = await api_client.get(f"/jobs/{test.job_id}") - assert resp.status == 400 + assert resp.status == 404 result = await resp.json() assert result["message"] == f"No job found with id {test.job_id}" @pytest.mark.parametrize( ("method", "url"), - [("get", "/services/bad"), ("post", "/services/bad"), ("delete", "/services/bad")], + [("get", "/jobs/bad"), ("delete", "/jobs/bad")], ) async def test_job_not_found(api_client: TestClient, method: str, url: str): """Test job not found error.""" resp = await api_client.request(method, url) assert resp.status == 404 - resp = await resp.json() - assert resp["message"] == "Service does not exist" + body = await resp.json() + assert body["message"] == "Job does not exist" diff --git a/tests/api/test_network.py b/tests/api/test_network.py index 1a67a8d4712..e609df410b4 100644 --- a/tests/api/test_network.py +++ b/tests/api/test_network.py @@ -417,5 +417,5 @@ async def test_network_interface_not_found( """Test network interface not found error.""" resp = await api_client.request(method, url) assert resp.status == 404 - resp = await resp.json() - assert resp["message"] == "Interface bad does not exist" + body = await resp.json() + assert body["message"] == "Interface bad does not exist" diff --git a/tests/api/test_resolution.py b/tests/api/test_resolution.py index 0358dc576ee..b3fc68af67a 100644 --- a/tests/api/test_resolution.py +++ b/tests/api/test_resolution.py @@ -182,8 +182,8 @@ async def test_issue_not_found(api_client: TestClient, method: str, url: str): """Test issue not found error.""" resp = await api_client.request(method, url) assert resp.status == 404 - resp = await resp.json() - assert resp["message"] == "The supplied UUID is not a valid issue" + body = await resp.json() + assert body["message"] == "The supplied UUID is not a valid issue" @pytest.mark.parametrize( @@ -194,8 +194,8 @@ async def test_suggestion_not_found(api_client: TestClient, method: str, url: st """Test suggestion not found error.""" resp = await api_client.request(method, url) assert resp.status == 404 - resp = await resp.json() - assert resp["message"] == "The supplied UUID is not a valid suggestion" + body = await resp.json() + assert body["message"] == "The supplied UUID is not a valid suggestion" @pytest.mark.parametrize( @@ -206,5 +206,5 @@ async def test_check_not_found(api_client: TestClient, method: str, url: str): """Test check not found error.""" resp = await api_client.request(method, url) assert resp.status == 404 - resp = await resp.json() - assert resp["message"] == "The supplied check slug is not available" + body = await resp.json() + assert body["message"] == "The supplied check slug is not available" diff --git a/tests/api/test_services.py b/tests/api/test_services.py index 01788291b2b..d2652afc6fe 100644 --- a/tests/api/test_services.py +++ b/tests/api/test_services.py @@ -6,11 +6,11 @@ @pytest.mark.parametrize( ("method", "url"), - [("get", "/jobs/bad"), ("delete", "/jobs/bad")], + [("get", "/services/bad"), ("post", "/services/bad"), ("delete", "/services/bad")], ) -async def test_job_not_found(api_client: TestClient, method: str, url: str): - """Test job not found error.""" +async def test_service_not_found(api_client: TestClient, method: str, url: str): + """Test service not found error.""" resp = await api_client.request(method, url) assert resp.status == 404 - resp = await resp.json() - assert resp["message"] == "Job does not exist" + body = await resp.json() + assert body["message"] == "Service does not exist" diff --git a/tests/api/test_store.py b/tests/api/test_store.py index f75d7237393..f571c2ad8fb 100644 --- a/tests/api/test_store.py +++ b/tests/api/test_store.py @@ -4,6 +4,7 @@ from pathlib import Path from unittest.mock import MagicMock, PropertyMock, patch +from aiohttp import ClientResponse from aiohttp.test_utils import TestClient import pytest @@ -280,3 +281,74 @@ async def test_api_detached_addon_documentation( assert resp.status == 200 result = await resp.text() assert result == "Addon local_ssh with version latest does not exist in the store" + + +async def get_message(resp: ClientResponse, json_expected: bool) -> str: + """Get message from response based on response type.""" + if json_expected: + body = await resp.json() + return body["message"] + return await resp.text() + + +@pytest.mark.parametrize( + ("method", "url", "json_expected"), + [ + ("get", "/store/addons/bad", True), + ("get", "/store/addons/bad/1", True), + ("get", "/store/addons/bad/icon", False), + ("get", "/store/addons/bad/logo", False), + ("post", "/store/addons/bad/install", True), + ("post", "/store/addons/bad/install/1", True), + ("post", "/store/addons/bad/update", True), + ("post", "/store/addons/bad/update/1", True), + # Legacy paths + ("get", "/addons/bad/icon", False), + ("get", "/addons/bad/logo", False), + ("post", "/addons/bad/install", True), + ("post", "/addons/bad/update", True), + ], +) +async def test_store_addon_not_found( + api_client: TestClient, method: str, url: str, json_expected: bool +): + """Test store addon not found error.""" + resp = await api_client.request(method, url) + assert resp.status == 404 + assert ( + await get_message(resp, json_expected) + == "Addon bad does not exist in the store" + ) + + +@pytest.mark.parametrize( + ("method", "url"), + [ + ("post", "/store/addons/local_ssh/update"), + ("post", "/store/addons/local_ssh/update/1"), + # Legacy paths + ("post", "/addons/local_ssh/update"), + ], +) +@pytest.mark.usefixtures("repository") +async def test_store_addon_not_installed(api_client: TestClient, method: str, url: str): + """Test store addon not installed error.""" + resp = await api_client.request(method, url) + assert resp.status == 400 + body = await resp.json() + assert body["message"] == "Addon local_ssh is not installed" + + +@pytest.mark.parametrize( + ("method", "url"), + [ + ("get", "/store/repositories/bad"), + ("delete", "/store/repositories/bad"), + ], +) +async def test_repository_not_found(api_client: TestClient, method: str, url: str): + """Test repository not found error.""" + resp = await api_client.request(method, url) + assert resp.status == 404 + body = await resp.json() + assert body["message"] == "Repository bad does not exist in the store"