Mattsson
edfdbe2d2a
* 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>
54 lines
2.8 KiB
SQL
54 lines
2.8 KiB
SQL
-- Makes a BankID session usable exactly once, atomically.
|
|
--
|
|
-- The BankID flow now lives in a cookie rather than per-tab client storage, so
|
|
-- every tab of the browser shares one session. That is what lets the tab the
|
|
-- BankID app returns the user to finish a flow another tab started, and it is
|
|
-- also why two tabs can observe `status: complete` inside the same poll tick
|
|
-- and both try to finish it.
|
|
--
|
|
-- Both would call supabase.auth.admin.generateLink(), and the second magic link
|
|
-- invalidates the first, so whichever tab the user is actually looking at may
|
|
-- be the one that fails with "Inloggningen med BankID misslyckades".
|
|
--
|
|
-- Clearing the flow cookie on the way out does NOT prevent this: a Set-Cookie
|
|
-- only takes effect once a response reaches the browser, so two requests that
|
|
-- both already carried the cookie both pass the check. There is no shared
|
|
-- process memory to coordinate in on serverless either. A unique index is the
|
|
-- only thing here that is genuinely atomic: the second INSERT raises 23505 and
|
|
-- that request stops before minting anything.
|
|
--
|
|
-- Rows are bookkeeping, not business data: a BankID session id is meaningless
|
|
-- to TIC within the hour, and nothing reads a row back. They are nonetheless
|
|
-- retained rather than deleted, so a replayed id is still recognised as spent
|
|
-- instead of silently becoming reusable.
|
|
--
|
|
-- There is deliberately NO cleanup job in this migration. At current volume
|
|
-- (single-digit BankID authentications per day) the table takes decades to
|
|
-- reach a megabyte, and a pruning cron is a moving part that can fail open.
|
|
-- consumed_at is indexed so an age-based prune can be added later without a
|
|
-- second migration. What the retention does mean, and is worth being explicit
|
|
-- about: this is a permanent record of when each BankID identification
|
|
-- happened, which is why the table is service-role only.
|
|
|
|
CREATE TABLE public.bankid_consumed_sessions (
|
|
session_id TEXT PRIMARY KEY,
|
|
consumed_at TIMESTAMPTZ NOT NULL DEFAULT now()
|
|
);
|
|
|
|
-- For a future age-based prune; nothing queries it today.
|
|
CREATE INDEX idx_bankid_consumed_sessions_consumed_at
|
|
ON public.bankid_consumed_sessions (consumed_at);
|
|
|
|
ALTER TABLE public.bankid_consumed_sessions ENABLE ROW LEVEL SECURITY;
|
|
|
|
-- No policies at all, deliberately. The table is written and read only by the
|
|
-- TIC extension's BankID handlers through createServiceClient(), which bypasses
|
|
-- RLS. Nothing user-facing may see which sessions exist: the ids are bearer
|
|
-- credentials for a personnummer and for a Supabase session, and the row set is
|
|
-- a record of who authenticated and when.
|
|
|
|
COMMENT ON TABLE public.bankid_consumed_sessions IS
|
|
'Spent BankID session ids. The PK is the single-use guard for /bankid/complete and /bankid/link; see migration header.';
|
|
|
|
NOTIFY pgrst, 'reload schema';
|