Skip to content

Commit 1941a0c

Browse files
committed
fix(auth): end the session on account deletion and refuse a deleted owner
Deleting an account removes the user and session rows, but the signed cookie cache keeps authenticating that browser for up to five minutes. The settings page then clears the query cache concurrently with signing out, every mounted query refetches, and the workspace list finds nothing and tries to create a default workspace for a user who no longer exists. The insert fails on the `workspace` -> `user` foreign key and surfaces as an unhandled 500 on roughly half of all deletions. The deletion response now clears the session cookies itself, so no later request from that browser carries them. The route builder keeps each cleared cookie on its own header line. Workspace creation classifies a user foreign key violation as `WorkspaceOwnerMissingError`, which both workspace routes answer with 401 instead of a logged fault.
1 parent 1622f02 commit 1941a0c

9 files changed

Lines changed: 232 additions & 3 deletions

File tree

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const mocks = vi.hoisted(() => ({
7+
defineInternalJsonRoute: vi.fn(() => vi.fn()),
8+
signOut: vi.fn(),
9+
}))
10+
11+
vi.mock('@/lib/api/server/routes', () => ({
12+
defineInternalJsonRoute: mocks.defineInternalJsonRoute,
13+
internalOrchestrationErrorPolicy: { project: vi.fn(), unhandled: vi.fn() },
14+
internalRateLimits: { none: vi.fn(() => ({ kind: 'none' })) },
15+
internalSessionAuth: { authenticate: vi.fn() },
16+
}))
17+
18+
vi.mock('@/lib/auth', () => ({
19+
auth: { api: { signOut: mocks.signOut } },
20+
getSession: vi.fn(),
21+
}))
22+
23+
vi.mock('@/lib/users/application/delete-account', () => ({
24+
deleteAccountUseCase: { execute: vi.fn() },
25+
previewAccountDeletionUseCase: { execute: vi.fn() },
26+
}))
27+
28+
vi.mock('@/lib/users/application/operations', () => ({
29+
userAccountOperations: { delete: { id: 'user.delete' }, previewDeletion: { id: 'user.preview' } },
30+
}))
31+
32+
import '@/app/api/users/me/deletion/route'
33+
34+
type RouteOptions = {
35+
finalizeResponse?: (args: { request: Request }) => Promise<{ headers?: HeadersInit }>
36+
}
37+
38+
/** Captured at import time; the route registers itself once when the module loads. */
39+
const routeOptions = mocks.defineInternalJsonRoute.mock.calls.map((call) => call[0] as RouteOptions)
40+
41+
describe('POST /api/users/me/deletion', () => {
42+
beforeEach(() => {
43+
mocks.signOut.mockReset()
44+
})
45+
46+
/**
47+
* The session row is deleted with the account, but the signed cookie cache
48+
* still authenticates the browser for its TTL; the response must clear it.
49+
*/
50+
it('clears the session cookies on the deletion response', async () => {
51+
const options = routeOptions.find(
52+
(candidate) => typeof candidate.finalizeResponse === 'function'
53+
)
54+
expect(options?.finalizeResponse).toBeDefined()
55+
56+
const cleared = new Headers([['set-cookie', 'better-auth.session_token=; Max-Age=0']])
57+
mocks.signOut.mockResolvedValue({ headers: cleared, response: { success: true } })
58+
const request = new Request('http://localhost/api/users/me/deletion', {
59+
method: 'POST',
60+
headers: { cookie: 'better-auth.session_token=abc' },
61+
})
62+
63+
const finalization = await options!.finalizeResponse!({ request })
64+
65+
expect(mocks.signOut).toHaveBeenCalledWith({ headers: request.headers, returnHeaders: true })
66+
expect(finalization.headers).toBe(cleared)
67+
})
68+
69+
it('never fails a completed deletion because the cookies could not be cleared', async () => {
70+
const options = routeOptions.find(
71+
(candidate) => typeof candidate.finalizeResponse === 'function'
72+
)
73+
mocks.signOut.mockRejectedValue(new Error('sign-out unavailable'))
74+
75+
await expect(
76+
options!.finalizeResponse!({
77+
request: new Request('http://localhost/api/users/me/deletion', { method: 'POST' }),
78+
})
79+
).resolves.toEqual({})
80+
})
81+
})

