Files
accounted/tests/pg/oauth-flows.pg.test.ts
Mattsson f047c3d7d1 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>
2026-09-07 16:43:38 +02:00

166 lines
6.3 KiB
TypeScript

import { randomBytes } from 'node:crypto'
import { describe, expect, it } from 'vitest'
import { getClient, getPool } from '@/tests/pg/setup'
import { seedCompany } from '@/tests/pg/fixtures'
/**
* Migration 20260907120000_oauth_flows.sql: the one-row-per-flow table behind
* lib/auth/oauth-flows.ts. Locks in the three things the application relies
* on and cannot prove with mocks:
*
* - the table is service-role only (RLS enabled, browser roles revoked);
* - consuming the state and consuming the handoff are single statements
* whose WHERE clause carries the whole check, so two concurrent
* deliveries yield exactly one winner;
* - the handoff can only be claimed from the origin it was minted for.
*/
async function insertFlow(params: {
companyId: string
userId: string
origin?: string
expiresInSeconds?: number
}): Promise<string> {
const id = randomBytes(32).toString('base64url')
await getPool().query(
`INSERT INTO public.oauth_flows
(id, kind, company_id, user_id, origin, redirect_uri, expires_at)
VALUES ($1, 'skatteverket', $2, $3, $4, 'https://oauth.testbrand.example/cb',
now() + make_interval(secs => $5))`,
[id, params.companyId, params.userId, params.origin ?? 'https://app.testbrand.example', params.expiresInSeconds ?? 600],
)
return id
}
// Mirrors consumeOAuthFlowState's PostgREST statement.
const CONSUME_STATE = `
UPDATE public.oauth_flows SET used_at = now()
WHERE id = $1 AND kind = 'skatteverket' AND used_at IS NULL AND expires_at > now()
RETURNING id`
// Mirrors consumeOAuthFlowHandoff's PostgREST statement.
const CONSUME_HANDOFF = `
DELETE FROM public.oauth_flows
WHERE handoff_id = $1 AND origin = $2 AND kind = 'skatteverket'
AND handoff_expires_at > now()
RETURNING id, handoff_code`
async function mintHandoff(id: string, expiresInSeconds = 120): Promise<string> {
const handoffId = randomBytes(32).toString('base64url')
await getPool().query(
`UPDATE public.oauth_flows
SET handoff_id = $2, handoff_code = 'v1:ciphertext',
handoff_expires_at = now() + make_interval(secs => $3)
WHERE id = $1`,
[id, handoffId, expiresInSeconds],
)
return handoffId
}
describe('oauth_flows (pg)', () => {
async function expectDenied(role: 'anon' | 'authenticated', sql: string) {
const client = await getClient()
try {
await client.query('BEGIN')
await client.query(`SET LOCAL ROLE ${role}`)
await expect(client.query(sql)).rejects.toThrow(/permission denied/i)
} finally {
await client.query('ROLLBACK').catch(() => {})
client.release()
}
}
it('denies the browser-facing roles entirely', async () => {
await expectDenied('anon', 'SELECT * FROM public.oauth_flows LIMIT 1')
await expectDenied('authenticated', 'SELECT * FROM public.oauth_flows LIMIT 1')
await expectDenied(
'authenticated',
`INSERT INTO public.oauth_flows (id, kind, company_id, user_id, origin, redirect_uri, expires_at)
VALUES ('x', 'skatteverket', gen_random_uuid(), gen_random_uuid(), 'https://a', 'https://b', now())`,
)
})
it('lets exactly one of two concurrent deliveries consume the state', async () => {
const { companyId, userId } = await seedCompany()
const id = await insertFlow({ companyId, userId })
const a = await getClient()
const b = await getClient()
try {
const [ra, rb] = await Promise.all([
a.query(CONSUME_STATE, [id]),
b.query(CONSUME_STATE, [id]),
])
expect(ra.rowCount! + rb.rowCount!).toBe(1)
} finally {
a.release()
b.release()
}
// And nothing after that: the state is spent.
const again = await getPool().query(CONSUME_STATE, [id])
expect(again.rowCount).toBe(0)
})
it('refuses an expired state', async () => {
const { companyId, userId } = await seedCompany()
const id = await insertFlow({ companyId, userId, expiresInSeconds: -1 })
const res = await getPool().query(CONSUME_STATE, [id])
expect(res.rowCount).toBe(0)
})
it('claims the handoff once, and only from the recorded origin', async () => {
const { companyId, userId } = await seedCompany()
const id = await insertFlow({ companyId, userId, origin: 'https://brand.testbrand.example' })
await getPool().query(CONSUME_STATE, [id])
const handoffId = await mintHandoff(id)
// Wrong origin: nothing, and the row is still there for the right one.
const wrong = await getPool().query(CONSUME_HANDOFF, [handoffId, 'https://app.testbrand.example'])
expect(wrong.rowCount).toBe(0)
const a = await getClient()
const b = await getClient()
try {
const [ra, rb] = await Promise.all([
a.query(CONSUME_HANDOFF, [handoffId, 'https://brand.testbrand.example']),
b.query(CONSUME_HANDOFF, [handoffId, 'https://brand.testbrand.example']),
])
expect(ra.rowCount! + rb.rowCount!).toBe(1)
const winner = ra.rowCount === 1 ? ra : rb
expect(winner.rows[0]!.handoff_code).toBe('v1:ciphertext')
} finally {
a.release()
b.release()
}
// DELETE RETURNING: the held code left the database with the claim.
const gone = await getPool().query('SELECT 1 FROM public.oauth_flows WHERE id = $1', [id])
expect(gone.rowCount).toBe(0)
})
it('refuses an expired handoff', async () => {
const { companyId, userId } = await seedCompany()
const id = await insertFlow({ companyId, userId, origin: 'https://brand.testbrand.example' })
await getPool().query(CONSUME_STATE, [id])
const handoffId = await mintHandoff(id, -1)
const res = await getPool().query(CONSUME_HANDOFF, [handoffId, 'https://brand.testbrand.example'])
expect(res.rowCount).toBe(0)
})
it('rejects a handoff written onto an unconsumed state', async () => {
// The shape constraint: a handoff only exists for a state hop 1 consumed,
// so a stray write can never make an unconsumed state claimable twice.
const { companyId, userId } = await seedCompany()
const id = await insertFlow({ companyId, userId })
await expect(
getPool().query(
`UPDATE public.oauth_flows
SET handoff_id = 'h', handoff_code = 'v1:x', handoff_expires_at = now() + interval '2 minutes'
WHERE id = $1`,
[id],
),
).rejects.toThrow(/oauth_flows_handoff_shape/)
})
})