fix(company): revoke store connections when archiving a company (#2096)
* fix(company): revoke store connections when archiving a company Archiving a company left its WooCommerce/Shopify/Stripe connection rows at status 'active'. The store-uniqueness partial indexes (one active company per store) then blocked reconnecting the same store from any new company with 'Butiken är redan ansluten till ett företag', and since user_company_ids() hides archived companies there was no user-reachable disconnect. The archive flow now flips pending/active connections to 'revoked' and nulls their secrets, mirroring the manual disconnect paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016GTvGjEvpyqkepRmrtr5Vj * fix(company): audit connection revocations and cover error path on archive Review findings (compliance swarm A.8.15/A.8.29 + skeptic pass): - write audit_log rows for each revoked store connection (the tables have no auto-audit trigger) - revoke via .neq('status','revoked') so 'error'-state rows, which can also carry credentials, are cleared too - test that the archive still succeeds when a connection revoke fails Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016GTvGjEvpyqkepRmrtr5Vj * fix(company): literal payloads for connection revokes (phantom-column ceiling) The spread/mapped payloads registered as unresolvable expressions in tests/schema/no-phantom-columns.test.ts (392 > ceiling 391). Inline each table's update as an object literal and insert audit rows one per row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016GTvGjEvpyqkepRmrtr5Vj --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Mattsson
co-authored by
Claude Fable 5
Claude Fable 5
parent
5f81c0638c
commit
b74b5e3c0d
@@ -24,7 +24,10 @@ const mockCreateServiceClient = vi.mocked(createServiceClient)
|
||||
|
||||
type Row = { data: unknown; error: unknown }
|
||||
|
||||
function mockService(rowsByTable: Record<string, Row[]>) {
|
||||
function mockService(
|
||||
rowsByTable: Record<string, Row[]>,
|
||||
updateResults: Record<string, Row> = {},
|
||||
) {
|
||||
const insertSpy = vi.fn().mockResolvedValue({ data: null, error: null })
|
||||
const updateSpies: Record<string, ReturnType<typeof vi.fn>> = {}
|
||||
|
||||
@@ -32,12 +35,22 @@ function mockService(rowsByTable: Record<string, Row[]>) {
|
||||
const queue = rowsByTable[table] ?? []
|
||||
const next = queue.shift() ?? { data: null, error: null }
|
||||
|
||||
const updateFn = vi.fn().mockReturnValue({
|
||||
eq: vi.fn().mockReturnValue({
|
||||
eq: vi.fn().mockResolvedValue({ data: null, error: null }),
|
||||
then: (resolve: (v: unknown) => void) => resolve({ data: null, error: null }),
|
||||
}),
|
||||
})
|
||||
// Awaitable at any depth: update().eq(...), update().eq(...).eq(...),
|
||||
// update().eq(...).neq(...).select(...) all resolve to the table's
|
||||
// configured update result (default: one revoked row, no error).
|
||||
const updateResult = updateResults[table] ?? { data: [{ id: `${table}-1` }], error: null }
|
||||
const updateChain: {
|
||||
eq: ReturnType<typeof vi.fn>
|
||||
neq: ReturnType<typeof vi.fn>
|
||||
select: ReturnType<typeof vi.fn>
|
||||
then: (resolve: (v: unknown) => void) => void
|
||||
} = {
|
||||
eq: vi.fn(() => updateChain),
|
||||
neq: vi.fn(() => updateChain),
|
||||
select: vi.fn(() => updateChain),
|
||||
then: (resolve: (v: unknown) => void) => resolve(updateResult),
|
||||
}
|
||||
const updateFn = vi.fn().mockReturnValue(updateChain)
|
||||
updateSpies[table] = updateFn
|
||||
|
||||
return {
|
||||
@@ -244,4 +257,117 @@ describe('POST /api/company/[id]/delete', () => {
|
||||
expect(emitted).toHaveLength(1)
|
||||
expect(emitted[0]).toMatchObject({ companyId: 'c1', userId: 'user-1' })
|
||||
})
|
||||
|
||||
it('revokes store connections on archive so the store-uniqueness indexes free up', async () => {
|
||||
// Regression: archived companies are hidden by user_company_ids(), so an
|
||||
// 'active' connection left behind blocks reconnecting the same store from
|
||||
// a new company ("Butiken är redan ansluten till ett företag") with no
|
||||
// user-reachable disconnect.
|
||||
mockAuth('user-1')
|
||||
const { updateSpies } = mockService({
|
||||
companies: [{ data: { id: 'c1', name: 'Acme AB', archived_at: null }, error: null }],
|
||||
company_members: [{ data: { role: 'owner' }, error: null }],
|
||||
})
|
||||
|
||||
const req = createMockRequest('/api/company/c1/delete', {
|
||||
method: 'POST',
|
||||
body: { confirm_name: 'Acme AB' },
|
||||
})
|
||||
const { status } = await parseJsonResponse(
|
||||
await POST(req, createMockRouteParams({ id: 'c1' }))
|
||||
)
|
||||
expect(status).toBe(200)
|
||||
|
||||
expect(updateSpies.woocommerce_connections).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
status: 'revoked',
|
||||
disconnected_at: expect.any(String),
|
||||
consumer_key_encrypted: null,
|
||||
consumer_secret_encrypted: null,
|
||||
oauth_state: null,
|
||||
})
|
||||
)
|
||||
expect(updateSpies.shopify_connections).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
status: 'revoked',
|
||||
disconnected_at: expect.any(String),
|
||||
client_id_encrypted: null,
|
||||
client_secret_encrypted: null,
|
||||
})
|
||||
)
|
||||
expect(updateSpies.stripe_connections).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
status: 'revoked',
|
||||
disconnected_at: expect.any(String),
|
||||
oauth_state: null,
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('writes audit_log rows for each revoked connection', async () => {
|
||||
mockAuth('user-1')
|
||||
const { insertSpy } = mockService({
|
||||
companies: [{ data: { id: 'c1', name: 'Acme AB', archived_at: null }, error: null }],
|
||||
company_members: [{ data: { role: 'owner' }, error: null }],
|
||||
})
|
||||
|
||||
const req = createMockRequest('/api/company/c1/delete', {
|
||||
method: 'POST',
|
||||
body: { confirm_name: 'Acme AB' },
|
||||
})
|
||||
const { status } = await parseJsonResponse(
|
||||
await POST(req, createMockRouteParams({ id: 'c1' }))
|
||||
)
|
||||
expect(status).toBe(200)
|
||||
|
||||
// One audit insert per revoked connection row.
|
||||
for (const table of [
|
||||
'woocommerce_connections',
|
||||
'shopify_connections',
|
||||
'stripe_connections',
|
||||
]) {
|
||||
expect(insertSpy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
action: 'UPDATE',
|
||||
table_name: table,
|
||||
record_id: `${table}-1`,
|
||||
company_id: 'c1',
|
||||
user_id: 'user-1',
|
||||
new_state: expect.objectContaining({ status: 'revoked' }),
|
||||
}),
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
it('still archives the company when a connection revoke update fails', async () => {
|
||||
mockAuth('user-1')
|
||||
const { insertSpy, updateSpies } = mockService(
|
||||
{
|
||||
companies: [{ data: { id: 'c1', name: 'Acme AB', archived_at: null }, error: null }],
|
||||
company_members: [{ data: { role: 'owner' }, error: null }],
|
||||
},
|
||||
{
|
||||
woocommerce_connections: { data: null, error: { message: 'boom' } },
|
||||
},
|
||||
)
|
||||
|
||||
const req = createMockRequest('/api/company/c1/delete', {
|
||||
method: 'POST',
|
||||
body: { confirm_name: 'Acme AB' },
|
||||
})
|
||||
const { status } = await parseJsonResponse(
|
||||
await POST(req, createMockRouteParams({ id: 'c1' }))
|
||||
)
|
||||
|
||||
// Non-fatal by design: the archive already happened.
|
||||
expect(status).toBe(200)
|
||||
expect(updateSpies.companies).toHaveBeenCalled()
|
||||
// No audit rows for the failed table, but the other tables still processed.
|
||||
expect(insertSpy).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({ table_name: 'woocommerce_connections' }),
|
||||
)
|
||||
expect(insertSpy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ table_name: 'shopify_connections' }),
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -136,7 +136,95 @@ export async function POST(
|
||||
.eq('user_id', user.id)
|
||||
.eq('active_company_id', companyId)
|
||||
|
||||
// 6. Write audit log row. companies has no auto-audit trigger, so do it
|
||||
// 6. Revoke the company's store connections. The store-uniqueness indexes
|
||||
// (e.g. woocommerce_connections_store_active_uniq) allow a store to be
|
||||
// actively connected to at most ONE company, and user_company_ids() hides
|
||||
// archived companies, so an 'active' row left behind would block
|
||||
// reconnecting the store from any new company with no user-reachable
|
||||
// disconnect. Same status flip as the manual disconnect paths, secrets
|
||||
// nulled. Non-fatal: the archive already happened, but never silent.
|
||||
// .neq('revoked') rather than pending/active: 'error'-state rows also
|
||||
// carry credentials and are just as unreachable after the archive. Each
|
||||
// update carries its payload as an object literal so the phantom-column
|
||||
// scanner can resolve every column.
|
||||
const revokes = [
|
||||
{
|
||||
table: 'woocommerce_connections',
|
||||
result: await service
|
||||
.from('woocommerce_connections')
|
||||
.update({
|
||||
status: 'revoked',
|
||||
disconnected_at: archivedAt,
|
||||
consumer_key_encrypted: null,
|
||||
consumer_secret_encrypted: null,
|
||||
oauth_state: null,
|
||||
})
|
||||
.eq('company_id', companyId)
|
||||
.neq('status', 'revoked')
|
||||
.select('id'),
|
||||
},
|
||||
{
|
||||
table: 'shopify_connections',
|
||||
result: await service
|
||||
.from('shopify_connections')
|
||||
.update({
|
||||
status: 'revoked',
|
||||
disconnected_at: archivedAt,
|
||||
client_id_encrypted: null,
|
||||
client_secret_encrypted: null,
|
||||
})
|
||||
.eq('company_id', companyId)
|
||||
.neq('status', 'revoked')
|
||||
.select('id'),
|
||||
},
|
||||
{
|
||||
table: 'stripe_connections',
|
||||
result: await service
|
||||
.from('stripe_connections')
|
||||
.update({
|
||||
status: 'revoked',
|
||||
disconnected_at: archivedAt,
|
||||
oauth_state: null,
|
||||
})
|
||||
.eq('company_id', companyId)
|
||||
.neq('status', 'revoked')
|
||||
.select('id'),
|
||||
},
|
||||
]
|
||||
for (const { table, result } of revokes) {
|
||||
if (result.error) {
|
||||
log.error('Failed to revoke store connections on company archive', {
|
||||
companyId,
|
||||
table,
|
||||
error: result.error.message,
|
||||
})
|
||||
continue
|
||||
}
|
||||
// Credential revocation is a compliance-critical mutation: audit it like
|
||||
// the archive itself (the tables have no auto-audit trigger). Non-fatal,
|
||||
// same doctrine as the archive's own audit write below.
|
||||
for (const row of (result.data ?? []) as Array<{ id: string }>) {
|
||||
const { error: revokeAuditError } = await service.from('audit_log').insert({
|
||||
user_id: user.id,
|
||||
company_id: companyId,
|
||||
action: 'UPDATE',
|
||||
table_name: table,
|
||||
record_id: row.id,
|
||||
actor_id: user.id,
|
||||
new_state: { status: 'revoked', disconnected_at: archivedAt },
|
||||
description: `Store connection revoked on company archive: ${company.name}`,
|
||||
})
|
||||
if (revokeAuditError) {
|
||||
log.error('Failed to write audit_log row for connection revocation', {
|
||||
companyId,
|
||||
table,
|
||||
error: revokeAuditError.message,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// 7. Write audit log row. companies has no auto-audit trigger, so do it
|
||||
// explicitly. Service client bypasses audit_log RLS (no INSERT policy).
|
||||
// The archive already happened — don't fail the request, but an audit
|
||||
// write failing on an irreversible action must never be silent.
|
||||
@@ -158,13 +246,13 @@ export async function POST(
|
||||
})
|
||||
}
|
||||
|
||||
// 7. Emit event
|
||||
// 8. Emit event
|
||||
await eventBus.emit({
|
||||
type: 'company.deleted',
|
||||
payload: { companyId, userId: user.id, archivedAt },
|
||||
})
|
||||
|
||||
// 8. Build response and clear company cookie if it matched
|
||||
// 9. Build response and clear company cookie if it matched
|
||||
const response = NextResponse.json({ data: { companyId, archivedAt } })
|
||||
|
||||
const cookieCompanyId = request.headers.get('cookie')?.match(/gnubok-company-id=([^;]+)/)?.[1]
|
||||
|
||||
Reference in New Issue
Block a user