fix(sso): support skipping the OIDC UserInfo endpoint at registration (#5386)

* fix(sso): support skipping the OIDC UserInfo endpoint at registration

* fix(sso): cap OIDC discovery fetch at 10s to avoid stalling registration

* test(sso): default-mock discovery fetch so intent is explicit

* fix(sso): prefer client_secret_post and surface discovery failure reasons

* fix(sso): always resolve token auth method and skip SSRF-checking a discarded userInfoEndpoint
This commit is contained in:
Waleed
2026-07-03 12:09:22 -07:00
committed by GitHub
parent 331875b7fe
commit e1b8200660
4 changed files with 313 additions and 92 deletions
@@ -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' }))
@@ -112,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' })
})
@@ -193,4 +196,169 @@ 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('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' }))
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)
expect(config.oidcConfig.tokenEndpointAuthentication).toBe('client_secret_post')
})
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()
})
})
+130 -91
View File
@@ -19,6 +19,53 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
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_post'
}
if (supportedMethods.includes('client_secret_post')) return 'client_secret_post'
if (supportedMethods.includes('client_secret_basic')) return 'client_secret_basic'
return 'client_secret_post'
}
type DiscoveryResult =
| { ok: true; discovery: Record<string, unknown> }
| { ok: false; error: string }
const OIDC_DISCOVERY_TIMEOUT_MS = 10000
async function fetchOIDCDiscoveryDocument(discoveryUrl: string): Promise<DiscoveryResult> {
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' },
timeout: OIDC_DISCOVERY_TIMEOUT_MS,
})
if (!response.ok) {
return { ok: false, error: `Discovery request failed with status ${response.status}` }
}
return { ok: true, discovery: (await response.json()) as Record<string, unknown> }
} catch (error) {
return { ok: false, error: getErrorMessage(error, 'Unknown error') }
}
}
export const POST = withRouteHandler(async (request: NextRequest) => {
try {
if (!env.SSO_ENABLED) {
@@ -132,6 +179,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
authorizationEndpoint,
tokenEndpoint,
userInfoEndpoint,
skipUserInfoEndpoint,
jwksEndpoint,
} = body
@@ -180,8 +228,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
const userProvidedEndpoints: Record<string, string | undefined> = {
authorizationEndpoint,
tokenEndpoint,
userInfoEndpoint,
jwksEndpoint,
...(skipUserInfoEndpoint ? {} : { userInfoEndpoint }),
}
for (const [name, endpointUrl] of Object.entries(userProvidedEndpoints)) {
@@ -206,104 +254,74 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
const needsDiscovery =
!oidcConfig.authorizationEndpoint || !oidcConfig.tokenEndpoint || !oidcConfig.jwksEndpoint
const discoveryUrl = `${issuer.replace(/\/$/, '')}/.well-known/openid-configuration`
const discoveryResult = await fetchOIDCDiscoveryDocument(discoveryUrl)
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,
})
logger.info('Fetching OIDC discovery document for missing endpoints', {
discoveryUrl,
hasAuthEndpoint: !!oidcConfig.authorizationEndpoint,
hasTokenEndpoint: !!oidcConfig.tokenEndpoint,
hasJwksEndpoint: !!oidcConfig.jwksEndpoint,
})
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 }
)
}
const discoveryResponse = await secureFetchWithPinnedIP(
discoveryUrl,
urlValidation.resolvedIP,
{
headers: { Accept: 'application/json' },
}
)
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<string, unknown>
const discoveredEndpoints: Record<string, unknown> = {
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 }
)
}
}
}
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
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,
})
if (!discoveryResult.ok) {
logger.error('Failed to fetch OIDC discovery document', { discoveryResult })
return NextResponse.json(
{
error:
'Failed to fetch OIDC discovery document. Please verify the issuer URL is correct or provide all endpoints explicitly.',
error: `Failed to fetch OIDC discovery document: ${discoveryResult.error}. Provide all endpoints explicitly or verify the issuer URL.`,
},
{ status: 400 }
)
}
const { discovery } = discoveryResult
const discoveredEndpoints: Record<string, unknown> = {
authorization_endpoint: discovery.authorization_endpoint,
token_endpoint: discovery.token_endpoint,
jwks_uri: discovery.jwks_uri,
...(skipUserInfoEndpoint ? {} : { userinfo_endpoint: discovery.userinfo_endpoint }),
}
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.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,
tokenEndpointAuthentication: oidcConfig.tokenEndpointAuthentication,
})
} else {
logger.info('Using explicitly provided OIDC endpoints (all present)', {
providerId,
@@ -313,6 +331,26 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
userInfoEndpoint: oidcConfig.userInfoEndpoint,
jwksEndpoint: oidcConfig.jwksEndpoint,
})
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) {
oidcConfig.userInfoEndpoint = undefined
logger.info('Skipping UserInfo endpoint for provider, claims will come from the ID token', {
providerId,
})
}
if (
@@ -339,6 +377,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
)
}
oidcConfig.skipDiscovery = true
providerConfig.oidcConfig = oidcConfig
} else if (providerType === 'saml') {
const {
+1
View File
@@ -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({
@@ -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<boolean> {
})
}
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 ||