Skip to content

Commit bc24734

Browse files
committed
fix(billing): close TOCTOU race in subscription transfer, centralize stripe test mocks
1 parent d9209f9 commit bc24734

File tree

14 files changed

+443
-473
lines changed

14 files changed

+443
-473
lines changed

.claude/rules/sim-testing.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -217,13 +217,16 @@ it('reads a row', async () => {
217217
```
218218

219219
**Default chains supported:**
220-
- `select()/selectDistinct()/selectDistinctOn() → from() → where()/innerJoin()/leftJoin() → where() → limit()/orderBy()/returning()/groupBy()`
220+
- `select()/selectDistinct()/selectDistinctOn() → from() → where()/innerJoin()/leftJoin() → where() → limit()/orderBy()/returning()/groupBy()/for()`
221221
- `insert() → values() → returning()/onConflictDoUpdate()/onConflictDoNothing()`
222-
- `update() → set() → where() → limit()/orderBy()/returning()`
223-
- `delete() → where() → limit()/orderBy()/returning()`
222+
- `update() → set() → where() → limit()/orderBy()/returning()/for()`
223+
- `delete() → where() → limit()/orderBy()/returning()/for()`
224224
- `db.execute()` resolves `[]`
225225
- `db.transaction(cb)` calls cb with `dbChainMock.db`
226226

227+
`.for('update')` (Postgres row-level locking) is supported as a terminal on
228+
`where` builders. Override with `dbChainMockFns.for.mockResolvedValueOnce([...])`.
229+
227230
All terminals default to `Promise.resolve([])`. Override per-test with `dbChainMockFns.<terminal>.mockResolvedValueOnce(...)`.
228231

229232
Use `resetDbChainMock()` in `beforeEach` only when tests replace wiring with `.mockReturnValue` / `.mockResolvedValue` (permanent). Tests using only `...Once` variants don't need it.
Lines changed: 68 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -1,185 +1,118 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { createSession, loggerMock } from '@sim/testing'
4+
import {
5+
authMock,
6+
authMockFns,
7+
createSession,
8+
dbChainMock,
9+
dbChainMockFns,
10+
resetDbChainMock,
11+
} from '@sim/testing'
512
import { beforeEach, describe, expect, it, vi } from 'vitest'
613

7-
const { mockDbState, mockGetSession, mockHasPaidSubscription } = vi.hoisted(() => ({
8-
mockDbState: {
9-
selectResults: [] as any[],
10-
updateCalls: [] as Array<{ table: unknown; values: Record<string, unknown> }>,
11-
},
12-
mockGetSession: vi.fn(),
13-
mockHasPaidSubscription: vi.fn(),
14-
}))
15-
16-
vi.mock('@sim/db', () => ({
17-
db: {
18-
select: vi.fn().mockImplementation(() => {
19-
const chain: any = {}
20-
chain.from = vi.fn().mockReturnValue(chain)
21-
chain.where = vi.fn().mockReturnValue(chain)
22-
chain.then = vi
23-
.fn()
24-
.mockImplementation((callback: (rows: any[]) => any) =>
25-
Promise.resolve(callback(mockDbState.selectResults.shift() ?? []))
26-
)
27-
return chain
28-
}),
29-
update: vi.fn().mockImplementation((table: unknown) => ({
30-
set: vi.fn().mockImplementation((values: Record<string, unknown>) => {
31-
mockDbState.updateCalls.push({ table, values })
32-
return {
33-
where: vi.fn().mockResolvedValue(undefined),
34-
}
35-
}),
36-
})),
37-
},
38-
}))
39-
40-
vi.mock('@sim/db/schema', () => ({
41-
member: {
42-
userId: 'member.userId',
43-
organizationId: 'member.organizationId',
44-
},
45-
organization: {
46-
id: 'organization.id',
47-
},
48-
subscription: {
49-
id: 'subscription.id',
50-
referenceId: 'subscription.referenceId',
51-
},
52-
}))
53-
54-
vi.mock('drizzle-orm', () => ({
55-
and: vi.fn((...conditions: unknown[]) => ({ type: 'and', conditions })),
56-
eq: vi.fn((field: unknown, value: unknown) => ({ field, value })),
57-
}))
58-
59-
vi.mock('@sim/logger', () => loggerMock)
60-
61-
vi.mock('@/lib/auth', () => ({
62-
getSession: mockGetSession,
63-
}))
64-
65-
vi.mock('@/lib/billing', () => ({
66-
hasPaidSubscription: mockHasPaidSubscription,
67-
}))
14+
vi.mock('@sim/db', () => dbChainMock)
15+
vi.mock('@/lib/auth', () => authMock)
6816

6917
vi.mock('@/lib/billing/plan-helpers', () => ({
7018
isOrgPlan: (plan: string) => plan === 'team' || plan === 'enterprise',
7119
}))
7220

7321
vi.mock('@/lib/billing/subscriptions/utils', () => ({
22+
ENTITLED_SUBSCRIPTION_STATUSES: ['active', 'past_due'],
7423
hasPaidSubscriptionStatus: (status: string) => status === 'active' || status === 'past_due',
7524
}))
7625

7726
import { POST } from '@/app/api/users/me/subscription/[id]/transfer/route'
7827

28+
function makeRequest(body: unknown, id = 'sub-1') {
29+
return POST(
30+
new Request(`http://localhost/api/users/me/subscription/${id}/transfer`, {
31+
method: 'POST',
32+
headers: { 'Content-Type': 'application/json' },
33+
body: JSON.stringify(body),
34+
}) as any,
35+
{ params: Promise.resolve({ id }) }
36+
)
37+
}
38+
7939
describe('POST /api/users/me/subscription/[id]/transfer', () => {
8040
beforeEach(() => {
8141
vi.clearAllMocks()
82-
mockDbState.selectResults = []
83-
mockDbState.updateCalls = []
84-
mockHasPaidSubscription.mockResolvedValue(false)
85-
})
86-
87-
it('rejects transfers for non-organization subscriptions', async () => {
88-
mockGetSession.mockResolvedValue(
42+
resetDbChainMock()
43+
authMockFns.mockGetSession.mockResolvedValue(
8944
createSession({
9045
userId: 'user-1',
9146
email: 'owner@example.com',
9247
name: 'Owner',
9348
})
9449
)
95-
mockDbState.selectResults = [
96-
[{ id: 'sub-1', referenceId: 'user-1', plan: 'pro', status: 'active' }],
97-
]
98-
99-
const response = await POST(
100-
new Request('http://localhost/api/users/me/subscription/sub-1/transfer', {
101-
method: 'POST',
102-
headers: { 'Content-Type': 'application/json' },
103-
body: JSON.stringify({ organizationId: 'org-1' }),
104-
}) as any,
105-
{ params: Promise.resolve({ id: 'sub-1' }) }
106-
)
50+
})
51+
52+
it('rejects transfers for non-organization subscriptions', async () => {
53+
dbChainMockFns.for.mockResolvedValueOnce([
54+
{ id: 'sub-1', referenceId: 'user-1', plan: 'pro', status: 'active' },
55+
])
56+
57+
const response = await makeRequest({ organizationId: 'org-1' })
10758

10859
expect(response.status).toBe(400)
10960
await expect(response.json()).resolves.toEqual({
11061
error: 'Only active Team or Enterprise subscriptions can be transferred to an organization.',
11162
})
112-
expect(mockDbState.updateCalls).toEqual([])
63+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
11364
})
11465

11566
it('transfers an active organization subscription to an admin-owned organization', async () => {
116-
mockGetSession.mockResolvedValue(
117-
createSession({
118-
userId: 'user-1',
119-
email: 'owner@example.com',
120-
name: 'Owner',
121-
})
122-
)
123-
mockDbState.selectResults = [
124-
[{ id: 'sub-1', referenceId: 'user-1', plan: 'team', status: 'active' }],
125-
[{ id: 'org-1' }],
126-
[{ id: 'member-1', role: 'owner' }],
127-
]
128-
129-
const response = await POST(
130-
new Request('http://localhost/api/users/me/subscription/sub-1/transfer', {
131-
method: 'POST',
132-
headers: { 'Content-Type': 'application/json' },
133-
body: JSON.stringify({ organizationId: 'org-1' }),
134-
}) as any,
135-
{ params: Promise.resolve({ id: 'sub-1' }) }
136-
)
67+
dbChainMockFns.for
68+
.mockResolvedValueOnce([
69+
{ id: 'sub-1', referenceId: 'user-1', plan: 'team', status: 'active' },
70+
])
71+
.mockResolvedValueOnce([{ id: 'org-1' }])
72+
dbChainMockFns.limit.mockResolvedValueOnce([{ role: 'owner' }]).mockResolvedValueOnce([])
73+
74+
const response = await makeRequest({ organizationId: 'org-1' })
13775

13876
expect(response.status).toBe(200)
13977
await expect(response.json()).resolves.toEqual({
14078
success: true,
14179
message: 'Subscription transferred successfully',
14280
})
143-
expect(mockDbState.updateCalls).toEqual([
144-
{
145-
table: expect.objectContaining({
146-
id: 'subscription.id',
147-
referenceId: 'subscription.referenceId',
148-
}),
149-
values: { referenceId: 'org-1' },
150-
},
151-
])
81+
expect(dbChainMockFns.update).toHaveBeenCalled()
82+
expect(dbChainMockFns.set).toHaveBeenCalledWith({ referenceId: 'org-1' })
15283
})
15384

15485
it('treats an already-transferred organization subscription as a successful no-op', async () => {
155-
mockGetSession.mockResolvedValue(
156-
createSession({
157-
userId: 'user-1',
158-
email: 'owner@example.com',
159-
name: 'Owner',
160-
})
161-
)
162-
mockDbState.selectResults = [
163-
[{ id: 'sub-1', referenceId: 'org-1', plan: 'team', status: 'active' }],
164-
[{ id: 'org-1' }],
165-
[{ id: 'member-1', role: 'owner' }],
166-
]
167-
168-
const response = await POST(
169-
new Request('http://localhost/api/users/me/subscription/sub-1/transfer', {
170-
method: 'POST',
171-
headers: { 'Content-Type': 'application/json' },
172-
body: JSON.stringify({ organizationId: 'org-1' }),
173-
}) as any,
174-
{ params: Promise.resolve({ id: 'sub-1' }) }
175-
)
86+
dbChainMockFns.for.mockResolvedValueOnce([
87+
{ id: 'sub-1', referenceId: 'org-1', plan: 'team', status: 'active' },
88+
])
89+
90+
const response = await makeRequest({ organizationId: 'org-1' })
17691

17792
expect(response.status).toBe(200)
17893
await expect(response.json()).resolves.toEqual({
17994
success: true,
18095
message: 'Subscription already belongs to this organization',
18196
})
182-
expect(mockDbState.updateCalls).toEqual([])
183-
expect(mockHasPaidSubscription).not.toHaveBeenCalled()
97+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
98+
})
99+
100+
it('rejects the transfer when the target organization already has an active subscription', async () => {
101+
dbChainMockFns.for
102+
.mockResolvedValueOnce([
103+
{ id: 'sub-1', referenceId: 'user-1', plan: 'team', status: 'active' },
104+
])
105+
.mockResolvedValueOnce([{ id: 'org-1' }])
106+
dbChainMockFns.limit
107+
.mockResolvedValueOnce([{ role: 'owner' }])
108+
.mockResolvedValueOnce([{ id: 'existing-sub' }])
109+
110+
const response = await makeRequest({ organizationId: 'org-1' })
111+
112+
expect(response.status).toBe(409)
113+
await expect(response.json()).resolves.toEqual({
114+
error: 'Organization already has an active subscription',
115+
})
116+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
184117
})
185118
})

0 commit comments

Comments
 (0)