Skip to content

Commit

Permalink
Fix #648 Option to disable signature verification (#1088)
Browse files Browse the repository at this point in the history
* Fix #648 Option to disable signature verification
* Rename requestVerification to signatureVerification
* Fix lint errors
  • Loading branch information
seratch committed Aug 28, 2021
1 parent 2f463ce commit da3baca
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 6 deletions.
7 changes: 5 additions & 2 deletions src/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export interface AppOptions {
signingSecret?: HTTPReceiverOptions['signingSecret'];
endpoints?: HTTPReceiverOptions['endpoints'];
processBeforeResponse?: HTTPReceiverOptions['processBeforeResponse'];
signatureVerification?: HTTPReceiverOptions['signatureVerification'];
clientId?: HTTPReceiverOptions['clientId'];
clientSecret?: HTTPReceiverOptions['clientSecret'];
stateSecret?: HTTPReceiverOptions['stateSecret']; // required when using default stateStore
Expand Down Expand Up @@ -221,6 +222,7 @@ export default class App {
ignoreSelf = true,
clientOptions = undefined,
processBeforeResponse = false,
signatureVerification = true,
clientId = undefined,
clientSecret = undefined,
stateSecret = undefined,
Expand Down Expand Up @@ -337,7 +339,7 @@ export default class App {
logLevel: this.logLevel,
installerOptions: this.installerOptions,
});
} else if (signingSecret === undefined) {
} else if (signatureVerification && signingSecret === undefined) {
// No custom receiver
throw new AppInitializationError(
'Signing secret not found, so could not initialize the default receiver. Set a signing secret or use a ' +
Expand All @@ -347,9 +349,10 @@ export default class App {
this.logger.debug('Initializing HTTPReceiver');
// Create default HTTPReceiver
this.receiver = new HTTPReceiver({
signingSecret,
signingSecret: signingSecret || '',
endpoints,
processBeforeResponse,
signatureVerification,
clientId,
clientSecret,
stateSecret,
Expand Down
45 changes: 42 additions & 3 deletions src/receivers/ExpressReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export interface ExpressReceiverOptions {
| {
[endpointType: string]: string;
};
signatureVerification?: boolean;
processBeforeResponse?: boolean;
clientId?: string;
clientSecret?: string;
Expand Down Expand Up @@ -123,6 +124,8 @@ export default class ExpressReceiver implements Receiver {

private processBeforeResponse: boolean;

private signatureVerification: boolean;

public router: IRouter;

public installer: InstallProvider | undefined = undefined;
Expand All @@ -133,6 +136,7 @@ export default class ExpressReceiver implements Receiver {
logLevel = LogLevel.INFO,
endpoints = { events: '/slack/events' },
processBeforeResponse = false,
signatureVerification = true,
clientId = undefined,
clientSecret = undefined,
stateSecret = undefined,
Expand All @@ -151,8 +155,12 @@ export default class ExpressReceiver implements Receiver {
this.logger.setLevel(logLevel);
}

this.signatureVerification = signatureVerification;
const bodyParser = this.signatureVerification ?
buildVerificationBodyParserMiddleware(this.logger, signingSecret) :
buildBodyParserMiddleware(this.logger);
const expressMiddleware: RequestHandler[] = [
verifySignatureAndParseRawBody(this.logger, signingSecret),
bodyParser,
respondToSslCheck,
respondToUrlVerification,
this.requestHandler.bind(this),
Expand Down Expand Up @@ -375,12 +383,19 @@ export default class ExpressReceiver implements Receiver {
}
}

export function verifySignatureAndParseRawBody(
logger: Logger,
signingSecret: string | (() => PromiseLike<string>),
): RequestHandler {
return buildVerificationBodyParserMiddleware(logger, signingSecret);
}

/**
* This request handler has two responsibilities:
* - Verify the request signature
* - Parse request.body and assign the successfully parsed object to it.
*/
export function verifySignatureAndParseRawBody(
function buildVerificationBodyParserMiddleware(
logger: Logger,
signingSecret: string | (() => PromiseLike<string>),
): RequestHandler {
Expand Down Expand Up @@ -468,7 +483,7 @@ function verifyRequestSignature(
* - Verify the request signature
* - Parse request.body and assign the successfully parsed object to it.
*/
function verifySignatureAndParseBody(
export function verifySignatureAndParseBody(
signingSecret: string,
body: string,
headers: Record<string, any>,
Expand All @@ -485,6 +500,30 @@ function verifySignatureAndParseBody(
return parseRequestBody(body, contentType);
}

function buildBodyParserMiddleware(logger: Logger): RequestHandler {
return async (req, res, next) => {
let stringBody: string;
// On some environments like GCP (Google Cloud Platform),
// req.body can be pre-parsed and be passed as req.rawBody here
const preparsedRawBody: any = (req as any).rawBody;
if (preparsedRawBody !== undefined) {
stringBody = preparsedRawBody.toString();
} else {
stringBody = (await rawBody(req)).toString();
}
try {
const { 'content-type': contentType } = req.headers;
req.body = parseRequestBody(stringBody, contentType);
} catch (error) {
if (error) {
logError(logger, 'Parsing request body failed', error);
return res.status(400).send();
}
}
return next();
};
}

function parseRequestBody(stringBody: string, contentType: string | undefined): any {
if (contentType === 'application/x-www-form-urlencoded') {
const parsedBody = querystring.parse(stringBody);
Expand Down
14 changes: 13 additions & 1 deletion src/receivers/HTTPReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export interface HTTPReceiverOptions {
logger?: Logger;
logLevel?: LogLevel;
processBeforeResponse?: boolean;
signatureVerification?: boolean;
clientId?: string;
clientSecret?: string;
stateSecret?: InstallProviderOptions['stateSecret']; // required when using default stateStore
Expand Down Expand Up @@ -92,6 +93,8 @@ export default class HTTPReceiver implements Receiver {

private processBeforeResponse: boolean;

private signatureVerification: boolean;

private app?: App;

public requestListener: RequestListener;
Expand Down Expand Up @@ -120,6 +123,7 @@ export default class HTTPReceiver implements Receiver {
logger = undefined,
logLevel = LogLevel.INFO,
processBeforeResponse = false,
signatureVerification = true,
clientId = undefined,
clientSecret = undefined,
stateSecret = undefined,
Expand All @@ -130,6 +134,7 @@ export default class HTTPReceiver implements Receiver {
// Initialize instance variables, substituting defaults for each value
this.signingSecret = signingSecret;
this.processBeforeResponse = processBeforeResponse;
this.signatureVerification = signatureVerification;
this.logger = logger ??
(() => {
const defaultLogger = new ConsoleLogger();
Expand Down Expand Up @@ -317,7 +322,14 @@ export default class HTTPReceiver implements Receiver {

// Verify authenticity
try {
bufferedReq = await verifySlackAuthenticity({ signingSecret: this.signingSecret }, req);
bufferedReq = await verifySlackAuthenticity(
{
// If enabled: false, this method returns bufferredReq without verification
enabled: this.signatureVerification,
signingSecret: this.signingSecret,
},
req,
);
} catch (err) {
const e = err as any;
this.logger.warn(`Request verification failed: ${e.message}`);
Expand Down
6 changes: 6 additions & 0 deletions src/receivers/verify-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type { IncomingMessage, ServerResponse } from 'http';
const verifyErrorPrefix = 'Failed to verify authenticity';

export interface VerifyOptions {
enabled?: boolean;
signingSecret: string;
nowMs?: () => number;
logger?: Logger;
Expand Down Expand Up @@ -54,6 +55,11 @@ export async function verify(
// Consume the readable stream (or use the previously consumed readable stream)
const bufferedReq = await bufferIncomingMessage(req);

if (options.enabled !== undefined && !options.enabled) {
// As the validation is disabled, immediately return the bufferred reuest
return bufferedReq;
}

// Find the relevant request headers
const signature = getHeader(req, 'x-slack-signature');
const requestTimestampSec = Number(getHeader(req, 'x-slack-request-timestamp'));
Expand Down

0 comments on commit da3baca

Please sign in to comment.