From d6c816d9054c88863c2733ead7470af942188287 Mon Sep 17 00:00:00 2001 From: waleed Date: Sat, 20 Jun 2026 14:20:31 -0700 Subject: [PATCH 1/3] fix(auth): close nOAuth account takeover via email-based OAuth linking Restrict the unauthenticated sign-in endpoints to first-party login providers, trim trustedProviders to providers that verify email ownership, and stop hardcoding emailVerified for multi-tenant Microsoft and Salesforce connectors. --- apps/sim/lib/auth/auth.ts | 98 ++++++++++++++--------------- apps/sim/lib/auth/constants.test.ts | 48 ++++++++++++++ apps/sim/lib/auth/constants.ts | 27 ++++++++ 3 files changed, 121 insertions(+), 52 deletions(-) create mode 100644 apps/sim/lib/auth/constants.test.ts diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index df12ab0f86f..ed0c2a5a2fc 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -88,6 +88,7 @@ import { syncAllWebhooksForCredentialSet } from '@/lib/webhooks/utils.server' import { disableUserResources } from '@/lib/workflows/lifecycle' import { SSO_TRUSTED_PROVIDERS } from '@/ee/sso/constants' import { createAnonymousSession, ensureAnonymousUserExists } from './anonymous' +import { isSignInProviderAllowed } from './constants' const logger = createLogger('Auth') @@ -128,12 +129,25 @@ function getMicrosoftUserInfoFromIdToken(tokens: { accessToken?: string }, provi ) } + /** + * Azure AD's `email`/`upn` claims are unverified and mutable on multi-tenant + * (`/common/`) endpoints, so trust the email only when the token explicitly + * proves ownership via `email_verified` or the verified-email claims, mirroring + * Better Auth's built-in Microsoft provider. Never hardcode verification. + */ + const verifiedPrimaryEmail = payload.verified_primary_email as string[] | undefined + const verifiedSecondaryEmail = payload.verified_secondary_email as string[] | undefined + const emailVerified = + payload.email_verified !== undefined + ? Boolean(payload.email_verified) + : Boolean(verifiedPrimaryEmail?.includes(email) || verifiedSecondaryEmail?.includes(email)) + const now = new Date() return { id: `${payload.oid || payload.sub}-${generateId()}`, name: (payload.name as string) || 'Microsoft User', email, - emailVerified: true, + emailVerified, createdAt: now, updatedAt: now, } @@ -661,60 +675,21 @@ export const auth = betterAuth({ enabled: true, allowDifferentEmails: true, requireLocalEmailVerified: false, + /** + * Only providers that verify email ownership may auto-link to an existing + * account during sign-in. Integration connectors are deliberately absent: + * they connect through the authenticated `/oauth2/link` flow, which binds + * to the current session user and never consults this list. `microsoft` is + * also excluded because it authenticates against the multi-tenant + * `/common/` endpoint where the email claim is attacker-controllable; + * leaving it trusted would bypass the email-verified check and allow + * nOAuth account takeover. Microsoft sign-in still works — it just links + * to an existing account only when the IdP asserts a verified email. + */ trustedProviders: [ 'google', 'github', 'email-password', - 'confluence', - 'x', - 'notion', - 'microsoft', - 'slack', - 'reddit', - 'webflow', - 'asana', - 'pipedrive', - 'hubspot', - 'linkedin', - 'spotify', - 'google-email', - 'google-calendar', - 'google-contacts', - 'google-drive', - 'google-docs', - 'google-sheets', - 'google-forms', - 'google-ads', - 'google-bigquery', - 'google-vault', - 'google-groups', - 'google-meet', - 'google-tasks', - 'vertex-ai', - - 'microsoft-ad', - 'microsoft-dataverse', - 'microsoft-teams', - 'microsoft-excel', - 'microsoft-planner', - 'outlook', - 'onedrive', - 'sharepoint', - 'jira', - 'airtable', - 'box', - 'dropbox', - 'salesforce', - 'wealthbox', - 'zoom', - 'wordpress', - 'linear', - 'monday', - 'attio', - 'shopify', - 'trello', - 'calcom', - 'docusign', ...SSO_TRUSTED_PROVIDERS, ...additionalTrustedSsoProviders, ], @@ -883,6 +858,25 @@ export const auth = betterAuth({ }, hooks: { before: createAuthMiddleware(async (ctx) => { + /** + * Restrict the unauthenticated sign-in endpoints to first-party login + * providers. Better Auth registers every generic-OAuth integration + * connector as a social provider, so without this guard `microsoft-ad`, + * `salesforce`, `jira`, and the rest are reachable through + * `/sign-in/social` and `/sign-in/oauth2` and can mint a session for any + * user by email (nOAuth account takeover). Connectors are connected only + * through the authenticated `/oauth2/link` flow, which is unaffected. + */ + if (ctx.path === '/sign-in/social' || ctx.path === '/sign-in/oauth2') { + const requestedProviderId = ctx.body?.provider ?? ctx.body?.providerId + if (!isSignInProviderAllowed(requestedProviderId)) { + throw new APIError('FORBIDDEN', { + message: + 'This provider can only be connected from a signed-in account and cannot be used to sign in.', + }) + } + } + if (ctx.path.startsWith('/sign-up') && isRegistrationDisabled) throw new APIError('FORBIDDEN', { message: 'Registration is disabled, please contact your admin.', @@ -1921,7 +1915,7 @@ export const auth = betterAuth({ id: `${(data.user_id || data.sub).toString()}-${generateId()}`, name: data.name || 'Salesforce User', email: data.email || `salesforce-${data.user_id}@salesforce.com`, - emailVerified: data.email_verified || true, + emailVerified: data.email_verified === true, image: data.picture || undefined, createdAt: new Date(), updatedAt: new Date(), diff --git a/apps/sim/lib/auth/constants.test.ts b/apps/sim/lib/auth/constants.test.ts new file mode 100644 index 00000000000..375821891cf --- /dev/null +++ b/apps/sim/lib/auth/constants.test.ts @@ -0,0 +1,48 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { isSignInProviderAllowed, SIGN_IN_PROVIDER_IDS } from '@/lib/auth/constants' + +describe('sign-in provider allowlist', () => { + it('permits only the first-party login providers', () => { + expect([...SIGN_IN_PROVIDER_IDS]).toEqual(['google', 'github', 'microsoft']) + }) + + it('allows first-party login providers to sign in', () => { + for (const providerId of SIGN_IN_PROVIDER_IDS) { + expect(isSignInProviderAllowed(providerId)).toBe(true) + } + }) + + it('rejects integration connectors from the sign-in endpoints', () => { + const connectors = [ + 'microsoft-ad', + 'microsoft-teams', + 'microsoft-excel', + 'outlook', + 'onedrive', + 'sharepoint', + 'salesforce', + 'jira', + 'confluence', + 'hubspot', + 'box', + 'wordpress', + 'google-drive', + 'google-sheets', + 'vertex-ai', + ] + for (const providerId of connectors) { + expect(isSignInProviderAllowed(providerId)).toBe(false) + } + }) + + it('rejects missing or malformed provider identifiers', () => { + expect(isSignInProviderAllowed(undefined)).toBe(false) + expect(isSignInProviderAllowed(null)).toBe(false) + expect(isSignInProviderAllowed('')).toBe(false) + expect(isSignInProviderAllowed(123)).toBe(false) + expect(isSignInProviderAllowed({ provider: 'google' })).toBe(false) + }) +}) diff --git a/apps/sim/lib/auth/constants.ts b/apps/sim/lib/auth/constants.ts index 46eca038cfc..0e8082fc339 100644 --- a/apps/sim/lib/auth/constants.ts +++ b/apps/sim/lib/auth/constants.ts @@ -8,3 +8,30 @@ export const ANONYMOUS_USER = { emailVerified: true, image: null, } as const + +/** + * Provider IDs permitted to authenticate (create or link a session) through the + * unauthenticated sign-in endpoints `/sign-in/social` and `/sign-in/oauth2`. + * + * Only first-party login providers that verify email ownership belong here. + * Integration connectors (Salesforce, Jira, the Microsoft/Google work + * connectors, etc.) are deliberately excluded: they are connected exclusively + * through the authenticated `/oauth2/link` flow, which binds the new account to + * the current session user and never mints a session. Allowing a connector to + * reach the sign-in endpoints enables nOAuth-style account takeover, where a + * multi-tenant IdP asserting an attacker-controlled, unverified email mints a + * session for the matching existing user. SSO uses a separate `/sign-in/sso` + * endpoint and is unaffected by this list. + */ +export const SIGN_IN_PROVIDER_IDS = ['google', 'github', 'microsoft'] as const + +const signInProviderIdSet: ReadonlySet = new Set(SIGN_IN_PROVIDER_IDS) + +/** + * Returns true when `providerId` is a first-party login provider allowed to sign + * in. Used to reject integration connectors at the sign-in endpoints so they can + * only ever be connected through the authenticated link flow. + */ +export function isSignInProviderAllowed(providerId: unknown): boolean { + return typeof providerId === 'string' && signInProviderIdSet.has(providerId) +} From 82a1d17fe3a43da91efda5c6dad331832292058b Mon Sep 17 00:00:00 2001 From: waleed Date: Sat, 20 Jun 2026 14:37:41 -0700 Subject: [PATCH 2/3] test(auth): cover Microsoft id-token emailVerified derivation Extract the Microsoft ID-token email-verification logic into a pure deriveMicrosoftEmailVerified helper and add unit coverage for explicit, verified-claim, partial, absent, and malformed Azure AD claim combinations. --- apps/sim/lib/auth/auth.ts | 19 +++---- apps/sim/lib/oauth/microsoft.test.ts | 74 ++++++++++++++++++++++++++++ apps/sim/lib/oauth/microsoft.ts | 21 ++++++++ 3 files changed, 101 insertions(+), 13 deletions(-) create mode 100644 apps/sim/lib/oauth/microsoft.test.ts diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index ed0c2a5a2fc..f6627784182 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -92,7 +92,11 @@ import { isSignInProviderAllowed } from './constants' const logger = createLogger('Auth') -import { getMicrosoftRefreshTokenExpiry, isMicrosoftProvider } from '@/lib/oauth/microsoft' +import { + deriveMicrosoftEmailVerified, + getMicrosoftRefreshTokenExpiry, + isMicrosoftProvider, +} from '@/lib/oauth/microsoft' import { getCanonicalScopesForProvider } from '@/lib/oauth/utils' /** @@ -129,18 +133,7 @@ function getMicrosoftUserInfoFromIdToken(tokens: { accessToken?: string }, provi ) } - /** - * Azure AD's `email`/`upn` claims are unverified and mutable on multi-tenant - * (`/common/`) endpoints, so trust the email only when the token explicitly - * proves ownership via `email_verified` or the verified-email claims, mirroring - * Better Auth's built-in Microsoft provider. Never hardcode verification. - */ - const verifiedPrimaryEmail = payload.verified_primary_email as string[] | undefined - const verifiedSecondaryEmail = payload.verified_secondary_email as string[] | undefined - const emailVerified = - payload.email_verified !== undefined - ? Boolean(payload.email_verified) - : Boolean(verifiedPrimaryEmail?.includes(email) || verifiedSecondaryEmail?.includes(email)) + const emailVerified = deriveMicrosoftEmailVerified(payload, email) const now = new Date() return { diff --git a/apps/sim/lib/oauth/microsoft.test.ts b/apps/sim/lib/oauth/microsoft.test.ts new file mode 100644 index 00000000000..a3eaf4d9c60 --- /dev/null +++ b/apps/sim/lib/oauth/microsoft.test.ts @@ -0,0 +1,74 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { deriveMicrosoftEmailVerified, isMicrosoftProvider } from '@/lib/oauth/microsoft' + +const EMAIL = 'user@contoso.com' + +describe('deriveMicrosoftEmailVerified', () => { + it('honors an explicit email_verified=true claim', () => { + expect(deriveMicrosoftEmailVerified({ email_verified: true }, EMAIL)).toBe(true) + }) + + it('honors an explicit email_verified=false claim over verified-email claims', () => { + expect( + deriveMicrosoftEmailVerified( + { email_verified: false, verified_primary_email: [EMAIL] }, + EMAIL + ) + ).toBe(false) + }) + + it('treats a verified primary email matching the email as verified', () => { + expect(deriveMicrosoftEmailVerified({ verified_primary_email: [EMAIL] }, EMAIL)).toBe(true) + }) + + it('treats a verified secondary email matching the email as verified', () => { + expect( + deriveMicrosoftEmailVerified({ verified_secondary_email: ['x@y.com', EMAIL] }, EMAIL) + ).toBe(true) + }) + + it('does not verify when the verified-email claims do not include the email', () => { + expect( + deriveMicrosoftEmailVerified( + { + verified_primary_email: ['other@contoso.com'], + verified_secondary_email: ['another@contoso.com'], + }, + EMAIL + ) + ).toBe(false) + }) + + it('defaults to false when no verification claim is present (typical Azure AD token)', () => { + expect(deriveMicrosoftEmailVerified({ name: 'User', oid: 'abc' }, EMAIL)).toBe(false) + }) + + it('defaults to false for an empty claim set', () => { + expect(deriveMicrosoftEmailVerified({}, EMAIL)).toBe(false) + }) + + it('coerces a truthy non-boolean email_verified claim', () => { + expect(deriveMicrosoftEmailVerified({ email_verified: 'true' }, EMAIL)).toBe(true) + }) + + it('treats a malformed verified-email claim as unverified', () => { + expect(deriveMicrosoftEmailVerified({ verified_primary_email: 'not-an-array' }, EMAIL)).toBe( + false + ) + }) +}) + +describe('isMicrosoftProvider', () => { + it('recognizes Microsoft connector provider IDs', () => { + expect(isMicrosoftProvider('microsoft-ad')).toBe(true) + expect(isMicrosoftProvider('outlook')).toBe(true) + }) + + it('rejects non-Microsoft provider IDs', () => { + expect(isMicrosoftProvider('google')).toBe(false) + expect(isMicrosoftProvider('microsoft')).toBe(false) + }) +}) diff --git a/apps/sim/lib/oauth/microsoft.ts b/apps/sim/lib/oauth/microsoft.ts index 18e05eb9138..2ecf050e62f 100644 --- a/apps/sim/lib/oauth/microsoft.ts +++ b/apps/sim/lib/oauth/microsoft.ts @@ -19,3 +19,24 @@ export function isMicrosoftProvider(providerId: string): boolean { export function getMicrosoftRefreshTokenExpiry(): Date { return new Date(Date.now() + MICROSOFT_REFRESH_TOKEN_LIFETIME_DAYS * 24 * 60 * 60 * 1000) } + +/** + * Derives whether a Microsoft ID token proves ownership of `email`. Azure AD's + * `email`/`upn` claims are unverified and mutable on multi-tenant (`/common/`) + * endpoints, so the email is trusted only when the token explicitly proves it via + * the `email_verified` claim or the verified-email claims, mirroring Better + * Auth's built-in Microsoft provider. Defaults to `false` when no claim asserts + * verification, so an attacker-controlled tenant can never assert a verified + * email it does not own. + */ +export function deriveMicrosoftEmailVerified( + claims: Record, + email: string +): boolean { + if (claims.email_verified !== undefined) { + return Boolean(claims.email_verified) + } + const verifiedPrimaryEmail = claims.verified_primary_email as string[] | undefined + const verifiedSecondaryEmail = claims.verified_secondary_email as string[] | undefined + return Boolean(verifiedPrimaryEmail?.includes(email) || verifiedSecondaryEmail?.includes(email)) +} From dc5cf93ac865edc237eda910b3386f79771f0e91 Mon Sep 17 00:00:00 2001 From: waleed Date: Sat, 20 Jun 2026 14:47:11 -0700 Subject: [PATCH 3/3] fix(auth): check the provider field the sign-in handler actually uses The allowlist guard resolved the provider with `provider ?? providerId`, but Better Auth reads `provider` on /sign-in/social and `providerId` on /sign-in/oauth2. A request to /sign-in/oauth2 with an allowed `provider` and a blocked `providerId` could pass the guard while the handler started OAuth for the blocked connector. Resolve the field per path via getRequestedSignInProviderId so the guard checks the same field the handler acts on. --- apps/sim/lib/auth/auth.ts | 4 +-- apps/sim/lib/auth/constants.test.ts | 40 ++++++++++++++++++++++++++++- apps/sim/lib/auth/constants.ts | 17 ++++++++++++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index f6627784182..3640723a32f 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -88,7 +88,7 @@ import { syncAllWebhooksForCredentialSet } from '@/lib/webhooks/utils.server' import { disableUserResources } from '@/lib/workflows/lifecycle' import { SSO_TRUSTED_PROVIDERS } from '@/ee/sso/constants' import { createAnonymousSession, ensureAnonymousUserExists } from './anonymous' -import { isSignInProviderAllowed } from './constants' +import { getRequestedSignInProviderId, isSignInProviderAllowed } from './constants' const logger = createLogger('Auth') @@ -861,7 +861,7 @@ export const auth = betterAuth({ * through the authenticated `/oauth2/link` flow, which is unaffected. */ if (ctx.path === '/sign-in/social' || ctx.path === '/sign-in/oauth2') { - const requestedProviderId = ctx.body?.provider ?? ctx.body?.providerId + const requestedProviderId = getRequestedSignInProviderId(ctx.path, ctx.body) if (!isSignInProviderAllowed(requestedProviderId)) { throw new APIError('FORBIDDEN', { message: diff --git a/apps/sim/lib/auth/constants.test.ts b/apps/sim/lib/auth/constants.test.ts index 375821891cf..4f37d67e031 100644 --- a/apps/sim/lib/auth/constants.test.ts +++ b/apps/sim/lib/auth/constants.test.ts @@ -2,7 +2,11 @@ * @vitest-environment node */ import { describe, expect, it } from 'vitest' -import { isSignInProviderAllowed, SIGN_IN_PROVIDER_IDS } from '@/lib/auth/constants' +import { + getRequestedSignInProviderId, + isSignInProviderAllowed, + SIGN_IN_PROVIDER_IDS, +} from '@/lib/auth/constants' describe('sign-in provider allowlist', () => { it('permits only the first-party login providers', () => { @@ -46,3 +50,37 @@ describe('sign-in provider allowlist', () => { expect(isSignInProviderAllowed({ provider: 'google' })).toBe(false) }) }) + +describe('getRequestedSignInProviderId', () => { + it('reads the `provider` field on /sign-in/social', () => { + expect(getRequestedSignInProviderId('/sign-in/social', { provider: 'microsoft' })).toBe( + 'microsoft' + ) + }) + + it('reads the `providerId` field on /sign-in/oauth2', () => { + expect(getRequestedSignInProviderId('/sign-in/oauth2', { providerId: 'salesforce' })).toBe( + 'salesforce' + ) + }) + + it('checks the field the /sign-in/oauth2 handler uses, ignoring a decoy `provider`', () => { + const body = { provider: 'google', providerId: 'salesforce' } + const resolved = getRequestedSignInProviderId('/sign-in/oauth2', body) + expect(resolved).toBe('salesforce') + expect(isSignInProviderAllowed(resolved)).toBe(false) + }) + + it('checks the field the /sign-in/social handler uses, ignoring a decoy `providerId`', () => { + const body = { provider: 'salesforce', providerId: 'google' } + const resolved = getRequestedSignInProviderId('/sign-in/social', body) + expect(resolved).toBe('salesforce') + expect(isSignInProviderAllowed(resolved)).toBe(false) + }) + + it('returns undefined for unrelated paths and missing bodies', () => { + expect(getRequestedSignInProviderId('/sign-in/email', { provider: 'google' })).toBeUndefined() + expect(getRequestedSignInProviderId('/sign-in/social', undefined)).toBeUndefined() + expect(getRequestedSignInProviderId('/sign-in/oauth2', null)).toBeUndefined() + }) +}) diff --git a/apps/sim/lib/auth/constants.ts b/apps/sim/lib/auth/constants.ts index 0e8082fc339..90972142791 100644 --- a/apps/sim/lib/auth/constants.ts +++ b/apps/sim/lib/auth/constants.ts @@ -35,3 +35,20 @@ const signInProviderIdSet: ReadonlySet = new Set(SIGN_IN_PROVIDER_IDS) export function isSignInProviderAllowed(providerId: unknown): boolean { return typeof providerId === 'string' && signInProviderIdSet.has(providerId) } + +/** + * Resolves the provider identifier the sign-in handler will actually act on for a + * given auth path. Better Auth reads `provider` on `/sign-in/social` and + * `providerId` on `/sign-in/oauth2`, so the allowlist guard must read the same + * field the handler uses. Reading the wrong field would let a request pass the + * guard with an allowed value in one field while the handler starts OAuth for a + * blocked value in the other, reopening connector sign-in. + */ +export function getRequestedSignInProviderId( + path: string, + body: { provider?: unknown; providerId?: unknown } | null | undefined +): unknown { + if (path === '/sign-in/social') return body?.provider + if (path === '/sign-in/oauth2') return body?.providerId + return undefined +}