‎apps/sim/app/api/users/me/deletion/route.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
1+
import { createLogger } from '@sim/logger'
12
import { deleteAccountContract, getAccountDeletionPlanContract } from '@/lib/api/contracts'
23
import {
34
defineInternalJsonRoute,
45
internalOrchestrationErrorPolicy,
56
internalRateLimits,
67
internalSessionAuth,
78
} from '@/lib/api/server/routes'
9+
import { auth } from '@/lib/auth'
810
import {
911
deleteAccountUseCase,
1012
previewAccountDeletionUseCase,
1113
} from '@/lib/users/application/delete-account'
1214
import { userAccountOperations } from '@/lib/users/application/operations'
1315

16+
const logger = createLogger('AccountDeletionRoute')
17+
1418
export const dynamic = 'force-dynamic'
1519

1620
export const GET = defineInternalJsonRoute({
@@ -39,4 +43,19 @@ export const POST = defineInternalJsonRoute({
3943
mapInput: ({ body }) => ({ confirmEmail: body.confirmEmail }),
4044
useCase: deleteAccountUseCase,
4145
present: () => ({ success: true as const }),
46+
/**
47+
* The session row is gone, but the signed cookie cache authenticates this
48+
* browser for up to five more minutes. Clear the cookies on the deletion
49+
* response itself so nothing the page does afterwards can carry them.
50+
*/
51+
finalizeResponse: async ({ request }) => {
52+
try {
53+
const { headers } = await auth.api.signOut({ headers: request.headers, returnHeaders: true })
54+
return { headers }
55+
} catch (error) {
56+
/** The account is gone either way; the client's own sign-out and full reload still drop the cookie. */
57+
logger.warn('Could not clear session cookies after account deletion', { error })
58+
return {}
59+
}
60+
},
4261
})

‎apps/sim/app/api/workspaces/route.test.ts‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,21 @@ vi.mock('@sim/audit', () => ({
4747
vi.mock('@/lib/workspaces/policy', async () => {
4848
class WorkspaceCreationCapabilityWithheldError extends Error {}
4949
class WorkspaceCreationContextChangedError extends Error {}
50+
class WorkspaceOwnerMissingError extends Error {}
5051
return {
5152
getWorkspaceCreationPolicy: mockGetWorkspaceCreationPolicy,
5253
WorkspaceCreationCapabilityWithheldError,
5354
WorkspaceCreationContextChangedError,
55+
WorkspaceOwnerMissingError,
5456
}
5557
})
5658

57-
import { WorkspaceCreationCapabilityWithheldError } from '@/lib/workspaces/policy'
58-
import { POST } from '@/app/api/workspaces/route'
59+
import { listWorkspacesForViewer } from '@/lib/workspaces/list'
60+
import {
61+
WorkspaceCreationCapabilityWithheldError,
62+
WorkspaceOwnerMissingError,
63+
} from '@/lib/workspaces/policy'
64+
import { GET, POST } from '@/app/api/workspaces/route'
5965

6066
function createRequest() {
6167
return createMockRequest('POST', { name: 'New workspace' })
@@ -116,4 +122,27 @@ describe('POST /api/workspaces capability refusal', () => {
116122
expect(body.error).toBe('Your organization subscription is inactive.')
117123
expect(body.details).toBeUndefined()
118124
})
125+
126+
/**
127+
* After account deletion the browser's cached session cookie stays valid for
128+
* a few minutes, and the next list load finds no workspaces and tries to
129+
* create the default one for a user who no longer exists.
130+
*/
131+
it('answers a default-workspace insert for a deleted user with 401', async () => {
132+
mockGetSession.mockResolvedValue({ user: { id: 'user-1', name: 'Gone' } })
133+
vi.mocked(listWorkspacesForViewer).mockResolvedValue({
134+
workspaces: [],
135+
lastActiveWorkspaceId: null,
136+
pinnedWorkspaceIds: [],
137+
creationPolicy: { canCreate: true, organizationId: null, billedAccountUserId: 'user-1' },
138+
} as never)
139+
mockCreateWorkspace.mockRejectedValue(new WorkspaceOwnerMissingError('user-1'))
140+
141+
const response = await GET(
142+
createMockRequest('GET', undefined, undefined, 'http://localhost/api/workspaces?scope=active')
143+
)
144+
145+
expect(response.status).toBe(401)
146+
await expect(response.json()).resolves.toEqual({ error: 'Unauthorized' })
147+
})
119148
})

‎apps/sim/app/api/workspaces/route.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
getWorkspaceCreationPolicy,
2020
WorkspaceCreationCapabilityWithheldError,
2121
WorkspaceCreationContextChangedError,
22+
WorkspaceOwnerMissingError,
2223
} from '@/lib/workspaces/policy'
2324

2425
const logger = createLogger('Workspaces')
@@ -87,6 +88,10 @@ export const GET = withRouteHandler(async (request: Request) => {
8788
})
8889
return NextResponse.json(refreshedPayload)
8990
}
91+
/** A cached session cookie outlived the account it belongs to. */
92+
if (error instanceof WorkspaceOwnerMissingError) {
93+
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
94+
}
9095
throw error
9196
}
9297

@@ -207,6 +212,10 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
207212
{ status: 409 }
208213
)
209214
}
215+
/** A cached session cookie outlived the account it belongs to. */
216+
if (error instanceof WorkspaceOwnerMissingError) {
217+
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
218+
}
210219
/**
211220
* A lock timeout is contention, not a fault: creation serializes on the
212221
* organization's mutation locks and now also on `permission_group:<org>`,

‎apps/sim/lib/api/server/routes/internal-json-route.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,37 @@ describe('defineInternalJsonRoute', () => {
492492
})
493493
})
494494

495+
it('keeps every cookie a finalizer clears on its own header line', async () => {
496+
const handler = defineInternalJsonRoute({
497+
contract,
498+
auth,
499+
operation,
500+
rateLimit: internalRateLimits.none({ reason: 'Unit test' }),
501+
errorPolicy: internalOrchestrationErrorPolicy,
502+
mapInput: () => undefined,
503+
useCase: {
504+
operation,
505+
async execute() {
506+
return { value: 'ok' }
507+
},
508+
},
509+
finalizeResponse: () => ({
510+
headers: new Headers([
511+
['set-cookie', 'session_token=; Max-Age=0; Path=/'],
512+
['set-cookie', 'session_data=; Max-Age=0; Path=/'],
513+
]),
514+
}),
515+
})
516+
517+
const response = await handler(new NextRequest('http://localhost/api/test/internal-json-route'))
518+
519+
expect(response.status).toBe(200)
520+
expect(response.headers.getSetCookie()).toEqual([
521+
'session_token=; Max-Age=0; Path=/',
522+
'session_data=; Max-Age=0; Path=/',
523+
])
524+
})
525+
495526
it('selects a declared success status from the application result', async () => {
496527
const replayableContract = defineRouteContract({
497528
method: 'POST',

‎apps/sim/lib/api/server/routes/internal-json-route.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,11 @@ function appendFinalizedHeaders(base: HeadersInit | undefined, additions?: Heade
327327
const headers = new Headers(base)
328328
if (!additions) return headers
329329
new Headers(additions).forEach((value, key) => {
330+
/** A finalizer may clear several cookies at once; each needs its own header line. */
331+
if (key === 'set-cookie') {
332+
headers.append(key, value)
333+
return
334+
}
330335
if (headers.has(key)) {
331336
throw new Error(`Internal JSON response finalizer cannot replace header "${key}"`)
332337
}

