diff --git a/backend/app/gateway/auth_middleware.py b/backend/app/gateway/auth_middleware.py index fda05e93d..a2ef846c0 100644 --- a/backend/app/gateway/auth_middleware.py +++ b/backend/app/gateway/auth_middleware.py @@ -47,12 +47,23 @@ def _is_public(path: str) -> bool: class AuthMiddleware(BaseHTTPMiddleware): - """Coarse-grained auth gate: reject requests without a valid session cookie. + """Strict auth gate: reject requests without a valid session. - This does NOT verify JWT signature or user existence — that is the job of - ``get_current_user_from_request`` in deps.py (called by ``@require_auth``). - The middleware only checks *presence* of the cookie so that new endpoints - that forget ``@require_auth`` are not completely exposed. + Two-stage check for non-public paths: + + 1. Cookie presence — return 401 NOT_AUTHENTICATED if missing + 2. JWT validation via ``get_optional_user_from_request`` — return 401 + TOKEN_INVALID if the token is absent, malformed, expired, or the + signed user does not exist / is stale + + On success, stamps ``request.state.user`` and the + ``deerflow.runtime.user_context`` contextvar so that repository-layer + owner filters work downstream without every route needing a + ``@require_auth`` decorator. Routes that need per-resource + authorization (e.g. "user A cannot read user B's thread by guessing + the URL") should additionally use ``@require_permission(..., + owner_check=True)`` for explicit enforcement — but authentication + itself is fully handled here. """ def __init__(self, app: ASGIApp) -> None: @@ -74,18 +85,29 @@ class AuthMiddleware(BaseHTTPMiddleware): }, ) - # Resolve the full user now so repository-layer owner filters - # can read from the contextvar. We use the "optional" flavour so - # middleware never raises on bad tokens — the cookie-presence - # check above plus the @require_auth decorator provide the - # strict gates. A stale/invalid token yields user=None here; - # the request continues without a contextvar, and any protected - # endpoint will still be rejected by @require_auth. - from app.gateway.deps import get_optional_user_from_request + # Strict JWT validation: reject junk/expired tokens with 401 + # right here instead of silently passing through. This closes + # the "junk cookie bypass" gap (AUTH_TEST_PLAN test 7.5.8): + # without this, non-isolation routes like /api/models would + # accept any cookie-shaped string as authentication. + # + # We call the *strict* resolver so that fine-grained error + # codes (token_expired, token_invalid, user_not_found, …) + # propagate from AuthErrorCode, not get flattened into one + # generic code. BaseHTTPMiddleware doesn't let HTTPException + # bubble up, so we catch and render it as JSONResponse here. + # + # On success we stamp request.state.user and the contextvar + # so repository-layer owner filters work downstream without + # every route needing a decorator. + from fastapi import HTTPException - user = await get_optional_user_from_request(request) - if user is None: - return await call_next(request) + from app.gateway.deps import get_current_user_from_request + + try: + user = await get_current_user_from_request(request) + except HTTPException as exc: + return JSONResponse(status_code=exc.status_code, content={"detail": exc.detail}) request.state.user = user token = set_current_user(user) diff --git a/backend/tests/test_auth_middleware.py b/backend/tests/test_auth_middleware.py index 64f8604f0..398f9cec6 100644 --- a/backend/tests/test_auth_middleware.py +++ b/backend/tests/test_auth_middleware.py @@ -161,9 +161,12 @@ def test_protected_path_no_cookie_returns_401(client): assert body["detail"]["code"] == "not_authenticated" -def test_protected_path_with_cookie_passes(client): +def test_protected_path_with_junk_cookie_rejected(client): + """Junk cookie → 401. Middleware strictly validates the JWT now + (AUTH_TEST_PLAN test 7.5.8); it no longer silently passes bad + tokens through to the route handler.""" res = client.get("/api/models", cookies={"access_token": "some-token"}) - assert res.status_code == 200 + assert res.status_code == 401 def test_protected_post_no_cookie_returns_401(client): @@ -189,16 +192,18 @@ def test_protected_patch_no_cookie(client): assert res.status_code == 401 -def test_put_with_cookie_passes(client): +def test_put_with_junk_cookie_rejected(client): + """Junk cookie on PUT → 401 (strict JWT validation in middleware).""" client.cookies.set("access_token", "tok") res = client.put("/api/mcp/config") - assert res.status_code == 200 + assert res.status_code == 401 -def test_delete_with_cookie_passes(client): +def test_delete_with_junk_cookie_rejected(client): + """Junk cookie on DELETE → 401 (strict JWT validation in middleware).""" client.cookies.set("access_token", "tok") res = client.delete("/api/threads/abc") - assert res.status_code == 200 + assert res.status_code == 401 # ── Fail-closed: unknown future endpoints ───────────────────────────────── @@ -210,7 +215,8 @@ def test_unknown_endpoint_no_cookie_returns_401(client): assert res.status_code == 401 -def test_unknown_endpoint_with_cookie_passes(client): +def test_unknown_endpoint_with_junk_cookie_rejected(client): + """New endpoints are also protected by strict JWT validation.""" client.cookies.set("access_token", "tok") res = client.get("/api/future-endpoint") - assert res.status_code == 200 + assert res.status_code == 401