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

Check X-Twilio-Signature in webhook middleware #447

Merged
merged 1 commit into from
May 14, 2019

Conversation

neerajwadhwa
Copy link
Contributor

Implemented a check in lib/webhooks/webhooks.js's webhook() middleware to check if the X-Twilio-Signature header exists or not.

If the request is not made from Twilio servers then we won't have X-Twilio-Signature header in the request and the middleware will throw cryptic error from validateRequest() function.

…to check if the X-Twilio-Signature header exists or not
@childish-sambino childish-sambino merged commit 1d74f2c into twilio:master May 14, 2019
@khalilchoudhry
Copy link
Contributor

khalilchoudhry commented Jun 15, 2019

@neerajwadhwa, @childish-sambino I was wondering, why this check is added here before checking the validate option. I am sending requests from POSTMAN and I have not set X-Twilio-Signature. I am on dev/test environment, which means the validate option is false, so there is no need to check if the header is present or not. Similarly, if I write a test for Express route in my code, the middleware will fail despite the fact that validate option is false.

@khalilchoudhry
Copy link
Contributor

khalilchoudhry commented Jun 15, 2019

@neerajwadhwa It also checks for X-Twilio-Signature here which will not fail with validate set to false.

@childish-sambino
Copy link
Contributor

@khalilchoudhry The changes mentioned were made in very close time proximity so the coordination wasn't perfect. Will follow-up on #460

@ghost ghost mentioned this pull request Jul 8, 2019
@childish-sambino childish-sambino added the type: bug bug in the library label Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug bug in the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants