From 1964cb7d8805f4b78a75a920c411b68b4cbf9a39 Mon Sep 17 00:00:00 2001 From: Ruben Vallejo Date: Thu, 28 Sep 2023 10:59:18 -0400 Subject: [PATCH] Working oidc authenticator with passing unit and module tests Signed-off-by: Ruben Vallejo --- .../config.d.ts | 1 + .../dev/index.ts | 6 +- .../src/authenticator.test.ts | 53 +++- .../src/authenticator.ts | 32 ++- .../src/index.ts | 2 +- .../src/module.test.ts | 8 +- .../auth-backend/src/providers/oidc/index.ts | 1 - .../src/providers/oidc/provider.ts | 242 ------------------ 8 files changed, 84 insertions(+), 261 deletions(-) diff --git a/plugins/auth-backend-module-oidc-provider/config.d.ts b/plugins/auth-backend-module-oidc-provider/config.d.ts index 803b9add73..16131c0761 100644 --- a/plugins/auth-backend-module-oidc-provider/config.d.ts +++ b/plugins/auth-backend-module-oidc-provider/config.d.ts @@ -27,6 +27,7 @@ export interface Config { clientSecret: string; metadataUrl: string; callbackUrl?: string; + tokenEndpointAuthMethod?: string; tokenSignedResponseAlg?: string; scope?: string; prompt?: string; diff --git a/plugins/auth-backend-module-oidc-provider/dev/index.ts b/plugins/auth-backend-module-oidc-provider/dev/index.ts index 4d027a19c2..d3c18c1d48 100644 --- a/plugins/auth-backend-module-oidc-provider/dev/index.ts +++ b/plugins/auth-backend-module-oidc-provider/dev/index.ts @@ -15,12 +15,10 @@ */ import { createBackend } from '@backstage/backend-defaults'; -import { authPlugin } from '@backstage/plugin-auth-backend'; -import { authModuleOidcProvider } from '../src'; const backend = createBackend(); -backend.add(authPlugin); -backend.add(authModuleOidcProvider); +backend.add(import('@backstage/plugin-auth-backend')); +backend.add(import('../src')); backend.start(); diff --git a/plugins/auth-backend-module-oidc-provider/src/authenticator.test.ts b/plugins/auth-backend-module-oidc-provider/src/authenticator.test.ts index 6f22517361..4d21db8442 100644 --- a/plugins/auth-backend-module-oidc-provider/src/authenticator.test.ts +++ b/plugins/auth-backend-module-oidc-provider/src/authenticator.test.ts @@ -103,7 +103,7 @@ describe('oidcAuthenticator', () => { id_token: idToken, refresh_token: 'refreshToken', scope: 'testScope', - token_type: '', + expires_in: 3600, }) : ctx.status(401), ); @@ -119,6 +119,7 @@ describe('oidcAuthenticator', () => { given_name: 'Alice', family_name: 'Adams', email: 'alice@test.com', + picture: 'http://testPictureUrl/photo.jpg', }), ), ), @@ -127,7 +128,7 @@ describe('oidcAuthenticator', () => { implementation = oidcAuthenticator.initialize({ callbackUrl: 'https://backstage.test/callback', config: new ConfigReader({ - metadataUrl: 'https://oidc.test', + metadataUrl: 'https://oidc.test/.well-known/openid-configuration', clientId: 'clientId', clientSecret: 'clientSecret', }), @@ -219,7 +220,6 @@ describe('oidcAuthenticator', () => { expect(fakeSession['oidc:oidc.test'].code_verifier).toBeDefined(); }); - // without the offline_access scope refresh should not work should we add in the test that is a required scope to test for? curently dont see that by defualt in the provider implementation. it('requests default scopes if none are provided in config', async () => { const startResponse = await oidcAuthenticator.start( startRequest, @@ -310,6 +310,53 @@ describe('oidcAuthenticator', () => { expect(responseScope).toEqual('testScope'); }); + it('returns a default session.tokentype field', async () => { + const handlerResponse = await oidcAuthenticator.authenticate( + handlerRequest, + implementation, + ); + const tokenType = handlerResponse.session.tokenType; + + expect(tokenType).toEqual('bearer'); + }); + + it('returns defined fullProfile with picture and email', async () => { + const handlerResponse = await oidcAuthenticator.authenticate( + handlerRequest, + implementation, + ); + const displayName = handlerResponse.fullProfile.displayName; + const email = handlerResponse.fullProfile.emails![0].value; + const picture = handlerResponse.fullProfile.photos![0].value; + + expect(displayName).toEqual('Alice Adams'); + expect(email).toEqual('alice@test.com'); + expect(picture).toEqual('http://testPictureUrl/photo.jpg'); + }); + + it('returns defined response with an idToken', async () => { + const handlerResponse = await oidcAuthenticator.authenticate( + handlerRequest, + implementation, + ); + + expect(handlerResponse).toMatchObject({ + fullProfile: { + displayName: 'Alice Adams', + id: 'test', + provider: 'oidc', + }, + session: { + accessToken: 'accessToken', + expiresInSeconds: 3600, + idToken, + refreshToken: 'refreshToken', + scope: 'testScope', + tokenType: 'bearer', + }, + }); + }); + it('fails without authorization code', async () => { handlerRequest.req.url = 'https://test.com'; return expect( diff --git a/plugins/auth-backend-module-oidc-provider/src/authenticator.ts b/plugins/auth-backend-module-oidc-provider/src/authenticator.ts index 7a7ebafe5c..a95010e353 100644 --- a/plugins/auth-backend-module-oidc-provider/src/authenticator.ts +++ b/plugins/auth-backend-module-oidc-provider/src/authenticator.ts @@ -26,7 +26,6 @@ import { PassportOAuthAuthenticatorHelper, PassportOAuthDoneCallback, } from '@backstage/plugin-auth-node'; -// import { BACKSTAGE_SESSION_EXPIRATION } from '@backstage/plugin-auth-backend/src/lib/session'; /** @public */ export const oidcAuthenticator = createOAuthAuthenticator({ @@ -46,9 +45,8 @@ export const oidcAuthenticator = createOAuthAuthenticator({ ); const initializedScope = config.getOptionalString('scope'); const initializedPrompt = config.getOptionalString('prompt'); - const issuer = await Issuer.discover( - `${metadataUrl}/.well-known/openid-configuration`, - ); + + const issuer = await Issuer.discover(metadataUrl); const client = new issuer.Client({ access_type: 'offline', // this option must be passed to provider to receive a refresh token client_id: clientId, @@ -77,9 +75,18 @@ export const oidcAuthenticator = createOAuthAuthenticator({ ); } - const expiresInSeconds = !tokenset.expires_in - ? 3600 - : Math.min(tokenset.expires_in!, 3600); + const emails = userinfo.email ? [{ value: userinfo.email }] : undefined; + const photos = userinfo.picture + ? [{ value: userinfo.picture }] + : undefined; + const name = + userinfo.family_name && userinfo.given_name + ? { + familyName: userinfo.family_name, + givenName: userinfo.given_name, + middleName: userinfo.middle_name, + } + : undefined; done( undefined, @@ -88,11 +95,17 @@ export const oidcAuthenticator = createOAuthAuthenticator({ provider: 'oidc', id: userinfo.sub, displayName: userinfo.name!, + username: userinfo.preferred_username, + name, + emails, + photos, }, accessToken: tokenset.access_token!, params: { + id_token: tokenset.id_token, scope: tokenset.scope!, - expires_in: expiresInSeconds, + expires_in: tokenset.expires_in!, + token_type: tokenset.token_type, }, }, { @@ -139,6 +152,9 @@ export const oidcAuthenticator = createOAuthAuthenticator({ if (!tokenset.access_token) { throw new Error('Refresh failed'); } + if (!tokenset.scope) { + tokenset.scope = input.scope; + } const userinfo = await client.userinfo(tokenset.access_token); return new Promise((resolve, reject) => { diff --git a/plugins/auth-backend-module-oidc-provider/src/index.ts b/plugins/auth-backend-module-oidc-provider/src/index.ts index 4b5ddc123a..b14d350d4d 100644 --- a/plugins/auth-backend-module-oidc-provider/src/index.ts +++ b/plugins/auth-backend-module-oidc-provider/src/index.ts @@ -21,5 +21,5 @@ */ export { oidcAuthenticator } from './authenticator'; -export { authModuleOidcProvider } from './module'; +export { authModuleOidcProvider as default } from './module'; export { oidcSignInResolvers } from './resolvers'; diff --git a/plugins/auth-backend-module-oidc-provider/src/module.test.ts b/plugins/auth-backend-module-oidc-provider/src/module.test.ts index 5e01ae343f..09ad38b84c 100644 --- a/plugins/auth-backend-module-oidc-provider/src/module.test.ts +++ b/plugins/auth-backend-module-oidc-provider/src/module.test.ts @@ -17,7 +17,6 @@ import request from 'supertest'; import { AuthProviderRouteHandlers, - AuthResolverContext, createOAuthRouteHandlers, decodeOAuthState, } from '@backstage/plugin-auth-node'; @@ -121,6 +120,7 @@ describe('authModuleOidcProvider', () => { refresh_token: 'refreshToken', scope: 'testScope', token_type: '', + expires_in: 3600, }) : ctx.status(401), ); @@ -136,6 +136,7 @@ describe('authModuleOidcProvider', () => { given_name: 'Alice', family_name: 'Adams', email: 'alice@test.com', + picture: 'http://testPictureUrl/photo.jpg', }), ), ), @@ -172,7 +173,7 @@ describe('authModuleOidcProvider', () => { isOriginAllowed: _ => true, providerId: 'oidc', config: new ConfigReader({ - metadataUrl: 'https://oidc.test', + metadataUrl: 'https://oidc.test/.well-known/openid-configuration', clientId: 'clientId', clientSecret: 'clientSecret', }), @@ -264,9 +265,12 @@ describe('authModuleOidcProvider', () => { type: 'authorization_response', response: { profile: { + email: 'alice@test.com', + picture: 'http://testPictureUrl/photo.jpg', displayName: 'Alice Adams', }, providerInfo: { + idToken, accessToken: 'accessToken', scope: 'testScope', expiresInSeconds: 3600, diff --git a/plugins/auth-backend/src/providers/oidc/index.ts b/plugins/auth-backend/src/providers/oidc/index.ts index 6b2282411c..c4343b1547 100644 --- a/plugins/auth-backend/src/providers/oidc/index.ts +++ b/plugins/auth-backend/src/providers/oidc/index.ts @@ -15,4 +15,3 @@ */ export { oidc } from './provider'; -export type { OidcAuthResult } from './provider'; diff --git a/plugins/auth-backend/src/providers/oidc/provider.ts b/plugins/auth-backend/src/providers/oidc/provider.ts index ce8809c55a..18170e104e 100644 --- a/plugins/auth-backend/src/providers/oidc/provider.ts +++ b/plugins/auth-backend/src/providers/oidc/provider.ts @@ -22,255 +22,13 @@ import { adaptLegacyOAuthHandler, adaptLegacyOAuthSignInResolver, } from '../../lib/legacy'; - import { oidcAuthenticator } from '@backstage/plugin-auth-backend-module-oidc-provider'; -// type PrivateInfo = { -// refreshToken?: string; -// }; - -// type OidcImpl = { -// strategy: OidcStrategy; -// client: Client; -// }; - -// /** -// * authentication result for the OIDC which includes the token set and user information (a profile response sent by OIDC server) -// * @public -// */ -// export type OidcAuthResult = { -// tokenset: TokenSet; -// userinfo: UserinfoResponse; -// }; - -// export type Options = OAuthProviderOptions & { -// metadataUrl: string; -// scope?: string; -// prompt?: string; -// tokenEndpointAuthMethod?: ClientAuthMethod; -// tokenSignedResponseAlg?: string; -// signInResolver?: SignInResolver; -// authHandler: AuthHandler; -// resolverContext: AuthResolverContext; -// }; - -// export class OidcAuthProvider implements OAuthHandlers { -// private readonly implementation: Promise; -// private readonly scope?: string; -// private readonly prompt?: string; - -// private readonly signInResolver?: SignInResolver; -// private readonly authHandler: AuthHandler; -// private readonly resolverContext: AuthResolverContext; - -// constructor(options: Options) { -// this.implementation = this.setupStrategy(options); -// this.scope = options.scope; -// this.prompt = options.prompt; -// this.signInResolver = options.signInResolver; -// this.authHandler = options.authHandler; -// this.resolverContext = options.resolverContext; -// } - -// async start(req: OAuthStartRequest): Promise { -// const { strategy } = await this.implementation; -// const options: Record = { -// scope: req.scope || this.scope || 'openid profile email', -// state: encodeState(req.state), -// }; -// const prompt = this.prompt || 'none'; -// if (prompt !== 'auto') { -// options.prompt = prompt; -// } -// return await executeRedirectStrategy(req, strategy, options); -// } - -// async handler(req: express.Request) { -// const { strategy } = await this.implementation; -// const { result, privateInfo } = await executeFrameHandlerStrategy< -// OidcAuthResult, -// PrivateInfo -// >(req, strategy); - - // async refresh(req: OAuthRefreshRequest) { - // const { client } = await this.implementation; - // const tokenset = await client.refresh(req.refreshToken); - // if (!tokenset.access_token) { - // throw new Error('Refresh failed'); - // } - // if (!tokenset.scope) { - // tokenset.scope = req.scope; - // } - // const userinfo = await client.userinfo(tokenset.access_token); -// return { -// response: await this.handleResult(result), -// refreshToken: privateInfo.refreshToken, -// }; -// } - -// async refresh(req: OAuthRefreshRequest) { -// const { client } = await this.implementation; -// const tokenset = await client.refresh(req.refreshToken); -// if (!tokenset.access_token) { -// throw new Error('Refresh failed'); -// } -// const userinfo = await client.userinfo(tokenset.access_token); - -// return { -// response: await this.handleResult({ tokenset, userinfo }), -// refreshToken: tokenset.refresh_token, -// }; -// } - -// private async setupStrategy(options: Options): Promise { -// const issuer = await Issuer.discover(options.metadataUrl); -// const client = new issuer.Client({ -// access_type: 'offline', // this option must be passed to provider to receive a refresh token -// client_id: options.clientId, -// client_secret: options.clientSecret, -// redirect_uris: [options.callbackUrl], -// response_types: ['code'], -// token_endpoint_auth_method: -// options.tokenEndpointAuthMethod || 'client_secret_basic', -// id_token_signed_response_alg: options.tokenSignedResponseAlg || 'RS256', -// scope: options.scope || '', -// }); - -// const strategy = new OidcStrategy( -// { -// client, -// passReqToCallback: false, -// }, -// ( -// tokenset: TokenSet, -// userinfo: UserinfoResponse, -// done: PassportDoneCallback, -// ) => { -// if (typeof done !== 'function') { -// throw new Error( -// 'OIDC IdP must provide a userinfo_endpoint in the metadata response', -// ); -// } -// done( -// undefined, -// { tokenset, userinfo }, -// { -// refreshToken: tokenset.refresh_token, -// }, -// ); -// }, -// ); -// strategy.error = console.error; -// return { strategy, client }; -// } - -// Use this function to grab the user profile info from the token -// Then populate the profile with it -// private async handleResult(result: OidcAuthResult): Promise { -// const { profile } = await this.authHandler(result, this.resolverContext); - -// const expiresInSeconds = -// result.tokenset.expires_in === undefined -// ? BACKSTAGE_SESSION_EXPIRATION -// : Math.min(result.tokenset.expires_in, BACKSTAGE_SESSION_EXPIRATION); - -// let backstageIdentity = undefined; -// if (this.signInResolver) { -// backstageIdentity = await this.signInResolver( -// { -// result, -// profile, -// }, -// this.resolverContext, -// ); -// } - -// return { -// backstageIdentity, -// providerInfo: { -// idToken: result.tokenset.id_token, -// accessToken: result.tokenset.access_token!, -// scope: result.tokenset.scope!, -// expiresInSeconds, -// }, -// profile, -// }; -// } -// } - /** * Auth provider integration for generic OpenID Connect auth * * @public */ -// export const oidc = createAuthProviderIntegration({ -// create(options?: { -// authHandler?: AuthHandler; - -// signIn?: { -// resolver: SignInResolver; -// }; -// }) { -// return ({ providerId, globalConfig, config, resolverContext }) => -// OAuthEnvironmentHandler.mapConfig(config, envConfig => { -// const clientId = envConfig.getString('clientId'); -// const clientSecret = envConfig.getString('clientSecret'); -// const customCallbackUrl = envConfig.getOptionalString('callbackUrl'); -// const callbackUrl = -// customCallbackUrl || -// `${globalConfig.baseUrl}/${providerId}/handler/frame`; -// const metadataUrl = envConfig.getString('metadataUrl'); -// const tokenEndpointAuthMethod = envConfig.getOptionalString( -// 'tokenEndpointAuthMethod', -// ) as ClientAuthMethod; -// const tokenSignedResponseAlg = envConfig.getOptionalString( -// 'tokenSignedResponseAlg', -// ); -// const scope = envConfig.getOptionalString('scope'); -// const prompt = envConfig.getOptionalString('prompt'); - -// const authHandler: AuthHandler = options?.authHandler -// ? options.authHandler -// : async ({ userinfo }) => ({ -// profile: { -// displayName: userinfo.name, -// email: userinfo.email, -// picture: userinfo.picture, -// }, -// }); - -// const provider = new OidcAuthProvider({ -// clientId, -// clientSecret, -// callbackUrl, -// tokenEndpointAuthMethod, -// tokenSignedResponseAlg, -// metadataUrl, -// scope, -// prompt, -// signInResolver: options?.signIn?.resolver, -// authHandler, -// resolverContext, -// }); - -// return OAuthAdapter.fromConfig(globalConfig, provider, { -// providerId, -// callbackUrl, -// }); -// }); -// }, -// resolvers: { -// /** -// * Looks up the user by matching their email local part to the entity name. -// */ -// emailLocalPartMatchingUserEntityName: () => commonByEmailLocalPartResolver, -// /** -// * Looks up the user by matching their email to the entity email. -// */ -// emailMatchingUserEntityProfileEmail: () => commonByEmailResolver, -// }, -// }); - export const oidc = createAuthProviderIntegration({ create(options?: { /**