fix(skatteverket): treat AGI kvittens gateway refusals as errors and name the connector operator (#2234)
* fix(skatteverket): treat AGI kvittens gateway refusals as errors and name the connector operator The AGI kvittens cron bucketed ACCESS_DENIED as apigw_config with a warn-once suppression because the APIGW client was known to lack the AGI hantera subscription in Utvecklarportalen (#963). With that subscription being put in place (#2226), a gateway refusal is a regression and belongs in the ordinary error path, so the bucket, its "known configuration gap" comment and the apigwConfig response field are gone. The connector-mode gateway-refusal message said "kontakta supporten"; hosted is now itself a Connect installation for the canary companies, so the message names the connector operator by host instead. Refs #2226 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159Mi1sTAHfUDZkSysntYz2 * fix(skatteverket): keep the ACCESS_DENIED code in the kvittens cron body Skeptic finding on PR #2234: the generic error path ran the gateway refusal through getErrorMessage, whose Swedish keyword heuristic misses the gateway wording and collapsed it into "Något gick fel". Echo the machine-readable code instead, as the expired_token and grant_revoked rows already do; the full guidance stays in the error log. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159Mi1sTAHfUDZkSysntYz2 --------- 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
cb39cded81
commit
80b87c55fc
@@ -16,7 +16,7 @@ vi.mock('@/lib/auth/cron', () => ({
|
||||
// The route and the reconcile helper log through '@/lib/logger'. The real
|
||||
// logger suppresses info/warn under NODE_ENV=test, so warn/error output is
|
||||
// observed via these recorders instead of console spies. Console spies stay
|
||||
// for the route's remaining console.* lines (APIGW warn, summary, skip).
|
||||
// for the route's remaining console.* lines (summary, skip).
|
||||
const { warnRecorder, errorRecorder } = vi.hoisted(() => ({
|
||||
warnRecorder: vi.fn(),
|
||||
errorRecorder: vi.fn(),
|
||||
@@ -304,7 +304,6 @@ describe('AGI kvittenser cron', () => {
|
||||
expect(body.processed).toBe(1)
|
||||
expect(body.grantRevoked).toBe(1)
|
||||
expect(body.errors).toBe(0)
|
||||
expect(body.apigwConfig).toBe(0)
|
||||
expect(body.results[0]).toMatchObject({ status: 'grant_revoked', error: 'OMBUD_GRANT_MISSING' })
|
||||
expect(mockMarkGrantRevoked).toHaveBeenCalledWith('comp-1', expect.any(String), 'lasombud', 'OMBUD_GRANT_MISSING')
|
||||
|
||||
@@ -312,7 +311,7 @@ describe('AGI kvittenser cron', () => {
|
||||
expect(summaryLine).toContain('1 grants revoked')
|
||||
})
|
||||
|
||||
it('logs a warn (not error) and records apigw_config on ACCESS_DENIED', async () => {
|
||||
it('treats ACCESS_DENIED as a real error now that the APIGW subscription is expected (#2226)', async () => {
|
||||
mockCreateClient.mockReturnValueOnce(stubHappyTables())
|
||||
mockAgiGetKvittenser.mockRejectedValueOnce(
|
||||
new SkatteverketAuthError('Skatteverkets API-gateway nekade anropet.', 'ACCESS_DENIED'),
|
||||
@@ -322,80 +321,35 @@ describe('AGI kvittenser cron', () => {
|
||||
const body = await res.json()
|
||||
|
||||
expect(body.processed).toBe(1)
|
||||
expect(body.apigwConfig).toBe(1)
|
||||
expect(body.errors).toBe(0)
|
||||
expect(body.errors).toBe(1)
|
||||
// The old known-gap bucket is gone from the response contract.
|
||||
expect(body).not.toHaveProperty('apigwConfig')
|
||||
expect(body.results[0]).toMatchObject({
|
||||
declarationId: 'decl-1',
|
||||
status: 'apigw_config',
|
||||
status: 'error',
|
||||
// Machine-readable code, never the generic "Något gick fel" fallback.
|
||||
error: 'ACCESS_DENIED',
|
||||
})
|
||||
// companyId is internal log context, never response payload.
|
||||
expect(body.results[0]).not.toHaveProperty('companyId')
|
||||
|
||||
// Warn carries the actionable config hint plus the context to act on it.
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1)
|
||||
const [warnMessage, warnContext] = warnSpy.mock.calls[0]
|
||||
expect(warnMessage).toContain('Utvecklarportalen')
|
||||
expect(warnMessage).toContain('SKATTEVERKET_APIGW_CLIENT_ID')
|
||||
expect(warnContext).toMatchObject({
|
||||
// A gateway refusal is a regression, not a known gap: error level with
|
||||
// the gateway's message in context, no warn-once suppression, no
|
||||
// reconsent flagging.
|
||||
expect(errorRecorder).toHaveBeenCalledTimes(1)
|
||||
expect(String(errorRecorder.mock.calls[0][0])).toContain('Reconciliation failed')
|
||||
expect(errorRecorder.mock.calls[0][1]).toMatchObject({
|
||||
declarationId: 'decl-1',
|
||||
companyId: 'comp-1',
|
||||
period: expect.any(String),
|
||||
message: 'Skatteverkets API-gateway nekade anropet.',
|
||||
})
|
||||
|
||||
// The whole point: no error-level log for a config gap retries cannot heal.
|
||||
expect(errorSpy).not.toHaveBeenCalled()
|
||||
expect(errorRecorder).not.toHaveBeenCalled()
|
||||
expect(warnSpy).not.toHaveBeenCalled()
|
||||
expect(warnRecorder).not.toHaveBeenCalled()
|
||||
expect(mockMarkNeedsReconsent).not.toHaveBeenCalled()
|
||||
|
||||
// The config gap stays visible in the run summary.
|
||||
const summaryLine = logSpy.mock.calls.map(c => String(c[0])).find(m => m.includes('Processed'))
|
||||
expect(summaryLine).toContain('1 apigw config gaps')
|
||||
})
|
||||
|
||||
it('warns once per run on ACCESS_DENIED but records apigw_config for every declaration', async () => {
|
||||
mockCreateClient.mockReturnValueOnce(
|
||||
makeSupabaseStub({
|
||||
agi_declarations: {
|
||||
data: [
|
||||
PENDING_DECLARATION,
|
||||
{ ...PENDING_DECLARATION, id: 'decl-2', company_id: 'comp-2' },
|
||||
{ ...PENDING_DECLARATION, id: 'decl-3', company_id: 'comp-3' },
|
||||
],
|
||||
},
|
||||
skatteverket_tokens: { data: [{ user_id: 'user-1', status: 'active' }] },
|
||||
company_settings: { data: { org_number: '556123-4567', entity_type: 'aktiebolag' } },
|
||||
}),
|
||||
)
|
||||
for (let i = 0; i < 3; i++) {
|
||||
mockAgiGetKvittenser.mockRejectedValueOnce(
|
||||
new SkatteverketAuthError('Skatteverkets API-gateway nekade anropet.', 'ACCESS_DENIED'),
|
||||
)
|
||||
}
|
||||
|
||||
const res = await GET(makeRequest())
|
||||
const body = await res.json()
|
||||
|
||||
// Every affected declaration still gets its apigw_config outcome.
|
||||
expect(body.processed).toBe(3)
|
||||
expect(body.apigwConfig).toBe(3)
|
||||
expect(body.errors).toBe(0)
|
||||
expect(body.results.map((r: { declarationId: string }) => r.declarationId)).toEqual([
|
||||
'decl-1',
|
||||
'decl-2',
|
||||
'decl-3',
|
||||
])
|
||||
expect(body.results.every((r: { status: string }) => r.status === 'apigw_config')).toBe(true)
|
||||
expect(body.results.every((r: Record<string, unknown>) => !('companyId' in r))).toBe(true)
|
||||
|
||||
// But the identical config-gap warning is logged exactly once per run.
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1)
|
||||
expect(String(warnSpy.mock.calls[0][0])).toContain('Utvecklarportalen')
|
||||
expect(errorSpy).not.toHaveBeenCalled()
|
||||
expect(errorRecorder).not.toHaveBeenCalled()
|
||||
|
||||
const summaryLine = logSpy.mock.calls.map(c => String(c[0])).find(m => m.includes('Processed'))
|
||||
expect(summaryLine).toContain('3 apigw config gaps')
|
||||
expect(summaryLine).toContain('1 errors')
|
||||
expect(summaryLine).not.toContain('apigw')
|
||||
})
|
||||
|
||||
it('still flags reconsent codes as expired_token and marks the connection', async () => {
|
||||
@@ -408,7 +362,6 @@ describe('AGI kvittenser cron', () => {
|
||||
const body = await res.json()
|
||||
|
||||
expect(body.expired).toBe(1)
|
||||
expect(body.apigwConfig).toBe(0)
|
||||
expect(body.results[0]).toMatchObject({ status: 'expired_token', error: 'SESSION_EXPIRED' })
|
||||
expect(mockMarkNeedsReconsent).toHaveBeenCalledWith(expect.anything(), 'user-1', 'comp-1', 'SESSION_EXPIRED')
|
||||
expect(errorSpy).not.toHaveBeenCalled()
|
||||
@@ -443,7 +396,6 @@ describe('AGI kvittenser cron', () => {
|
||||
const body = await res.json()
|
||||
|
||||
expect(body.errors).toBe(1)
|
||||
expect(body.apigwConfig).toBe(0)
|
||||
expect(body.results[0]).toMatchObject({ status: 'error', error: 'Du har inte behörighet.' })
|
||||
expect(errorRecorder).toHaveBeenCalledTimes(1)
|
||||
expect(String(errorRecorder.mock.calls[0][0])).toContain('Reconciliation failed')
|
||||
|
||||
@@ -18,8 +18,8 @@ ensureInitialized()
|
||||
export const maxDuration = 60
|
||||
|
||||
// Failure logs route through the structured logger so third-party error
|
||||
// strings pass its redaction. The APIGW warn / budget + summary logs /
|
||||
// capability skip stay on console.*: their content is fixed internal strings.
|
||||
// strings pass its redaction. The budget + summary logs / capability skip
|
||||
// stay on console.*: their content is fixed internal strings.
|
||||
const log = createLogger('agi-kvittenser-cron')
|
||||
|
||||
// Cron responses must never be cached: they report a point-in-time run.
|
||||
@@ -104,14 +104,10 @@ export async function GET(request: Request) {
|
||||
type Result = {
|
||||
declarationId: string
|
||||
period: string
|
||||
status: 'signed' | 'still_pending' | 'already_claimed' | 'no_token' | 'no_company_settings' | 'expired_token' | 'grant_revoked' | 'apigw_config' | 'error'
|
||||
status: 'signed' | 'still_pending' | 'already_claimed' | 'no_token' | 'no_company_settings' | 'expired_token' | 'grant_revoked' | 'error'
|
||||
error?: string
|
||||
}
|
||||
const results: Result[] = []
|
||||
// The APIGW subscription gap is one run-level configuration problem, not a
|
||||
// per-declaration one: warn once per run instead of spamming an identical
|
||||
// warning for every affected declaration.
|
||||
let apigwAccessDeniedWarned = false
|
||||
|
||||
for (const decl of pending) {
|
||||
if (Date.now() - startTime > TIME_BUDGET_MS) {
|
||||
@@ -181,30 +177,23 @@ export async function GET(request: Request) {
|
||||
results.push({ declarationId, period, status: 'expired_token', error: err.code })
|
||||
continue
|
||||
}
|
||||
if (err instanceof SkatteverketAuthError && err.code === 'ACCESS_DENIED') {
|
||||
// Skatteverkets API gateway rejected our client credentials before
|
||||
// the user's bearer was ever evaluated: the APIGW client
|
||||
// (SKATTEVERKET_APIGW_CLIENT_ID) lacks an Utvecklarportalen
|
||||
// subscription for the AGI hantera API. Retrying every run cannot
|
||||
// heal this and the user reconnecting via BankID does not help, so
|
||||
// log at warn level instead of error to keep the 2h cron from
|
||||
// producing error-noise for a known configuration gap. The distinct
|
||||
// status keeps the gap visible in the run summary until fixed. The
|
||||
// warn is emitted once per run (context is the first affected
|
||||
// declaration); every affected declaration still lands in results.
|
||||
if (!apigwAccessDeniedWarned) {
|
||||
apigwAccessDeniedWarned = true
|
||||
console.warn(
|
||||
'[agi-kvittenser-cron] APIGW client lacks Utvecklarportalen subscription for the AGI hantera API; check SKATTEVERKET_APIGW_CLIENT_ID subscriptions. Skipping affected declarations until the subscription is added.',
|
||||
{ declarationId, companyId, period, message },
|
||||
)
|
||||
}
|
||||
results.push({ declarationId, period, status: 'apigw_config', error: err.code })
|
||||
continue
|
||||
}
|
||||
|
||||
// ACCESS_DENIED (Skatteverket's gateway refusing the APIGW client) is
|
||||
// no longer bucketed as a known configuration gap: the AGI hantera
|
||||
// subscription is expected to be in place (#2226), so a gateway refusal
|
||||
// is a real regression and lands in the error path below like any
|
||||
// other failure.
|
||||
log.error('Reconciliation failed', { declarationId, companyId, period, message })
|
||||
results.push({ declarationId, period, status: 'error', error: getErrorMessage(err) })
|
||||
// A gateway refusal keeps its machine-readable code in the body (as the
|
||||
// expired_token / grant_revoked rows do): the keyword heuristic in
|
||||
// getErrorMessage misses the gateway wording and would collapse it
|
||||
// into "Något gick fel". The full guidance is in the error log above.
|
||||
const isGatewayRefusal = err instanceof SkatteverketAuthError && err.code === 'ACCESS_DENIED'
|
||||
results.push({
|
||||
declarationId,
|
||||
period,
|
||||
status: 'error',
|
||||
error: isGatewayRefusal ? err.code : getErrorMessage(err),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -213,11 +202,10 @@ export async function GET(request: Request) {
|
||||
const alreadyClaimed = results.filter(r => r.status === 'already_claimed').length
|
||||
const expired = results.filter(r => r.status === 'expired_token').length
|
||||
const grantRevoked = results.filter(r => r.status === 'grant_revoked').length
|
||||
const apigwConfig = results.filter(r => r.status === 'apigw_config').length
|
||||
const errors = results.filter(r => r.status === 'error').length
|
||||
|
||||
console.log(
|
||||
`[agi-kvittenser-cron] Processed ${results.length}: ${signed} signed, ${stillPending} still pending, ${alreadyClaimed} already claimed, ${expired} expired, ${grantRevoked} grants revoked, ${apigwConfig} apigw config gaps, ${errors} errors`,
|
||||
`[agi-kvittenser-cron] Processed ${results.length}: ${signed} signed, ${stillPending} still pending, ${alreadyClaimed} already claimed, ${expired} expired, ${grantRevoked} grants revoked, ${errors} errors`,
|
||||
)
|
||||
|
||||
return NextResponse.json(
|
||||
@@ -228,7 +216,6 @@ export async function GET(request: Request) {
|
||||
alreadyClaimed,
|
||||
expired,
|
||||
grantRevoked,
|
||||
apigwConfig,
|
||||
errors,
|
||||
results,
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user