Merge pull request #372 from yellowcooln/fix/web-request-handling

fix(web): normalize OPTIONS handling

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

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")

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)

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
 

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):

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") == "<html>ok</html>"
 
 
+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