Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1181 Add port property to installerOptions in the HTTPReceiver #1184

Merged
merged 1 commit into from
Oct 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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