feat(connect): wire the SKV extension through the connector broker + data proxy (PR6b-2) (#2103)
* feat(connect): wire the SKV extension through the connector broker + data proxy (PR6b-2) In connector mode (GNUBOK_CONNECTOR_KEY set, no own SKV credentials) the Skatteverket extension now routes through the hosted connector stack (#1757) instead of calling Skatteverket directly: - skvRequestWithAuth routes to the data proxy: base URL maps to a service segment (moms/skattekonto/agd-inlamning/agd-period), the user's SKV Bearer moves to X-Connector-Upstream-Authorization, the connector key authenticates the proxy, and the gateway Client_Id/Client_Secret are omitted (the proxy adds Arcim's). Connector-layer 4xx bodies (code CONNECTOR_*) are classified before the SKV-shaped 401/403 sniffing so a broker refusal surfaces operator guidance (check GNUBOK_CONNECTOR_KEY), never APIGW/BankID guidance for knobs the instance does not have. - OAuth: /authorize starts the consent via the broker's authorize-url (persisting its redirect_uri + connector_state), the hosted SKV callback bounces the code back to the instance, and exchangeCodeForTokens / refreshAccessToken exchange through the broker's /oauth/token, unwrapping its { data } envelope. Tokens still rest encrypted on the instance; client_id/client_secret never exist there. - Broker refresh 404 CONNECTOR_NOT_OWNED maps to SESSION_EXPIRED (terminal; reconnect fixes); broker 502 stays a raw error so a transient SKV outage never re-arms the reconnect banner (#1155). - getSkatteverketEnvironment() reports 'prod' in connector mode: the upstream env is hosted's, and the instance's unset defaults would show a false Testmiljo badge on real filings. - System (CCG/ombud) auth is deliberately not brokered: hosted-only, stays direct. Hosted and own-credentials self-hosts are byte-identical: every branch gates on skatteverketConnectorMode(), which is null whenever own SKV credentials exist or no connector key is set. Direct-path tests pin that. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KRfamAKDqvRNwbjr5XD2VS * fix(connect): classify SKV dead-refresh-token dialects broker-side; forward diagnostic headers; connector-aware gateway guidance Skeptic refutation on PR #2103 (found independently by the correctness and compliance skeptics): the broker's /oauth/token catch-all collapsed SKV's terminal dead-refresh-token dialects (404 id_not_found, 400 invalid_grant, "Refresh Token status is expired": the dominant refresh outcome, per-flow tokens live 65 minutes) into the generic 502 CONNECTOR_SKV_TOKEN_FAILED, so a connector instance could never classify ordinary session expiry: raw English 500s instead of the reconnect flow, staged filing operations consumed as non-recoverable, crons retrying raw forever. - Broker /oauth/token: re-codes those dialects as 401 CONNECTOR_SKV_REFRESH_DEAD, refresh grant only (invalid_grant on the code exchange means an expired one-shot code and keeps the generic 502). The classifier (isSkvDeadRefreshTokenError) uses the same regex set the extension's direct path classifies with. - Instance dead-token classifier maps CONNECTOR_SKV_REFRESH_DEAD to SESSION_EXPIRED alongside 404 CONNECTOR_NOT_OWNED; the generic 502 stays a raw error so a transient SKV outage never re-arms the reconnect banner. - Data proxy: forwards WWW-Authenticate and x-skv-*/x-amzn-*/x-api-* response headers (the instance's MISSING_SCOPE classification reads them; body-less gateway rejections carry no other signal). - Instance gateway-refusal guidance is connector-aware: a self-host has no SKATTEVERKET_APIGW_CLIENT_ID and no Utvecklarportalen access, so connector mode points at /api/connector/status and support instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KRfamAKDqvRNwbjr5XD2VS * fix(connect): reject redirects on the instance's broker OAuth requests CodeRabbit inline finding (CWE-200): the connector-mode authorize-url and token requests followed redirects by default, so a 307/308 would resend the connector key (and code/refresh token) to the redirect target. redirect 'error', matching the broker's own postToken rule; the token response must only ever come from the broker endpoint itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KRfamAKDqvRNwbjr5XD2VS --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Mattsson
co-authored by
Claude Fable 5
Claude Fable 5
parent
ee22c9c7b7
commit
08b1119c7d
@@ -89,6 +89,30 @@ describe('skv data proxy', () => {
|
||||
expect(fetchMock.mock.calls[0][0]).toBe('https://api.skv/agd/inlamning/v1/underlag')
|
||||
expect(fetchMock.mock.calls[0][1].body).toBe('<xml/>')
|
||||
})
|
||||
it('forwards SKV diagnostic headers (WWW-Authenticate, x-skv-*) but nothing else', async () => {
|
||||
// The instance's 401 classifier reads WWW-Authenticate (insufficient_scope
|
||||
// → MISSING_SCOPE) and the x-skv-* family on body-less gateway
|
||||
// rejections; stripping them blinded connector-mode classification.
|
||||
fetchMock.mockResolvedValueOnce(new Response('', {
|
||||
status: 401,
|
||||
headers: {
|
||||
'content-type': 'application/json',
|
||||
'WWW-Authenticate': 'Bearer error="insufficient_scope", scope="agd"',
|
||||
'x-skv-trace': 'abc',
|
||||
'x-amzn-requestid': 'req-1',
|
||||
'set-cookie': 'secret=1',
|
||||
'x-internal-other': 'nope',
|
||||
},
|
||||
}))
|
||||
const res = await GET(req('GET', '/moms/x'))
|
||||
expect(res.status).toBe(401)
|
||||
expect(res.headers.get('WWW-Authenticate')).toBe('Bearer error="insufficient_scope", scope="agd"')
|
||||
expect(res.headers.get('x-skv-trace')).toBe('abc')
|
||||
expect(res.headers.get('x-amzn-requestid')).toBe('req-1')
|
||||
expect(res.headers.get('set-cookie')).toBeNull()
|
||||
expect(res.headers.get('x-internal-other')).toBeNull()
|
||||
})
|
||||
|
||||
it('429 when the budget is exhausted', async () => {
|
||||
hh.budget.mockResolvedValue({ ok: false, scope: 'hour', retryAfterSec: 3600 })
|
||||
const res = await GET(req('GET', '/moms/x'))
|
||||
|
||||
@@ -106,7 +106,20 @@ async function handle(request: Request, ctx: ConnectorContext): Promise<Response
|
||||
})
|
||||
const text = await res.text()
|
||||
if ([204, 205, 304].includes(res.status)) return new NextResponse(null, { status: res.status })
|
||||
return new NextResponse(text, { status: res.status, headers: { 'Content-Type': res.headers.get('content-type') ?? 'application/json' } })
|
||||
// Forward SKV's diagnostic headers: WWW-Authenticate carries OAuth's
|
||||
// machine-readable failure reason (the instance's insufficient_scope →
|
||||
// MISSING_SCOPE classification depends on it), and the x-skv-*/x-amzn-*/
|
||||
// x-api-* families are the only signal on body-less gateway rejections.
|
||||
// Nothing secret rides in them; stripping them blinded the instance's
|
||||
// 401 classifier (skeptic finding on PR6b-2).
|
||||
const headers: Record<string, string> = { 'Content-Type': res.headers.get('content-type') ?? 'application/json' }
|
||||
res.headers.forEach((v, k) => {
|
||||
const lk = k.toLowerCase()
|
||||
if (lk === 'www-authenticate' || lk.startsWith('x-skv-') || lk.startsWith('x-amzn-') || lk.startsWith('x-api-')) {
|
||||
headers[k] = v
|
||||
}
|
||||
})
|
||||
return new NextResponse(text, { status: res.status, headers })
|
||||
} finally {
|
||||
clearTimeout(timeout)
|
||||
}
|
||||
|
||||
@@ -20,7 +20,16 @@ const hh = vi.hoisted(() => ({
|
||||
findRefresh: vi.fn(),
|
||||
}))
|
||||
vi.mock('@/lib/connect/hosted/upstream-budget', () => ({ reserveUpstream: (...a: unknown[]) => hh.budget(...a) }))
|
||||
vi.mock('@/lib/connect/upstreams/skatteverket-oauth', () => ({ exchangeSkvCode: (...a: unknown[]) => hh.exchange(...a), refreshSkvToken: (...a: unknown[]) => hh.refresh(...a) }))
|
||||
vi.mock('@/lib/connect/upstreams/skatteverket-oauth', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('@/lib/connect/upstreams/skatteverket-oauth')>()
|
||||
return {
|
||||
exchangeSkvCode: (...a: unknown[]) => hh.exchange(...a),
|
||||
refreshSkvToken: (...a: unknown[]) => hh.refresh(...a),
|
||||
// The dead-dialect classifier stays REAL: these tests pin that the route
|
||||
// re-codes exactly the dialects the classifier recognizes.
|
||||
isSkvDeadRefreshTokenError: actual.isSkvDeadRefreshTokenError,
|
||||
}
|
||||
})
|
||||
vi.mock('@/lib/connect/hosted/ledger', () => ({
|
||||
activateByPendingState: (...a: unknown[]) => hh.activate(...a),
|
||||
findPendingByState: (...a: unknown[]) => hh.findPending(...a),
|
||||
@@ -102,4 +111,45 @@ describe('POST /api/connect/skv/oauth/token', () => {
|
||||
const res = await POST(createMockRequest('/x', { method: 'POST', body: { grant_type: 'authorization_code', code: 'c', redirect_uri: 'https://app.gnubok.se/cb', connector_state: 'ck1.signed' } }))
|
||||
expect(res.status).toBe(502)
|
||||
})
|
||||
|
||||
// SKV per-flow refresh tokens live 65 minutes, so a dead refresh token is
|
||||
// the DOMINANT refresh outcome. The broker must re-code SKV's terminal
|
||||
// dialects distinctly: the instance maps this to SESSION_EXPIRED and shows
|
||||
// its reconnect flow. Collapsing it into the generic 502 stripped every
|
||||
// connector instance of that flow (skeptic refutation on PR6b-2).
|
||||
it('401 CONNECTOR_SKV_REFRESH_DEAD when SKV declares the refresh token dead', async () => {
|
||||
hh.refresh.mockRejectedValue(new Error(
|
||||
'Skatteverket token refresh failed (404): {"error":"id_not_found","error_description":"The refresh token is not found"}',
|
||||
))
|
||||
const res = await POST(createMockRequest('/x', { method: 'POST', body: { grant_type: 'refresh_token', refresh_token: 'rt' } }))
|
||||
expect(res.status).toBe(401)
|
||||
expect((await res.json()).code).toBe('CONNECTOR_SKV_REFRESH_DEAD')
|
||||
})
|
||||
|
||||
it('401 CONNECTOR_SKV_REFRESH_DEAD on the 400 invalid_grant dialect too', async () => {
|
||||
hh.refresh.mockRejectedValue(new Error(
|
||||
'Skatteverket token refresh failed (400): {"error": "invalid_grant"}',
|
||||
))
|
||||
const res = await POST(createMockRequest('/x', { method: 'POST', body: { grant_type: 'refresh_token', refresh_token: 'rt' } }))
|
||||
expect(res.status).toBe(401)
|
||||
expect((await res.json()).code).toBe('CONNECTOR_SKV_REFRESH_DEAD')
|
||||
})
|
||||
|
||||
it('keeps transient refresh failures as the generic 502 (never a reconnect signal)', async () => {
|
||||
hh.refresh.mockRejectedValue(new Error('Skatteverket token refresh failed (503): upstream unavailable'))
|
||||
const res = await POST(createMockRequest('/x', { method: 'POST', body: { grant_type: 'refresh_token', refresh_token: 'rt' } }))
|
||||
expect(res.status).toBe(502)
|
||||
expect((await res.json()).code).toBe('CONNECTOR_SKV_TOKEN_FAILED')
|
||||
})
|
||||
|
||||
it('does NOT re-code a dead-dialect failure on the authorization_code grant', async () => {
|
||||
// An expired one-shot code also answers 400 invalid_grant, but that is
|
||||
// not "refresh token dead": the exchange keeps the generic 502.
|
||||
hh.exchange.mockRejectedValue(new Error(
|
||||
'Skatteverket token exchange failed (400): {"error": "invalid_grant"}',
|
||||
))
|
||||
const res = await POST(createMockRequest('/x', { method: 'POST', body: { grant_type: 'authorization_code', code: 'c', redirect_uri: 'https://app.gnubok.se/cb', connector_state: 'ck1.signed' } }))
|
||||
expect(res.status).toBe(502)
|
||||
expect((await res.json()).code).toBe('CONNECTOR_SKV_TOKEN_FAILED')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -2,7 +2,7 @@ import { NextResponse } from 'next/server'
|
||||
import { z } from 'zod'
|
||||
import { validateBody } from '@/lib/api/validate'
|
||||
import { withConnectorAuth, type ConnectorContext } from '@/lib/connect/hosted/with-connector-auth'
|
||||
import { exchangeSkvCode, refreshSkvToken, type SkvTokenResponse } from '@/lib/connect/upstreams/skatteverket-oauth'
|
||||
import { exchangeSkvCode, isSkvDeadRefreshTokenError, refreshSkvToken, type SkvTokenResponse } from '@/lib/connect/upstreams/skatteverket-oauth'
|
||||
import { reserveUpstream } from '@/lib/connect/hosted/upstream-budget'
|
||||
import { activateByPendingState, findByRefreshHash, findPendingByState, hashHandle } from '@/lib/connect/hosted/ledger'
|
||||
import { verifyConnectorState } from '@/lib/connect/hosted/state'
|
||||
@@ -140,7 +140,22 @@ export const POST = withConnectorAuth('connect.skv', async (request, ctx) => {
|
||||
}
|
||||
return tokenResponse(tokens)
|
||||
} catch (err) {
|
||||
ctx.log.warn('skv token exchange failed', { err: err instanceof Error ? err.message : String(err) })
|
||||
const message = err instanceof Error ? err.message : String(err)
|
||||
// Ordinary session expiry must be distinguishable from a transient
|
||||
// failure: SKV's per-flow refresh tokens live 65 minutes, so a dead
|
||||
// refresh token is the DOMINANT outcome here, and collapsing it into the
|
||||
// generic 502 stripped every connector instance of its reconnect flow
|
||||
// (raw 500s, no banner, cron retry spam). The instance maps this code to
|
||||
// SESSION_EXPIRED; everything else stays the opaque 502 so a transient
|
||||
// SKV outage never masquerades as "reconnect needed".
|
||||
if (parsed.data.grant_type === 'refresh_token' && isSkvDeadRefreshTokenError(message)) {
|
||||
ctx.log.info('skv refresh token expired at upstream', { code: 'CONNECTOR_SKV_REFRESH_DEAD' })
|
||||
return NextResponse.json(
|
||||
{ error: 'Skatteverket refresh token is no longer valid; a new BankID consent is required', code: 'CONNECTOR_SKV_REFRESH_DEAD' },
|
||||
{ status: 401 },
|
||||
)
|
||||
}
|
||||
ctx.log.warn('skv token exchange failed', { err: message })
|
||||
return NextResponse.json({ error: 'Skatteverket token exchange failed', code: 'CONNECTOR_SKV_TOKEN_FAILED' }, { status: 502 })
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user