diff --git a/plugins/auth-backend-module-pinniped-provider/package.json b/plugins/auth-backend-module-pinniped-provider/package.json index 087a90cdbb..5541d008da 100644 --- a/plugins/auth-backend-module-pinniped-provider/package.json +++ b/plugins/auth-backend-module-pinniped-provider/package.json @@ -25,14 +25,15 @@ "dependencies": { "@backstage/backend-common": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", + "@backstage/config": "workspace:^", "@backstage/plugin-auth-node": "workspace:^", + "luxon": "^3.4.3", "openid-client": "^5.4.3" }, "devDependencies": { "@backstage/backend-defaults": "workspace:^", "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", - "@backstage/config": "workspace:^", "@backstage/plugin-auth-backend": "workspace:^", "cookie-parser": "^1.4.6", "express": "^4.18.2", diff --git a/plugins/auth-backend-module-pinniped-provider/src/authenticator.test.ts b/plugins/auth-backend-module-pinniped-provider/src/authenticator.test.ts index f908c1ea13..8b4c092e3a 100644 --- a/plugins/auth-backend-module-pinniped-provider/src/authenticator.test.ts +++ b/plugins/auth-backend-module-pinniped-provider/src/authenticator.test.ts @@ -21,7 +21,7 @@ import { decodeOAuthState, encodeOAuthState, } from '@backstage/plugin-auth-node'; -import { pinnipedAuthenticator } from './authenticator'; +import { pinnipedAuthenticator, getIssuer } from './authenticator'; import { setupServer } from 'msw/node'; import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; import { ConfigReader } from '@backstage/config'; @@ -30,7 +30,7 @@ import { rest } from 'msw'; import express from 'express'; describe('pinnipedAuthenticator', () => { - let implementation: any; + let authCtx: any; let oauthState: OAuthState; let idToken: string; let publicKey: JWK; @@ -128,7 +128,7 @@ describe('pinnipedAuthenticator', () => { }), ); - implementation = pinnipedAuthenticator.initialize({ + authCtx = pinnipedAuthenticator.initialize({ callbackUrl: 'https://backstage.test/callback', config: new ConfigReader({ federationDomain: 'https://federationDomain.test', @@ -143,6 +143,27 @@ describe('pinnipedAuthenticator', () => { }; }); + describe('#initialize', () => { + it('always returns a PinnipedStrategyFactory', async () => { + const pinnipedStrategyFactory = pinnipedAuthenticator.initialize({ + callbackUrl: 'https://backstage.test/callback', + config: new ConfigReader({ + federationDomain: 'https://federationDomain.test', + clientId: 'clientId', + clientSecret: 'clientSecret', + }), + }); + + const { providerStrategy, client } = + await pinnipedStrategyFactory.getStrategy(); + + expect(providerStrategy).toBeDefined(); + expect(client.issuer.authorization_endpoint).toMatch( + 'https://pinniped.test/oauth2/authorize', + ); + }); + }); + describe('#start', () => { let fakeSession: Record; let startRequest: OAuthAuthenticatorStartInput; @@ -162,7 +183,7 @@ describe('pinnipedAuthenticator', () => { it('redirects to authorization endpoint returned from OIDC metadata endpoint', async () => { const startResponse = await pinnipedAuthenticator.start( startRequest, - implementation, + authCtx, ); const url = new URL(startResponse.url); @@ -174,7 +195,7 @@ describe('pinnipedAuthenticator', () => { it('initiates authorization code grant', async () => { const startResponse = await pinnipedAuthenticator.start( startRequest, - implementation, + authCtx, ); const { searchParams } = new URL(startResponse.url); @@ -185,7 +206,7 @@ describe('pinnipedAuthenticator', () => { startRequest.req.query = { audience: 'test-cluster' }; const startResponse = await pinnipedAuthenticator.start( startRequest, - implementation, + authCtx, ); const { searchParams } = new URL(startResponse.url); const stateParam = searchParams.get('state'); @@ -201,7 +222,7 @@ describe('pinnipedAuthenticator', () => { it('passes client ID from config', async () => { const startResponse = await pinnipedAuthenticator.start( startRequest, - implementation, + authCtx, ); const { searchParams } = new URL(startResponse.url); @@ -211,7 +232,7 @@ describe('pinnipedAuthenticator', () => { it('passes callback URL from config', async () => { const startResponse = await pinnipedAuthenticator.start( startRequest, - implementation, + authCtx, ); const { searchParams } = new URL(startResponse.url); @@ -223,7 +244,7 @@ describe('pinnipedAuthenticator', () => { it('generates PKCE challenge', async () => { const startResponse = await pinnipedAuthenticator.start( startRequest, - implementation, + authCtx, ); const { searchParams } = new URL(startResponse.url); @@ -232,14 +253,14 @@ describe('pinnipedAuthenticator', () => { }); it('stores PKCE verifier in session', async () => { - await pinnipedAuthenticator.start(startRequest, implementation); + await pinnipedAuthenticator.start(startRequest, authCtx); expect(fakeSession['oidc:pinniped.test'].code_verifier).toBeDefined(); }); it('requests sufficient scopes for token exchange by default', async () => { const startResponse = await pinnipedAuthenticator.start( startRequest, - implementation, + authCtx, ); const { searchParams } = new URL(startResponse.url); const scopes = searchParams.get('scope')?.split(' ') ?? []; @@ -257,7 +278,7 @@ describe('pinnipedAuthenticator', () => { it('encodes OAuth state in query param', async () => { const startResponse = await pinnipedAuthenticator.start( startRequest, - implementation, + authCtx, ); const { searchParams } = new URL(startResponse.url); const stateParam = searchParams.get('state'); @@ -276,10 +297,94 @@ describe('pinnipedAuthenticator', () => { url: 'test', }, } as unknown as OAuthAuthenticatorStartInput, - implementation, + authCtx, ), ).rejects.toThrow('authentication requires session support'); }); + + it('refreshes oidc metadata after a failed fetch', async () => { + mswServer.use( + rest.get( + 'https://federationDomain.test/.well-known/openid-configuration', + (_req, res, _ctx) => res.networkError('Timeout'), + ), + ); + + const authCtxCreatedWhileSupervisorUnavailable = + pinnipedAuthenticator.initialize({ + callbackUrl: 'https://backstage.test/callback', + config: new ConfigReader({ + federationDomain: 'https://federationDomain.test', + clientId: 'clientId', + clientSecret: 'clientSecret', + }), + }); + + mswServer.use( + rest.get( + 'https://federationDomain.test/.well-known/openid-configuration', + (_req, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(issuerMetadata), + ), + ), + ); + + const response = await pinnipedAuthenticator.start( + startRequest, + authCtxCreatedWhileSupervisorUnavailable, + ); + + expect(response.url).toMatch('https://pinniped.test/oauth2/authorize'); + }); + + it('only refreshes metadata after a failure', async () => { + mswServer.use( + rest.get( + 'https://federationDomain.test/.well-known/openid-configuration', + (_req, res, _ctx) => res.networkError('Timeout'), + ), + ); + + const authCtxCreatedWhileSupervisorUnavailable = + pinnipedAuthenticator.initialize({ + callbackUrl: 'https://backstage.test/callback', + config: new ConfigReader({ + federationDomain: 'https://federationDomain.test', + clientId: 'clientId', + clientSecret: 'clientSecret', + }), + }); + + let supervisorCalls: number = 0; + + mswServer.use( + rest.get( + 'https://federationDomain.test/.well-known/openid-configuration', + (_req, res, ctx) => { + supervisorCalls += 1; + return res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(issuerMetadata), + ); + }, + ), + ); + + await pinnipedAuthenticator.start( + startRequest, + authCtxCreatedWhileSupervisorUnavailable, + ); + await pinnipedAuthenticator.start( + startRequest, + authCtxCreatedWhileSupervisorUnavailable, + ); + + expect(supervisorCalls).toEqual(1); + }); }); describe('#authenticate', () => { @@ -304,7 +409,7 @@ describe('pinnipedAuthenticator', () => { it('exchanges authorization code for access token', async () => { const handlerResponse = await pinnipedAuthenticator.authenticate( handlerRequest, - implementation, + authCtx, ); const accessToken = handlerResponse.session.accessToken; @@ -314,7 +419,7 @@ describe('pinnipedAuthenticator', () => { it('exchanges authorization code for refresh token', async () => { const handlerResponse = await pinnipedAuthenticator.authenticate( handlerRequest, - implementation, + authCtx, ); const refreshToken = handlerResponse.session.refreshToken; @@ -324,7 +429,7 @@ describe('pinnipedAuthenticator', () => { it('returns granted scope', async () => { const handlerResponse = await pinnipedAuthenticator.authenticate( handlerRequest, - implementation, + authCtx, ); const responseScope = handlerResponse.session.scope; @@ -349,7 +454,7 @@ describe('pinnipedAuthenticator', () => { const handlerResponse = await pinnipedAuthenticator.authenticate( handlerRequest, - implementation, + authCtx, ); expect(handlerResponse.session.idToken).toEqual(clusterScopedIdToken); @@ -413,7 +518,7 @@ describe('pinnipedAuthenticator', () => { }; await expect( - pinnipedAuthenticator.authenticate(handlerRequest, implementation), + pinnipedAuthenticator.authenticate(handlerRequest, authCtx), ).rejects.toThrow( `Failed to get cluster specific ID token for "test_cluster": Error: RFC8693 token exchange failed with error: NetworkError: Connection timed out`, ); @@ -422,7 +527,7 @@ describe('pinnipedAuthenticator', () => { it('fails without authorization code', async () => { handlerRequest.req.url = 'https://test.com'; return expect( - pinnipedAuthenticator.authenticate(handlerRequest, implementation), + pinnipedAuthenticator.authenticate(handlerRequest, authCtx), ).rejects.toThrow('Unexpected redirect'); }); @@ -440,7 +545,7 @@ describe('pinnipedAuthenticator', () => { }, } as unknown as express.Request, }, - implementation, + authCtx, ), ).rejects.toThrow( 'Authentication rejected, state missing from the response', @@ -456,10 +561,105 @@ describe('pinnipedAuthenticator', () => { url: 'https://test.com', } as unknown as express.Request, }, - implementation, + authCtx, ), ).rejects.toThrow('authentication requires session support'); }); + + it('refreshes oidc metadata after a failed fetch', async () => { + mswServer.use( + rest.get( + 'https://federationDomain.test/.well-known/openid-configuration', + (_req, res, _ctx) => res.networkError('Timeout'), + ), + ); + + const authCtxCreatedWhileSupervisorUnavailable = + pinnipedAuthenticator.initialize({ + callbackUrl: 'https://backstage.test/callback', + config: new ConfigReader({ + federationDomain: 'https://federationDomain.test', + clientId: 'clientId', + clientSecret: 'clientSecret', + }), + }); + + mswServer.use( + rest.get( + 'https://federationDomain.test/.well-known/openid-configuration', + (_req, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(issuerMetadata), + ), + ), + ); + + const response = await pinnipedAuthenticator.authenticate( + handlerRequest, + authCtxCreatedWhileSupervisorUnavailable, + ); + expect(response.session.accessToken).toEqual('accessToken'); + }); + + it('only refreshes metadata after a failure', async () => { + mswServer.use( + rest.get( + 'https://federationDomain.test/.well-known/openid-configuration', + (_req, res, _ctx) => res.networkError('Timeout'), + ), + ); + + const authCtxCreatedWhileSupervisorUnavailable = + pinnipedAuthenticator.initialize({ + callbackUrl: 'https://backstage.test/callback', + config: new ConfigReader({ + federationDomain: 'https://federationDomain.test', + clientId: 'clientId', + clientSecret: 'clientSecret', + }), + }); + + let supervisorCalls: number = 0; + + mswServer.use( + rest.get( + 'https://federationDomain.test/.well-known/openid-configuration', + (_req, res, ctx) => { + supervisorCalls += 1; + return res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(issuerMetadata), + ); + }, + ), + ); + + await pinnipedAuthenticator.authenticate( + handlerRequest, + authCtxCreatedWhileSupervisorUnavailable, + ); + await pinnipedAuthenticator.authenticate( + { + req: { + method: 'GET', + url: `https://test?code=authorization_code&state=${encodeOAuthState( + oauthState, + )}`, + session: { + 'oidc:pinniped.test': { + state: encodeOAuthState(oauthState), + }, + }, + } as unknown as express.Request, + }, + authCtxCreatedWhileSupervisorUnavailable, + ); + + expect(supervisorCalls).toEqual(1); + }); }); describe('#refresh', () => { @@ -476,7 +676,7 @@ describe('pinnipedAuthenticator', () => { it('gets new refresh token', async () => { const refreshResponse = await pinnipedAuthenticator.refresh( refreshRequest, - implementation, + authCtx, ); expect(refreshResponse.session.refreshToken).toBe('refreshToken'); @@ -485,7 +685,7 @@ describe('pinnipedAuthenticator', () => { it('gets access token', async () => { const refreshResponse = await pinnipedAuthenticator.refresh( refreshRequest, - implementation, + authCtx, ); expect(refreshResponse.session.accessToken).toBe('accessToken'); @@ -494,10 +694,93 @@ describe('pinnipedAuthenticator', () => { it('gets id token', async () => { const refreshResponse = await pinnipedAuthenticator.refresh( refreshRequest, - implementation, + authCtx, ); expect(refreshResponse.session.idToken).toBe(idToken); }); + + it('refreshes oidc metadata after a failed fetch', async () => { + mswServer.use( + rest.get( + 'https://federationDomain.test/.well-known/openid-configuration', + (_req, res, _ctx) => res.networkError('Timeout'), + ), + ); + + const authCtxCreatedWhileSupervisorUnavailable = + pinnipedAuthenticator.initialize({ + callbackUrl: 'https://backstage.test/callback', + config: new ConfigReader({ + federationDomain: 'https://federationDomain.test', + clientId: 'clientId', + clientSecret: 'clientSecret', + }), + }); + + mswServer.use( + rest.get( + 'https://federationDomain.test/.well-known/openid-configuration', + (_req, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(issuerMetadata), + ), + ), + ); + + const response = await pinnipedAuthenticator.refresh( + refreshRequest, + authCtxCreatedWhileSupervisorUnavailable, + ); + expect(response.session.accessToken).toEqual('accessToken'); + }); + + it('only refreshes metadata after a failure', async () => { + mswServer.use( + rest.get( + 'https://federationDomain.test/.well-known/openid-configuration', + (_req, res, _ctx) => res.networkError('Timeout'), + ), + ); + + const authCtxCreatedWhileSupervisorUnavailable = + pinnipedAuthenticator.initialize({ + callbackUrl: 'https://backstage.test/callback', + config: new ConfigReader({ + federationDomain: 'https://federationDomain.test', + clientId: 'clientId', + clientSecret: 'clientSecret', + }), + }); + + let supervisorCalls: number = 0; + + mswServer.use( + rest.get( + 'https://federationDomain.test/.well-known/openid-configuration', + (_req, res, ctx) => { + supervisorCalls += 1; + return res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(issuerMetadata), + ); + }, + ), + ); + + await pinnipedAuthenticator.refresh( + refreshRequest, + authCtxCreatedWhileSupervisorUnavailable, + ); + await pinnipedAuthenticator.refresh( + refreshRequest, + authCtxCreatedWhileSupervisorUnavailable, + ); + + expect(supervisorCalls).toEqual(1); + }); }); }); diff --git a/plugins/auth-backend-module-pinniped-provider/src/authenticator.ts b/plugins/auth-backend-module-pinniped-provider/src/authenticator.ts index 83c4204ad0..91aca3140e 100644 --- a/plugins/auth-backend-module-pinniped-provider/src/authenticator.ts +++ b/plugins/auth-backend-module-pinniped-provider/src/authenticator.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { Config } from '@backstage/config'; import { PassportDoneCallback } from '@backstage/plugin-auth-node'; import { createOAuthAuthenticator, @@ -24,7 +25,9 @@ import { Issuer, TokenSet, Strategy as OidcStrategy, + BaseClient, } from 'openid-client'; +import { DateTime } from 'luxon'; const rfc8693TokenExchange = async ({ subject_token, @@ -53,22 +56,74 @@ const rfc8693TokenExchange = async ({ }); }; -/** @public */ -export const pinnipedAuthenticator = createOAuthAuthenticator({ - defaultProfileTransform: async (_r, _c) => ({ profile: {} }), - async initialize({ callbackUrl, config }) { +class PinnipedStrategyFactory { + private readonly callbackUrl: string; + private readonly config: Config; + private strategyPromise: Promise<{ + providerStrategy: OidcStrategy<{ tokenset: TokenSet }, BaseClient>; + client: BaseClient; + }>; + + private cachedPromise?: Promise<{ + providerStrategy: OidcStrategy<{ tokenset: TokenSet }, BaseClient>; + client: BaseClient; + }>; + private cachedPromiseExpiry?: Date; + + constructor(callbackUrl: string, config: Config) { + this.callbackUrl = callbackUrl; + this.config = config; + this.strategyPromise = this.buildStrategy(); + } + + // need some sort of unit test to capture that a cached strategy will retry if expiration date is expired + public async getStrategy(): Promise<{ + providerStrategy: OidcStrategy<{ tokenset: TokenSet }, BaseClient>; + client: BaseClient; + }> { + if (this.cachedPromise) { + if ( + this.cachedPromiseExpiry && + DateTime.fromJSDate(this.cachedPromiseExpiry) > DateTime.local() + ) { + return this.cachedPromise; + } + // cachedPromise has expired, generate new strategy + delete this.cachedPromise; + } + + this.cachedPromise = this.strategyPromise; + this.cachedPromiseExpiry = DateTime.utc() + .plus({ seconds: 3600 }) + .toJSDate(); + try { + // if we fail to generate a strategy, retry and overwrite strategy + await this.strategyPromise; + } catch (error) { + this.strategyPromise = this.buildStrategy(); + delete this.cachedPromise; + delete this.cachedPromiseExpiry; + } + + return this.strategyPromise; + } + + private async buildStrategy(): Promise<{ + providerStrategy: OidcStrategy<{ tokenset: TokenSet }, BaseClient>; + client: BaseClient; + }> { const issuer = await Issuer.discover( - `${config.getString( + `${this.config.getString( 'federationDomain', )}/.well-known/openid-configuration`, ); const client = new issuer.Client({ - access_type: 'offline', // this option must be passed to provider to receive a refresh token - client_id: config.getString('clientId'), - client_secret: config.getString('clientSecret'), - redirect_uris: [callbackUrl], + access_type: 'offline', + client_id: this.config.getString('clientId'), + client_secret: this.config.getString('clientSecret'), + redirect_uris: [this.callbackUrl], response_types: ['code'], - scope: config.getOptionalString('scope') || '', + scope: this.config.getOptionalString('scope') || '', id_token_signed_response_alg: 'ES256', }); const providerStrategy = new OidcStrategy( @@ -88,12 +143,18 @@ export const pinnipedAuthenticator = createOAuthAuthenticator({ done(undefined, { tokenset }, {}); }, ); - return { providerStrategy, client }; - }, + } +} - async start(input, ctx) { - const { providerStrategy } = await ctx; +/** @public */ +export const pinnipedAuthenticator = createOAuthAuthenticator({ + defaultProfileTransform: async (_r, _c) => ({ profile: {} }), + initialize({ callbackUrl, config }) { + return new PinnipedStrategyFactory(callbackUrl, config); + }, + async start(input, ctx): Promise<{ url: string; status?: number }> { + const { providerStrategy } = await ctx.getStrategy(); const stringifiedAudience = input.req.query?.audience as string; const decodedState = decodeOAuthState(input.state); const state = { ...decodedState, audience: stringifiedAudience }; @@ -117,7 +178,7 @@ export const pinnipedAuthenticator = createOAuthAuthenticator({ }, async authenticate(input, ctx) { - const { providerStrategy } = await ctx; + const { providerStrategy } = await ctx.getStrategy(); const { req } = input; const { searchParams } = new URL(req.url, 'https://pinniped.com'); const stateParam = searchParams.get('state'); @@ -132,7 +193,7 @@ export const pinnipedAuthenticator = createOAuthAuthenticator({ ? rfc8693TokenExchange({ subject_token: user.tokenset.access_token, target_audience: audience, - ctx, + ctx: ctx.getStrategy(), }).catch(err => reject( new Error( @@ -172,7 +233,7 @@ export const pinnipedAuthenticator = createOAuthAuthenticator({ }, async refresh(input, ctx) { - const { client } = await ctx; + const { client } = await ctx.getStrategy(); const tokenset = await client.refresh(input.refreshToken); return new Promise((resolve, reject) => { diff --git a/yarn.lock b/yarn.lock index 956c7a7895..fef1b6c95a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5038,6 +5038,7 @@ __metadata: express-promise-router: ^4.1.1 express-session: ^1.17.3 jose: ^4.14.6 + luxon: ^3.4.3 msw: ^1.3.0 openid-client: ^5.4.3 passport: ^0.6.0