From 5c6e51ea8f11809a4fbeb202779e6665bc59f703 Mon Sep 17 00:00:00 2001 From: waleed Date: Fri, 3 Jul 2026 11:11:26 -0700 Subject: [PATCH 1/5] fix(sso): support skipping the OIDC UserInfo endpoint at registration --- .../app/api/auth/sso/register/route.test.ts | 77 ++++++- apps/sim/app/api/auth/sso/register/route.ts | 204 ++++++++++-------- apps/sim/lib/api/contracts/auth.ts | 1 + packages/db/scripts/register-sso-provider.ts | 13 ++ 4 files changed, 207 insertions(+), 88 deletions(-) diff --git a/apps/sim/app/api/auth/sso/register/route.test.ts b/apps/sim/app/api/auth/sso/register/route.test.ts index 87f4cdd6744..3e5900b9bea 100644 --- a/apps/sim/app/api/auth/sso/register/route.test.ts +++ b/apps/sim/app/api/auth/sso/register/route.test.ts @@ -9,6 +9,7 @@ const { mockRegisterSSOProvider, mockHasSSOAccess, mockValidateUrlWithDNS, + mockSecureFetchWithPinnedIP, dbState, memberTable, ssoProviderTable, @@ -17,6 +18,7 @@ const { mockRegisterSSOProvider: vi.fn(), mockHasSSOAccess: vi.fn(), mockValidateUrlWithDNS: vi.fn(), + mockSecureFetchWithPinnedIP: vi.fn(), dbState: { members: [] as any[], providers: [] as any[] }, memberTable: { userId: 'member.userId', @@ -80,7 +82,7 @@ vi.mock('@/lib/auth/sso/domain', () => ({ vi.mock('@/lib/core/security/input-validation.server', () => ({ validateUrlWithDNS: mockValidateUrlWithDNS, - secureFetchWithPinnedIP: vi.fn(), + secureFetchWithPinnedIP: mockSecureFetchWithPinnedIP, })) vi.mock('@/lib/core/config/env', () => createEnvMock({ SSO_ENABLED: 'true' })) @@ -193,4 +195,77 @@ describe('POST /api/auth/sso/register', () => { const config = mockRegisterSSOProvider.mock.calls[0][0].body expect(config.domain).toBe('acme.com') }) + + it('passes skipDiscovery since Sim already resolved and validated the OIDC endpoints', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + expect(res.status).toBe(200) + const config = mockRegisterSSOProvider.mock.calls[0][0].body + expect(config.oidcConfig.skipDiscovery).toBe(true) + }) + + it('omits userInfoEndpoint when skipUserInfoEndpoint is requested, forcing ID token claims', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + const res = await POST(request({ ...OIDC_BODY, skipUserInfoEndpoint: true, orgId: 'org1' })) + expect(res.status).toBe(200) + const config = mockRegisterSSOProvider.mock.calls[0][0].body + expect(config.oidcConfig.userInfoEndpoint).toBeUndefined() + }) + + it('keeps userInfoEndpoint when skipUserInfoEndpoint is not requested', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + expect(res.status).toBe(200) + const config = mockRegisterSSOProvider.mock.calls[0][0].body + expect(config.oidcConfig.userInfoEndpoint).toBe('https://idp.acme.com/userinfo') + }) + + it('selects tokenEndpointAuthentication from the discovery document when endpoints are auto-discovered', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + mockSecureFetchWithPinnedIP.mockResolvedValue({ + ok: true, + json: async () => ({ + authorization_endpoint: 'https://idp.acme.com/authorize', + token_endpoint: 'https://idp.acme.com/token', + userinfo_endpoint: 'https://idp.acme.com/userinfo', + jwks_uri: 'https://idp.acme.com/jwks', + token_endpoint_auth_methods_supported: ['client_secret_post'], + }), + }) + const discoveredBody = { + ...OIDC_BODY, + authorizationEndpoint: undefined, + tokenEndpoint: undefined, + jwksEndpoint: undefined, + } + const res = await POST(request({ ...discoveredBody, orgId: 'org1' })) + expect(res.status).toBe(200) + const config = mockRegisterSSOProvider.mock.calls[0][0].body + expect(config.oidcConfig.tokenEndpointAuthentication).toBe('client_secret_post') + }) + + it('still selects tokenEndpointAuthentication from discovery when all endpoints are explicit', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + mockSecureFetchWithPinnedIP.mockResolvedValue({ + ok: true, + json: async () => ({ + token_endpoint_auth_methods_supported: ['client_secret_post'], + }), + }) + const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + expect(res.status).toBe(200) + const config = mockRegisterSSOProvider.mock.calls[0][0].body + expect(config.oidcConfig.tokenEndpointAuthentication).toBe('client_secret_post') + expect(config.oidcConfig.authorizationEndpoint).toBe(OIDC_BODY.authorizationEndpoint) + }) + + it('registers successfully when discovery is unreachable and all endpoints are explicit', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + mockSecureFetchWithPinnedIP.mockRejectedValue(new Error('ECONNREFUSED')) + const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + expect(res.status).toBe(200) + const config = mockRegisterSSOProvider.mock.calls[0][0].body + expect(config.oidcConfig.skipDiscovery).toBe(true) + expect(config.oidcConfig.authorizationEndpoint).toBe(OIDC_BODY.authorizationEndpoint) + }) }) diff --git a/apps/sim/app/api/auth/sso/register/route.ts b/apps/sim/app/api/auth/sso/register/route.ts index d5649607044..4b7125bdc4c 100644 --- a/apps/sim/app/api/auth/sso/register/route.ts +++ b/apps/sim/app/api/auth/sso/register/route.ts @@ -19,6 +19,44 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler' const logger = createLogger('SSORegisterRoute') +type TokenEndpointAuthMethod = 'client_secret_basic' | 'client_secret_post' + +function selectTokenEndpointAuthMethod( + supportedMethods: unknown, + existing?: TokenEndpointAuthMethod +): TokenEndpointAuthMethod { + if (existing) return existing + if (!Array.isArray(supportedMethods) || supportedMethods.length === 0) { + return 'client_secret_basic' + } + if (supportedMethods.includes('client_secret_basic')) return 'client_secret_basic' + if (supportedMethods.includes('client_secret_post')) return 'client_secret_post' + return 'client_secret_basic' +} + +type DiscoveryResult = + | { ok: true; discovery: Record } + | { ok: false; error: string } + +async function fetchOIDCDiscoveryDocument(discoveryUrl: string): Promise { + const urlValidation = await validateUrlWithDNS(discoveryUrl, 'OIDC discovery URL') + if (!urlValidation.isValid || !urlValidation.resolvedIP) { + return { ok: false, error: urlValidation.error ?? 'SSRF validation failed' } + } + + try { + const response = await secureFetchWithPinnedIP(discoveryUrl, urlValidation.resolvedIP, { + headers: { Accept: 'application/json' }, + }) + if (!response.ok) { + return { ok: false, error: `Discovery request failed with status ${response.status}` } + } + return { ok: true, discovery: (await response.json()) as Record } + } catch (error) { + return { ok: false, error: getErrorMessage(error, 'Unknown error') } + } +} + export const POST = withRouteHandler(async (request: NextRequest) => { try { if (!env.SSO_ENABLED) { @@ -132,6 +170,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { authorizationEndpoint, tokenEndpoint, userInfoEndpoint, + skipUserInfoEndpoint, jwksEndpoint, } = body @@ -206,104 +245,75 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const needsDiscovery = !oidcConfig.authorizationEndpoint || !oidcConfig.tokenEndpoint || !oidcConfig.jwksEndpoint - if (needsDiscovery) { - const discoveryUrl = `${issuer.replace(/\/$/, '')}/.well-known/openid-configuration` - try { - logger.info('Fetching OIDC discovery document for missing endpoints', { - discoveryUrl, - hasAuthEndpoint: !!oidcConfig.authorizationEndpoint, - hasTokenEndpoint: !!oidcConfig.tokenEndpoint, - hasJwksEndpoint: !!oidcConfig.jwksEndpoint, - }) + const discoveryUrl = `${issuer.replace(/\/$/, '')}/.well-known/openid-configuration` + const discoveryResult = await fetchOIDCDiscoveryDocument(discoveryUrl) - const urlValidation = await validateUrlWithDNS(discoveryUrl, 'OIDC discovery URL') - if (!urlValidation.isValid || !urlValidation.resolvedIP) { - logger.warn('OIDC discovery URL failed SSRF validation', { - discoveryUrl, - error: urlValidation.error, - }) - return NextResponse.json( - { error: urlValidation.error ?? 'SSRF validation failed' }, - { status: 400 } - ) - } + if (needsDiscovery) { + logger.info('Fetching OIDC discovery document for missing endpoints', { + discoveryUrl, + hasAuthEndpoint: !!oidcConfig.authorizationEndpoint, + hasTokenEndpoint: !!oidcConfig.tokenEndpoint, + hasJwksEndpoint: !!oidcConfig.jwksEndpoint, + }) - const discoveryResponse = await secureFetchWithPinnedIP( - discoveryUrl, - urlValidation.resolvedIP, + if (!discoveryResult.ok) { + logger.error('Failed to fetch OIDC discovery document', { discoveryResult }) + return NextResponse.json( { - headers: { Accept: 'application/json' }, - } + error: + 'Failed to fetch OIDC discovery document. Provide all endpoints explicitly or verify the issuer URL.', + }, + { status: 400 } ) + } - if (!discoveryResponse.ok) { - logger.error('Failed to fetch OIDC discovery document', { - status: discoveryResponse.status, - }) - return NextResponse.json( - { - error: - 'Failed to fetch OIDC discovery document. Provide all endpoints explicitly or verify the issuer URL.', - }, - { status: 400 } - ) - } - - const discovery = (await discoveryResponse.json()) as Record + const { discovery } = discoveryResult - const discoveredEndpoints: Record = { - authorization_endpoint: discovery.authorization_endpoint, - token_endpoint: discovery.token_endpoint, - userinfo_endpoint: discovery.userinfo_endpoint, - jwks_uri: discovery.jwks_uri, - } + const discoveredEndpoints: Record = { + authorization_endpoint: discovery.authorization_endpoint, + token_endpoint: discovery.token_endpoint, + userinfo_endpoint: discovery.userinfo_endpoint, + jwks_uri: discovery.jwks_uri, + } - for (const [key, value] of Object.entries(discoveredEndpoints)) { - if (typeof value === 'string') { - const endpointValidation = await validateUrlWithDNS(value, `OIDC ${key}`) - if (!endpointValidation.isValid) { - logger.warn('OIDC discovered endpoint failed SSRF validation', { - endpoint: key, - url: value, - error: endpointValidation.error, - }) - return NextResponse.json( - { - error: `Discovered OIDC ${key} failed security validation: ${endpointValidation.error}`, - }, - { status: 400 } - ) - } + for (const [key, value] of Object.entries(discoveredEndpoints)) { + if (typeof value === 'string') { + const endpointValidation = await validateUrlWithDNS(value, `OIDC ${key}`) + if (!endpointValidation.isValid) { + logger.warn('OIDC discovered endpoint failed SSRF validation', { + endpoint: key, + url: value, + error: endpointValidation.error, + }) + return NextResponse.json( + { + error: `Discovered OIDC ${key} failed security validation: ${endpointValidation.error}`, + }, + { status: 400 } + ) } } + } - oidcConfig.authorizationEndpoint = - oidcConfig.authorizationEndpoint || discovery.authorization_endpoint - oidcConfig.tokenEndpoint = oidcConfig.tokenEndpoint || discovery.token_endpoint - oidcConfig.userInfoEndpoint = oidcConfig.userInfoEndpoint || discovery.userinfo_endpoint - oidcConfig.jwksEndpoint = oidcConfig.jwksEndpoint || discovery.jwks_uri + oidcConfig.authorizationEndpoint = + oidcConfig.authorizationEndpoint || discovery.authorization_endpoint + oidcConfig.tokenEndpoint = oidcConfig.tokenEndpoint || discovery.token_endpoint + oidcConfig.userInfoEndpoint = oidcConfig.userInfoEndpoint || discovery.userinfo_endpoint + oidcConfig.jwksEndpoint = oidcConfig.jwksEndpoint || discovery.jwks_uri + oidcConfig.tokenEndpointAuthentication = selectTokenEndpointAuthMethod( + discovery.token_endpoint_auth_methods_supported, + oidcConfig.tokenEndpointAuthentication + ) - logger.info('Merged OIDC endpoints (user-provided + discovery)', { - providerId, - issuer, - authorizationEndpoint: oidcConfig.authorizationEndpoint, - tokenEndpoint: oidcConfig.tokenEndpoint, - userInfoEndpoint: oidcConfig.userInfoEndpoint, - jwksEndpoint: oidcConfig.jwksEndpoint, - }) - } catch (error) { - logger.error('Error fetching OIDC discovery document', { - error: getErrorMessage(error, 'Unknown error'), - discoveryUrl, - }) - return NextResponse.json( - { - error: - 'Failed to fetch OIDC discovery document. Please verify the issuer URL is correct or provide all endpoints explicitly.', - }, - { status: 400 } - ) - } + logger.info('Merged OIDC endpoints (user-provided + discovery)', { + providerId, + issuer, + authorizationEndpoint: oidcConfig.authorizationEndpoint, + tokenEndpoint: oidcConfig.tokenEndpoint, + userInfoEndpoint: oidcConfig.userInfoEndpoint, + jwksEndpoint: oidcConfig.jwksEndpoint, + tokenEndpointAuthentication: oidcConfig.tokenEndpointAuthentication, + }) } else { logger.info('Using explicitly provided OIDC endpoints (all present)', { providerId, @@ -313,6 +323,25 @@ export const POST = withRouteHandler(async (request: NextRequest) => { userInfoEndpoint: oidcConfig.userInfoEndpoint, jwksEndpoint: oidcConfig.jwksEndpoint, }) + + if (discoveryResult.ok) { + oidcConfig.tokenEndpointAuthentication = selectTokenEndpointAuthMethod( + discoveryResult.discovery.token_endpoint_auth_methods_supported, + oidcConfig.tokenEndpointAuthentication + ) + } else { + logger.info('OIDC discovery unavailable; falling back to the default token auth method', { + providerId, + discoveryUrl, + }) + } + } + + if (skipUserInfoEndpoint) { + oidcConfig.userInfoEndpoint = undefined + logger.info('Skipping UserInfo endpoint for provider, claims will come from the ID token', { + providerId, + }) } if ( @@ -339,6 +368,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) } + oidcConfig.skipDiscovery = true providerConfig.oidcConfig = oidcConfig } else if (providerType === 'saml') { const { diff --git a/apps/sim/lib/api/contracts/auth.ts b/apps/sim/lib/api/contracts/auth.ts index 8c47ce7d684..62966d70326 100644 --- a/apps/sim/lib/api/contracts/auth.ts +++ b/apps/sim/lib/api/contracts/auth.ts @@ -52,6 +52,7 @@ export const ssoRegistrationBodySchema = z.discriminatedUnion('providerType', [ authorizationEndpoint: z.string().url().optional(), tokenEndpoint: z.string().url().optional(), userInfoEndpoint: z.string().url().optional(), + skipUserInfoEndpoint: z.boolean().default(false), jwksEndpoint: z.string().url().optional(), }), z.object({ diff --git a/packages/db/scripts/register-sso-provider.ts b/packages/db/scripts/register-sso-provider.ts index 5efd45742b5..a52c6fa45e1 100644 --- a/packages/db/scripts/register-sso-provider.ts +++ b/packages/db/scripts/register-sso-provider.ts @@ -22,6 +22,11 @@ * SSO_OIDC_CLIENT_SECRET=your_client_secret * SSO_OIDC_SCOPES=openid,profile,email (optional) * SSO_OIDC_TOKEN_ENDPOINT_AUTH=client_secret_post|client_secret_basic (optional, defaults to client_secret_post) + * SSO_OIDC_SKIP_USERINFO_ENDPOINT=true (optional; reads claims from the verified ID token + * instead of calling the discovered UserInfo endpoint, matching better-auth's ID-token + * path in its OIDC callback. Use this for IdPs whose UserInfo endpoint omits claims that + * are present on the ID token, e.g. Microsoft Entra ID's Graph userinfo endpoint dropping + * `email` for some tenants) * * SAML Providers: * SSO_SAML_ENTRY_POINT=https://your-idp/sso @@ -55,6 +60,7 @@ interface OIDCConfig { authorizationEndpoint?: string tokenEndpoint?: string userInfoEndpoint?: string + skipUserInfoEndpoint?: boolean jwksEndpoint?: string discoveryEndpoint?: string tokenEndpointAuthentication?: 'client_secret_post' | 'client_secret_basic' @@ -223,6 +229,7 @@ function buildSSOConfigFromEnv(): SSOProviderConfig | null { ? process.env.SSO_OIDC_TOKEN_ENDPOINT_AUTH : undefined, userInfoEndpoint: process.env.SSO_OIDC_USERINFO_ENDPOINT, + skipUserInfoEndpoint: process.env.SSO_OIDC_SKIP_USERINFO_ENDPOINT === 'true', jwksEndpoint: process.env.SSO_OIDC_JWKS_ENDPOINT, discoveryEndpoint: process.env.SSO_OIDC_DISCOVERY_ENDPOINT || @@ -351,6 +358,7 @@ function getExampleEnvVars( SSO_OIDC_CLIENT_ID: 'your-application-id', SSO_OIDC_CLIENT_SECRET: 'your-client-secret', SSO_MAPPING_ID: 'oid', + SSO_OIDC_SKIP_USERINFO_ENDPOINT: 'true', }, generic: { ...baseVars, @@ -525,6 +533,11 @@ async function registerSSOProvider(): Promise { }) } + if (ssoConfig.oidcConfig.skipUserInfoEndpoint) { + ssoConfig.oidcConfig.userInfoEndpoint = undefined + logger.info('Skipping UserInfo endpoint: claims will be read from the verified ID token') + } + if ( !ssoConfig.oidcConfig.authorizationEndpoint || !ssoConfig.oidcConfig.tokenEndpoint || From 85b78c7c3e5651b7951b2ee24c5be71f69a3c483 Mon Sep 17 00:00:00 2001 From: waleed Date: Fri, 3 Jul 2026 11:20:10 -0700 Subject: [PATCH 2/5] fix(sso): cap OIDC discovery fetch at 10s to avoid stalling registration --- apps/sim/app/api/auth/sso/register/route.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/sim/app/api/auth/sso/register/route.ts b/apps/sim/app/api/auth/sso/register/route.ts index 4b7125bdc4c..4557c731e34 100644 --- a/apps/sim/app/api/auth/sso/register/route.ts +++ b/apps/sim/app/api/auth/sso/register/route.ts @@ -38,6 +38,8 @@ type DiscoveryResult = | { ok: true; discovery: Record } | { ok: false; error: string } +const OIDC_DISCOVERY_TIMEOUT_MS = 10000 + async function fetchOIDCDiscoveryDocument(discoveryUrl: string): Promise { const urlValidation = await validateUrlWithDNS(discoveryUrl, 'OIDC discovery URL') if (!urlValidation.isValid || !urlValidation.resolvedIP) { @@ -47,6 +49,7 @@ async function fetchOIDCDiscoveryDocument(discoveryUrl: string): Promise Date: Fri, 3 Jul 2026 11:26:22 -0700 Subject: [PATCH 3/5] test(sso): default-mock discovery fetch so intent is explicit --- apps/sim/app/api/auth/sso/register/route.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/sim/app/api/auth/sso/register/route.test.ts b/apps/sim/app/api/auth/sso/register/route.test.ts index 3e5900b9bea..05c364e29c6 100644 --- a/apps/sim/app/api/auth/sso/register/route.test.ts +++ b/apps/sim/app/api/auth/sso/register/route.test.ts @@ -114,6 +114,7 @@ describe('POST /api/auth/sso/register', () => { mockGetSession.mockResolvedValue({ user: { id: 'u1' } }) mockHasSSOAccess.mockResolvedValue(true) mockValidateUrlWithDNS.mockResolvedValue({ isValid: true, resolvedIP: '1.2.3.4' }) + mockSecureFetchWithPinnedIP.mockRejectedValue(new Error('discovery not mocked for this test')) mockRegisterSSOProvider.mockResolvedValue({ providerId: 'acme-oidc' }) }) From faa2f67c79fb206fcbb98fbd9b5684c7b6031d06 Mon Sep 17 00:00:00 2001 From: waleed Date: Fri, 3 Jul 2026 11:28:45 -0700 Subject: [PATCH 4/5] fix(sso): prefer client_secret_post and surface discovery failure reasons --- .../app/api/auth/sso/register/route.test.ts | 47 +++++++++++++++++++ apps/sim/app/api/auth/sso/register/route.ts | 15 ++++-- 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/apps/sim/app/api/auth/sso/register/route.test.ts b/apps/sim/app/api/auth/sso/register/route.test.ts index 05c364e29c6..ca08b9cb82f 100644 --- a/apps/sim/app/api/auth/sso/register/route.test.ts +++ b/apps/sim/app/api/auth/sso/register/route.test.ts @@ -269,4 +269,51 @@ describe('POST /api/auth/sso/register', () => { expect(config.oidcConfig.skipDiscovery).toBe(true) expect(config.oidcConfig.authorizationEndpoint).toBe(OIDC_BODY.authorizationEndpoint) }) + + it('prefers client_secret_post over client_secret_basic when an IdP supports both', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + mockSecureFetchWithPinnedIP.mockResolvedValue({ + ok: true, + json: async () => ({ + token_endpoint_auth_methods_supported: ['client_secret_basic', 'client_secret_post'], + }), + }) + const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + expect(res.status).toBe(200) + const config = mockRegisterSSOProvider.mock.calls[0][0].body + expect(config.oidcConfig.tokenEndpointAuthentication).toBe('client_secret_post') + }) + + it('defaults to client_secret_post when discovery advertises no auth methods', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + mockSecureFetchWithPinnedIP.mockResolvedValue({ + ok: true, + json: async () => ({}), + }) + const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + expect(res.status).toBe(200) + const config = mockRegisterSSOProvider.mock.calls[0][0].body + expect(config.oidcConfig.tokenEndpointAuthentication).toBe('client_secret_post') + }) + + it('surfaces the specific discovery failure reason when endpoints are missing', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + mockValidateUrlWithDNS.mockImplementation(async (url: string, label: string) => { + if (label === 'OIDC discovery URL') { + return { isValid: false, error: 'resolves to a private IP address' } + } + return { isValid: true, resolvedIP: '1.2.3.4' } + }) + const discoveredBody = { + ...OIDC_BODY, + authorizationEndpoint: undefined, + tokenEndpoint: undefined, + jwksEndpoint: undefined, + } + const res = await POST(request({ ...discoveredBody, orgId: 'org1' })) + const json = await res.json() + expect(res.status).toBe(400) + expect(json.error).toContain('resolves to a private IP address') + expect(mockRegisterSSOProvider).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/app/api/auth/sso/register/route.ts b/apps/sim/app/api/auth/sso/register/route.ts index 4557c731e34..da7df8df66f 100644 --- a/apps/sim/app/api/auth/sso/register/route.ts +++ b/apps/sim/app/api/auth/sso/register/route.ts @@ -21,17 +21,23 @@ const logger = createLogger('SSORegisterRoute') type TokenEndpointAuthMethod = 'client_secret_basic' | 'client_secret_post' +/** + * Prefers client_secret_post over client_secret_basic when an IdP supports both: + * better-auth sends client_secret_basic credentials without URL-encoding per + * RFC 6749 ยง2.3.1, so a '+' in the client secret is decoded as a space, causing + * invalid_client errors. Matches the same default in register-sso-provider.ts. + */ function selectTokenEndpointAuthMethod( supportedMethods: unknown, existing?: TokenEndpointAuthMethod ): TokenEndpointAuthMethod { if (existing) return existing if (!Array.isArray(supportedMethods) || supportedMethods.length === 0) { - return 'client_secret_basic' + return 'client_secret_post' } - if (supportedMethods.includes('client_secret_basic')) return 'client_secret_basic' if (supportedMethods.includes('client_secret_post')) return 'client_secret_post' - return 'client_secret_basic' + if (supportedMethods.includes('client_secret_basic')) return 'client_secret_basic' + return 'client_secret_post' } type DiscoveryResult = @@ -263,8 +269,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { logger.error('Failed to fetch OIDC discovery document', { discoveryResult }) return NextResponse.json( { - error: - 'Failed to fetch OIDC discovery document. Provide all endpoints explicitly or verify the issuer URL.', + error: `Failed to fetch OIDC discovery document: ${discoveryResult.error}. Provide all endpoints explicitly or verify the issuer URL.`, }, { status: 400 } ) From 45b6d31d77769678ddbfd4369d461f9d7ca9dd07 Mon Sep 17 00:00:00 2001 From: waleed Date: Fri, 3 Jul 2026 11:41:26 -0700 Subject: [PATCH 5/5] fix(sso): always resolve token auth method and skip SSRF-checking a discarded userInfoEndpoint --- .../app/api/auth/sso/register/route.test.ts | 45 +++++++++++++++++++ apps/sim/app/api/auth/sso/register/route.ts | 17 +++---- 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/apps/sim/app/api/auth/sso/register/route.test.ts b/apps/sim/app/api/auth/sso/register/route.test.ts index ca08b9cb82f..cdacc7c6c88 100644 --- a/apps/sim/app/api/auth/sso/register/route.test.ts +++ b/apps/sim/app/api/auth/sso/register/route.test.ts @@ -213,6 +213,50 @@ describe('POST /api/auth/sso/register', () => { expect(config.oidcConfig.userInfoEndpoint).toBeUndefined() }) + it('does not SSRF-validate userInfoEndpoint when skipUserInfoEndpoint is requested', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + mockValidateUrlWithDNS.mockImplementation(async (url: string, label: string) => { + if (label === 'OIDC userInfoEndpoint') { + return { isValid: false, error: 'resolves to a private IP address' } + } + return { isValid: true, resolvedIP: '1.2.3.4' } + }) + const res = await POST(request({ ...OIDC_BODY, skipUserInfoEndpoint: true, orgId: 'org1' })) + expect(res.status).toBe(200) + const config = mockRegisterSSOProvider.mock.calls[0][0].body + expect(config.oidcConfig.userInfoEndpoint).toBeUndefined() + }) + + it('does not SSRF-validate a discovered userinfo_endpoint when skipUserInfoEndpoint is requested', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + mockValidateUrlWithDNS.mockImplementation(async (url: string, label: string) => { + if (label === 'OIDC userinfo_endpoint') { + return { isValid: false, error: 'resolves to a private IP address' } + } + return { isValid: true, resolvedIP: '1.2.3.4' } + }) + mockSecureFetchWithPinnedIP.mockResolvedValue({ + ok: true, + json: async () => ({ + authorization_endpoint: 'https://idp.acme.com/authorize', + token_endpoint: 'https://idp.acme.com/token', + userinfo_endpoint: 'http://169.254.169.254/userinfo', + jwks_uri: 'https://idp.acme.com/jwks', + }), + }) + const discoveredBody = { + ...OIDC_BODY, + authorizationEndpoint: undefined, + tokenEndpoint: undefined, + jwksEndpoint: undefined, + skipUserInfoEndpoint: true, + } + const res = await POST(request({ ...discoveredBody, orgId: 'org1' })) + expect(res.status).toBe(200) + const config = mockRegisterSSOProvider.mock.calls[0][0].body + expect(config.oidcConfig.userInfoEndpoint).toBeUndefined() + }) + it('keeps userInfoEndpoint when skipUserInfoEndpoint is not requested', async () => { dbState.members = [{ organizationId: 'org1', role: 'owner' }] const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) @@ -268,6 +312,7 @@ describe('POST /api/auth/sso/register', () => { const config = mockRegisterSSOProvider.mock.calls[0][0].body expect(config.oidcConfig.skipDiscovery).toBe(true) expect(config.oidcConfig.authorizationEndpoint).toBe(OIDC_BODY.authorizationEndpoint) + expect(config.oidcConfig.tokenEndpointAuthentication).toBe('client_secret_post') }) it('prefers client_secret_post over client_secret_basic when an IdP supports both', async () => { diff --git a/apps/sim/app/api/auth/sso/register/route.ts b/apps/sim/app/api/auth/sso/register/route.ts index da7df8df66f..d826c4641fb 100644 --- a/apps/sim/app/api/auth/sso/register/route.ts +++ b/apps/sim/app/api/auth/sso/register/route.ts @@ -228,8 +228,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const userProvidedEndpoints: Record = { authorizationEndpoint, tokenEndpoint, - userInfoEndpoint, jwksEndpoint, + ...(skipUserInfoEndpoint ? {} : { userInfoEndpoint }), } for (const [name, endpointUrl] of Object.entries(userProvidedEndpoints)) { @@ -280,8 +280,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const discoveredEndpoints: Record = { authorization_endpoint: discovery.authorization_endpoint, token_endpoint: discovery.token_endpoint, - userinfo_endpoint: discovery.userinfo_endpoint, jwks_uri: discovery.jwks_uri, + ...(skipUserInfoEndpoint ? {} : { userinfo_endpoint: discovery.userinfo_endpoint }), } for (const [key, value] of Object.entries(discoveredEndpoints)) { @@ -332,17 +332,18 @@ export const POST = withRouteHandler(async (request: NextRequest) => { jwksEndpoint: oidcConfig.jwksEndpoint, }) - if (discoveryResult.ok) { - oidcConfig.tokenEndpointAuthentication = selectTokenEndpointAuthMethod( - discoveryResult.discovery.token_endpoint_auth_methods_supported, - oidcConfig.tokenEndpointAuthentication - ) - } else { + if (!discoveryResult.ok) { logger.info('OIDC discovery unavailable; falling back to the default token auth method', { providerId, discoveryUrl, }) } + oidcConfig.tokenEndpointAuthentication = selectTokenEndpointAuthMethod( + discoveryResult.ok + ? discoveryResult.discovery.token_endpoint_auth_methods_supported + : undefined, + oidcConfig.tokenEndpointAuthentication + ) } if (skipUserInfoEndpoint) {