Skip to content

Commit af54aa6

Browse files
committed
close race with personal pro / org inclusions
1 parent 3a3a372 commit af54aa6

5 files changed

Lines changed: 282 additions & 25 deletions

File tree

apps/sim/lib/auth/auth.ts

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ import {
5050
ensureOrganizationForTeamSubscription,
5151
syncSubscriptionUsageLimits,
5252
} from '@/lib/billing/organization'
53-
import { isTeam } from '@/lib/billing/plan-helpers'
53+
import { pauseProSubscriptionForOrgCoverage } from '@/lib/billing/organizations/membership'
54+
import { isPro, isTeam } from '@/lib/billing/plan-helpers'
5455
import { getPlans, resolvePlanFromStripeSubscription } from '@/lib/billing/plans'
5556
import { syncSeatsFromStripeQuantity } from '@/lib/billing/validation/seat-management'
5657
import { handleAbandonedCheckout } from '@/lib/billing/webhooks/checkout'
@@ -3161,11 +3162,11 @@ export const auth = betterAuth({
31613162
authorizeReference: async ({ user, referenceId, action }, ctx) => {
31623163
const body: unknown = ctx?.body
31633164
const requestedPlan =
3164-
body &&
31653165
typeof body === 'object' &&
3166+
body !== null &&
31663167
'plan' in body &&
3167-
typeof (body as { plan: unknown }).plan === 'string'
3168-
? (body as { plan: string }).plan
3168+
typeof body.plan === 'string'
3169+
? body.plan
31693170
: undefined
31703171
return await authorizeSubscriptionReference(
31713172
user.id,
@@ -3206,7 +3207,7 @@ export const auth = betterAuth({
32063207
)
32073208
}
32083209

3209-
await syncSubscriptionPlan(
3210+
const syncedPlan = await syncSubscriptionPlan(
32103211
subscription.id,
32113212
subscription.plan,
32123213
planFromStripe,
@@ -3215,7 +3216,7 @@ export const auth = betterAuth({
32153216

32163217
const subscriptionForOrg = {
32173218
...subscription,
3218-
plan: planFromStripe ?? subscription.plan,
3219+
plan: syncedPlan ?? subscription.plan,
32193220
enterpriseOperationId: stripeSubscription.metadata?.enterpriseOperationId ?? null,
32203221
}
32213222

@@ -3242,6 +3243,17 @@ export const auth = betterAuth({
32423243

32433244
await syncSubscriptionUsageLimits(resolvedSubscription)
32443245

3246+
/**
3247+
* Transactional fence behind the personal-checkout admission
3248+
* guard: if the user joined a paid organization while their
3249+
* checkout was in flight, pause the fresh personal Pro at
3250+
* period end (same state a paid-org joiner's personal Pro
3251+
* enters; restored automatically if they leave the org).
3252+
*/
3253+
if (isPro(resolvedSubscription.plan)) {
3254+
await pauseProSubscriptionForOrgCoverage(resolvedSubscription.referenceId)
3255+
}
3256+
32453257
await writeBillingInterval(resolvedSubscription.id, isAnnual ? 'year' : 'month')
32463258

32473259
await sendPlanWelcomeEmail(resolvedSubscription)
@@ -3274,8 +3286,6 @@ export const auth = betterAuth({
32743286
const isUpgradeToTeam =
32753287
isTeamPlan && !isTeam(subscription.plan) && referenceOrganizationId == null
32763288

3277-
const effectivePlanForTeamFeatures = planFromStripe ?? subscription.plan
3278-
32793289
logger.info('[onSubscriptionUpdate] Subscription updated', {
32803290
subscriptionId: subscription.id,
32813291
status: subscription.status,
@@ -3294,16 +3304,24 @@ export const auth = betterAuth({
32943304
)
32953305
}
32963306

3297-
await syncSubscriptionPlan(
3307+
const syncedPlan = await syncSubscriptionPlan(
32983308
subscription.id,
32993309
subscription.plan,
33003310
planFromStripe,
33013311
subscription.referenceId
33023312
)
33033313

3314+
/**
3315+
* All downstream processing keys off the plan the DB actually
3316+
* holds after the sync — a plan write refused by the org/plan
3317+
* invariant must not leak the rejected Stripe plan into org
3318+
* resolution, seat sync, or usage limits.
3319+
*/
3320+
const effectivePlanForTeamFeatures = syncedPlan ?? subscription.plan
3321+
33043322
const subscriptionForOrg = {
33053323
...subscription,
3306-
plan: planFromStripe ?? subscription.plan,
3324+
plan: effectivePlanForTeamFeatures,
33073325
enterpriseOperationId: stripeSubscription.metadata?.enterpriseOperationId ?? null,
33083326
}
33093327

apps/sim/lib/billing/core/subscription.test.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,37 +111,39 @@ describe('syncSubscriptionPlan', () => {
111111
vi.clearAllMocks()
112112
})
113113

114-
it('writes the resolved plan for a user-referenced subscription', async () => {
114+
it('writes the resolved plan for a user-referenced subscription and returns it', async () => {
115115
dbChainMockFns.limit.mockResolvedValueOnce([])
116116

117117
await expect(syncSubscriptionPlan('sub-1', 'pro_6000', 'pro_25000', 'user-1')).resolves.toBe(
118-
true
118+
'pro_25000'
119119
)
120120
expect(dbChainMockFns.update).toHaveBeenCalled()
121121
})
122122

123123
it('writes a team plan onto an org-referenced subscription without an org lookup', async () => {
124124
await expect(syncSubscriptionPlan('sub-1', 'pro_6000', 'team_6000', 'org-1')).resolves.toBe(
125-
true
125+
'team_6000'
126126
)
127127
expect(dbChainMockFns.limit).not.toHaveBeenCalled()
128128
expect(dbChainMockFns.update).toHaveBeenCalled()
129129
})
130130

131-
it('refuses to write a pro plan onto an org-referenced subscription', async () => {
131+
it('refuses a pro plan on an org-referenced subscription and returns the current plan', async () => {
132132
dbChainMockFns.limit.mockResolvedValueOnce([{ id: 'org-1' }])
133133

134134
await expect(syncSubscriptionPlan('sub-1', 'team_6000', 'pro_6000', 'org-1')).resolves.toBe(
135-
false
135+
'team_6000'
136136
)
137137
expect(dbChainMockFns.update).not.toHaveBeenCalled()
138138
})
139139

140-
it('no-ops when the plan is unchanged or unresolved', async () => {
140+
it('returns the current plan when the Stripe plan is unchanged or unresolved', async () => {
141141
await expect(syncSubscriptionPlan('sub-1', 'team_6000', 'team_6000', 'org-1')).resolves.toBe(
142-
false
142+
'team_6000'
143+
)
144+
await expect(syncSubscriptionPlan('sub-1', 'team_6000', null, 'org-1')).resolves.toBe(
145+
'team_6000'
143146
)
144-
await expect(syncSubscriptionPlan('sub-1', 'team_6000', null, 'org-1')).resolves.toBe(false)
145147
expect(dbChainMockFns.update).not.toHaveBeenCalled()
146148
})
147149
})

apps/sim/lib/billing/core/subscription.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ export async function writeBillingInterval(
8989
/**
9090
* Sync the subscription's `plan` column to match Stripe. Closes a gap
9191
* where plan changes (Pro → Team upgrades, tier swaps) updated price,
92-
* seats, and referenceId at Stripe but left the DB plan stale. Returns
93-
* `true` if a write was issued, `false` if no change was needed.
92+
* seats, and referenceId at Stripe but left the DB plan stale.
9493
*
9594
* Enforces the billing invariant that organization-referenced
9695
* subscriptions only ever hold Team or Enterprise plans: when Stripe
@@ -99,6 +98,11 @@ export async function writeBillingInterval(
9998
* an error is logged so operators fix the price in Stripe — the DB row
10099
* never becomes an org-scoped Pro subscription.
101100
*
101+
* Returns the plan the DB row holds after the call. Callers must drive all
102+
* downstream processing (org ensure, seat sync, usage limits) from this
103+
* value — never from the raw Stripe plan — so a refused write cannot leak
104+
* the rejected plan into the rest of the webhook handler.
105+
*
102106
* The organization lookup is inlined rather than delegated to
103107
* `isSubscriptionOrgScoped` because that helper lives in `core/billing.ts`,
104108
* which imports this module — delegating would create an import cycle.
@@ -108,9 +112,9 @@ export async function syncSubscriptionPlan(
108112
currentPlan: string | null,
109113
planFromStripe: string | null,
110114
referenceId: string
111-
): Promise<boolean> {
112-
if (!planFromStripe) return false
113-
if (currentPlan === planFromStripe) return false
115+
): Promise<string | null> {
116+
if (!planFromStripe) return currentPlan
117+
if (currentPlan === planFromStripe) return currentPlan
114118

115119
if (!isOrgPlan(planFromStripe)) {
116120
const [referencedOrganization] = await db
@@ -129,7 +133,7 @@ export async function syncSubscriptionPlan(
129133
rejectedPlan: planFromStripe,
130134
}
131135
)
132-
return false
136+
return currentPlan
133137
}
134138
}
135139

@@ -144,7 +148,7 @@ export async function syncSubscriptionPlan(
144148
newPlan: planFromStripe,
145149
})
146150

147-
return true
151+
return planFromStripe
148152
}
149153

150154
/**

apps/sim/lib/billing/organizations/membership.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,113 @@ export async function restoreUserProSubscription(userId: string): Promise<Restor
312312
return result
313313
}
314314

315+
export interface PauseProForOrgCoverageResult {
316+
paused: boolean
317+
subscriptionId?: string
318+
organizationId?: string
319+
}
320+
321+
/**
322+
* Pause (cancel at period end) a user's entitled personal Pro subscription
323+
* because an organization with an entitled paid subscription already covers
324+
* them. Transactional fence behind the checkout admission guard: it closes
325+
* the race where a user starts a personal checkout while uncovered, joins a
326+
* paid org mid-checkout, and completes payment after the join.
327+
*
328+
* The user keeps the period they paid for and the subscription simply does
329+
* not renew — the same state a paid-org joiner's personal Pro enters, so the
330+
* billing UI already renders it. If they leave the org before period end,
331+
* `restoreUserProSubscription` un-pauses it automatically.
332+
*
333+
* Unlike `applyPaidOrgJoinBillingTx` (the join-time pause), this does NOT
334+
* snapshot or zero current-period usage: by the time this runs the user's
335+
* usage already attributes to the organization pool, and moving it into the
336+
* pro snapshot would undercount the org's period.
337+
*
338+
* Idempotent: no-ops when there is no entitled personal Pro, when it is
339+
* already pausing, or when no entitled paid org covers the user. All checks
340+
* re-run under the user billing identity lock and a `FOR UPDATE` read of the
341+
* personal subscription row — the same serialization points as the join and
342+
* restore paths.
343+
*/
344+
export async function pauseProSubscriptionForOrgCoverage(
345+
userId: string
346+
): Promise<PauseProForOrgCoverageResult> {
347+
const result: PauseProForOrgCoverageResult = { paused: false }
348+
349+
await db.transaction(async (tx) => {
350+
await acquireUserBillingIdentityLock(tx, userId)
351+
352+
const [personalPro] = await tx
353+
.select()
354+
.from(subscriptionTable)
355+
.where(
356+
and(
357+
eq(subscriptionTable.referenceId, userId),
358+
inArray(subscriptionTable.status, ENTITLED_SUBSCRIPTION_STATUSES),
359+
sqlIsPro(subscriptionTable.plan)
360+
)
361+
)
362+
.for('update')
363+
.limit(1)
364+
365+
if (!personalPro || personalPro.cancelAtPeriodEnd) return
366+
367+
const memberships = await tx
368+
.select({ organizationId: member.organizationId })
369+
.from(member)
370+
.where(eq(member.userId, userId))
371+
if (memberships.length === 0) return
372+
373+
const organizationSubscriptions = await tx
374+
.select({ plan: subscriptionTable.plan, referenceId: subscriptionTable.referenceId })
375+
.from(subscriptionTable)
376+
.where(
377+
and(
378+
inArray(
379+
subscriptionTable.referenceId,
380+
memberships.map((membership) => membership.organizationId)
381+
),
382+
inArray(subscriptionTable.status, ENTITLED_SUBSCRIPTION_STATUSES)
383+
)
384+
)
385+
const coveringSubscription = organizationSubscriptions.find((organizationSubscription) =>
386+
isPaid(organizationSubscription.plan)
387+
)
388+
if (!coveringSubscription) return
389+
390+
await tx
391+
.update(subscriptionTable)
392+
.set({ cancelAtPeriodEnd: true })
393+
.where(eq(subscriptionTable.id, personalPro.id))
394+
395+
if (personalPro.stripeSubscriptionId) {
396+
await enqueueOutboxEvent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, {
397+
stripeSubscriptionId: personalPro.stripeSubscriptionId,
398+
subscriptionId: personalPro.id,
399+
reason: 'covered-by-organization',
400+
})
401+
}
402+
403+
result.paused = true
404+
result.subscriptionId = personalPro.id
405+
result.organizationId = coveringSubscription.referenceId
406+
})
407+
408+
if (result.paused) {
409+
logger.warn(
410+
'Paused personal Pro created while covered by an organization subscription (kept until period end, Stripe sync queued)',
411+
{
412+
userId,
413+
subscriptionId: result.subscriptionId,
414+
organizationId: result.organizationId,
415+
}
416+
)
417+
}
418+
419+
return result
420+
}
421+
315422
export interface AddMemberParams {
316423
userId: string
317424
organizationId: string

0 commit comments

Comments
 (0)