diff --git a/components/api_server/src/healthcheck.py b/components/api_server/src/healthcheck.py index 04a9a93548..0f7df23cc8 100644 --- a/components/api_server/src/healthcheck.py +++ b/components/api_server/src/healthcheck.py @@ -8,7 +8,7 @@ api_server_port = getenv("API_SERVER_PORT", "5001") exit_code = 1 try: - with urlopen(f"http://localhost:{api_server_port}/api/health") as response: # nosec B310 - exit_code = 0 if response.status == HTTPStatus.OK else 1 + with urlopen(f"http://localhost:{api_server_port}/api/internal/health") as response: # nosec B310 + exit_code = 0 if response.status == HTTPStatus.OK and response.read() == '{"healthy": true}' else 1 finally: sys.exit(exit_code) diff --git a/components/api_server/src/routes/__init__.py b/components/api_server/src/routes/__init__.py index c20e052e08..ae4a9181c4 100644 --- a/components/api_server/src/routes/__init__.py +++ b/components/api_server/src/routes/__init__.py @@ -9,7 +9,8 @@ get_report_changelog, ) from .datamodel import get_data_model -from .documentation import get_api +from .documentation import get_api_docs +from .health import get_health from .logo import get_logo from .measurement import get_metric_measurements, get_measurements, set_entity_attribute, stream_nr_measurements from .metric import ( diff --git a/components/api_server/src/routes/documentation.py b/components/api_server/src/routes/documentation.py index e1ec34f9f5..b25d9e1457 100644 --- a/components/api_server/src/routes/documentation.py +++ b/components/api_server/src/routes/documentation.py @@ -6,17 +6,18 @@ @bottle.get("/api", authentication_required=False) -@bottle.get("/api/", authentication_required=False) -@bottle.get("/api//", authentication_required=False) -def get_api(version="", fragment=""): - """Return the API. Use version and/or fragment to limit the routes returned.""" +@bottle.get("/api/", authentication_required=False) +def get_api_docs() -> dict[str, dict[str, str]]: + """Return the API docs.""" port = os.environ.get("API_SERVER_PORT", "5001") - routes = [route for route in bottle.default_app().routes if version in route.rule and fragment in route.rule] - return { + docs = { route.rule: { "url": f"https://www.quality-time.example.org:{port}{route.rule}", "method": route.method, "description": route.callback.__doc__, } - for route in routes + for route in bottle.default_app().routes } + if not docs: + bottle.abort(404, bottle.HTTP_CODES[404]) + return docs diff --git a/components/api_server/src/routes/health.py b/components/api_server/src/routes/health.py new file mode 100644 index 0000000000..67c9033571 --- /dev/null +++ b/components/api_server/src/routes/health.py @@ -0,0 +1,9 @@ +"""Health end-point.""" + +import bottle + + +@bottle.get("/api/internal/health", authentication_required=False) +def get_health(): + """Return the API-server health.""" + return {"healthy": True} diff --git a/components/api_server/tests/routes/test_documentation.py b/components/api_server/tests/routes/test_documentation.py index 9e5c2d2f71..5a85122750 100644 --- a/components/api_server/tests/routes/test_documentation.py +++ b/components/api_server/tests/routes/test_documentation.py @@ -2,7 +2,9 @@ import unittest -from routes import get_api +import bottle + +from routes import get_api_docs EXTERNAL_API_VERSION = "v3" @@ -16,22 +18,8 @@ class DocumentationTest(unittest.TestCase): def test_get_api(self): """Test that the API can be retrieved.""" - api_json = get_api() + api_json = get_api_docs() self.assertIn("/api", api_json) self.assertIn(self.login_internal, api_json) self.assertIn(self.login_external, api_json) self.assertIn(self.server_external, api_json) - - def test_get_api_by_version(self): - """Test that the API can be filtered by version.""" - api_json = get_api(EXTERNAL_API_VERSION) - self.assertNotIn(self.login_internal, api_json) - self.assertIn(self.login_external, api_json) - self.assertIn(self.server_external, api_json) - - def test_get_api_by_fragment(self): - """Test that the API can be filtered by version and a fragment.""" - api_json = get_api(EXTERNAL_API_VERSION, "login") - self.assertNotIn(self.login_internal, api_json) - self.assertIn(self.login_external, api_json) - self.assertNotIn(self.server_external, api_json) diff --git a/components/api_server/tests/routes/test_health.py b/components/api_server/tests/routes/test_health.py new file mode 100644 index 0000000000..d613a04ee6 --- /dev/null +++ b/components/api_server/tests/routes/test_health.py @@ -0,0 +1,13 @@ +"""Unit tests for the health route.""" + +import unittest + +from routes import get_health + + +class HealthTest(unittest.TestCase): + """Unit tests for the health route.""" + + def test_health(self): + """Test that the health status can be retrieved.""" + self.assertEqual({"healthy": True}, get_health()) diff --git a/components/api_server/tests/test_healthcheck.py b/components/api_server/tests/test_healthcheck.py index 2f3f7a98fa..a60b1b4f09 100644 --- a/components/api_server/tests/test_healthcheck.py +++ b/components/api_server/tests/test_healthcheck.py @@ -14,6 +14,7 @@ def test_healthy(self, mock_urlopen, mock_exit): """Test that the server is healthy.""" response = MagicMock() response.status = HTTPStatus.OK + response.read.return_value = '{"healthy": true}' response.__enter__.return_value = response mock_urlopen.return_value = response import healthcheck # noqa: F401 diff --git a/docs/src/changelog.md b/docs/src/changelog.md index 1b76d0b473..d90e7e774b 100644 --- a/docs/src/changelog.md +++ b/docs/src/changelog.md @@ -12,6 +12,12 @@ If your currently installed *Quality-time* version is not the latest version, pl +## [Unreleased] + +### Fixed + +- Make the API-server return HTTP status 404 on non-existing endpoints instead of 200. Fixes [#9860](https://github.com/ICTU/quality-time/issues/9860). + ## v5.24.0 - 2025-02-06 ### Added diff --git a/docs/src/development.md b/docs/src/development.md index 2562baf8bf..d8781b1568 100644 --- a/docs/src/development.md +++ b/docs/src/development.md @@ -77,6 +77,12 @@ python src/quality_time_server.py The API of the API-server is served at [http://localhost:5001](http://localhost:5001), e.g. access [http://localhost:5001/api/internal/report](http://localhost:5001/api/internal/report) to get the available reports combined with their recent measurements. +To quickly see all endpoints, point curl at `/api`: + +```console +curl http://localhost:5001/api | jq . +``` + ```{note} If you're new to Python virtual environments, note that: - Creating a virtual environment (`uv venv`) has to be done once. Only when the Python version changes, you want to recreate the virtual environment. diff --git a/tests/feature_tests/src/features/auth.feature b/tests/feature_tests/src/features/auth.feature index 09ace4a0ab..9c53797a6b 100644 --- a/tests/feature_tests/src/features/auth.feature +++ b/tests/feature_tests/src/features/auth.feature @@ -11,6 +11,10 @@ Feature: authentication When the client tries to log in with incorrect credentials Then the server tells the client the credentials are incorrect + Scenario: non-existing endpoint + When the client tries to access a non-existing endpoint + Then the server tells the client the endpoint does not exist + Scenario: change editors When jadoe logs in And the client changes the reports_overview permissions to "{"edit_reports": ["jadoe", "other_user"], "edit_entities": []}" diff --git a/tests/feature_tests/src/steps/auth.py b/tests/feature_tests/src/steps/auth.py index 0ce614ccbd..d0647e0c83 100644 --- a/tests/feature_tests/src/steps/auth.py +++ b/tests/feature_tests/src/steps/auth.py @@ -26,6 +26,12 @@ def log_in_with_incorrect_credentials(context: Context) -> None: context.post("login", {"username": "jadoe", "password": "wrong"}) # nosec +@when("the client tries to access a non-existing endpoint") +def access_non_existing_endpoint(context: Context) -> None: + """Try to access non-existing endpoint.""" + context.get("does-not-exist") + + @then("the server tells the client the credentials are incorrect") def check_invalid_credentials(context: Context) -> None: """Check that the server responded that the credentials are invalid.""" @@ -44,6 +50,12 @@ def check_unauthorized(context: Context) -> None: assert_equal(403, context.response.status_code) +@then("the server tells the client the endpoint does not exist") +def check_does_not_exist(context: Context) -> None: + """Check that the server responded with a resource does not exist error message.""" + assert_equal(404, context.response.status_code) + + @when("the client requests the public key") def get_public_key(context: Context) -> None: """Get the public key."""