Skip to content

Commit ec38a1a

Browse files
committed
fix(sso): withhold domain trust from personal providers on the hosted deployment
A personal (org-less) provider has no verified domain behind it, but the trust grant treated it as authoritative anyway. On the multi-tenant deployment that is an account-takeover primitive: anyone able to register one could claim a domain they do not own, point it at their own IdP, and have a sign-in auto-link to an existing account on that domain. Sim's UI always registers org-scoped, so this only reaches direct API callers. Self-hosted deployments are single-tenant — the operator is the only tenant — so the org-less path keeps working there. Also clears the attribute mapping when the protocol changes: claim names are protocol-specific, so an OIDC override carried into a SAML config would save a mapping the IdP cannot resolve.
1 parent 1a0203e commit ec38a1a

3 files changed

Lines changed: 37 additions & 5 deletions

File tree

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,26 @@ describe('POST /api/auth/sso/register', () => {
311311
expect(dbChainMockFns.delete).toHaveBeenCalled() // …then rolled back
312312
})
313313

314+
/**
315+
* A personal provider has no verified domain behind it. On the hosted
316+
* multi-tenant deployment that must grant no linking authority, or anyone able
317+
* to register one could claim a domain they do not own and have their own IdP
318+
* auto-link to existing accounts on it.
319+
*/
320+
it('does not grant domain trust to a personal provider when hosted', async () => {
321+
setEnvFlags({ isSsoEnabled: true, isHosted: true })
322+
const res = await POST(request(OIDC_BODY))
323+
expect(res.status).toBe(200)
324+
expect(dbChainMockFns.set).toHaveBeenCalledWith({ domainVerified: false })
325+
})
326+
327+
it('grants domain trust to a personal provider when self-hosted', async () => {
328+
setEnvFlags({ isSsoEnabled: true, isHosted: false })
329+
const res = await POST(request(OIDC_BODY))
330+
expect(res.status).toBe(200)
331+
expect(dbChainMockFns.set).toHaveBeenCalledWith({ domainVerified: true })
332+
})
333+
314334
it('nests the attribute mapping inside oidcConfig (Better Auth reads it there)', async () => {
315335
queueMembers([{ organizationId: 'org1', role: 'owner' }])
316336
await POST(

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { ssoRegistrationContract } from '@/lib/api/contracts/auth'
88
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
99
import { auth, getSession } from '@/lib/auth'
1010
import { hasSSOAccess } from '@/lib/billing'
11-
import { isSsoEnabled } from '@/lib/core/config/env-flags'
11+
import { isHosted, isSsoEnabled } from '@/lib/core/config/env-flags'
1212
import {
1313
secureFetchWithPinnedIP,
1414
validateUrlWithDNS,
@@ -659,12 +659,19 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
659659
* clause, so the write matches nothing once the proof is gone and reports that
660660
* as `false`. Paired with the domain-delete route clearing this flag in the
661661
* same transaction that removes the proof, the provider cannot end up trusted
662-
* without current ownership in either commit order. Org-less (personal) SSO is
663-
* not domain-gated by Sim, so it grants unconditionally as it always has.
662+
* without current ownership in either commit order.
663+
*
664+
* Org-less (personal) SSO has no verified domain behind it, so on the hosted
665+
* multi-tenant deployment it is granted no linking authority — otherwise anyone
666+
* able to register one could claim a domain they do not own and have their own
667+
* IdP auto-link to existing accounts on it. Sim's UI always registers
668+
* org-scoped, so this only affects direct API callers. Self-hosted deployments
669+
* are single-tenant, where the operator is the only tenant and the org-less
670+
* path keeps working as before.
664671
*/
665672
const grantProviderDomainTrust = async (): Promise<boolean> => {
666673
if (!orgId) {
667-
await setProviderDomainVerified(true)
674+
await setProviderDomainVerified(!isHosted)
668675
return true
669676
}
670677
const granted = await db

apps/sim/ee/sso/components/sso-settings.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,8 +379,13 @@ function OrganizationSsoSettings({ organizationId }: SSOProps) {
379379

380380
const handleInputChange = (field: keyof typeof formData, value: string | boolean) => {
381381
const next = { ...formData, [field]: value }
382-
382+
// Claim names are protocol-specific — OIDC's `email` means nothing to a SAML
383+
// IdP — so carrying an override across a protocol switch would save a mapping
384+
// that cannot resolve. Clear them and fall back to the new protocol's defaults.
383385
if (field === 'providerType') {
386+
next.mapId = ''
387+
next.mapEmail = ''
388+
next.mapName = ''
384389
setShowErrors(false)
385390
}
386391

0 commit comments

Comments
 (0)