fix(skatteverket): let company members read data connected by another member (#1691)
Closes #1673. Token rows are per (user, company), but the read resolver short-circuited to the caller's own row whenever a userId was passed, so a member who never pressed "Anslut" resolved to "no token" for a company that was connected. The company-wide fallback used .maybeSingle(), which errors as soon as two members have both connected and turned that into "nobody connected" for everyone. - resolve-auth: findCompanyTokenUser() reads all of the company's rows ordered by created_at desc and picks the caller's own active row first, then any other member's active row, then needs_reconsent rows; the userId branch no longer short-circuits. The auth carries the token OWNER's userId so refresh writes back to the owner's row. - /skattekonto/saldo and /skattekonto/sync resolve the company token instead of getTokens(caller); /declaration/submitted and /decided answer a SESSION_EXPIRED reconnect prompt for needs_reconsent instead of NOT_CONNECTED. Connect/disconnect//status stay on the caller's own row. - The connection.expired event names the token owner, not the caller who triggered the sync; the notification lookup filters by company too. - The two kvittens crons flag needs_reconsent through the same shared pick instead of .maybeSingle(). Tests: two members, one connects, both read; both connect, both read; no row -> NOT_CONNECTED; dead-only rows -> reconnect prompt; sync auth carries the owner; event recipient is the owner. Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Jakob Wennberg
Claude Fable 5
parent
619b446c52
commit
1af5846adf
@@ -181,6 +181,18 @@ describe('sendConnectionExpiredNotification', () => {
|
||||
expect(mockSendEmail).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keys the episode on the (user, company) token row, not the user alone', async () => {
|
||||
const { supabase, ops } = makeSupabase()
|
||||
|
||||
await sendConnectionExpiredNotification(supabase, baseInput)
|
||||
|
||||
// Rows are per (user, company): a multi-company operator holds several,
|
||||
// and a user-only maybeSingle() errored out and silently skipped the
|
||||
// email (#1673 follow-through).
|
||||
const tokenRead = ops.find((o) => o.table === 'skatteverket_tokens')
|
||||
expect(tokenRead?.filters).toMatchObject({ user_id: 'user-1', company_id: 'company-1' })
|
||||
})
|
||||
|
||||
it('skips when no token row exists (already disconnected)', async () => {
|
||||
const { supabase } = makeSupabase({ token: null })
|
||||
|
||||
|
||||
@@ -2,6 +2,10 @@
|
||||
* resolveReadAuth preference matrix: system credentials when the flag is on
|
||||
* and the grant is verified; the company's user token otherwise; explicit
|
||||
* no_token / needs_reconsent outcomes for the crons' quiet buckets.
|
||||
*
|
||||
* Token rows are per (user, company): the caller's own row wins when it
|
||||
* exists, any other member's active row serves otherwise (#1673), and two
|
||||
* connected members must never degrade to "nobody connected".
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
|
||||
@@ -22,16 +26,37 @@ vi.mock('../lib/system-auth/config', async (importOriginal) => {
|
||||
}
|
||||
})
|
||||
|
||||
import { resolveReadAuth, hasVerifiedGrant } from '../lib/resolve-auth'
|
||||
import { resolveReadAuth, hasVerifiedGrant, findCompanyTokenUser } from '../lib/resolve-auth'
|
||||
import type { SupabaseClient } from '@supabase/supabase-js'
|
||||
|
||||
function makeSupabase(tokenRow: { user_id: string; status: string } | null) {
|
||||
type TokenRow = { user_id: string; status: string; created_at?: string }
|
||||
|
||||
/**
|
||||
* Chain stub for the company token lookup: `.select().eq().order()` resolves
|
||||
* to the given rows, already in created_at DESC order (the DB does the
|
||||
* ordering; the resolver only picks). Pass a single row for the one-member
|
||||
* case, an array for several members, null for no rows.
|
||||
*/
|
||||
function makeSupabase(
|
||||
tokenRows: TokenRow | TokenRow[] | null,
|
||||
opts: { error?: { message: string } } = {},
|
||||
) {
|
||||
const rows = tokenRows === null ? [] : Array.isArray(tokenRows) ? tokenRows : [tokenRows]
|
||||
const chain: Record<string, unknown> = {}
|
||||
for (const m of ['select', 'eq']) {
|
||||
chain[m] = vi.fn(() => chain)
|
||||
}
|
||||
chain.maybeSingle = vi.fn().mockResolvedValue({ data: tokenRow, error: null })
|
||||
return { from: vi.fn(() => chain) } as unknown as SupabaseClient
|
||||
chain.order = vi.fn().mockResolvedValue(
|
||||
opts.error ? { data: null, error: opts.error } : { data: rows, error: null },
|
||||
)
|
||||
// Regression guard for #1673: the company lookup must never go through
|
||||
// maybeSingle(), which errors as soon as two members have connected.
|
||||
chain.maybeSingle = vi.fn(() => {
|
||||
throw new Error('maybeSingle() must not be used for the company token lookup')
|
||||
})
|
||||
return { from: vi.fn(() => chain), _chain: chain } as unknown as SupabaseClient & {
|
||||
_chain: Record<string, ReturnType<typeof vi.fn>>
|
||||
}
|
||||
}
|
||||
|
||||
const GRANTED_CONNECTION = {
|
||||
@@ -100,13 +125,26 @@ describe('resolveReadAuth', () => {
|
||||
expect(result).toMatchObject({ ok: true, source: 'user' })
|
||||
})
|
||||
|
||||
it('explicit userId short-circuits the company lookup', async () => {
|
||||
it('explicit userId with no company row at all -> no_token (no longer short-circuits to the caller)', async () => {
|
||||
const supabase = makeSupabase(null)
|
||||
const result = await resolveReadAuth(supabase, 'company-1', {
|
||||
requires: 'moms_ombud',
|
||||
userId: 'user-9',
|
||||
})
|
||||
expect(result).toMatchObject({ ok: true, source: 'user', tokenUserId: 'user-9' })
|
||||
expect(result).toEqual({ ok: false, reason: 'no_token' })
|
||||
})
|
||||
|
||||
it('orders the company lookup by created_at desc and filters by company', async () => {
|
||||
const supabase = makeSupabase({ user_id: 'user-1', status: 'active' })
|
||||
await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud' })
|
||||
expect(supabase._chain.eq).toHaveBeenCalledWith('company_id', 'company-1')
|
||||
expect(supabase._chain.order).toHaveBeenCalledWith('created_at', { ascending: false })
|
||||
})
|
||||
|
||||
it('lookup error -> no_token (never throws into the caller)', async () => {
|
||||
const supabase = makeSupabase(null, { error: { message: 'boom' } })
|
||||
const result = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud' })
|
||||
expect(result).toEqual({ ok: false, reason: 'no_token' })
|
||||
})
|
||||
|
||||
it('no token row -> no_token', async () => {
|
||||
@@ -122,6 +160,115 @@ describe('resolveReadAuth', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('resolveReadAuth: two members of the same company (#1673)', () => {
|
||||
const A = 'user-a'
|
||||
const B = 'user-b'
|
||||
const NEWER = '2026-08-18T10:00:00Z'
|
||||
const OLDER = '2026-08-18T09:00:00Z'
|
||||
|
||||
it('one connects, both read: the member who never connected resolves the connected token', async () => {
|
||||
const supabase = makeSupabase({ user_id: A, status: 'active', created_at: NEWER })
|
||||
|
||||
const asA = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: A })
|
||||
expect(asA).toMatchObject({ ok: true, source: 'user', tokenUserId: A })
|
||||
|
||||
const asB = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: B })
|
||||
expect(asB).toMatchObject({ ok: true, source: 'user', tokenUserId: A })
|
||||
// The auth handed to skvRequest carries the token OWNER: the refresh
|
||||
// writes back to A's row, never to B's (missing) one.
|
||||
if (asB.ok) expect(asB.auth).toEqual({ mode: 'user', supabase, userId: A, companyId: 'company-1' })
|
||||
})
|
||||
|
||||
it('both connect, both read: each member gets their own token, nobody degrades to no_token', async () => {
|
||||
const supabase = makeSupabase([
|
||||
{ user_id: B, status: 'active', created_at: NEWER },
|
||||
{ user_id: A, status: 'active', created_at: OLDER },
|
||||
])
|
||||
|
||||
const asA = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: A })
|
||||
expect(asA).toMatchObject({ ok: true, tokenUserId: A })
|
||||
|
||||
const asB = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: B })
|
||||
expect(asB).toMatchObject({ ok: true, tokenUserId: B })
|
||||
})
|
||||
|
||||
it('both connect, background read (no caller): the most recently issued row wins', async () => {
|
||||
const supabase = makeSupabase([
|
||||
{ user_id: B, status: 'active', created_at: NEWER },
|
||||
{ user_id: A, status: 'active', created_at: OLDER },
|
||||
])
|
||||
const result = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud' })
|
||||
expect(result).toMatchObject({ ok: true, tokenUserId: B })
|
||||
})
|
||||
|
||||
it('a dead own row does not shadow another member\'s live token', async () => {
|
||||
const supabase = makeSupabase([
|
||||
{ user_id: B, status: 'needs_reconsent', created_at: NEWER },
|
||||
{ user_id: A, status: 'active', created_at: OLDER },
|
||||
])
|
||||
const asB = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: B })
|
||||
expect(asB).toMatchObject({ ok: true, tokenUserId: A })
|
||||
})
|
||||
|
||||
it('every row flagged needs_reconsent -> needs_reconsent (own row reported when present)', async () => {
|
||||
const supabase = makeSupabase([
|
||||
{ user_id: A, status: 'needs_reconsent', created_at: NEWER },
|
||||
{ user_id: B, status: 'needs_reconsent', created_at: OLDER },
|
||||
])
|
||||
expect(await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: B })).toEqual({
|
||||
ok: false,
|
||||
reason: 'needs_reconsent',
|
||||
})
|
||||
expect(await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud' })).toEqual({
|
||||
ok: false,
|
||||
reason: 'needs_reconsent',
|
||||
})
|
||||
})
|
||||
|
||||
it('system mode keeps the caller as notification recipient, falls back to the token owner', async () => {
|
||||
mockMode.mockReturnValue('on')
|
||||
mockConfigured.mockReturnValue(true)
|
||||
mockGetConnection.mockResolvedValue(GRANTED_CONNECTION)
|
||||
const supabase = makeSupabase({ user_id: A, status: 'active', created_at: NEWER })
|
||||
|
||||
expect(
|
||||
await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: B }),
|
||||
).toMatchObject({ ok: true, source: 'system', tokenUserId: B })
|
||||
expect(await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud' })).toMatchObject({
|
||||
ok: true,
|
||||
source: 'system',
|
||||
tokenUserId: A,
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('findCompanyTokenUser', () => {
|
||||
it('returns null for a company with no rows', async () => {
|
||||
expect(await findCompanyTokenUser(makeSupabase(null), 'company-1')).toBeNull()
|
||||
})
|
||||
|
||||
it('prefers the given user, then active status, then recency', async () => {
|
||||
const supabase = makeSupabase([
|
||||
{ user_id: 'user-c', status: 'needs_reconsent', created_at: '3' },
|
||||
{ user_id: 'user-b', status: 'active', created_at: '2' },
|
||||
{ user_id: 'user-a', status: 'active', created_at: '1' },
|
||||
])
|
||||
expect(await findCompanyTokenUser(supabase, 'company-1')).toEqual({
|
||||
userId: 'user-b',
|
||||
needsReconsent: false,
|
||||
})
|
||||
expect(await findCompanyTokenUser(supabase, 'company-1', { preferUserId: 'user-a' })).toEqual({
|
||||
userId: 'user-a',
|
||||
needsReconsent: false,
|
||||
})
|
||||
// Preferring a user whose row is dead still yields the live row.
|
||||
expect(await findCompanyTokenUser(supabase, 'company-1', { preferUserId: 'user-c' })).toEqual({
|
||||
userId: 'user-b',
|
||||
needsReconsent: false,
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('hasVerifiedGrant', () => {
|
||||
it('requires both the grant and a verified/partial aggregate status', async () => {
|
||||
mockGetConnection.mockResolvedValue({
|
||||
|
||||
@@ -0,0 +1,163 @@
|
||||
/**
|
||||
* Skattekonto read routes resolve the COMPANY's connection, not the caller's
|
||||
* (#1673): a member who never pressed "Anslut" sees the saldo snapshot and can
|
||||
* trigger a sync on the token another member connected. The auth handed to
|
||||
* the sync carries the token owner's userId, so the refresh writes back to
|
||||
* the owner's row. Two connected members both keep working, and a company
|
||||
* with no row at all still answers 401 NOT_CONNECTED (the page's empty state).
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
|
||||
vi.mock('@/lib/entitlements/has-capability', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('@/lib/entitlements/has-capability')>()
|
||||
return { ...actual, requireCapability: vi.fn().mockResolvedValue(null) }
|
||||
})
|
||||
|
||||
const mockSyncSkattekonto = vi.fn()
|
||||
vi.mock('../lib/skattekonto-sync', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('../lib/skattekonto-sync')>()
|
||||
return { ...actual, syncSkattekonto: (...args: unknown[]) => mockSyncSkattekonto(...args) }
|
||||
})
|
||||
|
||||
import { skatteverketExtension } from '../index'
|
||||
import type { ExtensionContext } from '@/lib/extensions/types'
|
||||
|
||||
const A = 'user-a'
|
||||
const B = 'user-b'
|
||||
const COMPANY = 'company-1'
|
||||
type TokenRow = { user_id: string; status: string; created_at: string }
|
||||
|
||||
function makeContext(userId: string, tokenRows: TokenRow[]): ExtensionContext {
|
||||
const chain: Record<string, unknown> = {}
|
||||
for (const m of ['select', 'eq']) chain[m] = vi.fn(() => chain)
|
||||
chain.order = vi.fn().mockResolvedValue({ data: tokenRows, error: null })
|
||||
chain.maybeSingle = vi.fn(() => {
|
||||
throw new Error('maybeSingle() must not be used for the company token lookup')
|
||||
})
|
||||
const supabase = { from: vi.fn(() => chain) }
|
||||
return {
|
||||
userId,
|
||||
companyId: COMPANY,
|
||||
extensionId: 'skatteverket',
|
||||
requestId: 'req_test',
|
||||
supabase,
|
||||
emit: vi.fn().mockResolvedValue(undefined),
|
||||
log: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), child: vi.fn() },
|
||||
settings: {
|
||||
get: vi.fn(async (key: string) => {
|
||||
if (key === 'skattekonto_balance_snapshot') {
|
||||
return { saldo: { saldoSkatteverket: 1234.5, saldoKronofogden: 0 }, fetchedAt: 1_700_000_000_000 }
|
||||
}
|
||||
if (key === 'skattekonto_last_synced_at') return '2026-08-18T06:00:00.000Z'
|
||||
return null
|
||||
}),
|
||||
set: vi.fn().mockResolvedValue(undefined),
|
||||
clear: vi.fn().mockResolvedValue(undefined),
|
||||
},
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any
|
||||
}
|
||||
|
||||
function route(method: string, path: string) {
|
||||
const found = skatteverketExtension.apiRoutes?.find(r => r.method === method && r.path === path)
|
||||
if (!found) throw new Error(`${method} ${path} must be registered`)
|
||||
return found
|
||||
}
|
||||
|
||||
function req(method: string, path: string) {
|
||||
return new Request(`https://test.local/api/extensions/ext/skatteverket${path}`, { method })
|
||||
}
|
||||
|
||||
const A_ROW: TokenRow = { user_id: A, status: 'active', created_at: '2026-08-18T09:00:00Z' }
|
||||
const B_ROW: TokenRow = { user_id: B, status: 'active', created_at: '2026-08-18T10:00:00Z' }
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockSyncSkattekonto.mockResolvedValue({
|
||||
booked: 1,
|
||||
upcoming: 0,
|
||||
saldoSkatteverket: 1234.5,
|
||||
saldoKronofogden: 0,
|
||||
syncedAt: '2026-08-18T12:00:00.000Z',
|
||||
})
|
||||
})
|
||||
|
||||
describe('GET /skattekonto/saldo: company-scoped connected check', () => {
|
||||
it('one member connects, both members read the snapshot', async () => {
|
||||
const handler = route('GET', '/skattekonto/saldo').handler
|
||||
|
||||
const asA = await handler(req('GET', '/skattekonto/saldo'), makeContext(A, [A_ROW]))
|
||||
expect(asA.status).toBe(200)
|
||||
|
||||
const asB = await handler(req('GET', '/skattekonto/saldo'), makeContext(B, [A_ROW]))
|
||||
expect(asB.status).toBe(200)
|
||||
const body = (await asB.json()) as { data: { saldoSkatteverket: number } | null }
|
||||
expect(body.data?.saldoSkatteverket).toBe(1234.5)
|
||||
})
|
||||
|
||||
it('both members connect, both read (no maybeSingle failure on two rows)', async () => {
|
||||
const handler = route('GET', '/skattekonto/saldo').handler
|
||||
const asA = await handler(req('GET', '/skattekonto/saldo'), makeContext(A, [B_ROW, A_ROW]))
|
||||
const asB = await handler(req('GET', '/skattekonto/saldo'), makeContext(B, [B_ROW, A_ROW]))
|
||||
expect(asA.status).toBe(200)
|
||||
expect(asB.status).toBe(200)
|
||||
})
|
||||
|
||||
it('no member has connected -> 401 NOT_CONNECTED (page empty state)', async () => {
|
||||
const handler = route('GET', '/skattekonto/saldo').handler
|
||||
const res = await handler(req('GET', '/skattekonto/saldo'), makeContext(B, []))
|
||||
expect(res.status).toBe(401)
|
||||
expect(await res.json()).toMatchObject({ code: 'NOT_CONNECTED' })
|
||||
})
|
||||
|
||||
it('a row flagged needs_reconsent still counts as connected (stale snapshot stays visible)', async () => {
|
||||
const handler = route('GET', '/skattekonto/saldo').handler
|
||||
const res = await handler(
|
||||
req('GET', '/skattekonto/saldo'),
|
||||
makeContext(B, [{ ...A_ROW, status: 'needs_reconsent' }]),
|
||||
)
|
||||
expect(res.status).toBe(200)
|
||||
})
|
||||
})
|
||||
|
||||
describe('POST /skattekonto/sync: syncs on the company token, refresh goes to the owner', () => {
|
||||
it('member B triggers a sync on member A\'s token: auth carries A as the token owner', async () => {
|
||||
const ctx = makeContext(B, [A_ROW])
|
||||
const res = await route('POST', '/skattekonto/sync').handler(req('POST', '/skattekonto/sync'), ctx)
|
||||
expect(res.status).toBe(200)
|
||||
expect(mockSyncSkattekonto).toHaveBeenCalledTimes(1)
|
||||
const [passedCtx, auth] = mockSyncSkattekonto.mock.calls[0]
|
||||
expect(passedCtx).toBe(ctx)
|
||||
expect(auth).toEqual({ mode: 'user', supabase: ctx.supabase, userId: A, companyId: COMPANY })
|
||||
})
|
||||
|
||||
it('both connected: each member syncs on their own token', async () => {
|
||||
const asA = makeContext(A, [B_ROW, A_ROW])
|
||||
await route('POST', '/skattekonto/sync').handler(req('POST', '/skattekonto/sync'), asA)
|
||||
expect(mockSyncSkattekonto.mock.calls[0][1]).toMatchObject({ mode: 'user', userId: A })
|
||||
|
||||
const asB = makeContext(B, [B_ROW, A_ROW])
|
||||
await route('POST', '/skattekonto/sync').handler(req('POST', '/skattekonto/sync'), asB)
|
||||
expect(mockSyncSkattekonto.mock.calls[1][1]).toMatchObject({ mode: 'user', userId: B })
|
||||
})
|
||||
|
||||
it('nobody connected -> 401 NOT_CONNECTED without touching SKV', async () => {
|
||||
const res = await route('POST', '/skattekonto/sync').handler(
|
||||
req('POST', '/skattekonto/sync'),
|
||||
makeContext(B, []),
|
||||
)
|
||||
expect(res.status).toBe(401)
|
||||
expect(await res.json()).toMatchObject({ code: 'NOT_CONNECTED' })
|
||||
expect(mockSyncSkattekonto).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('only dead rows -> 401 SESSION_EXPIRED reconnect prompt, no refresh attempt', async () => {
|
||||
const res = await route('POST', '/skattekonto/sync').handler(
|
||||
req('POST', '/skattekonto/sync'),
|
||||
makeContext(B, [{ ...A_ROW, status: 'needs_reconsent' }]),
|
||||
)
|
||||
expect(res.status).toBe(401)
|
||||
expect(await res.json()).toMatchObject({ code: 'SESSION_EXPIRED' })
|
||||
expect(mockSyncSkattekonto).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,87 @@
|
||||
/**
|
||||
* When a sync run on another member's token dies with a terminal auth error,
|
||||
* the connection-expired event must name the token OWNER, not whoever
|
||||
* pressed "Synkronisera nu": only the owner can redo the BankID consent, and
|
||||
* the notification handler keys its dedup on the owner's token row (#1673).
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import { createQueuedMockSupabase } from '@/tests/helpers'
|
||||
import { eventBus } from '@/lib/events/bus'
|
||||
|
||||
const { supabase, enqueue, reset } = createQueuedMockSupabase()
|
||||
|
||||
const getSaldoMock = vi.fn()
|
||||
const getTransaktionerMock = vi.fn()
|
||||
vi.mock('../lib/skattekonto-client', () => ({
|
||||
getSaldo: (...args: unknown[]) => getSaldoMock(...args),
|
||||
getTransaktioner: (...args: unknown[]) => getTransaktionerMock(...args),
|
||||
}))
|
||||
|
||||
vi.mock('../lib/agi-tax-settlement', () => ({
|
||||
settleAgiTaxPayments: vi.fn().mockResolvedValue(undefined),
|
||||
}))
|
||||
|
||||
import { syncSkattekonto } from '../lib/skattekonto-sync'
|
||||
import { SkatteverketAuthError } from '../lib/api-client'
|
||||
import type { ExtensionContext } from '@/lib/extensions/types'
|
||||
import type { SupabaseClient } from '@supabase/supabase-js'
|
||||
|
||||
function makeCtx(userId: string): ExtensionContext {
|
||||
return {
|
||||
supabase,
|
||||
companyId: 'company-1',
|
||||
userId,
|
||||
settings: {
|
||||
get: vi.fn().mockResolvedValue(null),
|
||||
set: vi.fn().mockResolvedValue(undefined),
|
||||
},
|
||||
emit: vi.fn().mockResolvedValue(undefined),
|
||||
} as unknown as ExtensionContext
|
||||
}
|
||||
|
||||
describe('syncSkattekonto: connection.expired names the token owner', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
reset()
|
||||
eventBus.clear()
|
||||
enqueue({ data: { org_number: '556677-8899', entity_type: 'aktiebolag' } }) // company_settings
|
||||
const dead = new SkatteverketAuthError('Sessionen har gått ut. Logga in med BankID igen.', 'SESSION_EXPIRED')
|
||||
getSaldoMock.mockRejectedValue(dead)
|
||||
getTransaktionerMock.mockRejectedValue(dead)
|
||||
})
|
||||
|
||||
it('member B syncs on member A\'s token: the event carries A', async () => {
|
||||
const emitted: Array<{ type: string; payload: Record<string, unknown> }> = []
|
||||
const emitSpy = vi.spyOn(eventBus, 'emit').mockImplementation(async (event) => {
|
||||
emitted.push(event as { type: string; payload: Record<string, unknown> })
|
||||
})
|
||||
|
||||
await expect(
|
||||
syncSkattekonto(makeCtx('user-b'), {
|
||||
mode: 'user',
|
||||
supabase: supabase as unknown as SupabaseClient,
|
||||
userId: 'user-a',
|
||||
companyId: 'company-1',
|
||||
}),
|
||||
).rejects.toBeInstanceOf(SkatteverketAuthError)
|
||||
|
||||
expect(emitted).toHaveLength(1)
|
||||
expect(emitted[0]).toMatchObject({
|
||||
type: 'skattekonto.connection.expired',
|
||||
payload: { reason: 'SESSION_EXPIRED', userId: 'user-a', companyId: 'company-1' },
|
||||
})
|
||||
emitSpy.mockRestore()
|
||||
})
|
||||
|
||||
it('default (own token) keeps the ctx user as recipient', async () => {
|
||||
const emitted: Array<{ type: string; payload: Record<string, unknown> }> = []
|
||||
const emitSpy = vi.spyOn(eventBus, 'emit').mockImplementation(async (event) => {
|
||||
emitted.push(event as { type: string; payload: Record<string, unknown> })
|
||||
})
|
||||
|
||||
await expect(syncSkattekonto(makeCtx('user-a'))).rejects.toBeInstanceOf(SkatteverketAuthError)
|
||||
|
||||
expect(emitted[0]?.payload).toMatchObject({ userId: 'user-a' })
|
||||
emitSpy.mockRestore()
|
||||
})
|
||||
})
|
||||
@@ -171,6 +171,41 @@ async function requireSkvCapability(ctx: ExtensionContext): Promise<NextResponse
|
||||
return requireCapability(ctx.supabase, ctx.companyId, CAPABILITY.skatteverket)
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve auth for a company-scoped READ triggered from the UI: the caller's
|
||||
* own token if they connected, otherwise any active token another member of
|
||||
* the company connected (see resolve-auth.ts, #1673). Membership is the only
|
||||
* gate: ctx.companyId is resolved from the caller's own memberships and the
|
||||
* token table's SELECT policy is company-scoped, so this can never reach
|
||||
* another company's row.
|
||||
*/
|
||||
function resolveCompanyReadAuth(ctx: ExtensionContext) {
|
||||
return resolveReadAuth(ctx.supabase, ctx.companyId, { requires: 'lasombud', userId: ctx.userId })
|
||||
}
|
||||
|
||||
/**
|
||||
* The 401 a read route answers when no usable company token exists.
|
||||
* NOT_CONNECTED keeps the "inte anslutet" empty state (no row at all);
|
||||
* SESSION_EXPIRED mirrors what the token refresh would have thrown for a row
|
||||
* already flagged needs_reconsent, so the UI shows its reconnect prompt
|
||||
* instead of pretending the company never connected.
|
||||
*/
|
||||
function readAuthFailureResponse(reason: 'no_token' | 'needs_reconsent'): NextResponse {
|
||||
if (reason === 'needs_reconsent') {
|
||||
return NextResponse.json(
|
||||
{
|
||||
error: 'Anslutningen mot Skatteverket behöver förnyas. Anslut igen med BankID.',
|
||||
code: 'SESSION_EXPIRED',
|
||||
},
|
||||
{ status: 401 },
|
||||
)
|
||||
}
|
||||
return NextResponse.json(
|
||||
{ error: 'Inte ansluten till Skatteverket.', code: 'NOT_CONNECTED' },
|
||||
{ status: 401 },
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Defense-in-depth RBAC check for AGI write/validate endpoints. Ctx
|
||||
* presence alone (set by middleware) only confirms the user is signed in
|
||||
@@ -1085,17 +1120,13 @@ export const skatteverketExtension: Extension = {
|
||||
const { redovisare, redovisningsperiod } = parseQueryParams(request, ctx)
|
||||
|
||||
// resolveReadAuth: post-signing checks should outlive the user's
|
||||
// 65-minute session when the company has a moms_ombud grant.
|
||||
// 65-minute session when the company has a moms_ombud grant, and
|
||||
// any member may read what another member's token fetches (#1673).
|
||||
const resolved = await resolveReadAuth(ctx.supabase, ctx.companyId, {
|
||||
requires: 'moms_ombud',
|
||||
userId: ctx.userId,
|
||||
})
|
||||
if (!resolved.ok) {
|
||||
return NextResponse.json(
|
||||
{ error: 'Inte ansluten till Skatteverket.', code: 'NOT_CONNECTED' },
|
||||
{ status: 401 }
|
||||
)
|
||||
}
|
||||
if (!resolved.ok) return readAuthFailureResponse(resolved.reason)
|
||||
const response = await skvRequestWithAuth(
|
||||
resolved.auth,
|
||||
'GET',
|
||||
@@ -1146,12 +1177,7 @@ export const skatteverketExtension: Extension = {
|
||||
requires: 'moms_ombud',
|
||||
userId: ctx.userId,
|
||||
})
|
||||
if (!resolved.ok) {
|
||||
return NextResponse.json(
|
||||
{ error: 'Inte ansluten till Skatteverket.', code: 'NOT_CONNECTED' },
|
||||
{ status: 401 }
|
||||
)
|
||||
}
|
||||
if (!resolved.ok) return readAuthFailureResponse(resolved.reason)
|
||||
const response = await skvRequestWithAuth(
|
||||
resolved.auth,
|
||||
'GET',
|
||||
@@ -2063,17 +2089,20 @@ export const skatteverketExtension: Extension = {
|
||||
return NextResponse.json({ error: 'Extension context required' }, { status: 500 })
|
||||
}
|
||||
// The Skattekonto page keys its "inte anslutet" empty state off a 401
|
||||
// NOT_CONNECTED from this route. Connections are per (user, company):
|
||||
// without this check, a company that never connected rendered the
|
||||
// connected-but-unsynced view because the snapshot read below always
|
||||
// answered 200 (with null data), and "Synkronisera nu" then died on a
|
||||
// behorighet error at SKV.
|
||||
const tokens = await getTokens(ctx.supabase, ctx.userId, ctx.companyId)
|
||||
if (!tokens) {
|
||||
return NextResponse.json(
|
||||
{ error: 'Inte ansluten till Skatteverket.', code: 'NOT_CONNECTED' },
|
||||
{ status: 401 },
|
||||
)
|
||||
// NOT_CONNECTED from this route. Without this check, a company that
|
||||
// never connected rendered the connected-but-unsynced view because
|
||||
// the snapshot read below always answered 200 (with null data), and
|
||||
// "Synkronisera nu" then died on a behorighet error at SKV.
|
||||
//
|
||||
// "Connected" means the COMPANY has a token row (any member's) or a
|
||||
// verified system grant, not that the caller personally pressed
|
||||
// Anslut: the snapshot and the transactions belong to the company
|
||||
// (#1673). A row already flagged needs_reconsent still counts as
|
||||
// connected here so the stale snapshot stays visible; the sync route
|
||||
// is what surfaces the reconnect prompt.
|
||||
const resolved = await resolveCompanyReadAuth(ctx)
|
||||
if (!resolved.ok && resolved.reason === 'no_token') {
|
||||
return readAuthFailureResponse('no_token')
|
||||
}
|
||||
const snapshot = await ctx.settings.get<SkattekontoBalanceSnapshot>(SKATTEKONTO_BALANCE_SNAPSHOT_KEY)
|
||||
const lastSyncedAt = await ctx.settings.get<string>(SKATTEKONTO_LAST_SYNCED_AT_KEY)
|
||||
@@ -2165,7 +2194,14 @@ export const skatteverketExtension: Extension = {
|
||||
const blocked = await requireSkvCapability(ctx)
|
||||
if (blocked) return blocked
|
||||
try {
|
||||
const result = await syncSkattekonto(ctx)
|
||||
// Any member may trigger a sync on the company's connection: the
|
||||
// resolved auth carries the token OWNER's userId, so the refresh
|
||||
// writes back to their row, never to the caller's (#1673). A row
|
||||
// flagged needs_reconsent cannot heal on its own, so answer the
|
||||
// reconnect prompt directly instead of burning a refresh call.
|
||||
const resolved = await resolveCompanyReadAuth(ctx)
|
||||
if (!resolved.ok) return readAuthFailureResponse(resolved.reason)
|
||||
const result = await syncSkattekonto(ctx, resolved.auth)
|
||||
return NextResponse.json({ data: result })
|
||||
} catch (err) {
|
||||
return handleSkvError(err)
|
||||
|
||||
@@ -66,11 +66,14 @@ export async function sendConnectionExpiredNotification(
|
||||
if (!email.isConfigured()) return { sent: false, reason: 'email_not_configured' }
|
||||
|
||||
// Episode key: the token row's created_at. No token row means nothing to
|
||||
// reconnect (already disconnected): skip.
|
||||
// reconnect (already disconnected): skip. Rows are per (user, company):
|
||||
// without the company filter a multi-company operator's second row made
|
||||
// maybeSingle() error out and the email was never sent.
|
||||
const { data: token } = await supabase
|
||||
.from('skatteverket_tokens')
|
||||
.select('created_at')
|
||||
.eq('user_id', input.userId)
|
||||
.eq('company_id', input.companyId)
|
||||
.maybeSingle()
|
||||
const tokenCreatedAt = (token as { created_at?: string | null } | null)?.created_at
|
||||
if (!tokenCreatedAt) return { sent: false, reason: 'no_token' }
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { SupabaseClient } from '@supabase/supabase-js'
|
||||
import { createLogger } from '@/lib/logger'
|
||||
import { getSkatteverketEnvironment, type SkvAuth } from './api-client'
|
||||
import { getSystemAuthMode, isSystemAuthConfigured } from './system-auth/config'
|
||||
import {
|
||||
@@ -7,20 +8,35 @@ import {
|
||||
type SkvEnvironment,
|
||||
} from './connection-store'
|
||||
|
||||
const log = createLogger('skatteverket-resolve-auth')
|
||||
|
||||
/**
|
||||
* Auth resolution for background READ paths (skattekonto sync, kvittens
|
||||
* polling, moms inlamnat/beslutat checks).
|
||||
* Auth resolution for READ paths (skattekonto sync, kvittens polling, moms
|
||||
* inlamnat/beslutat checks, the skattekonto page's connected check).
|
||||
*
|
||||
* Preference order:
|
||||
* 1. System credentials, when SKATTEVERKET_SYSTEM_AUTH_MODE=on, the system
|
||||
* flow is configured, and the company's connection row shows the
|
||||
* required behorighet as granted for the current environment.
|
||||
* 2. The company's user token (the pre-hybrid behavior), looked up by
|
||||
* company_id like the kvittens cron always did.
|
||||
* 2. A user token connected to the company. Token rows are per
|
||||
* (user_id, company_id), so a company can hold one row per member who
|
||||
* ran the BankID consent. The caller's own row is preferred when it
|
||||
* exists; otherwise the most recently issued active row for the company
|
||||
* is used. That is what lets member B read data member A connected
|
||||
* (issue #1673): the fetched skattekonto/declaration data belongs to
|
||||
* the company, not to the person who happened to press "Anslut".
|
||||
*
|
||||
* Who may read: any member of the company. Membership is enforced upstream
|
||||
* (the extension dispatcher resolves ctx.companyId from the caller's own
|
||||
* memberships, and the token table's SELECT policy is company-scoped), so no
|
||||
* extra role check is added here. Reads never move the row: refresh writes
|
||||
* go back to the token owner's row (auth.userId is the owner, not the
|
||||
* caller).
|
||||
*
|
||||
* Write paths (moms utkast/las, AGI submit/spara/granskningsunderlag) stay
|
||||
* hard-wired to user mode: the personal flow needs no ombud grant and the
|
||||
* BankID signing step is personal by nature.
|
||||
* hard-wired to user mode with the CALLER's own token: the personal flow
|
||||
* needs no ombud grant and the BankID signing step is personal by nature.
|
||||
* Connect/disconnect likewise only touch the caller's own row.
|
||||
*
|
||||
* Retiring user-token reads later (the full ombud switch) is a policy change
|
||||
* inside this function only.
|
||||
@@ -63,21 +79,18 @@ export async function resolveReadAuth(
|
||||
ok: true,
|
||||
auth: { mode: 'system' },
|
||||
source: 'system',
|
||||
tokenUserId: opts.userId ?? (await lookupTokenUser(supabase, companyId))?.userId ?? null,
|
||||
tokenUserId:
|
||||
opts.userId ?? (await findCompanyTokenUser(supabase, companyId))?.userId ?? null,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (opts.userId) {
|
||||
return {
|
||||
ok: true,
|
||||
auth: { mode: 'user', supabase, userId: opts.userId, companyId },
|
||||
source: 'user',
|
||||
tokenUserId: opts.userId,
|
||||
}
|
||||
}
|
||||
|
||||
const token = await lookupTokenUser(supabase, companyId)
|
||||
// The caller's own token wins when it exists; any other member's active
|
||||
// token serves otherwise. Passing userId no longer short-circuits to "the
|
||||
// caller's row or nothing": a member who never connected used to resolve
|
||||
// to their own missing row and see NOT_CONNECTED for a company that is
|
||||
// connected (#1673).
|
||||
const token = await findCompanyTokenUser(supabase, companyId, { preferUserId: opts.userId })
|
||||
if (!token) return { ok: false, reason: 'no_token' }
|
||||
if (token.needsReconsent) return { ok: false, reason: 'needs_reconsent' }
|
||||
|
||||
@@ -89,21 +102,57 @@ export async function resolveReadAuth(
|
||||
}
|
||||
}
|
||||
|
||||
async function lookupTokenUser(
|
||||
export interface CompanyTokenUser {
|
||||
userId: string
|
||||
needsReconsent: boolean
|
||||
}
|
||||
|
||||
/**
|
||||
* Pick the token row that should serve a read for this company.
|
||||
*
|
||||
* Deterministic order:
|
||||
* 1. active rows before needs_reconsent rows (a dead row must not shadow a
|
||||
* live one connected by another member)
|
||||
* 2. within the same status, the preferred user's own row first
|
||||
* 3. then the most recently issued row (storeTokens is DELETE + INSERT, so
|
||||
* created_at is the last consent or refresh)
|
||||
*
|
||||
* Reads all of the company's rows (one per connected member; a handful at
|
||||
* most) instead of `.maybeSingle()`, which errors on the second row and used
|
||||
* to turn "two members connected" into "nobody connected" for everyone.
|
||||
*
|
||||
* Returns null when the company has no token row at all. The token table's
|
||||
* SELECT policy is company-scoped, so a user-session client only ever sees
|
||||
* rows for companies the caller belongs to.
|
||||
*/
|
||||
export async function findCompanyTokenUser(
|
||||
supabase: SupabaseClient,
|
||||
companyId: string
|
||||
): Promise<{ userId: string; needsReconsent: boolean } | null> {
|
||||
// The token table is user-scoped (one BankID identity per user) but
|
||||
// carries company_id: match on company so a multi-company operator's
|
||||
// token is only used for the company that owns the work.
|
||||
const { data } = await supabase
|
||||
companyId: string,
|
||||
opts: { preferUserId?: string } = {}
|
||||
): Promise<CompanyTokenUser | null> {
|
||||
const { data, error } = await supabase
|
||||
.from('skatteverket_tokens')
|
||||
.select('user_id, status')
|
||||
.select('user_id, status, created_at')
|
||||
.eq('company_id', companyId)
|
||||
.maybeSingle()
|
||||
if (!data?.user_id) return null
|
||||
.order('created_at', { ascending: false })
|
||||
|
||||
if (error) {
|
||||
log.warn('failed to look up company token rows', { companyId, error: error.message })
|
||||
return null
|
||||
}
|
||||
|
||||
const rows = ((data ?? []) as Array<{ user_id: string | null; status: string | null }>).filter(
|
||||
(row): row is { user_id: string; status: string | null } => typeof row.user_id === 'string'
|
||||
)
|
||||
if (rows.length === 0) return null
|
||||
|
||||
const active = rows.filter(row => row.status !== 'needs_reconsent')
|
||||
const pool = active.length > 0 ? active : rows
|
||||
const own = opts.preferUserId ? pool.find(row => row.user_id === opts.preferUserId) : undefined
|
||||
const pick = own ?? pool[0]
|
||||
|
||||
return {
|
||||
userId: data.user_id as string,
|
||||
needsReconsent: data.status === 'needs_reconsent',
|
||||
userId: pick.user_id,
|
||||
needsReconsent: pick.status === 'needs_reconsent',
|
||||
}
|
||||
}
|
||||
|
||||
@@ -120,9 +120,10 @@ function upcomingToRow(companyId: string, tx: SkatteverketUpcomingTransaction):
|
||||
*/
|
||||
export async function syncSkattekonto(
|
||||
ctx: ExtensionContext,
|
||||
// Defaults to the ctx user's personal token: the interactive manual-sync
|
||||
// route keeps its exact pre-hybrid behavior. The cron passes system auth
|
||||
// for companies with a verified lasombud grant.
|
||||
// Defaults to the ctx user's personal token (the post-connect refresh, where
|
||||
// the ctx user just consented). The manual-sync route and the cron pass
|
||||
// resolved auth: system credentials for companies with a verified lasombud
|
||||
// grant, otherwise the token of whichever member connected the company.
|
||||
auth: SkvAuth = { mode: 'user', supabase: ctx.supabase, userId: ctx.userId, companyId: ctx.companyId },
|
||||
): Promise<SkattekontoSyncResult> {
|
||||
const omfragad = await resolveOmfragad(ctx.supabase, ctx.companyId)
|
||||
@@ -141,11 +142,14 @@ export async function syncSkattekonto(
|
||||
err.code === 'SESSION_EXPIRED' ||
|
||||
err.code === 'TOKEN_CORRUPTED'
|
||||
) {
|
||||
// Notify the token OWNER, not whoever triggered the sync: only the
|
||||
// owner can redo the BankID consent, and the notification handler
|
||||
// keys its dedup on the owner's token row.
|
||||
await eventBus.emit({
|
||||
type: 'skattekonto.connection.expired',
|
||||
payload: {
|
||||
reason: err.code,
|
||||
userId: ctx.userId,
|
||||
userId: auth.mode === 'user' ? auth.userId : ctx.userId,
|
||||
companyId: ctx.companyId,
|
||||
},
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user