Skip to content

Commit d6c816d

Browse files
committed
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.
1 parent 2f7d607 commit d6c816d

3 files changed

Lines changed: 121 additions & 52 deletions

File tree

apps/sim/lib/auth/auth.ts

Lines changed: 46 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ import { syncAllWebhooksForCredentialSet } from '@/lib/webhooks/utils.server'
8888
import { disableUserResources } from '@/lib/workflows/lifecycle'
8989
import { SSO_TRUSTED_PROVIDERS } from '@/ee/sso/constants'
9090
import { createAnonymousSession, ensureAnonymousUserExists } from './anonymous'
91+
import { isSignInProviderAllowed } from './constants'
9192

9293
const logger = createLogger('Auth')
9394

@@ -128,12 +129,25 @@ function getMicrosoftUserInfoFromIdToken(tokens: { accessToken?: string }, provi
128129
)
129130
}
130131

132+
/**
133+
* Azure AD's `email`/`upn` claims are unverified and mutable on multi-tenant
134+
* (`/common/`) endpoints, so trust the email only when the token explicitly
135+
* proves ownership via `email_verified` or the verified-email claims, mirroring
136+
* Better Auth's built-in Microsoft provider. Never hardcode verification.
137+
*/
138+
const verifiedPrimaryEmail = payload.verified_primary_email as string[] | undefined
139+
const verifiedSecondaryEmail = payload.verified_secondary_email as string[] | undefined
140+
const emailVerified =
141+
payload.email_verified !== undefined
142+
? Boolean(payload.email_verified)
143+
: Boolean(verifiedPrimaryEmail?.includes(email) || verifiedSecondaryEmail?.includes(email))
144+
131145
const now = new Date()
132146
return {
133147
id: `${payload.oid || payload.sub}-${generateId()}`,
134148
name: (payload.name as string) || 'Microsoft User',
135149
email,
136-
emailVerified: true,
150+
emailVerified,
137151
createdAt: now,
138152
updatedAt: now,
139153
}
@@ -661,60 +675,21 @@ export const auth = betterAuth({
661675
enabled: true,
662676
allowDifferentEmails: true,
663677
requireLocalEmailVerified: false,
678+
/**
679+
* Only providers that verify email ownership may auto-link to an existing
680+
* account during sign-in. Integration connectors are deliberately absent:
681+
* they connect through the authenticated `/oauth2/link` flow, which binds
682+
* to the current session user and never consults this list. `microsoft` is
683+
* also excluded because it authenticates against the multi-tenant
684+
* `/common/` endpoint where the email claim is attacker-controllable;
685+
* leaving it trusted would bypass the email-verified check and allow
686+
* nOAuth account takeover. Microsoft sign-in still works — it just links
687+
* to an existing account only when the IdP asserts a verified email.
688+
*/
664689
trustedProviders: [
665690
'google',
666691
'github',
667692
'email-password',
668-
'confluence',
669-
'x',
670-
'notion',
671-
'microsoft',
672-
'slack',
673-
'reddit',
674-
'webflow',
675-
'asana',
676-
'pipedrive',
677-
'hubspot',
678-
'linkedin',
679-
'spotify',
680-
'google-email',
681-
'google-calendar',
682-
'google-contacts',
683-
'google-drive',
684-
'google-docs',
685-
'google-sheets',
686-
'google-forms',
687-
'google-ads',
688-
'google-bigquery',
689-
'google-vault',
690-
'google-groups',
691-
'google-meet',
692-
'google-tasks',
693-
'vertex-ai',
694-
695-
'microsoft-ad',
696-
'microsoft-dataverse',
697-
'microsoft-teams',
698-
'microsoft-excel',
699-
'microsoft-planner',
700-
'outlook',
701-
'onedrive',
702-
'sharepoint',
703-
'jira',
704-
'airtable',
705-
'box',
706-
'dropbox',
707-
'salesforce',
708-
'wealthbox',
709-
'zoom',
710-
'wordpress',
711-
'linear',
712-
'monday',
713-
'attio',
714-
'shopify',
715-
'trello',
716-
'calcom',
717-
'docusign',
718693
...SSO_TRUSTED_PROVIDERS,
719694
...additionalTrustedSsoProviders,
720695
],
@@ -883,6 +858,25 @@ export const auth = betterAuth({
883858
},
884859
hooks: {
885860
before: createAuthMiddleware(async (ctx) => {
861+
/**
862+
* Restrict the unauthenticated sign-in endpoints to first-party login
863+
* providers. Better Auth registers every generic-OAuth integration
864+
* connector as a social provider, so without this guard `microsoft-ad`,
865+
* `salesforce`, `jira`, and the rest are reachable through
866+
* `/sign-in/social` and `/sign-in/oauth2` and can mint a session for any
867+
* user by email (nOAuth account takeover). Connectors are connected only
868+
* through the authenticated `/oauth2/link` flow, which is unaffected.
869+
*/
870+
if (ctx.path === '/sign-in/social' || ctx.path === '/sign-in/oauth2') {
871+
const requestedProviderId = ctx.body?.provider ?? ctx.body?.providerId
872+
if (!isSignInProviderAllowed(requestedProviderId)) {
873+
throw new APIError('FORBIDDEN', {
874+
message:
875+
'This provider can only be connected from a signed-in account and cannot be used to sign in.',
876+
})
877+
}
878+
}
879+
886880
if (ctx.path.startsWith('/sign-up') && isRegistrationDisabled)
887881
throw new APIError('FORBIDDEN', {
888882
message: 'Registration is disabled, please contact your admin.',
@@ -1921,7 +1915,7 @@ export const auth = betterAuth({
19211915
id: `${(data.user_id || data.sub).toString()}-${generateId()}`,
19221916
name: data.name || 'Salesforce User',
19231917
email: data.email || `salesforce-${data.user_id}@salesforce.com`,
1924-
emailVerified: data.email_verified || true,
1918+
emailVerified: data.email_verified === true,
19251919
image: data.picture || undefined,
19261920
createdAt: new Date(),
19271921
updatedAt: new Date(),
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { isSignInProviderAllowed, SIGN_IN_PROVIDER_IDS } from '@/lib/auth/constants'
6+
7+
describe('sign-in provider allowlist', () => {
8+
it('permits only the first-party login providers', () => {
9+
expect([...SIGN_IN_PROVIDER_IDS]).toEqual(['google', 'github', 'microsoft'])
10+
})
11+
12+
it('allows first-party login providers to sign in', () => {
13+
for (const providerId of SIGN_IN_PROVIDER_IDS) {
14+
expect(isSignInProviderAllowed(providerId)).toBe(true)
15+
}
16+
})
17+
18+
it('rejects integration connectors from the sign-in endpoints', () => {
19+
const connectors = [
20+
'microsoft-ad',
21+
'microsoft-teams',
22+
'microsoft-excel',
23+
'outlook',
24+
'onedrive',
25+
'sharepoint',
26+
'salesforce',
27+
'jira',
28+
'confluence',
29+
'hubspot',
30+
'box',
31+
'wordpress',
32+
'google-drive',
33+
'google-sheets',
34+
'vertex-ai',
35+
]
36+
for (const providerId of connectors) {
37+
expect(isSignInProviderAllowed(providerId)).toBe(false)
38+
}
39+
})
40+
41+
it('rejects missing or malformed provider identifiers', () => {
42+
expect(isSignInProviderAllowed(undefined)).toBe(false)
43+
expect(isSignInProviderAllowed(null)).toBe(false)
44+
expect(isSignInProviderAllowed('')).toBe(false)
45+
expect(isSignInProviderAllowed(123)).toBe(false)
46+
expect(isSignInProviderAllowed({ provider: 'google' })).toBe(false)
47+
})
48+
})

apps/sim/lib/auth/constants.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,30 @@ export const ANONYMOUS_USER = {
88
emailVerified: true,
99
image: null,
1010
} as const
11+
12+
/**
13+
* Provider IDs permitted to authenticate (create or link a session) through the
14+
* unauthenticated sign-in endpoints `/sign-in/social` and `/sign-in/oauth2`.
15+
*
16+
* Only first-party login providers that verify email ownership belong here.
17+
* Integration connectors (Salesforce, Jira, the Microsoft/Google work
18+
* connectors, etc.) are deliberately excluded: they are connected exclusively
19+
* through the authenticated `/oauth2/link` flow, which binds the new account to
20+
* the current session user and never mints a session. Allowing a connector to
21+
* reach the sign-in endpoints enables nOAuth-style account takeover, where a
22+
* multi-tenant IdP asserting an attacker-controlled, unverified email mints a
23+
* session for the matching existing user. SSO uses a separate `/sign-in/sso`
24+
* endpoint and is unaffected by this list.
25+
*/
26+
export const SIGN_IN_PROVIDER_IDS = ['google', 'github', 'microsoft'] as const
27+
28+
const signInProviderIdSet: ReadonlySet<string> = new Set(SIGN_IN_PROVIDER_IDS)
29+
30+
/**
31+
* Returns true when `providerId` is a first-party login provider allowed to sign
32+
* in. Used to reject integration connectors at the sign-in endpoints so they can
33+
* only ever be connected through the authenticated link flow.
34+
*/
35+
export function isSignInProviderAllowed(providerId: unknown): boolean {
36+
return typeof providerId === 'string' && signInProviderIdSet.has(providerId)
37+
}

0 commit comments

Comments
 (0)