security(auth): strict JWT validation in middleware (fix junk cookie bypass)

AUTH_TEST_PLAN test 7.5.8 expects junk cookies to be rejected with
401. The previous middleware behaviour was "presence-only": check
that some access_token cookie exists, then pass through. In
combination with my Task-12 decision to skip @require_auth
decorators on routes, this created a gap where a request with any
cookie-shaped string (e.g. access_token=not-a-jwt) would bypass
authentication on routes that do not touch the repository
(/api/models, /api/mcp/config, /api/memory, /api/skills, …).

Fix: middleware now calls get_current_user_from_request() strictly
and catches the resulting HTTPException to render a 401 with the
proper fine-grained error code (token_invalid, token_expired,
user_not_found, …). On success it stamps request.state.user and
the contextvar so repository-layer owner filters work downstream.

The 4 old "_with_cookie_passes" tests in test_auth_middleware.py
were written for the presence-only behaviour; they asserted that
a junk cookie would make the handler return 200. They are renamed
to "_with_junk_cookie_rejected" and their assertions flipped to
401. The negative path (no cookie → 401 not_authenticated)
is unchanged.

Verified:
  no cookie       → 401 not_authenticated
  junk cookie     → 401 token_invalid     (the fixed bug)
  expired cookie  → 401 token_expired

Tests: 284 passed (auth + persistence + isolation)
Lint: clean
This commit is contained in:
greatmengqi 2026-04-08 13:18:44 +08:00
parent e7a881b577
commit 745bf4324e
2 changed files with 52 additions and 24 deletions

View File

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

View File

@ -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