‎apps/sim/lib/workspaces/create.test.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ import {
4747
createDefaultPersonalWorkspaceInTransaction,
4848
createWorkspace,
4949
} from '@/lib/workspaces/create'
50-
import { WORKSPACE_MODE } from '@/lib/workspaces/policy'
50+
import { WORKSPACE_MODE, WorkspaceOwnerMissingError } from '@/lib/workspaces/policy'
5151

5252
const params = {
5353
userId: 'creator-1',
@@ -127,6 +127,34 @@ describe('createWorkspace capability-gate placement', () => {
127127
})
128128
})
129129

130+
/**
131+
* A cached session cookie can outlive the user row by a few minutes. The
132+
* insert then fails on a `workspace` -> `user` foreign key, which the caller
133+
* must be able to tell apart from a fault so it answers 401, not 500.
134+
*/
135+
it('reports a missing owner as a typed error instead of a fault', async () => {
136+
mockResolveGoverningPermissionGroupOrganization.mockResolvedValue('org-1')
137+
dbChainMockFns.transaction.mockRejectedValue(
138+
Object.assign(new Error('insert or update on table "workspace" violates foreign key'), {
139+
code: '23503',
140+
constraint_name: 'workspace_billed_account_user_id_user_id_fk',
141+
})
142+
)
143+
144+
await expect(createWorkspace(params)).rejects.toBeInstanceOf(WorkspaceOwnerMissingError)
145+
})
146+
147+
it('rethrows other foreign key violations untouched', async () => {
148+
mockResolveGoverningPermissionGroupOrganization.mockResolvedValue('org-1')
149+
const failure = Object.assign(new Error('violates foreign key'), {
150+
code: '23503',
151+
constraint_name: 'workspace_organization_id_organization_id_fk',
152+
})
153+
dbChainMockFns.transaction.mockRejectedValue(failure)
154+
155+
await expect(createWorkspace(params)).rejects.toBe(failure)
156+
})
157+
130158
/**
131159
* The preflight policy resolved this value microseconds earlier in the same
132160
* request, and React's `cache()` memo does not span the two calls, so a

‎apps/sim/lib/workspaces/create.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { db } from '@sim/db'
22
import { permissions, type WorkspaceMode, workflow, workspace } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
4+
import { getPostgresConstraintName, getPostgresErrorCode } from '@sim/utils/errors'
45
import { generateId } from '@sim/utils/id'
56
import { PlatformEvents } from '@/lib/core/telemetry'
67
import type { DbOrTx } from '@/lib/db/types'
@@ -12,8 +13,15 @@ import {
1213
resolveGoverningPermissionGroupOrganization,
1314
resolveInviteFlags,
1415
WORKSPACE_MODE,
16+
WorkspaceOwnerMissingError,
1517
} from '@/lib/workspaces/policy'
1618

19+
/** Foreign keys from `workspace` to `user`; a violation means the acting user's row is gone. */
20+
const WORKSPACE_USER_FK_CONSTRAINTS = new Set([
21+
'workspace_owner_id_user_id_fk',
22+
'workspace_billed_account_user_id_user_id_fk',
23+
])
24+
1725
const logger = createLogger('WorkspaceCreate')
1826

