fix(auth): move browser sessions to HttpOnly cookies with CSRF origin guard - #286
fix(auth): move browser sessions to HttpOnly cookies with CSRF origin guard#286Ilyas-ek wants to merge 2 commits into
Conversation
… guard Session tokens no longer live in localStorage, closing the XSS-exfiltration path: signin/signup now set an HttpOnly, SameSite=Lax session cookie, and the SPA never persists the JWT anywhere page scripts can read. Backend: - signin/signup/login set the session cookie; signout clears it - get_current_user: explicit Authorization: Bearer wins (tests, tools, API clients); otherwise the cookie authenticates. Legacy "Bearer null/undefined" headers from pre-cookie UI code fall through to the cookie - POST /auths/cookie exchanges a bearer token (e.g. OAuth URL fragment) for a cookie session - CSRF middleware: non-safe methods carrying the session cookie must present an Origin matching this host or a configured CORS origin (second line of defence behind SameSite=Lax); bearer-only clients unaffected - Socket.IO handshake accepts the session cookie (auth payload and query token still work for tools) - AUTH_COOKIE_SECURE env opt-in for TLS deployments Frontend: - all API calls are same-origin: production already is (FastAPI serves the SPA); dev now routes /api, /auths and /realtime through the Vite proxy (BACKEND_URL overrides the target for Docker) — so the cookie rides along on every fetch with zero per-call credentials handling - auth page/layout: no localStorage.token writes; session is probed via cookie on load; stale legacy tokens are cleaned up - socket connects same-origin with withCredentials; no token in the handshake Tests: new tests/test_cookie_auth.py (cookie issuing, cookie auth, signout, bearer compatibility and precedence, bearer-to-cookie exchange, CSRF allow/ block/bypass cases, handshake cookie parsing); full backend suite green.
|
Salam @Ilyas-ek, 1. CSRF check will silently break once this runs behind real HTTPS It allows the request if Fix: make the check aware of 2. New It's not in Everything else looks solid — bearer-vs-cookie priority logic is correct, |
|
Thank you for this contribution and for your continued involvement in the project 🙌 |
There was a problem hiding this comment.
Pull request overview
This PR migrates the browser authentication channel from a script-readable JWT (previously stored in localStorage) to an HttpOnly, SameSite=Lax cookie, and introduces a CSRF origin guard for cookie-authenticated mutations. It also updates the SvelteKit dev setup to keep API/realtime calls same-origin via Vite proxying, and adds backend tests to validate cookie auth + CSRF behavior.
Changes:
- Set/clear an HttpOnly auth cookie on signin/signup/signout and authenticate requests via cookie fallback when no explicit Bearer token is provided.
- Add CSRF origin checking middleware for non-safe, cookie-authenticated requests.
- Update UI + Vite dev proxy to rely on same-origin requests (relative API URLs) and Socket.IO cookie handshake; add cookie-auth test coverage.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/vite.config.ts | Proxies /api, /auths, /realtime to backend to keep dev traffic same-origin for cookie auth. |
| ui/src/routes/auth/+page.svelte | Stops persisting tokens to localStorage; adds bearer→cookie exchange for OAuth fragment flow. |
| ui/src/routes/+layout.svelte | Socket.IO handshake switches to cookie-based auth; session probe now uses cookie path. |
| ui/src/lib/constants.ts | Makes backend URLs relative to enforce same-origin calls in dev/prod. |
| ui/src/lib/apis/auths/index.ts | Adds establishCookieSession() helper to exchange Bearer token for cookie session. |
| tests/test_files.py | Updates anonymous-access assertion now that auth failures return 401 and signup sets cookies. |
| tests/test_cookie_auth.py | Adds coverage for cookie issuance, bearer precedence, CSRF origin check, and realtime cookie parsing. |
| gateway/realtime/socket.py | Accepts session token from HttpOnly cookie during Socket.IO handshake. |
| gateway/http/routers/auth.py | Sets auth cookie on signin/signup; adds /auths/cookie; clears cookie on signout; returns tokenless session user payload. |
| gateway/http/dependencies.py | Prefers explicit Bearer tokens, otherwise authenticates via auth cookie; treats Bearer null/undefined as absent. |
| gateway/http/app.py | Adds CSRF origin guard middleware for cookie-authenticated non-safe requests. |
| config/settings.py | Adds cookie settings (name, secure flag, samesite). |
Comments suppressed due to low confidence (1)
ui/src/routes/auth/+page.svelte:65
setSessionUser()still emitsuser-joinwithsessionUser.token. After this PR,GET /api/v1/auths/(used by the OAuth fragment flow) no longer returns a token, so this becomesundefinedand the event is ignored server-side. It also unnecessarily re-exposes JWTs to page scripts even though Socket.IO connect can authenticate via the HttpOnly cookie.
if ($socket) {
$socket.emit('user-join', { auth: { token: sessionUser.token } });
}
| // Exchange the fragment token for an HttpOnly cookie session instead of | ||
| // persisting it anywhere page scripts could read it. | ||
| await establishCookieSession(token).catch(() => {}); | ||
| await setSessionUser(sessionUser); |
| } | ||
| // Probe the session — the HttpOnly cookie (sent automatically) decides | ||
| // whether we're signed in; nothing is read from localStorage. | ||
| const sessionUser = await getSessionUser('').catch(() => null); |
| // Save Session User to Store | ||
| $socket.emit('user-join', { auth: { token: sessionUser.token } }); | ||
|
|
| if request.method not in ("GET", "HEAD", "OPTIONS") and request.cookies.get( | ||
| settings.AUTH_COOKIE_NAME | ||
| ): | ||
| origin = request.headers.get("origin") | ||
| if origin: | ||
| allowed = origin in settings.cors_origins_list or origin == ( | ||
| f"{request.url.scheme}://{request.url.netloc}" | ||
| ) | ||
| if not allowed: | ||
| return JSONResponse( | ||
| status_code=403, | ||
| content={"detail": "Origin not allowed"}, | ||
| ) | ||
| return await call_next(request) |
| Signin/signup set an HttpOnly, SameSite=Lax session cookie; `get_current_user` | ||
| reads the cookie first and falls back to `Authorization: Bearer` (tests, tools). | ||
| A CSRF origin check guards cookie-authenticated mutations, and the Socket.IO |
| .catch((err) => { | ||
| console.log(err); | ||
| error = err.detail; | ||
| return null; | ||
| }); |
Addresses review findings (Eziane + Copilot) on the cookie-auth PR: - CSRF check behind a TLS-terminating proxy: the same-origin comparison now honours X-Forwarded-Proto/Host, so it no longer sees "http" internally and wrongly 403s legitimate https POSTs. CORS_ALLOW_ORIGIN stays the explicit allowlist for the deployed domain. - Closed the "missing Origin" bypass: a cookie-only unsafe request must present an Origin/Referer that matches — absent is rejected too. The guard now only runs for cookie-*only* requests (a Bearer token isn't a CSRF vector), which also keeps bearer clients/tests unaffected. Auth-establishment endpoints (signin/signup/login) are exempt — they don't act on an existing session. - Frontend nits: getSessionUser() omits the Authorization header entirely when there's no token (no empty "Bearer "); removed the dead user-join emit that sent an undefined token and instead reconnect the socket after login so the handshake carries the fresh cookie; establishCookieSession failures now surface (toast + stop) instead of being swallowed, and report non-JSON errors. - Docs: document AUTH_COOKIE_SECURE (.env.example + AGENTS.md) and note that CORS_ALLOW_ORIGIN must be the real origin in production for the CSRF check. - Fixed the test_cookie_auth docstring precedence to match the impl (bearer first, cookie fallback); added tests for missing-Origin rejection and the X-Forwarded-Proto same-origin path.
This PR closes the known platform-wide limitation where the session JWT was kept in
localStorage— readable by any script running on the page, so a single XSS bug anywherein the app could exfiltrate sessions. Browser sessions now live in an HttpOnly,
SameSite=Lax cookie that page scripts cannot read, with a CSRF origin guard on
cookie-authenticated mutations (the protection that becomes relevant once cookies are the
auth channel).
How it works
never persists the token anywhere script-readable.
get_current_user: an explicitAuthorization: Bearertoken wins (tests, curl,SDKs state exactly who they are); otherwise the cookie authenticates. Legacy
Bearer null/undefinedheaders from pre-cookie UI code fall through to the cookie,so nothing breaks during the transition.
Originheader mustmatch this host or a configured CORS origin — second line of defence behind
SameSite=Lax. Bearer-only clients are unaffected.
accepted for tools).
now routes
/api,/authsand/realtimethrough the Vite proxy (BACKEND_URLenvoverrides the target for Docker). Because every call is same-origin, the cookie rides
along automatically — no per-fetch
credentialschurn across the ~290 existing fetchcalls, and any future fetch is covered by default.
POST /auths/cookieexchanges a bearer token (e.g. OAuth URL fragment) for a cookiesession.
AUTH_COOKIE_SECURE=trueenv flag adds theSecureattribute for TLS deployments(off by default so plain-HTTP local/LAN dev keeps working).
Compatibility
Authorization: Bearerwork unchanged.token is ignored and cleaned up).
/wsproxy entry was removed from Vite config (the backend rejects thatpath by design).
How to test
uvicorn main:app --port 8080+cd ui && npm run dev, open http://localhost:5173.tokencookie marked HttpOnly —and
localStoragecontains no token.document.cookie→ the session token is not there (that's the fix).redirected to /auth.
/realtime/socket.iohandshake, notoken in the request payload).
pytest -q→ all green, including the newtests/test_cookie_auth.py(cookie issuing/auth/signout, bearer compatibility and precedence, bearer→cookie
exchange, CSRF allow/block/bypass, handshake cookie parsing).
UI changes
None visually — auth flow behaves identically; only the storage mechanism changed.
Breaking changes
None for the UI or API consumers. Deployments that terminate TLS should set
AUTH_COOKIE_SECURE=true.