fix(sso): make domain-trust grants atomic and propagate revocation

Greptile flagged that the ownership check and the domainVerified write were
separate statements, so a domain deleted between them still ended with trust
granted. Two changes close it from both sides.

The grant now folds the ownership test into the UPDATE's WHERE clause, so
Postgres evaluates both in one statement and the write matches nothing once the
proof is gone.

Removing a verified domain now clears domainVerified for providers on that
domain, in the same transaction as the delete. This was a standing gap, not just
a race: deleting a domain previously left linking trust set indefinitely.

Together the provider cannot end up trusted without current ownership in either
commit order — if the grant lands first the delete clears it, and if the delete
lands first the grant no-ops.

Author: waleedlatif1

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

@@ -2,7 +2,7 @@ import { db, member, ssoDomain, ssoProvider } from '@sim/db'
 import { createLogger } from '@sim/logger'
 import { getErrorMessage } from '@sim/utils/errors'
 import { normalizeSSODomain } from '@sim/utils/sso-domain'
-import { and, eq, isNull, sql } from 'drizzle-orm'
+import { and, eq, exists, isNull, sql } from 'drizzle-orm'
 import { type NextRequest, NextResponse } from 'next/server'
 import { ssoRegistrationContract } from '@/lib/api/contracts/auth'
 import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
@@ -656,6 +656,46 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
       await db.update(ssoProvider).set({ domainVerified: verified }).where(ownerClause)
     }
 
