From a8f6afda4a26e8242104ea850d5b698b6c7bc980 Mon Sep 17 00:00:00 2001 From: Ruben Vallejo Date: Mon, 30 Oct 2023 13:42:52 -0400 Subject: [PATCH] PR-chore: changeset,api-report Signed-off-by: Ruben Vallejo --- .changeset/metal-hornets-drum.md | 5 + .../api-report.md | 16 ++- .../src/authenticator.test.ts | 128 +++--------------- .../src/authenticator.ts | 9 +- .../src/index.ts | 2 +- 5 files changed, 42 insertions(+), 118 deletions(-) create mode 100644 .changeset/metal-hornets-drum.md diff --git a/.changeset/metal-hornets-drum.md b/.changeset/metal-hornets-drum.md new file mode 100644 index 0000000000..c7ee859fe8 --- /dev/null +++ b/.changeset/metal-hornets-drum.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend-module-pinniped-provider': patch +--- + +Introduced PinnipedStrategyCache to act as a metadata cache for the PinnipedAuthenticator. diff --git a/plugins/auth-backend-module-pinniped-provider/api-report.md b/plugins/auth-backend-module-pinniped-provider/api-report.md index b9b993bd1e..430099713e 100644 --- a/plugins/auth-backend-module-pinniped-provider/api-report.md +++ b/plugins/auth-backend-module-pinniped-provider/api-report.md @@ -5,6 +5,7 @@ ```ts import { BackendFeature } from '@backstage/backend-plugin-api'; import { BaseClient } from 'openid-client'; +import { Config } from '@backstage/config'; import { OAuthAuthenticator } from '@backstage/plugin-auth-node'; import { Strategy } from 'openid-client'; import { TokenSet } from 'openid-client'; @@ -14,7 +15,15 @@ export const authModulePinnipedProvider: () => BackendFeature; // @public (undocumented) export const pinnipedAuthenticator: OAuthAuthenticator< - Promise<{ + PinnipedStrategyCache, + unknown +>; + +// @public (undocumented) +export class PinnipedStrategyCache { + constructor(callbackUrl: string, config: Config); + // (undocumented) + getStrategy(): Promise<{ providerStrategy: Strategy< { tokenset: TokenSet; @@ -22,7 +31,6 @@ export const pinnipedAuthenticator: OAuthAuthenticator< BaseClient >; client: BaseClient; - }>, - unknown ->; + }>; +} ``` 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 b9703df619..2ecd54e80a 100644 --- a/plugins/auth-backend-module-pinniped-provider/src/authenticator.test.ts +++ b/plugins/auth-backend-module-pinniped-provider/src/authenticator.test.ts @@ -146,18 +146,8 @@ 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(); + it('always returns a PinnipedStrategyCache', async () => { + const { providerStrategy, client } = await authCtx.getStrategy(); expect(providerStrategy).toBeDefined(); expect(client.issuer.authorization_endpoint).toMatch( @@ -343,24 +333,8 @@ describe('pinnipedAuthenticator', () => { }); it('caches oidc metadata after a success', 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; + // we start with 1 because the supervisor was called once already when we initialize. + let supervisorCalls: number = 1; mswServer.use( rest.get( @@ -376,20 +350,15 @@ describe('pinnipedAuthenticator', () => { ), ); - await pinnipedAuthenticator.start( - startRequest, - authCtxCreatedWhileSupervisorUnavailable, - ); - await pinnipedAuthenticator.start( - startRequest, - authCtxCreatedWhileSupervisorUnavailable, - ); + await pinnipedAuthenticator.start(startRequest, authCtx); + await pinnipedAuthenticator.start(startRequest, authCtx); expect(supervisorCalls).toEqual(1); }); it('refreshes oidc metadata when current one in cache expires', async () => { - let supervisorCalls: number = 0; + // we start with 1 because the supervisor was called once already when we initialize. + let supervisorCalls: number = 1; const fixedTime = DateTime.local(); jest.spyOn(DateTime, 'local').mockImplementation(() => fixedTime); @@ -407,15 +376,6 @@ describe('pinnipedAuthenticator', () => { ), ); - authCtx = pinnipedAuthenticator.initialize({ - callbackUrl: 'https://backstage.test/callback', - config: new ConfigReader({ - federationDomain: 'https://federationDomain.test', - clientId: 'clientId', - clientSecret: 'clientSecret', - }), - }); - await pinnipedAuthenticator.start(startRequest, authCtx); jest @@ -644,31 +604,14 @@ describe('pinnipedAuthenticator', () => { expect(response.session.accessToken).toEqual('accessToken'); }); - it('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 metadataCalls: number = 0; + it('caches oidc metadata after a success', async () => { + let supervisorCalls: number = 1; mswServer.use( rest.get( 'https://federationDomain.test/.well-known/openid-configuration', (_req, res, ctx) => { - metadataCalls += 1; + supervisorCalls += 1; return res( ctx.status(200), ctx.set('Content-Type', 'application/json'), @@ -678,10 +621,7 @@ describe('pinnipedAuthenticator', () => { ), ); - await pinnipedAuthenticator.authenticate( - handlerRequest, - authCtxCreatedWhileSupervisorUnavailable, - ); + await pinnipedAuthenticator.authenticate(handlerRequest, authCtx); await pinnipedAuthenticator.authenticate( { @@ -697,10 +637,10 @@ describe('pinnipedAuthenticator', () => { }, } as unknown as express.Request, }, - authCtxCreatedWhileSupervisorUnavailable, + authCtx, ); - expect(metadataCalls).toEqual(1); + expect(supervisorCalls).toEqual(1); }); it('refreshes oidc metadata when current one in cache expires', async () => { @@ -834,24 +774,7 @@ describe('pinnipedAuthenticator', () => { }); it('caches oidc metadata after a success', 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; + let supervisorCalls: number = 1; mswServer.use( rest.get( @@ -867,20 +790,14 @@ describe('pinnipedAuthenticator', () => { ), ); - await pinnipedAuthenticator.refresh( - refreshRequest, - authCtxCreatedWhileSupervisorUnavailable, - ); - await pinnipedAuthenticator.refresh( - refreshRequest, - authCtxCreatedWhileSupervisorUnavailable, - ); + await pinnipedAuthenticator.refresh(refreshRequest, authCtx); + await pinnipedAuthenticator.refresh(refreshRequest, authCtx); expect(supervisorCalls).toEqual(1); }); it('refreshes oidc metadata when current one in cache expires', async () => { - let supervisorCalls: number = 0; + let supervisorCalls: number = 1; const fixedTime = DateTime.local(); jest.spyOn(DateTime, 'local').mockImplementation(() => fixedTime); @@ -898,15 +815,6 @@ describe('pinnipedAuthenticator', () => { ), ); - authCtx = pinnipedAuthenticator.initialize({ - callbackUrl: 'https://backstage.test/callback', - config: new ConfigReader({ - federationDomain: 'https://federationDomain.test', - clientId: 'clientId', - clientSecret: 'clientSecret', - }), - }); - await pinnipedAuthenticator.refresh(refreshRequest, authCtx); jest diff --git a/plugins/auth-backend-module-pinniped-provider/src/authenticator.ts b/plugins/auth-backend-module-pinniped-provider/src/authenticator.ts index 6aae150188..e07eaba759 100644 --- a/plugins/auth-backend-module-pinniped-provider/src/authenticator.ts +++ b/plugins/auth-backend-module-pinniped-provider/src/authenticator.ts @@ -56,7 +56,10 @@ const rfc8693TokenExchange = async ({ }); }; -class PinnipedStrategyFactory { +const OIDC_METADATA_TTL_SECONDS = 3600; + +/** @public */ +export class PinnipedStrategyCache { private readonly callbackUrl: string; private readonly config: Config; private strategyPromise: Promise<{ @@ -97,7 +100,7 @@ class PinnipedStrategyFactory { await this.strategyPromise; this.cachedPromise = this.strategyPromise; this.cachedPromiseExpiry = DateTime.utc() - .plus({ seconds: 3600 }) + .plus({ seconds: OIDC_METADATA_TTL_SECONDS }) .toJSDate(); } catch (error) { // if we fail to generate a strategy, retry and overwrite strategy @@ -152,7 +155,7 @@ class PinnipedStrategyFactory { export const pinnipedAuthenticator = createOAuthAuthenticator({ defaultProfileTransform: async (_r, _c) => ({ profile: {} }), initialize({ callbackUrl, config }) { - return new PinnipedStrategyFactory(callbackUrl, config); + return new PinnipedStrategyCache(callbackUrl, config); }, async start(input, ctx): Promise<{ url: string; status?: number }> { const { providerStrategy } = await ctx.getStrategy(); diff --git a/plugins/auth-backend-module-pinniped-provider/src/index.ts b/plugins/auth-backend-module-pinniped-provider/src/index.ts index 9a2ca6727e..df4cd58603 100644 --- a/plugins/auth-backend-module-pinniped-provider/src/index.ts +++ b/plugins/auth-backend-module-pinniped-provider/src/index.ts @@ -20,5 +20,5 @@ * @packageDocumentation */ -export { pinnipedAuthenticator } from './authenticator'; +export { pinnipedAuthenticator, PinnipedStrategyCache } from './authenticator'; export { authModulePinnipedProvider } from './module';