Merge pull request #1812 from GoWind/abstract_env

auth-backend: Encode `env` in the OAuth flow into the `state` parameter. Refs #1775

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];

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;
 

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<void> {
     // 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.

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));

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,

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,

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,

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');
 

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,

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<void> {
     res.send('noop');
   }
+
+  identifyEnv(): string | undefined {
+    return undefined;
+  }
 }
 
 type SAMLProviderOptions = {

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<void>;
+
+  /**
+   *(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;