twilio-labs / twilio-aspnet

Integrate Twilio Programmable Messaging and Voice with ASP.NET Respond to webhooks with TwiML in seconds
Apache License 2.0
59 stars 30 forks source link

Request validation incorrectly returns 403 #92

Closed rhegner closed 2 years ago

rhegner commented 2 years ago

In my ASP.NET Core application I updated the Twilio.AspNet.Common package to the latest version 6.0.0 and I wanted to start using the new ValidateRequestAttribute for request validation.

I added the [ValidateRequest] (and [AllowAnonymous]) attribute on the controller level, so all actions should behave the same with regards to request validation.

But interestingly, for some requests from Twilio the validation filter returns a 403 while others go through just fine.

I noticed one difference between failing and succeeding requests: All the failing request URLs contain custom query parameters, for example: https://xxx.xxx/api/Twilio/CallerIntroduction?applicationUserId=94c08247-320a-4e53-b3f2-8c175feb588e&iteration=0

The succeeding requests are plain urls without query parameters, for example: https://xxx.xxx/api/Twilio/DisclaimerAndWait

Does the ValidateRequestAttribute from the Twilio.AspNet.Common 6.0.0 package have a bug with regards to handling of query parameters, or am I doing something wrong?

rhegner commented 2 years ago

PS: I'm using the BaseUrlOverride feature, because my server is behind a reverse proxy.

Could it be that HttpContext.Request.Path does not include query parameters, so that they get lost here: https://github.com/twilio-labs/twilio-aspnet/blob/eba5055f7468a1a17c7fd69136ec735b5031dd9e/src/Twilio.AspNet.Core/ValidateRequestAttribute.cs#L62

That would explain to me why validation fails...

rhegner commented 2 years ago

The line above should probably read:

urlOverride = $"{BaseUrlOverride}{filterContext.HttpContext.Request.Path}{filterContext.HttpContext.Request.QueryString}"; 

to be consistent with https://github.com/twilio-labs/twilio-aspnet/blob/eba5055f7468a1a17c7fd69136ec735b5031dd9e/src/Twilio.AspNet.Core/RequestValidationHelper.cs#L47

Swimburger commented 2 years ago

Thank you for writing this up and fixing the issue. I applied the fix to .Mvc repo too.