Node-slack-sdk: verifyRequestSignature breaks if request does not contain required headers

[ramsgoli]

Description

verifyRequestSignature breaks if the POST request does not contain any of the required headers

What type of issue is this? (place an x in one of the [ ])

• [x] bug
• [ ] enhancement (feature request)
• [ ] question
• [ ] documentation related
• [ ] testing related
• [ ] discussion

Requirements (place an x in each of the [ ])

• [x] I've read and understood the Contributing guidelines and have done my best effort to follow them.
• [x] I've read and agree to the Code of Conduct.
• [x] I've searched for any related issues and avoided creating a duplicate issue.

---

Bug Report

Filling out the following details about bugs will help us solve your issue sooner.

Packages:

Select all that apply:

• [ ] @slack/web-api
• [x] @slack/events-api
• [ ] @slack/interactive-messages
• [ ] @slack/rtm-api
• [ ] @slack/webhooks
• [ ] @slack/oauth
• [ ] I don't know

Reproducible in:

package version: latest

node version: v10.15.3

OS version(s): macos 10.15.5

Steps to reproduce:

1. Register the listener to the app (before any bodyparsers)

app.use('/api/slack/event', slackEvents.requestListener())

1. send a POST to /api/slack/event but don't include any of the required headers (x-slack-signature, x-slack-request-timestamp)

Expected result:

The request should be rejected

Actual result:

The request results in an internal server error

/Users/ram/myproj/node_modules/@slack/events-api/dist/http-handler.js:41
    var _b = requestSignature.split('='), version = _b[0], hash = _b[1];
                              ^
TypeError: Cannot read property 'split' of undefined
    at verifyRequestSignature (/Users/ram/myproj/node_modules/@slack/events-api/dist/http-handler.js:41:31)
    at /Users/ram/myproj/node_modules/@slack/events-api/dist/http-handler.js:168:17
    at process._tickCallback (internal/process/next_tick.js:68:7)

This happens because at https://github.com/slackapi/node-slack-sdk/blob/c379711831e7077762fcbec016788b9b0bee49f1/packages/events-api/src/http-handler.ts#L173,
we cast the headers as strings. IMO they should be treated as string | undefined and then handled appropriately in verifyRequestSignature. This would be nicer than the app throwing errors when requests are missing headers.

[mwbrooks]

Hey @ramsgoli thanks for reporting this issue and documenting all of the details we need to re-create (and resolve) it! 👊🏻

I agree that the app should handle missing headers gracefully, rather than throwing an exception. I'll line up this issue to be fixed soon (see the assigned Project board). If you happen to want to submit a PR, please go for it and we'll help to review and merge it.

[ramsgoli]

@mwbrooks Sounds good! Just put up a PR that addresses this ☝️