Merge pull request #2293 from spotify/rugvip/reoauth

auth-backend: refactor OAuthHandlers to receive a single decorated request object

plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts

@@ -24,9 +24,9 @@ import {
 } from '../../providers/types';
 import { InputError } from '@backstage/backend-common';
 import { TokenIssuer } from '../../identity';
-import { verifyNonce, encodeState } from './helpers';
+import { verifyNonce } from './helpers';
 import { postMessageResponse, ensuresXRequestedWith } from '../flow';
-import { OAuthHandlers } from './types';
+import { OAuthHandlers, OAuthStartRequest, OAuthRefreshRequest } from './types';
 
 export const THOUSAND_DAYS_MS = 1000 * 24 * 60 * 60 * 1000;
 export const TEN_MINUTES_MS = 600 * 1000;
@@ -86,15 +86,12 @@ export class OAuthAdapter 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 state = { nonce: nonce, env: env };
+    const forwardReq = Object.assign(req, { scope, state });
 
-    const queryParameters = {
-      scope,
-      state: stateParameter,
-    };
-
-    const { url, status } = await this.handlers.start(req, queryParameters);
+    const { url, status } = await this.handlers.start(
+      forwardReq as OAuthStartRequest,
+    );
 
     res.statusCode = status || 302;
     res.setHeader('Location', url);
@@ -185,8 +182,12 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
 
       const scope = req.query.scope?.toString() ?? '';
 
+      const forwardReq = Object.assign(req, { scope, refreshToken });
+
       // get new access_token
-      const response = await this.handlers.refresh(refreshToken, scope);
+      const response = await this.handlers.refresh(
+        forwardReq as OAuthRefreshRequest,
+      );
 
       await this.populateIdentity(response.backstageIdentity);
 

plugins/auth-backend/src/lib/oauth/index.ts

@@ -16,10 +16,13 @@
 
 export { OAuthEnvironmentHandler } from './OAuthEnvironmentHandler';
 export { OAuthAdapter } from './OAuthAdapter';
+export { encodeState } from './helpers';
 export type {
   OAuthHandlers,
   OAuthProviderInfo,
   OAuthProviderOptions,
   OAuthResponse,
   OAuthState,
+  OAuthStartRequest,
+  OAuthRefreshRequest,
 } from './types';

plugins/auth-backend/src/lib/oauth/types.ts

@@ -67,6 +67,16 @@ export type OAuthState = {
   env: string;
 };
 
+export type OAuthStartRequest = express.Request<{}> & {
+  scope: string;
+  state: OAuthState;
+};
+
+export type OAuthRefreshRequest = express.Request<{}> & {
+  scope: string;
+  refreshToken: string;
+};
+
 /**
  * Any OAuth provider needs to implement this interface which has provider specific
  * handlers for different methods to perform authentication, get access tokens,
@@ -78,10 +88,7 @@ export interface OAuthHandlers {
    * @param {express.Request} req
    * @param options
    */
-  start(
-    req: express.Request,
-    options: Record<string, string>,
-  ): Promise<RedirectInfo>;
+  start(req: OAuthStartRequest): Promise<RedirectInfo>;
 
   /**
    * Handles the redirect from the auth provider when the user has signed in.
@@ -99,10 +106,7 @@ export interface OAuthHandlers {
    * @param {string} refreshToken
    * @param {string} scope
    */
