Skip to content

Commit 5481f44

Browse files
Bill Leoutsakoscursoragent
authored andcommitted
serialize sso provider mutations
Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 0094304 commit 5481f44

5 files changed

Lines changed: 122 additions & 6 deletions

File tree

apps/sim/app/api/auth/sso/providers/[id]/route.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const {
1212
mockGetSession,
1313
mockIsOrganizationOnEnterprisePlan,
1414
mockUpdateSSOProvider,
15+
mockWithSSOProviderMutationLock,
1516
ssoProviderTable,
1617
} = vi.hoisted(() => ({
1718
accountTable: {
@@ -32,6 +33,7 @@ const {
3233
mockGetSession: vi.fn(),
3334
mockIsOrganizationOnEnterprisePlan: vi.fn(),
3435
mockUpdateSSOProvider: vi.fn(),
36+
mockWithSSOProviderMutationLock: vi.fn((callback: () => Promise<unknown>) => callback()),
3537
ssoProviderTable: {
3638
id: 'sso.id',
3739
issuer: 'sso.issuer',
@@ -74,6 +76,7 @@ vi.mock('@sim/db', () => ({
7476
account: accountTable,
7577
member: memberTable,
7678
ssoProvider: ssoProviderTable,
79+
withSSOProviderMutationLock: mockWithSSOProviderMutationLock,
7780
}))
7881

7982
vi.mock('@/lib/auth', () => ({
@@ -167,6 +170,33 @@ describe('/api/auth/sso/providers/[id]', () => {
167170
)
168171
})
169172

173+
it('re-checks domain overlap inside the mutation lock before updating', async () => {
174+
mockWithSSOProviderMutationLock.mockImplementationOnce(
175+
async (callback: () => Promise<unknown>) => {
176+
dbState.providers = [
177+
PROVIDER,
178+
{
179+
id: 'concurrent',
180+
issuer: 'https://other-idp.example.com',
181+
domain: 'login.new.example.com',
182+
oidcConfig: '{}',
183+
samlConfig: null,
184+
userId: 'creator-2',
185+
providerId: 'concurrent-provider',
186+
organizationId: 'org-2',
187+
},
188+
]
189+
return callback()
190+
}
191+
)
192+
193+
const response = await PATCH(createMockRequest('PATCH', SAML_UPDATE), context)
194+
195+
expect(response.status).toBe(409)
196+
expect(mockWithSSOProviderMutationLock).toHaveBeenCalledOnce()
197+
expect(mockUpdateSSOProvider).not.toHaveBeenCalled()
198+
})
199+
170200
it('reports the compatibility-active state while verification enforcement is disabled', async () => {
171201
const previousValue = env.SSO_DOMAIN_VERIFICATION_ENABLED
172202
env.SSO_DOMAIN_VERIFICATION_ENABLED = undefined

apps/sim/app/api/auth/sso/providers/[id]/route.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { withSSOProviderMutationLock } from '@sim/db'
12
import { createLogger } from '@sim/logger'
23
import { getErrorMessage } from '@sim/utils/errors'
34
import type { NextRequest } from 'next/server'
@@ -68,9 +69,17 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Rout
6869
existingDomain: provider.domain,
6970
}
7071
)
71-
const updated = await auth.api.updateSSOProvider({
72-
body: providerConfig,
73-
headers: collectAuthHeaders(request),
72+
const updated = await withSSOProviderMutationLock(async () => {
73+
await assertSSOProviderAvailable({
74+
providerId: provider.providerId,
75+
domain,
76+
organizationId: provider.organizationId!,
77+
excludeRowId: provider.id,
78+
})
79+
return auth.api.updateSSOProvider({
80+
body: providerConfig,
81+
headers: collectAuthHeaders(request),
82+
})
7483
})
7584

7685
logger.info('SSO provider updated', {

apps/sim/app/api/auth/sso/register/route.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const {
1212
mockRegisterSSOProvider,
1313
mockSecureFetchWithPinnedIP,
1414
mockValidateUrlWithDNS,
15+
mockWithSSOProviderMutationLock,
1516
ssoProviderTable,
1617
} = vi.hoisted(() => ({
1718
dbState: {
@@ -28,6 +29,7 @@ const {
2829
mockRegisterSSOProvider: vi.fn(),
2930
mockSecureFetchWithPinnedIP: vi.fn(),
3031
mockValidateUrlWithDNS: vi.fn(),
32+
mockWithSSOProviderMutationLock: vi.fn((callback: () => Promise<unknown>) => callback()),
3133
ssoProviderTable: {
3234
id: 'sso.id',
3335
providerId: 'sso.providerId',
@@ -58,6 +60,7 @@ vi.mock('@sim/db', () => ({
5860
},
5961
member: memberTable,
6062
ssoProvider: ssoProviderTable,
63+
withSSOProviderMutationLock: mockWithSSOProviderMutationLock,
6164
}))
6265

6366
vi.mock('@/lib/auth', () => ({
@@ -196,6 +199,28 @@ describe('POST /api/auth/sso/register', () => {
196199
expect(mockRegisterSSOProvider).not.toHaveBeenCalled()
197200
})
198201

202+
it('re-checks domain overlap inside the mutation lock after configuration work', async () => {
203+
mockWithSSOProviderMutationLock.mockImplementationOnce(
204+
async (callback: () => Promise<unknown>) => {
205+
dbState.providers = [
206+
{
207+
id: 'concurrent',
208+
providerId: 'concurrent-provider',
209+
domain: 'login.acme.com',
210+
organizationId: 'org-concurrent',
211+
},
212+
]
213+
return callback()
214+
}
215+
)
216+
217+
const response = await POST(request(OIDC_BODY))
218+
219+
expect(response.status).toBe(409)
220+
expect(mockWithSSOProviderMutationLock).toHaveBeenCalledOnce()
221+
expect(mockRegisterSSOProvider).not.toHaveBeenCalled()
222+
})
223+
199224
it('enforces one provider per organization', async () => {
200225
dbState.providers = [
201226
{

apps/sim/app/api/auth/sso/register/route.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { withSSOProviderMutationLock } from '@sim/db'
12
import { createLogger } from '@sim/logger'
23
import { getErrorMessage } from '@sim/utils/errors'
34
import type { NextRequest } from 'next/server'
@@ -61,9 +62,16 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
6162
organizationId: body.orgId,
6263
}
6364
)
64-
const registration = await auth.api.registerSSOProvider({
65-
body: providerConfig,
66-
headers: collectAuthHeaders(request),
65+
const registration = await withSSOProviderMutationLock(async () => {
66+
await assertSSOProviderAvailable({
67+
providerId: body.providerId,
68+
domain,
69+
organizationId: body.orgId,
70+
})
71+
return auth.api.registerSSOProvider({
72+
body: providerConfig,
73+
headers: collectAuthHeaders(request),
74+
})
6775
})
6876

6977
logger.info('SSO provider registered', {

packages/db/db.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,50 @@ const postgresClient = instrumentPoolClient(
4747

4848
export const db = drizzle(postgresClient, { schema })
4949

50+
const SSO_PROVIDER_MUTATION_LOCK_KEY = 4_961_002_271n
51+
let ssoProviderMutationTail = Promise.resolve()
52+
53+
/**
54+
* Serializes SSO provider create/update mutations across app processes.
55+
*
56+
* Parent/child domain overlap cannot be represented by a normal unique index,
57+
* so callers must re-check availability while holding this lock and keep it
58+
* through the Better Auth write. The lock uses a reserved connection and a
59+
* transaction-scoped advisory lock so it remains safe behind transaction-mode
60+
* poolers and is released automatically if the callback or process fails.
61+
*
62+
* The per-process queue prevents concurrent requests from reserving multiple
63+
* primary-pool connections while they wait for the same cross-process lock.
64+
*/
65+
export async function withSSOProviderMutationLock<T>(callback: () => Promise<T>): Promise<T> {
66+
const predecessor = ssoProviderMutationTail
67+
let releaseLocalLock!: () => void
68+
ssoProviderMutationTail = new Promise<void>((resolve) => {
69+
releaseLocalLock = resolve
70+
})
71+
72+
await predecessor
73+
try {
74+
const reserved = await postgresClient.reserve()
75+
try {
76+
await reserved.unsafe('BEGIN')
77+
try {
78+
await reserved.unsafe(`SELECT pg_advisory_xact_lock(${SSO_PROVIDER_MUTATION_LOCK_KEY})`)
79+
const result = await callback()
80+
await reserved.unsafe('COMMIT')
81+
return result
82+
} catch (error) {
83+
await reserved.unsafe('ROLLBACK')
84+
throw error
85+
}
86+
} finally {
87+
reserved.release()
88+
}
89+
} finally {
90+
releaseLocalLock()
91+
}
92+
}
93+
5094
/**
5195
* Opt-in read-replica client for reads that tolerate bounded staleness and have
5296
* no read-your-writes dependency (logs, exports, dashboard aggregations). Never

0 commit comments

Comments
 (0)