Enforce MFA on critical mutation routes + post-audit foundation (A1) (#646)
* feat(lib): add canonical money + format + fetch primitives (audit Tier 0) Foundation for post-audit cleanup: shared primitives so subsequent refactors import one helper instead of reinventing (the duplication the audit found). - lib/money.ts: canonical roundOre/ORE_TOLERANCE (+ equalOre/isZeroOre/sumOre); lib/bokslut/rounding.ts re-exports for back-compat - lib/utils.ts: formatAmount, formatWholeKr, formatDateTime - lib/hooks/use-fetch.ts: generic client fetch hook (abort, bilingual errors, refetch) - components/common/DataState.tsx: loading/error/empty wrapper over Skeleton/EmptyState - messages: common.retry / common.load_error (sv+en) - tests: 16 tests incl. the 1.005 half-ore case and locale-robust format assertions Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci(guards): ratchet against new MFA-bypassing routes and naive ore-rounding Adds scripts/checks/no-new-antipatterns.mjs + committed baseline. Fails CI only when a PR ADDS a route hand-rolling supabase.auth.getUser() (which skips MFA AAL2 enforcement) or a new Math.round(x*100)/100. Baseline: 178 raw-auth routes, 668 naive rounds — ratchets down as the A1 (route-auth) and D1 (rounding) migrations land. Wired into core-build.yml; green at baseline. Note: scripts/ is gitignored (.gitignore:70 '/scripts') yet tracks 39 files via force-add; these two were force-added to match that existing pattern. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(api,errors): enforce MFA on journal-entry mutation routes via withRouteContext (A1) Migrates the 4 journal-entry mutation routes (commit, correct, reverse, recordate) off hand-rolled supabase.auth.getUser() onto withRouteContext, which enforces MFA AAL2 (requireAuth) + non-viewer role (requireWrite) and routes thrown errors through the canonical errorResponse envelope. Fixes audit finding A1 for the most compliance-critical mutations and folds in C8 for these routes (drops bookkeepingErrorResponse; they now emit message_en). Also fixes a latent bug: errorResponse()/extractBookkeepingDetails only handled 11 of 15 typed bookkeeping errors, so MeaninglessCorrection / NoOpenPeriodForDate / TargetPeriodClosed / TargetPeriodLocked silently degraded to a generic 500 (affecting existing v1 callers too). Adds the 4 missing registry codes + extract cases -> correct 400/409. Behavior change: untyped engine throws now return the canonical 500 envelope instead of 400+raw-string; typed errors keep their status (verified against the registry). Tests updated to the realistic typed-error contract + a 403 write-gate test on commit. Updates .claude/rules/api-routes.md to prescribe withRouteContext. Ratchets the antipattern guard 178 -> 174. Full unit suite green (5023); tsc: no new errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(api): enforce MFA on salary run authorization routes via withRouteContext (A1) Migrates the salary-run lifecycle write routes (approve, paid, revert) — the highest-PII A1 surface — off hand-rolled supabase.auth.getUser() onto withRouteContext (enforces MFA AAL2 + non-viewer role). Explicit { error } returns are preserved unchanged (passed through the wrapper); only auth changes, so no error-shape regression. Salary unit suite green (8). Ratchets the antipattern guard 174 -> 171. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * review: address PR #646 bot findings - guard: match withRouteContext/requireAuth at the CALL site (withRouteContext[<(]), not a bare import — closes the false-negative greptile flagged. It surfaced app/api/sandbox/seed (hand-rolled getUser; the loose regex had matched a code comment). Switched that route to requireAuth() — the documented stopgap for routes that can't use withRouteContext (it runs before a company exists; anonymous users, so MFA is a no-op but the auth path is now consistent). Guard stays at 171. - money.test: add the negative half-ore case roundOre(-1.005) === -1 to lock the rounding direction against regressions. - use-fetch: document keep-previous-data + deferred-loading (effect-tick) semantics. - structured-errors: drop the BFL 5 kap. 5 § citation from MEANINGLESS_CORRECTION per the swedish-compliance bot (5 § governs correction procedure, not the no-op precondition). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * review: enrich wrapper error logging + document sandbox GDPR controls (PR #646) - with-route-context: log unhandled errors and route errorResponse through the resolved { userId, companyId } logger, not just { requestId, operation } — closes the OWASP V16 audit-trail finding for all 82+ routes using the wrapper. Documented in the JSDoc. - sandbox/seed: document the GDPR Art.32 compensating controls for the anonymous write path (anonymous-only, /24 rate limit, synthetic demo data, own-company RLS scope). No functional change — the flagged behaviour is pre-existing by design; this records the reasoning inline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
c74b19df1b
commit
0b86901a2b
@@ -1,10 +1,12 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import { NextResponse } from 'next/server'
|
||||
import {
|
||||
createMockRequest,
|
||||
parseJsonResponse,
|
||||
createMockRouteParams,
|
||||
makeJournalEntry,
|
||||
} from '@/tests/helpers'
|
||||
import { JournalEntryNotBalancedError } from '@/lib/bookkeeping/errors'
|
||||
|
||||
const mockCreateClient = vi.fn()
|
||||
vi.mock('@/lib/supabase/server', () => ({
|
||||
@@ -20,8 +22,9 @@ vi.mock('@/lib/company/context', () => ({
|
||||
getActiveCompanyId: vi.fn().mockResolvedValue('company-1'),
|
||||
}))
|
||||
|
||||
const requireWriteMock = vi.fn()
|
||||
vi.mock('@/lib/auth/require-write', () => ({
|
||||
requireWritePermission: vi.fn().mockResolvedValue({ ok: true }),
|
||||
requireWritePermission: (...args: unknown[]) => requireWriteMock(...args),
|
||||
}))
|
||||
|
||||
const mockCommitEntry = vi.fn()
|
||||
@@ -48,6 +51,7 @@ describe('POST /api/bookkeeping/journal-entries/[id]/commit', () => {
|
||||
mockCreateClient.mockResolvedValue({
|
||||
auth: { getUser: vi.fn().mockResolvedValue({ data: { user: mockUser } }) },
|
||||
})
|
||||
requireWriteMock.mockResolvedValue({ ok: true })
|
||||
})
|
||||
|
||||
it('returns 401 when not authenticated', async () => {
|
||||
@@ -65,6 +69,25 @@ describe('POST /api/bookkeeping/journal-entries/[id]/commit', () => {
|
||||
expect(body).toEqual({ error: 'Unauthorized' })
|
||||
})
|
||||
|
||||
it('returns 403 when the caller lacks write permission (role/MFA write gate)', async () => {
|
||||
requireWriteMock.mockResolvedValue({
|
||||
ok: false,
|
||||
response: NextResponse.json(
|
||||
{ error: 'Du har endast läsbehörighet i detta företag.' },
|
||||
{ status: 403 },
|
||||
),
|
||||
})
|
||||
|
||||
const request = createMockRequest('/api/bookkeeping/journal-entries/entry-1/commit', {
|
||||
method: 'POST',
|
||||
})
|
||||
const response = await POST(request, createMockRouteParams({ id: 'entry-1' }))
|
||||
const { status } = await parseJsonResponse(response)
|
||||
|
||||
expect(status).toBe(403)
|
||||
expect(mockCommitEntry).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('returns posted entry on success', async () => {
|
||||
const postedEntry = makeJournalEntry({
|
||||
id: 'entry-1',
|
||||
@@ -91,16 +114,18 @@ describe('POST /api/bookkeeping/journal-entries/[id]/commit', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('returns 400 when engine throws', async () => {
|
||||
mockCommitEntry.mockRejectedValue(new Error('Entry not balanced'))
|
||||
it('maps a typed engine error to the canonical structured envelope', async () => {
|
||||
// commitEntry throws typed bookkeeping errors; the wrapper routes them
|
||||
// through errorResponse() → registry status + { error: { code, ... } }.
|
||||
mockCommitEntry.mockRejectedValue(new JournalEntryNotBalancedError(1000, 900))
|
||||
|
||||
const request = createMockRequest('/api/bookkeeping/journal-entries/entry-1/commit', {
|
||||
method: 'POST',
|
||||
})
|
||||
const response = await POST(request, createMockRouteParams({ id: 'entry-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: string }>(response)
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string; message_en?: string } }>(response)
|
||||
|
||||
expect(status).toBe(400)
|
||||
expect(body.error).toBe('Entry not balanced')
|
||||
expect(body.error.code).toBe('JOURNAL_ENTRY_NOT_BALANCED')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,49 +1,16 @@
|
||||
import { createClient } from '@/lib/supabase/server'
|
||||
import { NextResponse } from 'next/server'
|
||||
import { commitEntry } from '@/lib/bookkeeping/engine'
|
||||
import { bookkeepingErrorResponse } from '@/lib/bookkeeping/errors'
|
||||
import { ensureInitialized } from '@/lib/init'
|
||||
import { requireCompanyId } from '@/lib/company/context'
|
||||
import { requireWritePermission } from '@/lib/auth/require-write'
|
||||
import { createLogger } from '@/lib/logger'
|
||||
import { withRouteContext } from '@/lib/api/with-route-context'
|
||||
|
||||
ensureInitialized()
|
||||
|
||||
const log = createLogger('api.bookkeeping.commit')
|
||||
|
||||
export async function POST(
|
||||
request: Request,
|
||||
{ params }: { params: Promise<{ id: string }> }
|
||||
) {
|
||||
const { id } = await params
|
||||
const supabase = await createClient()
|
||||
const { data: { user } } = await supabase.auth.getUser()
|
||||
|
||||
if (!user) {
|
||||
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
|
||||
}
|
||||
|
||||
const writeCheck = await requireWritePermission(supabase, user.id)
|
||||
if (!writeCheck.ok) return writeCheck.response
|
||||
|
||||
const companyId = await requireCompanyId(supabase, user.id)
|
||||
|
||||
try {
|
||||
export const POST = withRouteContext<{ params: Promise<{ id: string }> }>(
|
||||
'bookkeeping.journal-entry.commit',
|
||||
async (_request, { supabase, companyId, user }, { params }) => {
|
||||
const { id } = await params
|
||||
const posted = await commitEntry(supabase, companyId, user.id, id, 'user_accept')
|
||||
return NextResponse.json({ data: posted })
|
||||
} catch (err) {
|
||||
const typed = bookkeepingErrorResponse(err)
|
||||
if (typed) return typed
|
||||
// Untyped error path: engine logging didn't fire, so log here.
|
||||
log.error('commit endpoint failed (untyped)', err as Error, {
|
||||
companyId,
|
||||
userId: user.id,
|
||||
entityType: 'journal_entry',
|
||||
entityId: id,
|
||||
})
|
||||
return NextResponse.json(
|
||||
{ error: err instanceof Error ? err.message : 'Failed to commit entry' },
|
||||
{ status: 400 }
|
||||
)
|
||||
}
|
||||
}
|
||||
},
|
||||
{ requireWrite: true },
|
||||
)
|
||||
|
||||
@@ -5,6 +5,10 @@ import {
|
||||
createMockRouteParams,
|
||||
makeJournalEntry,
|
||||
} from '@/tests/helpers'
|
||||
import {
|
||||
JournalEntryNotBalancedError,
|
||||
CannotCorrectNonPostedError,
|
||||
} from '@/lib/bookkeeping/errors'
|
||||
|
||||
const mockCreateClient = vi.fn()
|
||||
vi.mock('@/lib/supabase/server', () => ({
|
||||
@@ -112,10 +116,8 @@ describe('POST /api/bookkeeping/journal-entries/[id]/correct', () => {
|
||||
expect(mockCorrectEntry).toHaveBeenCalledWith(expect.anything(), 'company-1', 'user-1', 'entry-1', lines)
|
||||
})
|
||||
|
||||
it('returns 400 when correctEntry throws for unbalanced lines', async () => {
|
||||
mockCorrectEntry.mockRejectedValue(
|
||||
new Error('Corrected entry is not balanced: debits (1000) != credits (500)')
|
||||
)
|
||||
it('maps an unbalanced-correction engine error to the canonical envelope (400)', async () => {
|
||||
mockCorrectEntry.mockRejectedValue(new JournalEntryNotBalancedError(1000, 500, 'correction'))
|
||||
|
||||
const lines = [
|
||||
{ account_number: '1930', debit_amount: 1000, credit_amount: 0 },
|
||||
@@ -127,14 +129,14 @@ describe('POST /api/bookkeeping/journal-entries/[id]/correct', () => {
|
||||
body: { lines },
|
||||
})
|
||||
const response = await POST(request, createMockRouteParams({ id: 'entry-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: string }>(response)
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(response)
|
||||
|
||||
expect(status).toBe(400)
|
||||
expect(body.error).toContain('not balanced')
|
||||
expect(body.error.code).toBe('JOURNAL_ENTRY_NOT_BALANCED')
|
||||
})
|
||||
|
||||
it('returns 400 when entry is not found or not posted', async () => {
|
||||
mockCorrectEntry.mockRejectedValue(new Error('Can only correct posted entries'))
|
||||
it('maps a not-posted engine error to the canonical envelope (400)', async () => {
|
||||
mockCorrectEntry.mockRejectedValue(new CannotCorrectNonPostedError('draft'))
|
||||
|
||||
const lines = [
|
||||
{ account_number: '1930', debit_amount: 1000, credit_amount: 0 },
|
||||
@@ -146,9 +148,9 @@ describe('POST /api/bookkeeping/journal-entries/[id]/correct', () => {
|
||||
body: { lines },
|
||||
})
|
||||
const response = await POST(request, createMockRouteParams({ id: 'entry-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: string }>(response)
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(response)
|
||||
|
||||
expect(status).toBe(400)
|
||||
expect(body.error).toBe('Can only correct posted entries')
|
||||
expect(body.error.code).toBe('CANNOT_CORRECT_NON_POSTED')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,45 +1,20 @@
|
||||
import { createClient } from '@/lib/supabase/server'
|
||||
import { NextResponse } from 'next/server'
|
||||
import { correctEntry } from '@/lib/core/bookkeeping/storno-service'
|
||||
import { bookkeepingErrorResponse } from '@/lib/bookkeeping/errors'
|
||||
import { ensureInitialized } from '@/lib/init'
|
||||
import { validateBody } from '@/lib/api/validate'
|
||||
import { CorrectJournalEntrySchema } from '@/lib/api/schemas'
|
||||
import { requireCompanyId } from '@/lib/company/context'
|
||||
import { requireWritePermission } from '@/lib/auth/require-write'
|
||||
import { withRouteContext } from '@/lib/api/with-route-context'
|
||||
|
||||
ensureInitialized()
|
||||
|
||||
export async function POST(
|
||||
request: Request,
|
||||
{ params }: { params: Promise<{ id: string }> }
|
||||
) {
|
||||
const { id } = await params
|
||||
const supabase = await createClient()
|
||||
const { data: { user } } = await supabase.auth.getUser()
|
||||
|
||||
if (!user) {
|
||||
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
|
||||
}
|
||||
|
||||
const writeCheck = await requireWritePermission(supabase, user.id)
|
||||
if (!writeCheck.ok) return writeCheck.response
|
||||
|
||||
const companyId = await requireCompanyId(supabase, user.id)
|
||||
|
||||
const validation = await validateBody(request, CorrectJournalEntrySchema)
|
||||
if (!validation.success) return validation.response
|
||||
const body = validation.data
|
||||
|
||||
try {
|
||||
const result = await correctEntry(supabase, companyId, user.id, id, body.lines)
|
||||
export const POST = withRouteContext<{ params: Promise<{ id: string }> }>(
|
||||
'bookkeeping.journal-entry.correct',
|
||||
async (request, { supabase, companyId, user }, { params }) => {
|
||||
const { id } = await params
|
||||
const validation = await validateBody(request, CorrectJournalEntrySchema)
|
||||
if (!validation.success) return validation.response
|
||||
const result = await correctEntry(supabase, companyId, user.id, id, validation.data.lines)
|
||||
return NextResponse.json({ data: result })
|
||||
} catch (err) {
|
||||
const typed = bookkeepingErrorResponse(err)
|
||||
if (typed) return typed
|
||||
return NextResponse.json(
|
||||
{ error: err instanceof Error ? err.message : 'Failed to correct entry' },
|
||||
{ status: 400 }
|
||||
)
|
||||
}
|
||||
}
|
||||
},
|
||||
{ requireWrite: true },
|
||||
)
|
||||
|
||||
@@ -1,47 +1,20 @@
|
||||
import { createClient } from '@/lib/supabase/server'
|
||||
import { NextResponse } from 'next/server'
|
||||
import { recordateEntry } from '@/lib/core/bookkeeping/storno-service'
|
||||
import { bookkeepingErrorResponse } from '@/lib/bookkeeping/errors'
|
||||
import { ensureInitialized } from '@/lib/init'
|
||||
import { validateBody } from '@/lib/api/validate'
|
||||
import { RecordateJournalEntrySchema } from '@/lib/api/schemas'
|
||||
import { requireCompanyId } from '@/lib/company/context'
|
||||
import { requireWritePermission } from '@/lib/auth/require-write'
|
||||
import { withRouteContext } from '@/lib/api/with-route-context'
|
||||
|
||||
ensureInitialized()
|
||||
|
||||
export async function POST(
|
||||
request: Request,
|
||||
{ params }: { params: Promise<{ id: string }> }
|
||||
) {
|
||||
const { id } = await params
|
||||
const supabase = await createClient()
|
||||
const { data: { user } } = await supabase.auth.getUser()
|
||||
|
||||
if (!user) {
|
||||
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
|
||||
}
|
||||
|
||||
const writeCheck = await requireWritePermission(supabase, user.id)
|
||||
if (!writeCheck.ok) return writeCheck.response
|
||||
|
||||
const companyId = await requireCompanyId(supabase, user.id)
|
||||
|
||||
const validation = await validateBody(request, RecordateJournalEntrySchema)
|
||||
if (!validation.success) return validation.response
|
||||
const body = validation.data
|
||||
|
||||
try {
|
||||
const result = await recordateEntry(supabase, companyId, user.id, id, body.new_entry_date)
|
||||
export const POST = withRouteContext<{ params: Promise<{ id: string }> }>(
|
||||
'bookkeeping.journal-entry.recordate',
|
||||
async (request, { supabase, companyId, user }, { params }) => {
|
||||
const { id } = await params
|
||||
const validation = await validateBody(request, RecordateJournalEntrySchema)
|
||||
if (!validation.success) return validation.response
|
||||
const result = await recordateEntry(supabase, companyId, user.id, id, validation.data.new_entry_date)
|
||||
return NextResponse.json({ data: result })
|
||||
} catch (err) {
|
||||
const typed = bookkeepingErrorResponse(err)
|
||||
if (typed) return typed
|
||||
// Not a recognized domain error — an unexpected server fault, not a client
|
||||
// error, so surface it as 500.
|
||||
return NextResponse.json(
|
||||
{ error: err instanceof Error ? err.message : 'Failed to move entry' },
|
||||
{ status: 500 }
|
||||
)
|
||||
}
|
||||
}
|
||||
},
|
||||
{ requireWrite: true },
|
||||
)
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
createMockRouteParams,
|
||||
makeJournalEntry,
|
||||
} from '@/tests/helpers'
|
||||
import { EntryAlreadyReversedError } from '@/lib/bookkeeping/errors'
|
||||
|
||||
// Mock dependencies before imports
|
||||
const mockCreateClient = vi.fn()
|
||||
@@ -76,16 +77,18 @@ describe('POST /api/bookkeeping/journal-entries/[id]/reverse', () => {
|
||||
expect(mockReverseEntry).toHaveBeenCalledWith(expect.anything(), 'company-1', 'user-1', 'entry-1')
|
||||
})
|
||||
|
||||
it('returns 400 when engine throws', async () => {
|
||||
mockReverseEntry.mockRejectedValue(new Error('Entry already reversed'))
|
||||
it('maps a typed concurrent-reversal error to the canonical envelope (409)', async () => {
|
||||
// reverseEntry throws the typed error on a concurrent storno; the wrapper
|
||||
// routes it through errorResponse() → 409 + { error: { code, ... } }.
|
||||
mockReverseEntry.mockRejectedValue(new EntryAlreadyReversedError())
|
||||
|
||||
const request = createMockRequest('/api/bookkeeping/journal-entries/entry-1/reverse', {
|
||||
method: 'POST',
|
||||
})
|
||||
const response = await POST(request, createMockRouteParams({ id: 'entry-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: string }>(response)
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(response)
|
||||
|
||||
expect(status).toBe(400)
|
||||
expect(body.error).toBe('Entry already reversed')
|
||||
expect(status).toBe(409)
|
||||
expect(body.error.code).toBe('ENTRY_ALREADY_REVERSED')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,39 +1,16 @@
|
||||
import { createClient } from '@/lib/supabase/server'
|
||||
import { NextResponse } from 'next/server'
|
||||
import { reverseEntry } from '@/lib/bookkeeping/engine'
|
||||
import { bookkeepingErrorResponse } from '@/lib/bookkeeping/errors'
|
||||
import { ensureInitialized } from '@/lib/init'
|
||||
import { requireCompanyId } from '@/lib/company/context'
|
||||
import { requireWritePermission } from '@/lib/auth/require-write'
|
||||
import { withRouteContext } from '@/lib/api/with-route-context'
|
||||
|
||||
ensureInitialized()
|
||||
|
||||
export async function POST(
|
||||
request: Request,
|
||||
{ params }: { params: Promise<{ id: string }> }
|
||||
) {
|
||||
const { id } = await params
|
||||
const supabase = await createClient()
|
||||
const { data: { user } } = await supabase.auth.getUser()
|
||||
|
||||
if (!user) {
|
||||
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
|
||||
}
|
||||
|
||||
const writeCheck = await requireWritePermission(supabase, user.id)
|
||||
if (!writeCheck.ok) return writeCheck.response
|
||||
|
||||
const companyId = await requireCompanyId(supabase, user.id)
|
||||
|
||||
try {
|
||||
export const POST = withRouteContext<{ params: Promise<{ id: string }> }>(
|
||||
'bookkeeping.journal-entry.reverse',
|
||||
async (_request, { supabase, companyId, user }, { params }) => {
|
||||
const { id } = await params
|
||||
const reversalEntry = await reverseEntry(supabase, companyId, user.id, id)
|
||||
return NextResponse.json({ data: reversalEntry })
|
||||
} catch (err) {
|
||||
const typed = bookkeepingErrorResponse(err)
|
||||
if (typed) return typed
|
||||
return NextResponse.json(
|
||||
{ error: err instanceof Error ? err.message : 'Failed to reverse entry' },
|
||||
{ status: 400 }
|
||||
)
|
||||
}
|
||||
}
|
||||
},
|
||||
{ requireWrite: true },
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user