From 6acd34a56b330689af6752abd0b9edc1f09defc2 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 15 Aug 2026 11:37:16 -0700 Subject: [PATCH 1/4] fix(auth): fence off plugin-served SSO provider mutation endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The auth catch-all forwarded every non-organization POST to the better-auth SSO plugin, leaving sso/update-provider and sso/delete-provider reachable alongside the app-owned sso/register route. update-provider is gated only on provider ownership and merges the caller's samlConfig, so a provider owner could set mapping.emailVerified — a field the register contract deliberately omits and the plugin's identity-boundary guard does not inspect, so it never trips the linked-account conflict. With trustEmailVerified enabled, a subsequent assertion carrying an arbitrary verified email auto-links to that user's account. Block SSO POST paths by default, allowing only the sso/saml2/ protocol endpoints the IdP posts to, mirroring the existing organization fence. --- apps/sim/app/api/auth/[...all]/route.test.ts | 67 ++++++++++++++++++++ apps/sim/app/api/auth/[...all]/route.ts | 33 ++++++++++ 2 files changed, 100 insertions(+) diff --git a/apps/sim/app/api/auth/[...all]/route.test.ts b/apps/sim/app/api/auth/[...all]/route.test.ts index 4c577409990..4989d2d2f8c 100644 --- a/apps/sim/app/api/auth/[...all]/route.test.ts +++ b/apps/sim/app/api/auth/[...all]/route.test.ts @@ -132,3 +132,70 @@ describe('auth catch-all route organization mutations', () => { expect(json).toEqual({ data: { ok: true } }) }) }) + +describe('auth catch-all route SSO provider mutations', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it.each([ + 'sso/update-provider', + 'sso/delete-provider', + 'sso/request-domain-verification', + 'sso/verify-domain', + ])('blocks the plugin-served %s endpoint', async (path) => { + const req = createMockRequest('POST', undefined, {}, `http://localhost:3000/api/auth/${path}`) + + const res = await POST(req as any) + const json = await res.json() + + expect(res.status).toBe(404) + expect(handlerMocks.betterAuthPOST).not.toHaveBeenCalled() + expect(json).toEqual({ + error: 'SSO provider mutations are handled by application API routes.', + }) + }) + + it.each([ + 'sso/saml2/callback/acme', + 'sso/saml2/sp/acs/acme', + 'sso/saml2/sp/slo/acme', + 'sso/saml2/logout/acme', + ])('allows the SAML protocol endpoint %s', async (path) => { + const { NextResponse } = await import('next/server') + handlerMocks.betterAuthPOST.mockResolvedValueOnce( + new NextResponse(JSON.stringify({ data: { ok: true } }), { + headers: { 'content-type': 'application/json' }, + }) as any + ) + + const req = createMockRequest('POST', undefined, {}, `http://localhost:3000/api/auth/${path}`) + + const res = await POST(req as any) + const json = await res.json() + + expect(handlerMocks.betterAuthPOST).toHaveBeenCalledTimes(1) + expect(json).toEqual({ data: { ok: true } }) + }) + + it('leaves the SSO sign-in endpoint reachable', async () => { + const { NextResponse } = await import('next/server') + handlerMocks.betterAuthPOST.mockResolvedValueOnce( + new NextResponse(JSON.stringify({ data: { url: 'https://idp.example.com' } }), { + headers: { 'content-type': 'application/json' }, + }) as any + ) + + const req = createMockRequest( + 'POST', + undefined, + {}, + 'http://localhost:3000/api/auth/sign-in/sso' + ) + + const res = await POST(req as any) + + expect(handlerMocks.betterAuthPOST).toHaveBeenCalledTimes(1) + expect(await res.json()).toEqual({ data: { url: 'https://idp.example.com' } }) + }) +}) diff --git a/apps/sim/app/api/auth/[...all]/route.ts b/apps/sim/app/api/auth/[...all]/route.ts index 8456afff4cf..07ba572fbac 100644 --- a/apps/sim/app/api/auth/[...all]/route.ts +++ b/apps/sim/app/api/auth/[...all]/route.ts @@ -10,6 +10,14 @@ export const dynamic = 'force-dynamic' const { GET: betterAuthGET, POST: betterAuthPOST } = toNextJsHandler(auth.handler) const SAFE_ORGANIZATION_POST_PATHS = new Set(['organization/check-slug', 'organization/set-active']) +/** + * SAML protocol endpoints the IdP posts to (`saml2/callback/:id`, + * `saml2/sp/acs/:id`, `saml2/sp/slo/:id`, `saml2/logout/:id`). These are the + * only SSO paths the plugin must keep serving on POST — every other SSO POST + * endpoint it registers is a provider mutation. + */ +const SAML_PROTOCOL_POST_PREFIX = 'sso/saml2/' + function getAuthPath(request: NextRequest): string { const pathname = request.nextUrl?.pathname ?? new URL(request.url).pathname return pathname.replace('/api/auth/', '') @@ -19,6 +27,24 @@ function isBlockedOrganizationMutationPath(path: string): boolean { return path.startsWith('organization/') && !SAFE_ORGANIZATION_POST_PATHS.has(path) } +/** + * SSO provider configuration is owned by `/api/auth/sso/register`, which proves + * domain ownership before granting trust and restricts the attribute mapping to + * `id`/`email`/`name`/`image`. The plugin's own `sso/update-provider` bypasses + * both: it is gated only on provider ownership and merges the caller's config, + * so a provider owner could add `mapping.emailVerified` — a change the plugin's + * identity-boundary guard does not consider, so it never trips the linked-account + * conflict — and then assert an arbitrary victim's email as verified to auto-link + * into their account. `sso/delete-provider` likewise lets an owner drop a login + * path outside the application's flow. + * + * Deny-by-default rather than a blocklist so a future plugin version cannot + * introduce another unshadowed provider mutation. + */ +function isBlockedSsoMutationPath(path: string): boolean { + return path.startsWith('sso/') && !path.startsWith(SAML_PROTOCOL_POST_PREFIX) +} + export const GET = withRouteHandler(async (request: NextRequest) => { const path = getAuthPath(request) @@ -40,5 +66,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) } + if (isBlockedSsoMutationPath(path)) { + return NextResponse.json( + { error: 'SSO provider mutations are handled by application API routes.' }, + { status: 404 } + ) + } + return betterAuthPOST(request) }) From aea0d54eafdb4c9c34d03c39a465c94bd0399ea8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 15 Aug 2026 11:53:52 -0700 Subject: [PATCH 2/4] fix(auth): stop trusting IdP email_verified for SSO account linking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Better Auth's link gate is `!isTrustedProvider && !userInfo.emailVerified`, so trustEmailVerified let a true email_verified claim stand in for the domain binding. Any principal able to register an SSO provider — an Enterprise org admin, or any signed-in user when self-hosted — could point it at an IdP they control, assert an arbitrary victim's address as verified, and auto-link into that account across tenant boundaries, persisting as an account row. With it off, linking requires isTrustedProvider, which is domainVerified plus validateEmailDomain(email, provider.domain) — a provider can only claim identities inside the domain it proved. That is the model the codebase already documents for trustProviderByName: false. The option only ever set emailVerified on the local row; it was never what made linking work, since Entra omits the claim and SAML ignores it without a mapping the register contract does not accept. --- apps/sim/lib/auth/auth.ts | 29 ++++++++++++++------- apps/sim/lib/auth/sso-trust.test.ts | 39 +++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 9 deletions(-) create mode 100644 apps/sim/lib/auth/sso-trust.test.ts diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index 7906d93707d..915cc5d0cdd 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -1169,14 +1169,24 @@ export const auth = betterAuth({ ? [ sso({ /** - * Honor the IdP's `email_verified` claim so the local account is - * verified rather than forced to false. + * MUST stay false. Better Auth's link gate is + * `!isTrustedProvider && !userInfo.emailVerified`, so a true + * `email_verified` claim substitutes for the domain binding + * entirely: an IdP could assert any address — including one from a + * domain it does not own — and auto-link into that user's existing + * account. Since a provider row can be registered by any Enterprise + * org admin (and by any signed-in user when self-hosted), trusting + * the claim makes every account reachable from any tenant's IdP. * - * This is not what enables linking — Entra omits the claim entirely, - * and SAML ignores it without an explicit `mapping.emailVerified`. - * `domainVerification` below establishes linking trust. + * Turning it on only ever set `emailVerified` on the local row; it + * was never what made linking work. Entra omits the claim, and SAML + * ignores it without an explicit `mapping.emailVerified` that the + * register contract does not accept — so SSO users are created + * unverified either way, and `domainVerification` below is the sole + * linking trust source, which is what `trustProviderByName: false` + * already assumes. */ - trustEmailVerified: true, + trustEmailVerified: false, /** * Marks a provider authoritative for its domain, which is what lets an * SSO sign-in auto-link to an existing same-email account. Without it @@ -1187,9 +1197,10 @@ export const auth = betterAuth({ * proven by the `sso_domain` flow before registration, and the register * route mirrors that decision onto this flag. * - * It narrows nothing on its own — an IdP asserting `email_verified` - * links regardless of domain (see `trustEmailVerified` above). It - * exists so linking survives IdPs that omit the claim. + * With `trustEmailVerified` off this is the only path to linking, and + * it is domain-scoped: `isTrustedProvider` additionally requires + * `validateEmailDomain(userInfo.email, provider.domain)`, so a + * provider can only ever claim identities inside the domain it proved. */ domainVerification: { enabled: true }, organizationProvisioning: { diff --git a/apps/sim/lib/auth/sso-trust.test.ts b/apps/sim/lib/auth/sso-trust.test.ts new file mode 100644 index 00000000000..1b620d4529c --- /dev/null +++ b/apps/sim/lib/auth/sso-trust.test.ts @@ -0,0 +1,39 @@ +/** + * @vitest-environment node + * + * Locks the SSO linking trust model. Better Auth's account-link gate is + * `!isTrustedProvider && !userInfo.emailVerified`, so a truthy + * `trustEmailVerified` lets any registered IdP assert an out-of-domain address + * as verified and auto-link into that user's account, bypassing the + * domain-verification proof entirely. + */ +import { resetEnvFlagsMock, setEnvFlags } from '@sim/testing' +import { afterAll, expect, it, vi } from 'vitest' + +const { ssoOptions } = vi.hoisted(() => ({ + ssoOptions: { current: undefined as Record | undefined }, +})) + +vi.mock('@better-auth/sso', () => ({ + sso: (options: Record) => { + ssoOptions.current = options + return { id: 'sso' } + }, +})) + +setEnvFlags({ isSsoEnabled: true }) + +afterAll(resetEnvFlagsMock) + +it('never trusts the IdP-supplied email_verified claim for SSO linking', async () => { + await import('@/lib/auth/auth') + + expect(ssoOptions.current).toBeDefined() + expect(ssoOptions.current?.trustEmailVerified).toBe(false) +}) + +it('keeps domain verification as the sole SSO linking trust source', async () => { + await import('@/lib/auth/auth') + + expect(ssoOptions.current?.domainVerification).toEqual({ enabled: true }) +}) From 76b03b01b781484bf12dd1503309e36c6b79ff68 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 15 Aug 2026 12:01:00 -0700 Subject: [PATCH 3/4] docs(auth): note that the SSO fence and trustEmailVerified are layered --- apps/sim/app/api/auth/[...all]/route.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/sim/app/api/auth/[...all]/route.ts b/apps/sim/app/api/auth/[...all]/route.ts index 07ba572fbac..51f2fdd5282 100644 --- a/apps/sim/app/api/auth/[...all]/route.ts +++ b/apps/sim/app/api/auth/[...all]/route.ts @@ -38,6 +38,12 @@ function isBlockedOrganizationMutationPath(path: string): boolean { * into their account. `sso/delete-provider` likewise lets an owner drop a login * path outside the application's flow. * + * `trustEmailVerified: false` independently defuses that claim, so these two + * guards are layered, not redundant: this one keeps provider configuration + * owned by the register route (which alone proves domain ownership) and is what + * stops the mapping rewrite from becoming live again if that option is ever + * reconsidered. + * * Deny-by-default rather than a blocklist so a future plugin version cannot * introduce another unshadowed provider mutation. */ From 350d26acf77d38bbab58a97903c25989864d63fb Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 15 Aug 2026 14:06:17 -0700 Subject: [PATCH 4/4] test(auth): drop unnecessary any casts from the auth catch-all tests createMockRequest already returns a NextRequest and the handler mocks are untyped vi.fn()s, so every cast in the file was suppressing type checking for no reason. --- apps/sim/app/api/auth/[...all]/route.test.ts | 22 ++++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/apps/sim/app/api/auth/[...all]/route.test.ts b/apps/sim/app/api/auth/[...all]/route.test.ts index 4989d2d2f8c..3207be6bd32 100644 --- a/apps/sim/app/api/auth/[...all]/route.test.ts +++ b/apps/sim/app/api/auth/[...all]/route.test.ts @@ -50,7 +50,7 @@ describe('auth catch-all route (DISABLE_AUTH get-session)', () => { 'http://localhost:3000/api/auth/get-session' ) - const res = await GET(req as any) + const res = await GET(req) const json = await res.json() expect(handlerMocks.ensureAnonymousUserExists).toHaveBeenCalledTimes(1) @@ -68,7 +68,7 @@ describe('auth catch-all route (DISABLE_AUTH get-session)', () => { handlerMocks.betterAuthGET.mockResolvedValueOnce( new NextResponse(JSON.stringify({ data: { ok: true } }), { headers: { 'content-type': 'application/json' }, - }) as any + }) ) const req = createMockRequest( @@ -78,7 +78,7 @@ describe('auth catch-all route (DISABLE_AUTH get-session)', () => { 'http://localhost:3000/api/auth/get-session' ) - const res = await GET(req as any) + const res = await GET(req) const json = await res.json() expect(handlerMocks.ensureAnonymousUserExists).not.toHaveBeenCalled() @@ -100,7 +100,7 @@ describe('auth catch-all route organization mutations', () => { 'http://localhost:3000/api/auth/organization/create' ) - const res = await POST(req as any) + const res = await POST(req) const json = await res.json() expect(res.status).toBe(404) @@ -115,7 +115,7 @@ describe('auth catch-all route organization mutations', () => { handlerMocks.betterAuthPOST.mockResolvedValueOnce( new NextResponse(JSON.stringify({ data: { ok: true } }), { headers: { 'content-type': 'application/json' }, - }) as any + }) ) const req = createMockRequest( @@ -125,7 +125,7 @@ describe('auth catch-all route organization mutations', () => { 'http://localhost:3000/api/auth/organization/set-active' ) - const res = await POST(req as any) + const res = await POST(req) const json = await res.json() expect(handlerMocks.betterAuthPOST).toHaveBeenCalledTimes(1) @@ -146,7 +146,7 @@ describe('auth catch-all route SSO provider mutations', () => { ])('blocks the plugin-served %s endpoint', async (path) => { const req = createMockRequest('POST', undefined, {}, `http://localhost:3000/api/auth/${path}`) - const res = await POST(req as any) + const res = await POST(req) const json = await res.json() expect(res.status).toBe(404) @@ -166,12 +166,12 @@ describe('auth catch-all route SSO provider mutations', () => { handlerMocks.betterAuthPOST.mockResolvedValueOnce( new NextResponse(JSON.stringify({ data: { ok: true } }), { headers: { 'content-type': 'application/json' }, - }) as any + }) ) const req = createMockRequest('POST', undefined, {}, `http://localhost:3000/api/auth/${path}`) - const res = await POST(req as any) + const res = await POST(req) const json = await res.json() expect(handlerMocks.betterAuthPOST).toHaveBeenCalledTimes(1) @@ -183,7 +183,7 @@ describe('auth catch-all route SSO provider mutations', () => { handlerMocks.betterAuthPOST.mockResolvedValueOnce( new NextResponse(JSON.stringify({ data: { url: 'https://idp.example.com' } }), { headers: { 'content-type': 'application/json' }, - }) as any + }) ) const req = createMockRequest( @@ -193,7 +193,7 @@ describe('auth catch-all route SSO provider mutations', () => { 'http://localhost:3000/api/auth/sign-in/sso' ) - const res = await POST(req as any) + const res = await POST(req) expect(handlerMocks.betterAuthPOST).toHaveBeenCalledTimes(1) expect(await res.json()).toEqual({ data: { url: 'https://idp.example.com' } })