-  refresh?(
-    refreshToken: string,
-    scope: string,
-  ): Promise<AuthResponse<OAuthProviderInfo>>;
+  refresh?(req: OAuthRefreshRequest): Promise<AuthResponse<OAuthProviderInfo>>;
 
   /**
    * (Optional) Sign out of the auth provider.

plugins/auth-backend/src/providers/auth0/provider.ts

@@ -23,6 +23,9 @@ import {
   OAuthHandlers,
   OAuthResponse,
   OAuthEnvironmentHandler,
+  OAuthStartRequest,
+  encodeState,
+  OAuthRefreshRequest,
 } from '../../lib/oauth';
 import {
   executeFetchUserProfileStrategy,
@@ -81,16 +84,13 @@ export class Auth0AuthProvider implements OAuthHandlers {
     );
   }
 
-  async start(
-    req: express.Request,
-    options: Record<string, string>,
-  ): Promise<RedirectInfo> {
-    const providerOptions = {
-      ...options,
+  async start(req: OAuthStartRequest): Promise<RedirectInfo> {
+    return await executeRedirectStrategy(req, this._strategy, {
       accessType: 'offline',
       prompt: 'consent',
-    };
-    return await executeRedirectStrategy(req, this._strategy, providerOptions);
+      scope: req.scope,
+      state: encodeState(req.state),
+    });
   }
 
   async handler(
@@ -107,11 +107,11 @@ export class Auth0AuthProvider implements OAuthHandlers {
     };
   }
 
-  async refresh(refreshToken: string, scope: string): Promise<OAuthResponse> {
+  async refresh(req: OAuthRefreshRequest): Promise<OAuthResponse> {
     const { accessToken, params } = await executeRefreshTokenStrategy(
       this._strategy,
-      refreshToken,
-      scope,
+      req.refreshToken,
+      req.scope,
     );
 
     const profile = await executeFetchUserProfileStrategy(

plugins/auth-backend/src/providers/github/provider.ts

@@ -29,6 +29,8 @@ import {
   OAuthHandlers,
   OAuthResponse,
   OAuthEnvironmentHandler,
+  OAuthStartRequest,
+  encodeState,
 } from '../../lib/oauth';
 import passport from 'passport';
 
@@ -117,11 +119,11 @@ export class GithubAuthProvider implements OAuthHandlers {
     );
   }
 
-  async start(
-    req: express.Request,
-    options: Record<string, string>,
-  ): Promise<RedirectInfo> {
-    return await executeRedirectStrategy(req, this._strategy, options);
+  async start(req: OAuthStartRequest): Promise<RedirectInfo> {
+    return await executeRedirectStrategy(req, this._strategy, {
+      scope: req.scope,
+      state: encodeState(req.state),
+    });
   }
 
   async handler(req: express.Request) {

plugins/auth-backend/src/providers/gitlab/provider.ts

@@ -29,6 +29,8 @@ import {
   OAuthHandlers,
   OAuthResponse,
   OAuthEnvironmentHandler,
+  OAuthStartRequest,
+  encodeState,
 } from '../../lib/oauth';
 import passport from 'passport';
 
@@ -122,11 +124,11 @@ export class GitlabAuthProvider implements OAuthHandlers {
     );
   }
 
-  async start(
-    req: express.Request,
-    options: Record<string, string>,
-  ): Promise<RedirectInfo> {
-    return await executeRedirectStrategy(req, this._strategy, options);
+  async start(req: OAuthStartRequest): Promise<RedirectInfo> {
+    return await executeRedirectStrategy(req, this._strategy, {
+      scope: req.scope,
+      state: encodeState(req.state),
+    });
   }
 
   async handler(req: express.Request): Promise<{ response: OAuthResponse }> {

plugins/auth-backend/src/providers/google/provider.ts

@@ -31,6 +31,9 @@ import {
   OAuthProviderOptions,
   OAuthResponse,
   OAuthEnvironmentHandler,
+  OAuthStartRequest,
+  encodeState,
+  OAuthRefreshRequest,
 } from '../../lib/oauth';
 import passport from 'passport';
 
@@ -79,16 +82,13 @@ export class GoogleAuthProvider implements OAuthHandlers {
     );
   }
 
-  async start(
-    req: express.Request,
-    options: Record<string, string>,
-  ): Promise<RedirectInfo> {
-    const providerOptions = {
-      ...options,
+  async start(req: OAuthStartRequest): Promise<RedirectInfo> {
+    return await executeRedirectStrategy(req, this._strategy, {
       accessType: 'offline',
       prompt: 'consent',
-    };
-    return await executeRedirectStrategy(req, this._strategy, providerOptions);
+      scope: req.scope,
+      state: encodeState(req.state),
+    });
   }
 
   async handler(
@@ -105,11 +105,11 @@ export class GoogleAuthProvider implements OAuthHandlers {
     };
   }
 
-  async refresh(refreshToken: string, scope: string): Promise<OAuthResponse> {
+  async refresh(req: OAuthRefreshRequest): Promise<OAuthResponse> {
     const { accessToken, params } = await executeRefreshTokenStrategy(
       this._strategy,
-      refreshToken,
-      scope,
+      req.refreshToken,
+      req.scope,
     );
 
     const profile = await executeFetchUserProfileStrategy(

plugins/auth-backend/src/providers/microsoft/provider.ts

@@ -35,6 +35,9 @@ import {
   OAuthHandlers,
   OAuthResponse,
   OAuthEnvironmentHandler,
+  OAuthStartRequest,
+  encodeState,
+  OAuthRefreshRequest,
 } from '../../lib/oauth';
 
 import got from 'got';
@@ -111,11 +114,11 @@ export class MicrosoftAuthProvider implements OAuthHandlers {
     );
   }
 
-  async start(
-    req: express.Request,
-    options: Record<string, string>,
-  ): Promise<RedirectInfo> {
-    return await executeRedirectStrategy(req, this._strategy, options);
+  async start(req: OAuthStartRequest): Promise<RedirectInfo> {
+    return await executeRedirectStrategy(req, this._strategy, {
+      scope: req.scope,
+      state: encodeState(req.state),
+    });
   }
 
   async handler(
@@ -132,11 +135,11 @@ export class MicrosoftAuthProvider implements OAuthHandlers {
     };
   }
 
-  async refresh(refreshToken: string, scope: string): Promise<OAuthResponse> {
+  async refresh(req: OAuthRefreshRequest): Promise<OAuthResponse> {
     const { accessToken, params } = await executeRefreshTokenStrategy(
       this._strategy,
-      refreshToken,
-      scope,
+      req.refreshToken,
+      req.scope,
     );
 
     const profile = await executeFetchUserProfileStrategy(

plugins/auth-backend/src/providers/oauth2/provider.ts

@@ -23,6 +23,9 @@ import {
   OAuthHandlers,
   OAuthResponse,
   OAuthEnvironmentHandler,
+  OAuthStartRequest,
+  encodeState,
+  OAuthRefreshRequest,
 } from '../../lib/oauth';
 import {
   executeFetchUserProfileStrategy,
@@ -84,16 +87,13 @@ export class OAuth2AuthProvider implements OAuthHandlers {
     );
   }
 
-  async start(
-    req: express.Request,
-    options: Record<string, string>,
-  ): Promise<RedirectInfo> {
-    const providerOptions = {
-      ...options,
+  async start(req: OAuthStartRequest): Promise<RedirectInfo> {
+    return await executeRedirectStrategy(req, this._strategy, {
       accessType: 'offline',
       prompt: 'consent',
-    };
-    return await executeRedirectStrategy(req, this._strategy, providerOptions);
+      scope: req.scope,
+      state: encodeState(req.state),
+    });
   }
 
   async handler(
@@ -110,11 +110,11 @@ export class OAuth2AuthProvider implements OAuthHandlers {
     };
   }
 
-  async refresh(refreshToken: string, scope: string): Promise<OAuthResponse> {
+  async refresh(req: OAuthRefreshRequest): Promise<OAuthResponse> {
     const refreshTokenResponse = await executeRefreshTokenStrategy(
       this._strategy,
-      refreshToken,
-      scope,
+      req.refreshToken,
+      req.scope,
     );
     const {
       accessToken,

plugins/auth-backend/src/providers/okta/provider.ts

@@ -20,6 +20,9 @@ import {
   OAuthHandlers,
   OAuthResponse,
   OAuthEnvironmentHandler,
+  OAuthStartRequest,
+  encodeState,
+  OAuthRefreshRequest,
 } from '../../lib/oauth';
 import { Strategy as OktaStrategy } from 'passport-okta-oauth';
 import passport from 'passport';
@@ -101,16 +104,13 @@ export class OktaAuthProvider implements OAuthHandlers {
     );
   }
 
-  async start(
-    req: express.Request,
-    options: Record<string, string>,
-  ): Promise<RedirectInfo> {
-    const providerOptions = {
-      ...options,
+  async start(req: OAuthStartRequest): Promise<RedirectInfo> {
+    return await executeRedirectStrategy(req, this._strategy, {
       accessType: 'offline',
       prompt: 'consent',
-    };
-    return await executeRedirectStrategy(req, this._strategy, providerOptions);
+      scope: req.scope,
+      state: encodeState(req.state),
+    });
   }
 
   async handler(
@@ -127,11 +127,11 @@ export class OktaAuthProvider implements OAuthHandlers {
     };
   }
 
-  async refresh(refreshToken: string, scope: string): Promise<OAuthResponse> {
+  async refresh(req: OAuthRefreshRequest): Promise<OAuthResponse> {
     const { accessToken, params } = await executeRefreshTokenStrategy(
       this._strategy,
-      refreshToken,
-      scope,
+      req.refreshToken,
+      req.scope,
     );
 
     const profile = await executeFetchUserProfileStrategy(