fix(providers): stop the Fortnox reconnect loop, and make the attachment scopes opt-in (#1761)
The Fortnox document import needs the archive and connectfile scopes, which the registered Fortnox app does not have. Since #1549 pulled them out of the connect request (they broke every connect with invalid_scope before login), every attachment call fails and the user was told "Koppla om Fortnox och godkann behorigheterna", under a button that reruns an authorize URL still not asking for those scopes. Klura AB followed that loop four times and bought the Fortnox Arkiv module trying to satisfy it. Prod evidence: no Fortnox attachment has ever imported, across 166 companies and 24 consents since the feature shipped, and no live token carries the scopes. The error and the scope list now derive from one flag, FORTNOX_DOCUMENT_SCOPES_APPROVED. While it is false a permission failure maps to a new PROVIDER_DOCUMENT_SCOPES_UNAVAILABLE, which says the permission is missing on our side, that reconnecting will not help, and that the rest of the migration came through; the card offers no button, because no user action can succeed. The attachment scopes also become an opt-in consent rather than part of every connect. Fortnox derives customer licence requirements from what an integration requests, so asking everyone for Arkivplats would put a licence in front of customers who never import a receipt; and keeping it off the default connect caps the blast radius of a wrong portal registration at the underlag flow rather than every Fortnox connection. buildFortnoxAuthUrl already took per-call scopes, provider-client simply never passed any, so this threads documentScopes from that one button through /connect into the authorize URL. A document consent is always a superset of an ordinary one: the callback overwrites the consent's tokens in place, so a narrower grant would revoke the migration's own ledger access. Pinned by a test that holds either way the flag is set, alongside one for the 400-with-behorighet answer that six companies hit between 08-13 and 08-19 and saw only a generic retry for. Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Jakob Wennberg
Claude Opus 5
parent
c5b7716f74
commit
f3e4fdcf32
@@ -0,0 +1,80 @@
|
||||
import { describe, it, expect, vi } from 'vitest'
|
||||
|
||||
/**
|
||||
* The voucher-attachment scopes (Arkivplats + Koppla filer) are opt-in per
|
||||
* authorize call rather than part of every Fortnox connect. Two reasons, both
|
||||
* of which have already cost us once:
|
||||
*
|
||||
* 1. Fortnox derives its customer licence requirements from what the
|
||||
* integration requests, so asking every customer for Arkivplats would put
|
||||
* a licence in front of people who never import a receipt.
|
||||
* 2. A scope the registered app lacks makes authorize reject with
|
||||
* invalid_scope before login. Keeping it off the ordinary connect means
|
||||
* the blast radius of getting it wrong is the underlag flow, not every
|
||||
* Fortnox connection in production (incident 2026-08-13).
|
||||
*/
|
||||
|
||||
vi.mock('@/lib/supabase/server', () => ({
|
||||
createServiceClient: () => ({ from: vi.fn() }),
|
||||
createServiceClientNoCookies: () => ({ from: vi.fn() }),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/providers/oauth-config', () => ({
|
||||
getOAuthConfig: () => ({
|
||||
clientId: 'client-id',
|
||||
clientSecret: 'client-secret',
|
||||
redirectUri: 'https://accounted.example.test/callback',
|
||||
}),
|
||||
}))
|
||||
|
||||
import {
|
||||
fortnoxConsentScopes,
|
||||
FORTNOX_DOCUMENT_SCOPES,
|
||||
} from '@/lib/providers/fortnox/oauth'
|
||||
import { getAuthUrl } from '../lib/provider-client'
|
||||
|
||||
const scopesOf = (url: string) =>
|
||||
new URL(url).searchParams.get('scope')?.split(' ') ?? []
|
||||
|
||||
describe('Fortnox document scopes are opt-in', () => {
|
||||
it('leaves the attachment scopes out of an ordinary consent', () => {
|
||||
expect(fortnoxConsentScopes()).not.toContain('archive')
|
||||
expect(fortnoxConsentScopes()).not.toContain('connectfile')
|
||||
})
|
||||
|
||||
// The OAuth callback overwrites the consent's tokens in place, so a document
|
||||
// consent that dropped the base scopes would silently revoke the migration's
|
||||
// own access to the ledger it just imported.
|
||||
it('never drops the base scopes when asking for the attachment scopes', () => {
|
||||
const base = fortnoxConsentScopes()
|
||||
const withDocuments = fortnoxConsentScopes({ documents: true })
|
||||
|
||||
for (const scope of base) {
|
||||
expect(withDocuments).toContain(scope)
|
||||
}
|
||||
expect(withDocuments.length).toBeGreaterThanOrEqual(base.length)
|
||||
expect(new Set(withDocuments).size).toBe(withDocuments.length)
|
||||
})
|
||||
|
||||
it('names both attachment scopes, so the portal registration has a source of truth', () => {
|
||||
expect(FORTNOX_DOCUMENT_SCOPES).toEqual(['archive', 'connectfile'])
|
||||
})
|
||||
|
||||
it('builds the ordinary connect URL without the attachment scopes', async () => {
|
||||
const { url } = await getAuthUrl('fortnox', 'state-1', 'https://cb.test')
|
||||
|
||||
expect(scopesOf(url)).toEqual(fortnoxConsentScopes())
|
||||
expect(scopesOf(url)).not.toContain('archive')
|
||||
})
|
||||
|
||||
it('builds the underlag reconnect URL from the document consent scopes', async () => {
|
||||
const { url } = await getAuthUrl('fortnox', 'state-2', 'https://cb.test', {
|
||||
documentScopes: true,
|
||||
})
|
||||
|
||||
expect(scopesOf(url)).toEqual(fortnoxConsentScopes({ documents: true }))
|
||||
for (const scope of fortnoxConsentScopes()) {
|
||||
expect(scopesOf(url)).toContain(scope)
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -18,6 +18,19 @@ vi.mock('../lib/import-documents', () => {
|
||||
}
|
||||
})
|
||||
|
||||
const fortnoxOAuth = vi.hoisted(() => ({ documentScopesApproved: false }))
|
||||
|
||||
vi.mock('@/lib/providers/fortnox/oauth', async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<typeof import('@/lib/providers/fortnox/oauth')>()
|
||||
return {
|
||||
...actual,
|
||||
get FORTNOX_DOCUMENT_SCOPES_APPROVED() {
|
||||
return fortnoxOAuth.documentScopesApproved
|
||||
},
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('../lib/provider-client', () => {
|
||||
class ProviderTokenInvalidError extends Error {
|
||||
constructor(
|
||||
@@ -106,6 +119,7 @@ describe('POST /import-documents', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
eventBus.clear()
|
||||
fortnoxOAuth.documentScopesApproved = false
|
||||
})
|
||||
|
||||
it('passes dry-run discovery through without storing documents', async () => {
|
||||
@@ -138,7 +152,8 @@ describe('POST /import-documents', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('returns an actionable 403 when Fortnox lacks archive/connectfile scopes', async () => {
|
||||
it('asks the user to reconnect only once the connect request carries the scopes', async () => {
|
||||
fortnoxOAuth.documentScopesApproved = true
|
||||
;(importProviderDocuments as Mock).mockRejectedValue(
|
||||
new FortnoxDocumentScopesRequiredError(),
|
||||
)
|
||||
@@ -153,6 +168,27 @@ describe('POST /import-documents', () => {
|
||||
expect(body.error.message).toContain('Koppla om Fortnox')
|
||||
expect(body.error.message_en).toContain('Reconnect Fortnox')
|
||||
})
|
||||
|
||||
// Klura AB, 2026-08-20: the connect request does not ask Fortnox for Arkiv
|
||||
// and Koppla fil at all, so the reconnect advice sent the user around a loop
|
||||
// four times (and to buy the Fortnox Arkiv module) for nothing.
|
||||
it('says the permission is missing on our side while the scopes are unapproved', async () => {
|
||||
fortnoxOAuth.documentScopesApproved = false
|
||||
;(importProviderDocuments as Mock).mockRejectedValue(
|
||||
new FortnoxDocumentScopesRequiredError(),
|
||||
)
|
||||
|
||||
const response = await handler(request(false), buildContext())
|
||||
const { status, body } = await parseJsonResponse<{
|
||||
error: { code: string; message: string; message_en?: string }
|
||||
}>(response)
|
||||
|
||||
expect(status).toBe(403)
|
||||
expect(body.error.code).toBe('PROVIDER_DOCUMENT_SCOPES_UNAVAILABLE')
|
||||
expect(body.error.message).not.toContain('Koppla om Fortnox')
|
||||
expect(body.error.message).toContain('Att koppla om hjälper inte')
|
||||
expect(body.error.message_en).toContain('Reconnecting will not help')
|
||||
})
|
||||
})
|
||||
|
||||
describe('POST /submit-token Bokio error mapping', () => {
|
||||
|
||||
@@ -479,6 +479,36 @@ describe('importProviderDocuments', () => {
|
||||
).rejects.toThrow('Fortnox consent lacks archive/connectfile scope: reconnect required')
|
||||
})
|
||||
|
||||
// Six companies hit exactly this between 2026-08-13 and 08-19 and were told
|
||||
// "kunde inte importera underlag, försök igen", with a retry that could not
|
||||
// work: Fortnox answers an unscoped attachment listing with 400 plus a
|
||||
// behörighet message, not only with 403.
|
||||
it('treats a Fortnox 400 that names the missing permission as the same failure', async () => {
|
||||
const supabase = wireFortnox()
|
||||
mockFetchFortnoxFinancialYears.mockRejectedValue(
|
||||
new FortnoxApiError(
|
||||
'bad request',
|
||||
400,
|
||||
JSON.stringify({
|
||||
ErrorInformation: {
|
||||
error: 1,
|
||||
message: 'Otillräcklig behörighet för att utföra anropet',
|
||||
code: 2000663,
|
||||
},
|
||||
}),
|
||||
),
|
||||
)
|
||||
|
||||
await expect(
|
||||
importProviderDocuments({
|
||||
supabase,
|
||||
companyId: COMPANY,
|
||||
userId: USER,
|
||||
consentId: 'c1',
|
||||
}),
|
||||
).rejects.toThrow('Fortnox consent lacks archive/connectfile scope: reconnect required')
|
||||
})
|
||||
|
||||
it('explains that a Fortnox archive-download 403 requires reconnecting', async () => {
|
||||
const supabase = wireFortnox()
|
||||
mockDownloadFortnoxArchiveFile.mockRejectedValue(
|
||||
|
||||
@@ -323,7 +323,47 @@ describe('OAuth redirect_uri symmetry between authorize and exchange', () => {
|
||||
)
|
||||
|
||||
expect(res.status).toBe(200)
|
||||
expect(getAuthUrl).toHaveBeenCalledWith('fortnox', 'otc-code-1', OVERRIDE_URI)
|
||||
expect(getAuthUrl).toHaveBeenCalledWith(
|
||||
'fortnox',
|
||||
'otc-code-1',
|
||||
OVERRIDE_URI,
|
||||
// A first connect never asks for the voucher-attachment scopes: those
|
||||
// carry a Fortnox licence requirement and belong to the opt-in underlag
|
||||
// reconnect only.
|
||||
{ documentScopes: undefined },
|
||||
)
|
||||
})
|
||||
|
||||
// The underlag follow-up is the only caller allowed to widen the consent.
|
||||
it('reconnect asks for the attachment scopes only when the underlag flow requests them', async () => {
|
||||
;(listConsents as Mock).mockResolvedValue([
|
||||
{ id: 'consent-1', provider: 'fortnox', status: 1 },
|
||||
])
|
||||
|
||||
const reconnect = (documentScopes?: boolean) =>
|
||||
connectHandler(
|
||||
createMockRequest('http://localhost/api/extensions/ext/arcim-migration/connect', {
|
||||
method: 'POST',
|
||||
body: { provider: 'fortnox', reconnect: true, ...(documentScopes === undefined ? {} : { documentScopes }) },
|
||||
}),
|
||||
connectCtx(),
|
||||
)
|
||||
|
||||
expect((await reconnect(true)).status).toBe(200)
|
||||
expect(getAuthUrl).toHaveBeenLastCalledWith(
|
||||
'fortnox',
|
||||
'otc-code-1',
|
||||
OVERRIDE_URI,
|
||||
{ documentScopes: true },
|
||||
)
|
||||
|
||||
expect((await reconnect()).status).toBe(200)
|
||||
expect(getAuthUrl).toHaveBeenLastCalledWith(
|
||||
'fortnox',
|
||||
'otc-code-1',
|
||||
OVERRIDE_URI,
|
||||
{ documentScopes: false },
|
||||
)
|
||||
})
|
||||
|
||||
it('exchange leg passes the SAME env-override redirect URI to exchangeAuthToken', async () => {
|
||||
|
||||
@@ -34,6 +34,7 @@ import { suggestMappings, getMappingStats, isSystemAccount } from '@/lib/import/
|
||||
import { loadMappings, generateImportPreview, executeSIEImport, findOverlappingPeriodImports } from '@/lib/import/sie-import'
|
||||
import { BAS_REFERENCE } from '@/lib/bookkeeping/bas-reference'
|
||||
import type { ProviderName } from '@/lib/providers/types'
|
||||
import { FORTNOX_DOCUMENT_SCOPES_APPROVED } from '@/lib/providers/fortnox/oauth'
|
||||
import { errorResponseFromCode } from '@/lib/errors/get-structured-error'
|
||||
import { classifyProviderError } from '@/lib/providers/with-provider-call'
|
||||
import { FortnoxApiError, fortnoxErrorMessage } from '@/lib/providers/fortnox/client'
|
||||
@@ -119,7 +120,11 @@ function resolveArcimCallbackUrl(provider: ArcimProvider | ProviderName): string
|
||||
* consent_id, so re-running OAuth against the same consent overwrites a dead
|
||||
* refresh-token pair in place: no disconnect/recreate needed.
|
||||
*/
|
||||
async function buildArcimOAuthUrl(consentId: string, provider: ArcimProvider): Promise<string> {
|
||||
async function buildArcimOAuthUrl(
|
||||
consentId: string,
|
||||
provider: ArcimProvider,
|
||||
options?: { documentScopes?: boolean },
|
||||
): Promise<string> {
|
||||
// Server-side state row: consent id, provider (via the consent), expiry and a
|
||||
// consumed marker all live in provider_otc. The `state` handed to the provider
|
||||
// is that row's opaque random primary key, nothing more.
|
||||
@@ -131,7 +136,9 @@ async function buildArcimOAuthUrl(consentId: string, provider: ArcimProvider): P
|
||||
// above. It deliberately encodes NOTHING. The previous base64url JSON payload
|
||||
// was attacker-authored input the callback trusted, so anyone who learned a
|
||||
// consent id could redirect their own provider tokens onto that consent.
|
||||
const { url } = await getAuthUrl(provider, otc.code, callbackUrl)
|
||||
const { url } = await getAuthUrl(provider, otc.code, callbackUrl, {
|
||||
documentScopes: options?.documentScopes,
|
||||
})
|
||||
return url
|
||||
}
|
||||
|
||||
@@ -259,11 +266,23 @@ export const arcimMigrationExtension: Extension = {
|
||||
|
||||
const companyId = ctx?.companyId ?? user.id
|
||||
|
||||
const { provider, companyName, orgNumber, reconnect } = await request.json() as {
|
||||
const {
|
||||
provider,
|
||||
companyName,
|
||||
orgNumber,
|
||||
reconnect,
|
||||
documentScopes,
|
||||
} = await request.json() as {
|
||||
provider: ArcimProvider
|
||||
companyName?: string
|
||||
orgNumber?: string
|
||||
reconnect?: boolean
|
||||
/**
|
||||
* Reconnect specifically to grant the voucher-attachment scopes.
|
||||
* Only the underlag follow-up sets it, so an ordinary connect never
|
||||
* asks the customer for Arkivplats and Koppla filer.
|
||||
*/
|
||||
documentScopes?: boolean
|
||||
}
|
||||
|
||||
if (!provider) {
|
||||
@@ -300,7 +319,9 @@ export const arcimMigrationExtension: Extension = {
|
||||
await ctx.settings.set('provider', provider)
|
||||
}
|
||||
if (providerInfo.authType === 'oauth') {
|
||||
const authUrl = await buildArcimOAuthUrl(stale.id, provider)
|
||||
const authUrl = await buildArcimOAuthUrl(stale.id, provider, {
|
||||
documentScopes: documentScopes === true,
|
||||
})
|
||||
return NextResponse.json({
|
||||
consentId: stale.id,
|
||||
authType: 'oauth',
|
||||
@@ -1386,8 +1407,13 @@ export const arcimMigrationExtension: Extension = {
|
||||
error instanceof FortnoxApiError ? error.body?.slice(0, 500) : undefined,
|
||||
})
|
||||
if (error instanceof FortnoxDocumentScopesRequiredError) {
|
||||
// Reconnecting only helps once the connect request actually asks
|
||||
// for Arkiv and Koppla fil. While it does not, say so plainly
|
||||
// instead of sending the user around a loop that cannot succeed.
|
||||
return errorResponseFromCode(
|
||||
'PROVIDER_DOCUMENT_SCOPES_REQUIRED',
|
||||
FORTNOX_DOCUMENT_SCOPES_APPROVED
|
||||
? 'PROVIDER_DOCUMENT_SCOPES_REQUIRED'
|
||||
: 'PROVIDER_DOCUMENT_SCOPES_UNAVAILABLE',
|
||||
moduleLog,
|
||||
{ status: 403 },
|
||||
)
|
||||
|
||||
@@ -10,7 +10,10 @@ import { randomBytes } from 'node:crypto'
|
||||
import { createServiceClient } from '@/lib/supabase/server'
|
||||
import type { ProviderName } from '@/lib/providers/types'
|
||||
import { getOAuthConfig } from '@/lib/providers/oauth-config'
|
||||
import { buildFortnoxAuthUrl } from '@/lib/providers/fortnox/oauth'
|
||||
import {
|
||||
buildFortnoxAuthUrl,
|
||||
fortnoxConsentScopes,
|
||||
} from '@/lib/providers/fortnox/oauth'
|
||||
import { exchangeFortnoxCode } from '@/lib/providers/fortnox/oauth'
|
||||
import { buildVismaAuthUrl, exchangeVismaCode } from '@/lib/providers/visma/oauth'
|
||||
import { refreshBjornLundenToken } from '@/lib/providers/bjornlunden/oauth'
|
||||
@@ -314,6 +317,12 @@ export async function getAuthUrl(
|
||||
provider: ProviderName,
|
||||
state?: string,
|
||||
redirectUri?: string,
|
||||
/**
|
||||
* Ask Fortnox for the voucher-attachment scopes as well. Opt-in, so only a
|
||||
* user who wants their underlag is put in front of the extra permissions
|
||||
* (and their licence requirements).
|
||||
*/
|
||||
options?: { documentScopes?: boolean },
|
||||
): Promise<{ url: string }> {
|
||||
const config = getOAuthConfig(provider)
|
||||
|
||||
@@ -323,7 +332,10 @@ export async function getAuthUrl(
|
||||
: config
|
||||
|
||||
if (provider === 'fortnox') {
|
||||
const url = buildFortnoxAuthUrl(effectiveConfig, { state })
|
||||
const url = buildFortnoxAuthUrl(effectiveConfig, {
|
||||
state,
|
||||
scopes: fortnoxConsentScopes({ documents: options?.documentScopes }),
|
||||
})
|
||||
return { url }
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user