Skip to content

Commit fea201b

Browse files
Bill Leoutsakoscursoragent
authored andcommitted
fix(auth): serialize SSO configuration mutations
Block provider and verification-token changes while callback or DNS verification operations are active so in-flight authentication observes a stable configuration. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent e37ad74 commit fea201b

4 files changed

Lines changed: 48 additions & 1 deletion

File tree

apps/sim/app/api/auth/sso/providers/[id]/domain-verification/request/route.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
getManagedSSOProvider,
1414
ssoManagementErrorResponse,
1515
} from '@/lib/auth/sso/management'
16+
import { assertNoActiveSSOProviderOperations } from '@/lib/auth/sso/provider-operation-intent'
1617
import { env, isTruthy } from '@/lib/core/config/env'
1718
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1819

@@ -40,6 +41,7 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Route
4041
const provider = await getManagedSSOProvider(parsed.data.params.id, session.user.id, {
4142
requireCreator: true,
4243
})
44+
await assertNoActiveSSOProviderOperations(provider.providerId)
4345
const result = await auth.api.requestDomainVerification({
4446
body: { providerId: provider.providerId },
4547
headers: collectAuthHeaders(request),

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
77
const {
88
dbState,
99
memberTable,
10+
mockAssertNoActiveSSOProviderOperations,
1011
mockGetSession,
1112
mockIsOrganizationOnEnterprisePlan,
1213
mockRequestDomainVerification,
@@ -24,6 +25,7 @@ const {
2425
organizationId: 'member.organizationId',
2526
role: 'member.role',
2627
},
28+
mockAssertNoActiveSSOProviderOperations: vi.fn(),
2729
mockGetSession: vi.fn(),
2830
mockIsOrganizationOnEnterprisePlan: vi.fn(),
2931
mockRequestDomainVerification: vi.fn(),
@@ -81,6 +83,7 @@ vi.mock('@/lib/auth', () => ({
8183
}))
8284

8385
vi.mock('@/lib/auth/sso/provider-operation-intent', () => ({
86+
assertNoActiveSSOProviderOperations: mockAssertNoActiveSSOProviderOperations,
8487
withSSODomainVerificationIntent: mockWithSSODomainVerificationIntent,
8588
}))
8689

@@ -92,6 +95,7 @@ vi.mock('@/lib/core/config/env', () =>
9295
createEnvMock({ SSO_ENABLED: 'true', SSO_DOMAIN_VERIFICATION_ENABLED: 'true' })
9396
)
9497

98+
import { SSOManagementError } from '@/lib/auth/sso/management'
9599
import { POST as requestVerification } from '@/app/api/auth/sso/providers/[id]/domain-verification/request/route'
96100
import { POST as verifyDomain } from '@/app/api/auth/sso/providers/[id]/domain-verification/verify/route'
97101

@@ -158,6 +162,23 @@ describe('SSO domain verification façades', () => {
158162
})
159163
})
160164

165+
it('does not replace the verification token while verification is in progress', async () => {
166+
mockAssertNoActiveSSOProviderOperations.mockRejectedValueOnce(
167+
new SSOManagementError(
168+
'An SSO operation is currently completing for this provider. Try again shortly.',
169+
409,
170+
'SSO_OPERATION_IN_PROGRESS'
171+
)
172+
)
173+
174+
const response = await requestVerification(createMockRequest('POST'), context)
175+
176+
expect(response.status).toBe(409)
177+
await expect(response.json()).resolves.toMatchObject({ code: 'SSO_OPERATION_IN_PROGRESS' })
178+
expect(mockAssertNoActiveSSOProviderOperations).toHaveBeenCalledWith('acme-saml')
179+
expect(mockRequestDomainVerification).not.toHaveBeenCalled()
180+
})
181+
161182
it('delegates DNS verification to Better Auth', async () => {
162183
const response = await verifyDomain(createMockRequest('POST'), context)
163184
expect(response.status).toBe(200)

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,30 @@ describe('/api/auth/sso/providers/[id]', () => {
221221
expect(mockUpdateSSOProvider).not.toHaveBeenCalled()
222222
})
223223

224+
it('rejects a configuration-only update while an SSO callback is in progress', async () => {
225+
mockAssertNoActiveSSOProviderOperations.mockRejectedValueOnce(
226+
new SSOManagementError(
227+
'An SSO operation is currently completing for this provider. Try again shortly.',
228+
409,
229+
'SSO_OPERATION_IN_PROGRESS'
230+
)
231+
)
232+
233+
const response = await PATCH(
234+
createMockRequest('PATCH', {
235+
...SAML_UPDATE,
236+
issuer: PROVIDER.issuer,
237+
domain: PROVIDER.domain,
238+
}),
239+
context
240+
)
241+
242+
expect(response.status).toBe(409)
243+
await expect(response.json()).resolves.toMatchObject({ code: 'SSO_OPERATION_IN_PROGRESS' })
244+
expect(mockAssertNoActiveSSOProviderOperations).toHaveBeenCalledWith('acme-saml')
245+
expect(mockUpdateSSOProvider).not.toHaveBeenCalled()
246+
})
247+
224248
it('rejects a stale update when the provider changes before lock acquisition', async () => {
225249
mockWithSSOProviderMutationLock.mockImplementationOnce(
226250
async (callback: () => Promise<unknown>) => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Rout
9292
}
9393

9494
const currentDomain = requireNormalizedSSODomain(body.domain, currentProvider.domain)
95+
await assertNoActiveSSOProviderOperations(currentProvider.providerId)
9596
if (currentDomain !== currentProvider.domain || body.issuer !== currentProvider.issuer) {
96-
await assertNoActiveSSOProviderOperations(currentProvider.providerId)
9797
await assertSSOProviderHasNoAccountLinks(currentProvider.providerId)
9898
}
9999
await assertSSOProviderAvailable({

0 commit comments

Comments
 (0)