From 5e0341f01c33b32d4943deaa53f91a1575b6d1a4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 19 Aug 2020 11:25:22 +0200 Subject: [PATCH 1/2] plugins/auth-backend: refactor OAuthProvider to derive options from app.baseUrl --- .../src/lib/OAuthProvider.test.ts | 3 +- plugins/auth-backend/src/lib/OAuthProvider.ts | 50 ++++++++++++------- .../src/providers/auth0/provider.ts | 11 ++-- .../src/providers/github/provider.ts | 11 ++-- .../src/providers/gitlab/provider.ts | 11 ++-- .../src/providers/google/provider.ts | 11 ++-- .../src/providers/oauth2/provider.ts | 20 ++++---- .../src/providers/okta/provider.ts | 11 ++-- plugins/auth-backend/src/providers/types.ts | 5 ++ plugins/auth-backend/src/service/router.ts | 3 +- 10 files changed, 67 insertions(+), 69 deletions(-) diff --git a/plugins/auth-backend/src/lib/OAuthProvider.test.ts b/plugins/auth-backend/src/lib/OAuthProvider.test.ts index 0778b875d0..06bbd15c51 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.test.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.test.ts @@ -205,8 +205,9 @@ describe('OAuthProvider', () => { providerId: 'test-provider', secure: false, disableRefresh: true, - baseUrl: 'http://localhost:7000/auth', appOrigin: 'http://localhost:3000', + cookieDomain: 'localhost', + cookiePath: '/auth/test-provider', tokenIssuer: { issueToken: async () => 'my-id-token', listPublicKeys: async () => ({ keys: [] }), diff --git a/plugins/auth-backend/src/lib/OAuthProvider.ts b/plugins/auth-backend/src/lib/OAuthProvider.ts index f2b8c8e09c..dccab34da1 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.ts @@ -23,6 +23,7 @@ import { WebMessageResponse, BackstageIdentity, OAuthState, + AuthProviderConfig, } from '../providers/types'; import { InputError } from '@backstage/backend-common'; import { TokenIssuer } from '../identity'; @@ -35,7 +36,8 @@ export type Options = { secure: boolean; disableRefresh?: boolean; persistScopes?: boolean; - baseUrl: string; + cookieDomain: string; + cookiePath: string; appOrigin: string; tokenIssuer: TokenIssuer; }; @@ -120,17 +122,31 @@ export const ensuresXRequestedWith = (req: express.Request) => { }; export class OAuthProvider implements AuthProviderRouteHandlers { - private readonly domain: string; - private readonly basePath: string; + static fromConfig( + config: AuthProviderConfig, + providerHandlers: OAuthProviderHandlers, + options: Pick< + Options, + 'providerId' | 'persistScopes' | 'disableRefresh' | 'tokenIssuer' + >, + ): OAuthProvider { + const { origin: appOrigin } = new URL(config.appUrl); + const secure = config.baseUrl.startsWith('https://'); + const url = new URL(config.baseUrl); + const cookiePath = `${url.pathname}/${options.providerId}`; + return new OAuthProvider(providerHandlers, { + ...options, + appOrigin, + cookieDomain: url.hostname, + cookiePath, + secure, + }); + } constructor( private readonly providerHandlers: OAuthProviderHandlers, private readonly options: Options, - ) { - const url = new URL(options.baseUrl); - this.domain = url.hostname; - this.basePath = url.pathname; - } + ) {} async start(req: express.Request, res: express.Response): Promise { // retrieve scopes from request @@ -298,8 +314,8 @@ export class OAuthProvider implements AuthProviderRouteHandlers { maxAge: TEN_MINUTES_MS, secure: this.options.secure, sameSite: 'lax', - domain: this.domain, - path: `${this.basePath}/${this.options.providerId}/handler`, + domain: this.options.cookieDomain, + path: `${this.options.cookiePath}/handler`, httpOnly: true, }); }; @@ -309,8 +325,8 @@ export class OAuthProvider implements AuthProviderRouteHandlers { maxAge: TEN_MINUTES_MS, secure: this.options.secure, sameSite: 'lax', - domain: this.domain, - path: `${this.basePath}/${this.options.providerId}/handler`, + domain: this.options.cookieDomain, + path: `${this.options.cookiePath}/handler`, httpOnly: true, }); }; @@ -327,8 +343,8 @@ export class OAuthProvider implements AuthProviderRouteHandlers { maxAge: THOUSAND_DAYS_MS, secure: this.options.secure, sameSite: 'lax', - domain: this.domain, - path: `${this.basePath}/${this.options.providerId}`, + domain: this.options.cookieDomain, + path: this.options.cookiePath, httpOnly: true, }); }; @@ -336,10 +352,10 @@ export class OAuthProvider implements AuthProviderRouteHandlers { private removeRefreshTokenCookie = (res: express.Response) => { res.cookie(`${this.options.providerId}-refresh-token`, '', { maxAge: 0, - secure: false, + secure: this.options.secure, sameSite: 'lax', - domain: `${this.domain}`, - path: `${this.basePath}/${this.options.providerId}`, + domain: this.options.cookieDomain, + path: this.options.cookiePath, httpOnly: true, }); }; diff --git a/plugins/auth-backend/src/providers/auth0/provider.ts b/plugins/auth-backend/src/providers/auth0/provider.ts index 6dfcf3830a..903f6dc63c 100644 --- a/plugins/auth-backend/src/providers/auth0/provider.ts +++ b/plugins/auth-backend/src/providers/auth0/provider.ts @@ -141,19 +141,17 @@ export class Auth0AuthProvider implements OAuthProviderHandlers { } export function createAuth0Provider( - { baseUrl }: AuthProviderConfig, + config: AuthProviderConfig, _: string, envConfig: Config, logger: Logger, tokenIssuer: TokenIssuer, ) { const providerId = 'auth0'; - const secure = envConfig.getBoolean('secure'); - const appOrigin = envConfig.getString('appOrigin'); const clientID = envConfig.getString('clientId'); const clientSecret = envConfig.getString('clientSecret'); const domain = envConfig.getString('domain'); - const callbackURL = `${baseUrl}/${providerId}/handler/frame`; + const callbackURL = `${config.baseUrl}/${providerId}/handler/frame`; const opts = { clientID, @@ -175,12 +173,9 @@ export function createAuth0Provider( return undefined; } - return new OAuthProvider(new Auth0AuthProvider(opts), { + return OAuthProvider.fromConfig(config, new Auth0AuthProvider(opts), { disableRefresh: true, providerId, - secure, - baseUrl, - appOrigin, tokenIssuer, }); } diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index 863cfa9f5a..78db99c75c 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -125,15 +125,13 @@ export class GithubAuthProvider implements OAuthProviderHandlers { } export function createGithubProvider( - { baseUrl }: AuthProviderConfig, + config: AuthProviderConfig, _: string, envConfig: Config, logger: Logger, tokenIssuer: TokenIssuer, ) { const providerId = 'github'; - const secure = envConfig.getBoolean('secure'); - const appOrigin = envConfig.getString('appOrigin'); const clientID = envConfig.getString('clientId'); const clientSecret = envConfig.getString('clientSecret'); const enterpriseInstanceUrl = envConfig.getOptionalString( @@ -148,7 +146,7 @@ export function createGithubProvider( const userProfileURL = enterpriseInstanceUrl ? `${enterpriseInstanceUrl}/api/v3/user` : undefined; - const callbackURL = `${baseUrl}/${providerId}/handler/frame`; + const callbackURL = `${config.baseUrl}/${providerId}/handler/frame`; const opts = { clientID, @@ -171,13 +169,10 @@ export function createGithubProvider( ); return undefined; } - return new OAuthProvider(new GithubAuthProvider(opts), { + return OAuthProvider.fromConfig(config, new GithubAuthProvider(opts), { disableRefresh: true, persistScopes: true, providerId, - secure, - baseUrl, - appOrigin, tokenIssuer, }); } diff --git a/plugins/auth-backend/src/providers/gitlab/provider.ts b/plugins/auth-backend/src/providers/gitlab/provider.ts index 22d0bd4f0a..e9246a30c0 100644 --- a/plugins/auth-backend/src/providers/gitlab/provider.ts +++ b/plugins/auth-backend/src/providers/gitlab/provider.ts @@ -132,20 +132,18 @@ export class GitlabAuthProvider implements OAuthProviderHandlers { } export function createGitlabProvider( - { baseUrl }: AuthProviderConfig, + config: AuthProviderConfig, _: string, envConfig: Config, logger: Logger, tokenIssuer: TokenIssuer, ) { const providerId = 'gitlab'; - const secure = envConfig.getBoolean('secure'); - const appOrigin = envConfig.getString('appOrigin'); const clientID = envConfig.getString('clientId'); const clientSecret = envConfig.getString('clientSecret'); const audience = envConfig.getString('audience'); const baseURL = audience || 'https://gitlab.com'; - const callbackURL = `${baseUrl}/${providerId}/handler/frame`; + const callbackURL = `${config.baseUrl}/${providerId}/handler/frame`; const opts = { clientID, @@ -166,12 +164,9 @@ export function createGitlabProvider( ); return undefined; } - return new OAuthProvider(new GitlabAuthProvider(opts), { + return OAuthProvider.fromConfig(config, new GitlabAuthProvider(opts), { disableRefresh: true, providerId, - secure, - baseUrl, - appOrigin, tokenIssuer, }); } diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index b8070949f9..d956cb2f44 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -144,18 +144,16 @@ export class GoogleAuthProvider implements OAuthProviderHandlers { } export function createGoogleProvider( - { baseUrl }: AuthProviderConfig, + config: AuthProviderConfig, _: string, envConfig: Config, logger: Logger, tokenIssuer: TokenIssuer, ) { const providerId = 'google'; - const secure = envConfig.getBoolean('secure'); - const appOrigin = envConfig.getString('appOrigin'); const clientID = envConfig.getString('clientId'); const clientSecret = envConfig.getString('clientSecret'); - const callbackURL = `${baseUrl}/${providerId}/handler/frame`; + const callbackURL = `${config.baseUrl}/${providerId}/handler/frame`; const opts = { clientID, @@ -175,12 +173,9 @@ export function createGoogleProvider( ); return undefined; } - return new OAuthProvider(new GoogleAuthProvider(opts), { + return OAuthProvider.fromConfig(config, new GoogleAuthProvider(opts), { disableRefresh: false, providerId, - secure, - baseUrl, - appOrigin, tokenIssuer, }); } diff --git a/plugins/auth-backend/src/providers/oauth2/provider.ts b/plugins/auth-backend/src/providers/oauth2/provider.ts index c03f7c4371..ed3bbba6ea 100644 --- a/plugins/auth-backend/src/providers/oauth2/provider.ts +++ b/plugins/auth-backend/src/providers/oauth2/provider.ts @@ -29,7 +29,7 @@ import { } from '../../lib/PassportStrategyHelper'; import { AuthProviderConfig, - GenericOAuth2ProviderOptions, + OAuthProviderOptions, OAuthProviderHandlers, OAuthResponse, PassportDoneCallback, @@ -41,10 +41,15 @@ type PrivateInfo = { refreshToken: string; }; +export type OAuth2AuthProviderOptions = OAuthProviderOptions & { + authorizationURL: string; + tokenURL: string; +}; + export class OAuth2AuthProvider implements OAuthProviderHandlers { private readonly _strategy: OAuth2Strategy; - constructor(options: GenericOAuth2ProviderOptions) { + constructor(options: OAuth2AuthProviderOptions) { this._strategy = new OAuth2Strategy( { ...options, passReqToCallback: false as true }, ( @@ -142,18 +147,16 @@ export class OAuth2AuthProvider implements OAuthProviderHandlers { } export function createOAuth2Provider( - { baseUrl }: AuthProviderConfig, + config: AuthProviderConfig, _: string, envConfig: Config, logger: Logger, tokenIssuer: TokenIssuer, ) { const providerId = 'oauth2'; - const secure = envConfig.getBoolean('secure'); - const appOrigin = envConfig.getString('appOrigin'); const clientID = envConfig.getString('clientId'); const clientSecret = envConfig.getString('clientSecret'); - const callbackURL = `${baseUrl}/${providerId}/handler/frame`; + const callbackURL = `${config.baseUrl}/${providerId}/handler/frame`; const authorizationURL = envConfig.getString('authorizationURL'); const tokenURL = envConfig.getString('tokenURL'); @@ -182,12 +185,9 @@ export function createOAuth2Provider( ); return undefined; } - return new OAuthProvider(new OAuth2AuthProvider(opts), { + return OAuthProvider.fromConfig(config, new OAuth2AuthProvider(opts), { disableRefresh: false, providerId, - secure, - baseUrl, - appOrigin, tokenIssuer, }); } diff --git a/plugins/auth-backend/src/providers/okta/provider.ts b/plugins/auth-backend/src/providers/okta/provider.ts index 058300cef6..cfae366f7e 100644 --- a/plugins/auth-backend/src/providers/okta/provider.ts +++ b/plugins/auth-backend/src/providers/okta/provider.ts @@ -164,19 +164,17 @@ export class OktaAuthProvider implements OAuthProviderHandlers { } export function createOktaProvider( - { baseUrl }: AuthProviderConfig, + config: AuthProviderConfig, _: string, envConfig: Config, logger: Logger, tokenIssuer: TokenIssuer, ) { const providerId = 'okta'; - const secure = envConfig.getBoolean('secure'); - const appOrigin = envConfig.getString('appOrigin'); const clientID = envConfig.getString('clientId'); const clientSecret = envConfig.getString('clientSecret'); const audience = envConfig.getString('audience'); - const callbackURL = `${baseUrl}/${providerId}/handler/frame`; + const callbackURL = `${config.baseUrl}/${providerId}/handler/frame`; const opts = { audience, @@ -197,12 +195,9 @@ export function createOktaProvider( ); return undefined; } - return new OAuthProvider(new OktaAuthProvider(opts), { + return OAuthProvider.fromConfig(config, new OktaAuthProvider(opts), { disableRefresh: false, providerId, - secure, - baseUrl, - appOrigin, tokenIssuer, }); } diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index d372fd1b94..78cd4e1fe3 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -93,6 +93,11 @@ export type AuthProviderConfig = { * callbackURL to redirect to once the user signs in to the auth provider. */ baseUrl: string; + + /** + * The base URL of the app as provided by app.baseUrl + */ + appUrl: string; }; /** diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 69f058f6db..70cd6d8f99 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -36,6 +36,7 @@ export async function createRouter( const router = Router(); const logger = options.logger.child({ plugin: 'auth' }); + const appUrl = options.config.getString('app.baseUrl'); const backendUrl = options.config.getString('backend.baseUrl'); const authUrl = `${backendUrl}/auth`; @@ -64,7 +65,7 @@ export async function createRouter( const providerConfig = providersConfig.getConfig(providerId); const providerRouter = createAuthProviderRouter( providerId, - { baseUrl: authUrl }, + { baseUrl: authUrl, appUrl }, providerConfig, logger, tokenIssuer, From 1d9007184caa52b0899844e3d5b7dceb4dcd2dd2 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 19 Aug 2020 11:26:29 +0200 Subject: [PATCH 2/2] app-config: remove unnecessary auth config --- app-config.yaml | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/app-config.yaml b/app-config.yaml index 99415b5b41..4321ef138c 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -54,8 +54,6 @@ auth: providers: google: development: - appOrigin: 'http://localhost:3000/' - secure: false clientId: $secret: env: AUTH_GOOGLE_CLIENT_ID @@ -64,8 +62,6 @@ auth: env: AUTH_GOOGLE_CLIENT_SECRET github: development: - appOrigin: 'http://localhost:3000/' - secure: false clientId: $secret: env: AUTH_GITHUB_CLIENT_ID @@ -77,8 +73,6 @@ auth: env: AUTH_GITHUB_ENTERPRISE_INSTANCE_URL gitlab: development: - appOrigin: 'http://localhost:3000/' - secure: false clientId: $secret: env: AUTH_GITLAB_CLIENT_ID @@ -94,8 +88,6 @@ auth: # issuer: "passport-saml" okta: development: - appOrigin: 'http://localhost:3000/' - secure: false clientId: $secret: env: AUTH_OKTA_CLIENT_ID @@ -107,8 +99,6 @@ auth: env: AUTH_OKTA_AUDIENCE oauth2: development: - appOrigin: 'http://localhost:3000/' - secure: false clientId: $secret: env: AUTH_OAUTH2_CLIENT_ID @@ -123,8 +113,6 @@ auth: env: AUTH_OAUTH2_TOKEN_URL auth0: development: - appOrigin: 'http://localhost:3000/' - secure: false clientId: $secret: env: AUTH_AUTH0_CLIENT_ID