Add callback to allow users to override state

This is a slightly different implementation. It allows the user to pass in a reference to a callback so that the state
can be set. It was a suggestion from @Rugvip on a discussion we had offline.

The callback is an async function that returns a Promise<string>

Note: due to the way the OAuthAdapter works, this callback must include an env + nonce. Without them, your oauth request will fail.

Signed-off-by: Nicolas Arnold <nic@roadie.io>

plugins/auth-backend/api-report.md

@@ -290,12 +290,10 @@ export type GithubOAuthResult = {
 // @public (undocumented)
 export type GithubProviderOptions = {
   authHandler?: AuthHandler<GithubOAuthResult>;
-  extraState?: {
-    [key: string]: string;
-  };
   signIn?: {
     resolver?: SignInResolver<GithubOAuthResult>;
   };
+  stateHandler?: StateHandler;
 };
 
 // Warning: (ae-missing-release-tag) "GitlabProviderOptions" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
@@ -491,7 +489,7 @@ export type OAuthStartRequest = express.Request<{}> & {
 export type OAuthState = {
   nonce: string;
   env: string;
-  [key: string]: string;
+  origin?: string;
 };
 
 // Warning: (ae-missing-release-tag) "oktaEmailSignInResolver" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
@@ -585,6 +583,7 @@ export type WebMessageResponse =
 // src/providers/atlassian/provider.d.ts:37:5 - (ae-forgotten-export) The symbol "AuthHandler" needs to be exported by the entry point index.d.ts
 // src/providers/atlassian/provider.d.ts:42:9 - (ae-forgotten-export) The symbol "SignInResolver" needs to be exported by the entry point index.d.ts
 // src/providers/aws-alb/provider.d.ts:77:5 - (ae-forgotten-export) The symbol "AwsAlbResult" needs to be exported by the entry point index.d.ts
+// src/providers/github/provider.d.ts:65:5 - (ae-forgotten-export) The symbol "StateHandler" needs to be exported by the entry point index.d.ts
 // src/providers/types.d.ts:99:5 - (ae-forgotten-export) The symbol "AuthProviderConfig" needs to be exported by the entry point index.d.ts
 // src/providers/types.d.ts:121:8 - (tsdoc-missing-deprecation-message) The @deprecated block must include a deprecation message, e.g. describing the recommended alternative
 ```

plugins/auth-backend/src/lib/oauth/helpers.test.ts

@@ -20,31 +20,16 @@ import { verifyNonce, encodeState, readState } from './helpers';
 describe('OAuthProvider Utils', () => {
   describe('encodeState', () => {
     it('should serialized values', () => {
-      const state = {
-        nonce: '123',
-        env: 'development',
-      };
-
-      const encoded = encodeState(state);
-      expect(encoded).toBe(
-        Buffer.from('nonce=123&env=development').toString('hex'),
-      );
-
-      expect(readState(encoded)).toEqual(state);
-    });
-
-    it('should serialized values with extra values', () => {
       const state = {
         nonce: '123',
         env: 'development',
         origin: 'https://example.com',
-        redirect_url: 'https://someurl.com/foo/bar',
       };
 
       const encoded = encodeState(state);
       expect(encoded).toBe(
         Buffer.from(
-          'nonce=123&env=development&origin=https%3A%2F%2Fexample.com&redirect_url=https%3A%2F%2Fsomeurl.com%2Ffoo%2Fbar',
+          'nonce=123&env=development&origin=https%3A%2F%2Fexample.com',
         ).toString('hex'),
       );
 
@@ -52,12 +37,9 @@ describe('OAuthProvider Utils', () => {
     });
 
     it('should not include undefined values', () => {
-      const state = {
-        nonce: '123',
-        env: 'development',
-      };
-      // @ts-ignore
-      const encoded = encodeState({ test: undefined, ...state });
+      const state = { nonce: '123', env: 'development', origin: undefined };
+
+      const encoded = encodeState(state);
       expect(encoded).toBe(
         Buffer.from('nonce=123&env=development').toString('hex'),
       );

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

@@ -77,7 +77,7 @@ export type OAuthState = {
    */
   nonce: string;
   env: string;
-  [key: string]: string;
+  origin?: string;
 };
 
 export type OAuthStartRequest = express.Request<{}> & {

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

@@ -25,6 +25,7 @@ import {
 } from './provider';
 import * as helpers from '../../lib/passport/PassportStrategyHelper';
 import { makeProfileInfo } from '../../lib/passport/PassportStrategyHelper';
+import { OAuthStartRequest, encodeState } from '../../lib/oauth';
 
 const mockFrameHandler = jest.spyOn(
   helpers,
@@ -56,6 +57,9 @@ describe('GithubAuthProvider', () => {
     authHandler: async ({ fullProfile }) => ({
       profile: makeProfileInfo(fullProfile),
     }),
+    stateHandler: async (req: OAuthStartRequest) => {
+      return encodeState(req.state);
+    },
     callbackUrl: 'mock',
     clientId: 'mock',
     clientSecret: 'mock',

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

@@ -31,6 +31,7 @@ import {
   AuthProviderFactory,
   AuthHandler,
   SignInResolver,
+  StateHandler,
 } from '../types';
 import {
   OAuthAdapter,
@@ -64,9 +65,9 @@ export type GithubAuthProviderOptions = OAuthProviderOptions & {
   tokenUrl?: string;
   userProfileUrl?: string;
   authorizationUrl?: string;
-  extraState?: { [key: string]: string };
   signInResolver?: SignInResolver<GithubOAuthResult>;
   authHandler: AuthHandler<GithubOAuthResult>;
+  stateHandler: StateHandler;
   tokenIssuer: TokenIssuer;
   catalogIdentityClient: CatalogIdentityClient;
   logger: Logger;
@@ -79,14 +80,14 @@ export class GithubAuthProvider implements OAuthHandlers {
   private readonly tokenIssuer: TokenIssuer;
   private readonly catalogIdentityClient: CatalogIdentityClient;
   private readonly logger: Logger;
-  private readonly extraState: { [key: string]: string };
+  private readonly stateHandler: StateHandler;
 
   constructor(options: GithubAuthProviderOptions) {
     this.signInResolver = options.signInResolver;
     this.authHandler = options.authHandler;
+    this.stateHandler = options.stateHandler;
     this.tokenIssuer = options.tokenIssuer;
     this.catalogIdentityClient = options.catalogIdentityClient;
-    this.extraState = options.extraState || {};
     this.logger = options.logger;
     this._strategy = new GithubStrategy(
       {
@@ -112,7 +113,7 @@ export class GithubAuthProvider implements OAuthHandlers {
   async start(req: OAuthStartRequest): Promise<RedirectInfo> {
     return await executeRedirectStrategy(req, this._strategy, {
       scope: req.scope,
-      state: encodeState({ ...this.extraState, ...req.state }),
+      state: await this.stateHandler(req.state),
     });
   }
 
@@ -203,11 +204,6 @@ export type GithubProviderOptions = {
    */
   authHandler?: AuthHandler<GithubOAuthResult>;
 
-  /**
-   * The extra state you would like to pass into the OAuth state
-   */
-  extraState?: { [key: string]: string };
-
   /**
    * Configure sign-in for this provider, without it the provider can not be used to sign users in.
    */
@@ -217,6 +213,11 @@ export type GithubProviderOptions = {
      */
     resolver?: SignInResolver<GithubOAuthResult>;
   };
+
+  /**
+   * The state handler that sets the uri query param 'state'
+   */
+  stateHandler?: StateHandler;
 };
 
 export const createGithubProvider = (
@@ -271,7 +272,11 @@ export const createGithubProvider = (
           logger,
         });
 
-      const extraState = options?.extraState ? options.extraState : undefined;
+      const stateHandler: StateHandler = options?.stateHandler
+        ? options.stateHandler
+        : async (req: OAuthStartRequest) => {
+            return encodeState(req.state);
+          };
 
       const provider = new GithubAuthProvider({
         clientId,
@@ -284,7 +289,7 @@ export const createGithubProvider = (
         authHandler,
         tokenIssuer,
         catalogIdentityClient,
-        extraState,
+        stateHandler,
         logger,
       });
 

plugins/auth-backend/src/providers/types.ts

@@ -223,3 +223,5 @@ export type AuthHandlerResult = { profile: ProfileInfo };
 export type AuthHandler<AuthResult> = (
   input: AuthResult,
 ) => Promise<AuthHandlerResult>;
+
+export type StateHandler = (input: any) => Promise<string>;