Signup creates session but never authenticates the user #3

New Issue

marcel · 2026-04-02T11:20:16+02:00

marcel commented

2026-04-02 11:20:16 +02:00

Problem

In AuthController.signup(), a session is created via httpRequest.getSession(true) but the user is never authenticated into the Spring SecurityContext. The session cookie is issued but has no associated authentication, which is confusing and potentially exploitable.

Affected files

AuthController.java:28-29

UserResponse user = authService.signup(request);
httpRequest.getSession(true);  // session created, never populated

Recommended fix

Either:

Authenticate the user into the SecurityContext after signup (SecurityContextHolder.getContext().setAuthentication(...)) if auto-login after signup is desired, or
Remove the getSession(true) call entirely if signup should not create a session.

Severity

Critical — orphaned sessions can cause session confusion and bypass session-fixation protection.

## Problem In `AuthController.signup()`, a session is created via `httpRequest.getSession(true)` but the user is never authenticated into the Spring SecurityContext. The session cookie is issued but has no associated authentication, which is confusing and potentially exploitable. ## Affected files - `AuthController.java:28-29` ```java UserResponse user = authService.signup(request); httpRequest.getSession(true); // session created, never populated ``` ## Recommended fix Either: 1. Authenticate the user into the SecurityContext after signup (`SecurityContextHolder.getContext().setAuthentication(...)`) if auto-login after signup is desired, or 2. Remove the `getSession(true)` call entirely if signup should not create a session. ## Severity Critical — orphaned sessions can cause session confusion and bypass session-fixation protection.

marcel added the kind/security priority/critical labels 2026-04-02 11:20:57 +02:00

marcel commented

2026-04-03 09:27:23 +02:00

👨‍💻 Kai — Frontend Engineer

This bug directly affects the signup → auto-login → redirect flow that the frontend depends on. I need clarity on the intended post-signup behavior before I can implement the correct frontend handling.

Current frontend assumption:
The frontend likely does one of two things after a successful signup POST:

Expects to be auto-authenticated and redirects to the app (e.g., /plan)
Redirects to login with a "signup successful" message

If option 1 is intended but the session is never authenticated (which is the bug), the redirect to /plan will immediately 401 back to login, creating a confusing loop.

Questions for the team:

Is auto-login after signup the intended product behavior? This is a UX/product decision that determines which fix path the backend takes.
If we go with option 1 (auto-login after signup): the backend fix authenticates the user into the SecurityContext after signup. My +page.server.ts signup action should then redirect to /plan (or onboarding). Confirm the redirect target.
If we go with option 2 (redirect to login): the getSession(true) call should just be removed. My signup action redirects to /login?signup=success and the login page shows a confirmation banner.

What I'll handle regardless:

The +page.server.ts signup action needs to handle both 201 (success) and any error responses cleanly
The session state in hooks.server.ts needs to correctly detect an unauthenticated session (the orphaned session from this bug) and treat it as "not logged in"

No code changes from me until the backend fix approach is decided — the correct frontend flow depends entirely on which fix path is chosen.

## 👨‍💻 Kai — Frontend Engineer This bug directly affects the signup → auto-login → redirect flow that the frontend depends on. I need clarity on the intended post-signup behavior before I can implement the correct frontend handling. **Current frontend assumption:** The frontend likely does one of two things after a successful signup POST: 1. Expects to be auto-authenticated and redirects to the app (e.g., `/plan`) 2. Redirects to login with a "signup successful" message If option 1 is intended but the session is never authenticated (which is the bug), the redirect to `/plan` will immediately 401 back to login, creating a confusing loop. **Questions for the team:** - Is auto-login after signup the intended product behavior? This is a UX/product decision that determines which fix path the backend takes. - If we go with option 1 (auto-login after signup): the backend fix authenticates the user into the SecurityContext after signup. My `+page.server.ts` signup action should then redirect to `/plan` (or onboarding). Confirm the redirect target. - If we go with option 2 (redirect to login): the `getSession(true)` call should just be removed. My signup action redirects to `/login?signup=success` and the login page shows a confirmation banner. **What I'll handle regardless:** - The `+page.server.ts` signup action needs to handle both 201 (success) and any error responses cleanly - The session state in `hooks.server.ts` needs to correctly detect an unauthenticated session (the orphaned session from this bug) and treat it as "not logged in" **No code changes from me until the backend fix approach is decided** — the correct frontend flow depends entirely on which fix path is chosen.

