diff --git a/repeater/web/auth/cherrypy_tool.py b/repeater/web/auth/cherrypy_tool.py index 1e18acc..f280224 100644 --- a/repeater/web/auth/cherrypy_tool.py +++ b/repeater/web/auth/cherrypy_tool.py @@ -14,8 +14,10 @@ def check_auth(): Sets cherrypy.request.user on success. Returns 401 JSON response on failure. """ - # Skip auth check for OPTIONS requests (CORS preflight) + # Terminate CORS preflight before CherryPy dispatches the protected handler. if cherrypy.request.method == "OPTIONS": + cherrypy.response.status = 204 + cherrypy.request.handler = None return # Skip auth check for /auth/login endpoint diff --git a/repeater/web/auth/middleware.py b/repeater/web/auth/middleware.py index 4e7e343..0de314a 100644 --- a/repeater/web/auth/middleware.py +++ b/repeater/web/auth/middleware.py @@ -10,9 +10,10 @@ def require_auth(func): @wraps(func) def wrapper(*args, **kwargs): - # Skip authentication for OPTIONS requests (CORS preflight) + # Terminate CORS preflight without invoking the protected handler. if cherrypy.request.method == "OPTIONS": - return func(*args, **kwargs) + cherrypy.response.status = 204 + return b"" # Get auth handlers from global cherrypy config (not app config) jwt_handler = cherrypy.config.get("jwt_handler") diff --git a/repeater/web/http_server.py b/repeater/web/http_server.py index fa3053b..05fb589 100644 --- a/repeater/web/http_server.py +++ b/repeater/web/http_server.py @@ -46,6 +46,21 @@ _ORIGINAL_UNRAISABLEHOOK = sys.unraisablehook _CHEROOT_UNRAISABLE_HOOK_INSTALLED = False +def _cors_response_headers( + methods: str = "GET, POST, PUT, DELETE, OPTIONS", +) -> list[tuple[str, str]]: + """Return wildcard CORS headers for header-authenticated API requests. + + Browser credentials are intentionally disabled: wildcard origins cannot be + combined with Access-Control-Allow-Credentials. + """ + return [ + ("Access-Control-Allow-Origin", "*"), + ("Access-Control-Allow-Methods", methods), + ("Access-Control-Allow-Headers", "Authorization, Content-Type, X-API-Key"), + ] + + def _looks_like_cheroot_makefile_context(unraisable: object) -> bool: context = ( f"{getattr(unraisable, 'object', '')!r} {getattr(unraisable, 'err_msg', '')!r}".lower() @@ -277,6 +292,12 @@ class StatsApp: cherrypy.response.headers["Content-Type"] = guessed_type or "application/octet-stream" return target.read_bytes() + @cherrypy.expose + def favicon_ico(self): + """Serve the favicon bundled with the compiled frontend.""" + self._resolve_html_dir() + return self._serve_static_file(self.html_dir, ("favicon.ico",)) + @cherrypy.expose def index(self, **kwargs): """Serve the Vue.js application index.html.""" @@ -539,12 +560,7 @@ class HTTPStatsServer: cors_config = { "cors.expose.on": True, "tools.response_headers.on": True, - "tools.response_headers.headers": [ - ("Access-Control-Allow-Origin", "*"), - ("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS"), - ("Access-Control-Allow-Headers", "Authorization, Content-Type, X-API-Key"), - ("Access-Control-Allow-Credentials", "true"), - ], + "tools.response_headers.headers": _cors_response_headers(), # Disable automatic trailing slash redirects to prevent CORS issues "tools.trailing_slash.on": False, } @@ -609,14 +625,7 @@ class HTTPStatsServer: if self._cors_enabled: auth_config["/"]["cors.expose.on"] = True # Add CORS headers for OPTIONS requests - auth_config["/"]["tools.response_headers.headers"].extend( - [ - ("Access-Control-Allow-Origin", "*"), - ("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS"), - ("Access-Control-Allow-Headers", "Authorization, Content-Type, X-API-Key"), - ("Access-Control-Allow-Credentials", "true"), - ] - ) + auth_config["/"]["tools.response_headers.headers"].extend(_cors_response_headers()) cherrypy.tree.mount(self.auth_app, "/auth", auth_config) @@ -634,11 +643,7 @@ class HTTPStatsServer: if self._cors_enabled: doc_config["/"]["cors.expose.on"] = True doc_config["/"]["tools.response_headers.headers"].extend( - [ - ("Access-Control-Allow-Origin", "*"), - ("Access-Control-Allow-Methods", "GET, POST, OPTIONS"), - ("Access-Control-Allow-Headers", "Authorization, Content-Type, X-API-Key"), - ] + _cors_response_headers("GET, POST, OPTIONS") ) cherrypy.tree.mount(self.doc_app, "/doc", doc_config) diff --git a/tests/test_auth_components.py b/tests/test_auth_components.py index 2762637..a73c73a 100644 --- a/tests/test_auth_components.py +++ b/tests/test_auth_components.py @@ -8,6 +8,7 @@ import pytest from repeater.web.auth.api_tokens import APITokenManager from repeater.web.auth.cherrypy_tool import check_auth from repeater.web.auth.jwt_handler import JWTHandler +from repeater.web.auth.middleware import require_auth def test_jwt_handler_create_and_verify_and_invalid_cases(): @@ -65,10 +66,28 @@ def _set_cp(monkeypatch, method="GET", path="/api/private", headers=None, params return req, resp -def test_check_auth_skips_options_and_login(monkeypatch): - _set_cp(monkeypatch, method="OPTIONS") - assert check_auth() is None +def test_check_auth_options_terminates_request_before_handler_dispatch(monkeypatch): + handler = MagicMock(return_value={"secret": "must-not-run"}) + req, resp = _set_cp(monkeypatch, method="OPTIONS") + req.handler = handler + assert check_auth() is None + assert req.handler is None + assert resp.status == 204 + + +def test_require_auth_options_does_not_invoke_wrapped_handler(monkeypatch): + handler = MagicMock(return_value={"secret": "must-not-run"}) + _req, resp = _set_cp(monkeypatch, method="OPTIONS") + + result = require_auth(handler)() + + assert result == b"" + handler.assert_not_called() + assert resp.status == 204 + + +def test_check_auth_skips_public_login(monkeypatch): _set_cp(monkeypatch, method="GET", path="/auth/login") assert check_auth() is None diff --git a/tests/test_auth_endpoints.py b/tests/test_auth_endpoints.py index 7c017c6..31f8248 100644 --- a/tests/test_auth_endpoints.py +++ b/tests/test_auth_endpoints.py @@ -67,8 +67,9 @@ def test_auth_api_endpoints_constructs_tokens_endpoint(): def test_tokens_index_options_and_missing_manager(cp_ctx): endpoint = TokensAPIEndpoint() - cp_ctx(method="OPTIONS") - assert endpoint.index() == {} + _req, response, _cfg = cp_ctx(method="OPTIONS") + assert endpoint.index() == b"" + assert response.status == 204 cp_ctx(method="GET", headers={"Authorization": "Bearer x"}) with pytest.raises(cherrypy.HTTPError): diff --git a/tests/test_http_server_unit.py b/tests/test_http_server_unit.py index d2083f4..9887b6b 100644 --- a/tests/test_http_server_unit.py +++ b/tests/test_http_server_unit.py @@ -130,6 +130,20 @@ def test_stats_app_index_and_default_routing(monkeypatch, tmp_path): assert app.default("route") == "ok" +def test_stats_app_exposes_compiled_ui_favicon(monkeypatch, tmp_path): + favicon = b"compiled-ui-favicon" + (tmp_path / "favicon.ico").write_bytes(favicon) + + fake_api = SimpleNamespace(config_manager=object(), docs=lambda: "d") + monkeypatch.setattr(hs, "APIEndpoints", lambda *args, **kwargs: fake_api) + monkeypatch.setattr(cherrypy, "response", SimpleNamespace(headers={}), raising=False) + + app = hs.StatsApp(config={"web": {"web_path": str(tmp_path)}}) + + assert app.favicon_ico() == favicon + assert cherrypy.response.headers["Content-Type"] == "image/x-icon" + + def test_stats_app_index_error_paths(monkeypatch, tmp_path): fake_api = SimpleNamespace(config_manager=object(), docs=lambda: "d") monkeypatch.setattr(hs, "APIEndpoints", lambda *args, **kwargs: fake_api) @@ -185,3 +199,12 @@ def test_http_server_utility_methods(monkeypatch, tmp_path): ) server.stop() assert exited["v"] is True + + +def test_cors_response_headers_allow_bearer_preflight_without_credentials(): + headers = dict(hs._cors_response_headers()) + + assert headers["Access-Control-Allow-Origin"] == "*" + assert "OPTIONS" in headers["Access-Control-Allow-Methods"] + assert "Authorization" in headers["Access-Control-Allow-Headers"] + assert "Access-Control-Allow-Credentials" not in headers