Responding with 401 status, not 500 for signature verification failures

[seratch]

Description

Currently, ExpressReceiver responds an internal server error (status code: 500) when detecting a request with an invalid signature. The pattern is not recoverable even the client retries the request later. So, Bolt apps should respond with 40x status. My suggestion is status code 401 but I'm fine with other status codes if others have strong opinions on it.

Code Change Proposal

To address this, I'm going to change these lines of code to:

      try {
        await verifyRequestSignature(signingSecret, stringBody, signature, ts);
      } catch (e) {
        logger.warn(`Request verification failed (code: ${e.code}, message: ${e.message}`);
        return res.status(401).send();
      }

Versioning

We may need to have this change as a breaking change of behavior in Semantic Versioning. So, we should bump a major version for the release including this change.

An option to allow users to return other status code

I don't think anyone will be unhappy with the change. But, if many people were to really want to keep the current behavior, we may consider having an option to customize the http status code to respond.

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

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

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

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

---

Reproducible in:

package version: all versions
node version: any
OS version(s): any

[seratch]

I've come up with pull request #362
The PR changes the behavior to respond with 401 for signature verification errors while it returns 400 for request body parser errors.

[aoberoi]

I agree that using a 401 status code response is an improvement over using 500!

In the strictest sense, this could be seen as a semver major change, but I don't think we should view it that way. It will not break compatibility with existing code, but it would change the user experience in Slack if for some reason the application is using the wrong verification secret. It would also change the behavior if a malicious actor was sending requests with bad secrets. In both of these cases, I think the behavior is an improvement with no backwards incompatible side effects, and that's why I think it should not be landed with a semver major version change.

As for an option to choose the old behavior, I don't think that is necessary now, but would be open to adding it if any of our users actually prefer the old behavior.