marcel commented

2026-04-03 09:27:34 +02:00

🏗️ Backend Engineer — Spring Boot / PostgreSQL Specialist

Marked Critical — correctly. The orphaned session is the dangerous part. Let me lay out the two fix paths precisely.

Fix path A: Auto-login after signup (authenticate into SecurityContext)

UserResponse user = authService.signup(request);
UsernamePasswordAuthenticationToken auth =
    new UsernamePasswordAuthenticationToken(user.email(), null, List.of(new SimpleGrantedAuthority("ROLE_USER")));
SecurityContextHolder.getContext().setAuthentication(auth);
HttpSession session = httpRequest.getSession(true);
session.setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY,
    SecurityContextHolder.getContext());

This is the correct implementation if auto-login is desired. The session is created AND tied to a real Authentication object.

Fix path B: No session at signup (remove getSession(true))

Just remove the httpRequest.getSession(true) call. Signup returns the user resource, no session created. Frontend redirects to login.

My strong recommendation: Fix path A (auto-login) — it's the better UX and the more secure option because:

The orphaned session from the current bug is eliminated
Session fixation protection fires correctly (Spring rotates the session ID on authentication)
The user immediately has a properly established authenticated session

Questions:

What roles should the newly signed-up user have? Just ROLE_USER? Or is there a UserDetailsService lookup we should go through to load the full UserDetails?
Should the authService.signup() return enough information to build the Authentication object (email, roles), or does it need to do a follow-up loadUserByUsername call?
Is authService.signup() transactional? The session should only be created if the DB write succeeds — want to confirm the failure path doesn't leave a session behind.

## 🏗️ Backend Engineer — Spring Boot / PostgreSQL Specialist Marked Critical — correctly. The orphaned session is the dangerous part. Let me lay out the two fix paths precisely. **Fix path A: Auto-login after signup (authenticate into SecurityContext)** ```java UserResponse user = authService.signup(request); UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(user.email(), null, List.of(new SimpleGrantedAuthority("ROLE_USER"))); SecurityContextHolder.getContext().setAuthentication(auth); HttpSession session = httpRequest.getSession(true); session.setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, SecurityContextHolder.getContext()); ``` This is the correct implementation if auto-login is desired. The session is created AND tied to a real `Authentication` object. **Fix path B: No session at signup (remove `getSession(true)`)** Just remove the `httpRequest.getSession(true)` call. Signup returns the user resource, no session created. Frontend redirects to login. **My strong recommendation: Fix path A (auto-login)** — it's the better UX and the more secure option because: - The orphaned session from the current bug is eliminated - Session fixation protection fires correctly (Spring rotates the session ID on authentication) - The user immediately has a properly established authenticated session **Questions:** - What roles should the newly signed-up user have? Just `ROLE_USER`? Or is there a `UserDetailsService` lookup we should go through to load the full `UserDetails`? - Should the `authService.signup()` return enough information to build the `Authentication` object (email, roles), or does it need to do a follow-up `loadUserByUsername` call? - Is `authService.signup()` transactional? The session should only be created if the DB write succeeds — want to confirm the failure path doesn't leave a session behind.

marcel commented

2026-04-03 09:27:45 +02:00

🧪 QA Engineer

Critical severity means I want this covered with both unit and integration tests before the fix ships. Here's the full test plan.

Current behavior to confirm (regression baseline):

Sign up → receive a session cookie → use that cookie to hit an authenticated endpoint → confirm it currently returns 401 (proving the session has no auth). This documents the bug and becomes the test that must fail before the fix and pass after.

Fix path A tests (auto-login after signup):

POST /v1/auth/signup with valid data → 201, response contains user info, Set-Cookie with JSESSIONID
Use the returned session cookie to GET /v1/auth/me (or any authenticated endpoint) → 200 (not 401) — this is the key test proving the session is authenticated
Attempt signup with duplicate email → 409, no session created
Attempt signup with invalid data (missing fields, bad email format) → 400/422, no session created
Verify session fixation protection: the session ID in the cookie after signup should differ from any pre-signup session ID (if applicable)

Fix path B tests (no session at signup):

POST /v1/auth/signup → 201, no Set-Cookie header (or cookie cleared)

Edge cases for either fix:

