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

HTTPReceiver does not immediately respond to an invalid signature request (no response instead) #1509

Closed
6 of 10 tasks
nirvparekh opened this issue Jul 1, 2022 · 15 comments · Fixed by #1528
Closed
6 of 10 tasks
Assignees
Labels
bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch
Milestone

Comments

@nirvparekh
Copy link

nirvparekh commented Jul 1, 2022

Description

My app was in review submission. Reviewers checked if signature verification failure has been handled or not.
In my Nodejs code App initialization, I didn't pass signatureVerification as by default TRUE.
They sent request by dummy signing secrete, and signature verification mismatched and in slack, it shows operation timeout error.
While I logged the output in server, it was showing
[warn] Slack request signing verification failed. Signature mismatch.

How to handle it and show the users proper error message?

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

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • example code 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.

Bug Report

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

Reproducible in:

package version:
@slack/bolt v3.11.0

node version:
v16.13.2

OS version(s):
Linux Ubuntu 18.4

Steps to reproduce:

  1. send request with fake signing secrete

Expected result:

The error should be caught in app.error()

Actual result:

The error is not being caught in app.error()

Attachments:

Logs, screenshots, screencast, sample project, funny gif, etc.

@seratch seratch added question M-T: User needs support to use the project needs info An issue that is claimed to be a bug and hasn't been reproduced, or otherwise needs more info labels Jul 1, 2022
@seratch
Copy link
Member

seratch commented Jul 1, 2022

Hi @nirvparekh, thanks for writing in. With the default settings, bolt-js automatically returns HTTP status 401 for you. The behavior should be accepted by the review team. If your app does not work this way, do you customize some settings or implement your own Receiver?

@nirvparekh
Copy link
Author

Hi @seratch,
No, I didn't customize settings and didn't implement any receiver.

I resubmitted, but still reviewers has rejected.
Could you help me out?
This is my initialization:

https://www.loom.com/share/7a3a6409a347419f8d15f3ac51ba7702

@seratch
Copy link
Member

seratch commented Jul 1, 2022

We are unable to help you for the review process but one thing that I suggest is to make sure if your app immediately responds with 401 HTTP status (the quickest way is to send an HTTP POST request using curl command). If not, perhaps other factors specific to yours may affect the behavior.

@nirvparekh
Copy link
Author

But how can I send 401 HTTP status, when my app doesn't know what happened? it's not being caught in app.error() as well.

@nirvparekh
Copy link
Author

nirvparekh commented Jul 1, 2022

Could you please provide example snippet how to handle signature mismatch and send 401 HTTP response using Bolt?
Or could you help how to read the HTTP request headers came from Slack?

@seratch
Copy link
Member

seratch commented Jul 2, 2022

@nirvparekh The following snippet may be helpful to you.

curl -XPOST https://{your public domain}/slack/events -d'this data does not matter' -H'x-slack-signature: xxx' -H'x-slack-request-timestamp: '`date '+%s'`  -v

@nirvparekh
Copy link
Author

@nirvparekh The following snippet may be helpful to you.

curl -XPOST https://{your public domain}/slack/events -d'this data does not matter' -H'x-slack-signature: xxx' -H'x-slack-request-timestamp: '`date '+%s'`  -v

@seratch thanks, but I want example for how to handle this request at NodeJS app.

@seratch
Copy link
Member

seratch commented Jul 3, 2022

@nirvparekh There is nothing special for it. If you use bolt-js, straight-forward configuration like this should work for you.

Therefore, I would suggest verifying whether your app correctly returns 401 HTTP status response using the above curl script. If it does not return an HTTP response, there is some factors preventing your bolt-js app from returning the valid response.

@nirvparekh
Copy link
Author

@seratch the app is logging as [warn] instead of [error], and that's why I'm not able to catch in app.error and so in Slack.

image

@seratch
Copy link
Member

seratch commented Jul 4, 2022

@nirvparekh As you can see here, Bolt automatically returns 401 HTTP status and it does not allow your app.error handlers to customize the behavior as of today. The logging level does not matter in this case. If you see the warn logging with invalid signature plus your app returns 401 HTTP status code immediately, it is an expected and valid behavior. If your app does not work this way, I would suggest you to check what HTTP response your app returns by using the above curl code snippet.

