Skip to content

Commit f14eac2

Browse files
committed
fix(billing): gate subscription transfer noop behind membership check
Previously the 'already belongs to this organization' early return fired before the org/member lookups, letting any authenticated caller probe sub-to-org pairings without being a member of the target org. Move the noop check after the admin/owner verification so unauthorized callers hit the 403 first.
1 parent 2dc98a3 commit f14eac2

File tree

2 files changed

+35
-15
lines changed

2 files changed

+35
-15
lines changed

apps/sim/app/api/users/me/subscription/[id]/transfer/route.test.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,12 @@ describe('POST /api/users/me/subscription/[id]/transfer', () => {
8383
})
8484

8585
it('treats an already-transferred organization subscription as a successful no-op', async () => {
86-
dbChainMockFns.for.mockResolvedValueOnce([
87-
{ id: 'sub-1', referenceId: 'org-1', plan: 'team', status: 'active' },
88-
])
86+
dbChainMockFns.for
87+
.mockResolvedValueOnce([
88+
{ id: 'sub-1', referenceId: 'org-1', plan: 'team', status: 'active' },
89+
])
90+
.mockResolvedValueOnce([{ id: 'org-1' }])
91+
dbChainMockFns.limit.mockResolvedValueOnce([{ role: 'owner' }])
8992

9093
const response = await makeRequest({ organizationId: 'org-1' })
9194

@@ -97,6 +100,23 @@ describe('POST /api/users/me/subscription/[id]/transfer', () => {
97100
expect(dbChainMockFns.update).not.toHaveBeenCalled()
98101
})
99102

103+
it('rejects the noop probe when the requester is not a member of the target organization', async () => {
104+
dbChainMockFns.for
105+
.mockResolvedValueOnce([
106+
{ id: 'sub-1', referenceId: 'org-1', plan: 'team', status: 'active' },
107+
])
108+
.mockResolvedValueOnce([{ id: 'org-1' }])
109+
dbChainMockFns.limit.mockResolvedValueOnce([])
110+
111+
const response = await makeRequest({ organizationId: 'org-1' })
112+
113+
expect(response.status).toBe(403)
114+
await expect(response.json()).resolves.toEqual({
115+
error: 'Unauthorized - user is not admin of organization',
116+
})
117+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
118+
})
119+
100120
it('rejects the transfer when the target organization already has an active subscription', async () => {
101121
dbChainMockFns.for
102122
.mockResolvedValueOnce([

apps/sim/app/api/users/me/subscription/[id]/transfer/route.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,18 +80,6 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
8080
}
8181
}
8282

83-
if (sub.referenceId === organizationId) {
84-
return { kind: 'noop', message: 'Subscription already belongs to this organization' }
85-
}
86-
87-
if (sub.referenceId !== userId) {
88-
return {
89-
kind: 'error',
90-
status: 403,
91-
error: 'Unauthorized - subscription does not belong to user',
92-
}
93-
}
94-
9583
const [org] = await tx
9684
.select({ id: organization.id })
9785
.from(organization)
@@ -116,6 +104,18 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
116104
}
117105
}
118106

107+
if (sub.referenceId === organizationId) {
108+
return { kind: 'noop', message: 'Subscription already belongs to this organization' }
109+
}
110+
111+
if (sub.referenceId !== userId) {
112+
return {
113+
kind: 'error',
114+
status: 403,
115+
error: 'Unauthorized - subscription does not belong to user',
116+
}
117+
}
118+
119119
const [existingOrgSub] = await tx
120120
.select({ id: subscription.id })
121121
.from(subscription)

0 commit comments

Comments
 (0)