mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-08-05 02:45:28 +00:00
fix(auth): normalize mounted request paths
This commit is contained in:
parent
d96c7af3df
commit
de5d480b7d
3 changed files with 271 additions and 5 deletions
22
app.py
22
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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
228
tests/test_auth_root_path.py
Normal file
228
tests/test_auth_root_path.py
Normal file
|
|
@ -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}
|
||||
Loading…
Add table
Reference in a new issue