The App Directory review team verifies the following points:

  1. Your app handles incoming requests with signature that is generated using your app's valid signing secret
  2. Your app rejects incoming requests with any invalid signature (responding with 401 HTTP status is not the only option for it, though)

There are already many App Directory apps built with bolt-js. As long as you configure your app correctly, the 401 response to invalid signature should be accepted by the review team.

I don't have any further guidance here. I hope you will figure out the proper way to run your app soon.

@nirvparekh nirvparekh reopened this Jul 22, 2022
@nirvparekh
Copy link
Author

nirvparekh commented Jul 22, 2022

@seratch
I resubmitted the app and the reviewer team rejected again with the same issue. I tried to call the events endpoint using Postman to see if my app is returning 401 in case of signature mismatch. I observed the postman is not getting any response and it gets timed out
Proof:
https://www.loom.com/share/e0fb40fd7e2242c0b3ac3e35a224f4b7

I have simply initialized with the Bolt framework. then Bolt should send 401 http code. but the postman is not receiving any response.

What I'm missing?

@seratch
Copy link
Member

seratch commented Jul 22, 2022

@nirvparekh When I run the above curl script with my simple bolt-js app, the app immediately returns 401 response as below.

$ curl -XPOST http://localhost:3000/slack/events -d'this data does not matter' -H'x-slack-signature: xxx' -H'x-slack-request-timestamp: '`date '+%s'`  -v
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1:3000...
* Connected to localhost (127.0.0.1) port 3000 (#0)
> POST /slack/events HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.79.1
> Accept: */*
> x-slack-signature: xxx
> x-slack-request-timestamp: 1658469124
> Content-Length: 25
> Content-Type: application/x-www-form-urlencoded
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 401 Unauthorized
< X-Powered-By: Express
< Date: Fri, 22 Jul 2022 05:52:04 GMT
< Connection: keep-alive
< Keep-Alive: timeout=5
< Content-Length: 0
<
* Connection #0 to host localhost left intact

What happens if you run the following scripts with your app? If your app does not return any responses, that is quite different from the situation I am aware of. Also, I would suggest checking how a different simple app works when you send invalid requests.

curl -XPOST http://localhost:3000/slack/events -d'this data does not matter' -H'x-slack-signature: xxx' -H'x-slack-request-timestamp: '`date '+%s'`  -v
curl -XPOST https://xxx.ngrok.io/slack/events -d'this data does not matter' -H'x-slack-signature: xxx' -H'x-slack-request-timestamp: '`date '+%s'`  -v

@nirvparekh
Copy link
Author

@seratch when I hit the
curl -XPOST 'http://localhost:3001/slack/events' -H 'x-slack-signature:v0=asfasf' -H 'x-slack-request-timestamp:1658741612'
It doesn't respond at all. just keeps waiting for any response.
https://www.loom.com/share/2ab43f56eb3a4e5a8d42a1dddde11f74

@seratch
Copy link
Member

seratch commented Jul 25, 2022

@nirvparekh Thanks for your response here and I am sorry to say that I had been wrong here. My above testing was done with ExpressReceiver (my bad). ExpressReceiver does not have any issues but as you pointed out, the default HTTPReceiver has the issue that you've been facing. I will come up with a quick fix for it.

@seratch seratch added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch and removed question M-T: User needs support to use the project needs info An issue that is claimed to be a bug and hasn't been reproduced, or otherwise needs more info labels Jul 25, 2022
@seratch seratch self-assigned this Jul 25, 2022
@seratch seratch added this to the 3.12.1 milestone Jul 25, 2022
@seratch seratch changed the title how to check if signature verification failed? HTTPReceiver does not immediately respond to an invalid signature request (no response instead) Jul 25, 2022
seratch added a commit to seratch/bolt-js that referenced this issue Jul 25, 2022
…alid signature request (no response instead)
@nirvparekh
Copy link
Author

@seratch thanks a lot man... 👍

seratch added a commit that referenced this issue Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch
Projects
None yet
2 participants