diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index df12ab0f86f..3640723a32f 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -88,10 +88,15 @@ 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 { getRequestedSignInProviderId, 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' /** @@ -128,12 +133,14 @@ function getMicrosoftUserInfoFromIdToken(tokens: { accessToken?: string }, provi ) } + const emailVerified = deriveMicrosoftEmailVerified(payload, 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 +668,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 +851,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 = getRequestedSignInProviderId(ctx.path, ctx.body) + 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 +1908,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..4f37d67e031 --- /dev/null +++ b/apps/sim/lib/auth/constants.test.ts @@ -0,0 +1,86 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + getRequestedSignInProviderId, + 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) + }) +}) + +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 46eca038cfc..90972142791 100644 --- a/apps/sim/lib/auth/constants.ts +++ b/apps/sim/lib/auth/constants.ts @@ -8,3 +8,47 @@ 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) +} + +/** + * 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 +} 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)) +}