Skip to content

Commit 2c8c116

Browse files
waleedlatif1waleedlatif1
committed
tested edge cases, ack'd PR comments
1 parent 05324ad commit 2c8c116

File tree

2 files changed

+106
-67
lines changed

2 files changed

+106
-67
lines changed

‎apps/sim/app/api/auth/sso/register/route.ts‎

Lines changed: 54 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -138,26 +138,23 @@ export async function POST(request: NextRequest) {
138138
pkce: pkce ?? true,
139139
}
140140

141-
const hasExplicitEndpoints = authorizationEndpoint && tokenEndpoint && jwksEndpoint
141+
oidcConfig.authorizationEndpoint = authorizationEndpoint
142+
oidcConfig.tokenEndpoint = tokenEndpoint
143+
oidcConfig.userInfoEndpoint = userInfoEndpoint
144+
oidcConfig.jwksEndpoint = jwksEndpoint
142145

143-
if (hasExplicitEndpoints) {
144-
oidcConfig.authorizationEndpoint = authorizationEndpoint
145-
oidcConfig.tokenEndpoint = tokenEndpoint
146-
oidcConfig.userInfoEndpoint = userInfoEndpoint
147-
oidcConfig.jwksEndpoint = jwksEndpoint
146+
const needsDiscovery =
147+
!oidcConfig.authorizationEndpoint || !oidcConfig.tokenEndpoint || !oidcConfig.jwksEndpoint
148148

149-
logger.info('Using explicitly provided OIDC endpoints', {
150-
providerId,
151-
issuer,
152-
authorizationEndpoint: oidcConfig.authorizationEndpoint,
153-
tokenEndpoint: oidcConfig.tokenEndpoint,
154-
userInfoEndpoint: oidcConfig.userInfoEndpoint,
155-
jwksEndpoint: oidcConfig.jwksEndpoint,
156-
})
157-
} else {
149+
if (needsDiscovery) {
158150
const discoveryUrl = `${issuer.replace(/\/$/, '')}/.well-known/openid-configuration`
159151
try {
160-
logger.info('Fetching OIDC discovery document', { discoveryUrl })
152+
logger.info('Fetching OIDC discovery document for missing endpoints', {
153+
discoveryUrl,
154+
hasAuthEndpoint: !!oidcConfig.authorizationEndpoint,
155+
hasTokenEndpoint: !!oidcConfig.tokenEndpoint,
156+
hasJwksEndpoint: !!oidcConfig.jwksEndpoint,
157+
})
161158

162159
const discoveryResponse = await fetch(discoveryUrl, {
163160
headers: { Accept: 'application/json' },
@@ -170,39 +167,21 @@ export async function POST(request: NextRequest) {
170167
})
171168
return NextResponse.json(
172169
{
173-
error: `Failed to fetch OIDC discovery document from ${discoveryUrl}. Status: ${discoveryResponse.status}`,
170+
error: `Failed to fetch OIDC discovery document from ${discoveryUrl}. Status: ${discoveryResponse.status}. Provide all endpoints explicitly or verify the issuer URL.`,
174171
},
175172
{ status: 400 }
176173
)
177174
}
178175

179176
const discovery = await discoveryResponse.json()
180177

181-
if (
182-
!discovery.authorization_endpoint ||
183-
!discovery.token_endpoint ||
184-
!discovery.jwks_uri
185-
) {
186-
logger.error('OIDC discovery document missing required endpoints', {
187-
hasAuthEndpoint: !!discovery.authorization_endpoint,
188-
hasTokenEndpoint: !!discovery.token_endpoint,
189-
hasJwksUri: !!discovery.jwks_uri,
190-
})
191-
return NextResponse.json(
192-
{
193-
error:
194-
'OIDC discovery document is missing required endpoints (authorization_endpoint, token_endpoint, jwks_uri)',
195-
},
196-
{ status: 400 }
197-
)
198-
}
199-
200-
oidcConfig.authorizationEndpoint = discovery.authorization_endpoint
201-
oidcConfig.tokenEndpoint = discovery.token_endpoint
202-
oidcConfig.userInfoEndpoint = discovery.userinfo_endpoint
203-
oidcConfig.jwksEndpoint = discovery.jwks_uri
178+
oidcConfig.authorizationEndpoint =
179+
oidcConfig.authorizationEndpoint || discovery.authorization_endpoint
180+
oidcConfig.tokenEndpoint = oidcConfig.tokenEndpoint || discovery.token_endpoint
181+
oidcConfig.userInfoEndpoint = oidcConfig.userInfoEndpoint || discovery.userinfo_endpoint
182+
oidcConfig.jwksEndpoint = oidcConfig.jwksEndpoint || discovery.jwks_uri
204183

205-
logger.info('Successfully fetched OIDC endpoints from discovery', {
184+
logger.info('Merged OIDC endpoints (user-provided + discovery)', {
206185
providerId,
207186
issuer,
208187
authorizationEndpoint: oidcConfig.authorizationEndpoint,
@@ -217,11 +196,44 @@ export async function POST(request: NextRequest) {
217196
})
218197
return NextResponse.json(
219198
{
220-
error: `Failed to fetch OIDC discovery document from ${discoveryUrl}. Please verify the issuer URL is correct.`,
199+
error: `Failed to fetch OIDC discovery document from ${discoveryUrl}. Please verify the issuer URL is correct or provide all endpoints explicitly.`,
221200
},
222201
{ status: 400 }
223202
)
224203
}
204+
} else {
205+
logger.info('Using explicitly provided OIDC endpoints (all present)', {
206+
providerId,
207+
issuer,
208+
authorizationEndpoint: oidcConfig.authorizationEndpoint,
209+
tokenEndpoint: oidcConfig.tokenEndpoint,
210+
userInfoEndpoint: oidcConfig.userInfoEndpoint,
211+
jwksEndpoint: oidcConfig.jwksEndpoint,
212+
})
213+
}
214+
215+
if (
216+
!oidcConfig.authorizationEndpoint ||
217+
!oidcConfig.tokenEndpoint ||
218+
!oidcConfig.jwksEndpoint
219+
) {
220+
const missing: string[] = []
221+
if (!oidcConfig.authorizationEndpoint) missing.push('authorizationEndpoint')
222+
if (!oidcConfig.tokenEndpoint) missing.push('tokenEndpoint')
223+
if (!oidcConfig.jwksEndpoint) missing.push('jwksEndpoint')
224+
225+
logger.error('Missing required OIDC endpoints after discovery merge', {
226+
missing,
227+
authorizationEndpoint: oidcConfig.authorizationEndpoint,
228+
tokenEndpoint: oidcConfig.tokenEndpoint,
229+
jwksEndpoint: oidcConfig.jwksEndpoint,
230+
})
231+
return NextResponse.json(
232+
{
233+
error: `Missing required OIDC endpoints: ${missing.join(', ')}. Please provide these explicitly or verify the issuer supports OIDC discovery.`,
234+
},
235+
{ status: 400 }
236+
)
225237
}
226238

227239
providerConfig.oidcConfig = oidcConfig

‎packages/db/scripts/register-sso-provider.ts‎

Lines changed: 52 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -391,16 +391,21 @@ async function registerSSOProvider(): Promise<boolean> {
391391
}
392392

393393
if (ssoConfig.providerType === 'oidc' && ssoConfig.oidcConfig) {
394-
const hasAllEndpoints =
395-
ssoConfig.oidcConfig.authorizationEndpoint &&
396-
ssoConfig.oidcConfig.tokenEndpoint &&
397-
ssoConfig.oidcConfig.jwksEndpoint
394+
const needsDiscovery =
395+
!ssoConfig.oidcConfig.authorizationEndpoint ||
396+
!ssoConfig.oidcConfig.tokenEndpoint ||
397+
!ssoConfig.oidcConfig.jwksEndpoint
398398

399-
if (!hasAllEndpoints) {
399+
if (needsDiscovery) {
400400
const discoveryUrl =
401401
ssoConfig.oidcConfig.discoveryEndpoint ||
402402
`${ssoConfig.issuer.replace(/\/$/, '')}/.well-known/openid-configuration`
403-
logger.info('Fetching OIDC discovery document...', { discoveryUrl })
403+
logger.info('Fetching OIDC discovery document for missing endpoints...', {
404+
discoveryUrl,
405+
hasAuthEndpoint: !!ssoConfig.oidcConfig.authorizationEndpoint,
406+
hasTokenEndpoint: !!ssoConfig.oidcConfig.tokenEndpoint,
407+
hasJwksEndpoint: !!ssoConfig.oidcConfig.jwksEndpoint,
408+
})
404409

405410
try {
406411
const response = await fetch(discoveryUrl, {
@@ -412,30 +417,24 @@ async function registerSSOProvider(): Promise<boolean> {
412417
status: response.status,
413418
statusText: response.statusText,
414419
})
420+
logger.error(
421+
'Provide all endpoints explicitly via SSO_OIDC_AUTHORIZATION_ENDPOINT, SSO_OIDC_TOKEN_ENDPOINT, SSO_OIDC_JWKS_ENDPOINT'
422+
)
415423
return false
416424
}
417425

418426
const discovery = await response.json()
419427

420-
if (
421-
!discovery.authorization_endpoint ||
422-
!discovery.token_endpoint ||
423-
!discovery.jwks_uri
424-
) {
425-
logger.error('OIDC discovery document missing required endpoints', {
426-
hasAuthEndpoint: !!discovery.authorization_endpoint,
427-
hasTokenEndpoint: !!discovery.token_endpoint,
428-
hasJwksUri: !!discovery.jwks_uri,
429-
})
430-
return false
431-
}
432-
433-
ssoConfig.oidcConfig.authorizationEndpoint = discovery.authorization_endpoint
434-
ssoConfig.oidcConfig.tokenEndpoint = discovery.token_endpoint
435-
ssoConfig.oidcConfig.userInfoEndpoint = discovery.userinfo_endpoint
436-
ssoConfig.oidcConfig.jwksEndpoint = discovery.jwks_uri
428+
ssoConfig.oidcConfig.authorizationEndpoint =
429+
ssoConfig.oidcConfig.authorizationEndpoint || discovery.authorization_endpoint
430+
ssoConfig.oidcConfig.tokenEndpoint =
431+
ssoConfig.oidcConfig.tokenEndpoint || discovery.token_endpoint
432+
ssoConfig.oidcConfig.userInfoEndpoint =
433+
ssoConfig.oidcConfig.userInfoEndpoint || discovery.userinfo_endpoint
434+
ssoConfig.oidcConfig.jwksEndpoint =
435+
ssoConfig.oidcConfig.jwksEndpoint || discovery.jwks_uri
437436

438-
logger.info('✅ Successfully fetched OIDC endpoints from discovery', {
437+
logger.info('Merged OIDC endpoints (user-provided + discovery)', {
439438
authorizationEndpoint: ssoConfig.oidcConfig.authorizationEndpoint,
440439
tokenEndpoint: ssoConfig.oidcConfig.tokenEndpoint,
441440
userInfoEndpoint: ssoConfig.oidcConfig.userInfoEndpoint,
@@ -447,10 +446,38 @@ async function registerSSOProvider(): Promise<boolean> {
447446
discoveryUrl,
448447
})
449448
logger.error(
450-
'Please provide explicit endpoints via SSO_OIDC_AUTHORIZATION_ENDPOINT, etc.'
449+
'Please provide explicit endpoints via SSO_OIDC_AUTHORIZATION_ENDPOINT, SSO_OIDC_TOKEN_ENDPOINT, SSO_OIDC_JWKS_ENDPOINT'
451450
)
452451
return false
453452
}
453+
} else {
454+
logger.info('Using explicitly provided OIDC endpoints (all present)', {
455+
authorizationEndpoint: ssoConfig.oidcConfig.authorizationEndpoint,
456+
tokenEndpoint: ssoConfig.oidcConfig.tokenEndpoint,
457+
userInfoEndpoint: ssoConfig.oidcConfig.userInfoEndpoint,
458+
jwksEndpoint: ssoConfig.oidcConfig.jwksEndpoint,
459+
})
460+
}
461+
462+
if (
463+
!ssoConfig.oidcConfig.authorizationEndpoint ||
464+
!ssoConfig.oidcConfig.tokenEndpoint ||
465+
!ssoConfig.oidcConfig.jwksEndpoint
466+
) {
467+
const missing: string[] = []
468+
if (!ssoConfig.oidcConfig.authorizationEndpoint)
469+
missing.push('SSO_OIDC_AUTHORIZATION_ENDPOINT')
470+
if (!ssoConfig.oidcConfig.tokenEndpoint) missing.push('SSO_OIDC_TOKEN_ENDPOINT')
471+
if (!ssoConfig.oidcConfig.jwksEndpoint) missing.push('SSO_OIDC_JWKS_ENDPOINT')
472+
473+
logger.error('Missing required OIDC endpoints after discovery merge', {
474+
missing,
475+
authorizationEndpoint: ssoConfig.oidcConfig.authorizationEndpoint,
476+
tokenEndpoint: ssoConfig.oidcConfig.tokenEndpoint,
477+
jwksEndpoint: ssoConfig.oidcConfig.jwksEndpoint,
478+
})
479+
logger.error(`Please provide: ${missing.join(', ')}`)
480+
return false
454481
}
455482
}
456483

0 commit comments

Comments
 (0)