fix(skatteverket): finish the BankID consent on the initiating origin, bound to the initiating user (#2373)

* fix(skatteverket): finish the BankID consent on the initiating origin, bound to the initiating user

The Skatteverket OAuth callback answered NEXT_PUBLIC_APP_URL regardless of
where the flow started, so on a white-label brand domain the popup's
postMessage was dropped and the fallback redirect landed on the wrong
origin without a session. On hosted, the initiator check from #2155 was
bypassed by design because the registered callback host carries no app
cookies, so a lured victim's BankID-authorised tokens could be stored
under the user who started the flow.

Flow state moves from six per-company extension_data keys to one
oauth_flows row per flow (migration 20260907120000), consumed atomically.
Hop 1 on the registered OAuth host consumes the state, stashes the
provider code or error encrypted under a separate handoff id and 302s to
the recorded origin; hop 2 there claims the handoff bound to that origin,
requires the initiating user's session, re-checks membership and
exchanges the code. Error pages keep the tab open. The self-hosted
single-hop and the connector broker branch keep working. The hosted
no-session exception, the legacy cookie-user fallback and the optional
PKCE verifier are gone.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T1YDNadz81eWo94j115bhH

* fix(skatteverket): decide the callback hop by host, close the tab when the flow is unknown

Skeptic findings on #2373. The hop comparison and the handoff claim used
the request origin including its scheme, which Next derives from
x-forwarded-proto; a self-hosted proxy that forwards Host without it (or
rewrites Host to the upstream address) made every connect end in a state
error. Hops are now compared by host only, and the handoff is claimed for
the validated origin the host resolves to, scheme from configuration.

Error pages answered before the flow row is known (unknown, expired or
replayed state or handoff) post to a guessed origin that a brand opener
never hears; they now close the tab so the panels' closed-tab watcher
resets them instead of leaving Connect disabled.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T1YDNadz81eWo94j115bhH

* test(skatteverket): mock resolveBrandResultByHost for the merged login-redirect resolver

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T1YDNadz81eWo94j115bhH

* fix(skatteverket): bind the initiator before the flow is spent

Superagent P2 on #2373: hop 2 deleted the handoff before the session and
membership checks, so a signed-out or wrong-user arrival burned a live
consent. The finishing hop now peeks the row for its initiator, binds the
completing session to it, and only then consumes atomically. A
session-less arrival is sent to /login on the initiating origin and
resumes into the same callback URL; a different user is refused with the
row left claimable for the initiator. The handoff TTL is five minutes so
a sign-in fits.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T1YDNadz81eWo94j115bhH

* fix(skatteverket): check membership before the flow is spent, answer the callback page on a failed mint

Second review cycle on #2373. Superagent: the company-membership check
ran after the consume, so a revoked initiator burned the provider code on
the way to being refused; it now runs inside the pre-consume binding.
CodeRabbit: a failed handoff mint escaped as a framework error page the
opener never hears; it now answers the callback error page on the
initiating origin.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T1YDNadz81eWo94j115bhH

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Mattsson
2026-09-07 16:43:38 +02:00
committed by GitHub
co-authored by Claude Fable 5.1
parent cb962fae88
commit f047c3d7d1
11 changed files with 1789 additions and 551 deletions
+2
View File
@@ -1636,6 +1636,7 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
[2026-09-06] Bundled SKV ROT/RUT payout books ONE voucher (one 1513 leg per begäran) and the set is suggested at read time with no hint column: one bank row = one verifikat (match-batch precedent) and a uuid[] hint would need six clear paths and go stale; N vouchers + the 1:N reconciliation split was rejected because its half-failure state has no UI exit, and begäran, not the invoice, is the unit under fakturamodellen.
[2026-09-06] Utlägg via lön settles claims with an idempotent RPC after the salary verifikat is posted (pre-checked before posting), not with a trigger on salary_runs -> booked: a raise inside that trigger after the entries exist would leave a paid run with posted verifikat and a retry would double-post; the RPC path fails to "booked, claims still open, re-runnable".
[2026-09-06] A privately paid supplier invoice is booked through registerExpenseClaim (verifikat + expense_claims row, source_type expense_claim) with the invoice's kontering as custom lines, and a person-paid inbox document goes to the core route with inbox_item_id instead of the extension's convert endpoint: the form's switch, the second entry generator and the convert bypass were three write paths for one fact, so one writer wins over adding a claims insert beside the old generator (the issue's shape) or copying the branch into the convert handler.
[2026-09-07] Skatteverket OAuth moved from six per-company extension_data keys to one oauth_flows row per flow (migration 20260907120000) with a two-hop brand handoff modelled on PR #2305: hop 1 on the registered callback host consumes the state and 302s an encrypted handoff to the recorded app/brand origin, hop 2 there requires the initiating user's session before the exchange. The hosted no-session exception from PR #2155 is gone (it made the initiator check a no-op on every hosted connect). Rejected: extending provider_otc (its consent FK is NOT NULL and points at migration consents) and a purge cron (the row set is tiny; /authorize sweeps expired rows). In-flight connects at deploy time fail with a state error and are restarted; the legacy cookie-user and optional-PKCE fallbacks were deleted with them.
[2026-09-07] WooCommerce activation gate: the wc-auth callback now only STAGES verified keys on the pending row; activation additionally requires browser_confirmed_at from the session-bound return leg, enforced by a DB CHECK (20260907143000) rather than an application guard, so 'active implies keys plus a recorded confirmation' holds by construction and a connection can never go live headless. Skeptic review established the limit of that guarantee and it is recorded here on purpose: the second signal comes from the INITIATING user, so it does not stop a store admin from approving a link someone else generated (wc-auth delivers keys server-to-server and identifies no approver); that needs a proof of store control from the initiator, tracked as a follow-up. Rejected the proposed separate pending-attempt table and handshake token: the pending row and oauth_state already are that state. Expiry: 15-minute TTL on the return leg plus a sweep at the start of the nightly orders cron; NOT on the callback leg, because WooCommerce treats any non-200 as a failed handshake (deletes the key, store-side error page, no redirect) and would strand slow approvals. Prod audit before the migration found no active row still carrying a state; one hand-inserted demo row (active, no keys, sync off) is parked as error by the migration because it could never sync. Returning the browser to the initiating brand origin is a separate change.
[2026-09-07] Björn Lundén connect: Lundify activation redirect reuses the OAuth state row (provider_otc) and the callback, with the User-Key arriving as publicKey and the state as extra; the manual User-Key field stays as a folded fallback: BL issued the activation key on 2026-09-07, the redirect removes the GUID copy-paste that failed every real customer, and reusing the atomic state consumption plus initiator binding keeps the same tenant boundary as Fortnox/Visma instead of a second callback with its own checks. BL's ON/OFF backend callback and settings URL are not built: they need a registered URL at BL and an orgNr-to-company mapping, filed as follow-up.
[2026-09-07] Auth-link hosts resolve against the brands table only; NEXT_PUBLIC_WHITELABEL_DOMAINS removed (supersedes 2026-08-18): the env var was a second copy of brands.domain compiled into the browser, so every brand needed four registrations (row, env var, GoTrue allowlist, redeploy) and two partners shipped with it stale (canonical-branded reset mails). Password reset moved to POST /api/auth/password-reset so the server resolves the host; invite, email change and signup share the same resolver, which also trusts this deployment's own VERCEL_URL/VERCEL_BRANCH_URL so previews keep working. A drift check between the copies was rejected: it would be a fifth thing to maintain. GoTrue's redirect allowlist stays as the backstop; hosted carries the wildcards https://*.accounted.se/auth/callback** and https://*.accounted.se/invite/** there (config, not code; GoTrue matches the full URL with query, and * stops at . and /) so only bring-your-own-domain partners need a manual entry. A failed brands lookup refuses with 503 (BrandLookupFailedError) instead of a canonical fallback: a canonical link is a wrong-brand mail for a white-label user, which is the bug this replaces.
@@ -1645,4 +1646,5 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
[2026-09-07] Line breaks in line descriptions are collapsed by every single-line consumer (Peppol cbc:Name, accrual voucher text) and additionally in the SIE writer's quoted-text escaper: SIE is one record per line by spec, so the writer guards the format regardless of where the text came from.
[2026-09-07] Enable Banking callbacks return to the initiating white-label host by recording the allowlisted request origin on the pending row and replaying the callback there, instead of extending the provider_otc handoff from PR #2305: the brand host already holds the session, so a /login?next=<callback> bounce on that host forwards straight back into the callback with cookies, needing one nullable column and no encrypted payload, no second table and no cron. The provider redirect URI stays canonical, so nothing changes in the Enable Banking console. Stripe, Gmail and cloud backup have the same shape but no partner-domain users yet; tracked as a follow-up issue rather than built speculatively.
[2026-09-07] Stripe checkout and portal return URLs resolve through the existing resolveRequestAppOrigin allowlist (NEXT_PUBLIC_WHITELABEL_DOMAINS), not a DB brand lookup: it is the same trust boundary invites and email-change links already use, so one allowlist governs every host we redirect a browser to. Return paths stay fixed literals; no caller-supplied URL is accepted. Session-expiry and company-switch handling were left alone: the middleware already bounces to /login on the same host with the path preserved, and the webhook keys on company_id metadata.
[2026-09-07] PR #2373 review: the Skatteverket callback now peeks the flow row for its initiator and binds the session BEFORE consuming state or handoff (Superagent P2: a DELETE-before-auth let a signed-out or wrong-user arrival burn a live consent). A session-less arrival is sent to /login on the initiating origin and resumes into the same URL; a wrong user is refused with the row left claimable. This reverses the earlier 'no login resume' decision for this flow; the consume stays the atomic gate, the peek only decides who may attempt it. Handoff TTL raised from two to five minutes to fit a sign-in.
[2026-09-07] BankID confirmation mail and the Send Email hook resolve their link host through lib/domains/trusted-app-origin instead of the raw forwarded host / GoTrue's redirect_to: the BankID mail is the one auth link GoTrue's redirect allowlist never sees (built here, sent via Resend), and the hook's signature proves the sender, not the destination, while the GoTrue allowlist is a hand-configured glob. Unknown, lookalike, credential-bearing, non-default-port and malformed destinations collapse to the canonical /auth/callback with no next path; a registered brand host over http is upgraded to https. Brand sender identity is resolved from the RESOLVED host so mail branding and link destination always agree. A brands-table read failure refuses (BankID: step resolve_origin, signup rolls back; hook: 500 so Supabase retries) rather than mailing a canonical link to a white-label user. Dropped from the audit's plan 7 as already in place after #2376: signup route, HTTPS enforcement, credential/port checks, recovery/invite/email-change coverage.