fix(auth): move the BankID flow into a signed, user-gated, single-use cookie (#1625)

* fix(auth): move the BankID flow into a signed, single-use, confirm-on-resume cookie

A user's BankID signup identified successfully four times and created no
account. His screenshots show four tabs, one on the finished "Verifierad med
BankID, ange e-post" step, and the tab he was looking at showing the idle
button. Prod agreed: no bankid_identities row, no auth.users row.

On iOS outside plain Safari the BankID return URL is handed to the OS, which
opens a NEW tab. The session lived in per-tab sessionStorage, so that tab
started empty and rendered the start button while the completed flow sat
stranded. Login hid it (self-finishing, cookie-backed session); signup waits
for a human to type an e-mail into the stranded tab, so it dies there.

The session id is no longer handed to the browser. It lives in a signed
__Host- HttpOnly cookie set at /start; /poll, /complete, /link and /cancel
read it. Cookies are shared by every tab of the origin, which is what the
handoff needed. The id had to leave the client because it is an
unauthenticated bearer credential: /poll was skipAuth and returned
user.personalNumber, and /complete with mode 'login' returns a tokenHash that
verifyOtp turns into a session, MFA skipped for bankid_linked accounts.

A completed identification must never be consumed by whoever merely opens the
page. A shared cookie plus a shared machine means the tab that finds a
completed flow cannot prove the person at it is the one who made it, and no
client-side token can prove otherwise: nothing survives an iOS same-tab reload
yet dies on reopen-closed-tab / session restore / tab duplication. So a resume
is never automatic. The mount probe routes any found live flow to a confirm
card ("Fortsätt bara om det var du") that reveals no name, and only that click
polls and consumes. Auto-consume happens only inside the live component
instance that called startSession (desktop QR; the pre-navigation mobile
launch), which by construction is the originator. Cost: one tap after
returning from the BankID app on iOS, exactly where the reported bug lives;
desktop and Android never hit the resume path.

The rest is defence the four review rounds proved load-bearing:
- __Host- with Path=/ and unconditional Secure, so a script cannot plant the
  same name at a longer path; readBankIdFlow fails closed on duplicates and on
  a malformed percent-escape.
- Single-use is a unique index (bankid_consumed_sessions), claimed before
  generateLink, not a Set-Cookie. Fail-closed on any non-23505 error, so the
  migration MUST be applied before the code.
- A link flow requires auth at /start and pins userId; /link rejects a flow
  owned by anyone else, before any TIC call. mode is pinned and /poll rejects a
  body mode that does not match, so a login session cannot finish through the
  signup panel. /poll withholds the holder name from a probe. The 900s
  verified-step window is capped by MAX_TOTAL_LIFE from a signed startedAt.
  /poll never clears the cookie (an untargeted Set-Cookie would delete a newer
  flow); only /cancel and terminal /complete + /link exits clear. Avbryt holds
  a 'cancelling' state until /cancel resolves so a new /start cannot race the
  clear. Session id is logged only as an 8-char prefix.

The launch is untouched: iOS keeps its return URL, Android keeps redirect=null
(#194 closed that path deliberately).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(auth): bind BankID actions to the resumed flow

* docs: record BankID staging migration drift

* fix(auth): address BankID PR review

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mattsson
2026-08-15 23:07:29 +02:00
committed by GitHub
co-authored by Claude Opus 5
parent 2deea05d42
commit edfdbe2d2a
13 changed files with 2294 additions and 227 deletions
+3 -1
View File
@@ -683,6 +683,8 @@ export function createMockRequest(
method?: string
body?: unknown
searchParams?: Record<string, string>
/** Extra request headers, e.g. `cookie` for routes that read one. */
headers?: Record<string, string>
}
): Request {
const fullUrl = new URL(url, 'http://localhost:3000')
@@ -693,7 +695,7 @@ export function createMockRequest(
}
return new Request(fullUrl.toString(), {
method: options?.method || 'GET',
headers: { 'Content-Type': 'application/json' },
headers: { 'Content-Type': 'application/json', ...options?.headers },
...(options?.body ? { body: JSON.stringify(options.body) } : {}),
})
}
@@ -0,0 +1,99 @@
import { randomUUID } from 'node:crypto'
import { describe, expect, it } from 'vitest'
import { getPool, withUserContext } from './setup'
import { insertAuthUser } from './fixtures'
/**
* bankid_consumed_sessions (migration 20260815120000) is the single-use guard
* for a BankID identification.
*
* The flow now lives in a cookie, so every tab of a browser shares one session
* and two of them can observe `status: complete` in the same poll tick. Both
* would call generateLink(), and the second magic link invalidates the first,
* so the tab the user is looking at may be the one that fails. Expiring the
* cookie on the way out does not prevent that: a Set-Cookie only applies once a
* response reaches the browser, and both requests already carried it.
*
* These pin the two properties the application actually relies on: the claim is
* atomic, and the table is invisible to end users. A session id is a bearer
* credential for a personnummer and for a Supabase session, and the row set is
* a record of who authenticated and when.
*/
describe('bankid_consumed_sessions (pg)', () => {
it('lets exactly one claim win for a given session id', async () => {
const sessionId = `sess-${randomUUID()}`
await getPool().query(
'INSERT INTO public.bankid_consumed_sessions (session_id) VALUES ($1)',
[sessionId],
)
await expect(
getPool().query(
'INSERT INTO public.bankid_consumed_sessions (session_id) VALUES ($1)',
[sessionId],
),
).rejects.toMatchObject({ code: '23505' })
})
it('does not collide across different sessions', async () => {
const a = `sess-${randomUUID()}`
const b = `sess-${randomUUID()}`
await getPool().query(
'INSERT INTO public.bankid_consumed_sessions (session_id) VALUES ($1), ($2)',
[a, b],
)
const { rows } = await getPool().query<{ n: number }>(
'SELECT count(*)::int AS n FROM public.bankid_consumed_sessions WHERE session_id IN ($1, $2)',
[a, b],
)
expect(rows[0]!.n).toBe(2)
})
it('stamps consumed_at so spent sessions can be aged out', async () => {
const sessionId = `sess-${randomUUID()}`
await getPool().query(
'INSERT INTO public.bankid_consumed_sessions (session_id) VALUES ($1)',
[sessionId],
)
const { rows } = await getPool().query<{ consumed_at: Date }>(
'SELECT consumed_at FROM public.bankid_consumed_sessions WHERE session_id = $1',
[sessionId],
)
expect(rows[0]!.consumed_at).toBeInstanceOf(Date)
})
it('has RLS on with no policies, so authenticated users see nothing', async () => {
const { rows: flags } = await getPool().query<{ relrowsecurity: boolean }>(
`SELECT c.relrowsecurity
FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace
WHERE n.nspname = 'public' AND c.relname = 'bankid_consumed_sessions'`,
)
expect(flags[0]!.relrowsecurity).toBe(true)
const { rows: policies } = await getPool().query<{ n: number }>(
`SELECT count(*)::int AS n FROM pg_policies
WHERE schemaname = 'public' AND tablename = 'bankid_consumed_sessions'`,
)
expect(policies[0]!.n).toBe(0)
// And the guard actually bites for a real user, not just on paper: the
// handlers write through the service role, which bypasses RLS.
const sessionId = `sess-${randomUUID()}`
await getPool().query(
'INSERT INTO public.bankid_consumed_sessions (session_id) VALUES ($1)',
[sessionId],
)
const userId = await insertAuthUser()
await withUserContext(userId, async (client) => {
const { rows } = await client.query<{ n: number }>(
'SELECT count(*)::int AS n FROM public.bankid_consumed_sessions WHERE session_id = $1',
[sessionId],
)
expect(rows[0]!.n).toBe(0)
})
})
})