From ee8f53966f25adfaf273f038fa43cfc1b4bfefe5 Mon Sep 17 00:00:00 2001 From: Andre Wanlin <67169551+awanlin@users.noreply.github.com> Date: Fri, 8 Sep 2023 14:18:37 -0500 Subject: [PATCH] Changed to address feedback Signed-off-by: Andre Wanlin <67169551+awanlin@users.noreply.github.com> --- .changeset/dull-eyes-pay.md | 2 +- .../dev/index.ts | 6 +- .../src/index.ts | 2 +- .../src/module.test.ts | 3 +- plugins/auth-backend/package.json | 1 + .../src/providers/oauth2/provider.test.ts | 89 ------- .../src/providers/oauth2/provider.ts | 230 +----------------- yarn.lock | 3 +- 8 files changed, 21 insertions(+), 315 deletions(-) delete mode 100644 plugins/auth-backend/src/providers/oauth2/provider.test.ts diff --git a/.changeset/dull-eyes-pay.md b/.changeset/dull-eyes-pay.md index 8fb6c32e8f..2a2b996051 100644 --- a/.changeset/dull-eyes-pay.md +++ b/.changeset/dull-eyes-pay.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-auth-backend-module-oauth2-provider': patch +'@backstage/plugin-auth-backend-module-oauth2-provider': minor --- New module for `@backstage/plugin-auth-backend` that adds a `oauth2` auth provider. diff --git a/plugins/auth-backend-module-oauth2-provider/dev/index.ts b/plugins/auth-backend-module-oauth2-provider/dev/index.ts index a32ce8d39b..d3c18c1d48 100644 --- a/plugins/auth-backend-module-oauth2-provider/dev/index.ts +++ b/plugins/auth-backend-module-oauth2-provider/dev/index.ts @@ -15,12 +15,10 @@ */ import { createBackend } from '@backstage/backend-defaults'; -import { authPlugin } from '@backstage/plugin-auth-backend'; -import { authModuleOauth2Provider } from '../src'; const backend = createBackend(); -backend.add(authPlugin); -backend.add(authModuleOauth2Provider); +backend.add(import('@backstage/plugin-auth-backend')); +backend.add(import('../src')); backend.start(); diff --git a/plugins/auth-backend-module-oauth2-provider/src/index.ts b/plugins/auth-backend-module-oauth2-provider/src/index.ts index 7b4312e9df..c55556defc 100644 --- a/plugins/auth-backend-module-oauth2-provider/src/index.ts +++ b/plugins/auth-backend-module-oauth2-provider/src/index.ts @@ -21,5 +21,5 @@ */ export { oauth2Authenticator } from './authenticator'; -export { authModuleOauth2Provider } from './module'; +export { authModuleOauth2Provider as default } from './module'; export { oauth2SignInResolvers } from './resolvers'; diff --git a/plugins/auth-backend-module-oauth2-provider/src/module.test.ts b/plugins/auth-backend-module-oauth2-provider/src/module.test.ts index 2321e9b3d2..ed0032c2dc 100644 --- a/plugins/auth-backend-module-oauth2-provider/src/module.test.ts +++ b/plugins/auth-backend-module-oauth2-provider/src/module.test.ts @@ -15,7 +15,6 @@ */ import { mockServices, startTestBackend } from '@backstage/backend-test-utils'; -import { authPlugin } from '@backstage/plugin-auth-backend'; import { authModuleOauth2Provider } from './module'; import request from 'supertest'; import { decodeOAuthState } from '@backstage/plugin-auth-node'; @@ -24,7 +23,7 @@ describe('authModuleOauth2Provider', () => { it('should start', async () => { const { server } = await startTestBackend({ features: [ - authPlugin, + import('@backstage/plugin-auth-backend'), authModuleOauth2Provider, mockServices.rootConfig.factory({ data: { diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index 73bd3aa2ea..b23be5bba7 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -42,6 +42,7 @@ "@backstage/plugin-auth-backend-module-github-provider": "workspace:^", "@backstage/plugin-auth-backend-module-gitlab-provider": "workspace:^", "@backstage/plugin-auth-backend-module-google-provider": "workspace:^", + "@backstage/plugin-auth-backend-module-oauth2-provider": "workspace:^", "@backstage/plugin-auth-node": "workspace:^", "@backstage/plugin-catalog-node": "workspace:^", "@backstage/types": "workspace:^", diff --git a/plugins/auth-backend/src/providers/oauth2/provider.test.ts b/plugins/auth-backend/src/providers/oauth2/provider.test.ts deleted file mode 100644 index 9bdf7095f3..0000000000 --- a/plugins/auth-backend/src/providers/oauth2/provider.test.ts +++ /dev/null @@ -1,89 +0,0 @@ -/* - * Copyright 2020 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { OAuth2AuthProvider } from './provider'; -import * as helpers from '../../lib/passport/PassportStrategyHelper'; -import { OAuthResult } from '../../lib/oauth'; -import { AuthResolverContext } from '../types'; - -jest.mock('../../lib/passport/PassportStrategyHelper', () => { - return { - executeFrameHandlerStrategy: jest.fn(), - executeRefreshTokenStrategy: jest.fn(), - executeFetchUserProfileStrategy: jest.fn(), - }; -}); - -const mockFrameHandler = jest.spyOn( - helpers, - 'executeFrameHandlerStrategy', -) as unknown as jest.MockedFunction< - () => Promise<{ result: OAuthResult; privateInfo: any }> ->; - -describe('createOAuth2Provider', () => { - it('should auth', async () => { - const provider = new OAuth2AuthProvider({ - resolverContext: {} as AuthResolverContext, - authHandler: async ({ fullProfile }) => ({ - profile: { - email: fullProfile.emails![0]!.value, - displayName: fullProfile.displayName, - picture: 'http://backstage.io/lols', - }, - }), - clientId: 'mock', - clientSecret: 'mock', - callbackUrl: 'mock', - authorizationUrl: 'mock', - tokenUrl: 'mock', - }); - - mockFrameHandler.mockResolvedValueOnce({ - result: { - fullProfile: { - emails: [{ value: 'conrad@example.com' }], - displayName: 'Conrad', - id: 'conrad', - provider: 'oAuth2', - }, - params: { - id_token: 'idToken', - scope: 'scope', - expires_in: 123, - }, - accessToken: 'accessToken', - }, - privateInfo: { - refreshToken: 'wacka', - }, - }); - const { response } = await provider.handler({} as any); - expect(response).toEqual({ - providerInfo: { - accessToken: 'accessToken', - expiresInSeconds: 123, - idToken: 'idToken', - scope: 'scope', - }, - profile: { - email: 'conrad@example.com', - displayName: 'Conrad', - picture: 'http://backstage.io/lols', - }, - }); - }); -}); diff --git a/plugins/auth-backend/src/providers/oauth2/provider.ts b/plugins/auth-backend/src/providers/oauth2/provider.ts index 3e7720b14a..de6e7b1cfa 100644 --- a/plugins/auth-backend/src/providers/oauth2/provider.ts +++ b/plugins/auth-backend/src/providers/oauth2/provider.ts @@ -14,183 +14,15 @@ * limitations under the License. */ -import express from 'express'; -import passport from 'passport'; -import { Strategy as OAuth2Strategy } from 'passport-oauth2'; -import { - encodeState, - OAuthAdapter, - OAuthEnvironmentHandler, - OAuthHandlers, - OAuthProviderOptions, - OAuthRefreshRequest, - OAuthResponse, - OAuthResult, - OAuthStartRequest, -} from '../../lib/oauth'; -import { - executeFetchUserProfileStrategy, - executeFrameHandlerStrategy, - executeRedirectStrategy, - executeRefreshTokenStrategy, - makeProfileInfo, - PassportDoneCallback, -} from '../../lib/passport'; -import { - AuthHandler, - AuthResolverContext, - OAuthStartResponse, - SignInResolver, -} from '../types'; +import { OAuthResult } from '../../lib/oauth'; +import { AuthHandler, SignInResolver } from '../types'; import { createAuthProviderIntegration } from '../createAuthProviderIntegration'; -import { InputError } from '@backstage/errors'; - -type PrivateInfo = { - refreshToken: string; -}; - -export type OAuth2AuthProviderOptions = OAuthProviderOptions & { - signInResolver?: SignInResolver; - authHandler: AuthHandler; - authorizationUrl: string; - tokenUrl: string; - scope?: string; - resolverContext: AuthResolverContext; - includeBasicAuth?: boolean; - disableRefresh?: boolean; -}; - -export class OAuth2AuthProvider implements OAuthHandlers { - private readonly _strategy: OAuth2Strategy; - private readonly signInResolver?: SignInResolver; - private readonly authHandler: AuthHandler; - private readonly resolverContext: AuthResolverContext; - private readonly disableRefresh: boolean; - - constructor(options: OAuth2AuthProviderOptions) { - this.signInResolver = options.signInResolver; - this.authHandler = options.authHandler; - this.resolverContext = options.resolverContext; - this.disableRefresh = options.disableRefresh ?? false; - - this._strategy = new OAuth2Strategy( - { - clientID: options.clientId, - clientSecret: options.clientSecret, - callbackURL: options.callbackUrl, - authorizationURL: options.authorizationUrl, - tokenURL: options.tokenUrl, - passReqToCallback: false, - scope: options.scope, - customHeaders: options.includeBasicAuth - ? { - Authorization: `Basic ${this.encodeClientCredentials( - options.clientId, - options.clientSecret, - )}`, - } - : undefined, - }, - ( - accessToken: any, - refreshToken: any, - params: any, - fullProfile: passport.Profile, - done: PassportDoneCallback, - ) => { - done( - undefined, - { - fullProfile, - accessToken, - refreshToken, - params, - }, - { - refreshToken, - }, - ); - }, - ); - } - - async start(req: OAuthStartRequest): Promise { - return await executeRedirectStrategy(req, this._strategy, { - accessType: 'offline', - prompt: 'consent', - scope: req.scope, - state: encodeState(req.state), - }); - } - - async handler(req: express.Request) { - const { result, privateInfo } = await executeFrameHandlerStrategy< - OAuthResult, - PrivateInfo - >(req, this._strategy); - - return { - response: await this.handleResult(result), - refreshToken: privateInfo.refreshToken, - }; - } - - async refresh(req: OAuthRefreshRequest) { - if (this.disableRefresh) { - throw new InputError('Session refreshes have been disabled'); - } - const refreshTokenResponse = await executeRefreshTokenStrategy( - this._strategy, - req.refreshToken, - req.scope, - ); - const { accessToken, params, refreshToken } = refreshTokenResponse; - - const fullProfile = await executeFetchUserProfileStrategy( - this._strategy, - accessToken, - ); - - return { - response: await this.handleResult({ - fullProfile, - params, - accessToken, - }), - refreshToken, - }; - } - - private async handleResult(result: OAuthResult) { - const { profile } = await this.authHandler(result, this.resolverContext); - - const response: OAuthResponse = { - providerInfo: { - idToken: result.params.id_token, - accessToken: result.accessToken, - scope: result.params.scope, - expiresInSeconds: result.params.expires_in, - }, - profile, - }; - - if (this.signInResolver) { - response.backstageIdentity = await this.signInResolver( - { - result, - profile, - }, - this.resolverContext, - ); - } - - return response; - } - - encodeClientCredentials(clientID: string, clientSecret: string): string { - return Buffer.from(`${clientID}:${clientSecret}`).toString('base64'); - } -} +import { + adaptLegacyOAuthHandler, + adaptLegacyOAuthSignInResolver, +} from '../../lib/legacy'; +import { createOAuthProviderFactory } from '@backstage/plugin-auth-node'; +import { oauth2Authenticator } from '@backstage/plugin-auth-backend-module-oauth2-provider'; /** * Auth provider integration for generic OAuth2 auth @@ -205,46 +37,10 @@ export const oauth2 = createAuthProviderIntegration({ 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 authorizationUrl = envConfig.getString('authorizationUrl'); - const tokenUrl = envConfig.getString('tokenUrl'); - const scope = envConfig.getOptionalString('scope'); - const includeBasicAuth = - envConfig.getOptionalBoolean('includeBasicAuth'); - const disableRefresh = - envConfig.getOptionalBoolean('disableRefresh') ?? false; - - const authHandler: AuthHandler = options?.authHandler - ? options.authHandler - : async ({ fullProfile, params }) => ({ - profile: makeProfileInfo(fullProfile, params.id_token), - }); - - const provider = new OAuth2AuthProvider({ - clientId, - clientSecret, - callbackUrl, - signInResolver: options?.signIn?.resolver, - authHandler, - authorizationUrl, - tokenUrl, - scope, - includeBasicAuth, - resolverContext, - disableRefresh, - }); - - return OAuthAdapter.fromConfig(globalConfig, provider, { - providerId, - callbackUrl, - }); - }); + return createOAuthProviderFactory({ + authenticator: oauth2Authenticator, + profileTransform: adaptLegacyOAuthHandler(options?.authHandler), + signInResolver: adaptLegacyOAuthSignInResolver(options?.signIn?.resolver), + }); }, }); diff --git a/yarn.lock b/yarn.lock index 7389191f19..e70229ff09 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4926,7 +4926,7 @@ __metadata: languageName: unknown linkType: soft -"@backstage/plugin-auth-backend-module-oauth2-provider@workspace:plugins/auth-backend-module-oauth2-provider": +"@backstage/plugin-auth-backend-module-oauth2-provider@workspace:^, @backstage/plugin-auth-backend-module-oauth2-provider@workspace:plugins/auth-backend-module-oauth2-provider": version: 0.0.0-use.local resolution: "@backstage/plugin-auth-backend-module-oauth2-provider@workspace:plugins/auth-backend-module-oauth2-provider" dependencies: @@ -4961,6 +4961,7 @@ __metadata: "@backstage/plugin-auth-backend-module-github-provider": "workspace:^" "@backstage/plugin-auth-backend-module-gitlab-provider": "workspace:^" "@backstage/plugin-auth-backend-module-google-provider": "workspace:^" + "@backstage/plugin-auth-backend-module-oauth2-provider": "workspace:^" "@backstage/plugin-auth-node": "workspace:^" "@backstage/plugin-catalog-node": "workspace:^" "@backstage/types": "workspace:^"