feat(auth): server-side session model replacing Basic-auth cookie promotion #523
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Context
This is Phase 1 of 2 splitting #522.
PR #521 (closing #520) added an
AuthTokenCookieFilterthat promotes anauth_tokencookie (containingBasic <base64(email:password)>) to anAuthorizationheader so browser-direct/api/*calls authenticate in production. The fix unblocked the deploy. The underlying model is debt: the cookie is the credential — stolen cookie = stolen password, no server-side revocation, no audit signal on login.This issue replaces that machinery with a proper server-side session model. The Spring Security CSRF re-enable, password-change session invalidation, admin force-logout, and login rate limiting all land in the follow-up issue (Phase 2 — TODO: link once filed).
Why split
Phase 1 is the architectural cutover: one breaking deploy, every user re-logs in. Phase 2 is purely additive on top of a working session model. Folding them into one PR doubles review surface and forces the same risky cutover decisions to be made simultaneously. Independently, each phase is shippable and reviewable.
⚠ Corrected facts from #522 self-review
V2__drop_spring_session_tables.sqlremoved it as unused;backend/pom.xmlhas nospring-session-jdbcdependency.CLAUDE.md's "Tech Stack" line claiming it is in the stack is stale and must be updated alongside this work.User need
Functional requirements
FR-AUTH-001 — There SHALL be a first-class
POST /api/auth/loginendpoint in a newauthpackage (decided in #522 review) that accepts{ email, password }, validates againstapp_users, and on success establishes a server-side session. The response body SHALL be the sameAppUsershape currently returned by/api/users/me(drives the existing TypeScript binding via OpenAPI regen).FR-AUTH-002 — Session establishment SHALL set an opaque session identifier (not a credential) in a cookie with:
HttpOnly=trueSameSite=strictSecure=trueunconditional in non-dev profiles (no runtimeurl.protocolbranch — drop theisHttpsfootgun inlogin/+page.server.ts:50)Path=/MaxInactiveIntervalInSeconds=28800)The session record SHALL be stored server-side in the
spring_session/spring_session_attributestables via Spring Session JDBC.FR-AUTH-003 — There SHALL be a
POST /api/auth/logoutendpoint that invalidates the current session record only (single-device logout — decided in #522 review). After calling it, requests carrying the same cookie SHALL be treated as unauthenticated (401), even before the cookie'sMax-Ageelapses.FR-AUTH-006 —
AuthTokenCookieFilterand the Basic-auth machinery downstream of it SHALL be removed in the same PR that lands the new endpoints — no transitional/compat filter (Nora + Tobi both insist; "transitional" code becomes permanent). Specifically:AuthTokenCookieFilter.javaand its testsecure: isHttpsbranch infrontend/src/routes/login/+page.server.tsBasiccookie construction in the login action; the action POSTs to/api/auth/loginand relies on theSet-Cookiereturned by the backend/api/users/meround-trip validation step in the login action (the login endpoint returns the user payload)frontend/vite.config.ts(no longer needed; backend reads its own cookie)frontend/src/hooks.server.ts— thehandleFetchre-encoding logic disappears; theuserGrouphandle can read the principal from the session (or keep the/api/users/mecall if simpler — design choice for the implementer)formLogin(...)chain fromSecurityConfig(Felix's note — implicit form-login is dead code once/api/auth/loginexists)FR-AUTH-007 — Deployment story SHALL remain the same: SvelteKit form-action login, no client-side JWT handling in browser JS, no extra round-trips per request. Caddy
/api/*routing requires no header injection.Non-functional requirements
NFR-SEC-101 — No user password (plaintext, base64, or otherwise reversible) SHALL be present in any cookie sent to the browser at any point after login completes. The session cookie SHALL contain only an opaque identifier.
NFR-SEC-102 — A stolen session cookie SHALL become inert on the next request after operator-initiated invalidation (logout in this issue; password change/reset and admin force-logout in Phase 2). No grace period is permitted. Spring Session JDBC deletes are synchronous; this is asserted via the test: at T+0 delete the session row, at T+0+ε the next request returns 401.
NFR-PERF-101 — Authenticated request overhead SHALL NOT exceed 5 ms (p95) added by session lookup compared to the current in-memory Basic-decode path. Measured via a k6 smoke run (optional but recommended per Sara's pyramid): 50 RPS of
GET /api/users/meafter a successful login, p95 added latency reported in the PR.NFR-OBSV-101 (partial) — Login (success + failure) and logout SHALL produce structured audit log entries via the existing
AuditService:AuditKind.LOGIN_SUCCESS—{ userId, ip, ua (truncated 200 chars) }AuditKind.LOGIN_FAILED—{ email, ip, ua }— never the attempted passwordAuditKind.LOGOUT—{ userId, ip, ua }(
ADMIN_FORCE_LOGOUTis added in Phase 2.)NFR-COMPAT-101 — Existing dev tooling SHALL continue to work without code changes outside this PR:
vite.config.tsproxy (header-injection removed but proxy itself unchanged), the dev e2e test profile, thee2eprofile's deterministic admin seed (admin@familyarchive.local/admin123perPasswordResetTestHelper).Acceptance criteria
Resolved decisions (carried from #522 review)
authpackageuser; cross-domain call (AuthService → UserService) is fineLAST_ACCESS_TIME)Implementation hints (non-binding — for the implementer)
These are concrete starting points pulled from persona review; the implementer may diverge if a better path appears:
pom.xml— addspring-session-jdbc(pin the version explicitly; don't rely on BOM transitive)V33__recreate_spring_session_tables.sql— copy the canonical DDL Spring Session ships; leave a comment explaining this recreates whatV2droppedapplication.yaml— setspring.session.store-type=jdbc,spring.session.timeout=28800ssession_expiredkey inmessages/{de,en,es}.jsonmin-h-[44px](WCAG 2.2 SC 2.5.8) — fix in this PRINVALID_CREDENTIALS→ "E-Mail oder Passwort sind nicht korrekt." (no user-not-found leak)ErrorCodevalues:INVALID_CREDENTIALS,SESSION_EXPIRED— add toErrorCode.java, mirror infrontend/src/lib/shared/errors.ts, add i18n keys in all three locale filesdocs/adr/NNNN-stateful-auth-via-spring-session-jdbc.mdper MarkusCLAUDE.md"Tech Stack" line as part of this PRdocs/architecture/c4/seq-auth-flow.puml— cookie promotion is gone/api/auth/logoutfirst, then delete cookie, then redirect. If the backend call fails, still delete the local cookie and redirect (Leonie — defense in depth: the user wanted to log out)Out of scope (this issue — moves to Phase 2)
CookieCsrfTokenRepository.withHttpOnlyFalse())AuditKind.ADMIN_FORCE_LOGOUTErrorCode.CSRF_TOKEN_MISSING,TOO_MANY_LOGIN_ATTEMPTSscripts/force-logout-user.shbreak-glass helperAuthE2EControllerforce-create-session test seam (lands with Phase 2 if Sara still wants it)Cutover plan (operator)
handleAuthhook redirects to/login?reason=expired; the banner rendersPriority & sizing
Test strategy (Sara's pyramid, scoped to Phase 1)
AuthService.loginhappy + invalid creds;AuthService.logout; audit-log writes for both@WebMvcTest)AuthController.login200/401 paths;AuthController.logout204/401 paths/api/users/me→ logout → 401 on retry; idle-timeout: manually backdateLAST_ACCESS_TIME> 8h → next request 401/api/auth/login; logout action POSTs to/api/auth/logoutbefore deleting cookie; session-expired banner renders when?reason=expiredBasic)pom.xml; new code carries ≥80% own branch coverageDefinition of Done
AuthTokenCookieFilterand all references deletedCLAUDE.md"Tech Stack" line updated to reflect Spring Session JDBC re-introductiondocs/adr/docs/architecture/c4/seq-auth-flow.pumlrewrittennpm run generate:apiinfrontend/)main— Filed as part of splitting #522 (replaces it together with the Phase 2 issue). Closes the Phase-1 portion of the auth-model rewrite.
Phase 2 follow-up is filed: #524 (CSRF re-enable, session revocation on password change/reset, admin force-logout, login rate-limit). Replace the "TODO: link once filed" reference in the Context section with #524.
👨💻 Felix Brandt — Senior Fullstack Developer
Observations
V65__tbmp_promote_unique_to_pk.sql. The issue's hint saysV33__recreate_spring_session_tables.sql— that's wrong. The recreate migration must be V66 (or later if other branches land first).SecurityConfig.java:80is.httpBasic(Customizer.withDefaults()).formLogin(form -> form.usernameParameter("email")). The DoD removesformLogin, buthttpBasicalso goes — once/api/auth/loginexists nothing else issuesWWW-Authenticate: Basic. Removing both also removes the dev "browser pops a native dialog" footgun.frontend/src/hooks.server.ts:50-67("userGroup" handle) still readsauth_tokenand calls/api/users/me. After cutover, that whole block must change: forward theSESSIONcookie (or whatever cookie name we pick) on the call to/api/users/me, not anAuthorizationheader.handleFetchinhooks.server.ts:70-113currently rebuilds requests withAuthorization: <auth_token>. With server-side sessions, SvelteKit's fetch must forward the cookie header instead — the backend reads the session cookie itself.routes/logout/+page.server.ts) only deletes the local cookie. ACK that this PR adds a backendPOST /api/auth/logoutcall before cookie deletion.ErrorCode.javaalready hasUNAUTHORIZEDandFORBIDDEN. The newINVALID_CREDENTIALSandSESSION_EXPIREDcodes also need the 3-place propagation (ErrorCode.java,frontend/src/lib/shared/errors.ts,messages/{de,en,es}.json).application.yaml:45already setsforward-headers-strategy: native, soSecurecookies behind Caddy work without extra config. ✓AuthControllerlives in theuserpackage today. The resolved decision moves it to a newauthpackage — that's a file move, not just a new class. MovePasswordResetService+InviteServiceendpoints? No —AuthController(login/logout) is new; the existing password-reset/invite endpoints stay where they are. Theauthpackage owns only login/logout/session lifecycle.Recommendations
V66__recreate_spring_session_tables.sql. Copy the canonical DDL from Spring Session 3.x'sorg/springframework/session/jdbc/schema-postgresql.sqlverbatim. Add a header comment:-- Re-introduces tables dropped by V2. See ADR-012.cookies.seton the form-actioneventonly sets a cookie on the SvelteKit→browser response. The backend will return its ownSet-Cookie: SESSION=<opaque>on the inner/api/auth/loginfetch. The cleanest approach: parse the inner response'sSet-Cookieheader(s) withset-cookie-parser, then re-emit each one viacookies.set(name, value, opts). Don't try to construct the cookie value on the SvelteKit side — the backend is authoritative on the session ID.auth_tokencookies during cutover: browsers will still send the oldauth_tokencookie for a while after deploy. Add a single line tohandleAuth(oruserGroup) that deletes anyauth_tokencookie it sees — purely defensive. One commit, two-line change, prevents a stale-cookie support ticket.SecurityConfigmatcher: add/api/auth/loginand/api/auth/logoutto thepermitAllchain (login is unauthenticated by definition; logout returns 401 if no session, but Spring Security must let the request reach the controller to return that 401 explicitly). Drop/api/auth/reset-token-for-testfrom the permitAll chain only ifAuthE2EControllermoves with Phase 2 (issue confirms it does not — leave it).SESSION. Don't rename —SESSIONis the recognizable, framework-standard name; every developer touching this code in the next 5 years will know to look for it. Setserver.servlet.session.cookie.name: SESSIONexplicitly inapplication.yamlso a future Spring upgrade doesn't change it silently.=padding by default (DefaultCookieSerializer.setUseBase64Encoding(true)). The proposed regex^[A-Za-z0-9_-]{20,}$is close but wrong — it must allow=(or end-anchor before padding). Use^[A-Za-z0-9_=-]{20,}$and add the explicitnot matching "Basic "assertion as a second test.@RequirePermissiondoesn't apply to/api/auth/login— it's unauthenticated by design./api/auth/logoutdoesn't need it either; the requirement is "authenticated", not "has a specific permission". Comment this in the controller so the next reviewer doesn't add@RequirePermissionreflexively.@WebMvcTest(AuthController.class)slice → 200 on valid creds, 401 withINVALID_CREDENTIALSon wrong creds, noSet-Cookieon failure. The audit-log writes go inAuthServiceTestwith mockedAuditService.Open Decisions
LAST_ACCESS_TIME). The 24h absolute lifetime needs custom logic — typically a session attributecreatedAtchecked by a Spring Security filter. Option A: implement the filter now (adds ~30 LOC + tests). Option B: drop "24h absolute" from FR-AUTH-002 and rely on idle only — simpler, fewer moving parts, but a session held active by a polling bot lives forever. Option C: defer the absolute cap to Phase 2 alongside the rest of the revocation work. The spec asserts "Absolute lifetime: 24h" but the ACs only test idle. This needs an explicit pick.🏛️ Markus Keller — Application Architect
Observations
authpackage) is correct.AuthService → UserServiceis a clean cross-domain call. Resist the temptation to expand theuserpackage — login/session lifecycle is its own concern, distinct from "user CRUD".docs/architecture/c4/seq-auth-flow.pumlalready exists in the repo. The doc-update table makes this a blocker, not a concern: the diagram must be rewritten in the same PR (cookie-promotion arrow gone, server session round-trip in).application.yaml:45hasforward-headers-strategy: native— Secure cookies behind Caddy will respectX-Forwarded-Proto. No new infra config needed for that.docs/adr/run up to 011 (011-single-tenant-gitea-runner.md). The new ADR is 012.V33__recreate_spring_session_tables.sql— that was the placeholder number from a stale plan. The actual migration is V66.CLAUDE.mdfiles (root +backend/CLAUDE.md) list "Spring Session JDBC" in the tech stack. Root CLAUDE.md is already accurate-ish (it lists it once the PR lands); backend/CLAUDE.md already lists it stale — both must be touched.Recommendations
AuthControllercallsAuthService.login(email, password)which returns theAppUser.AuthServiceinjectsUserService(for the lookup) and Spring Security'sAuthenticationManager(for password verification).AuthServicedoes NOT injectAppUserRepositorydirectly — that's the existing rule.spring.session.jdbc.cleanup-cronor accept the default. Application code doesn't own this responsibility.spring.session.timeout: 28800sis correct for the idle timeout, but understand the semantics — this isMaxInactiveIntervalInSeconds, not absolute. The hint in the issue body and the FR are aligned, but document this in the ADR consequences section so future-Markus doesn't expect absolute-cap behavior from a single config line.AuthControllerfrom touchingAppUserRepository. Since this is one new class and the rule for*Serviceboundaries already exists, an ArchUnit assertionAuthController_should_only_call_AuthServiceis overkill for a single endpoint. Skip the rule.Open Decisions
🔐 Nora Steiner — Application Security Engineer
Observations
login/+page.server.ts:45const isHttps = url.protocol === 'https:'is exactly the footgun I'd flag if I hadn't already. Behind Caddy,url.protocolreflects what SvelteKit sees from the proxy (http), so this currently writesSecure: falsein production. The 24h Basic credential then rides the wire in plaintext on any HTTPS-stripping proxy. Killing this branch is correct and overdue — and that vulnerability is today's state, not theoretical.SecurityConfig.java:54disables CSRF with a long comment explaining thatSameSite=strictis the load-bearing defense. That comment stays load-bearing for Phase 1 too. CSRF re-enable moves to #524, so the SameSite=strict + CORS-default combination is still the entire CSRF story for this release. Update the comment so it references the SESSION cookie, not the (now-deleted)auth_tokencookie.secure: truerequirement in FR-AUTH-002 is "unconditional in non-dev profiles". I want to add: the dev profile is the only legitimate exception, andapplication-dev.yamlis the only place a non-secure cookie is acceptable. Encode this in config, not a JS branch.AppUseris the response body of/api/users/metoday. Returning it from/api/auth/login(FR-AUTH-001) is fine — but the entity carriespasswordHash. Verify the existing/api/users/mealready strips it (it does in current behavior — confirm with a test before assuming).migrateSession). Verify nothing inSecurityConfigoverrides it. After successful authentication a brand-new session ID must be issued; the test for this is: log in twice in a row, capture the SESSION cookie each time, assert they differ.Recommendations
AuthService.loginMUST returnINVALID_CREDENTIALSfor both "unknown email" and "wrong password" — and the timing must be similar enough that an attacker can't distinguish via response latency. Concretely: even on "user not found", run a BCrypt verify against a dummy hash. ~100ms either way. Add a regression test that records p50 latency for "unknown user" and "wrong password" and asserts the gap is <30ms.{email, ip, ua}, NEVER the attempted password. Add a Semgrep rule:/api/auth/loginwith bad creds → 401, noSet-Cookie; arbitrary/api/*endpoint with no session cookie → 401, no auth challenge. The currenthttpBasic(Customizer.withDefaults())issuesWWW-Authenticate: Basic, which is the cause of the browser popup. Once removed, unauthenticated requests must return 401 with noWWW-Authenticateheader. Test it.HttpOnly=true,Secure=true(in non-dev profile),SameSite=Strict,Path=/,Max-Age=28800. One missing attribute is a footgun; cover all five in one parameterized test.auth_tokendefense in depth: after deploy, browsers will keep sending the oldauth_tokencookie for a while. Worth explicitly deleting it server-side viaSet-Cookie: auth_token=; Max-Age=0; Path=/. Felix called this out already — I'm seconding it from the security side, because a stale credential lying around in a browser is a credential we haven't revoked.app.admin.password: admin123default inapplication.yaml:77— this is a known dev fallback, but Familienarchiv runs in a "self-hosted by a hobbyist" model. Add a startup guard: ifspring.profiles.activedoes not containdevore2eANDapp.admin.password == "admin123", log anERRORand refuse to start. Not strictly in scope for this PR but the same surface area, same file. Worth adding while we're here.Open Decisions
SESSIONcookie name visibility: the default Spring Session cookie nameSESSIONis technology-disclosing — an attacker probing the app learns "Spring Session is in use" from the cookie name alone. Rename tofa_session? Or keepSESSIONfor developer clarity (Felix's argument)? Trade-off is real but small. My preference: rename tofa_session— disclosure-minimization is cheap and prevents one fingerprinting signal. But Felix's recognizability argument is also valid; I won't block on it.🧪 Sara Holt — QA Engineer
Observations
AuditKind.javacurrently has 11 values — none for auth events. The PR adds three (LOGIN_SUCCESS,LOGIN_FAILED,LOGOUT). That makes them new code surface that the existing tests don't touch; coverage for them goes from zero.backend/CLAUDE.md), not 80%. The DoD says "Hold the 88% branch line" — that means the newAuthService+AuthControllerneed ≥88% own coverage to not pull the global number down.vitest.configcoverage thresholds are 80% lines/branches/functions/statements on the frontend server project. The new login form-action code lives insrc/lib/shared/server/**(via the API client) andsrc/routes/login/+page.server.ts. Verify the latter is in the coverageincludeglob — currently the includes are narrow and login may slip through.Recommendations
=padding may appear; ASCII range may also include~in some configurations. Hard-coding a regex that fails at runtime is worse than no regex.LAST_ACCESS_TIME> 8h". Concrete approach:@WithMockUserdoesn't work with server-side session tests —@WithMockUseroperates in Spring Security'sSecurityContext, not Spring Session. For integration tests that need an authenticated session, use aTestRestTemplatethat performs an actualPOST /api/auth/loginfirst, captures the cookie, and uses it on subsequent requests. This pattern reusable inAbstractAuthenticatedIntegrationTest.assertSessionEstablished()method.GET /api/users/meagainst the new session-lookup path, baseline against the current Basic-decode path. p95 added latency goes in the PR body as a real number, not "assumed <5ms".Open Decisions
None from my angle — the test strategy in the issue is concrete enough to execute. The choices left are technical (regex shape, k6 yes/no in CI) and the implementer can pick within the bounds the spec already sets.
🔧 Tobias Wendt — DevOps & Platform Engineer
Observations
application.yaml:45already hasforward-headers-strategy: native. Behind Caddy, Spring sees the originalhttpsscheme and will setSecurecookies correctly. The cutover requires zero Caddy config changes. Confirmed by readingdocs/infrastructure/production-compose.mdpatterns and the existing seq-auth-flow.docker-compose.ci.ymlalready provisions a real Postgres + MinIO for CI. Spring Session JDBC tables get created by Flyway in CI on the first test run — no CI overlay change needed.V33__. Latest on disk isV65__. The new migration is V66. (Same correction Felix and Markus flagged.)auth_tokencookie hasMax-Age=60*60*24(24h). Old browsers will keep sending it after deploy. There's noDomainset so it's bound to the apex host — once the backend ignores it, it's a slowly-decaying artifact.Recommendations
-- Re-introduces tables dropped by V2 (#523). See ADR-012.Copy the canonical DDL from Spring Session 3.xorg/springframework/session/jdbc/schema-postgresql.sql— do not invent the schema.pom.xml. The BOM transitive will pick whatever Spring Boot 4 bundles, which is fine today but will silently shift on the next Boot bump. Explicit pin = reproducible build. Add a Renovate rule for the version so it stays current without being a moving target.spring.session.timeout: 28800sinapplication.yaml(8h). Verify it doesn't conflict withserver.servlet.session.timeout(default 30min) — Spring Session overrides but a staleserver.servlet.session.timeoutis a misleading config smell. Remove it if set.spring_sessiontable will hold ≤ (active users × idle hours) rows = max a few hundred. Indexing is already canonical in Spring's DDL.spring_sessionandspring_session_attributesinpg_dump(default behavior). Document indocs/infrastructure/production-compose.mdthat on restore, all sessions are effectively invalidated — every user must re-log-in. That's correct behavior post-restore, no change needed.spring_session_jdbc_active_sessionsgauge via a@Scheduledquery ofSELECT COUNT(*) FROM spring_session. Useful for "is the cleanup task running?" observability. Defer to Phase 2 — not in this PR's scope.Open Decisions
None from infrastructure. The deploy is a single backend+frontend rollout to the same Compose stack, no Caddy changes, no DNS, no firewall rules.
🎨 Leonie Voss — UX & Accessibility
Observations
aria-live="polite", the German wording, and that there's no idle-timeout pop-up. All correct.min-h-[44px]on the login button (WCAG 2.2 SC 2.5.8). I checked the file — the login template is insrc/routes/login/+page.svelte(not directly readable in this comment but worth confirming). Touch-target compliance is in scope for the same PR per the hint.Recommendations
/login?reason=expired— move focus to the email input automatically on page load. Saves the senior user one Tab keystroke and signals "here's what to do next."/loginpath — that breaks keyboard-only landing on the first heading for screen readers. Only whenreason=expired.min-h-[44px](currently? verify — if not, fix).min-h-[44px]per the hint, plusw-fullon small screens so it's a fat tap target without horizontal aim.border-amber-400 bg-amber-50remaps in dark mode. If the project uses semantic tokens for status colors, preferborder-status-info bg-status-info-softand define the dark variants inlayout.css. If only the brand tokens are dark-mode-aware today, define new status tokens in this PR — banner colors must work in both themes (axe will catch contrast failures).motion-safe:transition-*. No bounce/slide on first render — keep it calm. Senior audience + vestibular sensitivity considerations apply.aria-live="polite". Some toast components auto-dismiss after 5s; this is a banner, not a toast. It dismisses when the user submits the form or navigates away.Open Decisions
None from my angle — the visual + a11y decisions are concrete enough to execute. The trade-offs (banner color, explainer wording, focus on expired-only) all have a clear right answer for this audience.
📋 Elicit — Requirements Engineer
Observations
Recommendations
main" — this is a process gate, not a code criterion. Move it out of the DoD list into a separate "Pre-merge ops checklist" or rename the section so it's clear the comms are pre-merge external work, not "did I write the code". Otherwise it sits in the PR template and gets ticked without anyone actually sending the WhatsApp message.Open Decisions
🗳️ Decision Queue — Action Required
3 decisions need your input before implementation starts.
Architecture / Security
LAST_ACCESS_TIME). FR-AUTH-002 promises "Absolute lifetime: 24 h" but the ACs only test idle. Options:createdAtsession attribute on every request; ~30 LOC + tests. Closes the "session held alive by a polling tab forever" gap.Raised by: Felix, Markus. Markus's weak preference: B. Felix is neutral.
Security
SESSION, which fingerprints the framework. Options:fa_session— minor disclosure-minimization win; one extra config line; not a recognized name for future developers.SESSION— framework-standard, instantly recognizable, no extra config to maintain.Raised by: Nora prefers A (will not block). Felix prefers B (recognizability).
Process / Spec
Raised by: Elicit. Either is fine — current form is just ambiguous.