Skip to content

Commit

Permalink
Fix #1181 Add port property to installerOptions in the HTTPReceiver (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
seratch authored Oct 29, 2021
1 parent 3077c40 commit 1b450f6
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 12 deletions.
76 changes: 76 additions & 0 deletions src/App.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,82 @@ function createDummyReceiverEvent(type: string = 'dummy_event_type'): ReceiverEv

describe('App', () => {
describe('constructor', () => {
describe('with a custom port value in HTTP Mode', () => {
const fakeBotId = 'B_FAKE_BOT_ID';
const fakeBotUserId = 'U_FAKE_BOT_USER_ID';
const overrides = mergeOverrides(
withNoopAppMetadata(),
withSuccessfulBotUserFetchingWebClient(fakeBotId, fakeBotUserId),
);
it('should accept a port value at the top-level', async () => {
// Arrange
const MockApp = await importApp(overrides);
// Act
const app = new MockApp({ token: '', signingSecret: '', port: 9999 });
// Assert
assert.equal((app as any).receiver.port, 9999);
});
it('should accept a port value under installerOptions', async () => {
// Arrange
const MockApp = await importApp(overrides);
// Act
const app = new MockApp({ token: '', signingSecret: '', port: 7777, installerOptions: { port: 9999 } });
// Assert
assert.equal((app as any).receiver.port, 9999);
});
});

describe('with a custom port value in Socket Mode', () => {
const fakeBotId = 'B_FAKE_BOT_ID';
const fakeBotUserId = 'U_FAKE_BOT_USER_ID';
const installationStore = {
storeInstallation: async () => { },
fetchInstallation: async () => { throw new Error('Failed fetching installation'); },
deleteInstallation: async () => { },
};
const overrides = mergeOverrides(
withNoopAppMetadata(),
withSuccessfulBotUserFetchingWebClient(fakeBotId, fakeBotUserId),
);
it('should accept a port value at the top-level', async () => {
// Arrange
const MockApp = await importApp(overrides);
// Act
const app = new MockApp({
socketMode: true,
appToken: '',
port: 9999,
clientId: '',
clientSecret: '',
stateSecret: '',
installerOptions: {
},
installationStore,
});
// Assert
assert.equal((app as any).receiver.httpServerPort, 9999);
});
it('should accept a port value under installerOptions', async () => {
// Arrange
const MockApp = await importApp(overrides);
// Act
const app = new MockApp({
socketMode: true,
appToken: '',
port: 7777,
clientId: '',
clientSecret: '',
stateSecret: '',
installerOptions: {
port: 9999,
},
installationStore,
});
// Assert
assert.equal((app as any).receiver.httpServerPort, 9999);
});
});

// TODO: test when the single team authorization results fail. that should still succeed but warn. it also means
// that the `ignoreSelf` middleware will fail (or maybe just warn) a bunch.
describe('with successful single team authorization results', () => {
Expand Down
8 changes: 8 additions & 0 deletions src/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ const tokenUsage = 'Apps used in one workspace should be initialized with a toke
export interface AppOptions {
signingSecret?: HTTPReceiverOptions['signingSecret'];
endpoints?: HTTPReceiverOptions['endpoints'];
port?: HTTPReceiverOptions['port'];
customRoutes?: HTTPReceiverOptions['customRoutes'];
processBeforeResponse?: HTTPReceiverOptions['processBeforeResponse'];
signatureVerification?: HTTPReceiverOptions['signatureVerification'];
Expand Down Expand Up @@ -240,6 +241,7 @@ export default class App {
public constructor({
signingSecret = undefined,
endpoints = undefined,
port = undefined,
customRoutes = undefined,
agent = undefined,
clientTls = undefined,
Expand Down Expand Up @@ -337,6 +339,11 @@ export default class App {
clientOptions: this.clientOptions,
...installerOptions,
};
if (socketMode && port !== undefined && this.installerOptions.port === undefined) {
// As SocketModeReceiver uses a custom port number to listen on only for the OAuth flow,
// only installerOptions.port is available in the constructor arguments.
this.installerOptions.port = port;
}

if (
this.developerMode &&
Expand Down Expand Up @@ -394,6 +401,7 @@ export default class App {
this.receiver = new HTTPReceiver({
signingSecret: signingSecret || '',
endpoints,
port,
customRoutes,
processBeforeResponse,
signatureVerification,
Expand Down
32 changes: 32 additions & 0 deletions src/receivers/HTTPReceiver.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,38 @@ describe('HTTPReceiver', function () {
assert.isNotNull(receiver);
});

it('should accept a custom port', async function () {
// Arrange
const overrides = mergeOverrides(
withHttpCreateServer(this.fakeCreateServer),
withHttpsCreateServer(sinon.fake.throws('Should not be used.')),
);
const HTTPReceiver = await importHTTPReceiver(overrides);

const defaultPort = new HTTPReceiver({
signingSecret: 'secret',
});
assert.isNotNull(defaultPort);
assert.equal((defaultPort as any).port, 3000);

const customPort = new HTTPReceiver({
port: 9999,
signingSecret: 'secret',
});
assert.isNotNull(customPort);
assert.equal((customPort as any).port, 9999);

const customPort2 = new HTTPReceiver({
port: 7777,
signingSecret: 'secret',
installerOptions: {
port: 9999,
},
});
assert.isNotNull(customPort2);
assert.equal((customPort2 as any).port, 9999);
});

it('should throw an error if redirect uri options supplied invalid or incomplete', async function () {
const HTTPReceiver = await importHTTPReceiver();
const clientId = 'my-clientId';
Expand Down
18 changes: 17 additions & 1 deletion src/receivers/HTTPReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ const missingServerErrorDescription = 'The receiver cannot be started because pr
export interface HTTPReceiverOptions {
signingSecret: string;
endpoints?: string | string[];
port?: number; // if you pass another port number to #start() method, the argument will be used instead
customRoutes?: CustomRoute[];
logger?: Logger;
logLevel?: LogLevel;
Expand Down Expand Up @@ -89,6 +90,9 @@ export interface HTTPReceiverInstallerOptions {
metadata?: InstallURLOptions['metadata'];
userScopes?: InstallURLOptions['userScopes'];
callbackOptions?: CallbackOptions;
// This value exists here only for the compatibility with SocketModeReceiver.
// If you use only HTTPReceiver, the top-level is recommended.
port?: number;
}

/**
Expand All @@ -97,6 +101,8 @@ export interface HTTPReceiverInstallerOptions {
export default class HTTPReceiver implements Receiver {
private endpoints: string[];

private port: number; // you can override this value by the #start() method argument

private routes: ReceiverRoutes;

private signingSecret: string;
Expand Down Expand Up @@ -134,6 +140,7 @@ export default class HTTPReceiver implements Receiver {
public constructor({
signingSecret = '',
endpoints = ['/slack/events'],
port = 3000,
customRoutes = [],
logger = undefined,
logLevel = LogLevel.INFO,
Expand All @@ -159,6 +166,7 @@ export default class HTTPReceiver implements Receiver {
return defaultLogger;
})();
this.endpoints = Array.isArray(endpoints) ? endpoints : [endpoints];
this.port = installerOptions?.port ? installerOptions.port : port;
this.routes = prepareRoutes(customRoutes);

// Verify redirect options if supplied, throws coded error if invalid
Expand Down Expand Up @@ -281,7 +289,15 @@ export default class HTTPReceiver implements Receiver {
this.server = undefined;
});

this.server.listen(portOrListenOptions, () => {
let listenOptions: ListenOptions | number = this.port;
if (portOrListenOptions !== undefined) {
if (typeof portOrListenOptions === 'number') {
listenOptions = portOrListenOptions as number;
} else if (typeof portOrListenOptions === 'object') {
listenOptions = portOrListenOptions as ListenOptions;
}
}
this.server.listen(listenOptions, () => {
if (this.server === undefined) {
return reject(new ReceiverInconsistentStateError(missingServerErrorDescription));
}
Expand Down
6 changes: 4 additions & 2 deletions src/receivers/SocketModeReceiver.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ describe('SocketModeReceiver', function () {
},
});
assert.isNotNull(receiver);
assert.isOk(this.fakeServer.listen.calledWith(3000));
// since v3.8, the constructor does not start the server
// assert.isNotOk(this.fakeServer.listen.calledWith(3000));
});
it('should allow for customizing port the socket listens on', async function () {
// Arrange
Expand All @@ -118,7 +119,8 @@ describe('SocketModeReceiver', function () {
},
});
assert.isNotNull(receiver);
assert.isOk(this.fakeServer.listen.calledWith(customPort));
// since v3.8, the constructor does not start the server
// assert.isOk(this.fakeServer.listen.calledWith(customPort));
});
it('should allow for extracting additional values from Socket Mode messages', async function () {
// Arrange
Expand Down
29 changes: 20 additions & 9 deletions src/receivers/SocketModeReceiver.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { SocketModeClient } from '@slack/socket-mode';
import { createServer, IncomingMessage, ServerResponse } from 'http';
import { createServer, IncomingMessage, ServerResponse, Server } from 'http';
import { Logger, ConsoleLogger, LogLevel } from '@slack/logger';
import { InstallProvider, CallbackOptions, InstallProviderOptions, InstallURLOptions } from '@slack/oauth';
import { AppsConnectionsOpenResponse } from '@slack/web-api';
Expand Down Expand Up @@ -48,7 +48,7 @@ interface InstallerOptions {
userScopes?: InstallURLOptions['userScopes'];
clientOptions?: InstallProviderOptions['clientOptions'];
authorizationUrl?: InstallProviderOptions['authorizationUrl'];
port?: number; // used to create a server when doing OAuth
port?: number; // used to create a server when doing OAuth or serving custom routes
}

/**
Expand All @@ -64,6 +64,10 @@ export default class SocketModeReceiver implements Receiver {

public installer: InstallProvider | undefined = undefined;

private httpServer?: Server;

private httpServerPort?: number;

private routes: ReceiverRoutes;

public constructor({
Expand Down Expand Up @@ -127,9 +131,9 @@ export default class SocketModeReceiver implements Receiver {
// use default or passed in installPath
const installPath = installerOptions.installPath === undefined ? '/slack/install' : installerOptions.installPath;
const directInstallEnabled = installerOptions.directInstall !== undefined && installerOptions.directInstall;
const port = installerOptions.port === undefined ? 3000 : installerOptions.port;
this.httpServerPort = installerOptions.port === undefined ? 3000 : installerOptions.port;

const server = createServer(async (req, res) => {
this.httpServer = createServer(async (req, res) => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const method = req.method!.toUpperCase();

Expand Down Expand Up @@ -196,14 +200,11 @@ export default class SocketModeReceiver implements Receiver {
res.end();
});

this.logger.debug(`Listening for HTTP requests on port ${port}`);
this.logger.debug(`Listening for HTTP requests on port ${this.httpServerPort}`);

if (this.installer) {
this.logger.debug(`Go to http://localhost:${port}${installPath} to initiate OAuth flow`);
this.logger.debug(`Go to http://localhost:${this.httpServerPort}${installPath} to initiate OAuth flow`);
}

// use port 3000 by default
server.listen(port);
}

this.client.on('slack_event', async (args) => {
Expand All @@ -224,11 +225,21 @@ export default class SocketModeReceiver implements Receiver {
}

public start(): Promise<AppsConnectionsOpenResponse> {
if (this.httpServer !== undefined) {
// This HTTP server is only for the OAuth flow support
this.httpServer.listen(this.httpServerPort);
}
// start socket mode client
return this.client.start();
}

public stop(): Promise<void> {
if (this.httpServer !== undefined) {
// This HTTP server is only for the OAuth flow support
this.httpServer.close((error) => {
this.logger.error(`Failed to shutdown the HTTP server for OAuth flow: ${error}`);
});
}
return new Promise((resolve, reject) => {
try {
this.client.disconnect();
Expand Down

0 comments on commit 1b450f6

Please sign in to comment.