diff --git a/DECISIONS.md b/DECISIONS.md index 904f1aea..b7101c32 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -993,6 +993,10 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-14] /sie-data validation stays newest-file-only (not per-file, not on the merged parse): preserves exactly which datasets are accepted today, and validateSIEFile assumes single-file invariants (balance yearIndexes relative to ONE current year) that mergeParsedSIEFiles deliberately does not preserve. Older files' problems still surface per-file at import time. [2026-08-14] SKV manual-verifikat deep link payload moved from URL params to single-use sessionStorage (supersedes same-day URL-params decision): compliance swarm flagged financial data in query strings landing in history/access logs/Referer (GDPR Art.5(1)(f), ISO A.8.12); URL now carries only the opaque row id. [2026-08-14] SKV prefill sessionStorage XSS window accepted as residual risk (ISO A.8.12 low, swarm PR #1621): script execution already implies full ledger read via authenticated APIs; a server-issued staging token adds a roundtrip, not protection. Documented in manual-verifikat-prefill.ts header. +[2026-08-15] BankID mobile stranded-tab fix: the session id never reaches the browser; it lives in a signed `__Host-` HttpOnly cookie (extensions/general/tic/lib/bankid-flow-cookie.ts) that /poll, /complete, /link and /cancel read. Reported bug: BankID does not reliably return the user to the tab that started the flow (outside plain Safari the iOS https redirect goes to the OS, which opens a NEW tab), the session lived in per-tab sessionStorage, so the new tab rendered the start button and the completed signup was stranded. Login hid it because its completion is self-finishing and the Supabase session is a cookie every tab shares. Cookies being origin-wide is exactly what the handoff needed, so there is no client storage, no cross-tab lock and no heartbeat. TWO REJECTED ATTEMPTS, both killed by adversarial review before merge, both worth not re-deriving. (1) localStorage plus an owner/heartbeat lock: a TIC sessionId is an unauthenticated bearer credential (/bankid/poll is skipAuth and returned user.personalNumber verbatim; /bankid/complete with mode 'login' returns a tokenHash that verifyOtp turns into a session, MFA skipped via bankid_linked), so origin-wide persistent storage turned ANY same-origin XSS into a login-fixation primitive, and the design also deadlocked in the very flow it fixed (the hidden tab stopped polling, the new tab cleared the record, and the storage event never reaches a tab iOS has frozen). (2) The first cookie version: it pinned `mode` but not the USER, so on a shared browser an abandoned link flow bound the first person's personnummer to the second person's account, which is the same class of flaw that killed (1); its cookie also used a narrow Path, which a script can shadow by setting the same name at a LONGER path (RFC 6265 sends longer paths first, and a Max-Age=0 at the shorter path cannot delete it); and its "single-use" was a Set-Cookie, which does not serialise two requests that already carried the cookie. Consequences now, all deliberate: `__Host-` with Path=/ and unconditional Secure, because the prefix forbids Domain and forces Path=/, leaving exactly one possible (name, domain, path) and making both the longer-path shadow and a subdomain toss unsettable (Secure costs nothing since isBankIdEnabled() is false when NEXT_PUBLIC_SELF_HOSTED is set, so the earlier conditional-Secure only risked shipping unprotected behind a proxy that omits x-forwarded-proto); readBankIdFlow fails closed on two cookies of the name rather than picking a winner; SameSite=Lax not Strict, because the BankID return is a top-level navigation from outside the site; a `link` flow requires auth at /start and pins userId, and /link rejects a flow belonging to anyone else; single-use is a unique index (bankid_consumed_sessions, migration 20260815120000) claimed BEFORE generateLink, since serverless has no shared memory and a second magic link invalidates the first; the claim fails closed on any error other than 23505; account_exists deliberately does NOT consume, so a mistyped e-mail is correctable; the window is 300s for the order and re-issued at 900s once /poll observes completion, because the signup e-mail step is a person typing and on the old design it was bounded only by TIC's retention; /poll whitelists its response fields, so the personnummer is no longer returned to anyone, and clears the flow when TIC 404/410s or answers without a status, so a dead session cannot make BankID look unavailable for the rest of the window. The client resumes from ONE probe against /poll, never a readable hint cookie: a hint goes stale, cannot tell a live order from a dead one, and made the component poll on mount in states where nothing was in flight. `link` never auto-resumes at all, since picking one up without a click is how an abandoned flow binds to the next person. Android stays on redirect=null; no Android report motivates changing it and #194 closed that path deliberately. +[2026-08-15] BankID flow cookie, final security invariants after a third skeptic round refuted the user-pinned-cookie version: a signed HttpOnly cookie is NOT a browser binding. It proves the server minted the value, not that this browser started the flow, so an attacker can mint one with curl, authenticate it with their own BankID, and leave it in a shared browser; and for login/signup there is no user to pin (unlike link). The auto-resume that fixes the stranded-tab bug is therefore the thing that made a completed identification transferable between PEOPLE: the next person to open /login (BankID is the default panel) would be signed straight into the planter's tenant with no prompt, and on /register would be invited to finish a signup binding the planter's personnummer to an account made with their own e-mail. Fix: a completed flow is resumed AUTOMATICALLY only when a per-tab sessionStorage marker (lib/auth/bankid-flow-ownership.ts) says the person here started it; otherwise BankIdAuth shows a "fortsätt bara om det var du" confirm card (status 'resumable') that never names the holder (naming would leak a stranger's identity). The marker is per-tab and cleared at every terminal state and on cancel, so a stale marker cannot authorise resuming a same-mode flow another tab started; the BankID app returning to a NEW tab lands in a tab without the marker, so it too must confirm, which is correct. `link` still never auto-resumes at all. Second, the re-issue-on-completion window (900s so the signup e-mail step does not expire mid-typing) was unbounded: /poll pushes expiresAt out, and with no fixed origin a caller could poll in a loop to keep a usable identification alive for TIC's whole retention. Added startedAt to the signed state and a MAX_TOTAL_LIFE cap (order window + verified window) enforced in verifyBankIdFlow. Third, /poll and the dead-session branch no longer CLEAR the cookie: a clearing Set-Cookie is untargeted, so a slow response about a dead session would delete whatever flow is in the jar by the time it lands, including one just started in another tab; the client settles on the status instead and the cookie expires on its own. Only /cancel (the user pressing Avbryt, client-awaited before starting anything new) and the terminal /complete + /link exits clear. Also: /poll rejects a body mode that does not match the cookie's mode (a login session must not be finishable through the signup panel); every /complete and /link exit that has minted or consumed settles; readBankIdFlow catches decodeURIComponent so a malformed cookie is "no flow" not a 500; the raw session id is logged only as an 8-char prefix. Single-use is bankid_consumed_sessions (migration 20260815120000) claimed before generateLink, fail-closed on any non-23505 error, so a deploy MUST apply the migration before the code or every BankID auth returns session_invalid. +[2026-08-15] BankID flow, corrections after a fourth skeptic round. (1) sessionStorage is NOT "gone when the tab closes": it is restored by reopen-closed-tab (Ctrl+Shift+T), browser session restore, and tab duplication, so the per-tab ownership marker can be inherited by a different person, which reopened the exact silent-auto-login-as-a-stranger vuln the marker was meant to close. The marker is therefore no longer trusted on its own: BankIdAuth auto-resumes (no confirm) ONLY a flow that is still PENDING on the mount probe's first observation AND flowStartedHere(mode). An already-`complete` flow finished before this mount existed and always routes to the confirm card. The pending check is the real boundary (you can only auto-consume a flow you watched complete live); the marker just distinguishes same-tab-reload from arrived-from-elsewhere among pending flows. (2) The component's own Avbryt raced the untargeted /cancel cookie clear: it set status 'idle' synchronously (start button reachable) then awaited /cancel, whose late Set-Cookie Max-Age=0 could delete a freshly started flow's cookie. Added a 'cancelling' status that renders a spinner and no start button until /cancel resolves, so no /start can race the clear. (The register Tillbaka already had the isCancelling disable.) (3) /poll returned the holder's givenName/surname on complete, readable by a probe before the confirm gesture, contradicting the card's no-name guarantee. The name is now withheld from a probe (request carries { mode }) and returned only to the active poll loop (no mode), which the signup e-mail step needs and which runs only after ownership/confirm. Confirmed sound and not changed: the MAX_TOTAL_LIFE cap (startedAt in the signed payload, re-issue preserves it, hard cap 1200s from /start); /poll not clearing the cookie; single-use consume before every mint; the mode-match check skipping the active poll (empty body → mode undefined). Known residual, documented in the PR: a person at the EXACT same abandoned tab another left mid-flow is out of any client marker's reach and degrades to ordinary unattended-authenticated-tab risk. +[2026-08-15] BankID resume: the per-tab sessionStorage ownership marker (bankid-flow-ownership.ts) was REMOVED, superseding the two prior entries that built on it. It could not close the residual it was meant to: sessionStorage is restored by reopen-closed-tab/session-restore/tab-duplication, so the marker can be inherited by a different person, and there is no client signal that survives an iOS same-tab reload (a navigation) yet dies on a restore (also a fresh load) - both look identical. Since no client-side token can prove "the person here is the one who started this flow" across any fresh document load, the only correct rule is to never auto-consume a resumed flow: the mount probe now routes ANY found live flow to the confirm card ("Fortsätt bara om det var du"), and only that click starts polling and consumes. Auto-consume happens ONLY inside the live component instance that called startSession (desktop QR click; the pre-navigation mobile launch), which by construction is the person who started it. Cost: one "Fortsätt" tap after returning from the BankID app on iOS (new tab or same-tab reload) - exactly the population with the reported stranded-tab bug, for whom a labelled one-tap continue is clearer than a silent resume anyway. Desktop QR (no reload, live component) and Android (returns to the live tab without reload) never hit the resume path and are unchanged. This fully closes both the complete-flow and pending-flow restore vectors and deletes the module plus its unsound premise. [2026-08-15] Underlag import (attaching a folder of receipts to SIE-migrated verifikat) matches on journal_entries.source_voucher_series/number, never on our own voucher_number: the importer renumbers per target series, so a file named after our number would land on the wrong verifikat exactly when the import skipped an empty/unbalanced voucher. The (period, series, number) resolver was lifted out of extensions/general/arcim-migration into lib/documents/voucher-ref-resolver.ts and is now shared by the provider sweep and the filename flow, so both have one resolution truth. [2026-08-15] Underlag import is a SEPARATE optional import mode (/import?mode=underlag), not a step inside the SIE wizard (founder call 2026-08-15): the receipts normally arrive later and from a different export, so a migration must never be blocked on having them ready. [2026-08-15] A filename that parses to a number with no series (31.pdf) is resolved but NEVER auto-selected, even when the lookup returns exactly one candidate, and a date-shaped name is refused outright rather than read as a voucher number. Linking a document to a posted verifikat is irreversible räkenskapsinformation (BFL 7 kap), so the cost of a wrong parse is permanent and the cost of asking is one click. The date guard is deliberately looser than the parser (unpadded components, two-digit years, space and slash separators, and any bare four-digit year-shaped number): a false positive costs one manual assignment, a false negative costs a permanent wrong link. @@ -1009,3 +1013,5 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-15] Declined (recorded per /resolve-pr triage): classifying validateDocumentFile failures by regex on the localized message in the attach route stays as-is; it mirrors the established pattern in app/api/documents/route.ts, and moving the validator to coded returns is a core-service contract change outside this PR. The Swedish-review notes on idempotency (content hash IS in deterministicDocumentId) and the period-lock error regex (DB trigger is the real guard) require no change. [2026-08-15] Underlag may attach only to posted or reversed verifikat, enforced in the attach route AND in the resolver reads (Swedish-review round on PR #1627). The SIE import RPC posts every entry inside its own transaction, so a draft with a source ref should be unobservable; the enforcement exists because the link is irreversible and an invariant living in another file is not one this surface may lean on. Reversed stays attachable: a storno'd original remains räkenskapsinformation and its underlag belongs on it. [2026-08-15] Confirmed intentional (Swedish-review note): with override=true and an unresolvable filename, the attach endpoint links a document to any same-company, same-declared-year, posted verifikat, migrated or not. This mirrors /api/documents/[id]/link, which imposes no filename check at all, so it introduces no new capability class; tenant, year and period-lock enforcement always apply. +[2026-08-15] BankID tabs bind to a random non-secret `flowId` signed into the shared flow cookie and sent as a request header after start or explicit resume: mode pinning alone cannot distinguish two same-mode tabs, so an older tab could otherwise silently follow, cancel, or complete a newer person's identification after `/start` replaced the origin-wide cookie. This supersedes the 2026-08-15 decision that deliberately skipped mode matching on active polls. +[2026-08-15] Did not apply BankID migration `20260815120000` to Supabase staging during PR #1625 follow-through: read-only reconciliation found 14 staging-only and 99 branch-only migration versions, so applying on top of that divergent ledger would violate the no-orphan rule. Production is reconciled with zero remote-only versions and exactly this PR migration local-only; hosted pg-real validates the migration until staging is reconciled. diff --git a/app/(auth)/register/page.tsx b/app/(auth)/register/page.tsx index 9bea6a0e..516a7288 100644 --- a/app/(auth)/register/page.tsx +++ b/app/(auth)/register/page.tsx @@ -62,11 +62,12 @@ function RegisterPageContent() { const [password, setPassword] = useState('') const [confirmPassword, setConfirmPassword] = useState('') const [isLoading, setIsLoading] = useState(false) + const [isCancelling, setIsCancelling] = useState(false) const [isRegistered, setIsRegistered] = useState(false) const [duplicateEmail, setDuplicateEmail] = useState(null) const [inviteEmail, setInviteEmail] = useState(null) const [bankIdUser, setBankIdUser] = useState<{ givenName?: string; surname?: string } | null>(null) - const [bankIdSessionId, setBankIdSessionId] = useState(null) + const [bankIdFlowId, setBankIdFlowId] = useState(null) const [bankIdEmail, setBankIdEmail] = useState('') // Signup failures render inline next to the form (see AuthFormError), never // as a toast. Field-level problems attach to their field; everything else @@ -166,29 +167,37 @@ function RegisterPageContent() { setFormError({ kind: 'bankid', message: t('bankid_failed_description') }) return } - // BankID verified: store sessionId and show email form + // BankID verified: show the email form. The session itself stays in the + // server's HttpOnly flow cookie, so there is nothing to hold on to here. setFormError(null) setBankIdUser({ givenName: result.givenName, surname: result.surname }) - if (result.sessionId) setBankIdSessionId(result.sessionId) + setBankIdFlowId(result.flowId ?? null) } const handleBankIdSignup = async (e: React.FormEvent) => { e.preventDefault() setFormError(null) - setIsLoading(true) const formData = new FormData(e.currentTarget) const emailValue = (formData.get('bankid_email') as string) || bankIdEmail + if (!bankIdFlowId) { + setFormError({ kind: 'bankid', message: t('bankid_failed_description') }) + return + } + + setIsLoading(true) + try { + // Only the e-mail travels: the session and the fact that this is a + // signup are both pinned in the server's flow cookie. const res = await fetch('/api/extensions/ext/tic/bankid/complete', { method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - sessionId: bankIdSessionId, - mode: 'signup', - email: emailValue, - }), + headers: { + 'Content-Type': 'application/json', + 'x-bankid-flow-id': bankIdFlowId, + }, + body: JSON.stringify({ email: emailValue }), }) const json = await res.json() @@ -546,7 +555,9 @@ function RegisterPageContent() { {inviteEmail ? t('invite_email_hint') : t('bankid_email_hint')}

- + + + + + ) + } + + if (status === 'complete') { + return ( +
+ + {t('bankid_completing')} +
+ ) + } if (status === 'idle') { const label = mode === 'login' @@ -521,22 +680,34 @@ export function BankIdAuth({ mode, onComplete, hero = false }: BankIdAuthProps) qrStartToken={session.qrStartToken} qrStartSecret={session.qrStartSecret} /> - + {/* A resumed tab never called /start, so it has the rotating QR + tokens but no autostart token. Rendering the button anyway would + deep-link `autostarttoken=` empty, which does nothing and says + nothing. The QR above still works. */} + {session.qrStartToken && session.qrStartSecret && session.autoStartToken && ( + + )} )} {isMobile() && (
-

Öppnar BankID-appen...

+ {/* A resumed tab never launched anything, so saying "opening the + BankID app" would be a lie followed by silence. */} +

+ {launchedApp + ? 'Öppnar BankID-appen...' + : t('bankid_finish_in_app')} +

)} @@ -544,6 +715,7 @@ export function BankIdAuth({ mode, onComplete, hero = false }: BankIdAuthProps)