diff --git a/DECISIONS.md b/DECISIONS.md index ea82ee2a..50c300c1 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1636,6 +1636,7 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-09-06] Bundled SKV ROT/RUT payout books ONE voucher (one 1513 leg per begäran) and the set is suggested at read time with no hint column: one bank row = one verifikat (match-batch precedent) and a uuid[] hint would need six clear paths and go stale; N vouchers + the 1:N reconciliation split was rejected because its half-failure state has no UI exit, and begäran, not the invoice, is the unit under fakturamodellen. [2026-09-06] Utlägg via lön settles claims with an idempotent RPC after the salary verifikat is posted (pre-checked before posting), not with a trigger on salary_runs -> booked: a raise inside that trigger after the entries exist would leave a paid run with posted verifikat and a retry would double-post; the RPC path fails to "booked, claims still open, re-runnable". [2026-09-06] A privately paid supplier invoice is booked through registerExpenseClaim (verifikat + expense_claims row, source_type expense_claim) with the invoice's kontering as custom lines, and a person-paid inbox document goes to the core route with inbox_item_id instead of the extension's convert endpoint: the form's switch, the second entry generator and the convert bypass were three write paths for one fact, so one writer wins over adding a claims insert beside the old generator (the issue's shape) or copying the branch into the convert handler. +[2026-09-07] WooCommerce activation gate: the wc-auth callback now only STAGES verified keys on the pending row; activation additionally requires browser_confirmed_at from the session-bound return leg, enforced by a DB CHECK (20260907143000) rather than an application guard, so 'active implies keys plus a recorded confirmation' holds by construction and a connection can never go live headless. Skeptic review established the limit of that guarantee and it is recorded here on purpose: the second signal comes from the INITIATING user, so it does not stop a store admin from approving a link someone else generated (wc-auth delivers keys server-to-server and identifies no approver); that needs a proof of store control from the initiator, tracked as a follow-up. Rejected the proposed separate pending-attempt table and handshake token: the pending row and oauth_state already are that state. Expiry: 15-minute TTL on the return leg plus a sweep at the start of the nightly orders cron; NOT on the callback leg, because WooCommerce treats any non-200 as a failed handshake (deletes the key, store-side error page, no redirect) and would strand slow approvals. Prod audit before the migration found no active row still carrying a state; one hand-inserted demo row (active, no keys, sync off) is parked as error by the migration because it could never sync. Returning the browser to the initiating brand origin is a separate change. [2026-09-07] Björn Lundén connect: Lundify activation redirect reuses the OAuth state row (provider_otc) and the callback, with the User-Key arriving as publicKey and the state as extra; the manual User-Key field stays as a folded fallback: BL issued the activation key on 2026-09-07, the redirect removes the GUID copy-paste that failed every real customer, and reusing the atomic state consumption plus initiator binding keeps the same tenant boundary as Fortnox/Visma instead of a second callback with its own checks. BL's ON/OFF backend callback and settings URL are not built: they need a registered URL at BL and an orgNr-to-company mapping, filed as follow-up. [2026-09-07] Auth-link hosts resolve against the brands table only; NEXT_PUBLIC_WHITELABEL_DOMAINS removed (supersedes 2026-08-18): the env var was a second copy of brands.domain compiled into the browser, so every brand needed four registrations (row, env var, GoTrue allowlist, redeploy) and two partners shipped with it stale (canonical-branded reset mails). Password reset moved to POST /api/auth/password-reset so the server resolves the host; invite, email change and signup share the same resolver, which also trusts this deployment's own VERCEL_URL/VERCEL_BRANCH_URL so previews keep working. A drift check between the copies was rejected: it would be a fifth thing to maintain. GoTrue's redirect allowlist stays as the backstop; hosted carries the wildcards https://*.accounted.se/auth/callback** and https://*.accounted.se/invite/** there (config, not code; GoTrue matches the full URL with query, and * stops at . and /) so only bring-your-own-domain partners need a manual entry. A failed brands lookup refuses with 503 (BrandLookupFailedError) instead of a canonical fallback: a canonical link is a wrong-brand mail for a white-label user, which is the bug this replaces. [2026-09-07] Draft stamp moved to the page margin (absolute + fixed) instead of the reporter's position:fixed corner badge: the 40pt top margin is the only place that is guaranteed empty on every page, and the stamp must not overlap the header title on the right. diff --git a/app/api/extensions/woocommerce/__tests__/callback.test.ts b/app/api/extensions/woocommerce/__tests__/callback.test.ts index 90b46d3e..41a95126 100644 --- a/app/api/extensions/woocommerce/__tests__/callback.test.ts +++ b/app/api/extensions/woocommerce/__tests__/callback.test.ts @@ -31,6 +31,30 @@ function makeCallbackRequest(body: unknown): Request { }) } +const PENDING_ROW = { + id: 'conn-1', + company_id: 'company-1', + user_id: 'user-1', + store_url: 'https://shop.example.se', + created_at: new Date().toISOString(), + consumer_key_encrypted: null as string | null, +} + +const STORE_INFO = { + name: 'Testbutiken', + currency: 'SEK', + prices_include_tax: true, + wc_version: '9.9.5', +} + +function mockServiceClient() { + const queued = createQueuedMockSupabase() + vi.mocked(createServiceClient).mockResolvedValue( + queued.supabase as unknown as Awaited>, + ) + return queued +} + const VALID_BODY = { key_id: 1, user_id: STATE, @@ -86,18 +110,8 @@ describe('POST /api/extensions/woocommerce/callback', () => { }) it('marks the row error and returns 502 when the credential probe fails', async () => { - const { supabase, enqueue, findCall } = createQueuedMockSupabase() - vi.mocked(createServiceClient).mockResolvedValue( - supabase as unknown as Awaited>, - ) - enqueue({ - data: { - id: 'conn-1', - company_id: 'company-1', - user_id: 'user-1', - store_url: 'https://shop.example.se', - }, - }) + const { enqueue, findCall } = mockServiceClient() + enqueue({ data: PENDING_ROW }) enqueue({ data: null }) // markError update vi.mocked(testConnectionAndFetchStoreInfo).mockRejectedValue(new Error('403')) @@ -107,7 +121,12 @@ describe('POST /api/extensions/woocommerce/callback', () => { string, unknown > - expect(errorUpdate.status).toBe('error') + expect(errorUpdate).toMatchObject({ + status: 'error', + oauth_state: null, + consumer_key_encrypted: null, + consumer_secret_encrypted: null, + }) // The probe ran against the STORED store_url, not anything the caller sent. expect(vi.mocked(testConnectionAndFetchStoreInfo).mock.calls[0][0]).toMatchObject({ storeUrl: 'https://shop.example.se', @@ -115,54 +134,169 @@ describe('POST /api/extensions/woocommerce/callback', () => { }) }) - it('encrypts the keys, activates the row and emits the audit event', async () => { - const { supabase, enqueue, findCalls } = createQueuedMockSupabase() - vi.mocked(createServiceClient).mockResolvedValue( - supabase as unknown as Awaited>, - ) - enqueue({ - data: { - id: 'conn-1', - company_id: 'company-1', - user_id: 'user-1', - store_url: 'https://shop.example.se', - }, - }) - enqueue({ - data: { - id: 'conn-1', - company_id: 'company-1', - user_id: 'user-1', - store_url: 'https://shop.example.se', - }, - }) // activation update - vi.mocked(testConnectionAndFetchStoreInfo).mockResolvedValue({ - name: 'Testbutiken', - currency: 'SEK', - prices_include_tax: true, - wc_version: '9.9.5', - }) + it('stages the verified keys but leaves the row pending until the browser confirms', async () => { + const { enqueue, findCalls, calls } = mockServiceClient() + enqueue({ data: PENDING_ROW }) + enqueue({ data: { id: 'conn-1' } }) // stage update + enqueue({ data: null }) // activateIfComplete: browser signal missing, zero rows + vi.mocked(testConnectionAndFetchStoreInfo).mockResolvedValue(STORE_INFO) const res = await POST(makeCallbackRequest(VALID_BODY)) expect(res.status).toBe(200) + expect(await res.json()).toEqual({ success: true, activated: false }) const updates = findCalls('woocommerce_connections', 'update') - const activation = updates[0][0] as Record - expect(activation.status).toBe('active') - expect(activation.transaction_sync_enabled).toBe(true) - // The state survives activation on purpose: this POST has no browser - // session, so the browser leg (../return) binds the completion to the - // initiator by looking the row up with this same state and consumes it - // there. Replay is blocked by the status 'pending' scope instead. - expect(activation).not.toHaveProperty('oauth_state') - expect(activation.store_name).toBe('Testbutiken') + expect(updates).toHaveLength(2) + const staged = updates[0][0] as Record + // Keys and store info are staged; nothing here says 'active' or turns + // the feed on, and the state stays so the return leg can find the row. + expect(staged).not.toHaveProperty('status') + expect(staged).not.toHaveProperty('transaction_sync_enabled') + expect(staged).not.toHaveProperty('oauth_state') + expect(staged.store_name).toBe('Testbutiken') // Secrets never stored in plaintext, and they decrypt back. - expect(String(activation.consumer_key_encrypted)).not.toContain('ck_new') - expect(decryptCredential(String(activation.consumer_key_encrypted))).toBe('ck_new') - expect(decryptCredential(String(activation.consumer_secret_encrypted))).toBe('cs_new') + expect(String(staged.consumer_key_encrypted)).not.toContain('ck_new') + expect(decryptCredential(String(staged.consumer_key_encrypted))).toBe('ck_new') + expect(decryptCredential(String(staged.consumer_secret_encrypted))).toBe('cs_new') + // The activation attempt is the conditional one: pending + both signals. + const activation = updates[1][0] as Record + expect(activation.status).toBe('active') + const notCalls = calls.filter((c) => c.method === 'not').map((c) => c.args) + expect(notCalls).toContainEqual(['consumer_key_encrypted', 'is', null]) + expect(notCalls).toContainEqual(['consumer_secret_encrypted', 'is', null]) + expect(notCalls).toContainEqual(['browser_confirmed_at', 'is', null]) + + expect(eventBus.emit).not.toHaveBeenCalled() + }) + + it('activates and emits when the browser confirmed before the keys arrived', async () => { + const { enqueue, findCalls } = mockServiceClient() + enqueue({ data: PENDING_ROW }) + enqueue({ data: { id: 'conn-1' } }) // stage update + enqueue({ + data: { + id: 'conn-1', + company_id: 'company-1', + user_id: 'user-1', + store_url: 'https://shop.example.se', + }, + }) // activateIfComplete: both signals present + vi.mocked(testConnectionAndFetchStoreInfo).mockResolvedValue(STORE_INFO) + + const res = await POST(makeCallbackRequest(VALID_BODY)) + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ success: true, activated: true }) + + const updates = findCalls('woocommerce_connections', 'update') + const activation = updates[1][0] as Record + expect(activation).toMatchObject({ + status: 'active', + transaction_sync_enabled: true, + oauth_state: null, + }) + expect(eventBus.emit).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'woocommerce.connected', + payload: expect.objectContaining({ connectionId: 'conn-1', companyId: 'company-1' }), + }), + ) + }) + + it('answers a duplicate POST for a state that already holds keys with 200, never re-probes, but still runs the flip', async () => { + const { enqueue, findCalls } = mockServiceClient() + enqueue({ data: { ...PENDING_ROW, consumer_key_encrypted: 'enc:already' } }) + enqueue({ data: null }) // activateIfComplete: browser not confirmed yet + + const res = await POST(makeCallbackRequest({ ...VALID_BODY, consumer_key: 'ck_forged' })) + // 200, not 4xx: WooCommerce deletes its key and shows a store-side error + // on any non-200, and a distinct status would tell whoever holds the + // state whether the merchant has approved yet. + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ success: true, activated: false }) + expect(testConnectionAndFetchStoreInfo).not.toHaveBeenCalled() + // The only write is the conditional activation: the forged keys in the + // body never reach the row. + const updates = findCalls('woocommerce_connections', 'update') + expect(updates).toHaveLength(1) + expect(updates[0][0]).toMatchObject({ status: 'active' }) + expect(updates[0][0]).not.toHaveProperty('consumer_key_encrypted') + }) + + it('completes a callback that was cut off between staging and activating when WooCommerce replays it', async () => { + const { enqueue } = mockServiceClient() + enqueue({ data: { ...PENDING_ROW, consumer_key_encrypted: 'enc:already' } }) + enqueue({ + data: { + id: 'conn-1', + company_id: 'company-1', + user_id: 'user-1', + store_url: 'https://shop.example.se', + }, + }) // activateIfComplete: browser had confirmed in the meantime + + const res = await POST(makeCallbackRequest(VALID_BODY)) + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ success: true, activated: true }) + expect(testConnectionAndFetchStoreInfo).not.toHaveBeenCalled() expect(eventBus.emit).toHaveBeenCalledWith( expect.objectContaining({ type: 'woocommerce.connected' }), ) }) + + it('stages keys for a slow approval past the handshake TTL instead of refusing (WooCommerce wp_dies on non-200)', async () => { + const { enqueue, findCalls, calls } = mockServiceClient() + enqueue({ + data: { ...PENDING_ROW, created_at: new Date(Date.now() - 45 * 60_000).toISOString() }, + }) + enqueue({ data: { id: 'conn-1' } }) // stage update + enqueue({ data: null }) // activateIfComplete: browser not confirmed + vi.mocked(testConnectionAndFetchStoreInfo).mockResolvedValue(STORE_INFO) + + const res = await POST(makeCallbackRequest(VALID_BODY)) + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ success: true, activated: false }) + const updates = findCalls('woocommerce_connections', 'update') + expect(updates).toHaveLength(2) + expect(updates[0][0]).not.toHaveProperty('status') + // Even if the initiator confirmed early, the flip carries the TTL, so a + // stale row cannot become active from this leg. + const gte = calls.filter((c) => c.method === 'gte').map((c) => c.args) + expect(gte).toHaveLength(1) + expect(gte[0][0]).toBe('created_at') + }) + + it('parks the row and returns 409 when activation hits the one-active-per-store index', async () => { + const { enqueue, findCalls } = mockServiceClient() + enqueue({ data: PENDING_ROW }) + enqueue({ data: { id: 'conn-1' } }) // stage update + enqueue({ data: null, error: { code: '23505', message: 'duplicate key' } }) + enqueue({ data: null }) // markError update + vi.mocked(testConnectionAndFetchStoreInfo).mockResolvedValue(STORE_INFO) + + const res = await POST(makeCallbackRequest(VALID_BODY)) + expect(res.status).toBe(409) + const updates = findCalls('woocommerce_connections', 'update') + expect(updates).toHaveLength(3) + // Parking wipes the staged secrets AND the store metadata the probe read. + expect(updates[2][0]).toMatchObject({ + status: 'error', + consumer_key_encrypted: null, + consumer_secret_encrypted: null, + store_name: null, + currency: null, + }) + expect(eventBus.emit).not.toHaveBeenCalled() + }) + + it('returns 404 when the row left pending between lookup and staging', async () => { + const { enqueue } = mockServiceClient() + enqueue({ data: PENDING_ROW }) + enqueue({ data: null }) // stage update matched zero rows + vi.mocked(testConnectionAndFetchStoreInfo).mockResolvedValue(STORE_INFO) + + const res = await POST(makeCallbackRequest(VALID_BODY)) + expect(res.status).toBe(404) + expect(eventBus.emit).not.toHaveBeenCalled() + }) }) diff --git a/app/api/extensions/woocommerce/__tests__/return.test.ts b/app/api/extensions/woocommerce/__tests__/return.test.ts index 28f87956..8f93c46c 100644 --- a/app/api/extensions/woocommerce/__tests__/return.test.ts +++ b/app/api/extensions/woocommerce/__tests__/return.test.ts @@ -4,11 +4,14 @@ vi.mock('@/lib/supabase/server', () => ({ createServiceClient: vi.fn(), createClient: vi.fn(), })) +vi.mock('@/lib/init', () => ({ ensureInitialized: vi.fn() })) +vi.mock('@/lib/events/bus', () => ({ eventBus: { emit: vi.fn() } })) vi.mock('@/lib/extensions/loader', () => ({ loadExtensions: vi.fn() })) vi.mock('@/lib/extensions/registry', () => ({ extensionRegistry: { get: vi.fn() } })) import { GET } from '../return/route' import { createServiceClient, createClient } from '@/lib/supabase/server' +import { eventBus } from '@/lib/events/bus' import { extensionRegistry } from '@/lib/extensions/registry' import { createQueuedMockSupabase } from '@/tests/helpers' @@ -38,12 +41,20 @@ function mockSession(userId: string | null) { return getUser } -const ROW = (status: 'pending' | 'active') => ({ +const ROW = (status: 'pending' | 'active', ageMs = 0) => ({ id: 'conn-1', user_id: 'user-1', status, + created_at: new Date(Date.now() - ageMs).toISOString(), }) +const ACTIVATED = { + id: 'conn-1', + company_id: 'company-1', + user_id: 'user-1', + store_url: 'https://shop.example.se', +} + describe('GET /api/extensions/woocommerce/return', () => { beforeEach(() => { vi.clearAllMocks() @@ -73,12 +84,25 @@ describe('GET /api/extensions/woocommerce/return', () => { `${BASE}/import?mode=woocommerce&woocommerce_error=denied`, ) const update = findCall('woocommerce_connections', 'update')?.[0] as Record - expect(update).toMatchObject({ status: 'error', oauth_state: null }) + // The callback may already have staged keys AND the store's metadata for + // this state (WooCommerce re-serves the approval page); a denial takes + // all of it back. + expect(update).toMatchObject({ + status: 'error', + oauth_state: null, + consumer_key_encrypted: null, + consumer_secret_encrypted: null, + store_name: null, + currency: null, + prices_include_tax: null, + wc_version: null, + key_permissions: null, + }) expect(supabase.from).toHaveBeenCalledTimes(1) }) describe('approved leg (success=1)', () => { - it('hands an active row over to its initiator and consumes the state', async () => { + it('hands a row the pre-gate callback activated over to its initiator and consumes the state', async () => { const { enqueue, findCalls } = mockServiceClient() const getUser = mockSession('user-1') enqueue({ data: ROW('active') }) // lookup by oauth_state @@ -94,18 +118,102 @@ describe('GET /api/extensions/woocommerce/return', () => { expect(updates[0][0]).toEqual({ oauth_state: null }) }) - it('leaves a still-pending row untouched for the initiator (the callback still needs the state)', async () => { - const { enqueue, findCalls } = mockServiceClient() - mockSession('user-1') - enqueue({ data: ROW('pending') }) + it('records the initiator confirmation and activates a pending row whose keys are staged', async () => { + const { enqueue, findCalls, calls } = mockServiceClient() + const getUser = mockSession('user-1') + enqueue({ data: ROW('pending') }) // lookup by oauth_state + enqueue({ data: null }) // browser_confirmed_at update + enqueue({ data: ACTIVATED }) // activateIfComplete: keys already staged const res = await GET(makeReturnRequest({ success: '1', user_id: STATE })) expect(res.headers.get('location')).toBe(CONNECTED) - expect(findCalls('woocommerce_connections', 'update')).toHaveLength(0) + expect(getUser).toHaveBeenCalledTimes(1) + const updates = findCalls('woocommerce_connections', 'update') + expect(updates).toHaveLength(2) + expect(Object.keys(updates[0][0] as object)).toEqual(['browser_confirmed_at']) + expect(updates[1][0]).toMatchObject({ + status: 'active', + transaction_sync_enabled: true, + oauth_state: null, + }) + // Activation is conditional on both signals, scoped to this pending row. + const notCalls = calls.filter((c) => c.method === 'not').map((c) => c.args) + expect(notCalls).toContainEqual(['consumer_key_encrypted', 'is', null]) + expect(notCalls).toContainEqual(['browser_confirmed_at', 'is', null]) + expect(eventBus.emit).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'woocommerce.connected', + payload: expect.objectContaining({ connectionId: 'conn-1', companyId: 'company-1' }), + }), + ) }) - it('revokes an already-activated row when a different user completes the handshake', async () => { + it('records the confirmation and waits (no connected toast) when the keys have not landed yet', async () => { + const { enqueue, findCalls } = mockServiceClient() + mockSession('user-1') + enqueue({ data: ROW('pending') }) + enqueue({ data: null }) // browser_confirmed_at update + enqueue({ data: null }) // activateIfComplete: no keys yet, zero rows + + const res = await GET(makeReturnRequest({ success: '1', user_id: STATE })) + + expect(res.headers.get('location')).toBe(`${BASE}/import?mode=woocommerce`) + const updates = findCalls('woocommerce_connections', 'update') + expect(updates).toHaveLength(2) + expect(Object.keys(updates[0][0] as object)).toEqual(['browser_confirmed_at']) + // The state is left on the row: the late callback still needs it. + expect(updates[0][0]).not.toHaveProperty('oauth_state') + expect(eventBus.emit).not.toHaveBeenCalled() + }) + + it('parks an expired pending row with its keys wiped instead of confirming it', async () => { + const { enqueue, findCalls } = mockServiceClient() + mockSession('user-1') + enqueue({ data: ROW('pending', 16 * 60_000) }) + enqueue({ data: null }) // expire update + + const res = await GET(makeReturnRequest({ success: '1', user_id: STATE })) + + expect(res.headers.get('location')).toBe( + `${BASE}/import?mode=woocommerce&woocommerce_error=expired`, + ) + const updates = findCalls('woocommerce_connections', 'update') + expect(updates).toHaveLength(1) + expect(updates[0][0]).toMatchObject({ + status: 'error', + oauth_state: null, + consumer_key_encrypted: null, + consumer_secret_encrypted: null, + // The store's name and settings the callback staged go too. + store_name: null, + currency: null, + prices_include_tax: null, + wc_version: null, + }) + expect(eventBus.emit).not.toHaveBeenCalled() + }) + + it('parks the row and reports a conflict when the store is already actively connected', async () => { + const { enqueue, findCalls } = mockServiceClient() + mockSession('user-1') + enqueue({ data: ROW('pending') }) + enqueue({ data: null }) // browser_confirmed_at update + enqueue({ data: null, error: { code: '23505', message: 'duplicate key' } }) + enqueue({ data: null }) // park update + + const res = await GET(makeReturnRequest({ success: '1', user_id: STATE })) + + expect(res.headers.get('location')).toBe( + `${BASE}/import?mode=woocommerce&woocommerce_error=conflict`, + ) + const updates = findCalls('woocommerce_connections', 'update') + expect(updates).toHaveLength(3) + expect(updates[2][0]).toMatchObject({ status: 'error', consumer_key_encrypted: null }) + expect(eventBus.emit).not.toHaveBeenCalled() + }) + + it('revokes a row the pre-gate callback already activated when a different user completes the handshake', async () => { const { enqueue, findCalls, calls } = mockServiceClient() mockSession('user-2') enqueue({ data: ROW('active') }) @@ -135,7 +243,7 @@ describe('GET /api/extensions/woocommerce/return', () => { expect(eqCalls).toContainEqual(['id', 'conn-1']) }) - it('closes a still-pending row when a different user completes it, so the late callback cannot activate it', async () => { + it('closes a still-pending row when a different user completes it, so the late callback cannot stage or activate it', async () => { const { enqueue, findCalls } = mockServiceClient() mockSession('user-2') enqueue({ data: ROW('pending') }) @@ -148,7 +256,14 @@ describe('GET /api/extensions/woocommerce/return', () => { ) const updates = findCalls('woocommerce_connections', 'update') expect(updates).toHaveLength(1) - expect(updates[0][0]).toMatchObject({ status: 'error', oauth_state: null }) + expect(updates[0][0]).toMatchObject({ + status: 'error', + oauth_state: null, + consumer_key_encrypted: null, + consumer_secret_encrypted: null, + }) + expect(updates[0][0]).not.toHaveProperty('browser_confirmed_at') + expect(eventBus.emit).not.toHaveBeenCalled() }) it('sends an anonymous browser to /login with the return URL preserved and touches nothing', async () => { diff --git a/app/api/extensions/woocommerce/callback/route.ts b/app/api/extensions/woocommerce/callback/route.ts index 52ee7828..a3dda054 100644 --- a/app/api/extensions/woocommerce/callback/route.ts +++ b/app/api/extensions/woocommerce/callback/route.ts @@ -11,6 +11,7 @@ import { isWooCommerceConfigured, } from '@/extensions/general/woocommerce/lib/credentials' import { testConnectionAndFetchStoreInfo } from '@/extensions/general/woocommerce/lib/api-client' +import { activateIfComplete } from '@/extensions/general/woocommerce/lib/connect' // This route emits woocommerce.connected (audit trail). ensureInitialized() // must run at module load so the event_log handler has subscribed before the @@ -32,6 +33,19 @@ export const maxDuration = 60 * unauthenticated: the single-use oauth_state riding in user_id locates the * pending row, and the received keys are verified against that row's stored * store_url before anything is persisted. + * + * This leg has no browser session, so it can only STAGE the verified keys on + * the pending row. The row becomes active when the initiator's session has + * confirmed on the return leg as well (activateIfComplete, either order). + * A pending row never syncs, so keys staged for a handshake nobody confirms + * are inert until the nightly sweep wipes them. + * + * No TTL refusal here on purpose: WooCommerce treats any non-200 as a + * failed handshake (deletes the key it just minted and shows the merchant an + * error page on the store, no redirect back), so refusing a slow approval + * here would strand a legitimate merchant. The TTL lives in the activation + * predicate and on the session-bound return leg, and the nightly sweep parks + * what is left; a stale pending row cannot flip and never syncs. */ export async function POST(request: Request) { loadExtensions() @@ -81,7 +95,7 @@ export async function POST(request: Request) { const { data: pending, error: findError } = await supabase .from('woocommerce_connections') - .select('id, company_id, user_id, store_url') + .select('id, company_id, user_id, store_url, consumer_key_encrypted') .eq('oauth_state', state) .eq('status', 'pending') .single() @@ -97,10 +111,34 @@ export async function POST(request: Request) { const markError = (message: string) => supabase .from('woocommerce_connections') - .update({ status: 'error', error_message: message, oauth_state: null }) + .update({ + status: 'error', + error_message: message, + oauth_state: null, + consumer_key_encrypted: null, + consumer_secret_encrypted: null, + store_name: null, + currency: null, + prices_include_tax: null, + wc_version: null, + key_permissions: null, + }) .eq('id', pending.id) .eq('status', 'pending') + // WooCommerce posts once per approval. A second POST for the same state is + // a replay of something we already hold: never re-probe or overwrite staged + // keys on its say-so. It still runs the activation flip, so a callback that + // was cut off between staging and activating is completed by its retry, and + // it answers 200 either way so the status code does not double as an "has + // the merchant approved yet" oracle for whoever holds the state. + if (pending.consumer_key_encrypted) { + log.warn('duplicate handshake callback for a state that already holds keys', { + connectionId: pending.id, + }) + return finishActivation(supabase, pending.id, markError) + } + // Authenticity check: the keys must actually work against the store URL the // user asked to connect. A forged callback with someone else's (or made-up) // keys fails here and never gets stored. @@ -120,7 +158,11 @@ export async function POST(request: Request) { return NextResponse.json({ error: 'Credential verification failed' }, { status: 502 }) } - const { data: activated, error: updateError } = await supabase + // Stage the verified keys. Status stays pending: this POST has no browser + // session, so the initiator binding happens on the return leg, and the row + // only flips to active once that leg has confirmed too. oauth_state stays + // until activation so the return leg can still find the row. + const { data: staged, error: stageError } = await supabase .from('woocommerce_connections') .update({ consumer_key_encrypted: encryptCredential(consumerKey), @@ -130,32 +172,55 @@ export async function POST(request: Request) { currency: storeInfo.currency, prices_include_tax: storeInfo.prices_include_tax, wc_version: storeInfo.wc_version, - status: 'active', - connected_at: new Date().toISOString(), error_message: null, - // oauth_state is deliberately KEPT here. This POST is server-to-server - // (the store calls it, no browser session), so the initiator check has - // to happen on the browser leg (../return), which locates the row by - // this same state and consumes it there. Replay is still blocked: both - // the lookup above and this update are scoped to status 'pending', so - // an active row can never be activated again. - // Feed-only product: connecting the store means fetching its orders, so - // the nightly feed starts on by default; the panel toggle is the opt-out. - transaction_sync_enabled: true, }) .eq('id', pending.id) .eq('status', 'pending') - .select('id, company_id, user_id, store_url') - .single() + .select('id') + .maybeSingle() - if (updateError || !activated) { + if (stageError || !staged) { + // The row left 'pending' between lookup and here (superseded, denied on + // the return leg, or expired by the sweep): nothing to stage against. + log.error('failed to stage verified credentials', { + connectionId: pending.id, + code: stageError?.code, + message: stageError?.message, + }) + return NextResponse.json( + { error: stageError ? 'Staging failed' : 'Unknown or expired state' }, + { status: stageError ? 500 : 404 }, + ) + } + + return finishActivation(supabase, pending.id, markError) +} + +/** + * Run the conditional flip for a row whose keys are staged and answer + * WooCommerce. Shared by the first callback and by a replayed one. + */ +async function finishActivation( + supabase: Awaited>, + connectionId: string, + markError: (message: string) => PromiseLike, +): Promise { + const activation = await activateIfComplete(supabase, connectionId) + + if (activation.outcome === 'incomplete') { + // Browser has not confirmed yet (the usual order: WooCommerce posts here + // before it redirects the merchant). The return leg finishes the job. + return NextResponse.json({ success: true, activated: false }) + } + + if (activation.outcome !== 'activated') { // 23505 = a partial unique index: the store is already actively connected // (to this or another company), or the company connected in a parallel tab. - const isConflict = updateError?.code === '23505' + const isConflict = activation.outcome === 'conflict' log.error('failed to activate connection', { - connectionId: pending.id, - code: updateError?.code, - message: updateError?.message, + connectionId, + code: activation.error.code, + message: activation.error.message, }) await markError( isConflict @@ -168,6 +233,7 @@ export async function POST(request: Request) { ) } + const activated = activation.connection try { await eventBus.emit({ type: 'woocommerce.connected', @@ -186,5 +252,5 @@ export async function POST(request: Request) { }) } - return NextResponse.json({ success: true }) + return NextResponse.json({ success: true, activated: true }) } diff --git a/app/api/extensions/woocommerce/orders/cron/__tests__/route.test.ts b/app/api/extensions/woocommerce/orders/cron/__tests__/route.test.ts new file mode 100644 index 00000000..6718dcd8 --- /dev/null +++ b/app/api/extensions/woocommerce/orders/cron/__tests__/route.test.ts @@ -0,0 +1,100 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + createServiceRoleClient: vi.fn(), + expireStaleHandshakes: vi.fn(), + syncWooCommerceOrders: vi.fn(), + hasCapability: vi.fn(), + registryGet: vi.fn(), +})) + +vi.mock('@/lib/supabase/service-client', () => ({ + createServiceRoleClient: mocks.createServiceRoleClient, +})) +vi.mock('@/lib/extensions/loader', () => ({ loadExtensions: vi.fn() })) +vi.mock('@/lib/extensions/registry', () => ({ + extensionRegistry: { get: mocks.registryGet }, +})) +vi.mock('@/lib/entitlements/has-capability', () => ({ hasCapability: mocks.hasCapability })) +vi.mock('@/extensions/general/woocommerce/lib/order-sync', () => ({ + syncWooCommerceOrders: mocks.syncWooCommerceOrders, +})) +vi.mock('@/extensions/general/woocommerce/lib/connect', () => ({ + expireStaleHandshakes: mocks.expireStaleHandshakes, +})) +vi.mock('@/lib/observability', () => ({ captureException: vi.fn(), captureMessage: vi.fn() })) + +import { GET } from '../route' +import { createQueuedMockSupabase } from '@/tests/helpers' + +function request() { + return new Request('https://app.testbrand.example/api/extensions/woocommerce/orders/cron', { + headers: { authorization: 'Bearer test-cron-secret' }, + }) +} + +describe('GET /api/extensions/woocommerce/orders/cron', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.stubEnv('CRON_SECRET', 'test-cron-secret') + vi.stubEnv('NEXT_PUBLIC_SUPABASE_URL', 'https://db.testbrand.example') + vi.stubEnv('SUPABASE_SERVICE_ROLE_KEY', 'service-key') + vi.stubEnv('WOOCOMMERCE_CREDENTIALS_ENCRYPTION_KEY', 'test-key') + mocks.registryGet.mockReturnValue({ id: 'woocommerce' }) + mocks.expireStaleHandshakes.mockResolvedValue({ expired: 0, error: null }) + mocks.hasCapability.mockResolvedValue(true) + }) + + afterEach(() => { + vi.unstubAllEnvs() + }) + + it('sweeps stale handshakes before it selects the active connections to sync', async () => { + const { supabase, enqueue, calls } = createQueuedMockSupabase() + mocks.createServiceRoleClient.mockReturnValue(supabase) + enqueue({ data: [] }) // active connections + mocks.expireStaleHandshakes.mockImplementation(async () => { + // Nothing has been read from the connections table yet. + expect(calls.filter((c) => c.table === 'woocommerce_connections')).toHaveLength(0) + return { expired: 2, error: null } + }) + + const res = await GET(request()) + + expect(res.status).toBe(200) + expect(mocks.expireStaleHandshakes).toHaveBeenCalledWith(supabase) + // Only ACTIVE rows with the feed on are ever handed to the sync. + const eqCalls = calls.filter((c) => c.method === 'eq').map((c) => c.args) + expect(eqCalls).toContainEqual(['status', 'active']) + expect(eqCalls).toContainEqual(['transaction_sync_enabled', true]) + expect(mocks.syncWooCommerceOrders).not.toHaveBeenCalled() + }) + + it('still syncs when the sweep fails: a pending row cannot sync either way', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + mocks.createServiceRoleClient.mockReturnValue(supabase) + mocks.expireStaleHandshakes.mockResolvedValue({ + expired: 0, + error: { code: '57014', message: 'canceled' }, + }) + const connection = { id: 'conn-1', company_id: 'company-1', status: 'active' } + enqueue({ data: [connection] }) + mocks.syncWooCommerceOrders.mockResolvedValue({ + inserted: 3, + updated: 0, + revoked: false, + deadlineReached: false, + }) + + const res = await GET(request()) + + expect(res.status).toBe(200) + expect(await res.json()).toMatchObject({ processed: 1, inserted: 3 }) + expect(mocks.syncWooCommerceOrders).toHaveBeenCalledWith( + supabase, + connection, + expect.anything(), + expect.any(Number), + ) + }) +}) diff --git a/app/api/extensions/woocommerce/orders/cron/route.ts b/app/api/extensions/woocommerce/orders/cron/route.ts index a31ed937..66b4808a 100644 --- a/app/api/extensions/woocommerce/orders/cron/route.ts +++ b/app/api/extensions/woocommerce/orders/cron/route.ts @@ -8,6 +8,7 @@ import { loadExtensions } from '@/lib/extensions/loader' import { extensionRegistry } from '@/lib/extensions/registry' import { isWooCommerceConfigured } from '@/extensions/general/woocommerce/lib/credentials' import { syncWooCommerceOrders } from '@/extensions/general/woocommerce/lib/order-sync' +import { expireStaleHandshakes } from '@/extensions/general/woocommerce/lib/connect' import type { WooCommerceConnection } from '@/extensions/general/woocommerce/types' export const maxDuration = 300 @@ -53,6 +54,28 @@ export const GET = withCronContext('cron.woocommerce_order_sync', async (_reques const supabase = createServiceRoleClient(supabaseUrl, supabaseServiceKey) + // The budget covers the whole handler, sweep included: maxDuration is + // wall-clock for the route, so the deadline is fixed before any awaited + // work, not after it. + const startTime = Date.now() + const TIME_BUDGET_MS = 240_000 // leave a minute of margin inside maxDuration + // Shared with syncWooCommerceOrders: it stops between pages and persists + // its cursor, so a truncated connection resumes next night. + const deadlineMs = startTime + TIME_BUDGET_MS + + // Hygiene first: park handshakes nobody completed and wipe their staged + // keys. A pending row never syncs (every consumer selects 'active'), so a + // failure here is logged, not fatal. + const sweep = await expireStaleHandshakes(supabase) + if (sweep.error) { + ctx.log.error('failed to expire stale woocommerce handshakes', { + code: sweep.error.code, + message: sweep.error.message, + }) + } else if (sweep.expired > 0) { + ctx.log.info('expired stale woocommerce handshakes', { expired: sweep.expired }) + } + const { data: connections, error: connError } = await supabase .from('woocommerce_connections') .select('*') @@ -76,12 +99,6 @@ export const GET = withCronContext('cron.woocommerce_order_sync', async (_reques }) } - const startTime = Date.now() - const TIME_BUDGET_MS = 240_000 // leave a minute of margin inside maxDuration - // Shared with syncWooCommerceOrders: it stops between pages and persists - // its cursor, so a truncated connection resumes next night. - const deadlineMs = startTime + TIME_BUDGET_MS - const results: Array<{ connectionId: string inserted: number diff --git a/app/api/extensions/woocommerce/return/route.ts b/app/api/extensions/woocommerce/return/route.ts index 4339ddcb..b9dac8d8 100644 --- a/app/api/extensions/woocommerce/return/route.ts +++ b/app/api/extensions/woocommerce/return/route.ts @@ -1,5 +1,7 @@ import { NextResponse } from 'next/server' import { createServiceClient } from '@/lib/supabase/server' +import { ensureInitialized } from '@/lib/init' +import { eventBus } from '@/lib/events/bus' import { loadExtensions } from '@/lib/extensions/loader' import { extensionRegistry } from '@/lib/extensions/registry' import { createLogger } from '@/lib/logger' @@ -7,6 +9,16 @@ import { requireFlowInitiator, FLOW_INITIATOR_MISMATCH_MESSAGE, } from '@/lib/auth/oauth-flow-binding' +import { + activateIfComplete, + HANDSHAKE_EXPIRED_MESSAGE, + isHandshakeExpired, +} from '@/extensions/general/woocommerce/lib/connect' + +// This route can be the leg that activates the row and emits +// woocommerce.connected; the event_log handler must be wired before the first +// emit on a cold instance. +ensureInitialized() const log = createLogger('woocommerce/return') @@ -20,15 +32,22 @@ const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/ * Browser leg of the wc-auth handshake: WooCommerce redirects the merchant * here with ?success=1|0&user_id=. The credentials arrive on * the separate server-to-server callback (usually before this redirect, but - * ordering is not guaranteed); the panel polls /status until the row is - * active. + * ordering is not guaranteed). * * This leg is the only point in the handshake where a browser session is * present, so it is where the completion is bound to the user who started the - * flow: the callback POST has no cookies (the store calls it) and so leaves - * the oauth_state on the row for this route to verify against and consume. - * Without that binding a victim lured into approving a connect someone else - * started would have their store's orders flowing into that someone's books. + * flow: the callback POST has no cookies (the store calls it) and so can only + * stage the keys. Confirmation here is the second signal; the row flips to + * active only when both are present (activateIfComplete, either order). + * + * What this binding does and does not give: activation always happens under + * the initiating user's session (auditable, never headless), and a return + * completed by a different signed-in user is refused and the keys taken + * back. It does NOT authenticate the person who approved in wp-admin: the + * wc-auth redirect carries only success and our own state, and the keys + * travel server-to-server, so a store admin who approves a link someone else + * generated still connects their store to that someone's company. Closing + * that needs a proof of store control from the initiator (follow-up). */ export async function GET(request: Request) { loadExtensions() @@ -63,6 +82,13 @@ export async function GET(request: Request) { status: 'error', error_message: 'Anslutningen nekades i butiken.', oauth_state: null, + consumer_key_encrypted: null, + consumer_secret_encrypted: null, + store_name: null, + currency: null, + prices_include_tax: null, + wc_version: null, + key_permissions: null, }) .eq('oauth_state', state) .eq('status', 'pending') @@ -78,8 +104,8 @@ export async function GET(request: Request) { /** * The approved leg: find the row this state belongs to, require that the - * browser completing it is the initiator's, then either hand the row over - * (consume the state) or take back what the callback stored. + * browser completing it is the initiator's, then record the confirmation and + * activate if the callback has already staged the keys. */ async function completeApproved( request: Request, @@ -98,7 +124,7 @@ async function completeApproved( const { data: row, error: findError } = await supabase .from('woocommerce_connections') - .select('id, user_id, status') + .select('id, user_id, status, created_at') .eq('oauth_state', state) .in('status', ['pending', 'active']) .single() @@ -120,10 +146,11 @@ async function completeApproved( return initiator.response } // A different user completed it. The callback POST may already have - // activated the row with the store's keys (it usually lands before this - // redirect), so refusing means taking that back: keys wiped, state - // consumed, row parked in 'error' with the reason. A still-pending row is - // closed the same way so the late callback finds nothing to activate. + // staged the store's keys (it usually lands before this redirect), so + // refusing means taking that back: keys wiped, state consumed, row parked + // in 'error' with the reason, so the late callback finds nothing to + // stage against either. Rows activated by the pre-gate callback are + // revoked the same way. const { error: revokeError } = await supabase .from('woocommerce_connections') .update({ @@ -132,6 +159,11 @@ async function completeApproved( oauth_state: null, consumer_key_encrypted: null, consumer_secret_encrypted: null, + store_name: null, + currency: null, + prices_include_tax: null, + wc_version: null, + key_permissions: null, }) .eq('id', row.id) .in('status', ['pending', 'active']) @@ -145,9 +177,9 @@ async function completeApproved( return NextResponse.redirect(`${returnUrl}&woocommerce_error=wrong_user`) } - // Initiator confirmed. An active row has been fully handed over: consume the - // state so the token cannot be presented again. A pending row keeps it: the - // callback POST has not landed yet and still needs it to find the row. + // Initiator confirmed. A row the pre-gate callback already activated has + // been fully handed over: consume the state so the token cannot be + // presented again. if (row.status === 'active') { const { error: consumeError } = await supabase .from('woocommerce_connections') @@ -161,6 +193,101 @@ async function completeApproved( message: consumeError.message, }) } + return NextResponse.redirect(connectedUrl) + } + + if (isHandshakeExpired(row.created_at)) { + await supabase + .from('woocommerce_connections') + .update({ + status: 'error', + error_message: HANDSHAKE_EXPIRED_MESSAGE, + oauth_state: null, + consumer_key_encrypted: null, + consumer_secret_encrypted: null, + store_name: null, + currency: null, + prices_include_tax: null, + wc_version: null, + key_permissions: null, + }) + .eq('id', row.id) + .eq('status', 'pending') + return NextResponse.redirect(`${returnUrl}&woocommerce_error=expired`) + } + + // Record the session-bound confirmation. The state stays on the row until + // activation: if the callback has not landed yet it still needs it. + const { error: confirmError } = await supabase + .from('woocommerce_connections') + .update({ browser_confirmed_at: new Date().toISOString() }) + .eq('id', row.id) + .eq('status', 'pending') + if (confirmError) { + log.error('failed to record browser confirmation', { + connectionId: row.id, + code: confirmError.code, + message: confirmError.message, + }) + return NextResponse.redirect(`${returnUrl}&woocommerce_error=failed`) + } + + const activation = await activateIfComplete(supabase, row.id) + + if (activation.outcome === 'incomplete') { + // Keys not staged yet: the callback is still in flight (or failed, in + // which case the row is parked in error and the panel shows why). No + // "connected" toast; the panel's pending note covers the wait. + return NextResponse.redirect(returnUrl) + } + + if (activation.outcome !== 'activated') { + const isConflict = activation.outcome === 'conflict' + log.error('failed to activate connection on the return leg', { + connectionId: row.id, + code: activation.error.code, + message: activation.error.message, + }) + await supabase + .from('woocommerce_connections') + .update({ + status: 'error', + error_message: isConflict + ? 'Butiken är redan ansluten till ett företag.' + : 'Anslutningen kunde inte slutföras.', + oauth_state: null, + consumer_key_encrypted: null, + consumer_secret_encrypted: null, + store_name: null, + currency: null, + prices_include_tax: null, + wc_version: null, + key_permissions: null, + }) + .eq('id', row.id) + .eq('status', 'pending') + return NextResponse.redirect( + `${returnUrl}&woocommerce_error=${isConflict ? 'conflict' : 'failed'}`, + ) + } + + const activated = activation.connection + try { + await eventBus.emit({ + type: 'woocommerce.connected', + payload: { + connectionId: activated.id, + storeUrl: activated.store_url, + userId: activated.user_id, + companyId: activated.company_id, + }, + }) + } catch (emitError) { + // Non-fatal: the DB state (source of truth) is already committed. + log.error('failed to emit woocommerce.connected', { + connectionId: activated.id, + message: emitError instanceof Error ? emitError.message : String(emitError), + }) } return NextResponse.redirect(connectedUrl) diff --git a/extensions/general/woocommerce/__tests__/api-routes.test.ts b/extensions/general/woocommerce/__tests__/api-routes.test.ts index 51574ebe..8d7051a2 100644 --- a/extensions/general/woocommerce/__tests__/api-routes.test.ts +++ b/extensions/general/woocommerce/__tests__/api-routes.test.ts @@ -30,6 +30,7 @@ import { CAPABILITY } from '@/lib/entitlements/keys' import { testConnectionAndFetchStoreInfo } from '../lib/api-client' import { syncWooCommerceOrders } from '../lib/order-sync' import { decryptCredential } from '../lib/credentials' +import { createServiceClientNoCookies } from '@/lib/auth/api-keys' import { createQueuedMockSupabase } from '@/tests/helpers' import type { ExtensionContext } from '@/lib/extensions/types' @@ -222,6 +223,9 @@ describe('woocommerce extension routes', () => { it('verifies, encrypts and activates on the happy path', async () => { const { supabase, enqueue, findCall } = createQueuedMockSupabase() supabase.auth.getUser.mockResolvedValue({ data: { user: USER }, error: null }) + // browser_confirmed_at is server-only, so the insert runs on the service + // client; route it to the same queue so the call is observable. + vi.mocked(createServiceClientNoCookies).mockReturnValueOnce(supabase as never) enqueue({ data: { is_sandbox: false } }) enqueue({ data: [] }) // no existing connection enqueue({ data: { id: 'conn-1', store_url: 'https://shop.example.se' } }) // insert @@ -246,6 +250,13 @@ describe('woocommerce extension routes', () => { string > expect(inserted.status).toBe('active') + // Keys typed in under a session count as the browser confirmation the + // activation CHECK (20260907143000) requires alongside the keys. + expect(typeof inserted.browser_confirmed_at).toBe('string') + // Scoped to the authenticated context, never to anything in the body. + expect(inserted.company_id).toBe('company-1') + expect(inserted.user_id).toBe(USER.id) + expect(createServiceClientNoCookies).toHaveBeenCalled() expect(inserted.store_name).toBe('Testbutiken') // Secrets never stored in plaintext, and they decrypt back. expect(inserted.consumer_key_encrypted).not.toContain('ck_x') diff --git a/extensions/general/woocommerce/__tests__/connect.test.ts b/extensions/general/woocommerce/__tests__/connect.test.ts new file mode 100644 index 00000000..e7e52b39 --- /dev/null +++ b/extensions/general/woocommerce/__tests__/connect.test.ts @@ -0,0 +1,125 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import type { SupabaseClient } from '@supabase/supabase-js' +import { + activateIfComplete, + expireStaleHandshakes, + HANDSHAKE_TTL_MS, + HANDSHAKE_EXPIRED_MESSAGE, + isHandshakeExpired, +} from '../lib/connect' +import { createQueuedMockSupabase } from '@/tests/helpers' + +const asClient = (supabase: unknown) => supabase as SupabaseClient + +describe('isHandshakeExpired', () => { + it('is false inside the TTL and true past it', () => { + const now = Date.parse('2026-09-07T12:00:00Z') + const fresh = new Date(now - HANDSHAKE_TTL_MS + 1_000).toISOString() + const stale = new Date(now - HANDSHAKE_TTL_MS - 1_000).toISOString() + expect(isHandshakeExpired(fresh, now)).toBe(false) + expect(isHandshakeExpired(stale, now)).toBe(true) + }) +}) + +describe('activateIfComplete', () => { + beforeEach(() => vi.clearAllMocks()) + + it('flips to active only where the row is pending AND both signals are present', async () => { + const { supabase, enqueue, calls, findCall } = createQueuedMockSupabase() + enqueue({ + data: { id: 'conn-1', company_id: 'c1', user_id: 'u1', store_url: 'https://s.example.se' }, + }) + + const now = Date.parse('2026-09-07T12:00:00Z') + const result = await activateIfComplete(asClient(supabase), 'conn-1', now) + + expect(result).toEqual({ + outcome: 'activated', + connection: { id: 'conn-1', company_id: 'c1', user_id: 'u1', store_url: 'https://s.example.se' }, + }) + const patch = findCall('woocommerce_connections', 'update')?.[0] as Record + expect(patch).toMatchObject({ + status: 'active', + transaction_sync_enabled: true, + oauth_state: null, + error_message: null, + }) + expect(typeof patch.connected_at).toBe('string') + const eqCalls = calls.filter((c) => c.method === 'eq').map((c) => c.args) + expect(eqCalls).toContainEqual(['id', 'conn-1']) + expect(eqCalls).toContainEqual(['status', 'pending']) + const notCalls = calls.filter((c) => c.method === 'not').map((c) => c.args) + expect(notCalls).toEqual([ + ['consumer_key_encrypted', 'is', null], + ['consumer_secret_encrypted', 'is', null], + ['browser_confirmed_at', 'is', null], + ]) + // The TTL is part of the flip: a row confirmed early and keyed late must + // not activate once it is older than the handshake window. + expect(calls.filter((c) => c.method === 'gte').map((c) => c.args)).toEqual([ + ['created_at', new Date(now - HANDSHAKE_TTL_MS).toISOString()], + ]) + }) + + it('reports incomplete when the conditional update matches no row', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: null }) + expect(await activateIfComplete(asClient(supabase), 'conn-1')).toEqual({ + outcome: 'incomplete', + }) + }) + + it('distinguishes the one-active-per-store conflict from other failures', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: null, error: { code: '23505', message: 'duplicate key' } }) + enqueue({ data: null, error: { code: '42P01', message: 'boom' } }) + expect(await activateIfComplete(asClient(supabase), 'conn-1')).toEqual({ + outcome: 'conflict', + error: { code: '23505', message: 'duplicate key' }, + }) + expect(await activateIfComplete(asClient(supabase), 'conn-1')).toEqual({ + outcome: 'failed', + error: { code: '42P01', message: 'boom' }, + }) + }) +}) + +describe('expireStaleHandshakes', () => { + it('parks only pending rows older than the TTL, wiping state and staged keys', async () => { + const { supabase, enqueue, calls, findCall } = createQueuedMockSupabase() + enqueue({ data: [{ id: 'a' }, { id: 'b' }] }) + const now = Date.parse('2026-09-07T12:00:00Z') + + const result = await expireStaleHandshakes(asClient(supabase), now) + + expect(result).toEqual({ expired: 2, error: null }) + const patch = findCall('woocommerce_connections', 'update')?.[0] + expect(patch).toEqual({ + status: 'error', + error_message: HANDSHAKE_EXPIRED_MESSAGE, + oauth_state: null, + consumer_key_encrypted: null, + consumer_secret_encrypted: null, + store_name: null, + currency: null, + prices_include_tax: null, + wc_version: null, + key_permissions: null, + }) + expect(calls.filter((c) => c.method === 'eq').map((c) => c.args)).toEqual([ + ['status', 'pending'], + ]) + expect(calls.filter((c) => c.method === 'lt').map((c) => c.args)).toEqual([ + ['created_at', new Date(now - HANDSHAKE_TTL_MS).toISOString()], + ]) + }) + + it('surfaces a database error without throwing', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: null, error: { code: '57014', message: 'canceled' } }) + expect(await expireStaleHandshakes(asClient(supabase))).toEqual({ + expired: 0, + error: { code: '57014', message: 'canceled' }, + }) + }) +}) diff --git a/extensions/general/woocommerce/__tests__/order-sync.test.ts b/extensions/general/woocommerce/__tests__/order-sync.test.ts index 6b640f22..ded0ff8f 100644 --- a/extensions/general/woocommerce/__tests__/order-sync.test.ts +++ b/extensions/general/woocommerce/__tests__/order-sync.test.ts @@ -60,6 +60,7 @@ function makeConnection(overrides: Partial = {}): WooComm key_permissions: 'read', status: 'active', oauth_state: null, + browser_confirmed_at: '2026-07-01T00:00:00.000Z', currency: 'SEK', prices_include_tax: true, wc_version: '9.9.5', diff --git a/extensions/general/woocommerce/api-routes.ts b/extensions/general/woocommerce/api-routes.ts index b573b5d1..ccea88a6 100644 --- a/extensions/general/woocommerce/api-routes.ts +++ b/extensions/general/woocommerce/api-routes.ts @@ -110,6 +110,13 @@ async function blockOrSupersedeExisting( status: 'error', error_message: 'Superseded by new connection attempt', oauth_state: null, + consumer_key_encrypted: null, + consumer_secret_encrypted: null, + store_name: null, + currency: null, + prices_include_tax: null, + wc_version: null, + key_permissions: null, }) .eq('company_id', auth.companyId) .eq('store_url', storeUrl) @@ -288,7 +295,12 @@ export const woocommerceApiRoutes: ApiRouteDefinition[] = [ ) } - const { data: created, error: insertError } = await auth.supabase + // The keys were typed in under this user's session: that IS the browser + // confirmation the activation CHECK requires. browser_confirmed_at is + // server-only (trigger, 20260907150000), so the insert runs on the + // service client; membership was already proven by requireUserAndCompany + // and company_id/user_id come from that context, never from the body. + const { data: created, error: insertError } = await createServiceClientNoCookies() .from('woocommerce_connections') .insert({ company_id: auth.companyId, @@ -302,6 +314,7 @@ export const woocommerceApiRoutes: ApiRouteDefinition[] = [ consumer_secret_encrypted: encryptCredential(consumerSecret), status: 'active', connected_at: new Date().toISOString(), + browser_confirmed_at: new Date().toISOString(), transaction_sync_enabled: true, }) .select('id, store_url') diff --git a/extensions/general/woocommerce/components/WooCommerceSettingsPanel.tsx b/extensions/general/woocommerce/components/WooCommerceSettingsPanel.tsx index 689dda26..ab16194f 100644 --- a/extensions/general/woocommerce/components/WooCommerceSettingsPanel.tsx +++ b/extensions/general/woocommerce/components/WooCommerceSettingsPanel.tsx @@ -110,6 +110,8 @@ export default function WooCommerceSettingsPanel() { const message = error === 'denied' ? t('error_denied') : error === 'wrong_user' ? t('error_wrong_user') + : error === 'expired' ? t('error_expired') + : error === 'conflict' ? t('error_conflict') : t('error_generic') toast({ title: t('connect_failed_title'), description: message, variant: 'destructive' }) } diff --git a/extensions/general/woocommerce/lib/connect.ts b/extensions/general/woocommerce/lib/connect.ts index ef6f1070..64e04be9 100644 --- a/extensions/general/woocommerce/lib/connect.ts +++ b/extensions/general/woocommerce/lib/connect.ts @@ -1,3 +1,4 @@ +import type { SupabaseClient } from '@supabase/supabase-js' import type { WooCredentials } from './api-client' import { decryptCredential } from './credentials' import type { WooCommerceConnection } from '../types' @@ -16,6 +17,23 @@ import type { WooCommerceConnection } from '../types' * probing the STORED store_url with the received keys before activation: a * forged POST would need working read credentials for the exact store the * user asked to connect. + * + * Activation itself needs TWO signals on the pending row: the verified keys + * (callback leg, no browser session) and browser_confirmed_at (return leg, + * bound to the initiator's session). Either leg may land first; both run + * activateIfComplete() after writing their own signal, and the row flips to + * active exactly once, under the DB CHECK that forbids an active row missing + * either signal (migration 20260907143000). Every credential consumer selects + * status = 'active', so staged keys on a pending row can never sync. + * + * Scope of the guarantee: a connection can never go live headless (callback + * alone) or under a session other than the initiator's, and abandoned + * handshakes are parked with their keys wiped. It is NOT a defense against a + * store admin approving a link the initiator generated for them: wc-auth + * cannot tell us who approved, and the initiator legitimately supplies the + * second signal. The confirmation column is member-writable through RLS + * like every other column on this row-scoped table; it records that the + * handshake completed under a session, it is not a trust boundary. */ const APP_NAME = 'Accounted' @@ -34,6 +52,123 @@ export function buildAuthorizeUrl(storeUrl: string, state: string): string { return `${storeUrl}/wc-auth/v1/authorize?${params.toString()}` } +/** + * A pending handshake older than this is dead: the return leg refuses it, + * activateIfComplete() will not flip it, and the nightly cron parks it with + * its staged keys wiped. The callback leg does NOT refuse it: WooCommerce + * turns any non-200 into a store-side error page and deletes the freshly + * minted key, which would strand a slow but legitimate approval; staging on + * a stale row is harmless because a pending row never syncs and the flip + * itself carries the TTL. + */ +export const HANDSHAKE_TTL_MS = 15 * 60_000 + +export const HANDSHAKE_EXPIRED_MESSAGE = + 'Anslutningen gick ut innan den slutfördes. Starta om anslutningen och ta bort den oanvända API-nyckeln i butikens WooCommerce-inställningar.' + +export function isHandshakeExpired(createdAt: string, now = Date.now()): boolean { + return now - new Date(createdAt).getTime() > HANDSHAKE_TTL_MS +} + +export interface ActivatedConnection { + id: string + company_id: string + user_id: string + store_url: string +} + +export type ActivationResult = + | { outcome: 'activated'; connection: ActivatedConnection } + /** The other signal has not landed yet (or the row is no longer pending). */ + | { outcome: 'incomplete' } + /** 23505: the store is already actively connected (this or another company). */ + | { outcome: 'conflict'; error: { code?: string; message: string } } + | { outcome: 'failed'; error: { code?: string; message: string } } + +/** + * Flip a pending row to active if, and only if, both signals are present. + * + * Both handshake legs call this after writing their own signal. Postgres + * serializes the two UPDATEs on the row, and the WHERE is re-evaluated + * against the latest committed version, so whichever leg runs last sees both + * signals and activates; the other matches zero rows and reports incomplete. + * The status = 'pending' scope makes it idempotent and blocks replay: an + * active row can never be activated again. + * + * The TTL is part of the predicate, not only of the return leg: the + * initiator can confirm early (the return URL is theirs to open), so + * without it a row confirmed at minute 1 would still flip when the keys + * land hours later. Past the TTL the flip matches zero rows and the sweep + * parks the row; the callback still answers 200 so the store does not + * wp_die on the merchant. + */ +export async function activateIfComplete( + supabase: SupabaseClient, + connectionId: string, + now = Date.now(), +): Promise { + const { data, error } = await supabase + .from('woocommerce_connections') + .update({ + status: 'active', + connected_at: new Date().toISOString(), + error_message: null, + // The state has done its job once both legs have found the row. + oauth_state: null, + // Feed-only product: connecting the store means fetching its orders, so + // the nightly feed starts on by default; the panel toggle is the opt-out. + transaction_sync_enabled: true, + }) + .eq('id', connectionId) + .eq('status', 'pending') + .not('consumer_key_encrypted', 'is', null) + .not('consumer_secret_encrypted', 'is', null) + .not('browser_confirmed_at', 'is', null) + .gte('created_at', new Date(now - HANDSHAKE_TTL_MS).toISOString()) + .select('id, company_id, user_id, store_url') + .maybeSingle() + + if (error) { + const err = { code: error.code, message: error.message } + return error.code === '23505' + ? { outcome: 'conflict', error: err } + : { outcome: 'failed', error: err } + } + if (!data) return { outcome: 'incomplete' } + return { outcome: 'activated', connection: data as ActivatedConnection } +} + +/** + * Park every pending handshake older than the TTL: status error, staged keys + * wiped, state consumed. Nothing on a pending row can sync, so this is + * hygiene for encrypted-at-rest secrets rather than a security boundary; + * running it once a night (from the orders cron) is enough. + */ +export async function expireStaleHandshakes( + supabase: SupabaseClient, + now = Date.now(), +): Promise<{ expired: number; error: { code?: string; message: string } | null }> { + const { data, error } = await supabase + .from('woocommerce_connections') + .update({ + status: 'error', + error_message: HANDSHAKE_EXPIRED_MESSAGE, + oauth_state: null, + consumer_key_encrypted: null, + consumer_secret_encrypted: null, + store_name: null, + currency: null, + prices_include_tax: null, + wc_version: null, + key_permissions: null, + }) + .eq('status', 'pending') + .lt('created_at', new Date(now - HANDSHAKE_TTL_MS).toISOString()) + .select('id') + if (error) return { expired: 0, error: { code: error.code, message: error.message } } + return { expired: data?.length ?? 0, error: null } +} + /** Decrypted API credentials for an active connection. */ export function credentialsOf( connection: Pick< diff --git a/extensions/general/woocommerce/types.ts b/extensions/general/woocommerce/types.ts index ca46cab9..4bb46650 100644 --- a/extensions/general/woocommerce/types.ts +++ b/extensions/general/woocommerce/types.ts @@ -11,6 +11,12 @@ export interface WooCommerceConnection { key_permissions: string | null status: 'pending' | 'active' | 'revoked' | 'error' oauth_state: string | null + /** + * Set by the session-bound leg (wc-auth return, or manual key entry). + * Together with stored credentials it is required for status 'active' + * (DB CHECK, migration 20260907143000). + */ + browser_confirmed_at: string | null currency: string | null prices_include_tax: boolean | null wc_version: string | null diff --git a/messages/en.json b/messages/en.json index 31da231b..7efc7b6d 100644 --- a/messages/en.json +++ b/messages/en.json @@ -468,6 +468,8 @@ "error_generic": "Something went wrong. Please try again.", "error_denied": "The connection was denied in the store.", "error_wrong_user": "The connection was started by another user in this company. Sign in as that user or restart the connection.", + "error_expired": "The connection timed out before it was completed. Start it again and remove the unused API key in the store's WooCommerce settings.", + "error_conflict": "The store is already connected to a company.", "sync_now": "Sync now", "syncing": "Syncing…", "sync_done_title": "Sync complete", diff --git a/messages/sv.json b/messages/sv.json index 6226a726..b86472bc 100644 --- a/messages/sv.json +++ b/messages/sv.json @@ -468,6 +468,8 @@ "error_generic": "Något gick fel. Försök igen.", "error_denied": "Anslutningen nekades i butiken.", "error_wrong_user": "Anslutningen startades av en annan användare i det här företaget. Logga in som den användaren eller starta om anslutningen.", + "error_expired": "Anslutningen gick ut innan den slutfördes. Starta om anslutningen och ta bort den oanvända API-nyckeln i butikens WooCommerce-inställningar.", + "error_conflict": "Butiken är redan ansluten till ett företag.", "sync_now": "Synka nu", "syncing": "Synkar…", "sync_done_title": "Synkronisering klar", diff --git a/supabase/migrations/20260907143000_woocommerce_activation_gate.sql b/supabase/migrations/20260907143000_woocommerce_activation_gate.sql new file mode 100644 index 00000000..864b82b6 --- /dev/null +++ b/supabase/migrations/20260907143000_woocommerce_activation_gate.sql @@ -0,0 +1,57 @@ +-- WooCommerce activation gate: an ACTIVE connection requires BOTH the +-- store-verified credentials (server-to-server wc-auth callback) AND the +-- initiating user's browser confirmation (the return leg, session-bound). +-- +-- Before this, the callback alone flipped a row to active: a connection could +-- go live headless, with nobody's session involved, and an abandoned handshake +-- stayed active (feed on) forever. browser_confirmed_at records the session- +-- bound confirmation of the initiating user. The CHECK makes "active implies +-- stored keys and a recorded confirmation" a database invariant instead of an +-- application promise: staged credentials live on PENDING rows, and every +-- consumer selects status = 'active'. +-- +-- Scope: this does not authenticate the person who approved in the store. +-- wc-auth delivers keys server-to-server and its redirect carries nothing +-- that identifies the approver, so a store admin who approves a link the +-- initiator generated still connects their store to the initiator's company. +-- The column is member-writable through RLS like the rest of the row; it is a +-- completion record, not a trust boundary. + +alter table public.woocommerce_connections + add column browser_confirmed_at timestamptz; + +comment on column public.woocommerce_connections.browser_confirmed_at is + 'Set when the initiating user''s browser session confirmed the wc-auth handshake (return leg) or when keys were entered manually under a session. Required, together with stored credentials, for status = active.'; + +-- Active rows without stored credentials cannot sync (credentialsOf() throws) +-- and would violate the invariant below: park them so the panel says why. +update public.woocommerce_connections + set status = 'error', + error_message = 'Anslutningen saknar API-nycklar. Anslut butiken igen.', + oauth_state = null + where status = 'active' + and (consumer_key_encrypted is null or consumer_secret_encrypted is null); + +-- Rows activated under the old flow completed the handshake in a browser; the +-- return leg just did not record it. Backfill from the activation timestamp. +update public.woocommerce_connections + set browser_confirmed_at = coalesce(connected_at, created_at) + where status = 'active' + and browser_confirmed_at is null; + +-- NOT VALID: enforced for every new and updated row from this statement on, +-- without the full-table scan under ACCESS EXCLUSIVE that a plain ADD +-- CONSTRAINT takes. The two UPDATEs above already made every existing row +-- conform; 20260907150000 runs VALIDATE CONSTRAINT under the weaker lock. +alter table public.woocommerce_connections + add constraint woocommerce_connections_active_requires_both_signals + check ( + status <> 'active' + or ( + consumer_key_encrypted is not null + and consumer_secret_encrypted is not null + and browser_confirmed_at is not null + ) + ) not valid; + +NOTIFY pgrst, 'reload schema'; diff --git a/supabase/migrations/20260907150000_woocommerce_browser_confirmation_server_only.sql b/supabase/migrations/20260907150000_woocommerce_browser_confirmation_server_only.sql new file mode 100644 index 00000000..252dc8b5 --- /dev/null +++ b/supabase/migrations/20260907150000_woocommerce_browser_confirmation_server_only.sql @@ -0,0 +1,67 @@ +-- Activation is server-only. Two things an end-user session (JWT role anon or +-- authenticated) must never do on woocommerce_connections: +-- +-- 1. write browser_confirmed_at, one of the two signals the activation +-- CHECK (20260907143000) requires: the row-scoped RLS policy would let +-- any writer of the company supply the initiator's signal on a +-- colleague's pending row; +-- 2. move a row INTO status 'active' at all: the CHECK only proves both +-- signals are present, while the 15-minute handshake TTL lives in the +-- server's conditional activation update (activateIfComplete), so a +-- direct UPDATE ... SET status = 'active' from PostgREST would flip a +-- fully staged but expired row. +-- +-- Both server paths (the wc-auth return leg and manual key entry) run on the +-- service role, as do the cron and migrations. Leaving 'active' (disconnect, +-- supersede, the sync marking a revoked key) stays member-writable. +-- +-- A trigger rather than column privileges: authenticated holds table-level +-- INSERT/UPDATE, and a column-level REVOKE is inert next to a table-level +-- grant (the alternative, revoke-all-then-regrant-per-column, breaks every +-- time a column is added). It keys on the JWT role claim exactly like +-- enforce_company_writer_role (20260902093000). Installed BEFORE the CHECK +-- below is validated, so at no point is the gate enforced with the signal +-- still member-writable. + +CREATE OR REPLACE FUNCTION public.woocommerce_browser_confirmation_server_only() +RETURNS trigger +LANGUAGE plpgsql +SET search_path = public +AS $$ +BEGIN + IF NOT public.jwt_caller_is_end_user() OR pg_trigger_depth() > 1 THEN + RETURN NEW; + END IF; + + IF TG_OP = 'INSERT' AND NEW.browser_confirmed_at IS NOT NULL THEN + RAISE EXCEPTION 'browser_confirmed_at is written by the server only' + USING ERRCODE = 'insufficient_privilege'; + END IF; + + IF TG_OP = 'UPDATE' AND NEW.browser_confirmed_at IS DISTINCT FROM OLD.browser_confirmed_at THEN + RAISE EXCEPTION 'browser_confirmed_at is written by the server only' + USING ERRCODE = 'insufficient_privilege'; + END IF; + + IF NEW.status = 'active' AND (TG_OP = 'INSERT' OR OLD.status IS DISTINCT FROM 'active') THEN + RAISE EXCEPTION 'woocommerce_connections are activated by the server only' + USING ERRCODE = 'insufficient_privilege'; + END IF; + + RETURN NEW; +END; +$$; + +DROP TRIGGER IF EXISTS ab_browser_confirmation_server_only ON public.woocommerce_connections; +CREATE TRIGGER ab_browser_confirmation_server_only + BEFORE INSERT OR UPDATE ON public.woocommerce_connections + FOR EACH ROW EXECUTE FUNCTION public.woocommerce_browser_confirmation_server_only(); + +-- Only now, with the signal and the transition server-only, validate the +-- activation CHECK added NOT VALID in 20260907143000. Existing rows were +-- already conformed there; VALIDATE takes SHARE UPDATE EXCLUSIVE, so writes +-- keep flowing while it scans. +alter table public.woocommerce_connections + validate constraint woocommerce_connections_active_requires_both_signals; + +NOTIFY pgrst, 'reload schema'; diff --git a/tests/pg/dashboard-nav-flags-rpc.pg.test.ts b/tests/pg/dashboard-nav-flags-rpc.pg.test.ts index 682a502d..f9400f1d 100644 --- a/tests/pg/dashboard-nav-flags-rpc.pg.test.ts +++ b/tests/pg/dashboard-nav-flags-rpc.pg.test.ts @@ -42,7 +42,12 @@ describe('get_dashboard_nav_flags()', () => { expect((await flagsAs(userId, companyId)).has_webshop).toBe(false) await getPool().query( - `UPDATE public.woocommerce_connections SET status = 'active' WHERE company_id = $1`, + // Active requires stored keys + browser confirmation (CHECK, 20260907143000). + `UPDATE public.woocommerce_connections + SET status = 'active', + consumer_key_encrypted = 'enc:k', consumer_secret_encrypted = 'enc:s', + browser_confirmed_at = now() + WHERE company_id = $1`, [companyId], ) expect((await flagsAs(userId, companyId)).has_webshop).toBe(true) diff --git a/tests/pg/woocommerce-connections.pg.test.ts b/tests/pg/woocommerce-connections.pg.test.ts index e711c8ae..ef180263 100644 --- a/tests/pg/woocommerce-connections.pg.test.ts +++ b/tests/pg/woocommerce-connections.pg.test.ts @@ -8,6 +8,12 @@ import { seedCompany } from './fixtures' // a previous run before the assertion under test is ever reached. const uniqueStore = (label: string) => 'https://' + label + '-' + randomUUID() + '.example.se' +// An ACTIVE row must carry both activation signals (CHECK, 20260907143000): +// stored credentials and the session-bound browser confirmation. +const ACTIVE_COLUMNS = + '(company_id, user_id, store_url, status, consumer_key_encrypted, consumer_secret_encrypted, browser_confirmed_at)' +const ACTIVE_VALUES = "'active', 'enc:k', 'enc:s', now()" + /** * Covers migration 20260806170000_woocommerce_connections: * 1. RLS: members insert and read their own company's connection, @@ -16,6 +22,20 @@ const uniqueStore = (label: string) => 'https://' + label + '-' + randomUUID() + * one-active-per-company index was dropped in 20260811073422). * 3. One store actively connected to at most one company. * 4. No DELETE policy: a member DELETE silently affects zero rows. + * Covers migration 20260907143000_woocommerce_activation_gate: + * 5. status = 'active' requires stored credentials AND browser_confirmed_at, + * on insert and on update, so the wc-auth callback (keys only) and the + * return leg (confirmation only) can each write their signal without + * either alone ever producing a syncable row. + * Covers migration 20260907150000_woocommerce_browser_confirmation_server_only: + * 6. browser_confirmed_at is server-only: an end-user session (JWT role + * authenticated) cannot insert or update it, even on its own company's + * row, so a colleague cannot supply the initiator's signal. Service-role + * and pool writes pass. + * 7. Activation is server-only: an end-user session cannot insert an + * 'active' row or move a row into 'active', even one that carries both + * signals (the TTL lives in the server's conditional update, which the + * CHECK alone does not enforce). Leaving 'active' stays member-writable. */ describe('woocommerce_connections RLS', () => { @@ -44,8 +64,8 @@ describe('woocommerce_connections RLS', () => { it('a non-member sees nothing and cannot insert for a foreign company', async () => { const { userId: ownerId, companyId } = await seedCompany() await getPool().query( - `INSERT INTO public.woocommerce_connections (company_id, user_id, store_url, status) - VALUES ($1, $2, $3, 'active')`, + `INSERT INTO public.woocommerce_connections ${ACTIVE_COLUMNS} + VALUES ($1, $2, $3, ${ACTIVE_VALUES})`, [companyId, ownerId, uniqueStore('foreign')], ) const { userId: outsiderId } = await seedCompany() // member of a DIFFERENT company @@ -70,13 +90,13 @@ describe('woocommerce_connections RLS', () => { it('a company may hold several ACTIVE connections (multi-store)', async () => { const { userId, companyId } = await seedCompany() await getPool().query( - `INSERT INTO public.woocommerce_connections (company_id, user_id, store_url, status) - VALUES ($1, $2, $3, 'active')`, + `INSERT INTO public.woocommerce_connections ${ACTIVE_COLUMNS} + VALUES ($1, $2, $3, ${ACTIVE_VALUES})`, [companyId, userId, uniqueStore('store-one')], ) const second = await getPool().query( - `INSERT INTO public.woocommerce_connections (company_id, user_id, store_url, status) - VALUES ($1, $2, $3, 'active') RETURNING id`, + `INSERT INTO public.woocommerce_connections ${ACTIVE_COLUMNS} + VALUES ($1, $2, $3, ${ACTIVE_VALUES}) RETURNING id`, [companyId, userId, uniqueStore('store-two')], ) expect(second.rows).toHaveLength(1) @@ -87,14 +107,14 @@ describe('woocommerce_connections RLS', () => { const { userId: userB, companyId: companyB } = await seedCompany() const sharedUrl = uniqueStore('shared') await getPool().query( - `INSERT INTO public.woocommerce_connections (company_id, user_id, store_url, status) - VALUES ($1, $2, $3, 'active')`, + `INSERT INTO public.woocommerce_connections ${ACTIVE_COLUMNS} + VALUES ($1, $2, $3, ${ACTIVE_VALUES})`, [companyA, userA, sharedUrl], ) await expect( getPool().query( - `INSERT INTO public.woocommerce_connections (company_id, user_id, store_url, status) - VALUES ($1, $2, $3, 'active')`, + `INSERT INTO public.woocommerce_connections ${ACTIVE_COLUMNS} + VALUES ($1, $2, $3, ${ACTIVE_VALUES})`, [companyB, userB, sharedUrl], ), ).rejects.toMatchObject({ code: '23505' }) @@ -111,8 +131,8 @@ describe('woocommerce_connections RLS', () => { it('members cannot DELETE (no DELETE policy; revoke is a status flip)', async () => { const { userId, companyId } = await seedCompany() const { rows } = await getPool().query( - `INSERT INTO public.woocommerce_connections (company_id, user_id, store_url, status) - VALUES ($1, $2, $3, 'active') RETURNING id`, + `INSERT INTO public.woocommerce_connections ${ACTIVE_COLUMNS} + VALUES ($1, $2, $3, ${ACTIVE_VALUES}) RETURNING id`, [companyId, userId, uniqueStore('keep')], ) await withUserContext(userId, async (client) => { @@ -128,4 +148,225 @@ describe('woocommerce_connections RLS', () => { ) expect(still.rows).toHaveLength(1) }) + + describe('activation gate (20260907143000)', () => { + const insertPending = async (companyId: string, userId: string, storeUrl: string) => { + const { rows } = await getPool().query( + `INSERT INTO public.woocommerce_connections (company_id, user_id, store_url, status, oauth_state) + VALUES ($1, $2, $3, 'pending', gen_random_uuid()) RETURNING id`, + [companyId, userId, storeUrl], + ) + return rows[0].id as string + } + + it('refuses an active row that has keys but no browser confirmation', async () => { + const { userId, companyId } = await seedCompany() + await expect( + getPool().query( + `INSERT INTO public.woocommerce_connections + (company_id, user_id, store_url, status, consumer_key_encrypted, consumer_secret_encrypted) + VALUES ($1, $2, $3, 'active', 'enc:k', 'enc:s')`, + [companyId, userId, uniqueStore('keys-only')], + ), + ).rejects.toMatchObject({ code: '23514' }) + }) + + it('refuses an active row that has a browser confirmation but no keys', async () => { + const { userId, companyId } = await seedCompany() + await expect( + getPool().query( + `INSERT INTO public.woocommerce_connections + (company_id, user_id, store_url, status, browser_confirmed_at) + VALUES ($1, $2, $3, 'active', now())`, + [companyId, userId, uniqueStore('browser-only')], + ), + ).rejects.toMatchObject({ code: '23514' }) + }) + + it('lets each leg stage its own signal on a pending row, and the conditional flip activates exactly once', async () => { + const { userId, companyId } = await seedCompany() + const id = await insertPending(companyId, userId, uniqueStore('two-legs')) + + // Callback leg: keys only. Still pending, so no consumer can select it. + await getPool().query( + `UPDATE public.woocommerce_connections + SET consumer_key_encrypted = 'enc:k', consumer_secret_encrypted = 'enc:s' + WHERE id = $1 AND status = 'pending'`, + [id], + ) + // Flipping now must be refused by the CHECK: browser has not confirmed. + await expect( + getPool().query( + `UPDATE public.woocommerce_connections SET status = 'active' WHERE id = $1`, + [id], + ), + ).rejects.toMatchObject({ code: '23514' }) + + // The application-side conditional flip (activateIfComplete) matches + // zero rows instead of erroring: the WHERE carries the same predicate. + const flip = () => + getPool().query( + `UPDATE public.woocommerce_connections + SET status = 'active', oauth_state = NULL + WHERE id = $1 AND status = 'pending' + AND consumer_key_encrypted IS NOT NULL + AND consumer_secret_encrypted IS NOT NULL + AND browser_confirmed_at IS NOT NULL + AND created_at >= now() - interval '15 minutes'`, + [id], + ) + expect((await flip()).rowCount).toBe(0) + + // Return leg: confirmation. Now the flip succeeds, and only once. + await getPool().query( + `UPDATE public.woocommerce_connections SET browser_confirmed_at = now() + WHERE id = $1 AND status = 'pending'`, + [id], + ) + expect((await flip()).rowCount).toBe(1) + expect((await flip()).rowCount).toBe(0) + + const { rows } = await getPool().query( + `SELECT status, oauth_state FROM public.woocommerce_connections WHERE id = $1`, + [id], + ) + expect(rows[0]).toEqual({ status: 'active', oauth_state: null }) + }) + + it('never flips a pending row older than the handshake TTL, even with both signals present', async () => { + const { userId, companyId } = await seedCompany() + const id = await insertPending(companyId, userId, uniqueStore('stale')) + // Initiator confirmed early, keys landed hours later. + await getPool().query( + `UPDATE public.woocommerce_connections + SET browser_confirmed_at = now(), + consumer_key_encrypted = 'enc:k', consumer_secret_encrypted = 'enc:s', + created_at = now() - interval '20 minutes' + WHERE id = $1`, + [id], + ) + const flip = await getPool().query( + `UPDATE public.woocommerce_connections + SET status = 'active', oauth_state = NULL + WHERE id = $1 AND status = 'pending' + AND consumer_key_encrypted IS NOT NULL + AND consumer_secret_encrypted IS NOT NULL + AND browser_confirmed_at IS NOT NULL + AND created_at >= now() - interval '15 minutes'`, + [id], + ) + expect(flip.rowCount).toBe(0) + const { rows } = await getPool().query( + `SELECT status FROM public.woocommerce_connections WHERE id = $1`, + [id], + ) + expect(rows[0].status).toBe('pending') + }) + + it('refuses browser_confirmed_at from an end-user session, on insert and on update', async () => { + const { userId, companyId } = await seedCompany() + const id = await insertPending(companyId, userId, uniqueStore('server-only')) + await getPool().query( + `UPDATE public.woocommerce_connections + SET consumer_key_encrypted = 'enc:k', consumer_secret_encrypted = 'enc:s' + WHERE id = $1`, + [id], + ) + // One user block per statement: withUserContext is a single transaction + // and the first refusal aborts it. + await withUserContext(userId, async (client) => { + // A member of the company (RLS passes) still cannot supply the signal. + await expect( + client.query( + `UPDATE public.woocommerce_connections SET browser_confirmed_at = now() WHERE id = $1`, + [id], + ), + ).rejects.toMatchObject({ code: '42501' }) + }) + await withUserContext(userId, async (client) => { + await expect( + client.query( + `INSERT INTO public.woocommerce_connections + (company_id, user_id, store_url, status, browser_confirmed_at) + VALUES ($1, $2, $3, 'pending', now())`, + [companyId, userId, uniqueStore('server-only-insert')], + ), + ).rejects.toMatchObject({ code: '42501' }) + }) + await withUserContext(userId, async (client) => { + // Other columns stay member-writable (the disconnect/supersede paths). + const ok = await client.query( + `UPDATE public.woocommerce_connections SET error_message = 'x' WHERE id = $1`, + [id], + ) + expect(ok.rowCount).toBe(1) + }) + // The server path (no end-user JWT) can. + const server = await getPool().query( + `UPDATE public.woocommerce_connections SET browser_confirmed_at = now() + WHERE id = $1 RETURNING browser_confirmed_at`, + [id], + ) + expect(server.rows[0].browser_confirmed_at).not.toBeNull() + }) + + it('refuses an end-user session moving a fully staged row into active, but lets it leave active', async () => { + const { userId, companyId } = await seedCompany() + const id = await insertPending(companyId, userId, uniqueStore('user-activate')) + // Both signals present, staged by the server, but already past the TTL: + // the CHECK would pass, the server's conditional flip would not. + await getPool().query( + `UPDATE public.woocommerce_connections + SET consumer_key_encrypted = 'enc:k', consumer_secret_encrypted = 'enc:s', + browser_confirmed_at = now(), created_at = now() - interval '20 minutes' + WHERE id = $1`, + [id], + ) + await withUserContext(userId, async (client) => { + await expect( + client.query( + `UPDATE public.woocommerce_connections SET status = 'active' WHERE id = $1`, + [id], + ), + ).rejects.toMatchObject({ code: '42501' }) + }) + await withUserContext(userId, async (client) => { + await expect( + client.query( + `INSERT INTO public.woocommerce_connections + (company_id, user_id, store_url, status, consumer_key_encrypted, consumer_secret_encrypted) + VALUES ($1, $2, $3, 'active', 'enc:k', 'enc:s')`, + [companyId, userId, uniqueStore('user-activate-insert')], + ), + ).rejects.toMatchObject({ code: '42501' }) + }) + // The server activates; the member may then disconnect (leave active). + await getPool().query( + `UPDATE public.woocommerce_connections SET status = 'active', created_at = now() WHERE id = $1`, + [id], + ) + await withUserContext(userId, async (client) => { + const revoked = await client.query( + `UPDATE public.woocommerce_connections + SET status = 'revoked', consumer_key_encrypted = NULL, consumer_secret_encrypted = NULL + WHERE id = $1`, + [id], + ) + expect(revoked.rowCount).toBe(1) + }) + }) + + it('lets a parked row drop its keys: the CHECK only constrains active rows', async () => { + const { userId, companyId } = await seedCompany() + const id = await insertPending(companyId, userId, uniqueStore('parked')) + const parked = await getPool().query( + `UPDATE public.woocommerce_connections + SET status = 'error', oauth_state = NULL, + consumer_key_encrypted = NULL, consumer_secret_encrypted = NULL + WHERE id = $1 RETURNING status`, + [id], + ) + expect(parked.rows[0].status).toBe('error') + }) + }) })