Skip to content

Commit 513292f

Browse files
authored
feat(sso): DNS domain verification gating org SSO registration (#5909)
* feat(sso): DNS domain verification gating org SSO registration Add org-scoped domain ownership verification (DNS TXT challenge) as the security precondition for configuring SSO. Closes the first-come domain-claim vuln where any org could wire another company's domain to its own IdP. - New sso_domain table + migration 0266; existing org SSO domains are grandfathered as verified so live tenants are unaffected - Verified-domains settings UI (enterprise-gated) with add/verify/remove - Register route now requires a verified domain for org-scoped registration; personal SSO and already-grandfathered domains are unaffected - Self-host register script writes the verified sso_domain row directly, so script-driven registration stays backwards compatible * fix(sso): harden domain verification against concurrency + fix CI lint Addresses review findings on state invariants under concurrent/failed writes: - Add unique index on (organization_id, domain) so concurrent claims can't create duplicate pending rows; POST re-reads and stays idempotent on conflict - Verify flips the row only if it's still the exact pending challenge checked (guards deletion/token-rotation mid-DNS-lookup) and maps the partial unique index violation to 409 instead of an unhandled 500 - Wrap the self-host script's provider write + verified-domain upsert in a transaction so a failed ownership write can't leave a provider committed - Format 0266 snapshot/journal with biome (fixes @sim/db lint:check) * fix(sso): re-check domain verification before provider write (TOCTOU) The register gate checked the verified sso_domain row only at handler entry, then ran OIDC discovery before writing the provider. A verified row removed during that window could still complete registration. Extract the check into a closure and call it both as an entry fast-fail and authoritatively right before registerSSOProvider, alongside the existing domain-conflict re-check. * fix(sso): stop rotating verification token on idempotent re-add Re-adding a pending domain rotated its verification token, which invalidated a TXT record the admin may have already published and — under two concurrent re-adds — could return a token the racing write had already superseded, so the admin's DNS record would never verify. Return the existing row unchanged instead; the pending token is always shown in the UI, so it is never lost. * fix(sso): close register TOCTOU with compensating delete + harden edges Audit-driven hardening: - Close the residual register TOCTOU: registerSSOProvider is create-only (throws if the providerId exists), so a compensating delete after the write is provably safe — it can only remove the just-created row. If verification was revoked during the write, roll the provider back and 403. - Verify is now idempotent under concurrency: a same-org row already flipped to verified by a racing request returns 200, not a confusing 409. - Grandfather backfill + self-host script now match normalizeSSODomain's dominant transforms (lower + trim + strip leading wildcard) so a non-canonical legacy domain can't miss the runtime gate's lookup. Prod backfill result is unchanged. - Cleanup: drop dead default export, align card radius to sibling convention. * fix(sso): redact domain tokens from non-admins + fix script stale-update Round-5 review findings: - GET /domains redacted the pending TXT verification token (a management secret) to any org member. Now only owner/admins read it; members see the list and status without it. Non-Enterprise orgs get an empty list (entitlement flag only), never the domains/tokens. - Self-host script decided update-vs-insert from a read taken OUTSIDE the transaction; a provider deleted mid-flight made the UPDATE match zero rows silently while the verified-domain upsert still committed (orphaned domain). The decision now happens inside the transaction from the UPDATE's row count. * docs(sso): drop unshipped enforce-SSO / auto-join copy from verified domains Verified domains currently only gate SSO configuration. Remove the forward-looking references to enforcing SSO and auto-joining members (deferred to a later release) from the docs, settings copy, nav description, and schema comment so we don't promise unshipped features. * fix(sso): guard rollback to new providers only + Enterprise-gate domain removal Round-6 review findings: - The compensating provider rollback now only fires when the provider did not exist before this request (providerExistedBefore). registerSSOProvider is create-only today so reaching the rollback already implies a fresh create, but this makes the safety local and future-proof: if Better Auth ever allowed updating an existing provider, a revoked-verification rollback must not delete that pre-existing row. - DELETE /domains now requires an Enterprise plan like add/list/verify, so all domain mutations share one entitlement (the UI already hides removal from non-Enterprise orgs). Adds a delete-route test. * fix(sso): roll back the SSO provider by row id, not logical keys The compensating rollback deleted by (providerId, orgId). providerId is unique, so if this request's row were deleted and recreated by a concurrent registration in the narrow window before the rollback, the logical-key delete would remove that other request's provider. Delete by the primary-key id registerSSOProvider returns instead, so only the exact row this request created is ever removed. * chore(sso): final-review polish — trim script read, unify copy, doc migration edge Cosmetic cleanup from a final 4-track adversarial review (no bugs found in the new logic): - Self-host script: narrow the pre-transaction existence read to select({ id }) instead of SELECT * (it only feeds a log line now). - Unify invalid-domain copy ("for example acme.com") and the verified-elsewhere 409 wording ("is already verified by another organization") across routes. - p-3 shorthand on the domain row card. - Document the migration's rare two-orgs-share-a-domain grandfather behavior (login unaffected; validated no such duplicates in prod). * fix(sso): apply attribute mapping + make SSO edit work; drop dead guard Two pre-existing SSO bugs the final review surfaced (prod has one SSO org, RVW, script-registered with the default mapping, so neither change affects it): - Attribute mapping was passed at the top level of the register payload, which Better Auth ignores — it reads oidcConfig.mapping / samlConfig.mapping. Nest it so custom mappings actually apply. (Default mapping is unchanged, so existing logins are unaffected.) - Editing an SSO provider was broken: registerSSOProvider is create-only and threw on the existing providerId → generic 500. Route now detects a provider the caller already owns and updates it via Better Auth's updateSSOProvider, and surfaces Better Auth's own error status/message instead of a blanket 500. Also drops the now-unnecessary providerExistedBefore guard (the rollback deletes by the created row's primary-key id and register is create-only) and the earlier final-review polish (script read, unified copy, migration edge note). Smoke-test SSO login + edit on staging before merge (auth-path change). * fix(sso): require null org on personal-mode provider lookups (gate bypass) The personal branch of both provider-ownership lookups keyed on (providerId, userId) without requiring organizationId IS NULL. Because org providers store userId = their creator and providerId is globally unique, an org admin could send a personal-mode request (no orgId) — which skips the membership check and the domain-verification gate — yet still match, and then via the new update path move, their org's provider to an unverified domain. Add isNull(organizationId) to the personal branch of both clauses so it can only match a genuinely personal provider, matching the route's own isOwnedByCaller. Found by an adversarial review of the update path added in 394bda9. * fix(sso): script updates the observed provider by id, not providerId Inside the registration transaction the script updated WHERE providerId — the logical key. If the observed provider was deregistered and a replacement created with the same providerId before the transaction ran, that update would clobber the replacement's config and ownership. Update the specific observed row by its primary-key id instead; if it's gone we insert, which fails cleanly on the providerId unique constraint rather than overwriting the replacement. * fix(sso): script upserts provider via delete-then-insert (no unique constraint) sso_provider.provider_id is a plain (non-unique) index and prod holds legitimate duplicates, so the previous "update by id, else insert" could create a duplicate provider when the observed row was deregistered and replaced before the transaction — the fallback insert would succeed. Delete every row for the providerId then insert exactly one, inside the transaction, so the providerId ends up as exactly this config atomically. Linked accounts key on the providerId string (not the row id), so existing logins are unaffected. * fix(sso): guard compensating-delete row id so rollback can't silently no-op * chore(sso): regenerate migration as 0268 after merging staging Staging landed migrations 0266/0267, colliding with our 0266. Removed our migration, merged staging, and regenerated cleanly with drizzle-kit as 0268_sso_domain_verification (identical sso_domain table + indexes), then re-appended the grandfather backfill. api-validation baseline reconciled to 973 (staging 970 + our 3 domain routes). Also make the register-route test's registerSSOProvider mock return an id so the guarded compensating delete runs. * refactor(sso): share normalizeSSODomain via @sim/utils so script matches gate The self-host script canonicalized SSO domains with a minimal inline transform (lower+trim+wildcard) that diverged from the app's full normalizeSSODomain (protocol, port, path, trailing dot, email local part) — equivalent spellings could store a different ownership key than the runtime gate looks up. Move normalizeSSODomain into @sim/utils/sso-domain (a pure function) so the register route, the domain-claim route, and the script all use the identical canonicalizer. The script now skips the verified-domain record when SSO_DOMAIN isn't a valid registrable domain instead of storing a malformed key.
1 parent 444c415 commit 513292f

33 files changed

Lines changed: 19647 additions & 36 deletions

File tree

apps/docs/content/docs/en/platform/enterprise/meta.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"pages": [
44
"index",
55
"sso",
6+
"verified-domains",
67
"session-policies",
78
"access-control",
89
"custom-blocks",
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
title: Verified Domains
3+
description: Prove ownership of your email domains before configuring single sign-on
4+
---
5+
6+
import { Callout } from 'fumadocs-ui/components/callout'
7+
import { FAQ } from '@/components/ui/faq'
8+
9+
Verified Domains let organization owners and admins on Enterprise plans prove they control an email domain (like `acme.com`) with a DNS TXT record. Verifying a domain is the security precondition for configuring single sign-on for it.
10+
11+
<Callout type="warning">
12+
Configuring SSO for a domain requires it to be verified first. Verifying proves your organization controls the domain — without it, anyone could point another company's domain at their own identity provider. Domains you had already configured for SSO are automatically treated as verified.
13+
</Callout>
14+
15+
---
16+
17+
## Verify a domain
18+
19+
Go to **Settings → Security → Verified domains** in your organization settings.
20+
21+
1. Enter the domain, for example `acme.com`, and click **Add domain**.
22+
2. Sim shows a DNS **TXT record** to publish — a host (`_sim-challenge.acme.com`) and a unique value (`sim-domain-verification=…`).
23+
3. Add that TXT record at your DNS provider.
24+
4. Click **Verify**. Sim looks up the record; on success the domain is marked **Verified**.
25+
26+
DNS changes can take up to 48 hours to propagate — if verification does not succeed immediately, wait and retry. You can remove the TXT record after the domain is verified; the verification persists.
27+
28+
Add each domain you own separately. Subdomains (`eng.acme.com`) are verified independently of the apex.
29+
30+
---
31+
32+
## FAQ
33+
34+
<FAQ
35+
items={[
36+
{
37+
question: 'Where does the TXT record go?',
38+
answer:
39+
'On a dedicated host, _sim-challenge.<your-domain>, rather than the root of your domain — this avoids colliding with your SPF, DMARC, or other root TXT records.',
40+
},
41+
{
42+
question: 'What happens to domains we already use for SSO?',
43+
answer:
44+
'They are automatically treated as verified, so existing single sign-on keeps working with no action needed.',
45+
},
46+
{
47+
question: 'Can two organizations verify the same domain?',
48+
answer:
49+
'No. A verified domain belongs to exactly one organization. Once verified, another organization cannot claim it.',
50+
},
51+
{
52+
question: 'What if I remove a verified domain?',
53+
answer:
54+
'You lose the ownership proof, so you cannot configure SSO for that domain until you re-add and re-verify it. Removing it does not sign anyone out — an already-configured SSO provider keeps working.',
55+
},
56+
]}
57+
/>

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

Lines changed: 81 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@ import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
1616
const {
1717
mockGetSession,
1818
mockRegisterSSOProvider,
19+
mockUpdateSSOProvider,
1920
mockHasSSOAccess,
2021
mockValidateUrlWithDNS,
2122
mockSecureFetchWithPinnedIP,
2223
} = vi.hoisted(() => ({
2324
mockGetSession: vi.fn(),
2425
mockRegisterSSOProvider: vi.fn(),
26+
mockUpdateSSOProvider: vi.fn(),
2527
mockHasSSOAccess: vi.fn(),
2628
mockValidateUrlWithDNS: vi.fn(),
2729
mockSecureFetchWithPinnedIP: vi.fn(),
@@ -45,14 +47,19 @@ function queueProviders(rows: Array<Record<string, unknown>>) {
4547

4648
vi.mock('@/lib/auth', () => ({
4749
getSession: mockGetSession,
48-
auth: { api: { registerSSOProvider: mockRegisterSSOProvider } },
50+
auth: {
51+
api: {
52+
registerSSOProvider: mockRegisterSSOProvider,
53+
updateSSOProvider: mockUpdateSSOProvider,
54+
},
55+
},
4956
}))
5057

5158
vi.mock('@/lib/billing', () => ({
5259
hasSSOAccess: mockHasSSOAccess,
5360
}))
5461

55-
vi.mock('@/lib/auth/sso/domain', () => ({
62+
vi.mock('@sim/utils/sso-domain', () => ({
5663
normalizeSSODomain: (input: unknown): string | null => {
5764
if (typeof input !== 'string') return null
5865
const value = input.trim().toLowerCase()
@@ -93,7 +100,17 @@ describe('POST /api/auth/sso/register', () => {
93100
mockHasSSOAccess.mockResolvedValue(true)
94101
mockValidateUrlWithDNS.mockResolvedValue({ isValid: true, resolvedIP: '1.2.3.4' })
95102
mockSecureFetchWithPinnedIP.mockRejectedValue(new Error('discovery not mocked for this test'))
96-
mockRegisterSSOProvider.mockResolvedValue({ providerId: 'acme-oidc' })
103+
mockRegisterSSOProvider.mockResolvedValue({ id: 'row-1', providerId: 'acme-oidc' })
104+
mockUpdateSSOProvider.mockResolvedValue({ providerId: 'acme-oidc' })
105+
// Default: the org has already verified the domain, so the ownership gate
106+
// passes and each test exercises the logic beyond it. The gate is checked
107+
// three times for a successful org-scoped registration (fail-fast entry +
108+
// authoritative re-check before the write + compensating re-check after the
109+
// write), so queue three rows. Gate-specific tests reset the queue to assert
110+
// the unverified paths.
111+
queueTableRows(schemaMock.ssoDomain, [{ id: 'verified-domain' }])
112+
queueTableRows(schemaMock.ssoDomain, [{ id: 'verified-domain' }])
113+
queueTableRows(schemaMock.ssoDomain, [{ id: 'verified-domain' }])
97114
})
98115

99116
afterAll(() => {
@@ -122,6 +139,43 @@ describe('POST /api/auth/sso/register', () => {
122139
expect(mockRegisterSSOProvider).not.toHaveBeenCalled()
123140
})
124141

142+
it('rejects configuring org SSO for a domain the org has not verified', async () => {
143+
resetDbChainMock()
144+
queueMembers([{ organizationId: 'org1', role: 'owner' }])
145+
queueTableRows(schemaMock.ssoDomain, []) // no verified sso_domain row
146+
const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' }))
147+
const json = await res.json()
148+
expect(res.status).toBe(403)
149+
expect(json.code).toBe('SSO_DOMAIN_NOT_VERIFIED')
150+
expect(mockRegisterSSOProvider).not.toHaveBeenCalled()
151+
})
152+
153+
it('re-checks verification before the write and 403s if it was revoked mid-registration', async () => {
154+
resetDbChainMock()
155+
queueMembers([{ organizationId: 'org1', role: 'owner' }])
156+
queueTableRows(schemaMock.ssoDomain, [{ id: 'v' }]) // entry gate: verified
157+
queueTableRows(schemaMock.ssoDomain, []) // re-check before write: revoked
158+
const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' }))
159+
const json = await res.json()
160+
expect(res.status).toBe(403)
161+
expect(json.code).toBe('SSO_DOMAIN_NOT_VERIFIED')
162+
expect(mockRegisterSSOProvider).not.toHaveBeenCalled()
163+
})
164+
165+
it('rolls back the newly-created provider if verification is revoked after the write', async () => {
166+
resetDbChainMock()
167+
queueMembers([{ organizationId: 'org1', role: 'owner' }])
168+
queueTableRows(schemaMock.ssoDomain, [{ id: 'v' }]) // entry gate: verified
169+
queueTableRows(schemaMock.ssoDomain, [{ id: 'v' }]) // pre-write re-check: verified
170+
queueTableRows(schemaMock.ssoDomain, []) // post-write compensating check: revoked
171+
const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' }))
172+
const json = await res.json()
173+
expect(res.status).toBe(403)
174+
expect(json.code).toBe('SSO_DOMAIN_NOT_VERIFIED')
175+
expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1) // it was created…
176+
expect(dbChainMockFns.delete).toHaveBeenCalled() // …then rolled back
177+
})
178+
125179
it('rejects a domain already registered by another organization', async () => {
126180
queueMembers([{ organizationId: 'org-attacker', role: 'owner' }])
127181
queueProviders([{ domain: 'acme.com', userId: 'u-victim', organizationId: 'org-victim' }])
@@ -152,6 +206,30 @@ describe('POST /api/auth/sso/register', () => {
152206
expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1)
153207
})
154208

209+
it('nests the attribute mapping inside oidcConfig (Better Auth reads it there)', async () => {
210+
queueMembers([{ organizationId: 'org1', role: 'owner' }])
211+
await POST(
212+
request({ ...OIDC_BODY, orgId: 'org1', mapping: { id: 'oid', email: 'upn', name: 'name' } })
213+
)
214+
expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1)
215+
const sent = mockRegisterSSOProvider.mock.calls[0][0].body
216+
expect(sent.mapping).toBeUndefined() // not passed at the top level (silently ignored there)
217+
expect(sent.oidcConfig.mapping).toMatchObject({ id: 'oid', email: 'upn', name: 'name' })
218+
})
219+
220+
it('routes an edit of an existing owned provider through updateSSOProvider', async () => {
221+
queueMembers([{ organizationId: 'org1', role: 'owner' }])
222+
queueTableRows(schemaMock.ssoProvider, []) // findDomainConflict #1 → no conflict
223+
queueTableRows(schemaMock.ssoProvider, []) // findDomainConflict #2 → no conflict
224+
queueTableRows(schemaMock.ssoProvider, [{ id: 'p1' }]) // provider already owned → edit
225+
const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' }))
226+
expect(res.status).toBe(200)
227+
const body = await res.json()
228+
expect(body.message).toContain('updated')
229+
expect(mockUpdateSSOProvider).toHaveBeenCalledTimes(1)
230+
expect(mockRegisterSSOProvider).not.toHaveBeenCalled()
231+
})
232+
155233
it('allows the owning tenant to update its own provider for the same domain', async () => {
156234
queueMembers([{ organizationId: 'org1', role: 'owner' }])
157235
queueProviders([{ domain: 'acme.com', userId: 'u1', organizationId: 'org1' }])

0 commit comments

Comments
 (0)