From c634cf9ae098c48a155a85ed92926bf373ee9c20 Mon Sep 17 00:00:00 2001 From: Mattsson <111893710+mattssonn@users.noreply.github.com> Date: Mon, 7 Sep 2026 14:14:36 +0200 Subject: [PATCH] fix(banking): return Enable Banking consent callbacks to the initiating brand host (#2371) * fix(banking): return Enable Banking consent callbacks to the initiating brand host Enable Banking redirects every consent to the one canonical callback URL while browser sessions are per host, so a white-label user reached the callback signed out and was bounced to the unbranded canonical login. The pending row now records the allowlisted origin the flow started from, and the callback uses it for the login bounce, the success redirect and the denial banner. The brand host already holds the session, so its /login forwards straight back into the callback with cookies; the provider redirect URI stays canonical, nothing changes in the Enable Banking console. The shared login redirect helper also stops dragging a callback that arrived on a registered brand host to the canonical login. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01YPom7vRc4jiUKuq3YwjCJz * fix(banking): reload the PostgREST schema cache after adding oauth_origin Skeptic finding: every other ADD COLUMN migration ends with the NOTIFY, and without it PostgREST can reject the new column on connect until its cache refreshes. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01YPom7vRc4jiUKuq3YwjCJz --------- Co-authored-by: Claude Fable 5.1 --- DECISIONS.md | 1 + .../callback/__tests__/route.test.ts | 117 ++++++++++++++++++ .../enable-banking/callback/route.ts | 32 ++++- docs/WHITELABEL.md | 2 +- .../__tests__/session-expired.test.ts | 48 +++++++ extensions/general/enable-banking/index.ts | 10 ++ lib/auth/__tests__/oauth-flow-binding.test.ts | 57 +++++++++ lib/auth/oauth-flow-binding.ts | 42 ++++++- ...07100000_bank_connections_oauth_origin.sql | 16 +++ 9 files changed, 312 insertions(+), 13 deletions(-) create mode 100644 supabase/migrations/20260907100000_bank_connections_oauth_origin.sql diff --git a/DECISIONS.md b/DECISIONS.md index d7b72b75..cc1e3fba 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1636,4 +1636,5 @@ One line per decision: `[YYYY-MM-DD] : `. 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] 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= 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. diff --git a/app/api/extensions/enable-banking/callback/__tests__/route.test.ts b/app/api/extensions/enable-banking/callback/__tests__/route.test.ts index 4ec965e6..ba9d34f0 100644 --- a/app/api/extensions/enable-banking/callback/__tests__/route.test.ts +++ b/app/api/extensions/enable-banking/callback/__tests__/route.test.ts @@ -233,6 +233,96 @@ describe('GET /api/extensions/enable-banking/callback', () => { expect(chain.delete).not.toHaveBeenCalled() }) + // White-label: Enable Banking only ever redirects to the canonical + // callback, but the initiator's session lives on the brand host they + // started from. The row records that origin and every redirect from the + // callback goes back there; the brand host's /login forwards straight into + // the callback again because the session already exists. + describe('initiating origin (white-label)', () => { + const BRAND_ROW = { ...PENDING_ROW, oauth_origin: 'https://books.partner.example' } + + beforeEach(() => { + vi.stubEnv('NEXT_PUBLIC_WHITELABEL_DOMAINS', 'books.partner.example') + }) + + afterEach(() => { + vi.stubEnv('NEXT_PUBLIC_WHITELABEL_DOMAINS', '') + }) + + it('sends an anonymous white-label user to their own brand login, not the canonical one', async () => { + const chain = mockChain({ data: BRAND_ROW, error: null }) + mockFrom.mockReturnValue(chain) + mockGetUser.mockResolvedValue({ data: { user: null }, error: null }) + + const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' })) + + expect(response.status).toBe(307) + const location = new URL(response.headers.get('location') || '') + expect(location.origin).toBe('https://books.partner.example') + expect(location.pathname).toBe('/login') + expect(location.searchParams.get('next')).toBe( + '/api/extensions/enable-banking/callback?code=auth-code&state=valid-state', + ) + // Nothing consumed: the replay on the brand host completes the flow. + expect(mockCreateSession).not.toHaveBeenCalled() + expect(chain.update).not.toHaveBeenCalled() + expect(chain.delete).not.toHaveBeenCalled() + }) + + it('finalizes on the brand host: the finalize page redirects to the recorded origin', async () => { + mockConnectionFlow(BRAND_ROW) + mockGetUser.mockResolvedValue({ data: { user: { id: 'user-1' } }, error: null }) + mockCreateSession.mockResolvedValue({ + session_id: 'sess-1', + accounts: [], + access: { valid_until: '2027-12-31T00:00:00Z' }, + aspsp: { name: 'TestBank', country: 'SE' }, + }) + + const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' })) + + expect(response.status).toBe(200) + expect(await response.text()).toContain( + 'https://books.partner.example/settings/banking?select_accounts=conn-1', + ) + }) + + it('returns an initiator mismatch to the brand host', async () => { + mockFrom.mockReturnValue(mockChain({ data: BRAND_ROW, error: null })) + mockGetUser.mockResolvedValue({ data: { user: { id: 'user-2' } }, error: null }) + + const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' })) + + const location = new URL(response.headers.get('location') || '') + expect(location.origin).toBe('https://books.partner.example') + expect(location.pathname).toBe('/settings/banking') + expect(location.searchParams.get('bank_error')).toContain('annat användarkonto') + }) + + it('collapses a recorded origin that is not a registered host to the canonical one', async () => { + vi.stubEnv('NEXT_PUBLIC_WHITELABEL_DOMAINS', '') + const chain = mockChain({ data: BRAND_ROW, error: null }) + mockFrom.mockReturnValue(chain) + mockGetUser.mockResolvedValue({ data: { user: null }, error: null }) + + const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' })) + + const location = new URL(response.headers.get('location') || '') + expect(location.origin).toBe('http://localhost:3000') + expect(location.pathname).toBe('/login') + }) + + it('keeps a row without a recorded origin on the canonical host (pre-existing rows, direct domain)', async () => { + const chain = mockChain({ data: { ...PENDING_ROW, oauth_origin: null }, error: null }) + mockFrom.mockReturnValue(chain) + mockGetUser.mockResolvedValue({ data: { user: null }, error: null }) + + const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' })) + + expect(new URL(response.headers.get('location') || '').origin).toBe('http://localhost:3000') + }) + }) + it('finalizes as before when the session belongs to the initiator', async () => { // mockConnectionFlow is the suite's standard script for the finalize // path (function declaration below, hoisted into this scope). @@ -1705,6 +1795,33 @@ describe('GET /api/extensions/enable-banking/callback', () => { expect(location).toContain('psu_type=business') }) + it('returns a bank denial to the recorded brand origin so the banner is seen where the session is', async () => { + vi.stubEnv('NEXT_PUBLIC_WHITELABEL_DOMAINS', 'books.partner.example') + mockFrom.mockImplementation(() => + mockChain({ + data: { + id: 'conn-1', + user_id: 'user-1', + bank_name: 'Handelsbanken', + psu_type: 'business', + status: 'pending', + oauth_origin: 'https://books.partner.example', + }, + error: null, + }) + ) + + const response = await GET(makeRequest({ error: 'access_denied', state: 'pending-state' })) + vi.stubEnv('NEXT_PUBLIC_WHITELABEL_DOMAINS', '') + + expect(response.status).toBe(307) + const location = new URL(response.headers.get('location') || '') + expect(location.origin).toBe('https://books.partner.example') + expect(location.pathname).toBe('/settings/banking') + expect(location.searchParams.get('bank_error_code')).toBe('access_denied') + expect(location.searchParams.get('bank_name')).toBe('Handelsbanken') + }) + it('redirects with error when code or state is missing', async () => { const response = await GET(makeRequest({ code: 'auth-code' })) diff --git a/app/api/extensions/enable-banking/callback/route.ts b/app/api/extensions/enable-banking/callback/route.ts index 618e569f..e3c698ef 100644 --- a/app/api/extensions/enable-banking/callback/route.ts +++ b/app/api/extensions/enable-banking/callback/route.ts @@ -20,6 +20,7 @@ import { supersedeSiblingConnections } from '@/extensions/general/enable-banking import { getBankConnectionErrorMessage } from '@/lib/errors/get-error-message' import { renderFinalizeShell, renderFinalizeRedirect } from './finalize-page' import { isConnectorState, verifyConnectorState } from '@/lib/connect/hosted/state' +import { getCanonicalAppOrigin, resolveTrustedAppOrigin } from '@/lib/domains/trusted-app-origin' import { requireFlowInitiator, FLOW_INITIATOR_MISMATCH_MESSAGE, @@ -92,7 +93,10 @@ export async function GET(request: Request) { // /sessions exchange to the pending ledger row. Null on the direct path. const connectorState = searchParams.get('connector_state') - const baseUrl = process.env.NEXT_PUBLIC_APP_URL || 'http://localhost:3000' + // Redirects issued before a pending row is known go to the canonical host + // (the only one the provider ever sends the browser to). Once the row is + // found, its recorded initiating origin takes over: see returnOrigin below. + const baseUrl = getCanonicalAppOrigin() // Connector branch: a self-hosted instance started this authorization through // the /api/connect/bank proxy, which replaced the upstream state with a @@ -143,7 +147,7 @@ export async function GET(request: Request) { // (which stays 'expired' during the round-trip) is also handled. const { data: pendingConn } = await supabase .from('bank_connections') - .select('id, user_id, company_id, bank_name, psu_type, status') + .select('id, user_id, company_id, bank_name, psu_type, status, oauth_origin') .eq('oauth_state', state) .in('status', ['pending', 'expired', 'error']) .single() @@ -220,7 +224,12 @@ export async function GET(request: Request) { bank_error_code: error, ...(pendingConn.psu_type ? { psu_type: pendingConn.psu_type } : {}), }) - return NextResponse.redirect(`${baseUrl}/settings/banking?${params.toString()}`) + // Back to the host the user started on: the denial banner is only + // visible where their session is (a white-label user has none on + // the canonical host and would be bounced to its login instead). + return NextResponse.redirect( + `${resolveTrustedAppOrigin(pendingConn.oauth_origin)}/settings/banking?${params.toString()}` + ) } } catch (cleanupError) { console.error('[enable-banking] Failed to clean up pending bank connection:', cleanupError) @@ -257,7 +266,7 @@ export async function GET(request: Request) { // state stays a plain redirect. const { data: pendingConnection, error: findError } = await supabase .from('bank_connections') - .select('id, user_id, company_id, bank_name, status, session_id, accounts_data') + .select('id, user_id, company_id, bank_name, status, session_id, accounts_data, oauth_origin') .eq('oauth_state', state) .in('status', ['pending', 'expired', 'error']) .single() @@ -279,8 +288,19 @@ export async function GET(request: Request) { // victim lured into approving a consent someone else started would have // their bank accounts attached to that someone's company. The connector // branch above is exempt on purpose (server-to-server, HMAC-verified). + // The initiator's session lives on the host they started from (cookies are + // per host) while Enable Banking always redirects to the canonical callback. + // A white-label user therefore arrives signed out. From here on every + // redirect, including the login bounce that re-runs this callback with the + // same code + state, goes to the recorded initiating origin: their brand + // host already holds the session, so its login page forwards straight back + // here and the callback completes with cookies. Allowlist-validated; an + // unregistered or missing origin collapses to the canonical host. + const returnOrigin = resolveTrustedAppOrigin(pendingConnection.oauth_origin) + const initiator = await requireFlowInitiator(request, pendingConnection.user_id, { flow: 'enable-banking.callback', + returnOrigin, }) if (!initiator.ok) { if (initiator.reason === 'no_session') { @@ -295,7 +315,7 @@ export async function GET(request: Request) { bank_error: FLOW_INITIATOR_MISMATCH_MESSAGE, ...(pendingConnection.bank_name ? { bank_name: pendingConnection.bank_name } : {}), }) - return NextResponse.redirect(`${baseUrl}/settings/banking?${params.toString()}`) + return NextResponse.redirect(`${returnOrigin}/settings/banking?${params.toString()}`) } // Kick the finalize work off eagerly, decoupled from the response stream: @@ -374,7 +394,7 @@ export async function GET(request: Request) { controller.enqueue(encoder.encode(renderFinalizeShell(pendingConnection.bank_name, cspNonce))) const targetPath = await finalizePromise try { - controller.enqueue(encoder.encode(renderFinalizeRedirect(`${baseUrl}${targetPath}`, cspNonce))) + controller.enqueue(encoder.encode(renderFinalizeRedirect(`${returnOrigin}${targetPath}`, cspNonce))) controller.close() } catch { // Stream already cancelled (client closed the tab). The finalize diff --git a/docs/WHITELABEL.md b/docs/WHITELABEL.md index e0860cd1..96d6ae1c 100644 --- a/docs/WHITELABEL.md +++ b/docs/WHITELABEL.md @@ -41,7 +41,7 @@ All branding can be set via env vars. Public ones use `NEXT_PUBLIC_BRANDING_*` ( | `BRANDING_SECURITY_EMAIL` | `securityEmail` | `security@arcim.io` | | `NEXT_PUBLIC_BRANDING_AUTH_EMAIL_FROM` | `authEmailFrom`: From address Supabase Auth sends verification / reset emails from. Used to pre-populate the `from:` query on the "open in Gmail" button after signup. Set to whatever you configured in your Supabase Auth SMTP. | `noreply@gnubok.se` | | `NEXT_PUBLIC_APP_URL` | `appUrl` | `https://app.gnubok.se` | -| `NEXT_PUBLIC_WHITELABEL_DOMAINS` | Exact comma-separated hostnames served by the same hosted deployment. No wildcards. Invite and auth redirects use a listed host and otherwise fall back to `NEXT_PUBLIC_APP_URL`. | `` | +| `NEXT_PUBLIC_WHITELABEL_DOMAINS` | Exact comma-separated hostnames served by the same hosted deployment. No wildcards. Invite and auth redirects use a listed host and otherwise fall back to `NEXT_PUBLIC_APP_URL`. Enable Banking consent callbacks started from a listed host return to it; an unlisted host is sent to the canonical login instead. | `` | | `NEXT_PUBLIC_BRANDING_LOGO_PATH` | `logoPath` | `/gnubokiceon-removebg-preview.png` | | `NEXT_PUBLIC_BRANDING_FAVICON_PATH` | `faviconPath` | `/favicon.ico` | | `NEXT_PUBLIC_BRANDING_APPLE_ICON_PATH` | `appleTouchIconPath` | `/icons/icon-192.png` | diff --git a/extensions/general/enable-banking/__tests__/session-expired.test.ts b/extensions/general/enable-banking/__tests__/session-expired.test.ts index 0b29e7c3..432c4922 100644 --- a/extensions/general/enable-banking/__tests__/session-expired.test.ts +++ b/extensions/general/enable-banking/__tests__/session-expired.test.ts @@ -28,6 +28,7 @@ import { } from '../lib/api-client' import { enableBankingExtension } from '../index' import { syncAccountTransactions } from '../lib/sync' +import { getCanonicalAppOrigin } from '@/lib/domains/trusted-app-origin' const CLOSED_SESSION_BODY = JSON.stringify({ code: 401, @@ -264,6 +265,9 @@ describe('POST /connect (enable-banking): reconnect in place', () => { const firstUpdate = updateSpy.mock.calls[0][0] expect(firstUpdate).toMatchObject({ oauth_state: expect.any(String), + // The host the renewal was started from, so the callback can return + // there (a white-label user's session exists only on their brand host). + oauth_origin: getCanonicalAppOrigin(), status: 'expired', error_message: null, }) @@ -388,6 +392,50 @@ describe('POST /connect (enable-banking): psu_type persistence', () => { expect(insertSpy).toHaveBeenCalledTimes(1) expect(insertSpy.mock.calls[0][0]).toMatchObject({ psu_type: 'personal' }) }) + + it('records the initiating brand origin on a fresh connect so the callback can return there', async () => { + vi.stubEnv('NEXT_PUBLIC_WHITELABEL_DOMAINS', 'books.partner.example') + stubAuth() + const insertSpy = vi.fn() + const ctx = makeContext({ id: 'conn-new', entity_type: 'aktiebolag' }, vi.fn(), insertSpy) + + const req = new Request('https://books.partner.example/api/extensions/ext/enable-banking/connect', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ aspsp_name: 'Handelsbanken', aspsp_country: 'SE' }), + }) + + const res = await connectRoute.handler(req, ctx) + vi.stubEnv('NEXT_PUBLIC_WHITELABEL_DOMAINS', '') + expect(res.status).toBe(200) + expect(insertSpy.mock.calls[0][0]).toMatchObject({ + oauth_origin: 'https://books.partner.example', + }) + // The provider-facing redirect URI is untouched: Enable Banking keeps + // sending the browser to the one registered canonical callback. + const authCall = (globalThis.fetch as Mock).mock.calls.find( + ([url]) => typeof url === 'string' && url.endsWith('/auth'), + ) + expect(authCall).toBeDefined() + const authBody = JSON.parse((authCall as unknown as [string, { body: string }])[1].body) + expect(authBody.redirect_url).toBe(`${process.env.NEXT_PUBLIC_APP_URL}/api/extensions/enable-banking/callback`) + }) + + it('never records an unregistered request host: it collapses to the canonical origin', async () => { + stubAuth() + const insertSpy = vi.fn() + const ctx = makeContext({ id: 'conn-new', entity_type: 'aktiebolag' }, vi.fn(), insertSpy) + + const req = new Request('https://evil.example/api/extensions/ext/enable-banking/connect', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ aspsp_name: 'Handelsbanken', aspsp_country: 'SE' }), + }) + + const res = await connectRoute.handler(req, ctx) + expect(res.status).toBe(200) + expect(insertSpy.mock.calls[0][0]).toMatchObject({ oauth_origin: getCanonicalAppOrigin() }) + }) }) describe('auth_method selection (Handelsbanken Mobile BankID)', () => { diff --git a/extensions/general/enable-banking/index.ts b/extensions/general/enable-banking/index.ts index 16edba9c..5cb0df28 100644 --- a/extensions/general/enable-banking/index.ts +++ b/extensions/general/enable-banking/index.ts @@ -30,6 +30,7 @@ import { resolveCashAccountScope } from '@/lib/reconciliation/cash-account-scope import { checkRateLimit } from '@/lib/auth/rate-limit-http' import { requireCapability } from '@/lib/entitlements/has-capability' import { CAPABILITY } from '@/lib/entitlements/keys' +import { resolveRequestAppOrigin } from '@/lib/domains/trusted-app-origin' import type { StoredAccount } from './types' import type { Transaction } from '@/types' @@ -519,6 +520,13 @@ export const enableBankingExtension: Extension = { const redirectUrl = `${process.env.NEXT_PUBLIC_APP_URL}/api/extensions/enable-banking/callback` + // The host the user started from. Their session lives only there + // (cookies are per host) while redirectUrl stays the canonical + // callback registered with Enable Banking, so the callback reads + // this back to return the browser home. Allowlist-validated: an + // unregistered Host header collapses to the canonical origin. + const oauthOrigin = resolveRequestAppOrigin(request) + // Generate cryptographic state token for CSRF protection const oauthState = crypto.randomUUID() @@ -542,6 +550,7 @@ export const enableBankingExtension: Extension = { .from('bank_connections') .update({ oauth_state: oauthState, + oauth_origin: oauthOrigin, status: 'expired', // session_id is deliberately KEPT here. The callback needs the // session being replaced to carry the renewed consent across to @@ -652,6 +661,7 @@ export const enableBankingExtension: Extension = { bank_name: resolvedAspspName, authorization_id, oauth_state: oauthState, + oauth_origin: oauthOrigin, status: 'pending', psu_type: psuType, }) diff --git a/lib/auth/__tests__/oauth-flow-binding.test.ts b/lib/auth/__tests__/oauth-flow-binding.test.ts index f9d97432..0c7237d3 100644 --- a/lib/auth/__tests__/oauth-flow-binding.test.ts +++ b/lib/auth/__tests__/oauth-flow-binding.test.ts @@ -86,6 +86,28 @@ describe('requireFlowInitiator', () => { ) }) + it('sends an anonymous browser to the recorded brand origin when the caller passes one', async () => { + // Provider redirect URIs are pinned to the canonical host while sessions + // are per host: a white-label user reaches the callback signed out and + // must be sent to THEIR brand login, where the session already exists. + vi.stubEnv('NEXT_PUBLIC_WHITELABEL_DOMAINS', 'books.partner.example') + sessionWith(null) + + const result = await requireFlowInitiator(new Request(CALLBACK_URL), 'user-1', { + returnOrigin: 'https://books.partner.example', + }) + + expect(result.ok).toBe(false) + if (result.ok) throw new Error('unreachable') + expect(result.reason).toBe('no_session') + const location = new URL(result.response.headers.get('location') ?? '') + expect(location.origin).toBe('https://books.partner.example') + expect(location.pathname).toBe('/login') + expect(location.searchParams.get('next')).toBe( + '/api/extensions/stripe/callback?code=ac_123&state=state-1', + ) + }) + it('treats a getUser error as no session (fail closed)', async () => { sessionWith(null, { message: 'invalid JWT' }) @@ -133,6 +155,41 @@ describe('buildLoginRedirect', () => { encodeURIComponent('/api/extensions/woocommerce/return?success=1&user_id=abc'), ) }) + + it('uses a recorded origin only when it is the canonical host or a registered white-label host', () => { + vi.stubEnv('NEXT_PUBLIC_APP_URL', 'https://app.example.se') + vi.stubEnv('NEXT_PUBLIC_WHITELABEL_DOMAINS', 'books.partner.example') + const request = new Request('https://app.example.se/api/extensions/stripe/callback?code=1&state=2') + + const brand = buildLoginRedirect(request, 'https://books.partner.example') + expect(new URL(brand.headers.get('location') ?? '').origin).toBe('https://books.partner.example') + + // A stored value that is no longer (or never was) registered must not + // become a redirect target: the allowlist is the only authority. + const unknown = buildLoginRedirect(request, 'https://evil.example') + expect(new URL(unknown.headers.get('location') ?? '').origin).toBe('https://app.example.se') + + const canonical = buildLoginRedirect(request, 'https://app.example.se') + expect(new URL(canonical.headers.get('location') ?? '').origin).toBe('https://app.example.se') + }) + + it('keeps a callback that arrived on a registered brand host on that host', () => { + // NEXT_PUBLIC_APP_URL used to win over the request origin here, dragging + // a brand-domain callback to the canonical login. The allowlisted request + // host is the fallback now; an unregistered host still collapses. + vi.stubEnv('NEXT_PUBLIC_APP_URL', 'https://app.example.se') + vi.stubEnv('NEXT_PUBLIC_WHITELABEL_DOMAINS', 'books.partner.example') + + const onBrand = buildLoginRedirect( + new Request('https://books.partner.example/api/extensions/stripe/callback?code=1&state=2'), + ) + expect(new URL(onBrand.headers.get('location') ?? '').origin).toBe('https://books.partner.example') + + const onUnknown = buildLoginRedirect( + new Request('https://evil.example/api/extensions/stripe/callback?code=1&state=2'), + ) + expect(new URL(onUnknown.headers.get('location') ?? '').origin).toBe('https://app.example.se') + }) }) describe('redactUserId', () => { diff --git a/lib/auth/oauth-flow-binding.ts b/lib/auth/oauth-flow-binding.ts index 8ca2ae3f..3118b412 100644 --- a/lib/auth/oauth-flow-binding.ts +++ b/lib/auth/oauth-flow-binding.ts @@ -1,6 +1,10 @@ import { NextResponse } from 'next/server' import { createClient } from '@/lib/supabase/server' import { createLogger } from '@/lib/logger' +import { + resolveRequestAppOrigin, + resolveTrustedAppOrigin, +} from '@/lib/domains/trusted-app-origin' /** * Bind the completion of a browser-driven OAuth/consent flow to the user who @@ -21,9 +25,15 @@ import { createLogger } from '@/lib/logger' * * Outcomes: * - ok: the session user is the initiator; carry on. - * - no_session: nobody is signed in (expired mid-flow, cookies cleared). - * `response` redirects to /login?next= so the initiator - * can sign in and the callback re-runs with the same code + state. + * - no_session: nobody is signed in (expired mid-flow, cookies cleared, or + * the session lives on another host). `response` redirects to + * /login?next= so the initiator can sign in and the + * callback re-runs with the same code + state. The login lives on the + * origin the flow was started from when the caller recorded one: provider + * redirect URIs are pinned to the canonical host while sessions are per + * host, so a white-label user reaches the callback signed out and must be + * sent to THEIR brand host, where the session already exists and the + * login page forwards straight back into the callback. * - mismatch: a different user is signed in. `response` is a 403 in the * canonical error envelope; a route whose UX is a settings redirect * inspects `reason` and builds its own redirect instead. The mismatch is @@ -52,6 +62,13 @@ export type FlowInitiatorResult = export interface RequireFlowInitiatorOptions { /** Short label for the log line, e.g. 'stripe.callback'. */ flow?: string + /** + * Origin the initiator started the flow on, as recorded by the start route. + * Validated against the canonical host and the registered white-label hosts; + * anything else falls back to the canonical origin. Omit when the flow has + * no record of it. + */ + returnOrigin?: string | null } /** @@ -67,10 +84,19 @@ export function redactUserId(id: string | null | undefined): string { * The /login redirect for a callback reached without a session. `next` is the * callback's own path + query (same-origin relative, which is the only form * the login page's safeReturnTo accepts), so signing in resumes the flow. + * + * The login host is, in order: the recorded initiating origin (allowlisted), + * the host the callback arrived on (allowlisted, so a brand-domain callback is + * never dragged to the canonical login), or the request origin itself on a + * self-hosted deployment with no NEXT_PUBLIC_APP_URL. */ -export function buildLoginRedirect(request: Request): Response { +export function buildLoginRedirect(request: Request, returnOrigin?: string | null): Response { const current = new URL(request.url) - const appOrigin = process.env.NEXT_PUBLIC_APP_URL || current.origin + const appOrigin = returnOrigin + ? resolveTrustedAppOrigin(returnOrigin) + : process.env.NEXT_PUBLIC_APP_URL + ? resolveRequestAppOrigin(request) + : current.origin const next = `${current.pathname}${current.search}` const login = new URL('/login', appOrigin) login.searchParams.set('next', next) @@ -106,7 +132,11 @@ export async function requireFlowInitiator( path, expectedUser: redactUserId(expectedUserId), }) - return { ok: false, reason: 'no_session', response: buildLoginRedirect(request) } + return { + ok: false, + reason: 'no_session', + response: buildLoginRedirect(request, options.returnOrigin), + } } if (sessionUserId !== expectedUserId) { diff --git a/supabase/migrations/20260907100000_bank_connections_oauth_origin.sql b/supabase/migrations/20260907100000_bank_connections_oauth_origin.sql new file mode 100644 index 00000000..94ea2f45 --- /dev/null +++ b/supabase/migrations/20260907100000_bank_connections_oauth_origin.sql @@ -0,0 +1,16 @@ +-- Record the origin a bank authorization was started from. +-- +-- Enable Banking redirects every consent to the one canonical callback URL +-- registered with it, while browser sessions are per host. A user on a +-- white-label domain therefore reaches the callback signed out. The callback +-- reads this column to send the browser back to the initiating host (where +-- the session lives) for the login bounce, the success redirect and the +-- denial banner. Null means "canonical", which is what every pre-existing row +-- and every direct-domain flow gets. +ALTER TABLE public.bank_connections + ADD COLUMN IF NOT EXISTS oauth_origin text; + +COMMENT ON COLUMN public.bank_connections.oauth_origin IS + 'Allowlist-validated app origin the OAuth flow was started from; null = canonical app URL.'; + +NOTIFY pgrst, 'reload schema';