Skip to content

Commit 69ea8b3

Browse files
authored
fix(auth): make require_auth async so the user ContextVar reaches the endpoint (#485)
Fixes #481. Makes `require_auth` and `require_admin` `async def` so the user ContextVar set inside the dep is visible to async endpoints; sync deps run via `anyio.to_thread.run_sync` under `copy_context()` and the set is discarded on return.
1 parent 9329879 commit 69ea8b3

2 files changed

Lines changed: 151 additions & 4 deletions

File tree

deeptutor/api/routers/auth.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ def _extract_token(authorization: str | None, dt_token: str | None) -> str | Non
157157
# ---------------------------------------------------------------------------
158158

159159

160-
def require_auth(
160+
async def require_auth(
161161
authorization: str | None = Header(default=None, alias="Authorization"),
162162
dt_token: str | None = Cookie(default=None),
163163
) -> TokenPayload | None:
@@ -168,11 +168,20 @@ def require_auth(
168168
- Authorization: Bearer <token> header
169169
- dt_token cookie
170170
171-
Works on both HTTP and WebSocket routes — ``Header`` and ``Cookie`` are
172-
WS-compatible, while ``HTTPBearer`` (which we used to use here) is not.
171+
``Header`` and ``Cookie`` are kept here in place of ``HTTPBearer`` so the
172+
function stays usable from WebSocket call sites that don't go through
173+
FastAPI's standard HTTP request lifecycle.
173174
174175
Returns the authenticated TokenPayload, or None if auth is disabled.
175176
Raises HTTP 401 if auth is enabled but the token is missing or invalid.
177+
178+
Declared ``async def`` so the ``set_current_user`` call runs in the same
179+
asyncio context as the endpoint. A sync dependency is dispatched via
180+
``anyio.to_thread.run_sync``, which executes the function in a worker
181+
thread under a *copy* of the request context; any ``ContextVar.set``
182+
inside that thread is discarded when the thread returns, leaving the
183+
endpoint to read the unset default. That regression was the root cause
184+
of #481.
176185
"""
177186
if not AUTH_ENABLED:
178187
from deeptutor.multi_user.context import set_current_user
@@ -248,14 +257,18 @@ async def ws_require_auth(ws: WebSocket) -> _CtxToken | _WsAuthFailed:
248257
return set_current_user(user_from_token_payload(payload))
249258

250259

251-
def require_admin(
260+
async def require_admin(
252261
payload: TokenPayload | None = Depends(require_auth),
253262
) -> TokenPayload:
254263
"""
255264
FastAPI dependency that requires the caller to be an admin.
256265
257266
Raises HTTP 403 if the authenticated user is not an admin.
258267
When AUTH_ENABLED=false, all requests are treated as admin.
268+
269+
``async def`` mirrors ``require_auth`` so the dependency chain stays on
270+
the event loop and the user ContextVar set by ``require_auth`` is visible
271+
to the endpoint.
259272
"""
260273
if not AUTH_ENABLED:
261274
from deeptutor.services.auth import TokenPayload as TP

tests/api/test_auth_contextvar.py

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
"""Regression tests for #481.
2+
3+
When ``require_auth`` was declared as a sync ``def``, FastAPI dispatched it
4+
through ``anyio.to_thread.run_sync``, which executes the function in a worker
5+
thread under a *copy* of the request context. Any ``ContextVar.set`` inside
6+
that thread is discarded when the thread returns, so the endpoint reads the
7+
unset default. The user-scoped path service then silently falls back to the
8+
admin workspace and non-admin users hit 404 on every session request.
9+
10+
These tests pin two invariants:
11+
12+
1. ``require_auth`` and ``require_admin`` are declared ``async``.
13+
2. With ``AUTH_ENABLED=true`` and a valid token, the user ContextVar set
14+
inside ``require_auth`` is visible from inside the endpoint, so
15+
``get_path_service()`` resolves to the per-user workspace.
16+
"""
17+
18+
from __future__ import annotations
19+
20+
import inspect
21+
22+
from fastapi import Depends, FastAPI
23+
from fastapi.testclient import TestClient
24+
25+
26+
def test_require_auth_is_async_def() -> None:
27+
from deeptutor.api.routers.auth import require_admin, require_auth
28+
29+
assert inspect.iscoroutinefunction(require_auth), (
30+
"require_auth must be async — a sync dep is run in a threadpool whose "
31+
"ContextVar mutations don't propagate back to the endpoint. See #481."
32+
)
33+
assert inspect.iscoroutinefunction(require_admin), (
34+
"require_admin must be async for the same reason."
35+
)
36+
37+
38+
def test_require_auth_propagates_user_contextvar_to_endpoint(monkeypatch) -> None:
39+
"""End-to-end: a valid token through require_auth makes the user
40+
ContextVar visible to the endpoint."""
41+
from deeptutor.api.routers import auth as auth_router
42+
from deeptutor.multi_user.context import get_current_user_or_none
43+
from deeptutor.services.auth import TokenPayload
44+
45+
monkeypatch.setattr(auth_router, "AUTH_ENABLED", True)
46+
monkeypatch.setattr(
47+
auth_router,
48+
"decode_token",
49+
lambda _t: TokenPayload(username="alice", role="user", user_id="u_alice"),
50+
)
51+
52+
app = FastAPI()
53+
54+
@app.get("/whoami")
55+
async def whoami(_=Depends(auth_router.require_auth)) -> dict:
56+
user = get_current_user_or_none()
57+
if user is None:
58+
return {"seen": None}
59+
return {"seen": user.username, "role": user.role, "scope_kind": user.scope.kind}
60+
61+
with TestClient(app) as client:
62+
resp = client.get("/whoami", headers={"Authorization": "Bearer test-token"})
63+
64+
assert resp.status_code == 200
65+
body = resp.json()
66+
assert body["seen"] == "alice", (
67+
"Endpoint should observe the user ContextVar set inside require_auth. "
68+
"If this returns None the dependency is being run in a threadpool and "
69+
"the ContextVar mutation is discarded — see #481."
70+
)
71+
assert body["role"] == "user"
72+
assert body["scope_kind"] == "user"
73+
74+
75+
def test_require_auth_propagates_admin_contextvar_to_endpoint(monkeypatch) -> None:
76+
from deeptutor.api.routers import auth as auth_router
77+
from deeptutor.multi_user.context import get_current_user_or_none
78+
from deeptutor.services.auth import TokenPayload
79+
80+
monkeypatch.setattr(auth_router, "AUTH_ENABLED", True)
81+
monkeypatch.setattr(
82+
auth_router,
83+
"decode_token",
84+
lambda _t: TokenPayload(username="root", role="admin", user_id="u_root"),
85+
)
86+
87+
app = FastAPI()
88+
89+
@app.get("/whoami")
90+
async def whoami(_=Depends(auth_router.require_auth)) -> dict:
91+
user = get_current_user_or_none()
92+
return {"role": None if user is None else user.role}
93+
94+
with TestClient(app) as client:
95+
resp = client.get("/whoami", headers={"Authorization": "Bearer test-token"})
96+
97+
assert resp.status_code == 200
98+
assert resp.json() == {"role": "admin"}
99+
100+
101+
def test_path_service_resolves_per_user_workspace_through_dependency(monkeypatch, tmp_path) -> None:
102+
"""The full chain that the reporter exercised in #481: a non-admin
103+
request lands on an endpoint that calls ``get_path_service()`` and
104+
that path service must point at ``multi-user/<uid>/``, not the
105+
admin fallback."""
106+
from deeptutor.api.routers import auth as auth_router
107+
from deeptutor.multi_user import paths as mu_paths
108+
from deeptutor.services.auth import TokenPayload
109+
from deeptutor.services.path_service import get_path_service
110+
111+
monkeypatch.setattr(auth_router, "AUTH_ENABLED", True)
112+
monkeypatch.setattr(mu_paths, "MULTI_USER_ROOT", tmp_path / "multi-user")
113+
monkeypatch.setattr(mu_paths, "_path_services", {})
114+
monkeypatch.setattr(
115+
auth_router,
116+
"decode_token",
117+
lambda _t: TokenPayload(username="alice", role="user", user_id="u_alice"),
118+
)
119+
120+
app = FastAPI()
121+
122+
@app.get("/db-path")
123+
async def db_path(_=Depends(auth_router.require_auth)) -> dict:
124+
service = get_path_service()
125+
return {"chat_db": str(service.get_chat_history_db())}
126+
127+
with TestClient(app) as client:
128+
resp = client.get("/db-path", headers={"Authorization": "Bearer test-token"})
129+
130+
assert resp.status_code == 200
131+
chat_db = resp.json()["chat_db"]
132+
assert "multi-user/u_alice" in chat_db, (
133+
f"Per-user request should resolve to multi-user/<uid>/ workspace, got: {chat_db}"
134+
)

0 commit comments

Comments
 (0)