validateExpressRequest not working correctly

[blazmrak]

Issue Summary

There is a duality in the validator code for validateExpressRequest as validateRequest requires body parameter to be an object, but validateRequestWithBody requires body to be a string. validateRequestWithBody is also called with

request.body || {}

so it fails even when body is not present.

Steps to Reproduce

1. call validateExpressRequest with a request which has a parsed body object or no body at all

Exception

If not before, exception occurs in getExpectedBodyHash because body is not a string

Technical details:

• twilio-node version: 3.56.0
• node version: 15.0.1

[shwetha-manvinkurke]

@blazmrak Thanks for reporting. This issue has been added to our internal backlog to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog.

Internal tracking

[blazmrak]

@shwetha-manvinkurke I have fixed it locally by stringifying the body object in getExpectedBodyHash:

.update(Buffer.from(JSON.stringify(body), 'utf-8'))

But to me this seems relatively ugly, because the request must also be edited in Flows for this to work (whitespaces have to be removed, because json body validation on backend executes on a raw string as far as I can tell). I'm relatively new to twilio, but I feel like this problem is more of a problem with how validation is working in general than the specific bug in the client code.

[shwetha-manvinkurke]

@blazmrak Can you walk me through your use case? You shouldn't have to update getExpectedBodyHash. The idea here is so the users can use validateExpressRequest for their express app when the webhooks are configured for the Twilio phone number. The callback then could be a GET or a POST. In case of a POST request, the body of the request would be a string and the query parameter bodySHA256 will be included in the request and hence that check in validateExpressRequest. The only issue I see is in this line - request.body || {} if we assume there won't be a body, which is again not the case since that request will always have a body. It would help to understand what exactly you are trying to do.

[blazmrak]

The body of the request is not a string, but an object, because it gets parsed. Your library expects it to be an object, because you also pass request.body to validateRequest function which only takes objects.

Also, the body does not need to be included, because the http request could serve as some sort of event trigger/custom call status webhook (this is not my usecase, but it could probably be used before the call gets diverted for example).

The workaround we used was to not make json requests.

[shwetha-manvinkurke]

@blazmrak I tried a couple of things -

1. I created a Studio Flow and in one of my steps, I set up a HTTP POST webhook with content type as Application/JSON and in my express app, I did a app.use(bodyParser.json()) and I could reproduce the issue that you saw. Meaning, the webhook()/validateExpressRequest() will fail because the body is not a string. I then did a JSON.stringify(request.body) in the validateExpressRequest and that works when there is are no whitespaces in the request body. If there are whitespaces, then the bodySHA wouldn’t match the hash that we calculate in getExpectedBodyHash. Basically, I ran through all the steps that you did.
2. Secondly, I set up a HTTP POST webhook directly in the console for my number and in my app, did a app.use(bodyParser.urlencoded({ extended: true })); and I could validate the request because in this case the content type is application/x-www-form-urlencoded and the validation logic is not the same as 1 as it does a validateRequest only.

While there is no clean way to do the validation in the client library if the request is parsed as json or rather, there is no obvious way at all, the best approach you could take is to parse the body as a text/string if the content type is application/json. If you are using body parser, this is what it would look like -

app.use(bodyParser.urlencoded({ extended: true }));
app.use(bodyParser.text({ type: "application/json" }));

If you do that, then you wouldn’t have a problem validating the webhooks. Let me know if you see a problem doing that. Also, let me know what parser you are using to parse the request.

[blazmrak]

If I was to change the parser, all of my other endpoints and middleware would die...

Also, having some bodies to be strings and some bodies to be objects seems incredibly inconsistent, especially in the language where types are a mystery anyways. It feels even uglier than the solution 1.

I have no idea what your codebase on the backend looks like and I have no idea how you guys think other codebases look like, but I have not yet seen a codebase where the json request body is not parsed into some sort of an object and remained a string.

The problem is a bit with this library, because it does not stringify the object, but the way you hash the body on the backend stinks, because you guys don't remove the whitespaces before hashing it (you are replacing the values in the {{}} anyways, so there really wouldn't be much of a performance impact).

We are using NestJS and it is probably using bodyparser under the hood, but I don't know how does a bodyparser relate to this issue.

[shwetha-manvinkurke]

@blazmrak we understand that there is a problem. We are not denying that and we will work with the other internal teams to get more clarity on the hashing itself and why they don't trim the whitespaces. We are also checking with the Studio team itself (assuming that's what you are using) to see if it's possible for them to trim the request body. In the meantime, would it be reasonable to ask you to feed in the raw body of the request into our library by doing something like this since we need the raw body for our validation? I understand that's not the most ideal solution but that would work if we change our library code to parse the request.rawBody instead for the body validation.

[blazmrak]

Yeah, this should work and not interfere with any of the existing code.

[ddc22]

To anyone who stumbles upon this and is using Nestjs and their .
1 - Make sure you are using the primary auth token not the secondary
2 - Makes sure you're using twilio.validateRequest other calls wont work since you need to provide a body has as a query param.
3 - When reconstructing the dev url in localhost always use https as the protocol even though you maybe on a http local tunnel