Add redirect uri opts validation + stateVerification tests

src/App.spec.ts

@@ -255,20 +255,7 @@ describe('App', () => {
         assert(fakeConversationContext.firstCall.calledWith(dummyConvoStore));
       });
     });
-    describe('with custom redirect supplied', () => {
-      it('should fail when missing redirectUri', async () => {
-        // Arrange
-        const MockApp = await importApp();
-
-        // Act
-        try {
-          new MockApp({ token: '', signingSecret: '', installerOptions: { redirectUriPath: '/redirect' } }); // eslint-disable-line no-new
-          assert.fail();
-        } catch (error: any) {
-          // Assert
-          assert.propertyVal(error, 'code', ErrorCode.AppInitializationError);
-        }
-      });
+    describe('with custom redirectUri supplied', () => {
       it('should fail when missing installerOptions', async () => {
         // Arrange
         const MockApp = await importApp();

src/App.ts

@@ -254,6 +254,7 @@ export default class App {
       this.logLevel = logLevel ?? LogLevel.INFO;
     }
 
+    // Set up logger
     if (typeof logger === 'undefined') {
       // Initialize with the default logger
       const consoleLogger = new ConsoleLogger();
@@ -267,6 +268,7 @@ export default class App {
     }
     this.errorHandler = defaultErrorHandler(this.logger);
 
+    // Set up client options
     this.clientOptions = clientOptions !== undefined ? clientOptions : {};
     if (agent !== undefined && this.clientOptions.agent === undefined) {
       this.clientOptions.agent = agent;
@@ -278,7 +280,7 @@ export default class App {
       // only logLevel is passed
       this.clientOptions.logLevel = logLevel;
     } else {
-      // Since v3.4, WebClient starts sharing loggger with App
+      // Since v3.4, WebClient starts sharing logger with App
       this.clientOptions.logger = this.logger;
     }
     // The public WebClient instance (app.client)
@@ -309,8 +311,8 @@ export default class App {
       this.developerMode &&
       this.installerOptions &&
       (typeof this.installerOptions.callbackOptions === 'undefined' ||
-        (typeof this.installerOptions.callbackOptions !== 'undefined' &&
-          typeof this.installerOptions.callbackOptions.failure === 'undefined'))
+      (typeof this.installerOptions.callbackOptions !== 'undefined' &&
+      typeof this.installerOptions.callbackOptions.failure === 'undefined'))
     ) {
       // add a custom failure callback for Developer Mode in case they are using OAuth
       this.logger.debug('adding Developer Mode custom OAuth failure handler');
@@ -323,11 +325,9 @@ export default class App {
       };
     }
 
-    // verify any redirect options supplied
-    this.verifyRedirectOpts(redirectUri);
-
-    // Check for required arguments of HTTPReceiver
+    // Initialize receiver
     if (receiver !== undefined) {
+      // Custom receiver
       if (this.socketMode) {
         throw new AppInitializationError('receiver cannot be passed when socketMode is set to true');
       }
@@ -436,26 +436,6 @@ export default class App {
     this.receiver.init(this);
   }
 
-  /**
-   * Verifies redirect uri and redirect uri path exist if either supplied
-   */
-  private verifyRedirectOpts(redirectUri: HTTPReceiverOptions['redirectUri']) {
-    // if either redirectUri or redirectUriPath are supplied
-    // both must be supplied
-    const { installerOptions } = this;
-    if (
-      (redirectUri && !installerOptions) ||
-        (redirectUri && installerOptions && !installerOptions.redirectUriPath) ||
-          (!redirectUri && installerOptions && installerOptions.redirectUriPath)
-    ) {
-      throw new AppInitializationError(
-        'To set a custom install redirect path, you must provide both redirectUri' +
-        ' and installerOptions.redirectUriPath during app initialization.' +
-        ' These should be consistent, e.g. https://example.com/redirect and /redirect',
-      );
-    }
-  }
-
   /**
    * Register a new middleware, processed in the order registered.
    *

src/receivers/ExpressReceiver.spec.ts

@@ -7,7 +7,7 @@ import { Application, IRouter, Request, Response } from 'express';
 import { Readable } from 'stream';
 import { EventEmitter } from 'events';
 import { Override, mergeOverrides } from '../test-helpers';
-import { ErrorCode, CodedError, ReceiverInconsistentStateError } from '../errors';
+import { ErrorCode, CodedError, ReceiverInconsistentStateError, AppInitializationError } from '../errors';
 
 import ExpressReceiver, {
   respondToSslCheck,
@@ -137,6 +137,59 @@ describe('ExpressReceiver', function () {
       assert((router.get as any).called);
       assert((router.post as any).calledOnce);
     });
+    it('should throw an error if redirect uri options supplied invalid or incomplete', async function () {
+      const clientId = 'my-clientId';
+      const clientSecret = 'my-clientSecret';
+      const signingSecret = 'secret';
+      const stateSecret = 'my-stateSecret';
+      const scopes = ['chat:write'];
+      const redirectUri = 'http://example.com/heyo';
+      const installerOptions = {
+        redirectUriPath: '/heyo',
+      };
+      // correct format with redirect options supplied
+      const receiver = new ExpressReceiver({
+        clientId,
+        clientSecret,
+        signingSecret,
+        stateSecret,
+        scopes,
+        redirectUri,
+        installerOptions,
+      });
+      assert.isNotNull(receiver);
+      // missing redirectUriPath
+      assert.throws(() => new ExpressReceiver({
+        clientId,
+        clientSecret,
+        signingSecret,
+        stateSecret,
+        scopes,
+        redirectUri,
+      }), AppInitializationError);
+      // inconsistent redirectUriPath
+      assert.throws(() => new ExpressReceiver({
+        clientId: 'my-clientId',
+        clientSecret,
+        signingSecret,
+        stateSecret,
+        scopes,
+        redirectUri,
+        installerOptions: {
+          redirectUriPath: '/hiya',
+        },
+      }), AppInitializationError);
+      // inconsistent redirectUri
+      assert.throws(() => new ExpressReceiver({
+        clientId: 'my-clientId',
+        clientSecret,
+        signingSecret,
+        stateSecret,
+        scopes,
+        redirectUri: 'http://example.com/hiya',
+        installerOptions,
+      }), AppInitializationError);
+    });
   });
 
   describe('#start()', function () {

src/receivers/ExpressReceiver.ts

@@ -19,6 +19,7 @@ import {
 } from '../errors';
 import { AnyMiddlewareArgs, Receiver, ReceiverEvent } from '../types';
 import defaultRenderHtmlForInstallPath from './render-html-for-install-path';
+import { verifyRedirectOpts } from './verify-redirect-opts';
 
 // Option keys for tls.createServer() and tls.createSecureContext(), exclusive of those for http.createServer()
 const httpsOptionKeys = [
@@ -133,6 +134,8 @@ export default class ExpressReceiver implements Receiver {
 
   public installer: InstallProvider | undefined = undefined;
 
+  public installerOptions?: InstallerOptions;
+
   public constructor({
     signingSecret = '',
     logger = undefined,
@@ -169,7 +172,6 @@ export default class ExpressReceiver implements Receiver {
       respondToUrlVerification,
       this.requestHandler.bind(this),
     ];
-
     this.processBeforeResponse = processBeforeResponse;
 
     const endpointList = typeof endpoints === 'string' ? [endpoints] : Object.values(endpoints);
@@ -178,6 +180,9 @@ export default class ExpressReceiver implements Receiver {
       this.router.post(endpoint, ...expressMiddleware);
     });
 
+    // Verify redirect options if supplied, throws coded error if invalid
+    verifyRedirectOpts({ redirectUri, redirectUriPath: installerOptions.redirectUriPath });
+
     if (
       clientId !== undefined &&
       clientSecret !== undefined &&
@@ -213,14 +218,14 @@ export default class ExpressReceiver implements Receiver {
         installerOptions.redirectUriPath;
       const { callbackOptions, stateVerification } = installerOptions;
       this.router.use(redirectUriPath, async (req, res) => {
-        if (stateVerification) {
+        if (stateVerification === false) {
+          // when stateVerification is disabled pass install options directly to handler
+          // since they won't be encoded in the state param of the generated url
           // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-          await this.installer!.handleCallback(req, res, callbackOptions);
+          await this.installer!.handleCallback(req, res, callbackOptions, installUrlOptions);
         } else {
-          // when stateVerification is disabled
-          // make installation options directly available to installation handler
           // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-          await this.installer!.handleCallback(req, res, callbackOptions, installUrlOptions);
+          await this.installer!.handleCallback(req, res, callbackOptions);
         }
       });