-
Notifications
You must be signed in to change notification settings - Fork 125
fix(server): handle OIDC token exchange errors gracefully & fix debug log modal #7673
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
7e6bdab
f014eed
ba4c2e2
2598128
5e0b263
94a56a6
c0c0669
7d8be22
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| 'hive': patch | ||
| --- | ||
|
|
||
| Handle OIDC token exchange errors gracefully instead of returning 500. Classifies OAuth 2.0 error codes into user-safe messages without leaking sensitive provider details. Fix OIDC debug log modal not displaying the log area. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,144 @@ | ||
| import { describeOIDCSignInError } from './oidc-provider'; | ||
|
|
||
| describe('describeOIDCSignInError', () => { | ||
| test('invalid_client error (e.g. expired client secret)', () => { | ||
| const error = new Error( | ||
| 'Received response with status 401 and body {"error":"invalid_client","error_description":"AAD2SKSASFSLKAF: The provided client secret keys for app \'369sdsds1-8513-4ssa-ae64-292942jsjs\' are expired."}', | ||
| ); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('invalid client credentials'); | ||
| expect(message).toContain('client secret has expired'); | ||
| expect(message).not.toContain('AAD2SKSASFSLKAF'); | ||
| expect(message).not.toContain('369sdsds1-8513-4ssa-ae64-292942jsjs'); | ||
| }); | ||
|
|
||
| test('invalid_grant error (e.g. expired authorization code)', () => { | ||
| const error = new Error( | ||
| 'Received response with status 400 and body {"error":"invalid_grant","error_description":"The authorization code has expired."}', | ||
| ); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('authorization code has expired'); | ||
| expect(message).toContain('try signing in again'); | ||
| }); | ||
|
|
||
| test('unauthorized_client error', () => { | ||
| const error = new Error( | ||
| 'Received response with status 403 and body {"error":"unauthorized_client","error_description":"The client is not authorized."}', | ||
| ); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('rejected the client authorization'); | ||
| expect(message).toContain('OIDC integration configuration'); | ||
| }); | ||
|
|
||
| test('invalid_request error', () => { | ||
| const error = new Error( | ||
| 'Received response with status 400 and body {"error":"invalid_request","error_description":"The request is missing a required parameter."}', | ||
| ); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('rejected the token request as malformed'); | ||
| expect(message).toContain('token endpoint URL'); | ||
| }); | ||
|
|
||
| test('unsupported_grant_type error', () => { | ||
| const error = new Error( | ||
| 'Received response with status 400 and body {"error":"unsupported_grant_type","error_description":"The authorization grant type is not supported."}', | ||
| ); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('does not support the authorization code grant type'); | ||
| }); | ||
|
|
||
| test('invalid_scope error', () => { | ||
| const error = new Error( | ||
| 'Received response with status 400 and body {"error":"invalid_scope","error_description":"The requested scope is invalid."}', | ||
| ); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('rejected the requested scopes'); | ||
| expect(message).toContain('additional scopes'); | ||
| }); | ||
|
|
||
| test('network error: ECONNREFUSED', () => { | ||
| const error = new Error( | ||
| 'request to https://login.example.com/token failed, reason: connect ECONNREFUSED 127.0.0.1:443', | ||
| ); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('Could not connect'); | ||
| expect(message).toContain('endpoint URLs'); | ||
| }); | ||
|
|
||
| test('network error: ENOTFOUND', () => { | ||
| const error = new Error( | ||
| 'request to https://nonexistent.example.com/token failed, reason: getaddrinfo ENOTFOUND nonexistent.example.com', | ||
| ); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('Could not connect'); | ||
| }); | ||
|
|
||
| test('network error: ETIMEDOUT', () => { | ||
| const error = new Error( | ||
| 'request to https://slow.example.com/token failed, reason: connect ETIMEDOUT', | ||
| ); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('Could not connect'); | ||
| }); | ||
|
|
||
| test('network error: fetch failed', () => { | ||
| const error = new TypeError('fetch failed'); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('Could not connect'); | ||
| }); | ||
|
|
||
| test('OIDC integration not found', () => { | ||
| const error = new Error('Could not find OIDC integration.'); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('could not be found'); | ||
| expect(message).toContain('contact your organization administrator'); | ||
| }); | ||
|
|
||
| test('userinfo endpoint returned non-200 status', () => { | ||
| const error = new Error( | ||
| "Received invalid status code. Could not retrieve user's profile info.", | ||
| ); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('user info endpoint returned an error'); | ||
| expect(message).toContain('verify the user info endpoint URL'); | ||
| }); | ||
|
|
||
| test('userinfo endpoint returned non-JSON response', () => { | ||
| const error = new Error('Could not parse JSON response.'); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('returned an invalid response'); | ||
| expect(message).toContain('verify the user info endpoint URL'); | ||
| }); | ||
|
|
||
| test('userinfo endpoint missing required fields (sub, email)', () => { | ||
| const error = new Error('Could not parse profile info.'); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('did not return the required fields'); | ||
| expect(message).toContain('sub, email'); | ||
| }); | ||
|
|
||
| test('unknown error returns generic message', () => { | ||
| const error = new Error('Something completely unexpected happened'); | ||
| const message = describeOIDCSignInError(error); | ||
| expect(message).toContain('unexpected error'); | ||
| expect(message).toContain('verify your OIDC integration configuration'); | ||
| }); | ||
|
|
||
| test('non-Error value is handled', () => { | ||
| const message = describeOIDCSignInError('string error with invalid_client'); | ||
| expect(message).toContain('invalid client credentials'); | ||
| }); | ||
|
|
||
| test('no sensitive information is leaked in any branch', () => { | ||
| const sensitiveError = new Error( | ||
| 'Received response with status 401 and body {"error":"invalid_client","error_description":"AADAASJAD213122: The provided client secret keys for app \'3693bbf1-8513-4cda-ae64-77e3ca237f17\' are expired. Visit the Azure portal to create new keys for your app: https://aka.ms/NewClientSecret Trace ID: b8b7152f-4489-46ed-8b78-11ad45520300 Correlation ID: 45c48f07-0191-431d-8a19-ba8319a7cd18"}', | ||
| ); | ||
| const message = describeOIDCSignInError(sensitiveError); | ||
| expect(message).not.toContain('3693bbf1'); | ||
| expect(message).not.toContain('b8b7152f'); | ||
| expect(message).not.toContain('45c48f07'); | ||
| expect(message).not.toContain('aka.ms'); | ||
| expect(message).not.toContain('Trace ID'); | ||
| expect(message).not.toContain('Correlation ID'); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -303,6 +303,66 @@ async function getOIDCConfigFromInput( | |
| return resolvedConfig; | ||
| } | ||
|
|
||
| /** | ||
| * Classify an OIDC sign-in error into a user-safe description. | ||
| * Avoids leaking sensitive details (app IDs, trace IDs, internal URLs) | ||
| * while still pointing administrators toward the likely cause. | ||
| */ | ||
| export function describeOIDCSignInError(error: unknown): string { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
|
|
||
| if (message.includes('invalid_client')) { | ||
| return 'Authentication with your OIDC provider failed due to invalid client credentials. This commonly happens when the client secret has expired or the client ID is incorrect. Please review your OIDC integration settings.'; | ||
| } | ||
|
|
||
| if (message.includes('invalid_grant')) { | ||
| return 'The authorization could not be completed. This can happen if the authorization code has expired. Please try signing in again.'; | ||
| } | ||
|
|
||
| if (message.includes('unauthorized_client')) { | ||
| return 'Your OIDC provider rejected the client authorization. Please verify your OIDC integration configuration.'; | ||
| } | ||
|
|
||
| if (message.includes('invalid_request')) { | ||
| return 'Your OIDC provider rejected the token request as malformed. This may indicate a misconfigured token endpoint URL. Please review your OIDC integration settings.'; | ||
| } | ||
|
|
||
| if (message.includes('unsupported_grant_type')) { | ||
| return 'Your OIDC provider does not support the authorization code grant type. Please verify the provider supports the OAuth 2.0 authorization code flow.'; | ||
| } | ||
|
|
||
| if (message.includes('invalid_scope')) { | ||
| return 'Your OIDC provider rejected the requested scopes. Please review the additional scopes configured in your OIDC integration settings.'; | ||
| } | ||
|
|
||
| if (message.includes('Could not find OIDC integration')) { | ||
| return 'The OIDC integration could not be found. It may have been removed or misconfigured. Please contact your organization administrator.'; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why contact org admin instead of
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On a note we should also consider that this is end user facing, people seeing this might not even have access to OIDC settings. So I think it might be okay to say, something like "please contact your organization admin about the OIDC configuration"? |
||
| } | ||
|
|
||
| if (message.includes("Could not retrieve user's profile info")) { | ||
| return "Your OIDC provider's user info endpoint returned an error. Please verify the user info endpoint URL in your OIDC integration settings is correct."; | ||
| } | ||
|
|
||
| if (message.includes('Could not parse JSON response')) { | ||
| return "Your OIDC provider's user info endpoint returned an invalid response. Please verify the user info endpoint URL in your OIDC integration settings is correct."; | ||
| } | ||
|
|
||
| if (message.includes('Could not parse profile info')) { | ||
| return "Your OIDC provider's user info endpoint did not return the required fields (sub, email). Please verify your OIDC provider is configured to include these claims."; | ||
| } | ||
|
|
||
| if ( | ||
| message.includes('ECONNREFUSED') || | ||
| message.includes('ENOTFOUND') || | ||
| message.includes('ETIMEDOUT') || | ||
| message.includes('fetch failed') | ||
| ) { | ||
| return 'Could not connect to your OIDC provider. Please verify the endpoint URLs in your OIDC integration settings are correct and the server is accessible.'; | ||
| } | ||
|
|
||
| return 'An unexpected error occurred while authenticating with your OIDC provider. Please verify your OIDC integration configuration or contact your administrator.'; | ||
adambenhassen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| const fetchOIDCConfig = async ( | ||
| internalApi: InternalApiCaller, | ||
| logger: FastifyBaseLogger, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally I try to avoid matching on message text. Are there any other fields we can match on that have more guarantees than the message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Valid concern in general, but I've investigated the supertokens source and
exchangeAuthCodeForOAuthTokensthrows plain Error objects with this format{"error":"invalid_client","error_description":"..."}There are no structured fields (error.code, typed subclasses, etc..), everything is embedded in the message string. the OAuth2 error codes (invalid_client, invalid_grant, etc) only exist inside the stringified response body within the message.