diff --git a/plugins/auth-backend/src/lib/EnvironmentHandler.ts b/plugins/auth-backend/src/lib/EnvironmentHandler.ts index 3eab4e793c..fb39ed86b8 100644 --- a/plugins/auth-backend/src/lib/EnvironmentHandler.ts +++ b/plugins/auth-backend/src/lib/EnvironmentHandler.ts @@ -15,7 +15,10 @@ */ import express from 'express'; -import { AuthProviderRouteHandlers } from '../providers/types'; +import { + AuthProviderRouteHandlers, + EnvironmentIdentifierFn, +} from '../providers/types'; export type EnvironmentHandlers = { [key: string]: AuthProviderRouteHandlers; @@ -25,13 +28,14 @@ export class EnvironmentHandler implements AuthProviderRouteHandlers { constructor( private readonly providerId: string, private readonly providers: EnvironmentHandlers, + private readonly envIdentifier: EnvironmentIdentifierFn, ) {} private getProviderForEnv( req: express.Request, res: express.Response, ): AuthProviderRouteHandlers | undefined { - const env = req.query.env?.toString(); + const env: string | undefined = this.envIdentifier(req); if (env && this.providers.hasOwnProperty(env)) { return this.providers[env]; diff --git a/plugins/auth-backend/src/lib/OAuthProvider.test.ts b/plugins/auth-backend/src/lib/OAuthProvider.test.ts index b08d279679..0778b875d0 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.test.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.test.ts @@ -21,6 +21,7 @@ import { THOUSAND_DAYS_MS, TEN_MINUTES_MS, verifyNonce, + encodeState, OAuthProvider, } from './OAuthProvider'; import { WebMessageResponse, OAuthProviderHandlers } from '../providers/types'; @@ -43,10 +44,11 @@ const mockResponseData = { describe('OAuthProvider Utils', () => { describe('verifyNonce', () => { it('should throw error if cookie nonce missing', () => { + const state = { nonce: 'NONCE', env: 'development' }; const mockRequest = ({ cookies: {}, query: { - state: 'NONCE', + state: encodeState(state), }, } as unknown) as express.Request; expect(() => { @@ -63,16 +65,17 @@ describe('OAuthProvider Utils', () => { } as unknown) as express.Request; expect(() => { verifyNonce(mockRequest, 'providera'); - }).toThrowError('Auth response is missing state nonce'); + }).toThrowError('Invalid state passed via request'); }); it('should throw error if nonce mismatch', () => { + const state = { nonce: 'NONCEB', env: 'development' }; const mockRequest = ({ cookies: { 'providera-nonce': 'NONCEA', }, query: { - state: 'NONCEB', + state: encodeState(state), }, } as unknown) as express.Request; expect(() => { @@ -81,12 +84,13 @@ describe('OAuthProvider Utils', () => { }); it('should not throw any error if nonce matches', () => { + const state = { nonce: 'NONCE', env: 'development' }; const mockRequest = ({ cookies: { 'providera-nonce': 'NONCE', }, query: { - state: 'NONCE', + state: encodeState(state), }, } as unknown) as express.Request; expect(() => { @@ -217,6 +221,7 @@ describe('OAuthProvider', () => { const mockRequest = ({ query: { scope: 'user', + env: 'development', }, } as unknown) as express.Request; @@ -249,12 +254,13 @@ describe('OAuthProvider', () => { disableRefresh: false, }); + const state = { nonce: 'nonce', env: 'development' }; const mockRequest = ({ cookies: { 'test-provider-nonce': 'nonce', }, query: { - state: 'nonce', + state: encodeState(state), }, } as unknown) as express.Request; diff --git a/plugins/auth-backend/src/lib/OAuthProvider.ts b/plugins/auth-backend/src/lib/OAuthProvider.ts index 9cd496536f..923a79b72c 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.ts @@ -22,6 +22,7 @@ import { OAuthProviderHandlers, WebMessageResponse, BackstageIdentity, + OAuthState, } from '../providers/types'; import { InputError } from '@backstage/backend-common'; import { TokenIssuer } from '../identity'; @@ -39,14 +40,41 @@ export type Options = { tokenIssuer: TokenIssuer; }; +const readState = (stateString: string): OAuthState => { + const state = Object.fromEntries( + new URLSearchParams(decodeURIComponent(stateString)), + ); + if ( + !state.nonce || + !state.env || + state.nonce?.length === 0 || + state.env?.length === 0 + ) { + throw Error(`Invalid state passed via request`); + } + return { + nonce: state.nonce, + env: state.env, + }; +}; + +export const encodeState = (state: OAuthState): string => { + const searchParams = new URLSearchParams(); + searchParams.append('nonce', state.nonce); + searchParams.append('env', state.env); + + return encodeURIComponent(searchParams.toString()); +}; + export const verifyNonce = (req: express.Request, providerId: string) => { const cookieNonce = req.cookies[`${providerId}-nonce`]; - const stateNonce = req.query.state; + const state: OAuthState = readState(req.query.state?.toString() ?? ''); + const stateNonce = state.nonce; if (!cookieNonce) { throw new Error('Auth response is missing cookie nonce'); } - if (!stateNonce) { + if (stateNonce.length === 0) { throw new Error('Auth response is missing state nonce'); } if (cookieNonce !== stateNonce) { @@ -104,6 +132,11 @@ export class OAuthProvider implements AuthProviderRouteHandlers { async start(req: express.Request, res: express.Response): Promise { // retrieve scopes from request const scope = req.query.scope?.toString() ?? ''; + const env = req.query.env?.toString(); + + if (!env) { + throw new InputError('No env provided in request query parameters'); + } if (this.options.persistScopes) { this.setScopesCookie(res, scope); @@ -113,9 +146,12 @@ export class OAuthProvider implements AuthProviderRouteHandlers { // set a nonce cookie before redirecting to oauth provider this.setNonceCookie(res, nonce); + const stateObject = { nonce: nonce, env: env }; + const stateParameter = encodeState(stateObject); + const queryParameters = { scope, - state: nonce, + state: stateParameter, }; const { url, status } = await this.providerHandlers.start( @@ -225,6 +261,19 @@ export class OAuthProvider implements AuthProviderRouteHandlers { } } + identifyEnv(req: express.Request): string | undefined { + const reqEnv = req.query.env?.toString(); + if (reqEnv) { + return reqEnv; + } + const stateParams = req.query.state?.toString(); + if (!stateParams) { + return undefined; + } + const env = readState(stateParams).env; + return env; + } + /** * If the response from the OAuth provider includes a Backstage identity, we * make sure it's populated with all the information we can derive from the user ID. diff --git a/plugins/auth-backend/src/providers/factories.ts b/plugins/auth-backend/src/providers/factories.ts index dcce98a986..701eef81af 100644 --- a/plugins/auth-backend/src/providers/factories.ts +++ b/plugins/auth-backend/src/providers/factories.ts @@ -23,7 +23,11 @@ import { createGoogleProvider } from './google'; import { createOAuth2Provider } from './oauth2'; import { createOktaProvider } from './okta'; import { createSamlProvider } from './saml'; -import { AuthProviderConfig, AuthProviderFactory } from './types'; +import { + AuthProviderConfig, + AuthProviderFactory, + EnvironmentIdentifierFn, +} from './types'; import { Config } from '@backstage/config'; import { EnvironmentHandlers, @@ -54,16 +58,26 @@ export const createAuthProviderRouter = ( const router = Router(); const envs = providerConfig.keys(); const envProviders: EnvironmentHandlers = {}; + let envIdentifier: EnvironmentIdentifierFn | undefined; for (const env of envs) { const envConfig = providerConfig.getConfig(env); const provider = factory(globalConfig, env, envConfig, logger, issuer); if (provider) { envProviders[env] = provider; + envIdentifier = provider.identifyEnv; } } - const handler = new EnvironmentHandler(providerId, envProviders); + if (typeof envIdentifier === 'undefined') { + throw Error(`No envIdentifier provided for '${providerId}'`); + } + + const handler = new EnvironmentHandler( + providerId, + envProviders, + envIdentifier, + ); router.get('/start', handler.start.bind(handler)); router.get('/handler/frame', handler.frameHandler.bind(handler)); diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index e9c719ea1c..863cfa9f5a 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -126,7 +126,7 @@ export class GithubAuthProvider implements OAuthProviderHandlers { export function createGithubProvider( { baseUrl }: AuthProviderConfig, - env: string, + _: string, envConfig: Config, logger: Logger, tokenIssuer: TokenIssuer, @@ -148,7 +148,7 @@ export function createGithubProvider( const userProfileURL = enterpriseInstanceUrl ? `${enterpriseInstanceUrl}/api/v3/user` : undefined; - const callbackURL = `${baseUrl}/${providerId}/handler/frame?env=${env}`; + const callbackURL = `${baseUrl}/${providerId}/handler/frame`; const opts = { clientID, diff --git a/plugins/auth-backend/src/providers/gitlab/provider.ts b/plugins/auth-backend/src/providers/gitlab/provider.ts index c5c546c6f3..22d0bd4f0a 100644 --- a/plugins/auth-backend/src/providers/gitlab/provider.ts +++ b/plugins/auth-backend/src/providers/gitlab/provider.ts @@ -133,7 +133,7 @@ export class GitlabAuthProvider implements OAuthProviderHandlers { export function createGitlabProvider( { baseUrl }: AuthProviderConfig, - env: string, + _: string, envConfig: Config, logger: Logger, tokenIssuer: TokenIssuer, @@ -145,7 +145,7 @@ export function createGitlabProvider( const clientSecret = envConfig.getString('clientSecret'); const audience = envConfig.getString('audience'); const baseURL = audience || 'https://gitlab.com'; - const callbackURL = `${baseUrl}/${providerId}/handler/frame?env=${env}`; + const callbackURL = `${baseUrl}/${providerId}/handler/frame`; const opts = { clientID, diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 5749d1465f..b8070949f9 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -145,7 +145,7 @@ export class GoogleAuthProvider implements OAuthProviderHandlers { export function createGoogleProvider( { baseUrl }: AuthProviderConfig, - env: string, + _: string, envConfig: Config, logger: Logger, tokenIssuer: TokenIssuer, @@ -155,7 +155,7 @@ export function createGoogleProvider( const appOrigin = envConfig.getString('appOrigin'); const clientID = envConfig.getString('clientId'); const clientSecret = envConfig.getString('clientSecret'); - const callbackURL = `${baseUrl}/${providerId}/handler/frame?env=${env}`; + const callbackURL = `${baseUrl}/${providerId}/handler/frame`; const opts = { clientID, diff --git a/plugins/auth-backend/src/providers/oauth2/provider.ts b/plugins/auth-backend/src/providers/oauth2/provider.ts index 967fd011f0..c03f7c4371 100644 --- a/plugins/auth-backend/src/providers/oauth2/provider.ts +++ b/plugins/auth-backend/src/providers/oauth2/provider.ts @@ -143,7 +143,7 @@ export class OAuth2AuthProvider implements OAuthProviderHandlers { export function createOAuth2Provider( { baseUrl }: AuthProviderConfig, - env: string, + _: string, envConfig: Config, logger: Logger, tokenIssuer: TokenIssuer, @@ -153,7 +153,7 @@ export function createOAuth2Provider( const appOrigin = envConfig.getString('appOrigin'); const clientID = envConfig.getString('clientId'); const clientSecret = envConfig.getString('clientSecret'); - const callbackURL = `${baseUrl}/${providerId}/handler/frame?env=${env}`; + const callbackURL = `${baseUrl}/${providerId}/handler/frame`; const authorizationURL = envConfig.getString('authorizationURL'); const tokenURL = envConfig.getString('tokenURL'); diff --git a/plugins/auth-backend/src/providers/okta/provider.ts b/plugins/auth-backend/src/providers/okta/provider.ts index 56e8b1220d..058300cef6 100644 --- a/plugins/auth-backend/src/providers/okta/provider.ts +++ b/plugins/auth-backend/src/providers/okta/provider.ts @@ -165,7 +165,7 @@ export class OktaAuthProvider implements OAuthProviderHandlers { export function createOktaProvider( { baseUrl }: AuthProviderConfig, - env: string, + _: string, envConfig: Config, logger: Logger, tokenIssuer: TokenIssuer, @@ -176,7 +176,7 @@ export function createOktaProvider( const clientID = envConfig.getString('clientId'); const clientSecret = envConfig.getString('clientSecret'); const audience = envConfig.getString('audience'); - const callbackURL = `${baseUrl}/${providerId}/handler/frame?env=${env}`; + const callbackURL = `${baseUrl}/${providerId}/handler/frame`; const opts = { audience, diff --git a/plugins/auth-backend/src/providers/saml/provider.ts b/plugins/auth-backend/src/providers/saml/provider.ts index 3ddfba4bb0..1ccee1a9cb 100644 --- a/plugins/auth-backend/src/providers/saml/provider.ts +++ b/plugins/auth-backend/src/providers/saml/provider.ts @@ -106,6 +106,10 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { async logout(_req: express.Request, res: express.Response): Promise { res.send('noop'); } + + identifyEnv(): string | undefined { + return undefined; + } } type SAMLProviderOptions = { diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 93d459bb0d..d372fd1b94 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -203,6 +203,15 @@ export interface AuthProviderRouteHandlers { * @param {express.Response} res */ logout?(req: express.Request, res: express.Response): Promise; + + /** + *(Optional) A method to identify the environment Context of the Request + * + *Request + *- contains the environment context information encoded in the request + * @param {express.Request} req + */ + identifyEnv?(req: express.Request): string | undefined; } export type AuthProviderFactory = ( @@ -332,3 +341,14 @@ export type SAMLProviderConfig = { export type SAMLEnvironmentProviderConfig = { [key: string]: SAMLProviderConfig; }; + +export type OAuthState = { + /* A type for the serialized value in the `state` parameter of the OAuth authorization flow + */ + nonce: string; + env: string; +}; + +export type EnvironmentIdentifierFn = ( + req: express.Request, +) => string | undefined;