Skip to content

Commit 303cc5d

Browse files
committed
fix(billing): checkout guard, admin panel case
1 parent 128054e commit 303cc5d

8 files changed

Lines changed: 470 additions & 18 deletions

File tree

apps/sim/app/workspace/[workspaceId]/upgrade/hooks/use-upgrade-state.test.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,22 @@ describe('useUpgradeState', () => {
120120
})
121121
})
122122

123+
it('shows checkout admission failures through the standard error toast', async () => {
124+
mockHandleUpgrade.mockRejectedValueOnce(
125+
new Error('Your subscription payment is still processing.')
126+
)
127+
128+
await act(async () => {
129+
root.render(<Harness />)
130+
})
131+
132+
await act(async () => {
133+
await currentState?.doUpgrade('team', 25000)
134+
})
135+
136+
expect(mockToastError).toHaveBeenCalledWith('Your subscription payment is still processing.')
137+
})
138+
123139
it('includes the routed workspace when switching the host billing interval', async () => {
124140
await act(async () => {
125141
root.render(<Harness />)

apps/sim/lib/auth/auth.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,9 +1001,9 @@ export const auth = betterAuth({
10011001
/**
10021002
* Personal checkout guard. The Stripe plugin's `authorizeReference`
10031003
* only runs for organization references (it skips references equal to
1004-
* the session user), so duplicate-coverage enforcement for personal
1005-
* checkouts lives here: a member of an org with an entitled paid
1006-
* subscription must not buy a personal plan on top of it.
1004+
* the session user), so personal checkout admission lives here. It
1005+
* prevents both a duplicate checkout while Stripe payment is pending
1006+
* and a personal plan for someone already covered by an organization.
10071007
*/
10081008
if (isBillingEnabled && ctx.path === '/subscription/upgrade') {
10091009
const session = await getSessionFromCtx(ctx)

apps/sim/lib/auth/stripe-adapter-guard.test.ts

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,12 @@ function createBaseAdapter() {
2222
const asAdapter = (base: ReturnType<typeof createBaseAdapter>) =>
2323
guardSubscriptionPlanWrites(base as unknown as Parameters<typeof guardSubscriptionPlanWrites>[0])
2424

25-
const ORG_ROW = { id: 'sub-1', referenceId: 'org-1', plan: 'team_6000' }
25+
const ORG_ROW = {
26+
id: 'sub-1',
27+
referenceId: 'org-1',
28+
plan: 'team_6000',
29+
stripeSubscriptionId: 'stripe-sub-1',
30+
}
2631
const WHERE = [{ field: 'id', value: 'sub-1' }]
2732

2833
describe('guardSubscriptionPlanWrites', () => {
@@ -144,6 +149,86 @@ describe('guardSubscriptionPlanWrites', () => {
144149
expect(base.update).toHaveBeenCalled()
145150
})
146151

152+
it('blocks rebinding a personal subscription to a different Stripe subscription', async () => {
153+
const base = createBaseAdapter()
154+
const personalPro = {
155+
id: 'personal-pro',
156+
referenceId: 'user-1',
157+
plan: 'pro',
158+
stripeSubscriptionId: 'sub_personal_pro',
159+
}
160+
base.findOne.mockResolvedValueOnce(personalPro)
161+
162+
const guarded = asAdapter(base)
163+
const result = await guarded.update({
164+
model: 'subscription',
165+
where: [{ field: 'id', value: personalPro.id }] as never,
166+
update: {
167+
stripeSubscriptionId: 'sub_enterprise',
168+
status: 'active',
169+
periodEnd: new Date('2026-09-11T18:36:09Z'),
170+
billingInterval: 'month',
171+
},
172+
})
173+
174+
expect(result).toBeNull()
175+
expect(base.update).not.toHaveBeenCalled()
176+
})
177+
178+
it('allows Stripe state updates when the subscription ID is unchanged', async () => {
179+
const base = createBaseAdapter()
180+
base.findOne.mockResolvedValueOnce({
181+
id: 'personal-pro',
182+
referenceId: 'user-1',
183+
plan: 'pro',
184+
stripeSubscriptionId: 'sub_personal_pro',
185+
})
186+
187+
const guarded = asAdapter(base)
188+
await guarded.update({
189+
model: 'subscription',
190+
where: WHERE as never,
191+
update: {
192+
stripeSubscriptionId: 'sub_personal_pro',
193+
status: 'active',
194+
cancelAtPeriodEnd: true,
195+
},
196+
})
197+
198+
expect(base.update).toHaveBeenCalledWith(
199+
expect.objectContaining({
200+
update: {
201+
stripeSubscriptionId: 'sub_personal_pro',
202+
status: 'active',
203+
cancelAtPeriodEnd: true,
204+
},
205+
})
206+
)
207+
})
208+
209+
it('allows binding an unbound local subscription to Stripe', async () => {
210+
const base = createBaseAdapter()
211+
base.findOne.mockResolvedValueOnce({
212+
id: 'new-subscription',
213+
referenceId: 'user-1',
214+
plan: 'pro',
215+
stripeSubscriptionId: null,
216+
})
217+
218+
const guarded = asAdapter(base)
219+
await guarded.update({
220+
model: 'subscription',
221+
where: WHERE as never,
222+
update: { stripeSubscriptionId: 'sub_new', status: 'incomplete' },
223+
})
224+
225+
expect(base.update).toHaveBeenCalledWith(
226+
expect.objectContaining({
227+
update: { stripeSubscriptionId: 'sub_new', status: 'incomplete' },
228+
})
229+
)
230+
})
231+
147232
it('rejects creating an org-referenced subscription with a non-org plan', async () => {
148233
const base = createBaseAdapter()
149234
dbChainMockFns.limit.mockResolvedValueOnce([{ id: 'org-1' }])

apps/sim/lib/auth/stripe-adapter-guard.ts

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@ type SubscriptionWriteSurface = Pick<
1717
/**
1818
* The Better Auth Stripe plugin persists webhook state through the raw
1919
* database adapter BEFORE invoking our subscription callbacks — including the
20-
* `plan` column resolved from the Stripe price. That makes the adapter the
21-
* only in-process seam that can enforce the billing invariant that
22-
* organization-referenced subscriptions hold Team/Enterprise plans: by the
23-
* time `syncSubscriptionPlan` runs in a callback, the plugin's write has
24-
* already landed.
20+
* Stripe subscription ID and `plan` column resolved from the Stripe price.
21+
* That makes the adapter the only in-process seam that can enforce two billing
22+
* invariants before the write lands:
23+
*
24+
* - an existing subscription cannot be rebound to a different Stripe
25+
* subscription merely because both subscriptions share a customer;
26+
* - organization-referenced subscriptions hold Team/Enterprise plans.
2527
*
2628
* Checkout admission blocks user-driven violations; this guard blocks the
2729
* remaining vector — an operator swapping an org subscription onto a personal
@@ -77,11 +79,28 @@ function guardWriteSurface<TAdapter extends SubscriptionWriteSurface>(
7779
return adapter.create(data)
7880
},
7981
update: async (data) => {
80-
if (data.model === 'subscription' && hasNonOrgPlanWrite(data.update)) {
82+
if (data.model === 'subscription' && needsSubscriptionRowInspection(data.update)) {
8183
const row = await adapter.findOne<SubscriptionRowSlice>({
8284
model: 'subscription',
8385
where: data.where,
8486
})
87+
88+
if (row && attemptsStripeSubscriptionRebind(row, data.update)) {
89+
logger.error(
90+
'Blocked rebinding an existing subscription to a different Stripe subscription',
91+
{
92+
subscriptionId: row.id,
93+
referenceId: row.referenceId,
94+
currentStripeSubscriptionId: row.stripeSubscriptionId,
95+
rejectedStripeSubscriptionId: (data.update as { stripeSubscriptionId: string })
96+
.stripeSubscriptionId,
97+
}
98+
)
99+
return null as never
100+
}
101+
102+
if (!hasNonOrgPlanWrite(data.update)) return adapter.update(data)
103+
85104
const sanitized = await stripPlanWhenOrgReferenced(
86105
row ? [row] : [],
87106
data.update as Record<string, unknown>
@@ -110,6 +129,27 @@ interface SubscriptionRowSlice {
110129
id: string
111130
referenceId: string
112131
plan: string
132+
stripeSubscriptionId: string | null
133+
}
134+
135+
function needsSubscriptionRowInspection(update: unknown): boolean {
136+
return hasStripeSubscriptionIdWrite(update) || hasNonOrgPlanWrite(update)
137+
}
138+
139+
function hasStripeSubscriptionIdWrite(
140+
update: unknown
141+
): update is { stripeSubscriptionId: unknown } {
142+
return Boolean(update && typeof update === 'object' && 'stripeSubscriptionId' in update)
143+
}
144+
145+
function attemptsStripeSubscriptionRebind(row: SubscriptionRowSlice, update: unknown): boolean {
146+
if (!hasStripeSubscriptionIdWrite(update)) return false
147+
const incomingStripeSubscriptionId = update.stripeSubscriptionId
148+
return (
149+
typeof row.stripeSubscriptionId === 'string' &&
150+
typeof incomingStripeSubscriptionId === 'string' &&
151+
incomingStripeSubscriptionId !== row.stripeSubscriptionId
152+
)
113153
}
114154

115155
function hasNonOrgPlanWrite(update: unknown): boolean {

apps/sim/lib/billing/authorization.test.ts

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { resetDbChainMock } from '@sim/testing'
4+
import { dbChainMockFns, hasMockCondition, resetDbChainMock, schemaMock } from '@sim/testing'
55
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
66

77
const {
@@ -96,6 +96,20 @@ describe('authorizeSubscriptionReference', () => {
9696
expect(mockIsOwnerOrAdmin).toHaveBeenCalledWith('owner-1', 'org-1')
9797
})
9898

99+
it('blocks an organization checkout while its bound Stripe subscription is incomplete', async () => {
100+
dbChainMockFns.limit.mockResolvedValueOnce([
101+
{ id: 'subscription-1', stripeSubscriptionId: 'sub_pending' },
102+
])
103+
104+
await expect(
105+
authorizeSubscriptionReference('owner-1', 'org-1', 'upgrade-subscription', 'team_6000')
106+
).rejects.toThrow(/subscription payment is still processing/)
107+
108+
expect(mockHasPaidSubscription).not.toHaveBeenCalled()
109+
expect(mockAssertNoUnresolved).not.toHaveBeenCalled()
110+
expect(mockIsOwnerOrAdmin).not.toHaveBeenCalled()
111+
})
112+
99113
it('rejects an organization checkout for a pro plan — org references only hold Team/Enterprise', async () => {
100114
await expect(
101115
authorizeSubscriptionReference('owner-1', 'org-1', 'upgrade-subscription', 'pro_6000')
@@ -148,6 +162,46 @@ describe('assertPersonalCheckoutAllowed', () => {
148162
await expect(assertPersonalCheckoutAllowed('user-1')).resolves.toBeUndefined()
149163
})
150164

165+
it('keeps abandoned, unbound checkout placeholders retryable', async () => {
166+
await assertPersonalCheckoutAllowed('user-1')
167+
168+
const predicate = dbChainMockFns.where.mock.calls[0]?.[0]
169+
expect(
170+
hasMockCondition(
171+
predicate,
172+
(node) => node.type === 'eq' && node.left === schemaMock.subscription.referenceId
173+
)
174+
).toBe(true)
175+
expect(
176+
hasMockCondition(
177+
predicate,
178+
(node) =>
179+
node.type === 'eq' &&
180+
node.left === schemaMock.subscription.status &&
181+
node.right === 'incomplete'
182+
)
183+
).toBe(true)
184+
expect(
185+
hasMockCondition(
186+
predicate,
187+
(node) =>
188+
node.type === 'isNotNull' && node.column === schemaMock.subscription.stripeSubscriptionId
189+
)
190+
).toBe(true)
191+
})
192+
193+
it('blocks a personal checkout while its bound Stripe subscription is incomplete', async () => {
194+
dbChainMockFns.limit.mockResolvedValueOnce([
195+
{ id: 'subscription-1', stripeSubscriptionId: 'sub_pending' },
196+
])
197+
198+
await expect(assertPersonalCheckoutAllowed('user-1')).rejects.toThrow(
199+
/subscription payment is still processing/
200+
)
201+
202+
expect(mockGetOrganizationCoverageForMember).not.toHaveBeenCalled()
203+
})
204+
151205
it('rejects checkout when an organization subscription already covers the user', async () => {
152206
mockGetOrganizationCoverageForMember.mockResolvedValueOnce({
153207
status: 'covered',

apps/sim/lib/billing/authorization.ts

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { db } from '@sim/db'
2+
import { subscription } from '@sim/db/schema'
23
import { createLogger } from '@sim/logger'
34
import { APIError } from 'better-auth/api'
5+
import { and, eq, isNotNull } from 'drizzle-orm'
46
import { hasPaidSubscription } from '@/lib/billing'
57
import { isOrganizationOwnerOrAdmin } from '@/lib/billing/core/organization'
68
import { getOrganizationCoverageForMember } from '@/lib/billing/core/subscription'
@@ -13,6 +15,44 @@ import { isOrgScopedSubscription } from '@/lib/billing/subscriptions/utils'
1315

1416
const logger = createLogger('BillingAuthorization')
1517

18+
/**
19+
* Prevent a billing reference from starting a second Stripe Checkout while a
20+
* previously completed Checkout still has a payment in flight.
21+
*
22+
* Better Auth intentionally reuses an unbound `incomplete` placeholder when a
23+
* customer retries an abandoned Checkout. Once Stripe has bound that row to a
24+
* subscription, however, reusing it would create another Stripe subscription
25+
* and make later webhooks race to own the same local row.
26+
*/
27+
async function assertNoBoundIncompleteSubscription(referenceId: string): Promise<void> {
28+
const [pendingSubscription] = await db
29+
.select({
30+
id: subscription.id,
31+
stripeSubscriptionId: subscription.stripeSubscriptionId,
32+
})
33+
.from(subscription)
34+
.where(
35+
and(
36+
eq(subscription.referenceId, referenceId),
37+
eq(subscription.status, 'incomplete'),
38+
isNotNull(subscription.stripeSubscriptionId)
39+
)
40+
)
41+
.limit(1)
42+
43+
if (!pendingSubscription) return
44+
45+
logger.warn('Blocking checkout - Stripe subscription payment is still pending', {
46+
referenceId,
47+
subscriptionId: pendingSubscription.id,
48+
stripeSubscriptionId: pendingSubscription.stripeSubscriptionId,
49+
})
50+
throw new APIError('CONFLICT', {
51+
message:
52+
'Your subscription payment is still processing. Wait for it to finish before starting another checkout. If this takes longer than expected, contact support.',
53+
})
54+
}
55+
1656
/**
1757
* Classify a `/subscription/upgrade` request as a personal checkout using
1858
* the same reference resolution as the Better Auth Stripe plugin
@@ -39,11 +79,9 @@ export function isPersonalCheckoutRequest(
3979
/**
4080
* Guard for personal (user-referenced) checkouts on `/subscription/upgrade`.
4181
*
42-
* A member of an organization with an entitled paid subscription is already
43-
* covered by that org — their usage pools to it and personal Pro
44-
* subscriptions are paused on join — so a personal checkout would bill the
45-
* same human twice. Throws {@link APIError} with a user-facing message; the
46-
* checkout UI surfaces it as-is.
82+
* Blocks both a bound, incomplete Stripe subscription and a member already
83+
* covered by an entitled organization subscription. Throws {@link APIError}
84+
* with a user-facing message; the checkout UI surfaces it as-is.
4785
*
4886
* Called from the Better Auth `hooks.before` middleware, NOT from
4987
* `authorizeReference`: the Stripe plugin skips `authorizeReference`
@@ -54,6 +92,8 @@ export function isPersonalCheckoutRequest(
5492
* rejected rather than risking a duplicate subscription.
5593
*/
5694
export async function assertPersonalCheckoutAllowed(userId: string): Promise<void> {
95+
await assertNoBoundIncompleteSubscription(userId)
96+
5797
const coverage = await getOrganizationCoverageForMember(userId)
5898

5999
if (coverage.status === 'covered') {
@@ -90,6 +130,8 @@ export async function assertPersonalCheckoutAllowed(userId: string): Promise<voi
90130
* reason instead of a generic "Unauthorized":
91131
* - Organizations can only check out Team or Enterprise plans — a `pro_*`
92132
* plan can never become org-referenced.
133+
* - A bound, incomplete Stripe subscription must finish before another
134+
* checkout can start.
93135
* - Organizations cannot start a checkout while they already have an
94136
* active subscription (prevents duplicates).
95137
* - Checkout is deferred while an Enterprise issuance is unresolved.
@@ -115,6 +157,10 @@ export async function authorizeSubscriptionReference(
115157
})
116158
}
117159

160+
if (action === 'upgrade-subscription') {
161+
await assertNoBoundIncompleteSubscription(referenceId)
162+
}
163+
118164
if (action === 'upgrade-subscription' && (await hasPaidSubscription(referenceId))) {
119165
logger.warn('Blocking checkout - active subscription already exists for organization', {
120166
userId,

0 commit comments

Comments
 (0)