refactor(website): opaque server-side session instead of JWT-in-cookie#6710
Closed
theosanderson-agent wants to merge 1 commit into
Closed
refactor(website): opaque server-side session instead of JWT-in-cookie#6710theosanderson-agent wants to merge 1 commit into
theosanderson-agent wants to merge 1 commit into
Conversation
The website previously stored the OIDC access / id / refresh tokens directly in the browser as httpOnly cookies, i.e. the JWTs were the session. This moves to a session-cookie approach: the browser holds only an opaque, random session id and the tokens live server-side in a session store, keyed by that id. Why (cf. https://gist.github.com/samsch/0d1f3d3b4745d778f78b230cf6061452): - Revocable: dropping the server-side entry (e.g. on logout) invalidates the session immediately, rather than relying on the access token's expiry. - The JWTs never reach the client at all, shrinking the token's exposure. Changes: - New `middleware/sessionStore.ts`: in-process store mapping session id -> tokens, with sliding TTL and a periodic sweep. Documented as correct for the default single-replica website; a horizontally-scaled deployment should swap in a shared store (Redis/Postgres) behind the same interface. - `authMiddleware.ts`: read/establish/clear the session via the store + an opaque `loculus_session` cookie (runtime-controlled `secure` flag, as before). Refreshed tokens are now persisted server-side. Legacy token cookies from the old scheme are cleared on the way through. - `logout.astro`: destroy the server-side session as well as the cookie. Integration tests: - The CLI bootstrap used to read the backend token out of the `access_token` cookie, which no longer exists. Added `GET /api/cli-bootstrap-token`, a dev/CI-only endpoint (HARD-DISABLED unless `insecureCookies`, i.e. 404 in production) that returns the current session's access token to that same session, and pointed the CLI page object at it. - logout spec now also asserts the `loculus_session` cookie is cleared. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on top of #6707 (
authelia-exp-merge). Changes the website browser session from "the JWTs are the cookies" to a session-cookie approach, inspired by Stop using JWT for sessions.Before
authMiddlewarestored the OIDCaccess/id/refreshtokens directly in three httpOnly cookies. The JWTs were the session — sent to the browser on every request, and (because they're stateless) valid until expiry no matter what.After
loculus_sessioncookie. The tokens live server-side in a session store keyed by that id and never reach the client.locals.session.tokenis still populated per-request (server-side) from the store, sogetAccessToken()and every caller keep working untouched.What changed
website/src/middleware/sessionStore.ts(new) — in-processsessionId → tokensstore with sliding TTL + periodic sweep. It is correct for the default single-replica website (replicas.website: 1); its limits are documented (lost on restart, not shared across replicas). A horizontally-scaled deployment should swap in a shared store (Redis/Postgres) behind the samecreate/get/put/deleteinterface — no other code changes.website/src/middleware/authMiddleware.ts— read/establish/clear the session via the store + the opaque cookie (same runtime-controlledsecure/sameSite/httpOnlyflags as before). Refreshed tokens are now persisted server-side; legacyaccess_token/refresh_token/oidc_access_tokencookies are cleared on the way through so returning users shed their old JWTs.website/src/pages/logout.astro— destroys the server-side session in addition to clearing the cookie.Why not Astro's built-in session API?
Astro's session cookie forces
securebased on build mode, but Loculus needs it runtime-controlled (!insecureCookies) so the http e2e/dev flow onloculus.test:3000works. A custom store + the existing cookie machinery keeps that behaviour.Integration tests
access_tokencookie, which no longer exists. AddedGET /api/cli-bootstrap-token— a dev/CI-only endpoint that returns the current session's access token to that same authenticated session. It is hard-disabled in production (404 unlessinsecureCookies, which is only true invalues_e2e_and_dev.yaml), so the token never leaves the server in a real deployment.CliPagenow fetches from it instead of reading the cookie.logout.spec.tsalso asserts theloculus_sessioncookie is cleared.Local checks
astro check+tsc(0 errors),eslint+prettier,vitest(676 passing),astro build, and integration-testsnpm run format.Known tradeoff
Session durability depends on the store. The in-process default loses sessions on website restart/redeploy (users re-login) and isn't multi-replica-safe — acceptable for the default single replica, flagged for anyone scaling the website. This is the inherent "you need a session store" cost the gist calls out.
🤖 Generated with Claude Code
🚀 Preview: Add
previewlabel to enable