Skip to content

Commit dc5cf93

Browse files
committed
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.
1 parent 82a1d17 commit dc5cf93

3 files changed

Lines changed: 58 additions & 3 deletions

File tree

apps/sim/lib/auth/auth.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +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'
91+
import { getRequestedSignInProviderId, isSignInProviderAllowed } from './constants'
9292

9393
const logger = createLogger('Auth')
9494

@@ -861,7 +861,7 @@ export const auth = betterAuth({
861861
* through the authenticated `/oauth2/link` flow, which is unaffected.
862862
*/
863863
if (ctx.path === '/sign-in/social' || ctx.path === '/sign-in/oauth2') {
864-
const requestedProviderId = ctx.body?.provider ?? ctx.body?.providerId
864+
const requestedProviderId = getRequestedSignInProviderId(ctx.path, ctx.body)
865865
if (!isSignInProviderAllowed(requestedProviderId)) {
866866
throw new APIError('FORBIDDEN', {
867867
message:

apps/sim/lib/auth/constants.test.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22
* @vitest-environment node
33
*/
44
import { describe, expect, it } from 'vitest'
5-
import { isSignInProviderAllowed, SIGN_IN_PROVIDER_IDS } from '@/lib/auth/constants'
5+
import {
6+
getRequestedSignInProviderId,
7+
isSignInProviderAllowed,
8+
SIGN_IN_PROVIDER_IDS,
9+
} from '@/lib/auth/constants'
610

711
describe('sign-in provider allowlist', () => {
812
it('permits only the first-party login providers', () => {
@@ -46,3 +50,37 @@ describe('sign-in provider allowlist', () => {
4650
expect(isSignInProviderAllowed({ provider: 'google' })).toBe(false)
4751
})
4852
})
53+
54+
describe('getRequestedSignInProviderId', () => {
55+
it('reads the `provider` field on /sign-in/social', () => {
56+
expect(getRequestedSignInProviderId('/sign-in/social', { provider: 'microsoft' })).toBe(
57+
'microsoft'
58+
)
59+
})
60+
61+
it('reads the `providerId` field on /sign-in/oauth2', () => {
62+
expect(getRequestedSignInProviderId('/sign-in/oauth2', { providerId: 'salesforce' })).toBe(
63+
'salesforce'
64+
)
65+
})
66+
67+
it('checks the field the /sign-in/oauth2 handler uses, ignoring a decoy `provider`', () => {
68+
const body = { provider: 'google', providerId: 'salesforce' }
69+
const resolved = getRequestedSignInProviderId('/sign-in/oauth2', body)
70+
expect(resolved).toBe('salesforce')
71+
expect(isSignInProviderAllowed(resolved)).toBe(false)
72+
})
73+
74+
it('checks the field the /sign-in/social handler uses, ignoring a decoy `providerId`', () => {
75+
const body = { provider: 'salesforce', providerId: 'google' }
76+
const resolved = getRequestedSignInProviderId('/sign-in/social', body)
77+
expect(resolved).toBe('salesforce')
78+
expect(isSignInProviderAllowed(resolved)).toBe(false)
79+
})
80+
81+
it('returns undefined for unrelated paths and missing bodies', () => {
82+
expect(getRequestedSignInProviderId('/sign-in/email', { provider: 'google' })).toBeUndefined()
83+
expect(getRequestedSignInProviderId('/sign-in/social', undefined)).toBeUndefined()
84+
expect(getRequestedSignInProviderId('/sign-in/oauth2', null)).toBeUndefined()
85+
})
86+
})

apps/sim/lib/auth/constants.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,20 @@ const signInProviderIdSet: ReadonlySet<string> = new Set(SIGN_IN_PROVIDER_IDS)
3535
export function isSignInProviderAllowed(providerId: unknown): boolean {
3636
return typeof providerId === 'string' && signInProviderIdSet.has(providerId)
3737
}
38+
39+
/**
40+
* Resolves the provider identifier the sign-in handler will actually act on for a
41+
* given auth path. Better Auth reads `provider` on `/sign-in/social` and
42+
* `providerId` on `/sign-in/oauth2`, so the allowlist guard must read the same
43+
* field the handler uses. Reading the wrong field would let a request pass the
44+
* guard with an allowed value in one field while the handler starts OAuth for a
45+
* blocked value in the other, reopening connector sign-in.
46+
*/
47+
export function getRequestedSignInProviderId(
48+
path: string,
49+
body: { provider?: unknown; providerId?: unknown } | null | undefined
50+
): unknown {
51+
if (path === '/sign-in/social') return body?.provider
52+
if (path === '/sign-in/oauth2') return body?.providerId
53+
return undefined
54+
}

0 commit comments

Comments
 (0)