From 5f13a53c6ac7c2693c0e46a8abfb80945d58377c Mon Sep 17 00:00:00 2001 From: Raghunandan Date: Fri, 12 Jun 2020 12:06:46 +0200 Subject: [PATCH 1/2] Making auth backend work based on config. Config is yet to be read from the YAML and passed in. Localhost development will fail gracefully if AUTH config env vars are not set --- .../implementations/auth/github/GithubAuth.ts | 2 +- .../implementations/auth/google/GoogleAuth.ts | 2 +- .../src/lib/EnvironmentHandler.ts | 62 ++++++++ .../{providers => lib}/OAuthProvider.test.ts | 113 +++++++------ .../src/{providers => lib}/OAuthProvider.ts | 148 +++++++++--------- .../PassportStrategyHelper.test.ts | 0 .../PassportStrategyHelper.ts | 6 +- plugins/auth-backend/src/providers/config.ts | 43 ----- .../auth-backend/src/providers/factories.ts | 15 +- .../src/providers/github/provider.ts | 60 +++++-- .../src/providers/google/provider.ts | 62 ++++++-- .../src/providers/saml/provider.ts | 58 +++++-- plugins/auth-backend/src/providers/types.ts | 35 ++++- plugins/auth-backend/src/service/router.ts | 63 +++++++- 14 files changed, 441 insertions(+), 228 deletions(-) create mode 100644 plugins/auth-backend/src/lib/EnvironmentHandler.ts rename plugins/auth-backend/src/{providers => lib}/OAuthProvider.test.ts (78%) rename plugins/auth-backend/src/{providers => lib}/OAuthProvider.ts (64%) rename plugins/auth-backend/src/{providers => lib}/PassportStrategyHelper.test.ts (100%) rename plugins/auth-backend/src/{providers => lib}/PassportStrategyHelper.ts (97%) delete mode 100644 plugins/auth-backend/src/providers/config.ts diff --git a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts index ac05b718e5..f4a7092f79 100644 --- a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts +++ b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts @@ -57,7 +57,7 @@ class GithubAuth implements OAuthApi, SessionStateApi { static create({ apiOrigin, basePath, - environment = 'dev', + environment = 'development', provider = DEFAULT_PROVIDER, oauthRequestApi, }: CreateOptions) { diff --git a/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts b/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts index 1fc6f4f6b8..183ea592b4 100644 --- a/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts @@ -66,7 +66,7 @@ class GoogleAuth static create({ apiOrigin, basePath, - environment = 'dev', + environment = 'development', provider = DEFAULT_PROVIDER, oauthRequestApi, }: CreateOptions) { diff --git a/plugins/auth-backend/src/lib/EnvironmentHandler.ts b/plugins/auth-backend/src/lib/EnvironmentHandler.ts new file mode 100644 index 0000000000..4fb80ca3e8 --- /dev/null +++ b/plugins/auth-backend/src/lib/EnvironmentHandler.ts @@ -0,0 +1,62 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 express from 'express'; +import { AuthProviderRouteHandlers } from '../providers/types'; +import { NotFoundError } from '@backstage/backend-common'; + +export type EnvironmentHandlers = { + [key: string]: AuthProviderRouteHandlers; +}; + +export class EnvironmentHandler implements AuthProviderRouteHandlers { + constructor(private readonly providers: EnvironmentHandlers) {} + + private getProviderForEnv(req: express.Request): AuthProviderRouteHandlers { + const env = req.query.env?.toString(); + if (!this.providers.hasOwnProperty(env)) { + throw new NotFoundError( + `No environment for ${env} found in this provider`, + ); + } + return this.providers[env]; + } + + async start(req: express.Request, res: express.Response): Promise { + const provider = this.getProviderForEnv(req); + provider.start(req, res); + } + + async frameHandler( + req: express.Request, + res: express.Response, + ): Promise { + const provider = this.getProviderForEnv(req); + provider.frameHandler(req, res); + } + + async refresh(req: express.Request, res: express.Response): Promise { + const provider = this.getProviderForEnv(req); + if (provider.refresh) { + provider.refresh(req, res); + } + } + + async logout(req: express.Request, res: express.Response): Promise { + const provider = this.getProviderForEnv(req); + provider.logout(req, res); + } +} diff --git a/plugins/auth-backend/src/providers/OAuthProvider.test.ts b/plugins/auth-backend/src/lib/OAuthProvider.test.ts similarity index 78% rename from plugins/auth-backend/src/providers/OAuthProvider.test.ts rename to plugins/auth-backend/src/lib/OAuthProvider.test.ts index 362653b5f7..3a211e7b95 100644 --- a/plugins/auth-backend/src/providers/OAuthProvider.test.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.test.ts @@ -18,15 +18,12 @@ import express from 'express'; import { ensuresXRequestedWith, postMessageResponse, - removeRefreshTokenCookie, - setRefreshTokenCookie, THOUSAND_DAYS_MS, - setNonceCookie, TEN_MINUTES_MS, verifyNonce, OAuthProvider, } from './OAuthProvider'; -import { AuthResponse, OAuthProviderHandlers } from './types'; +import { AuthResponse, OAuthProviderHandlers } from '../providers/types'; describe('OAuthProvider Utils', () => { describe('verifyNonce', () => { @@ -80,52 +77,8 @@ describe('OAuthProvider Utils', () => { }); }); - describe('setNonceCookie', () => { - it('should set nonce cookie', () => { - const mockResponse = ({ - cookie: jest.fn().mockReturnThis(), - } as unknown) as express.Response; - setNonceCookie(mockResponse, 'providera'); - expect(mockResponse.cookie).toBeCalledTimes(1); - expect(mockResponse.cookie).toBeCalledWith( - 'providera-nonce', - expect.any(String), - expect.objectContaining({ maxAge: TEN_MINUTES_MS }), - ); - }); - }); - - describe('setRefreshTokenCookie', () => { - it('should set refresh token cookie', () => { - const mockResponse = ({ - cookie: jest.fn().mockReturnThis(), - } as unknown) as express.Response; - setRefreshTokenCookie(mockResponse, 'providera', 'REFRESH_TOKEN'); - expect(mockResponse.cookie).toBeCalledTimes(1); - expect(mockResponse.cookie).toBeCalledWith( - 'providera-refresh-token', - 'REFRESH_TOKEN', - expect.objectContaining({ maxAge: THOUSAND_DAYS_MS }), - ); - }); - }); - - describe('removeRefreshTokenCookie', () => { - it('should remove refresh token cookie', () => { - const mockResponse = ({ - cookie: jest.fn().mockReturnThis(), - } as unknown) as express.Response; - removeRefreshTokenCookie(mockResponse, 'providera'); - expect(mockResponse.cookie).toBeCalledTimes(1); - expect(mockResponse.cookie).toBeCalledWith( - 'providera-refresh-token', - '', - expect.objectContaining({ maxAge: 0 }), - ); - }); - }); - describe('postMessageResponse', () => { + const appOrigin = 'http://localhost:3000'; it('should post a message back with payload success', () => { const mockResponse = ({ end: jest.fn().mockReturnThis(), @@ -144,7 +97,7 @@ describe('OAuthProvider Utils', () => { const jsonData = JSON.stringify(data); const base64Data = Buffer.from(jsonData, 'utf8').toString('base64'); - postMessageResponse(mockResponse, data); + postMessageResponse(mockResponse, appOrigin, data); expect(mockResponse.setHeader).toBeCalledTimes(2); expect(mockResponse.end).toBeCalledTimes(1); expect(mockResponse.end).toBeCalledWith( @@ -165,7 +118,7 @@ describe('OAuthProvider Utils', () => { const jsonData = JSON.stringify(data); const base64Data = Buffer.from(jsonData, 'utf8').toString('base64'); - postMessageResponse(mockResponse, data); + postMessageResponse(mockResponse, appOrigin, data); expect(mockResponse.setHeader).toBeCalledTimes(2); expect(mockResponse.end).toBeCalledTimes(1); expect(mockResponse.end).toBeCalledWith( @@ -221,10 +174,19 @@ describe('OAuthProvider', () => { } } const providerInstance = new MyAuthProvider(); - const providerId = 'test-provider'; + const oAuthProviderOptions = { + providerId: 'test-provider', + secure: false, + disableRefresh: true, + baseUrl: 'http://localhost:7000/auth', + appOrigin: 'http://localhost:3000', + }; it('sets the correct headers in start', async () => { - const oauthProvider = new OAuthProvider(providerInstance, providerId); + const oauthProvider = new OAuthProvider( + providerInstance, + oAuthProviderOptions, + ); const mockRequest = ({ query: { scope: 'user', @@ -239,6 +201,14 @@ describe('OAuthProvider', () => { } as unknown) as express.Response; await oauthProvider.start(mockRequest, mockResponse); + // nonce cookie checks + expect(mockResponse.cookie).toBeCalledTimes(1); + expect(mockResponse.cookie).toBeCalledWith( + `${oAuthProviderOptions.providerId}-nonce`, + expect.any(String), + expect.objectContaining({ maxAge: TEN_MINUTES_MS }), + ); + // redirect checks expect(mockResponse.setHeader).toHaveBeenCalledTimes(2); expect(mockResponse.setHeader).toHaveBeenCalledWith('Location', '/url'); expect(mockResponse.setHeader).toHaveBeenCalledWith('Content-Length', '0'); @@ -247,7 +217,11 @@ describe('OAuthProvider', () => { }); it('sets the refresh cookie if refresh is enabled', async () => { - const oauthProvider = new OAuthProvider(providerInstance, providerId); + oAuthProviderOptions.disableRefresh = false; + const oauthProvider = new OAuthProvider( + providerInstance, + oAuthProviderOptions, + ); const mockRequest = ({ cookies: { @@ -269,12 +243,19 @@ describe('OAuthProvider', () => { expect(mockResponse.cookie).toHaveBeenCalledWith( expect.stringContaining('test-provider-refresh-token'), expect.stringContaining('token'), - expect.objectContaining({ path: '/auth/test-provider' }), + expect.objectContaining({ + path: '/auth/test-provider', + maxAge: THOUSAND_DAYS_MS, + }), ); }); - it('does no set the refresh cookie if refresh is disabled', async () => { - const oauthProvider = new OAuthProvider(providerInstance, providerId, true); + it('does not set the refresh cookie if refresh is disabled', async () => { + oAuthProviderOptions.disableRefresh = true; + const oauthProvider = new OAuthProvider( + providerInstance, + oAuthProviderOptions, + ); const mockRequest = ({ cookies: { @@ -296,7 +277,11 @@ describe('OAuthProvider', () => { }); it('removes refresh cookie when logging out', async () => { - const oauthProvider = new OAuthProvider(providerInstance, providerId); + oAuthProviderOptions.disableRefresh = false; + const oauthProvider = new OAuthProvider( + providerInstance, + oAuthProviderOptions, + ); const mockRequest = ({ header: () => 'XMLHttpRequest', @@ -317,7 +302,11 @@ describe('OAuthProvider', () => { }); it('gets new access-token when refreshing', async () => { - const oauthProvider = new OAuthProvider(providerInstance, providerId); + oAuthProviderOptions.disableRefresh = false; + const oauthProvider = new OAuthProvider( + providerInstance, + oAuthProviderOptions, + ); const mockRequest = ({ header: () => 'XMLHttpRequest', @@ -341,7 +330,11 @@ describe('OAuthProvider', () => { }); it('handles refresh without capabilities', async () => { - const oauthProvider = new OAuthProvider(providerInstance, providerId, true); + oAuthProviderOptions.disableRefresh = true; + const oauthProvider = new OAuthProvider( + providerInstance, + oAuthProviderOptions, + ); const mockRequest = ({ header: () => 'XMLHttpRequest', diff --git a/plugins/auth-backend/src/providers/OAuthProvider.ts b/plugins/auth-backend/src/lib/OAuthProvider.ts similarity index 64% rename from plugins/auth-backend/src/providers/OAuthProvider.ts rename to plugins/auth-backend/src/lib/OAuthProvider.ts index a2be783155..62b565b60d 100644 --- a/plugins/auth-backend/src/providers/OAuthProvider.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.ts @@ -14,20 +14,29 @@ * limitations under the License. */ -import express, { CookieOptions } from 'express'; +import express from 'express'; import crypto from 'crypto'; +import { URL } from 'url'; import { AuthResponse, AuthProviderRouteHandlers, OAuthProviderHandlers, -} from './types'; +} from '../providers/types'; import { InputError } from '@backstage/backend-common'; export const THOUSAND_DAYS_MS = 1000 * 24 * 60 * 60 * 1000; export const TEN_MINUTES_MS = 600 * 1000; -export const verifyNonce = (req: express.Request, provider: string) => { - const cookieNonce = req.cookies[`${provider}-nonce`]; +export type Options = { + providerId: string; + secure: boolean; + disableRefresh?: boolean; + baseUrl: string; + appOrigin: string; +}; + +export const verifyNonce = (req: express.Request, providerId: string) => { + const cookieNonce = req.cookies[`${providerId}-nonce`]; const stateNonce = req.query.state; if (!cookieNonce || !stateNonce) { @@ -39,58 +48,9 @@ export const verifyNonce = (req: express.Request, provider: string) => { } }; -export const setNonceCookie = (res: express.Response, provider: string) => { - const nonce = crypto.randomBytes(16).toString('base64'); - - const options: CookieOptions = { - maxAge: TEN_MINUTES_MS, - secure: false, - sameSite: 'none', - domain: 'localhost', - path: `/auth/${provider}/handler`, - httpOnly: true, - }; - - res.cookie(`${provider}-nonce`, nonce, options); - - return nonce; -}; - -export const setRefreshTokenCookie = ( - res: express.Response, - provider: string, - refreshToken: string, -) => { - const options: CookieOptions = { - maxAge: THOUSAND_DAYS_MS, - secure: false, - sameSite: 'none', - domain: 'localhost', - path: `/auth/${provider}`, - httpOnly: true, - }; - - res.cookie(`${provider}-refresh-token`, refreshToken, options); -}; - -export const removeRefreshTokenCookie = ( - res: express.Response, - provider: string, -) => { - const options: CookieOptions = { - maxAge: 0, - secure: false, - sameSite: 'none', - domain: 'localhost', - path: `/auth/${provider}`, - httpOnly: true, - }; - - res.cookie(`${provider}-refresh-token`, '', options); -}; - export const postMessageResponse = ( res: express.Response, + appOrigin: string, data: AuthResponse, ) => { const jsonData = JSON.stringify(data); @@ -104,7 +64,7 @@ export const postMessageResponse = ( @@ -122,17 +82,16 @@ export const ensuresXRequestedWith = (req: express.Request) => { }; export class OAuthProvider implements AuthProviderRouteHandlers { - private readonly provider: string; - private readonly providerHandlers: OAuthProviderHandlers; - private readonly disableRefresh: boolean; + private readonly domain: string; + private readonly basePath: string; + constructor( - providerHandlers: OAuthProviderHandlers, - provider: string, - disableRefresh?: boolean, + private readonly providerHandlers: OAuthProviderHandlers, + private readonly options: Options, ) { - this.provider = provider; - this.providerHandlers = providerHandlers; - this.disableRefresh = disableRefresh ?? false; + const url = new URL(options.baseUrl); + this.domain = url.hostname; + this.basePath = url.pathname; } async start(req: express.Request, res: express.Response): Promise { @@ -143,8 +102,9 @@ export class OAuthProvider implements AuthProviderRouteHandlers { throw new InputError('missing scope parameter'); } + const nonce = crypto.randomBytes(16).toString('base64'); // set a nonce cookie before redirecting to oauth provider - const nonce = setNonceCookie(res, this.provider); + this.setNonceCookie(res, nonce); const options = { scope, @@ -152,6 +112,7 @@ export class OAuthProvider implements AuthProviderRouteHandlers { prompt: 'consent', state: nonce, }; + const { url, status } = await this.providerHandlers.start(req, options); res.statusCode = status || 302; @@ -166,11 +127,11 @@ export class OAuthProvider implements AuthProviderRouteHandlers { ): Promise { try { // verify nonce cookie and state cookie on callback - verifyNonce(req, this.provider); + verifyNonce(req, this.options.providerId); const { user, info } = await this.providerHandlers.handler(req); - if (!this.disableRefresh) { + if (!this.options.disableRefresh) { // throw error if missing refresh token const { refreshToken } = info; if (!refreshToken) { @@ -178,17 +139,17 @@ export class OAuthProvider implements AuthProviderRouteHandlers { } // set new refresh token - setRefreshTokenCookie(res, this.provider, refreshToken); + this.setRefreshTokenCookie(res, refreshToken); } // post message back to popup if successful - return postMessageResponse(res, { + return postMessageResponse(res, this.options.appOrigin, { type: 'auth-result', payload: user, }); } catch (error) { // post error message back to popup if failure - return postMessageResponse(res, { + return postMessageResponse(res, this.options.appOrigin, { type: 'auth-result', error: { name: error.name, @@ -203,9 +164,9 @@ export class OAuthProvider implements AuthProviderRouteHandlers { return res.status(401).send('Invalid X-Requested-With header'); } - if (!this.disableRefresh) { + if (!this.options.disableRefresh) { // remove refresh token cookie before logout - removeRefreshTokenCookie(res, this.provider); + this.removeRefreshTokenCookie(res); } return res.send('logout!'); } @@ -215,14 +176,15 @@ export class OAuthProvider implements AuthProviderRouteHandlers { return res.status(401).send('Invalid X-Requested-With header'); } - if (!this.providerHandlers.refresh || this.disableRefresh) { + if (!this.providerHandlers.refresh || this.options.disableRefresh) { return res.send( - `Refresh token not supported for provider: ${this.provider}`, + `Refresh token not supported for provider: ${this.options.providerId}`, ); } try { - const refreshToken = req.cookies[`${this.provider}-refresh-token`]; + const refreshToken = + req.cookies[`${this.options.providerId}-refresh-token`]; // throw error if refresh token is missing in the request if (!refreshToken) { @@ -241,4 +203,40 @@ export class OAuthProvider implements AuthProviderRouteHandlers { return res.status(401).send(`${error.message}`); } } + + private setNonceCookie = (res: express.Response, nonce: string) => { + res.cookie(`${this.options.providerId}-nonce`, nonce, { + maxAge: TEN_MINUTES_MS, + secure: this.options.secure, + sameSite: 'none', + domain: this.domain, + path: `${this.basePath}/${this.options.providerId}/handler`, + httpOnly: true, + }); + }; + + private setRefreshTokenCookie = ( + res: express.Response, + refreshToken: string, + ) => { + res.cookie(`${this.options.providerId}-refresh-token`, refreshToken, { + maxAge: THOUSAND_DAYS_MS, + secure: this.options.secure, + sameSite: 'none', + domain: this.domain, + path: `${this.basePath}/${this.options.providerId}`, + httpOnly: true, + }); + }; + + private removeRefreshTokenCookie = (res: express.Response) => { + res.cookie(`${this.options.providerId}-refresh-token`, '', { + maxAge: 0, + secure: false, + sameSite: 'none', + domain: `${this.domain}`, + path: `${this.basePath}/${this.options.providerId}`, + httpOnly: true, + }); + }; } diff --git a/plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts b/plugins/auth-backend/src/lib/PassportStrategyHelper.test.ts similarity index 100% rename from plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts rename to plugins/auth-backend/src/lib/PassportStrategyHelper.test.ts diff --git a/plugins/auth-backend/src/providers/PassportStrategyHelper.ts b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts similarity index 97% rename from plugins/auth-backend/src/providers/PassportStrategyHelper.ts rename to plugins/auth-backend/src/lib/PassportStrategyHelper.ts index 7b7e467281..f9e32a2b6f 100644 --- a/plugins/auth-backend/src/providers/PassportStrategyHelper.ts +++ b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts @@ -17,7 +17,11 @@ import express from 'express'; import passport from 'passport'; import jwtDecoder from 'jwt-decode'; -import { RedirectInfo, RefreshTokenResponse, ProfileInfo } from './types'; +import { + RedirectInfo, + RefreshTokenResponse, + ProfileInfo, +} from '../providers/types'; export const makeProfileInfo = ( profile: passport.Profile, diff --git a/plugins/auth-backend/src/providers/config.ts b/plugins/auth-backend/src/providers/config.ts deleted file mode 100644 index 5ec73b7827..0000000000 --- a/plugins/auth-backend/src/providers/config.ts +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * 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. - */ - -export const providers = [ - { - provider: 'google', - options: { - clientID: process.env.AUTH_GOOGLE_CLIENT_ID!, - clientSecret: process.env.AUTH_GOOGLE_CLIENT_SECRET!, - callbackURL: 'http://localhost:7000/auth/google/handler/frame', - }, - }, - { - provider: 'github', - options: { - clientID: process.env.AUTH_GITHUB_CLIENT_ID!, - clientSecret: process.env.AUTH_GITHUB_CLIENT_SECRET!, - callbackURL: 'http://localhost:7000/auth/github/handler/frame', - }, - disableRefresh: true, - }, - { - provider: 'saml', - options: { - path: '/auth/saml/handler/frame', - entryPoint: 'http://localhost:7001/', - issuer: 'passport-saml', - }, - }, -]; diff --git a/plugins/auth-backend/src/providers/factories.ts b/plugins/auth-backend/src/providers/factories.ts index 8576e36096..4cc9212620 100644 --- a/plugins/auth-backend/src/providers/factories.ts +++ b/plugins/auth-backend/src/providers/factories.ts @@ -19,6 +19,7 @@ import { createGithubProvider } from './github'; import { createGoogleProvider } from './google'; import { createSamlProvider } from './saml'; import { AuthProviderFactory, AuthProviderConfig } from './types'; +import { Logger } from 'winston'; const factories: { [providerId: string]: AuthProviderFactory } = { google: createGoogleProvider, @@ -26,17 +27,18 @@ const factories: { [providerId: string]: AuthProviderFactory } = { saml: createSamlProvider, }; -export function createAuthProvider(providerId: string, config: any) { +export const createAuthProviderRouter = ( + providerId: string, + globalConfig: AuthProviderConfig, + providerConfig: any, // TODO: make this a config reader object of sorts + logger: Logger, +) => { const factory = factories[providerId]; if (!factory) { throw Error(`No auth provider available for '${providerId}'`); } - return factory(config); -} -export const createAuthProviderRouter = (config: AuthProviderConfig) => { - const providerId = config.provider; - const provider = createAuthProvider(providerId, config); + const provider = factory(globalConfig, providerConfig, logger); const router = Router(); router.get('/start', provider.start.bind(provider)); @@ -46,5 +48,6 @@ export const createAuthProviderRouter = (config: AuthProviderConfig) => { if (provider.refresh) { router.get('/refresh', provider.refresh.bind(provider)); } + return router; }; diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index 09622d9303..aa96b0f38b 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -19,24 +19,30 @@ import { Strategy as GithubStrategy } from 'passport-github2'; import { executeFrameHandlerStrategy, executeRedirectStrategy, -} from '../PassportStrategyHelper'; +} from '../../lib/PassportStrategyHelper'; import { OAuthProviderHandlers, AuthProviderConfig, RedirectInfo, AuthInfoBase, AuthInfoPrivate, + EnvironmentProviderConfig, + OAuthProviderOptions, + OAuthProviderConfig, } from '../types'; -import { OAuthProvider } from '../OAuthProvider'; +import { OAuthProvider } from '../../lib/OAuthProvider'; +import { + EnvironmentHandlers, + EnvironmentHandler, +} from '../../lib/EnvironmentHandler'; +import { Logger } from 'winston'; export class GithubAuthProvider implements OAuthProviderHandlers { - private readonly providerConfig: AuthProviderConfig; private readonly _strategy: GithubStrategy; - constructor(providerConfig: AuthProviderConfig) { - this.providerConfig = providerConfig; + constructor(options: OAuthProviderOptions) { this._strategy = new GithubStrategy( - { ...this.providerConfig.options }, + { ...options }, (accessToken: any, _: any, params: any, profile: any, done: any) => { done(undefined, { profile, @@ -59,8 +65,42 @@ export class GithubAuthProvider implements OAuthProviderHandlers { } } -export function createGithubProvider(config: AuthProviderConfig) { - const provider = new GithubAuthProvider(config); - const oauthProvider = new OAuthProvider(provider, config.provider, true); - return oauthProvider; +export function createGithubProvider( + { baseUrl }: AuthProviderConfig, + providerConfig: EnvironmentProviderConfig, + logger: Logger, +) { + const envProviders: EnvironmentHandlers = {}; + + for (const [env, envConfig] of Object.entries(providerConfig)) { + const config = (envConfig as unknown) as OAuthProviderConfig; + const { secure, appOrigin } = config; + const callbackURLParam = env === 'development' ? '?env=development' : ''; + const opts = { + clientID: config.clientId, + clientSecret: config.clientSecret, + callbackURL: `${baseUrl}/github/handler/frame${callbackURLParam}`, + }; + + if (!opts.clientID || !opts.clientSecret) { + if (process.env.NODE_ENV !== 'development') { + throw new Error( + 'Failed to initialize Github auth provider, set AUTH_GITHUB_CLIENT_ID and AUTH_GITHUB_CLIENT_SECRET env vars', + ); + } + + logger.warn( + 'Github auth provider disabled, set AUTH_GITHUB_CLIENT_ID and AUTH_GITHUB_CLIENT_SECRET env vars to enable', + ); + continue; + } + + envProviders[env] = new OAuthProvider(new GithubAuthProvider(opts), { + providerId: 'github', + secure, + baseUrl, + appOrigin, + }); + } + return new EnvironmentHandler(envProviders); } diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 066e16e330..b1d82266fc 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -22,7 +22,7 @@ import { executeRefreshTokenStrategy, makeProfileInfo, executeFetchUserProfileStrategy, -} from '../PassportStrategyHelper'; +} from '../../lib/PassportStrategyHelper'; import { OAuthProviderHandlers, AuthInfoBase, @@ -30,19 +30,27 @@ import { RedirectInfo, AuthProviderConfig, AuthInfoWithProfile, + EnvironmentProviderConfig, + OAuthProviderOptions, + OAuthProviderConfig, } from '../types'; -import { OAuthProvider } from '../OAuthProvider'; +import { OAuthProvider } from '../../lib/OAuthProvider'; import passport from 'passport'; +import { + EnvironmentHandler, + EnvironmentHandlers, +} from '../../lib/EnvironmentHandler'; +import { Logger } from 'winston'; export class GoogleAuthProvider implements OAuthProviderHandlers { - private readonly providerConfig: AuthProviderConfig; private readonly _strategy: GoogleStrategy; - constructor(providerConfig: AuthProviderConfig) { - this.providerConfig = providerConfig; + constructor(options: OAuthProviderOptions) { // TODO: throw error if env variables not set? this._strategy = new GoogleStrategy( - { ...this.providerConfig.options }, + // We need passReqToCallback set to false to get params, but there's + // no matching type signature for that, so instead behold this beauty + { ...options, passReqToCallback: false as true }, ( accessToken: any, refreshToken: any, @@ -104,8 +112,42 @@ export class GoogleAuthProvider implements OAuthProviderHandlers { } } -export function createGoogleProvider(config: AuthProviderConfig) { - const provider = new GoogleAuthProvider(config); - const oauthProvider = new OAuthProvider(provider, config.provider); - return oauthProvider; +export function createGoogleProvider( + { baseUrl }: AuthProviderConfig, + providerConfig: EnvironmentProviderConfig, + logger: Logger, +) { + const envProviders: EnvironmentHandlers = {}; + + for (const [env, envConfig] of Object.entries(providerConfig)) { + const config = (envConfig as unknown) as OAuthProviderConfig; + const { secure, appOrigin } = config; + const callbackURLParam = env === 'development' ? '?env=development' : ''; + const opts = { + clientID: config.clientId, + clientSecret: config.clientSecret, + callbackURL: `${baseUrl}/google/handler/frame${callbackURLParam}`, + }; + + if (!opts.clientID || !opts.clientSecret) { + if (process.env.NODE_ENV !== 'development') { + throw new Error( + 'Failed to initialize Google auth provider, set AUTH_GOOGLE_CLIENT_ID and AUTH_GOOGLE_CLIENT_SECRET env vars', + ); + } + + logger.warn( + 'Google auth provider disabled, set AUTH_GOOGLE_CLIENT_ID and AUTH_GOOGLE_CLIENT_SECRET env vars to enable', + ); + continue; + } + + envProviders[env] = new OAuthProvider(new GoogleAuthProvider(opts), { + providerId: 'google', + secure, + baseUrl, + appOrigin, + }); + } + return new EnvironmentHandler(envProviders); } diff --git a/plugins/auth-backend/src/providers/saml/provider.ts b/plugins/auth-backend/src/providers/saml/provider.ts index 50bea3495e..621b75078c 100644 --- a/plugins/auth-backend/src/providers/saml/provider.ts +++ b/plugins/auth-backend/src/providers/saml/provider.ts @@ -19,16 +19,26 @@ import { Strategy as SamlStrategy } from 'passport-saml'; import { executeFrameHandlerStrategy, executeRedirectStrategy, -} from '../PassportStrategyHelper'; -import { AuthProviderConfig, AuthProviderRouteHandlers } from '../types'; -import { postMessageResponse } from '../OAuthProvider'; +} from '../../lib/PassportStrategyHelper'; +import { + AuthProviderConfig, + AuthProviderRouteHandlers, + EnvironmentProviderConfig, + SAMLProviderConfig, +} from '../types'; +import { postMessageResponse } from '../../lib/OAuthProvider'; +import { + EnvironmentHandlers, + EnvironmentHandler, +} from '../../lib/EnvironmentHandler'; +import { Logger } from 'winston'; export class SamlAuthProvider implements AuthProviderRouteHandlers { private readonly strategy: SamlStrategy; - constructor(providerConfig: AuthProviderConfig) { + constructor(options: SAMLProviderOptions) { this.strategy = new SamlStrategy( - { ...providerConfig.options }, + { ...options }, (profile: any, done: any) => { // TODO: There's plenty more validation and profile handling to do here, // this provider is currently only intended to validate the provider pattern @@ -57,12 +67,12 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { try { const { user } = await executeFrameHandlerStrategy(req, this.strategy); - return postMessageResponse(res, { + return postMessageResponse(res, 'http://localhost:3000', { type: 'auth-result', payload: user, }); } catch (error) { - return postMessageResponse(res, { + return postMessageResponse(res, 'http://localhost:3000', { type: 'auth-result', error: { name: error.name, @@ -77,6 +87,36 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { } } -export function createSamlProvider(config: AuthProviderConfig) { - return new SamlAuthProvider(config); +type SAMLProviderOptions = { + entryPoint: string; + issuer: string; + path: string; +}; + +export function createSamlProvider( + _authProviderConfig: AuthProviderConfig, + providerConfig: EnvironmentProviderConfig, + logger: Logger, +) { + const envProviders: EnvironmentHandlers = {}; + + for (const [env, envConfig] of Object.entries(providerConfig)) { + const config = (envConfig as unknown) as SAMLProviderConfig; + const opts = { + entryPoint: config.entryPoint, + issuer: config.issuer, + path: '/auth/saml/handler/frame', + }; + + if (!opts.entryPoint || !opts.issuer) { + logger.warn( + 'SAML auth provider disabled, set entryPoint and entryPoint in saml auth config to enable', + ); + continue; + } + + envProviders[env] = new SamlAuthProvider(opts); + } + + return new EnvironmentHandler(envProviders); } diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index b8252ddc97..419a041637 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -15,11 +15,32 @@ */ import express from 'express'; +import { Logger } from 'winston'; + +export type OAuthProviderOptions = { + clientID: string; + clientSecret: string; + callbackURL: string; +}; + +export type SAMLProviderConfig = { + entryPoint: string; + issuer: string; +}; + +export type EnvironmentProviderConfig = { + [key: string]: OAuthProviderConfig | SAMLProviderConfig; +}; export type AuthProviderConfig = { - provider: string; - options: any; - disableRefresh?: boolean; + baseUrl: string; +}; + +export type OAuthProviderConfig = { + secure: boolean; + appOrigin: string; // http://localhost:3000 + clientId: string; + clientSecret: string; }; export interface OAuthProviderHandlers { @@ -36,8 +57,14 @@ export interface AuthProviderRouteHandlers { logout(req: express.Request, res: express.Response): Promise; } +export type SAMLEnvironmentProviderConfig = { + [key: string]: SAMLProviderConfig; +}; + export type AuthProviderFactory = ( - config: AuthProviderConfig, + globalConfig: AuthProviderConfig, + providerConfig: EnvironmentProviderConfig, + logger: Logger, ) => AuthProviderRouteHandlers; export type AuthInfoBase = { diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 9de5fb80a5..20c35a4084 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -19,7 +19,6 @@ import Router from 'express-promise-router'; import cookieParser from 'cookie-parser'; import bodyParser from 'body-parser'; import { Logger } from 'winston'; -import { providers } from './../providers/config'; import { createAuthProviderRouter } from '../providers'; export interface RouterOptions { @@ -36,13 +35,61 @@ export async function createRouter( router.use(bodyParser.urlencoded({ extended: false })); router.use(bodyParser.json()); - // configure all the providers - for (const providerConfig of providers) { - const { provider } = providerConfig; - const providerRouter = createAuthProviderRouter(providerConfig); - logger.info(`Configuring provider, ${provider}`); - router.use(`/${provider}`, providerRouter); - } + // TODO: read from app config + const config = { + backend: { + baseUrl: 'http://localhost:7000', + }, + auth: { + providers: { + google: { + development: { + appOrigin: 'http://localhost:3000', + secure: false, + clientId: process.env.AUTH_GOOGLE_CLIENT_ID!, + clientSecret: process.env.AUTH_GOOGLE_CLIENT_SECRET!, + }, + production: { + appOrigin: 'http://localhost:3000', + secure: false, + clientId: '', + clientSecret: '', + }, + }, + github: { + development: { + appOrigin: 'http://localhost:3000', + secure: false, + clientId: process.env.AUTH_GITHUB_CLIENT_ID!, + clientSecret: process.env.AUTH_GITHUB_CLIENT_SECRET!, + }, + }, + saml: { + development: { + entryPoint: 'http://localhost:7001/', + issuer: 'passport-saml', + }, + }, + }, + }, + }; + const providerConfigs = config.auth.providers; + + for (const [providerId, providerConfig] of Object.entries(providerConfigs)) { + const baseUrl = `${config.backend.baseUrl}/auth`; + logger.info(`Configuring provider, ${providerId}`); + try { + const providerRouter = createAuthProviderRouter( + providerId, + { baseUrl }, + providerConfig, + logger, + ); + router.use(`/${providerId}`, providerRouter); + } catch (e) { + logger.error(e.message); + } + } return router; } From 2c6b8659d36478a22dab134bc7ea401c7d460416 Mon Sep 17 00:00:00 2001 From: Raghunandan Date: Fri, 12 Jun 2020 14:40:06 +0200 Subject: [PATCH 2/2] review fixes --- .../src/lib/OAuthProvider.test.ts | 44 +++++++++---------- .../src/providers/github/provider.ts | 2 +- .../src/providers/google/provider.ts | 2 +- 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/plugins/auth-backend/src/lib/OAuthProvider.test.ts b/plugins/auth-backend/src/lib/OAuthProvider.test.ts index 3a211e7b95..bb261d1380 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.test.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.test.ts @@ -217,11 +217,10 @@ describe('OAuthProvider', () => { }); it('sets the refresh cookie if refresh is enabled', async () => { - oAuthProviderOptions.disableRefresh = false; - const oauthProvider = new OAuthProvider( - providerInstance, - oAuthProviderOptions, - ); + const oauthProvider = new OAuthProvider(providerInstance, { + ...oAuthProviderOptions, + disableRefresh: false, + }); const mockRequest = ({ cookies: { @@ -251,11 +250,10 @@ describe('OAuthProvider', () => { }); it('does not set the refresh cookie if refresh is disabled', async () => { - oAuthProviderOptions.disableRefresh = true; - const oauthProvider = new OAuthProvider( - providerInstance, - oAuthProviderOptions, - ); + const oauthProvider = new OAuthProvider(providerInstance, { + ...oAuthProviderOptions, + disableRefresh: true, + }); const mockRequest = ({ cookies: { @@ -277,11 +275,10 @@ describe('OAuthProvider', () => { }); it('removes refresh cookie when logging out', async () => { - oAuthProviderOptions.disableRefresh = false; - const oauthProvider = new OAuthProvider( - providerInstance, - oAuthProviderOptions, - ); + const oauthProvider = new OAuthProvider(providerInstance, { + ...oAuthProviderOptions, + disableRefresh: false, + }); const mockRequest = ({ header: () => 'XMLHttpRequest', @@ -303,10 +300,10 @@ describe('OAuthProvider', () => { it('gets new access-token when refreshing', async () => { oAuthProviderOptions.disableRefresh = false; - const oauthProvider = new OAuthProvider( - providerInstance, - oAuthProviderOptions, - ); + const oauthProvider = new OAuthProvider(providerInstance, { + ...oAuthProviderOptions, + disableRefresh: false, + }); const mockRequest = ({ header: () => 'XMLHttpRequest', @@ -330,11 +327,10 @@ describe('OAuthProvider', () => { }); it('handles refresh without capabilities', async () => { - oAuthProviderOptions.disableRefresh = true; - const oauthProvider = new OAuthProvider( - providerInstance, - oAuthProviderOptions, - ); + const oauthProvider = new OAuthProvider(providerInstance, { + ...oAuthProviderOptions, + disableRefresh: true, + }); const mockRequest = ({ header: () => 'XMLHttpRequest', diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index aa96b0f38b..0f8f185015 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -75,7 +75,7 @@ export function createGithubProvider( for (const [env, envConfig] of Object.entries(providerConfig)) { const config = (envConfig as unknown) as OAuthProviderConfig; const { secure, appOrigin } = config; - const callbackURLParam = env === 'development' ? '?env=development' : ''; + const callbackURLParam = `?env=${env}`; const opts = { clientID: config.clientId, clientSecret: config.clientSecret, diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index b1d82266fc..971f588dce 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -122,7 +122,7 @@ export function createGoogleProvider( for (const [env, envConfig] of Object.entries(providerConfig)) { const config = (envConfig as unknown) as OAuthProviderConfig; const { secure, appOrigin } = config; - const callbackURLParam = env === 'development' ? '?env=development' : ''; + const callbackURLParam = `?env=${env}`; const opts = { clientID: config.clientId, clientSecret: config.clientSecret,