What happens if authService.signup() throws mid-execution? Is any partial session state left behind?
Concurrent signups with the same email — race condition between uniqueness check and insert. Is there a DB constraint (not just application-level check) that prevents duplicates?
Signup with extremely long field values — what are the column length limits and are they enforced with 422, not 500?

E2E coverage: The signup → dashboard flow should be a critical E2E test tagged @critical. It's the first thing a new user does — if it's broken, nothing else matters.

## 🧪 QA Engineer Critical severity means I want this covered with both unit and integration tests before the fix ships. Here's the full test plan. **Current behavior to confirm (regression baseline):** - Sign up → receive a session cookie → use that cookie to hit an authenticated endpoint → confirm it currently returns `401` (proving the session has no auth). This documents the bug and becomes the test that must fail before the fix and pass after. **Fix path A tests (auto-login after signup):** - `POST /v1/auth/signup` with valid data → `201`, response contains user info, `Set-Cookie` with `JSESSIONID` - Use the returned session cookie to `GET /v1/auth/me` (or any authenticated endpoint) → `200` (not `401`) — this is the key test proving the session is authenticated - Attempt signup with duplicate email → `409`, no session created - Attempt signup with invalid data (missing fields, bad email format) → `400`/`422`, no session created - Verify session fixation protection: the session ID in the cookie after signup should differ from any pre-signup session ID (if applicable) **Fix path B tests (no session at signup):** - `POST /v1/auth/signup` → `201`, no `Set-Cookie` header (or cookie cleared) **Edge cases for either fix:** - What happens if `authService.signup()` throws mid-execution? Is any partial session state left behind? - Concurrent signups with the same email — race condition between uniqueness check and insert. Is there a DB constraint (not just application-level check) that prevents duplicates? - Signup with extremely long field values — what are the column length limits and are they enforced with 422, not 500? **E2E coverage:** The signup → dashboard flow should be a critical E2E test tagged `@critical`. It's the first thing a new user does — if it's broken, nothing else matters.

marcel commented

2026-04-03 09:27:57 +02:00

🔒 Sable — Security Engineer

This is correctly rated Critical. The orphaned session is the most dangerous part — let me explain the attack surface clearly.

The orphaned session attack vector:

httpRequest.getSession(true) creates a real HTTP session with a JSESSIONID cookie. The session exists in the session store but has no Authentication in its SecurityContext. This means:

