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 <noreply@anthropic.com> 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YPom7vRc4jiUKuq3YwjCJz --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Mattsson
co-authored by
Claude Fable 5.1
Claude Fable 5.1
parent
9879fb53e9
commit
c634cf9ae0
@@ -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' }))
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user