1927
export interface CreateWorkspaceParams {
@@ -208,6 +216,13 @@ export async function createWorkspace(params: CreateWorkspaceParams) {
208216
createWorkspaceInTransaction(tx, { ...params, governingPermissionGroupOrganizationId })
209217
)
210218
} catch (error) {
219+
if (
220+
getPostgresErrorCode(error) === '23503' &&
221+
WORKSPACE_USER_FK_CONSTRAINTS.has(getPostgresConstraintName(error) ?? '')
222+
) {
223+
logger.warn('Workspace creation raced account deletion', { userId: params.userId })
224+
throw new WorkspaceOwnerMissingError(params.userId)
225+
}
211226
logger.error('Failed to create workspace', { userId: params.userId, error })
212227
throw error
213228
}

‎apps/sim/lib/workspaces/policy.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,18 @@ export interface WorkspaceCreationPolicy {
111111
blockedReasonCode?: 'organization-subscription-inactive' | 'permission-group-denied'
112112
}
113113

114+
/**
115+
* The acting user's row is gone, so no workspace can reference it. Reached
116+
* when a request still carrying a cached session cookie arrives after the
117+
* account was deleted; the caller should answer as unauthenticated.
118+
*/
119+
export class WorkspaceOwnerMissingError extends Error {
120+
constructor(userId: string) {
121+
super(`User ${userId} no longer exists`)
122+
this.name = 'WorkspaceOwnerMissingError'
123+
}
124+
}
125+
114126
export class WorkspaceCreationContextChangedError extends Error {
115127
constructor(message = 'Workspace creation context changed before the workspace was inserted') {
116128
super(message)

0 commit comments

Comments
 (0)