diff --git a/.changeset/fluffy-falcons-shout.md b/.changeset/fluffy-falcons-shout.md new file mode 100644 index 0000000000..7cdce8229c --- /dev/null +++ b/.changeset/fluffy-falcons-shout.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-auth-backend': patch +'@backstage/plugin-auth-node': patch +--- + +Improved error forwarding for OAuth refresh endpoints diff --git a/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.test.ts b/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.test.ts index 4d94634473..d9e80263ee 100644 --- a/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.test.ts +++ b/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.test.ts @@ -272,28 +272,63 @@ describe('PassportStrategyHelper', () => { ); }); - it('should reject with an error if refresh failed', async () => { - class MyCustomOAuth2Error { - getOAuthAccessToken( - _refreshToken: string, - _options: any, - callback: Function, - ) { - callback(new Error('Unknown error')); - } - } + it('should forward simple errors', async () => { class MyCustomRefreshTokenSuccess extends passport.Strategy { - _oauth2 = new MyCustomOAuth2Error(); + _oauth2 = new (class { + getOAuthAccessToken(_r: string, _o: any, cb: Function) { + cb(new Error('Unknown error')); + } + })(); } - const mockStrategy = new MyCustomRefreshTokenSuccess(); - const refreshTokenPromise = executeRefreshTokenStrategy( - mockStrategy, - 'REFRESH_TOKEN', - 'a', + await expect( + executeRefreshTokenStrategy( + new MyCustomRefreshTokenSuccess(), + 'REFRESH_TOKEN', + 'a', + ), + ).rejects.toThrow( + 'Failed to refresh access token; caused by Error: Unknown error', ); - await expect(refreshTokenPromise).rejects.toThrow( - 'Failed to refresh access token Error: Unknown error', + }); + + it('should forward string errors', async () => { + class MyCustomRefreshTokenSuccess extends passport.Strategy { + _oauth2 = new (class { + getOAuthAccessToken(_r: string, _o: any, cb: Function) { + cb('some silly string error'); + } + })(); + } + + await expect( + executeRefreshTokenStrategy( + new MyCustomRefreshTokenSuccess(), + 'REFRESH_TOKEN', + 'a', + ), + ).rejects.toThrow( + "Failed to refresh access token; caused by unknown error 'some silly string error'", + ); + }); + + it('should forward object errors', async () => { + class MyCustomRefreshTokenSuccess extends passport.Strategy { + _oauth2 = new (class { + getOAuthAccessToken(_r: string, _o: any, cb: Function) { + cb({ name: 'SomeError', message: 'some message' }); + } + })(); + } + + await expect( + executeRefreshTokenStrategy( + new MyCustomRefreshTokenSuccess(), + 'REFRESH_TOKEN', + 'a', + ), + ).rejects.toThrow( + 'Failed to refresh access token; caused by SomeError: some message', ); }); diff --git a/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.ts b/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.ts index 88d3dbfa04..ef9284c1fb 100644 --- a/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.ts +++ b/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.ts @@ -21,6 +21,7 @@ import { InternalOAuthError } from 'passport-oauth2'; import { ProfileInfo } from '@backstage/plugin-auth-node'; import { PassportProfile } from './types'; import { OAuthStartResponse } from '../../providers/types'; +import { ForwardedError } from '@backstage/errors'; export type PassportDoneCallback = ( err?: Error, @@ -66,7 +67,10 @@ export const makeProfileInfo = ( displayName = decoded.name; } } catch (e) { - throw new Error(`Failed to parse id token and get profile info, ${e}`); + throw new ForwardedError( + `Failed to parse id token and get profile info`, + e, + ); } } @@ -176,7 +180,7 @@ export const executeRefreshTokenStrategy = async ( params: any, ) => { if (err) { - reject(new Error(`Failed to refresh access token ${err.toString()}`)); + reject(new ForwardedError(`Failed to refresh access token`, err)); } if (!accessToken) { reject( diff --git a/plugins/auth-node/src/passport/PassportHelpers.ts b/plugins/auth-node/src/passport/PassportHelpers.ts index d1ddee20a7..9c9198fbf1 100644 --- a/plugins/auth-node/src/passport/PassportHelpers.ts +++ b/plugins/auth-node/src/passport/PassportHelpers.ts @@ -19,6 +19,7 @@ import { decodeJwt } from 'jose'; import { Strategy } from 'passport'; import { PassportProfile } from './types'; import { ProfileInfo } from '../types'; +import { ForwardedError } from '@backstage/errors'; // Re-declared here to avoid direct dependency on passport-oauth2 /** @internal */ @@ -76,7 +77,10 @@ export class PassportHelpers { displayName = decoded.name; } } catch (e) { - throw new Error(`Failed to parse id token and get profile info, ${e}`); + throw new ForwardedError( + `Failed to parse id token and get profile info`, + e, + ); } } @@ -191,9 +195,7 @@ export class PassportHelpers { params: any, ) => { if (err) { - reject( - new Error(`Failed to refresh access token ${err.toString()}`), - ); + reject(new ForwardedError(`Failed to refresh access token`, err)); } if (!accessToken) { reject(