Skip to content

Commit 775b120

Browse files
committed
fix(sso): re-grant provider trust when an already-verified domain is re-submitted
1 parent ab25755 commit 775b120

2 files changed

Lines changed: 43 additions & 12 deletions

File tree

apps/sim/app/api/organizations/[id]/domains/[domainId]/verify/route.test.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,12 +143,29 @@ describe('verify org domain route', () => {
143143
expect(grantWhere).toBeDefined()
144144
})
145145

146-
it('does not grant trust when the conditional update matched no row', async () => {
146+
/**
147+
* A provider can hold a verified domain while its own trust flag is off, after
148+
* an update whose grant was refused reverted the config and cleared it. Re-running
149+
* verification is the obvious recovery, so an already-verified domain must still
150+
* re-grant instead of returning success having done nothing.
151+
*/
152+
it('re-grants trust when the domain is already verified', async () => {
147153
queueAdminWithPendingRow()
148154
queueTableRows(ssoDomain, [])
149-
dbChainMockFns.returning.mockResolvedValueOnce([]) // lost the race
150-
queueTableRows(ssoDomain, [{ ...PENDING_ROW, status: 'verified' }])
151-
await POST(createMockRequest('POST'), routeContext)
155+
dbChainMockFns.returning.mockResolvedValueOnce([]) // conditional update matched nothing
156+
queueTableRows(ssoDomain, [{ ...PENDING_ROW, status: 'verified' }]) // re-read: verified
157+
const res = await POST(createMockRequest('POST'), routeContext)
158+
expect(res.status).toBe(200)
159+
expect(dbChainMockFns.set).toHaveBeenCalledWith({ domainVerified: true })
160+
})
161+
162+
it('does not grant trust when the challenge is genuinely stale', async () => {
163+
queueAdminWithPendingRow()
164+
queueTableRows(ssoDomain, [])
165+
dbChainMockFns.returning.mockResolvedValueOnce([]) // conditional update matched nothing
166+
queueTableRows(ssoDomain, []) // re-read: row deleted or re-tokenized
167+
const res = await POST(createMockRequest('POST'), routeContext)
168+
expect(res.status).toBe(409)
152169
expect(dbChainMockFns.set).not.toHaveBeenCalledWith({ domainVerified: true })
153170
})
154171

apps/sim/app/api/organizations/[id]/domains/[domainId]/verify/route.ts

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,17 @@ export const POST = withRouteHandler(
101101
// instead of mapping an undefined row or trusting a superseded challenge. A
102102
// concurrent cross-org verification trips the partial unique index; surface
103103
// that as a 409 rather than an unhandled 500.
104+
/**
105+
* Providers this proof covers. Normalized the way migration 0268 stored these
106+
* rows (lower, trimmed, leading `*.` dropped) and identical to the expression
107+
* the deletion path revokes with, so granting and revoking can never diverge.
108+
*/
109+
const providersOnDomain = (verifiedDomain: string) =>
110+
and(
111+
eq(ssoProvider.organizationId, organizationId),
112+
sql`lower(regexp_replace(btrim(${ssoProvider.domain}), '^\\*\\.', '')) = ${verifiedDomain}`
113+
)
114+
104115
let updated: (typeof row)[]
105116
try {
106117
updated = await db.transaction(async (tx) => {
@@ -118,18 +129,12 @@ export const POST = withRouteHandler(
118129

119130
// Restore trust this proof covers, mirroring the revocation on delete.
120131
// Without it a delete-then-reverify leaves the provider untrusted, and
121-
// since that flag gates sign-in the org sits in a silent SSO outage. The
122-
// comparison matches the revoking one exactly so the two stay symmetric.
132+
// since that flag gates sign-in the org sits in a silent SSO outage.
123133
if (flipped.length > 0) {
124134
await tx
125135
.update(ssoProvider)
126136
.set({ domainVerified: true })
127-
.where(
128-
and(
129-
eq(ssoProvider.organizationId, organizationId),
130-
sql`lower(regexp_replace(btrim(${ssoProvider.domain}), '^\\*\\.', '')) = ${flipped[0].domain}`
131-
)
132-
)
137+
.where(providersOnDomain(flipped[0].domain))
133138
}
134139

135140
return flipped
@@ -155,6 +160,15 @@ export const POST = withRouteHandler(
155160
.where(and(eq(ssoDomain.id, domainId), eq(ssoDomain.organizationId, organizationId)))
156161
.limit(1)
157162
if (current?.status === 'verified') {
163+
// Re-grant rather than returning early. A provider can hold a verified
164+
// domain while its own trust flag is off — an update whose grant was
165+
// refused reverts to the previous config and clears it. Re-running
166+
// verification is the obvious way to fix that, so it must actually do
167+
// something; the proof is present, which is exactly what authorizes this.
168+
await db
169+
.update(ssoProvider)
170+
.set({ domainVerified: true })
171+
.where(providersOnDomain(current.domain))
158172
return NextResponse.json({ success: true, data: { domain: toDomainResponse(current) } })
159173
}
160174
return NextResponse.json(

0 commit comments

Comments
 (0)