From 8fd3f112f88717a82103014b413c4a17bc1f20bc Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Wed, 22 Apr 2026 14:47:58 +0200 Subject: [PATCH] fix: surface active TIC companies + block duplicate org numbers (#344) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: surface active TIC companies + block duplicate org numbers Three fixes from live-prod testing: 1. Enrichment filter hid the user's directorships. Now accepts both Completed and PartiallyCompleted status from TIC (tenants without CompanyRoles enabled still get SPAR) and the /select-company role filter no longer requires companyStatus === 'Aktivt' — real TIC payloads have been observed with different values, and positionEnd alone is the authoritative "currently a director" signal. Added PII-free diagnostic logs so the next shape-mismatch is debuggable from Vercel logs without a round trip. 2. Manual wizard silently allowed duplicate org numbers. Added: - findExistingCompanyByOrgNumber helper in actions.ts (service role, bypasses RLS to see cross-tenant rows) - Server-side guard in createCompanyFromOnboarding — returns 'org_number_exists' before the create RPC so we don't leave ghost companies - New /api/company/check-org-number endpoint for debounced client checks - Warning + disabled submit in Step2CompanyDetails - Friendly error toasts in WelcomeOnboarding + BankIdCompanyPicker - Mirror cleaned org_number onto companies.org_number on creation so future duplicate checks and lookups are reliable 3. /onboarding ignored ?org_number= when the picker routed there as a fallback. Now reads searchParams and pre-fills settings; also fixed a latent bug where Step1's entity-type change wiped the pre-fill on *first* selection (it should only reset on a genuine change). Tests: duplicate-org guard (with formatted-input normalization), check-org-number route (auth + 400 + exists true/false + normalization). Co-Authored-By: Claude Opus 4.7 (1M context) * fix: address PR review feedback on duplicate-org guard Greptile P1 findings + swedish-compliance feedback: - findExistingCompanyByOrgNumber now throws on Supabase error instead of silently returning null. Previously a DB outage or RLS misconfiguration would bypass the entire duplicate guard and allow duplicates through. - createCompanyFromOnboarding catches the throw and returns a user-facing error ("Kunde inte verifiera organisationsnummer"), failing closed instead of open. - companies.update({ org_number }) error is now checked and triggers a rollback. Silent failure would leave the company without an org_number, breaking all future duplicate checks for that entity. - New normalizeOrgNumber helper validates 10- or 12-digit input, strips the century prefix for 12-digit personnummer form, and rejects anything else. Malformed input would have corrupted SIE4 (#ORGNR) and SRU (INFO.SRU) exports downstream. - /select-company now uses loose `== null` for positionEnd — TIC has been observed returning `undefined` for open-ended positions, which strict `=== null` would silently filter out. Documented the two downstream isCeased guards so future maintainers don't remove one without the other. - createCompanyFromTicRole refuses to provision when lookup.isCeased (BFL 2 kap — bokföringsskyldighet ends at avregistrering). BankIdCompanyPicker surfaces this client-side too. - WelcomeOnboarding + BankIdCompanyPicker recognise new error codes: org_number_invalid, company_ceased. Tests: +4 cases covering malformed input rejection, fail-closed behaviour on DB error, 12-digit personnummer normalization, and the ceased-company refusal path. Full suite: 2306 passing. Out of scope for this PR (follow-up): - Partial unique index on companies(org_number) WHERE archived_at IS NULL. Closes the race-condition window but needs a migration plus any existing-duplicate cleanup — too risky for this hotfix. - Rate limiting on /api/company/check-org-number. Endpoint is auth-gated so not an immediate concern. Co-Authored-By: Claude Opus 4.7 (1M context) * fix: add Luhn validation and extract org-number normalization Third round of PR review feedback (swedish-compliance): - Add Luhn-10 check-digit validation to normalizeOrgNumber. Rejects structurally invalid org_numbers (wrong check digit) at the boundary instead of letting them propagate into SIE4 #ORGNR and SRU INFO.SRU, where Skatteverket and receiving accounting systems would reject them later anyway. Reuses the existing luhnValidate helper from lib/bankgiro/luhn.ts (Bankgirot 10-modulen — same algorithm applies to both Bolagsverket org numbers and Swedish personnummer). - Extract normalizeOrgNumber into lib/company-lookup/normalize-org-number.ts so the server action and /api/company/check-org-number use the same rule. Previously the API route only stripped hyphens/spaces, so a 12-digit input would miss a stored 10-digit duplicate and mislead the client debounce check ("not a duplicate" → submit → server rejects). - /api/company/check-org-number now returns exists=false for Luhn-invalid input rather than querying the DB. The submit-time server action surfaces org_number_invalid, which is the right place for the error. Test coverage: dedicated normalize-org-number.test.ts (10 cases covering both-lengths, Luhn, whitespace tolerance, garbage). Updated existing tests to use Luhn-valid numbers (real Volvo 5560125790, synthetic personnummer 8001011231). New failing-Luhn test in actions.test.ts. New 12-digit-normalization and luhn-invalid-returns-false tests in route.test.ts. Full suite: 2315 passing. Not fixed (out of scope for this hotfix): - 10↔12 digit round-trip fragility for personnummer born 2000+. This is a codebase-wide architectural choice (see lib/skatteverket/format.ts which uses a two-digit-year heuristic to choose 19/20 at export). Migrating to 12-digit storage is a separate refactor. - Server-side re-fetch of TIC /lookup for isCeased. The trust boundary here is user-to-their-own-onboarding, not adversarial; doubling TIC API cost isn't proportionate. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- app/(onboarding)/onboarding/page.tsx | 22 +- app/(onboarding)/select-company/page.tsx | 16 +- .../check-org-number/__tests__/route.test.ts | 120 ++++++++ app/api/company/check-org-number/route.ts | 55 ++++ components/dashboard/WelcomeOnboarding.tsx | 44 ++- components/onboarding/BankIdCompanyPicker.tsx | 46 +++ components/onboarding/Step2CompanyDetails.tsx | 47 ++- extensions/general/tic/index.ts | 53 +++- .../__tests__/normalize-org-number.test.ts | 43 +++ lib/company-lookup/normalize-org-number.ts | 33 +++ lib/company/__tests__/actions.test.ts | 269 +++++++++++++++++- lib/company/actions.ts | 85 +++++- 12 files changed, 801 insertions(+), 32 deletions(-) create mode 100644 app/api/company/check-org-number/__tests__/route.test.ts create mode 100644 app/api/company/check-org-number/route.ts create mode 100644 lib/company-lookup/__tests__/normalize-org-number.test.ts create mode 100644 lib/company-lookup/normalize-org-number.ts diff --git a/app/(onboarding)/onboarding/page.tsx b/app/(onboarding)/onboarding/page.tsx index c0cc42d5..d50ab6fc 100644 --- a/app/(onboarding)/onboarding/page.tsx +++ b/app/(onboarding)/onboarding/page.tsx @@ -4,7 +4,11 @@ import WelcomeOnboarding from '@/components/dashboard/WelcomeOnboarding' export const dynamic = 'force-dynamic' -export default async function OnboardingPage() { +export default async function OnboardingPage({ + searchParams, +}: { + searchParams: Promise<{ org_number?: string }> +}) { const supabase = await createClient() const { data: { user } } = await supabase.auth.getUser() @@ -42,5 +46,19 @@ export default async function OnboardingPage() { const firstName = profile?.full_name?.split(' ')[0] || null - return + // The BankID picker routes here with ?org_number=… when TIC /lookup fails + // or the entity type isn't one-click-provisionable. Strip formatting so + // whatever Step2 displays matches what the rest of the flow will store. + const { org_number: rawOrgNumber } = await searchParams + const initialOrgNumber = rawOrgNumber ? rawOrgNumber.replace(/[\s-]/g, '') : undefined + + return ( + + ) } diff --git a/app/(onboarding)/select-company/page.tsx b/app/(onboarding)/select-company/page.tsx index 8dbe50b0..a6e46e50 100644 --- a/app/(onboarding)/select-company/page.tsx +++ b/app/(onboarding)/select-company/page.tsx @@ -105,8 +105,22 @@ export default async function SelectCompanyPage() { companyRoles?: EnrichmentCompanyRole[] } | null + // "Currently a director" = no position end date. We deliberately do NOT + // also require companyStatus === 'Aktivt': real TIC payloads have been + // observed with other values (locale/tenant variants), and filtering too + // strictly silently hides the user's real directorships. + // + // Ceased/struck-off companies would still render here, but two later + // guards block provisioning: + // 1. BankIdCompanyPicker calls TIC /lookup before provisioning and + // short-circuits with a toast when isCeased=true. + // 2. createCompanyFromTicRole refuses to provision when lookup.isCeased. + // Both guards are required — don't remove one without removing both. + // + // Loose `== null` on purpose: TIC payloads have been observed returning + // `undefined` for open-ended positions, which `=== null` would miss. const activeRoles = (enrichmentValue?.companyRoles ?? []).filter( - (r) => r.companyStatus === 'Aktivt' && r.positionEnd === null, + (r) => r.positionEnd == null, ) // Drop TIC roles that already appear in the user's gnubok memberships — diff --git a/app/api/company/check-org-number/__tests__/route.test.ts b/app/api/company/check-org-number/__tests__/route.test.ts new file mode 100644 index 00000000..e6cbfc7d --- /dev/null +++ b/app/api/company/check-org-number/__tests__/route.test.ts @@ -0,0 +1,120 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { createMockRequest, parseJsonResponse } from '@/tests/helpers' + +vi.mock('@/lib/supabase/server', () => ({ + createClient: vi.fn(), + createServiceClient: vi.fn(), +})) + +import { createClient, createServiceClient } from '@/lib/supabase/server' +import { GET } from '../route' + +const mockCreateClient = vi.mocked(createClient) +const mockCreateServiceClient = vi.mocked(createServiceClient) + +function mockAuth(user: { id: string } | null) { + mockCreateClient.mockResolvedValue({ + auth: { getUser: vi.fn().mockResolvedValue({ data: { user } }) }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any) +} + +/** + * Service-role mock. Returns a match only if the incoming `.eq('org_number', X)` + * value matches `existing`. Anything else (or empty `existing`) returns null. + */ +function mockService(existing?: string) { + let lastOrgNumber: string | null = null + const chain: Record = {} + const methods = ['select', 'eq', 'is', 'limit', 'maybeSingle'] + for (const m of methods) { + chain[m] = (...args: unknown[]) => { + if (m === 'eq' && args[0] === 'org_number') { + lastOrgNumber = String(args[1]) + } + if (m === 'maybeSingle') { + return Promise.resolve({ + data: existing && lastOrgNumber === existing ? { id: 'other' } : null, + error: null, + }) + } + return chain + } + } + mockCreateServiceClient.mockReturnValue({ + from: vi.fn().mockReturnValue(chain), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any) +} + +beforeEach(() => { + vi.clearAllMocks() +}) + +describe('GET /api/company/check-org-number', () => { + it('returns 401 when unauthenticated', async () => { + mockAuth(null) + mockService() + const req = createMockRequest('/api/company/check-org-number?org_number=5560125790') + const { status } = await parseJsonResponse(await GET(req)) + expect(status).toBe(401) + }) + + it('returns 400 when org_number is missing', async () => { + mockAuth({ id: 'user-1' }) + mockService() + const req = createMockRequest('/api/company/check-org-number') + const { status } = await parseJsonResponse(await GET(req)) + expect(status).toBe(400) + }) + + it('returns exists=false when the org number is not registered', async () => { + mockAuth({ id: 'user-1' }) + mockService(undefined) // no existing match + const req = createMockRequest('/api/company/check-org-number?org_number=5560125790') + const { status, body } = await parseJsonResponse(await GET(req)) + expect(status).toBe(200) + expect((body as { data: { exists: boolean } }).data.exists).toBe(false) + }) + + it('returns exists=true when the org number is already registered', async () => { + mockAuth({ id: 'user-1' }) + mockService('5560125790') + const req = createMockRequest('/api/company/check-org-number?org_number=5560125790') + const { status, body } = await parseJsonResponse(await GET(req)) + expect(status).toBe(200) + expect((body as { data: { exists: boolean } }).data.exists).toBe(true) + }) + + it('normalizes formatted org numbers before lookup (strips hyphens/spaces)', async () => { + mockAuth({ id: 'user-1' }) + mockService('5560125790') + const req = createMockRequest('/api/company/check-org-number?org_number=556012-5790') + const { status, body } = await parseJsonResponse(await GET(req)) + expect(status).toBe(200) + expect((body as { data: { exists: boolean } }).data.exists).toBe(true) + }) + + it('normalizes 12-digit input to 10-digit canonical before lookup', async () => { + // Stored form is 10-digit canonical (8001011231); user types 12-digit + // personnummer with century prefix. + mockAuth({ id: 'user-1' }) + mockService('8001011231') + const req = createMockRequest('/api/company/check-org-number?org_number=198001011231') + const { status, body } = await parseJsonResponse(await GET(req)) + expect(status).toBe(200) + expect((body as { data: { exists: boolean } }).data.exists).toBe(true) + }) + + it('returns exists=false for Luhn-invalid input (not a duplicate of anything)', async () => { + // The submit-time server action will reject this as org_number_invalid; + // here we just confirm the check endpoint doesn't produce a misleading + // "exists=true" result by accidentally matching an invalid number. + mockAuth({ id: 'user-1' }) + mockService('5560125790') // a real registered number + const req = createMockRequest('/api/company/check-org-number?org_number=5560125791') + const { status, body } = await parseJsonResponse(await GET(req)) + expect(status).toBe(200) + expect((body as { data: { exists: boolean } }).data.exists).toBe(false) + }) +}) diff --git a/app/api/company/check-org-number/route.ts b/app/api/company/check-org-number/route.ts new file mode 100644 index 00000000..4419b561 --- /dev/null +++ b/app/api/company/check-org-number/route.ts @@ -0,0 +1,55 @@ +import { NextResponse } from 'next/server' +import { createClient, createServiceClient } from '@/lib/supabase/server' +import { normalizeOrgNumber } from '@/lib/company-lookup/normalize-org-number' + +/** + * GET /api/company/check-org-number?org_number=XXXXXXXXXX + * + * Returns `{ data: { exists: boolean } }` indicating whether the given + * organisation number is already registered in any non-archived gnubok + * company. Used by the onboarding wizard to warn users before they try to + * create a duplicate. + * + * Normalizes the input with the same rule as the server action + * (`normalizeOrgNumber`) so that a 12-digit form typed in the UI still + * matches a 10-digit stored canonical. Returns `exists: false` for + * malformed input — the submit-time server action will reject it with + * `org_number_invalid`, which is the right place to surface the error. + * + * Requires authentication so the endpoint can't be used to enumerate the + * full set of org numbers on the platform. Uses the service role internally + * because RLS hides rows the caller isn't a member of — which is exactly + * what we need to detect ("owned by someone else"). + */ +export async function GET(request: Request) { + const supabase = await createClient() + const { data: { user } } = await supabase.auth.getUser() + if (!user) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + + const url = new URL(request.url) + const raw = url.searchParams.get('org_number') ?? '' + if (!raw) { + return NextResponse.json({ error: 'org_number is required' }, { status: 400 }) + } + + const canonical = normalizeOrgNumber(raw) + if (!canonical) { + // Invalid format/Luhn — not a duplicate of anything by definition. + return NextResponse.json({ data: { exists: false } }) + } + + const service = createServiceClient() + const { data, error } = await service + .from('companies') + .select('id') + .eq('org_number', canonical) + .is('archived_at', null) + .limit(1) + .maybeSingle() + + if (error) { + return NextResponse.json({ error: error.message }, { status: 500 }) + } + + return NextResponse.json({ data: { exists: !!data } }) +} diff --git a/components/dashboard/WelcomeOnboarding.tsx b/components/dashboard/WelcomeOnboarding.tsx index 4ef175e2..3a93e3ec 100644 --- a/components/dashboard/WelcomeOnboarding.tsx +++ b/components/dashboard/WelcomeOnboarding.tsx @@ -48,9 +48,17 @@ interface WelcomeOnboardingProps { teamId: string skipWelcome?: boolean hasExistingCompanies?: boolean + /** Pre-fill Step 2 org_number when the picker routed here via ?org_number=. */ + initialOrgNumber?: string } -export default function WelcomeOnboarding({ firstName, teamId, skipWelcome, hasExistingCompanies }: WelcomeOnboardingProps) { +export default function WelcomeOnboarding({ + firstName, + teamId, + skipWelcome, + hasExistingCompanies, + initialOrgNumber, +}: WelcomeOnboardingProps) { const router = useRouter() const { toast } = useToast() const supabase = createClient() @@ -59,7 +67,9 @@ export default function WelcomeOnboarding({ firstName, teamId, skipWelcome, hasE const [isLoading, setIsLoading] = useState(true) const [isSaving, setIsSaving] = useState(false) const [currentStep, setCurrentStep] = useState(1) - const [settings, setSettings] = useState>({}) + const [settings, setSettings] = useState>( + initialOrgNumber ? { org_number: initialOrgNumber } : {}, + ) const ticEnabled = ENABLED_EXTENSION_IDS.has('tic') const [ticLookup, setTicLookup] = useState(null) @@ -109,7 +119,15 @@ export default function WelcomeOnboarding({ firstName, teamId, skipWelcome, hasE }, [supabase, router]) const handleNext = async (stepData: Partial) => { - if (currentStep === 1 && stepData.entity_type && stepData.entity_type !== settings.entity_type) { + // Reset org_number/company_name only on a genuine change (user going back + // and picking a different entity type). First-time selection must not + // wipe a pre-fill (e.g. ?org_number= deep-link from /select-company). + if ( + currentStep === 1 && + stepData.entity_type && + settings.entity_type && + stepData.entity_type !== settings.entity_type + ) { stepData = { ...stepData, org_number: '', company_name: '' } setTicLookup(null) } @@ -163,11 +181,27 @@ export default function WelcomeOnboarding({ firstName, teamId, skipWelcome, hasE if (result.error || !result.companyId) { logError('create company action failed', { error: result.error }) + let title = 'Fel' + let description: string = result.error || 'Kunde inte skapa företag. Försök igen.' + let backToStep2 = false + if (result.error === 'org_number_exists') { + title = 'Företaget finns redan' + description = 'Det här företaget finns redan i gnubok. Be en befintlig administratör att bjuda in dig.' + backToStep2 = true + } else if (result.error === 'org_number_invalid') { + title = 'Ogiltigt organisationsnummer' + description = 'Kontrollera att du angett ett giltigt 10- eller 12-siffrigt organisationsnummer.' + backToStep2 = true + } toast({ - title: 'Fel', - description: result.error || 'Kunde inte skapa företag. Försök igen.', + title, + description, variant: 'destructive', }) + // Back user up to step 2 so they can correct the org number. + if (backToStep2) { + setCurrentStep(2) + } return } diff --git a/components/onboarding/BankIdCompanyPicker.tsx b/components/onboarding/BankIdCompanyPicker.tsx index ea83e2ca..fccae3f2 100644 --- a/components/onboarding/BankIdCompanyPicker.tsx +++ b/components/onboarding/BankIdCompanyPicker.tsx @@ -132,6 +132,18 @@ export default function BankIdCompanyPicker({ return } + // Block provisioning for companies that are avregistrerade/likviderade. + // Under BFL 2 kap, bokföringsskyldighet ends when a company is struck off. + if (lookup.isCeased) { + toast({ + title: 'Företaget är avregistrerat', + description: 'Det går inte att sätta upp bokföring för ett avregistrerat företag.', + variant: 'destructive', + }) + setSetup({ kind: 'idle' }) + return + } + setSetup({ kind: 'creating', orgNumber, step: 'provision' }) startTransition(async () => { @@ -151,6 +163,40 @@ export default function BankIdCompanyPicker({ return } + if (result.error === 'org_number_exists') { + toast({ + title: 'Företaget finns redan', + description: 'Be en befintlig administratör att bjuda in dig.', + variant: 'destructive', + }) + setSetup({ kind: 'idle' }) + return + } + + if (result.error === 'company_ceased') { + // Belt-and-suspenders: we already check lookup.isCeased client-side + // above, but the server-side guard catches any race where TIC's + // cached result differs between the two calls. + toast({ + title: 'Företaget är avregistrerat', + description: 'Det går inte att sätta upp bokföring för ett avregistrerat företag.', + variant: 'destructive', + }) + setSetup({ kind: 'idle' }) + return + } + + if (result.error === 'org_number_invalid') { + toast({ + title: 'Ogiltigt organisationsnummer', + description: 'Fortsätt med manuell uppsättning.', + variant: 'destructive', + }) + setSetup({ kind: 'idle' }) + router.push(`/onboarding?org_number=${encodeURIComponent(orgNumber)}`) + return + } + if (result.error || !result.companyId) { toast({ title: 'Kunde inte skapa företag', diff --git a/components/onboarding/Step2CompanyDetails.tsx b/components/onboarding/Step2CompanyDetails.tsx index 5dbdde5a..cb36f89c 100644 --- a/components/onboarding/Step2CompanyDetails.tsx +++ b/components/onboarding/Step2CompanyDetails.tsx @@ -68,10 +68,43 @@ export default function Step2CompanyDetails({ const [isLooking, setIsLooking] = useState(false) const [lookupError, setLookupError] = useState(null) const [lookupDone, setLookupDone] = useState(null) + const [orgNumberExists, setOrgNumberExists] = useState(false) const abortRef = useRef(null) + const dupAbortRef = useRef(null) const orgNumber = watch('org_number') + // Debounced duplicate check against gnubok's own companies table. Runs in + // parallel with the TIC lookup — they don't conflict. On match, the submit + // button is disabled; the server action would also reject ('org_number_exists') + // but blocking client-side avoids a wasted roundtrip. + useEffect(() => { + if (!orgNumber || !ORG_NUMBER_REGEX.test(orgNumber)) { + setOrgNumberExists(false) + return + } + const timer = setTimeout(() => { + dupAbortRef.current?.abort() + const controller = new AbortController() + dupAbortRef.current = controller + fetch(`/api/company/check-org-number?org_number=${encodeURIComponent(orgNumber)}`, { + signal: controller.signal, + }) + .then(async (res) => { + if (controller.signal.aborted || !res.ok) return + const { data } = await res.json() + setOrgNumberExists(!!data?.exists) + }) + .catch(() => { + // Network failure is non-fatal — the server action will re-check. + }) + }, 500) + return () => { + clearTimeout(timer) + dupAbortRef.current?.abort() + } + }, [orgNumber]) + useEffect(() => { if (!ticEnabled || !orgNumber || !ORG_NUMBER_REGEX.test(orgNumber)) { return @@ -201,6 +234,14 @@ export default function Step2CompanyDetails({ {ticEnabled && lookupError && (

{lookupError}

)} + {orgNumberExists && ( +
+ + + Det här företaget finns redan i gnubok. Be en befintlig administratör att bjuda in dig. + +
+ )}
@@ -262,7 +303,11 @@ export default function Step2CompanyDetails({ Tillbaka -