Merge de5d480b7d into 20e7fc0164

app.py

@@ -67,7 +67,13 @@ from core.constants import (
     REQUEST_TIMEOUT, OPENAI_API_KEY, AUTH_FILE,
 )
 from core.database import SessionLocal, ApiToken
-from core.middleware import SecurityHeadersMiddleware, is_cors_preflight
+from core.middleware import (
+    SecurityHeadersMiddleware,
+    get_application_route_path,
+    is_cors_preflight,
+    path_is_route_or_child,
+    with_asgi_root_path,
+)
 from core.auth import AuthManager, normalize_known_username
 from core.exceptions import (
     SessionNotFoundError, InvalidFileUploadError,
@@ -284,7 +290,7 @@ if AUTH_ENABLED:
     def _is_auth_exempt(path: str) -> bool:
         if path in AUTH_EXEMPT_EXACT:
             return True
-        if any(path.startswith(p) for p in AUTH_EXEMPT_PREFIXES):
+        if any(path_is_route_or_child(path, p) for p in AUTH_EXEMPT_PREFIXES):
             return True
         return any(p.match(path) for p in AUTH_EXEMPT_PATTERNS)
 
@@ -355,7 +361,7 @@ if AUTH_ENABLED:
 
     class AuthMiddleware(BaseHTTPMiddleware):
         async def dispatch(self, request: Request, call_next):
-            path = request.url.path
+            path = get_application_route_path(request.scope)
             # A genuine CORS preflight (OPTIONS + Access-Control-Request-Method)
             # carries no credentials by design and must reach CORSMiddleware to be
             # answered. AuthMiddleware is the outermost middleware, so gating the
@@ -399,7 +405,10 @@ if AUTH_ENABLED:
             if not auth_manager.is_configured:
                 # No users yet — redirect to login for first-time setup
                 if not path.startswith("/api/"):
-                    return RedirectResponse(url="/login", status_code=302)
+                    return RedirectResponse(
+                        url=with_asgi_root_path(request.scope, "/login"),
+                        status_code=302,
+                    )
                 return JSONResponse(status_code=401, content={"error": "Setup required"})
 
             # --- Bearer token auth (API tokens for external integrations) ---
@@ -461,7 +470,10 @@ if AUTH_ENABLED:
             if not auth_manager.validate_token(token):
                 if path.startswith("/api/"):
                     return JSONResponse(status_code=401, content={"error": "Not authenticated"})
-                return RedirectResponse(url="/login", status_code=302)
+                return RedirectResponse(
+                    url=with_asgi_root_path(request.scope, "/login"),
+                    status_code=302,
+                )
 
             # Attach current username to request state for downstream routes
             request.state.current_user = auth_manager.get_username_for_token(token)

core/middleware.py

@@ -3,10 +3,12 @@
 
 import os
 import secrets
+from collections.abc import Mapping
 
 from fastapi import HTTPException, Request
 from starlette.middleware.base import BaseHTTPMiddleware
 from starlette.responses import Response
+from starlette.routing import get_route_path
 
 
 # Per-process token that lets the in-app tool layer hit admin-gated
@@ -19,6 +21,30 @@ INTERNAL_TOOL_HEADER = "X-Odysseus-Internal-Token"
 INTERNAL_TOOL_USER = "internal-tool"
 
 
+def get_application_route_path(scope: Mapping[str, object]) -> str:
+    """Return the application-relative path used by Starlette routing.
+
+    Uvicorn prefixes ``scope["path"]`` with a configured ASGI ``root_path``;
+    Starlette removes that prefix before matching routes. Middleware policy
+    must use the same path form or a deployment prefix can change which policy
+    applies to an otherwise unchanged application route.
+    """
+    return get_route_path(scope)
+
+
+def with_asgi_root_path(scope: Mapping[str, object], path: str) -> str:
+    """Prefix an application path for a client-facing redirect target."""
+    root_path = scope.get("root_path", "")
+    if not isinstance(root_path, str) or not root_path:
+        return path
+    return f"{root_path.rstrip('/')}{path}"
+
+
+def path_is_route_or_child(path: str, prefix: str) -> bool:
+    """Return whether ``path`` is exactly ``prefix`` or below that route."""
+    return path == prefix or path.startswith(prefix + "/")
+
+
 def is_cors_preflight(method: str, headers) -> bool:
     """True for a genuine CORS preflight: an OPTIONS request carrying the
     Access-Control-Request-Method header. Such requests are credential-less by

tests/test_auth_root_path.py

@@ -0,0 +1,228 @@
+"""Auth middleware must evaluate the same path that Starlette routes."""
+
+import json
+import os
+from pathlib import Path
+import subprocess
+import sys
+import textwrap
+
+import pytest
+
+from core.middleware import (
+    get_application_route_path,
+    path_is_route_or_child,
+    with_asgi_root_path,
+)
+
+
+ROOT = Path(__file__).resolve().parents[1]
+
+
+@pytest.mark.parametrize(
+    ("root_path", "path", "expected"),
+    [
+        ("", "/api/models", "/api/models"),
+        ("/odysseus", "/odysseus/api/models", "/api/models"),
+        ("/odysseus/", "/odysseus//api/models", "/api/models"),
+        ("/", "//api/models", "/api/models"),
+        ("/odysseus", "/odyssey/api/models", "/odyssey/api/models"),
+        ("/app", "/application/api/models", "/application/api/models"),
+        ("/odysseus", "/odysseus", ""),
+    ],
+)
+def test_application_route_path_matches_starlette_semantics(
+    root_path,
+    path,
+    expected,
+):
+    assert get_application_route_path({
+        "root_path": root_path,
+        "path": path,
+    }) == expected
+
+
+@pytest.mark.parametrize(
+    ("root_path", "expected"),
+    [
+        ("", "/login"),
+        ("/odysseus", "/odysseus/login"),
+        ("/odysseus/", "/odysseus/login"),
+        ("/", "/login"),
+    ],
+)
+def test_client_redirect_path_includes_asgi_root_path(root_path, expected):
+    assert with_asgi_root_path({"root_path": root_path}, "/login") == expected
+
+
+def test_route_prefix_matching_is_segment_aware():
+    assert path_is_route_or_child("/assets", "/assets") is True
+    assert path_is_route_or_child("/assets/app.js", "/assets") is True
+    assert path_is_route_or_child("/assets-v2/app.js", "/assets") is False
+
+
+def test_real_auth_middleware_uses_application_relative_path(tmp_path):
+    env = os.environ.copy()
+    env.update({
+        "AUTH_ENABLED": "true",
+        "CHROMADB_CONNECT_TIMEOUT": "0.01",
+        "CHROMADB_HOST": "127.0.0.1",
+        "CHROMADB_PORT": "9",
+        "DATABASE_URL": f"sqlite:///{tmp_path / 'app.db'}",
+        "LOCALHOST_BYPASS": "false",
+        "ODYSSEUS_DATA_DIR": str(tmp_path),
+        "ODYSSEUS_DISABLE_MCP": "1",
+        "OPENAI_API_KEY": "",
+        "PYTHONPATH": str(ROOT),
+        "PYTHON_DOTENV_DISABLED": "1",
+    })
+    probe = textwrap.dedent(
+        """
+        import asyncio
+        import json
+
+        from starlette.applications import Starlette
+        from starlette.responses import JSONResponse
+        from starlette.routing import Route
+
+        import app as app_module
+
+
+        class _AuthManager:
+            def __init__(self, configured):
+                self.is_configured = configured
+
+            @staticmethod
+            def validate_token(_token):
+                return False
+
+            @staticmethod
+            def get_username_for_token(_token):
+                return None
+
+
+        def _scope(root_path, route_path, downstream):
+            full_path = root_path + route_path
+            return {
+                "type": "http",
+                "http_version": "1.1",
+                "method": "GET",
+                "scheme": "http",
+                "path": full_path,
+                "raw_path": full_path.encode(),
+                "root_path": root_path,
+                "query_string": b"",
+                "headers": [],
+                "client": ("192.0.2.10", 4321),
+                "server": ("testserver", 80),
+                "app": downstream,
+            }
+
+
+        async def _case(root_path, route_path, *, configured):
+            manager = _AuthManager(configured)
+            app_module.auth_manager = manager
+            calls = []
+
+            async def endpoint(request):
+                calls.append(request)
+                return JSONResponse({"reached": True})
+
+            downstream = Starlette(routes=[Route(route_path, endpoint)])
+            downstream.state.auth_manager = manager
+            middleware = app_module.AuthMiddleware(downstream)
+            scope = _scope(root_path, route_path, downstream)
+            sent = []
+            request_sent = False
+
+            async def receive():
+                nonlocal request_sent
+                if request_sent:
+                    return {"type": "http.disconnect"}
+                request_sent = True
+                return {"type": "http.request", "body": b"", "more_body": False}
+
+            async def send(message):
+                sent.append(message)
+
+            await middleware(scope, receive, send)
+            response_start = next(m for m in sent if m["type"] == "http.response.start")
+            headers = {k.decode().lower(): v.decode() for k, v in response_start["headers"]}
+            return {
+                "status": response_start["status"],
+                "location": headers.get("location"),
+                "called": len(calls),
+            }
+
+
+        async def main():
+            setup = await _case("/odysseus", "/api/auth/setup", configured=False)
+            mounted_api = await _case("/odysseus", "/api/models", configured=True)
+            mounted_browser = await _case("/odysseus", "/notes", configured=True)
+            webhook = await _case(
+                "/odysseus",
+                "/api/tasks/task-1/webhook/secret-token",
+                configured=True,
+            )
+            static_child = await _case(
+                "/odysseus",
+                "/static/app.js",
+                configured=True,
+            )
+            static_lookalike = await _case(
+                "/odysseus",
+                "/static-v2/app.js",
+                configured=True,
+            )
+            default_api = await _case("", "/api/models", configured=True)
+            print("RESULT=" + json.dumps({
+                "setup": setup,
+                "mounted_api": mounted_api,
+                "mounted_browser": mounted_browser,
+                "webhook": webhook,
+                "static_child": static_child,
+                "static_lookalike": static_lookalike,
+                "default_api": default_api,
+            }, sort_keys=True))
+
+
+        asyncio.run(main())
+        """
+    )
+    result = subprocess.run(
+        [sys.executable, "-c", probe],
+        cwd=ROOT,
+        env=env,
+        capture_output=True,
+        text=True,
+        timeout=30,
+        check=False,
+    )
+
+    assert result.returncode == 0, result.stderr
+    result_line = next(
+        (line for line in result.stdout.splitlines() if line.startswith("RESULT=")),
+        None,
+    )
+    assert result_line is not None, result.stdout
+    payload = json.loads(result_line.removeprefix("RESULT="))
+
+    assert payload["setup"] == {"status": 200, "location": None, "called": 1}
+    assert payload["webhook"] == {"status": 200, "location": None, "called": 1}
+    assert payload["static_child"] == {
+        "status": 200,
+        "location": None,
+        "called": 1,
+    }
+    assert payload["static_lookalike"] == {
+        "status": 302,
+        "location": "/odysseus/login",
+        "called": 0,
+    }
+    assert payload["mounted_browser"] == {
+        "status": 302,
+        "location": "/odysseus/login",
+        "called": 0,
+    }
+    for name in ("mounted_api", "default_api"):
+        assert payload[name] == {"status": 401, "location": None, "called": 0}