+    /**
+     * Grants domain trust with the ownership test folded into the UPDATE's WHERE
+     * clause, so Postgres evaluates both in one statement and the write simply
+     * matches nothing once the proof is gone. That removes the window a separate
+     * read-then-write leaves open.
+     *
+     * Together with the domain-delete route — which clears this flag in the same
+     * transaction that removes the proof — the provider cannot end up trusted
+     * without current ownership in either commit order: if this write lands first
+     * the delete clears it, and if the delete lands first this write no-ops.
+     * Org-less (personal) SSO is not domain-gated by Sim, so it grants
+     * unconditionally as it always has.
+     */
+    const grantProviderDomainTrust = async () => {
+      if (!orgId) {
+        await setProviderDomainVerified(true)
+        return
+      }
+      await db
+        .update(ssoProvider)
+        .set({ domainVerified: true })
+        .where(
+          and(
+            ownerClause,
+            exists(
+              db
+                .select({ one: sql`1` })
+                .from(ssoDomain)
+                .where(
+                  and(
+                    eq(ssoDomain.organizationId, orgId),
+                    eq(ssoDomain.domain, domain),
+                    eq(ssoDomain.status, 'verified')
+                  )
+                )
+            )
+          )
+        )
+    }
+
     if (existingOwnedProvider) {
       await auth.api.updateSSOProvider({
         body: {
@@ -668,11 +708,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
         headers,
       })
 
-      // Compensating re-check, mirroring the create path below: the verified
-      // sso_domain row can be deleted while updateSSOProvider is in flight.
-      // Granting trust here would re-authorize same-email account linking for a
-      // domain the org no longer proves it owns, so clear the flag instead —
-      // that both denies linking and blocks sign-in until it is re-verified.
+      // The verified sso_domain row can be deleted while updateSSOProvider is in
+      // flight. There is no newly-created row to roll back here, so on failure
+      // clear the flag: `updateSSOProvider` only resets it when the domain
+      // changes, so a same-domain edit would otherwise leave stale trust standing.
       if (orgId && !(await isOrgDomainVerified())) {
         await setProviderDomainVerified(false)
         logger.warn('Revoked SSO domain trust: verification was removed mid-update', {
@@ -684,7 +723,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
         return domainNotVerifiedResponse()
       }
 
-      await setProviderDomainVerified(true)
+      await grantProviderDomainTrust()
       logger.info('SSO provider updated successfully', { providerId, providerType, domain })
       return NextResponse.json({
         success: true,
@@ -699,9 +738,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
       headers,
     })
 
-    // Close the residual TOCTOU between the re-check above and Better Auth
-    // persisting the provider: the verified sso_domain row could be removed in
-    // that window. registerSSOProvider is create-only (it throws if the
+    // Grant trust in the same statement that re-tests ownership, closing the
+    // window between Better Auth persisting the provider and this write. A failure
+    // means the verified sso_domain row was removed in that window, so roll the
+    // provider back. registerSSOProvider is create-only (it throws if the
     // providerId already exists), so a successful call always created a brand-new
     // row — we roll it back by its primary-key `id` (not the logical providerId,
     // which a concurrent delete+recreate could point at a different row). Personal
@@ -734,7 +774,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
       return domainNotVerifiedResponse()
     }
 
-    await setProviderDomainVerified(true)
+    await grantProviderDomainTrust()
 
     logger.info('SSO provider registered successfully', {
       providerId,

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

@@ -83,4 +83,25 @@ describe('remove org domain route', () => {
       expect.objectContaining({ action: 'organization.domain.removed' })
     )
   })
+
+  /**
+   * `domainVerified` on a provider is what authorizes auto-linking an SSO sign-in
+   * to an existing same-email account. Removing the proof has to withdraw that
+   * trust in the same transaction, or the authorization outlives the ownership.
+   */
+  it('revokes SSO domain trust for providers on the removed domain', async () => {
+    queueTableRows(member, [{ role: 'owner' }])
+    dbChainMockFns.returning.mockResolvedValueOnce([{ domain: 'acme.com' }])
+    const res = await DELETE(createMockRequest('DELETE'), routeContext)
+    expect(res.status).toBe(200)
+    expect(dbChainMockFns.set).toHaveBeenCalledWith({ domainVerified: false })
+  })
+
+  it('does not revoke trust when no domain was removed', async () => {
+    queueTableRows(member, [{ role: 'owner' }])
+    dbChainMockFns.returning.mockResolvedValueOnce([]) // delete matched nothing
+    const res = await DELETE(createMockRequest('DELETE'), routeContext)
+    expect(res.status).toBe(404)
+    expect(dbChainMockFns.set).not.toHaveBeenCalled()
+  })
 })

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

@@ -1,9 +1,9 @@
 import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
 import { db } from '@sim/db'
-import { member, ssoDomain } from '@sim/db/schema'
+import { member, ssoDomain, ssoProvider } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { isOrgAdminRole } from '@sim/platform-authz/workspace'
-import { and, eq } from 'drizzle-orm'
+import { and, eq, sql } from 'drizzle-orm'
 import { type NextRequest, NextResponse } from 'next/server'
 import { removeOrganizationDomainContract } from '@/lib/api/contracts/organization'
 import { parseRequest } from '@/lib/api/server'
@@ -18,8 +18,10 @@ const logger = createLogger('OrgDomainDeleteAPI')
  * DELETE /api/organizations/[id]/domains/[domainId]
  * Removes a claimed/verified domain. Requires owner/admin role. Removing a
  * verified domain drops the ownership proof, so SSO can no longer be configured
- * for it until it is re-verified. It does not retroactively un-register an
- * already-configured SSO provider — that flows through the SSO provider itself.
+ * for it until it is re-verified, and any provider already on that domain loses
+ * its `domainVerified` trust in the same transaction. The provider itself is not
+ * un-registered — that flows through the SSO provider — but it can no longer
+ * auto-link sign-ins to existing accounts.
  */
 export const DELETE = withRouteHandler(
   async (request: NextRequest, context: { params: Promise<{ id: string; domainId: string }> }) => {
@@ -59,10 +61,31 @@ export const DELETE = withRouteHandler(
       )
     }
 
-    const [removed] = await db
-      .delete(ssoDomain)
-      .where(and(eq(ssoDomain.id, domainId), eq(ssoDomain.organizationId, organizationId)))
-      .returning({ domain: ssoDomain.domain })
+    // Removing the proof must also withdraw the trust it granted. `domainVerified`
+    // on a provider is what authorizes auto-linking an SSO sign-in to an existing
+    // same-email account, so leaving it set would keep that authorization alive
+    // indefinitely after ownership was revoked. Both writes share a transaction so
+    // a domain can never be gone while its provider still claims to be verified.
+    const removed = await db.transaction(async (tx) => {
+      const [deleted] = await tx
+        .delete(ssoDomain)
+        .where(and(eq(ssoDomain.id, domainId), eq(ssoDomain.organizationId, organizationId)))
+        .returning({ domain: ssoDomain.domain })
+
+      if (!deleted) return null
+
+      await tx
+        .update(ssoProvider)
+        .set({ domainVerified: false })
+        .where(
+          and(
+            eq(ssoProvider.organizationId, organizationId),
+            sql`lower(${ssoProvider.domain}) = ${deleted.domain}`
+          )
+        )
+
+      return deleted
+    })
 
     if (!removed) {
       return NextResponse.json({ error: 'Domain not found' }, { status: 404 })