Session fixation risk: If an attacker can predict or influence the session ID before signup (e.g., by pre-visiting the site and getting a session cookie, then injecting it into the signup request), they could potentially fix the session ID. Spring's session fixation protection (migrateSession()) is supposed to fire on authentication — but since authentication never happens here, that protection never runs. The session is created but fixation protection is bypassed.
Session confusion: The orphaned session persists until expiration. If another code path incorrectly treats "session exists" as "user is authenticated," the unauthenticated session could be misused.
Resource exhaustion: If signup is called at scale (even legitimately), orphaned sessions accumulate in the session store until they expire. Combined with the lack of rate limiting on /v1/auth/signup (see issue #1), this is a low-effort session store DoS vector.

Security requirements for the fix:

Whichever fix path is chosen, the getSession(true) call must either be removed or immediately followed by populating the SecurityContext — no intermediate state where a session exists without authentication
If auto-login is chosen, session fixation protection must fire: verify the session ID changes between pre-auth and post-auth states
Confirm the signup endpoint is covered by rate limiting (issue #1) — the combination of orphaned sessions + no rate limiting is the DoS scenario

Audit log consideration: Should signup events be logged? Not in admin_audit_log (that's admin actions), but a security event log that includes: timestamp, IP address, username, success/failure. Useful for detecting account creation abuse patterns.

## 🔒 Sable — Security Engineer This is correctly rated Critical. The orphaned session is the most dangerous part — let me explain the attack surface clearly. **The orphaned session attack vector:** `httpRequest.getSession(true)` creates a real HTTP session with a `JSESSIONID` cookie. The session exists in the session store but has no `Authentication` in its `SecurityContext`. This means: 1. **Session fixation risk:** If an attacker can predict or influence the session ID before signup (e.g., by pre-visiting the site and getting a session cookie, then injecting it into the signup request), they could potentially fix the session ID. Spring's session fixation protection (`migrateSession()`) is supposed to fire on authentication — but since authentication never happens here, that protection never runs. The session is created but fixation protection is bypassed. 2. **Session confusion:** The orphaned session persists until expiration. If another code path incorrectly treats "session exists" as "user is authenticated," the unauthenticated session could be misused. 3. **Resource exhaustion:** If signup is called at scale (even legitimately), orphaned sessions accumulate in the session store until they expire. Combined with the lack of rate limiting on `/v1/auth/signup` (see issue #1), this is a low-effort session store DoS vector. **Security requirements for the fix:** - Whichever fix path is chosen, the `getSession(true)` call must either be removed or immediately followed by populating the SecurityContext — no intermediate state where a session exists without authentication - If auto-login is chosen, session fixation protection must fire: verify the session ID changes between pre-auth and post-auth states - Confirm the signup endpoint is covered by rate limiting (issue #1) — the combination of orphaned sessions + no rate limiting is the DoS scenario **Audit log consideration:** Should signup events be logged? Not in `admin_audit_log` (that's admin actions), but a security event log that includes: timestamp, IP address, username, success/failure. Useful for detecting account creation abuse patterns.

marcel commented

2026-04-03 09:28:07 +02:00

🎨 Atlas — UI/UX Designer

The fix choice here has a direct UX consequence that I want to flag as a design decision, not just a backend implementation detail.

The two paths feel very different to a user:

Auto-login after signup (Fix A): User fills out signup form, submits, lands directly in the app. Zero friction. This is the modern standard — almost every app does this. The design assumes a single flow: signup form → onboarding/home. I'd need to design an onboarding screen (if it doesn't exist yet) that a new user sees immediately after signup.
Redirect to login after signup (Fix B): User fills out signup form, gets a success message, then has to log in again. Adds one extra step. This pattern is becoming rare for consumer apps, though it's still common for more enterprise/formal flows. If we go this route, the login screen needs a "signup successful — please log in" banner state.

My recommendation: Fix A (auto-login). It removes friction at the most critical moment — first-time user activation. A user who has to log in twice after signing up has already had a worse first experience than they should.

Design questions:

Is there an onboarding or "getting started" screen designed for new users? If auto-login is chosen, where does the user land immediately after signup? The empty state of the planner (C1) with a prompt to create their first plan? Or a dedicated welcome screen?
What does the signup form error state look like for a duplicate email? The message should be informative ("An account with this email already exists. Log in instead?") with a link — not just a red border.
Should the signup form have a password strength indicator? Currently out of scope for v1, but flagging it as a UX consideration since we're touching this flow.

## 🎨 Atlas — UI/UX Designer The fix choice here has a direct UX consequence that I want to flag as a design decision, not just a backend implementation detail. **The two paths feel very different to a user:** - **Auto-login after signup (Fix A):** User fills out signup form, submits, lands directly in the app. Zero friction. This is the modern standard — almost every app does this. The design assumes a single flow: `signup form → onboarding/home`. I'd need to design an onboarding screen (if it doesn't exist yet) that a new user sees immediately after signup. - **Redirect to login after signup (Fix B):** User fills out signup form, gets a success message, then has to log in again. Adds one extra step. This pattern is becoming rare for consumer apps, though it's still common for more enterprise/formal flows. If we go this route, the login screen needs a "signup successful — please log in" banner state. **My recommendation: Fix A (auto-login).** It removes friction at the most critical moment — first-time user activation. A user who has to log in twice after signing up has already had a worse first experience than they should. **Design questions:** - Is there an onboarding or "getting started" screen designed for new users? If auto-login is chosen, where does the user land immediately after signup? The empty state of the planner (C1) with a prompt to create their first plan? Or a dedicated welcome screen? - What does the signup form error state look like for a duplicate email? The message should be informative ("An account with this email already exists. Log in instead?") with a link — not just a red border. - Should the signup form have a password strength indicator? Currently out of scope for v1, but flagging it as a UX consideration since we're touching this flow.

marcel referenced this issue

2026-04-03 09:30:04 +02:00

Add rate limiting on auth and invite endpoints #1

marcel referenced this issue

2026-04-04 17:03:09 +02:00

feat: Add-to-Plan flows — C4 recipe picker, C5 quick actions, C6 day picker #42

marcel referenced this issue

2026-04-06 20:00:04 +02:00

feat: D1 — Shopping list (Issue #30) #43

Sign in to join this conversation.