diff --git a/.changeset/brown-pumpkins-impress.md b/.changeset/brown-pumpkins-impress.md new file mode 100644 index 0000000000..5748a9860b --- /dev/null +++ b/.changeset/brown-pumpkins-impress.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': patch +--- + +Refactored auth provider factories to accept options along with other internal refactoring of the auth providers. diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index c50789d4c1..281ea063fd 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -34,6 +34,7 @@ "@backstage/catalog-model": "^0.7.1", "@backstage/config": "^0.1.2", "@types/express": "^4.17.6", + "@types/passport": "^1.0.3", "compression": "^1.7.4", "cookie-parser": "^1.4.5", "cors": "^2.8.5", @@ -70,7 +71,6 @@ "@types/cookie-parser": "^1.4.2", "@types/express-session": "^1.17.2", "@types/jwt-decode": "^3.1.0", - "@types/passport": "^1.0.3", "@types/passport-github2": "^1.2.4", "@types/passport-google-oauth20": "^2.0.3", "@types/passport-microsoft": "^0.0.0", diff --git a/plugins/auth-backend/src/lib/oauth/index.ts b/plugins/auth-backend/src/lib/oauth/index.ts index 564a0e1e7c..bf3e0658e4 100644 --- a/plugins/auth-backend/src/lib/oauth/index.ts +++ b/plugins/auth-backend/src/lib/oauth/index.ts @@ -25,4 +25,5 @@ export type { OAuthState, OAuthStartRequest, OAuthRefreshRequest, + OAuthResult, } from './types'; diff --git a/plugins/auth-backend/src/lib/oauth/types.ts b/plugins/auth-backend/src/lib/oauth/types.ts index b2b7915a4e..dd477388bf 100644 --- a/plugins/auth-backend/src/lib/oauth/types.ts +++ b/plugins/auth-backend/src/lib/oauth/types.ts @@ -15,6 +15,7 @@ */ import express from 'express'; +import { Profile as PassportProfile } from 'passport'; import { AuthResponse, RedirectInfo } from '../../providers/types'; /** @@ -35,6 +36,17 @@ export type OAuthProviderOptions = { callbackUrl: string; }; +export type OAuthResult = { + fullProfile: PassportProfile; + params: { + id_token?: string; + scope: string; + expires_in: number; + }; + accessToken: string; + refreshToken?: string; +}; + export type OAuthResponse = AuthResponse; export type OAuthProviderInfo = { diff --git a/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.test.ts b/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.test.ts index f7a22e9ad9..c27eebbd9a 100644 --- a/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.test.ts +++ b/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.test.ts @@ -87,7 +87,7 @@ describe('PassportStrategyHelper', () => { expect(spyAuthenticate).toBeCalledTimes(1); await expect(frameHandlerStrategyPromise).resolves.toStrictEqual( expect.objectContaining({ - response: { accessToken: 'ACCESS_TOKEN' }, + result: { accessToken: 'ACCESS_TOKEN' }, privateInfo: { refreshToken: 'REFRESH_TOKEN' }, }), ); diff --git a/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.ts b/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.ts index 28f15ee9e8..8819da0a2f 100644 --- a/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.ts +++ b/plugins/auth-backend/src/lib/passport/PassportStrategyHelper.ts @@ -39,7 +39,7 @@ export const makeProfileInfo = ( } let picture: string | undefined = undefined; - if (profile.photos) { + if (profile.photos && profile.photos.length > 0) { const [firstPhoto] = profile.photos; picture = firstPhoto.value; } @@ -80,15 +80,15 @@ export const executeRedirectStrategy = async ( }); }; -export const executeFrameHandlerStrategy = async ( +export const executeFrameHandlerStrategy = async ( req: express.Request, providerStrategy: passport.Strategy, ) => { - return new Promise<{ response: T; privateInfo: PrivateInfo }>( + return new Promise<{ result: Result; privateInfo: PrivateInfo }>( (resolve, reject) => { const strategy = Object.create(providerStrategy); - strategy.success = (response: any, privateInfo: any) => { - resolve({ response, privateInfo }); + strategy.success = (result: any, privateInfo: any) => { + resolve({ result, privateInfo }); }; strategy.fail = ( info: { type: 'success' | 'error'; message?: string }, @@ -190,19 +190,17 @@ type ProviderStrategy = { export const executeFetchUserProfileStrategy = async ( providerStrategy: passport.Strategy, accessToken: string, - idToken?: string, -): Promise => { +): Promise => { return new Promise((resolve, reject) => { const anyStrategy = (providerStrategy as unknown) as ProviderStrategy; anyStrategy.userProfile( accessToken, - (error: Error, passportProfile: passport.Profile) => { + (error: Error, rawProfile: passport.Profile) => { if (error) { reject(error); + } else { + resolve(rawProfile); } - - const profile = makeProfileInfo(passportProfile, idToken); - resolve(profile); }, ); }); diff --git a/plugins/auth-backend/src/providers/auth0/index.ts b/plugins/auth-backend/src/providers/auth0/index.ts index 87c9aceaa4..480552a91d 100644 --- a/plugins/auth-backend/src/providers/auth0/index.ts +++ b/plugins/auth-backend/src/providers/auth0/index.ts @@ -15,3 +15,4 @@ */ export { createAuth0Provider } from './provider'; +export type { Auth0ProviderOptions } from './provider'; diff --git a/plugins/auth-backend/src/providers/auth0/provider.ts b/plugins/auth-backend/src/providers/auth0/provider.ts index 8772842e2a..2a2f743d12 100644 --- a/plugins/auth-backend/src/providers/auth0/provider.ts +++ b/plugins/auth-backend/src/providers/auth0/provider.ts @@ -26,6 +26,7 @@ import { OAuthStartRequest, encodeState, OAuthRefreshRequest, + OAuthResult, } from '../../lib/oauth'; import { executeFetchUserProfileStrategy, @@ -61,20 +62,16 @@ export class Auth0AuthProvider implements OAuthHandlers { accessToken: any, refreshToken: any, params: any, - rawProfile: passport.Profile, - done: PassportDoneCallback, + fullProfile: passport.Profile, + done: PassportDoneCallback, ) => { - const profile = makeProfileInfo(rawProfile, params.id_token); done( undefined, { - providerInfo: { - idToken: params.id_token, - accessToken, - scope: params.scope, - expiresInSeconds: params.expires_in, - }, - profile, + fullProfile, + accessToken, + refreshToken, + params, }, { refreshToken, @@ -96,13 +93,23 @@ export class Auth0AuthProvider implements OAuthHandlers { async handler( req: express.Request, ): Promise<{ response: OAuthResponse; refreshToken: string }> { - const { response, privateInfo } = await executeFrameHandlerStrategy< - OAuthResponse, + const { result, privateInfo } = await executeFrameHandlerStrategy< + OAuthResult, PrivateInfo >(req, this._strategy); + const profile = makeProfileInfo(result.fullProfile, result.params.id_token); + return { - response: await this.populateIdentity(response), + response: await this.populateIdentity({ + profile, + providerInfo: { + idToken: result.params.id_token, + accessToken: result.accessToken, + scope: result.params.scope, + expiresInSeconds: result.params.expires_in, + }, + }), refreshToken: privateInfo.refreshToken, }; } @@ -114,11 +121,11 @@ export class Auth0AuthProvider implements OAuthHandlers { req.scope, ); - const profile = await executeFetchUserProfileStrategy( + const fullProfile = await executeFetchUserProfileStrategy( this._strategy, accessToken, - params.id_token, ); + const profile = makeProfileInfo(fullProfile, params.id_token); return this.populateIdentity({ providerInfo: { @@ -148,28 +155,29 @@ export class Auth0AuthProvider implements OAuthHandlers { } } -export const createAuth0Provider: AuthProviderFactory = ({ - providerId, - globalConfig, - config, - tokenIssuer, -}) => - OAuthEnvironmentHandler.mapConfig(config, envConfig => { - const clientId = envConfig.getString('clientId'); - const clientSecret = envConfig.getString('clientSecret'); - const domain = envConfig.getString('domain'); - const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; +export type Auth0ProviderOptions = {}; - const provider = new Auth0AuthProvider({ - clientId, - clientSecret, - callbackUrl, - domain, - }); +export const createAuth0Provider = ( + _options?: Auth0ProviderOptions, +): AuthProviderFactory => { + return ({ providerId, globalConfig, config, tokenIssuer }) => + OAuthEnvironmentHandler.mapConfig(config, envConfig => { + const clientId = envConfig.getString('clientId'); + const clientSecret = envConfig.getString('clientSecret'); + const domain = envConfig.getString('domain'); + const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; - return OAuthAdapter.fromConfig(globalConfig, provider, { - disableRefresh: true, - providerId, - tokenIssuer, + const provider = new Auth0AuthProvider({ + clientId, + clientSecret, + callbackUrl, + domain, + }); + + return OAuthAdapter.fromConfig(globalConfig, provider, { + disableRefresh: true, + providerId, + tokenIssuer, + }); }); - }); +}; diff --git a/plugins/auth-backend/src/providers/aws-alb/index.ts b/plugins/auth-backend/src/providers/aws-alb/index.ts index f8b5c9e5d7..cd6d41f580 100644 --- a/plugins/auth-backend/src/providers/aws-alb/index.ts +++ b/plugins/auth-backend/src/providers/aws-alb/index.ts @@ -14,3 +14,4 @@ * limitations under the License. */ export { createAwsAlbProvider } from './provider'; +export type { AwsAlbProviderOptions } from './provider'; diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.ts b/plugins/auth-backend/src/providers/aws-alb/provider.ts index 61ea10947e..27baaded0f 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.ts @@ -106,22 +106,26 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { } } -export const createAwsAlbProvider = ({ - logger, - catalogApi, - config, - identityResolver, -}: AuthProviderFactoryOptions) => { - const region = config.getString('region'); - const issuer = config.getOptionalString('iss'); - if (identityResolver !== undefined) { - return new AwsAlbAuthProvider(logger, catalogApi, { - region, - issuer, - identityResolutionCallback: identityResolver, - }); - } - throw new Error( - 'Identity resolver is required to use this authentication provider', - ); +export type AwsAlbProviderOptions = {}; + +export const createAwsAlbProvider = (_options?: AwsAlbProviderOptions) => { + return ({ + logger, + catalogApi, + config, + identityResolver, + }: AuthProviderFactoryOptions) => { + const region = config.getString('region'); + const issuer = config.getOptionalString('iss'); + if (identityResolver !== undefined) { + return new AwsAlbAuthProvider(logger, catalogApi, { + region, + issuer, + identityResolutionCallback: identityResolver, + }); + } + throw new Error( + 'Identity resolver is required to use this authentication provider', + ); + }; }; diff --git a/plugins/auth-backend/src/providers/factories.ts b/plugins/auth-backend/src/providers/factories.ts index 619fb1c706..670dc84eec 100644 --- a/plugins/auth-backend/src/providers/factories.ts +++ b/plugins/auth-backend/src/providers/factories.ts @@ -28,15 +28,15 @@ import { AuthProviderFactory } from './types'; import { createAwsAlbProvider } from './aws-alb'; export const factories: { [providerId: string]: AuthProviderFactory } = { - google: createGoogleProvider, - github: createGithubProvider, - gitlab: createGitlabProvider, - saml: createSamlProvider, - okta: createOktaProvider, - auth0: createAuth0Provider, - microsoft: createMicrosoftProvider, - oauth2: createOAuth2Provider, - oidc: createOidcProvider, - onelogin: createOneLoginProvider, - awsalb: createAwsAlbProvider, + google: createGoogleProvider(), + github: createGithubProvider(), + gitlab: createGitlabProvider(), + saml: createSamlProvider(), + okta: createOktaProvider(), + auth0: createAuth0Provider(), + microsoft: createMicrosoftProvider(), + oauth2: createOAuth2Provider(), + oidc: createOidcProvider(), + onelogin: createOneLoginProvider(), + awsalb: createAwsAlbProvider(), }; diff --git a/plugins/auth-backend/src/providers/github/index.ts b/plugins/auth-backend/src/providers/github/index.ts index 60ad6998b7..f2ba73c2b9 100644 --- a/plugins/auth-backend/src/providers/github/index.ts +++ b/plugins/auth-backend/src/providers/github/index.ts @@ -15,3 +15,4 @@ */ export { createGithubProvider } from './provider'; +export type { GithubProviderOptions } from './provider'; diff --git a/plugins/auth-backend/src/providers/github/provider.test.ts b/plugins/auth-backend/src/providers/github/provider.test.ts index 61d4bb887f..6394ed9f2a 100644 --- a/plugins/auth-backend/src/providers/github/provider.test.ts +++ b/plugins/auth-backend/src/providers/github/provider.test.ts @@ -14,13 +14,31 @@ * limitations under the License. */ +import { Profile as PassportProfile } from 'passport'; import { GithubAuthProvider } from './provider'; +import * as helpers from '../../lib/passport'; +import { OAuthResult } from '../../lib/oauth'; + +const mockFrameHandler = (jest.spyOn( + helpers, + 'executeFrameHandlerStrategy', +) as unknown) as jest.MockedFunction< + () => Promise<{ + result: Omit & { params: { scope: string } }; + }> +>; describe('GithubAuthProvider', () => { + const provider = new GithubAuthProvider({ + callbackUrl: 'mock', + clientId: 'mock', + clientSecret: 'mock', + }); + describe('should transform to type OAuthResponse', () => { - it('when all fields are present, it should be able to map them', () => { + it('when all fields are present, it should be able to map them', async () => { const accessToken = '19xasczxcm9n7gacn9jdgm19me'; - const rawProfile = { + const fullProfile = { id: 'uid-123', username: 'jimmymarkum', provider: 'github', @@ -59,18 +77,17 @@ describe('GithubAuthProvider', () => { 'https://a1cf74336522e87f135f-2f21ace9a6cf0052456644b80fa06d4f.ssl.cf2.rackcdn.com/images/characters_opt/p-mystic-river-sean-penn.jpg', }, }; - expect( - GithubAuthProvider.transformOAuthResponse( - accessToken, - rawProfile, - params, - ), - ).toEqual(expected); + + mockFrameHandler.mockResolvedValueOnce({ + result: { fullProfile, accessToken, params }, + }); + const { response } = await provider.handler({} as any); + expect(response).toEqual(expected); }); - it('when "email" is missing, it should be able to create the profile without it', () => { + it('when "email" is missing, it should be able to create the profile without it', async () => { const accessToken = '19xasczxcm9n7gacn9jdgm19me'; - const rawProfile = { + const fullProfile = ({ id: 'uid-123', username: 'jimmymarkum', provider: 'github', @@ -82,7 +99,7 @@ describe('GithubAuthProvider', () => { 'https://a1cf74336522e87f135f-2f21ace9a6cf0052456644b80fa06d4f.ssl.cf2.rackcdn.com/images/characters_opt/p-mystic-river-sean-penn.jpg', }, ], - }; + } as unknown) as PassportProfile; const params = { scope: 'read:scope', @@ -105,18 +122,16 @@ describe('GithubAuthProvider', () => { }, }; - expect( - GithubAuthProvider.transformOAuthResponse( - accessToken, - rawProfile, - params, - ), - ).toEqual(expected); + mockFrameHandler.mockResolvedValueOnce({ + result: { fullProfile, accessToken, params }, + }); + const { response } = await provider.handler({} as any); + expect(response).toEqual(expected); }); - it('when "displayName" is missing, it should be able to create the profile and map "displayName" with "username"', () => { + it('when "displayName" is missing, it should be able to create the profile and map "displayName" with "username"', async () => { const accessToken = '19xasczxcm9n7gacn9jdgm19me'; - const rawProfile = { + const fullProfile = ({ id: 'uid-123', username: 'jimmymarkum', provider: 'github', @@ -128,7 +143,7 @@ describe('GithubAuthProvider', () => { 'https://a1cf74336522e87f135f-2f21ace9a6cf0052456644b80fa06d4f.ssl.cf2.rackcdn.com/images/characters_opt/p-mystic-river-sean-penn.jpg', }, ], - }; + } as unknown) as PassportProfile; const params = { scope: 'read:scope', @@ -150,19 +165,17 @@ describe('GithubAuthProvider', () => { }, }; - expect( - GithubAuthProvider.transformOAuthResponse( - accessToken, - rawProfile, - params, - ), - ).toEqual(expected); + mockFrameHandler.mockResolvedValueOnce({ + result: { fullProfile, accessToken, params }, + }); + const { response } = await provider.handler({} as any); + expect(response).toEqual(expected); }); - it('when "photos" is missing, it should be able to create the profile without it', () => { + it('when "photos" is missing, it should be able to create the profile without it', async () => { const accessToken = 'ajakljsdoiahoawxbrouawucmbawe.awkxjemaneasdxwe.sodijxqeqwexeqwxe'; - const rawProfile = { + const fullProfile = { id: 'ipd12039', username: 'daveboyle', provider: 'gitlab', @@ -195,13 +208,11 @@ describe('GithubAuthProvider', () => { }, }; - expect( - GithubAuthProvider.transformOAuthResponse( - accessToken, - rawProfile, - params, - ), - ).toEqual(expected); + mockFrameHandler.mockResolvedValueOnce({ + result: { fullProfile, accessToken, params }, + }); + const { response } = await provider.handler({} as any); + expect(response).toEqual(expected); }); }); }); diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index 6e3f9a6000..438cbca07b 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -27,12 +27,11 @@ import { OAuthAdapter, OAuthProviderOptions, OAuthHandlers, - OAuthResponse, OAuthEnvironmentHandler, OAuthStartRequest, encodeState, + OAuthResult, } from '../../lib/oauth'; -import passport from 'passport'; export type GithubAuthProviderOptions = OAuthProviderOptions & { tokenUrl?: string; @@ -43,55 +42,6 @@ export type GithubAuthProviderOptions = OAuthProviderOptions & { export class GithubAuthProvider implements OAuthHandlers { private readonly _strategy: GithubStrategy; - static transformPassportProfile(rawProfile: any): passport.Profile { - const profile: passport.Profile = { - id: rawProfile.username, - username: rawProfile.username, - provider: rawProfile.provider, - displayName: rawProfile.displayName || rawProfile.username, - photos: rawProfile.photos, - emails: rawProfile.emails, - }; - - return profile; - } - - static transformOAuthResponse( - accessToken: string, - rawProfile: any, - params: any = {}, - ): OAuthResponse { - const passportProfile = GithubAuthProvider.transformPassportProfile( - rawProfile, - ); - - const profile = makeProfileInfo(passportProfile, params.id_token); - const providerInfo = { - accessToken, - scope: params.scope, - expiresInSeconds: params.expires_in, - idToken: params.id_token, - }; - - // GitHub provides an id numeric value (123) - // as a fallback - const id = passportProfile!.id; - - if (params.expires_in) { - providerInfo.expiresInSeconds = params.expires_in; - } - if (params.id_token) { - providerInfo.idToken = params.id_token; - } - return { - providerInfo, - profile, - backstageIdentity: { - id, - }, - }; - } - constructor(options: GithubAuthProviderOptions) { this._strategy = new GithubStrategy( { @@ -104,17 +54,12 @@ export class GithubAuthProvider implements OAuthHandlers { }, ( accessToken: any, - _: any, + refreshToken: any, params: any, - rawProfile: any, - done: PassportDoneCallback, + fullProfile: any, + done: PassportDoneCallback, ) => { - const oauthResponse = GithubAuthProvider.transformOAuthResponse( - accessToken, - rawProfile, - params, - ); - done(undefined, oauthResponse); + done(undefined, { fullProfile, params, accessToken, refreshToken }); }, ); } @@ -127,51 +72,73 @@ export class GithubAuthProvider implements OAuthHandlers { } async handler(req: express.Request) { - const { response } = await executeFrameHandlerStrategy( - req, - this._strategy, + const { + result: { fullProfile, accessToken, params }, + } = await executeFrameHandlerStrategy(req, this._strategy); + + const profile = makeProfileInfo( + { + ...fullProfile, + id: fullProfile.username || fullProfile.id, + displayName: + fullProfile.displayName || fullProfile.username || fullProfile.id, + }, + params.id_token, ); - return { response }; + return { + response: { + profile, + providerInfo: { + accessToken, + scope: params.scope, + expiresInSeconds: params.expires_in, + }, + backstageIdentity: { + id: fullProfile.username || fullProfile.id, + }, + }, + }; } } -export const createGithubProvider: AuthProviderFactory = ({ - providerId, - globalConfig, - config, - tokenIssuer, -}) => - OAuthEnvironmentHandler.mapConfig(config, envConfig => { - const clientId = envConfig.getString('clientId'); - const clientSecret = envConfig.getString('clientSecret'); - const enterpriseInstanceUrl = envConfig.getOptionalString( - 'enterpriseInstanceUrl', - ); - const authorizationUrl = enterpriseInstanceUrl - ? `${enterpriseInstanceUrl}/login/oauth/authorize` - : undefined; - const tokenUrl = enterpriseInstanceUrl - ? `${enterpriseInstanceUrl}/login/oauth/access_token` - : undefined; - const userProfileUrl = enterpriseInstanceUrl - ? `${enterpriseInstanceUrl}/api/v3/user` - : undefined; - const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; +export type GithubProviderOptions = {}; - const provider = new GithubAuthProvider({ - clientId, - clientSecret, - callbackUrl, - tokenUrl, - userProfileUrl, - authorizationUrl, - }); +export const createGithubProvider = ( + _options?: GithubProviderOptions, +): AuthProviderFactory => { + return ({ providerId, globalConfig, config, tokenIssuer }) => + OAuthEnvironmentHandler.mapConfig(config, envConfig => { + const clientId = envConfig.getString('clientId'); + const clientSecret = envConfig.getString('clientSecret'); + const enterpriseInstanceUrl = envConfig.getOptionalString( + 'enterpriseInstanceUrl', + ); + const authorizationUrl = enterpriseInstanceUrl + ? `${enterpriseInstanceUrl}/login/oauth/authorize` + : undefined; + const tokenUrl = enterpriseInstanceUrl + ? `${enterpriseInstanceUrl}/login/oauth/access_token` + : undefined; + const userProfileUrl = enterpriseInstanceUrl + ? `${enterpriseInstanceUrl}/api/v3/user` + : undefined; + const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; - return OAuthAdapter.fromConfig(globalConfig, provider, { - disableRefresh: true, - persistScopes: true, - providerId, - tokenIssuer, + const provider = new GithubAuthProvider({ + clientId, + clientSecret, + callbackUrl, + tokenUrl, + userProfileUrl, + authorizationUrl, + }); + + return OAuthAdapter.fromConfig(globalConfig, provider, { + disableRefresh: true, + persistScopes: true, + providerId, + tokenIssuer, + }); }); - }); +}; diff --git a/plugins/auth-backend/src/providers/gitlab/index.ts b/plugins/auth-backend/src/providers/gitlab/index.ts index 7fba2dd95c..0a516021cd 100644 --- a/plugins/auth-backend/src/providers/gitlab/index.ts +++ b/plugins/auth-backend/src/providers/gitlab/index.ts @@ -15,3 +15,4 @@ */ export { createGitlabProvider } from './provider'; +export type { GitlabProviderOptions } from './provider'; diff --git a/plugins/auth-backend/src/providers/gitlab/provider.test.ts b/plugins/auth-backend/src/providers/gitlab/provider.test.ts index d4d1de17ec..b57d846f23 100644 --- a/plugins/auth-backend/src/providers/gitlab/provider.test.ts +++ b/plugins/auth-backend/src/providers/gitlab/provider.test.ts @@ -15,14 +15,21 @@ */ import { GitlabAuthProvider } from './provider'; +import * as helpers from '../../lib/passport'; +import { OAuthResult } from '../../lib/oauth'; + +const mockFrameHandler = (jest.spyOn( + helpers, + 'executeFrameHandlerStrategy', +) as unknown) as jest.MockedFunction<() => Promise<{ result: OAuthResult }>>; describe('GitlabAuthProvider', () => { - it('should transform to type OAuthResponse', () => { + it('should transform to type OAuthResponse', async () => { const tests = [ { - arguments: { + result: { accessToken: '19xasczxcm9n7gacn9jdgm19me', - rawProfile: { + fullProfile: { id: 'uid-123', username: 'jimmymarkum', provider: 'gitlab', @@ -58,10 +65,10 @@ describe('GitlabAuthProvider', () => { }, }, { - arguments: { + result: { accessToken: 'ajakljsdoiahoawxbrouawucmbawe.awkxjemaneasdxwe.sodijxqeqwexeqwxe', - rawProfile: { + fullProfile: { id: 'ipd12039', username: 'daveboyle', provider: 'gitlab', @@ -74,6 +81,7 @@ describe('GitlabAuthProvider', () => { }, params: { scope: 'read_repository', + expires_in: 200, }, }, expect: { @@ -83,6 +91,7 @@ describe('GitlabAuthProvider', () => { providerInfo: { accessToken: 'ajakljsdoiahoawxbrouawucmbawe.awkxjemaneasdxwe.sodijxqeqwexeqwxe', + expiresInSeconds: 200, scope: 'read_repository', }, profile: { @@ -93,14 +102,16 @@ describe('GitlabAuthProvider', () => { }, ]; + const provider = new GitlabAuthProvider({ + clientId: 'mock', + clientSecret: 'mock', + callbackUrl: 'mock', + baseUrl: 'mock', + }); for (const test of tests) { - expect( - GitlabAuthProvider.transformOAuthResponse( - test.arguments.accessToken, - test.arguments.rawProfile, - test.arguments.params, - ), - ).toEqual(test.expect); + mockFrameHandler.mockResolvedValueOnce({ result: test.result }); + const { response } = await provider.handler({} as any); + expect(response).toEqual(test.expect); } }); }); diff --git a/plugins/auth-backend/src/providers/gitlab/provider.ts b/plugins/auth-backend/src/providers/gitlab/provider.ts index df6d2daeb7..7c24757833 100644 --- a/plugins/auth-backend/src/providers/gitlab/provider.ts +++ b/plugins/auth-backend/src/providers/gitlab/provider.ts @@ -31,8 +31,8 @@ import { OAuthEnvironmentHandler, OAuthStartRequest, encodeState, + OAuthResult, } from '../../lib/oauth'; -import passport from 'passport'; export type GitlabAuthProviderOptions = OAuthProviderOptions & { baseUrl: string; @@ -41,64 +41,6 @@ export type GitlabAuthProviderOptions = OAuthProviderOptions & { export class GitlabAuthProvider implements OAuthHandlers { private readonly _strategy: GitlabStrategy; - static transformPassportProfile(rawProfile: any): passport.Profile { - const profile: passport.Profile = { - id: rawProfile.id, - username: rawProfile.username, - provider: rawProfile.provider, - displayName: rawProfile.displayName, - }; - - if (rawProfile.emails && rawProfile.emails.length > 0) { - profile.emails = rawProfile.emails; - } - if (rawProfile.avatarUrl) { - profile.photos = [{ value: rawProfile.avatarUrl }]; - } - - return profile; - } - - static transformOAuthResponse( - accessToken: string, - rawProfile: any, - params: any = {}, - ): OAuthResponse { - const passportProfile = GitlabAuthProvider.transformPassportProfile( - rawProfile, - ); - - const profile = makeProfileInfo(passportProfile, params.id_token); - const providerInfo = { - accessToken, - scope: params.scope, - expiresInSeconds: params.expires_in, - idToken: params.id_token, - }; - - // gitlab provides an id numeric value (123) - // as a fallback - let id = passportProfile!.id; - - if (profile.email) { - id = profile.email.split('@')[0]; - } - - if (params.expires_in) { - providerInfo.expiresInSeconds = params.expires_in; - } - if (params.id_token) { - providerInfo.idToken = params.id_token; - } - return { - providerInfo, - profile, - backstageIdentity: { - id, - }, - }; - } - constructor(options: GitlabAuthProviderOptions) { this._strategy = new GitlabStrategy( { @@ -109,17 +51,12 @@ export class GitlabAuthProvider implements OAuthHandlers { }, ( accessToken: any, - _: any, + refreshToken: any, params: any, - rawProfile: any, - done: PassportDoneCallback, + fullProfile: any, + done: PassportDoneCallback, ) => { - const oauthResponse = GitlabAuthProvider.transformOAuthResponse( - accessToken, - rawProfile, - params, - ); - done(undefined, oauthResponse); + done(undefined, { fullProfile, params, accessToken, refreshToken }); }, ); } @@ -132,36 +69,74 @@ export class GitlabAuthProvider implements OAuthHandlers { } async handler(req: express.Request): Promise<{ response: OAuthResponse }> { - return await executeFrameHandlerStrategy( + const { result } = await executeFrameHandlerStrategy( req, this._strategy, ); + const { accessToken, params } = result; + const fullProfile = result.fullProfile as OAuthResult['fullProfile'] & { + avatarUrl?: string; + }; + + const profile = makeProfileInfo( + { + ...fullProfile, + photos: [ + ...(fullProfile.photos ?? []), + ...(fullProfile.avatarUrl ? [{ value: fullProfile.avatarUrl }] : []), + ], + }, + params.id_token, + ); + + // gitlab provides an id numeric value (123) + // as a fallback + let id = fullProfile.id; + if (profile.email) { + id = profile.email.split('@')[0]; + } + + return { + response: { + profile, + providerInfo: { + accessToken, + scope: params.scope, + expiresInSeconds: params.expires_in, + idToken: params.id_token, + }, + backstageIdentity: { + id, + }, + }, + }; } } -export const createGitlabProvider: AuthProviderFactory = ({ - providerId, - globalConfig, - config, - tokenIssuer, -}) => - OAuthEnvironmentHandler.mapConfig(config, envConfig => { - const clientId = envConfig.getString('clientId'); - const clientSecret = envConfig.getString('clientSecret'); - const audience = envConfig.getString('audience'); - const baseUrl = audience || 'https://gitlab.com'; - const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; +export type GitlabProviderOptions = {}; - const provider = new GitlabAuthProvider({ - clientId, - clientSecret, - callbackUrl, - baseUrl, - }); +export const createGitlabProvider = ( + _options?: GitlabProviderOptions, +): AuthProviderFactory => { + return ({ providerId, globalConfig, config, tokenIssuer }) => + OAuthEnvironmentHandler.mapConfig(config, envConfig => { + const clientId = envConfig.getString('clientId'); + const clientSecret = envConfig.getString('clientSecret'); + const audience = envConfig.getString('audience'); + const baseUrl = audience || 'https://gitlab.com'; + const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; - return OAuthAdapter.fromConfig(globalConfig, provider, { - disableRefresh: true, - providerId, - tokenIssuer, + const provider = new GitlabAuthProvider({ + clientId, + clientSecret, + callbackUrl, + baseUrl, + }); + + return OAuthAdapter.fromConfig(globalConfig, provider, { + disableRefresh: true, + providerId, + tokenIssuer, + }); }); - }); +}; diff --git a/plugins/auth-backend/src/providers/google/index.ts b/plugins/auth-backend/src/providers/google/index.ts index b2cd85e6de..2615a2d8e5 100644 --- a/plugins/auth-backend/src/providers/google/index.ts +++ b/plugins/auth-backend/src/providers/google/index.ts @@ -15,3 +15,4 @@ */ export { createGoogleProvider } from './provider'; +export type { GoogleProviderOptions } from './provider'; diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 848a02bad6..a00644a90d 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -28,6 +28,7 @@ import { OAuthRefreshRequest, OAuthResponse, OAuthStartRequest, + OAuthResult, } from '../../lib/oauth'; import { executeFetchUserProfileStrategy, @@ -44,7 +45,7 @@ type PrivateInfo = { refreshToken: string; }; -export type GoogleAuthProviderOptions = OAuthProviderOptions & { +type Options = OAuthProviderOptions & { logger: Logger; identityClient: CatalogIdentityClient; tokenIssuer: TokenIssuer; @@ -56,7 +57,7 @@ export class GoogleAuthProvider implements OAuthHandlers { private readonly identityClient: CatalogIdentityClient; private readonly tokenIssuer: TokenIssuer; - constructor(options: GoogleAuthProviderOptions) { + constructor(options: Options) { this.logger = options.logger; this.identityClient = options.identityClient; this.tokenIssuer = options.tokenIssuer; @@ -74,20 +75,16 @@ export class GoogleAuthProvider implements OAuthHandlers { accessToken: any, refreshToken: any, params: any, - rawProfile: passport.Profile, - done: PassportDoneCallback, + fullProfile: passport.Profile, + done: PassportDoneCallback, ) => { - const profile = makeProfileInfo(rawProfile, params.id_token); done( undefined, { - providerInfo: { - idToken: params.id_token, - accessToken, - scope: params.scope, - expiresInSeconds: params.expires_in, - }, - profile, + fullProfile, + params, + accessToken, + refreshToken, }, { refreshToken, @@ -109,13 +106,23 @@ export class GoogleAuthProvider implements OAuthHandlers { async handler( req: express.Request, ): Promise<{ response: OAuthResponse; refreshToken: string }> { - const { response, privateInfo } = await executeFrameHandlerStrategy< - OAuthResponse, + const { result, privateInfo } = await executeFrameHandlerStrategy< + OAuthResult, PrivateInfo >(req, this._strategy); + const profile = makeProfileInfo(result.fullProfile, result.params.id_token); + return { - response: await this.populateIdentity(response), + response: await this.populateIdentity({ + providerInfo: { + idToken: result.params.id_token, + accessToken: result.accessToken, + scope: result.params.scope, + expiresInSeconds: result.params.expires_in, + }, + profile, + }), refreshToken: privateInfo.refreshToken, }; } @@ -127,11 +134,11 @@ export class GoogleAuthProvider implements OAuthHandlers { req.scope, ); - const profile = await executeFetchUserProfileStrategy( + const fullProfile = await executeFetchUserProfileStrategy( this._strategy, accessToken, - params.id_token, ); + const profile = makeProfileInfo(fullProfile, params.id_token); return this.populateIdentity({ providerInfo: { @@ -184,31 +191,37 @@ export class GoogleAuthProvider implements OAuthHandlers { } } -export const createGoogleProvider: AuthProviderFactory = ({ - providerId, - globalConfig, - config, - logger, - tokenIssuer, - catalogApi, -}) => - OAuthEnvironmentHandler.mapConfig(config, envConfig => { - const clientId = envConfig.getString('clientId'); - const clientSecret = envConfig.getString('clientSecret'); - const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; +export type GoogleProviderOptions = {}; - const provider = new GoogleAuthProvider({ - clientId, - clientSecret, - callbackUrl, - logger, - tokenIssuer, - identityClient: new CatalogIdentityClient({ catalogApi }), - }); +export const createGoogleProvider = ( + _options?: GoogleProviderOptions, +): AuthProviderFactory => { + return ({ + providerId, + globalConfig, + config, + logger, + tokenIssuer, + catalogApi, + }) => + OAuthEnvironmentHandler.mapConfig(config, envConfig => { + const clientId = envConfig.getString('clientId'); + const clientSecret = envConfig.getString('clientSecret'); + const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; - return OAuthAdapter.fromConfig(globalConfig, provider, { - disableRefresh: false, - providerId, - tokenIssuer, + const provider = new GoogleAuthProvider({ + clientId, + clientSecret, + callbackUrl, + logger, + tokenIssuer, + identityClient: new CatalogIdentityClient({ catalogApi }), + }); + + return OAuthAdapter.fromConfig(globalConfig, provider, { + disableRefresh: false, + providerId, + tokenIssuer, + }); }); - }); +}; diff --git a/plugins/auth-backend/src/providers/microsoft/index.ts b/plugins/auth-backend/src/providers/microsoft/index.ts index 2e4abd2d2c..f6c2ce40cd 100644 --- a/plugins/auth-backend/src/providers/microsoft/index.ts +++ b/plugins/auth-backend/src/providers/microsoft/index.ts @@ -15,3 +15,4 @@ */ export { createMicrosoftProvider } from './provider'; +export type { MicrosoftProviderOptions } from './provider'; diff --git a/plugins/auth-backend/src/providers/microsoft/provider.ts b/plugins/auth-backend/src/providers/microsoft/provider.ts index b0c3635be8..4dee90d4c6 100644 --- a/plugins/auth-backend/src/providers/microsoft/provider.ts +++ b/plugins/auth-backend/src/providers/microsoft/provider.ts @@ -38,6 +38,7 @@ import { OAuthStartRequest, encodeState, OAuthRefreshRequest, + OAuthResult, } from '../../lib/oauth'; import got from 'got'; @@ -54,32 +55,6 @@ export type MicrosoftAuthProviderOptions = OAuthProviderOptions & { export class MicrosoftAuthProvider implements OAuthHandlers { private readonly _strategy: MicrosoftStrategy; - static transformAuthResponse( - accessToken: string, - params: any, - rawProfile: any, - photoURL: any, - ): OAuthResponse { - let passportProfile: passport.Profile = rawProfile; - passportProfile = { - ...passportProfile, - photos: [{ value: photoURL }], - }; - - const profile = makeProfileInfo(passportProfile, params.id_token); - const providerInfo = { - idToken: params.id_token, - accessToken, - scope: params.scope, - expiresInSeconds: params.expires_in, - }; - - return { - providerInfo, - profile, - }; - } - constructor(options: MicrosoftAuthProviderOptions) { this._strategy = new MicrosoftStrategy( { @@ -94,22 +69,10 @@ export class MicrosoftAuthProvider implements OAuthHandlers { accessToken: any, refreshToken: any, params: any, - rawProfile: passport.Profile, - done: PassportDoneCallback, + fullProfile: passport.Profile, + done: PassportDoneCallback, ) => { - this.getUserPhoto(accessToken) - .then(photoURL => { - const authResponse = MicrosoftAuthProvider.transformAuthResponse( - accessToken, - params, - rawProfile, - photoURL, - ); - done(undefined, authResponse, { refreshToken }); - }) - .catch(error => { - throw new Error(`Error processing auth response: ${error}`); - }); + done(undefined, { fullProfile, accessToken, refreshToken, params }); }, ); } @@ -124,15 +87,37 @@ export class MicrosoftAuthProvider implements OAuthHandlers { async handler( req: express.Request, ): Promise<{ response: OAuthResponse; refreshToken: string }> { - const { response, privateInfo } = await executeFrameHandlerStrategy< - OAuthResponse, + const { result, privateInfo } = await executeFrameHandlerStrategy< + OAuthResult, PrivateInfo >(req, this._strategy); - return { - response: await this.populateIdentity(response), - refreshToken: privateInfo.refreshToken, - }; + try { + const photoUrl = await this.getUserPhoto(result.accessToken); + + const profile = makeProfileInfo( + { + ...result.fullProfile, + photos: photoUrl ? [{ value: photoUrl }] : undefined, + }, + result.params.id_token, + ); + + return { + response: await this.populateIdentity({ + profile, + providerInfo: { + idToken: result.params.id_token, + accessToken: result.accessToken, + scope: result.params.scope, + expiresInSeconds: result.params.expires_in, + }, + }), + refreshToken: privateInfo.refreshToken, + }; + } catch (error) { + throw new Error(`Error processing auth response: ${error}`); + } } async refresh(req: OAuthRefreshRequest): Promise { @@ -142,11 +127,11 @@ export class MicrosoftAuthProvider implements OAuthHandlers { req.scope, ); - const profile = await executeFetchUserProfileStrategy( + const fullProfile = await executeFetchUserProfileStrategy( this._strategy, accessToken, - params.id_token, ); + const profile = makeProfileInfo(fullProfile, params.id_token); const photo = await this.getUserPhoto(accessToken); if (photo) { profile.picture = photo; @@ -205,32 +190,33 @@ export class MicrosoftAuthProvider implements OAuthHandlers { } } -export const createMicrosoftProvider: AuthProviderFactory = ({ - providerId, - globalConfig, - config, - tokenIssuer, -}) => - OAuthEnvironmentHandler.mapConfig(config, envConfig => { - const clientId = envConfig.getString('clientId'); - const clientSecret = envConfig.getString('clientSecret'); - const tenantID = envConfig.getString('tenantId'); +export type MicrosoftProviderOptions = {}; - const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; - const authorizationUrl = `https://login.microsoftonline.com/${tenantID}/oauth2/v2.0/authorize`; - const tokenUrl = `https://login.microsoftonline.com/${tenantID}/oauth2/v2.0/token`; +export const createMicrosoftProvider = ( + _options?: MicrosoftProviderOptions, +): AuthProviderFactory => { + return ({ providerId, globalConfig, config, tokenIssuer }) => + OAuthEnvironmentHandler.mapConfig(config, envConfig => { + const clientId = envConfig.getString('clientId'); + const clientSecret = envConfig.getString('clientSecret'); + const tenantId = envConfig.getString('tenantId'); - const provider = new MicrosoftAuthProvider({ - clientId, - clientSecret, - callbackUrl, - authorizationUrl, - tokenUrl, + const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; + const authorizationUrl = `https://login.microsoftonline.com/${tenantId}/oauth2/v2.0/authorize`; + const tokenUrl = `https://login.microsoftonline.com/${tenantId}/oauth2/v2.0/token`; + + const provider = new MicrosoftAuthProvider({ + clientId, + clientSecret, + callbackUrl, + authorizationUrl, + tokenUrl, + }); + + return OAuthAdapter.fromConfig(globalConfig, provider, { + disableRefresh: false, + providerId, + tokenIssuer, + }); }); - - return OAuthAdapter.fromConfig(globalConfig, provider, { - disableRefresh: false, - providerId, - tokenIssuer, - }); - }); +}; diff --git a/plugins/auth-backend/src/providers/oauth2/index.ts b/plugins/auth-backend/src/providers/oauth2/index.ts index e607aa10e1..97848f4319 100644 --- a/plugins/auth-backend/src/providers/oauth2/index.ts +++ b/plugins/auth-backend/src/providers/oauth2/index.ts @@ -15,3 +15,4 @@ */ export { createOAuth2Provider } from './provider'; +export type { OAuth2ProviderOptions } from './provider'; diff --git a/plugins/auth-backend/src/providers/oauth2/provider.ts b/plugins/auth-backend/src/providers/oauth2/provider.ts index a44c4fb50b..fb347b836c 100644 --- a/plugins/auth-backend/src/providers/oauth2/provider.ts +++ b/plugins/auth-backend/src/providers/oauth2/provider.ts @@ -26,6 +26,7 @@ import { OAuthStartRequest, encodeState, OAuthRefreshRequest, + OAuthResult, } from '../../lib/oauth'; import { executeFetchUserProfileStrategy, @@ -65,21 +66,16 @@ export class OAuth2AuthProvider implements OAuthHandlers { accessToken: any, refreshToken: any, params: any, - rawProfile: passport.Profile, - done: PassportDoneCallback, + fullProfile: passport.Profile, + done: PassportDoneCallback, ) => { - const profile = makeProfileInfo(rawProfile, params.id_token); - done( undefined, { - providerInfo: { - idToken: params.id_token, - accessToken, - scope: params.scope, - expiresInSeconds: params.expires_in, - }, - profile, + fullProfile, + accessToken, + refreshToken, + params, }, { refreshToken, @@ -101,13 +97,23 @@ export class OAuth2AuthProvider implements OAuthHandlers { async handler( req: express.Request, ): Promise<{ response: OAuthResponse; refreshToken: string }> { - const { response, privateInfo } = await executeFrameHandlerStrategy< - OAuthResponse, + const { result, privateInfo } = await executeFrameHandlerStrategy< + OAuthResult, PrivateInfo >(req, this._strategy); + const profile = makeProfileInfo(result.fullProfile, result.params.id_token); + return { - response: await this.populateIdentity(response), + response: await this.populateIdentity({ + profile, + providerInfo: { + idToken: result.params.id_token, + accessToken: result.accessToken, + scope: result.params.scope, + expiresInSeconds: result.params.expires_in, + }, + }), refreshToken: privateInfo.refreshToken, }; } @@ -124,11 +130,11 @@ export class OAuth2AuthProvider implements OAuthHandlers { refreshToken: updatedRefreshToken, } = refreshTokenResponse; - const profile = await executeFetchUserProfileStrategy( + const rawProfile = await executeFetchUserProfileStrategy( this._strategy, accessToken, - params.id_token, ); + const profile = makeProfileInfo(rawProfile, params.id_token); return this.populateIdentity({ providerInfo: { @@ -158,32 +164,33 @@ export class OAuth2AuthProvider implements OAuthHandlers { } } -export const createOAuth2Provider: AuthProviderFactory = ({ - providerId, - globalConfig, - config, - tokenIssuer, -}) => - OAuthEnvironmentHandler.mapConfig(config, envConfig => { - const clientId = envConfig.getString('clientId'); - const clientSecret = envConfig.getString('clientSecret'); - const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; - const authorizationUrl = envConfig.getString('authorizationUrl'); - const tokenUrl = envConfig.getString('tokenUrl'); - const scope = envConfig.getOptionalString('scope'); +export type OAuth2ProviderOptions = {}; - const provider = new OAuth2AuthProvider({ - clientId, - clientSecret, - callbackUrl, - authorizationUrl, - tokenUrl, - scope, - }); +export const createOAuth2Provider = ( + _options?: OAuth2ProviderOptions, +): AuthProviderFactory => { + return ({ providerId, globalConfig, config, tokenIssuer }) => + OAuthEnvironmentHandler.mapConfig(config, envConfig => { + const clientId = envConfig.getString('clientId'); + const clientSecret = envConfig.getString('clientSecret'); + const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; + const authorizationUrl = envConfig.getString('authorizationUrl'); + const tokenUrl = envConfig.getString('tokenUrl'); + const scope = envConfig.getOptionalString('scope'); - return OAuthAdapter.fromConfig(globalConfig, provider, { - disableRefresh: false, - providerId, - tokenIssuer, + const provider = new OAuth2AuthProvider({ + clientId, + clientSecret, + callbackUrl, + authorizationUrl, + tokenUrl, + scope, + }); + + return OAuthAdapter.fromConfig(globalConfig, provider, { + disableRefresh: false, + providerId, + tokenIssuer, + }); }); - }); +}; diff --git a/plugins/auth-backend/src/providers/oidc/index.ts b/plugins/auth-backend/src/providers/oidc/index.ts index acacbde73d..63c530acfc 100644 --- a/plugins/auth-backend/src/providers/oidc/index.ts +++ b/plugins/auth-backend/src/providers/oidc/index.ts @@ -15,3 +15,4 @@ */ export { createOidcProvider } from './provider'; +export type { OidcProviderOptions } from './provider'; diff --git a/plugins/auth-backend/src/providers/oidc/provider.test.ts b/plugins/auth-backend/src/providers/oidc/provider.test.ts index fcbe9d7abc..a746c0573b 100644 --- a/plugins/auth-backend/src/providers/oidc/provider.test.ts +++ b/plugins/auth-backend/src/providers/oidc/provider.test.ts @@ -121,7 +121,7 @@ describe('OidcAuthProvider', () => { })), } as any) as Config, } as AuthProviderFactoryOptions; - const provider = createOidcProvider(options) as OAuthAdapter; + const provider = createOidcProvider()(options) as OAuthAdapter; expect(provider.start).toBeDefined(); await new Promise(resolve => process.nextTick(resolve)); // advance a tick to give nock a chance to intercept the request expect(scope.isDone()).toBeTruthy(); diff --git a/plugins/auth-backend/src/providers/oidc/provider.ts b/plugins/auth-backend/src/providers/oidc/provider.ts index 6ea7b8a8d3..0ec8d0bbf7 100644 --- a/plugins/auth-backend/src/providers/oidc/provider.ts +++ b/plugins/auth-backend/src/providers/oidc/provider.ts @@ -37,10 +37,10 @@ import { executeRedirectStrategy, PassportDoneCallback, } from '../../lib/passport'; -import { RedirectInfo, AuthProviderFactory, ProfileInfo } from '../types'; +import { RedirectInfo, AuthProviderFactory } from '../types'; type PrivateInfo = { - refreshToken: string; + refreshToken?: string; }; type OidcImpl = { @@ -48,7 +48,12 @@ type OidcImpl = { client: Client; }; -export type OidcAuthProviderOptions = OAuthProviderOptions & { +type AuthResult = { + tokenset: TokenSet; + userinfo: UserinfoResponse; +}; + +export type Options = OAuthProviderOptions & { metadataUrl: string; tokenSignedResponseAlg?: string; }; @@ -56,7 +61,7 @@ export type OidcAuthProviderOptions = OAuthProviderOptions & { export class OidcAuthProvider implements OAuthHandlers { private readonly implementation: Promise; - constructor(options: OidcAuthProviderOptions) { + constructor(options: Options) { this.implementation = this.setupStrategy(options); } @@ -72,15 +77,30 @@ export class OidcAuthProvider implements OAuthHandlers { async handler( req: express.Request, - ): Promise<{ response: OAuthResponse; refreshToken: string }> { + ): Promise<{ response: OAuthResponse; refreshToken?: string }> { const { strategy } = await this.implementation; - const { response, privateInfo } = await executeFrameHandlerStrategy< - OAuthResponse, - PrivateInfo - >(req, strategy); + const { + result: { userinfo, tokenset }, + privateInfo, + } = await executeFrameHandlerStrategy( + req, + strategy, + ); return { - response: await this.populateIdentity(response), + response: await this.populateIdentity({ + profile: { + displayName: userinfo.name, + email: userinfo.email, + picture: userinfo.picture, + }, + providerInfo: { + idToken: tokenset.id_token, + accessToken: tokenset.access_token || '', + scope: tokenset.scope || '', + expiresInSeconds: tokenset.expires_in, + }, + }), refreshToken: privateInfo.refreshToken, }; } @@ -105,9 +125,7 @@ export class OidcAuthProvider implements OAuthHandlers { }); } - private async setupStrategy( - options: OidcAuthProviderOptions, - ): Promise { + private async setupStrategy(options: Options): Promise { const issuer = await Issuer.discover(options.metadataUrl); const client = new issuer.Client({ client_id: options.clientId, @@ -125,27 +143,13 @@ export class OidcAuthProvider implements OAuthHandlers { ( tokenset: TokenSet, userinfo: UserinfoResponse, - done: PassportDoneCallback, + done: PassportDoneCallback, ) => { - const profile: ProfileInfo = { - displayName: userinfo.name, - email: userinfo.email, - picture: userinfo.picture, - }; - done( undefined, + { tokenset, userinfo }, { - providerInfo: { - idToken: tokenset.id_token || '', - accessToken: tokenset.access_token || '', - scope: tokenset.scope || '', - expiresInSeconds: tokenset.expires_in, - }, - profile, - }, - { - refreshToken: tokenset.refresh_token || '', + refreshToken: tokenset.refresh_token, }, ); }, @@ -170,32 +174,33 @@ export class OidcAuthProvider implements OAuthHandlers { } } -export const createOidcProvider: AuthProviderFactory = ({ - providerId, - globalConfig, - config, - tokenIssuer, -}) => - OAuthEnvironmentHandler.mapConfig(config, envConfig => { - const clientId = envConfig.getString('clientId'); - const clientSecret = envConfig.getString('clientSecret'); - const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; - const metadataUrl = envConfig.getString('metadataUrl'); - const tokenSignedResponseAlg = envConfig.getString( - 'tokenSignedResponseAlg', - ); +export type OidcProviderOptions = {}; - const provider = new OidcAuthProvider({ - clientId, - clientSecret, - callbackUrl, - tokenSignedResponseAlg, - metadataUrl, - }); +export const createOidcProvider = ( + _options?: OidcProviderOptions, +): AuthProviderFactory => { + return ({ providerId, globalConfig, config, tokenIssuer }) => + OAuthEnvironmentHandler.mapConfig(config, envConfig => { + const clientId = envConfig.getString('clientId'); + const clientSecret = envConfig.getString('clientSecret'); + const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; + const metadataUrl = envConfig.getString('metadataUrl'); + const tokenSignedResponseAlg = envConfig.getString( + 'tokenSignedResponseAlg', + ); - return OAuthAdapter.fromConfig(globalConfig, provider, { - disableRefresh: false, - providerId, - tokenIssuer, + const provider = new OidcAuthProvider({ + clientId, + clientSecret, + callbackUrl, + tokenSignedResponseAlg, + metadataUrl, + }); + + return OAuthAdapter.fromConfig(globalConfig, provider, { + disableRefresh: false, + providerId, + tokenIssuer, + }); }); - }); +}; diff --git a/plugins/auth-backend/src/providers/okta/index.ts b/plugins/auth-backend/src/providers/okta/index.ts index 05cc398f43..47a2549587 100644 --- a/plugins/auth-backend/src/providers/okta/index.ts +++ b/plugins/auth-backend/src/providers/okta/index.ts @@ -13,4 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + export { createOktaProvider } from './provider'; +export type { OktaProviderOptions } from './provider'; diff --git a/plugins/auth-backend/src/providers/okta/provider.ts b/plugins/auth-backend/src/providers/okta/provider.ts index 84b258e489..6320230e67 100644 --- a/plugins/auth-backend/src/providers/okta/provider.ts +++ b/plugins/auth-backend/src/providers/okta/provider.ts @@ -23,6 +23,7 @@ import { OAuthStartRequest, encodeState, OAuthRefreshRequest, + OAuthResult, } from '../../lib/oauth'; import { Strategy as OktaStrategy } from 'passport-okta-oauth'; import passport from 'passport'; @@ -80,21 +81,16 @@ export class OktaAuthProvider implements OAuthHandlers { accessToken: any, refreshToken: any, params: any, - rawProfile: passport.Profile, - done: PassportDoneCallback, + fullProfile: passport.Profile, + done: PassportDoneCallback, ) => { - const profile = makeProfileInfo(rawProfile, params.id_token); - done( undefined, { - providerInfo: { - idToken: params.id_token, - accessToken, - scope: params.scope, - expiresInSeconds: params.expires_in, - }, - profile, + accessToken, + refreshToken, + params, + fullProfile, }, { refreshToken, @@ -116,13 +112,23 @@ export class OktaAuthProvider implements OAuthHandlers { async handler( req: express.Request, ): Promise<{ response: OAuthResponse; refreshToken: string }> { - const { response, privateInfo } = await executeFrameHandlerStrategy< - OAuthResponse, + const { result, privateInfo } = await executeFrameHandlerStrategy< + OAuthResult, PrivateInfo >(req, this._strategy); + const profile = makeProfileInfo(result.fullProfile, result.params.id_token); + return { - response: await this.populateIdentity(response), + response: await this.populateIdentity({ + profile, + providerInfo: { + idToken: result.params.id_token, + accessToken: result.accessToken, + scope: result.params.scope, + expiresInSeconds: result.params.expires_in, + }, + }), refreshToken: privateInfo.refreshToken, }; } @@ -134,11 +140,11 @@ export class OktaAuthProvider implements OAuthHandlers { req.scope, ); - const profile = await executeFetchUserProfileStrategy( + const fullProfile = await executeFetchUserProfileStrategy( this._strategy, accessToken, - params.id_token, ); + const profile = makeProfileInfo(fullProfile, params.id_token); return this.populateIdentity({ providerInfo: { @@ -167,28 +173,29 @@ export class OktaAuthProvider implements OAuthHandlers { } } -export const createOktaProvider: AuthProviderFactory = ({ - providerId, - globalConfig, - config, - tokenIssuer, -}) => - OAuthEnvironmentHandler.mapConfig(config, envConfig => { - const clientId = envConfig.getString('clientId'); - const clientSecret = envConfig.getString('clientSecret'); - const audience = envConfig.getString('audience'); - const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; +export type OktaProviderOptions = {}; - const provider = new OktaAuthProvider({ - audience, - clientId, - clientSecret, - callbackUrl, - }); +export const createOktaProvider = ( + _options?: OktaProviderOptions, +): AuthProviderFactory => { + return ({ providerId, globalConfig, config, tokenIssuer }) => + OAuthEnvironmentHandler.mapConfig(config, envConfig => { + const clientId = envConfig.getString('clientId'); + const clientSecret = envConfig.getString('clientSecret'); + const audience = envConfig.getString('audience'); + const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; - return OAuthAdapter.fromConfig(globalConfig, provider, { - disableRefresh: false, - providerId, - tokenIssuer, + const provider = new OktaAuthProvider({ + audience, + clientId, + clientSecret, + callbackUrl, + }); + + return OAuthAdapter.fromConfig(globalConfig, provider, { + disableRefresh: false, + providerId, + tokenIssuer, + }); }); - }); +}; diff --git a/plugins/auth-backend/src/providers/onelogin/index.ts b/plugins/auth-backend/src/providers/onelogin/index.ts index 79af226cd1..60fd2ffca4 100644 --- a/plugins/auth-backend/src/providers/onelogin/index.ts +++ b/plugins/auth-backend/src/providers/onelogin/index.ts @@ -15,3 +15,4 @@ */ export { createOneLoginProvider } from './provider'; +export type { OneLoginProviderOptions } from './provider'; diff --git a/plugins/auth-backend/src/providers/onelogin/provider.ts b/plugins/auth-backend/src/providers/onelogin/provider.ts index 838b655d59..5cb0f84415 100644 --- a/plugins/auth-backend/src/providers/onelogin/provider.ts +++ b/plugins/auth-backend/src/providers/onelogin/provider.ts @@ -25,6 +25,7 @@ import { OAuthStartRequest, encodeState, OAuthRefreshRequest, + OAuthResult, } from '../../lib/oauth'; import passport from 'passport'; import { @@ -41,14 +42,14 @@ type PrivateInfo = { refreshToken: string; }; -export type OneLoginProviderOptions = OAuthProviderOptions & { +export type Options = OAuthProviderOptions & { issuer: string; }; export class OneLoginProvider implements OAuthHandlers { private readonly _strategy: any; - constructor(options: OneLoginProviderOptions) { + constructor(options: Options) { this._strategy = new OneLoginStrategy( { issuer: options.issuer, @@ -61,21 +62,16 @@ export class OneLoginProvider implements OAuthHandlers { accessToken: any, refreshToken: any, params: any, - rawProfile: passport.Profile, - done: PassportDoneCallback, + fullProfile: passport.Profile, + done: PassportDoneCallback, ) => { - const profile = makeProfileInfo(rawProfile, params.id_token); - done( undefined, { - providerInfo: { - idToken: params.id_token, - accessToken, - scope: params.scope, - expiresInSeconds: params.expires_in, - }, - profile, + accessToken, + refreshToken, + params, + fullProfile, }, { refreshToken, @@ -96,13 +92,23 @@ export class OneLoginProvider implements OAuthHandlers { async handler( req: express.Request, ): Promise<{ response: OAuthResponse; refreshToken: string }> { - const { response, privateInfo } = await executeFrameHandlerStrategy< - OAuthResponse, + const { result, privateInfo } = await executeFrameHandlerStrategy< + OAuthResult, PrivateInfo >(req, this._strategy); + const profile = makeProfileInfo(result.fullProfile, result.params.id_token); + return { - response: await this.populateIdentity(response), + response: await this.populateIdentity({ + profile, + providerInfo: { + idToken: result.params.id_token, + accessToken: result.accessToken, + scope: result.params.scope, + expiresInSeconds: result.params.expires_in, + }, + }), refreshToken: privateInfo.refreshToken, }; } @@ -114,11 +120,11 @@ export class OneLoginProvider implements OAuthHandlers { req.scope, ); - const profile = await executeFetchUserProfileStrategy( + const fullProfile = await executeFetchUserProfileStrategy( this._strategy, accessToken, - params.id_token, ); + const profile = makeProfileInfo(fullProfile, params.id_token); return this.populateIdentity({ providerInfo: { @@ -146,28 +152,29 @@ export class OneLoginProvider implements OAuthHandlers { } } -export const createOneLoginProvider: AuthProviderFactory = ({ - providerId, - globalConfig, - config, - tokenIssuer, -}) => - OAuthEnvironmentHandler.mapConfig(config, envConfig => { - const clientId = envConfig.getString('clientId'); - const clientSecret = envConfig.getString('clientSecret'); - const issuer = envConfig.getString('issuer'); - const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; +export type OneLoginProviderOptions = {}; - const provider = new OneLoginProvider({ - clientId, - clientSecret, - callbackUrl, - issuer, - }); +export const createOneLoginProvider = ( + _options?: OneLoginProviderOptions, +): AuthProviderFactory => { + return ({ providerId, globalConfig, config, tokenIssuer }) => + OAuthEnvironmentHandler.mapConfig(config, envConfig => { + const clientId = envConfig.getString('clientId'); + const clientSecret = envConfig.getString('clientSecret'); + const issuer = envConfig.getString('issuer'); + const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; - return OAuthAdapter.fromConfig(globalConfig, provider, { - disableRefresh: false, - providerId, - tokenIssuer, + const provider = new OneLoginProvider({ + clientId, + clientSecret, + callbackUrl, + issuer, + }); + + return OAuthAdapter.fromConfig(globalConfig, provider, { + disableRefresh: false, + providerId, + tokenIssuer, + }); }); - }); +}; diff --git a/plugins/auth-backend/src/providers/saml/index.ts b/plugins/auth-backend/src/providers/saml/index.ts index 582deb1608..597b20ee8d 100644 --- a/plugins/auth-backend/src/providers/saml/index.ts +++ b/plugins/auth-backend/src/providers/saml/index.ts @@ -15,3 +15,4 @@ */ export { createSamlProvider } from './provider'; +export type { SamlProviderOptions } from './provider'; diff --git a/plugins/auth-backend/src/providers/saml/provider.ts b/plugins/auth-backend/src/providers/saml/provider.ts index 74541d8294..c3a91b3b77 100644 --- a/plugins/auth-backend/src/providers/saml/provider.ts +++ b/plugins/auth-backend/src/providers/saml/provider.ts @@ -26,17 +26,17 @@ import { executeRedirectStrategy, PassportDoneCallback, } from '../../lib/passport'; -import { - AuthProviderRouteHandlers, - ProfileInfo, - AuthProviderFactory, -} from '../types'; +import { AuthProviderRouteHandlers, AuthProviderFactory } from '../types'; import { postMessageResponse } from '../../lib/flow'; import { TokenIssuer } from '../../identity'; type SamlInfo = { - userId: string; - profile: ProfileInfo; + fullProfile: any; +}; + +type Options = SamlConfig & { + tokenIssuer: TokenIssuer; + appUrl: string; }; export class SamlAuthProvider implements AuthProviderRouteHandlers { @@ -44,11 +44,11 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { private readonly tokenIssuer: TokenIssuer; private readonly appUrl: string; - constructor(options: SAMLProviderOptions) { + constructor(options: Options) { this.appUrl = options.appUrl; this.tokenIssuer = options.tokenIssuer; this.strategy = new SamlStrategy({ ...options }, (( - profile: SamlProfile, + fullProfile: SamlProfile, done: PassportDoneCallback, ) => { // TODO: There's plenty more validation and profile handling to do here, @@ -56,13 +56,7 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { // for non-oauth auth flows. // TODO: This flow doesn't issue an identity token that can be used to validate // the identity of the user in other backends, which we need in some form. - done(undefined, { - userId: profile.nameID!, - profile: { - email: profile.email!, - displayName: profile.displayName as string, - }, - }); + done(undefined, { fullProfile }); }) as VerifyWithoutRequest); } @@ -76,11 +70,13 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { res: express.Response, ): Promise { try { - const { - response: { userId, profile }, - } = await executeFrameHandlerStrategy(req, this.strategy); + const { result } = await executeFrameHandlerStrategy( + req, + this.strategy, + ); + + const id = result.fullProfile.nameID; - const id = userId; const idToken = await this.tokenIssuer.issueToken({ claims: { sub: id }, }); @@ -88,8 +84,11 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { return postMessageResponse(res, this.appUrl, { type: 'authorization_response', response: { + profile: { + email: result.fullProfile.email, + displayName: result.fullProfile.displayName, + }, providerInfo: {}, - profile, backstageIdentity: { id, idToken }, }, }); @@ -113,40 +112,36 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { } } -type SAMLProviderOptions = SamlConfig & { - tokenIssuer: TokenIssuer; - appUrl: string; -}; - type SignatureAlgorithm = 'sha1' | 'sha256' | 'sha512'; -export const createSamlProvider: AuthProviderFactory = ({ - providerId, - globalConfig, - config, - tokenIssuer, -}) => { - const opts = { - callbackUrl: `${globalConfig.baseUrl}/${providerId}/handler/frame`, - entryPoint: config.getString('entryPoint'), - logoutUrl: config.getOptionalString('logoutUrl'), - issuer: config.getString('issuer'), - cert: config.getOptionalString('cert'), - privateCert: config.getOptionalString('privateKey'), - decryptionPvk: config.getOptionalString('decryptionPvk'), - signatureAlgorithm: config.getOptionalString('signatureAlgorithm') as - | SignatureAlgorithm - | undefined, - digestAlgorithm: config.getOptionalString('digestAlgorithm'), +export type SamlProviderOptions = {}; - tokenIssuer, - appUrl: globalConfig.appUrl, +export const createSamlProvider = ( + _options?: SamlProviderOptions, +): AuthProviderFactory => { + return ({ providerId, globalConfig, config, tokenIssuer }) => { + const opts = { + callbackUrl: `${globalConfig.baseUrl}/${providerId}/handler/frame`, + entryPoint: config.getString('entryPoint'), + logoutUrl: config.getOptionalString('logoutUrl'), + issuer: config.getString('issuer'), + cert: config.getOptionalString('cert'), + privateCert: config.getOptionalString('privateKey'), + decryptionPvk: config.getOptionalString('decryptionPvk'), + signatureAlgorithm: config.getOptionalString('signatureAlgorithm') as + | SignatureAlgorithm + | undefined, + digestAlgorithm: config.getOptionalString('digestAlgorithm'), + + tokenIssuer, + appUrl: globalConfig.appUrl, + }; + + // passport-saml will return an error if the `cert` key is set, and the value is empty. + // Since we read from config (such as environment variables) an empty string should be equal to being unset. + if (!opts.cert) { + delete opts.cert; + } + return new SamlAuthProvider(opts); }; - - // passport-saml will return an error if the `cert` key is set, and the value is empty. - // Since we read from config (such as environment variables) an empty string should be equal to being unset. - if (!opts.cert) { - delete opts.cert; - } - return new SamlAuthProvider(opts); };