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

Add flag to AwsLambdaReceiver to enable/disable signature verification #2107

Merged
merged 5 commits into from
May 6, 2024
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
36 changes: 36 additions & 0 deletions src/receivers/AwsLambdaReceiver.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,42 @@ describe('AwsLambdaReceiver', function () {
);
assert.equal(response.statusCode, 401);
});

it('does not perform signature verification if signature verification flag is set to false', async () => {
const awsReceiver = new AwsLambdaReceiver({
signingSecret: '',
signatureVerification: false,
logger: noopLogger,
});
const handler = awsReceiver.toHandler();
const awsEvent = {
resource: '/slack/events',
path: '/slack/events',
httpMethod: 'POST',
headers: {
Accept: 'application/json,*/*',
'Content-Type': 'application/json',
Host: 'xxx.execute-api.ap-northeast-1.amazonaws.com',
'User-Agent': 'Slackbot 1.0 (+https://api.slack.com/robots)',
'X-Slack-Request-Timestamp': 'far back dude',
'X-Slack-Signature': 'very much invalid',
},
multiValueHeaders: {},
queryStringParameters: null,
multiValueQueryStringParameters: null,
pathParameters: null,
stageVariables: null,
requestContext: {},
body: urlVerificationBody,
isBase64Encoded: false,
};
const response = await handler(
awsEvent,
{},
(_error, _result) => {},
);
assert.equal(response.statusCode, 200);
});
});

// Composable overrides
Expand Down
50 changes: 44 additions & 6 deletions src/receivers/AwsLambdaReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,41 @@ export interface AwsResponse {
export type AwsHandler = (event: AwsEvent, context: any, callback: AwsCallback) => Promise<AwsResponse>;

export interface AwsLambdaReceiverOptions {
/**
* The Slack Signing secret to be used as an input to signature verification to ensure that requests are coming from
* Slack.
*
* If the {@link signatureVerification} flag is set to `false`, this can be set to any value as signature verification
* using this secret will not be performed.
*
* @see {@link https://api.slack.com/authentication/verifying-requests-from-slack#about} for details about signing secrets
*/
signingSecret: string;
/**
* The {@link Logger} for the receiver
*
* @default ConsoleLogger
*/
logger?: Logger;
/**
* The {@link LogLevel} to be used for the logger.
*
* @default LogLevel.INFO
*/
logLevel?: LogLevel;
/**
* Flag that determines whether Bolt should {@link https://api.slack.com/authentication/verifying-requests-from-slack|verify Slack's signature on incoming requests}.
*
* @default true
*/
signatureVerification?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're here, do you mind adding some JSDoc descriptors for what these properties mean? Would be a nice touch for developers to see that as they edit their code in their IDEs supporting autocomplete.

You don't have to do it for all the properties here, but at least for the ones you are changing, that would be much appreciated ❤️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a stab at adding JSDocs for everything. Let me know if any of them aren't quite correct!

/**
* Optional `function` that can extract custom properties from an incoming receiver event
* @param request The API Gateway event {@link AwsEvent}
* @returns An object containing custom properties
*
* @default noop
*/
customPropertiesExtractor?: (request: AwsEvent) => StringIndexed;
}

Expand All @@ -59,16 +91,20 @@ export default class AwsLambdaReceiver implements Receiver {

private logger: Logger;

private signatureVerification: boolean;

private customPropertiesExtractor: (request: AwsEvent) => StringIndexed;

public constructor({
signingSecret,
logger = undefined,
logLevel = LogLevel.INFO,
signatureVerification = true,
customPropertiesExtractor = (_) => ({}),
}: AwsLambdaReceiverOptions) {
// Initialize instance variables, substituting defaults for each value
this.signingSecret = signingSecret;
this.signatureVerification = signatureVerification;
this.logger = logger ??
(() => {
const defaultLogger = new ConsoleLogger();
Expand Down Expand Up @@ -130,12 +166,14 @@ export default class AwsLambdaReceiver implements Receiver {
return Promise.resolve({ statusCode: 200, body: '' });
}

// request signature verification
const signature = this.getHeaderValue(awsEvent.headers, 'X-Slack-Signature') as string;
const ts = Number(this.getHeaderValue(awsEvent.headers, 'X-Slack-Request-Timestamp'));
if (!this.isValidRequestSignature(this.signingSecret, rawBody, signature, ts)) {
this.logger.info(`Invalid request signature detected (X-Slack-Signature: ${signature}, X-Slack-Request-Timestamp: ${ts})`);
return Promise.resolve({ statusCode: 401, body: '' });
if (this.signatureVerification) {
// request signature verification
const signature = this.getHeaderValue(awsEvent.headers, 'X-Slack-Signature') as string;
const ts = Number(this.getHeaderValue(awsEvent.headers, 'X-Slack-Request-Timestamp'));
if (!this.isValidRequestSignature(this.signingSecret, rawBody, signature, ts)) {
this.logger.info(`Invalid request signature detected (X-Slack-Signature: ${signature}, X-Slack-Request-Timestamp: ${ts})`);
return Promise.resolve({ statusCode: 401, body: '' });
}
}

// url_verification (Events API)
Expand Down