|
While working through Skywalker's webshop tutorials I noticed that the ajax requests being sent to update the shopping cart quantity (from the client-side knockoutjs based script) were failing with HTTP 500 errors due to the __RequestVerificationToken not
being found.
I have AntiForgery enabled in the module, thus Orchard's AntiForgeryAuthorizationFilter is automatically attempting to validate POST requests. The ajax request being sent is a POST with JSON data.
I want to detail the solution I have come up with for feedback, and I also want to raise a few questions/concerns around the current implementation of Orchard's AntiForgeryAuthorizationFilter (Orchard.Framework/Mvc/AntiForgery/AntiForgeryAuthorizationFilter.cs)
As a background, the root issue is with the core System.Web.Mmvc.ValidateAntiForgeryTokenAttribute class which only looks for the _RequestVerificationToken within form fields in the post request. So if you are POSTing JSON data or using GET with the fields
in a query string, the anti-forgery token validation will fail.
Orchard has a partial work-around to this (at least for GET requests) through the provision of the ValidateAntiForgeryTokenOrchardAttribute which can be used on GET actions in a controller to force the AntiForgeryAuthorizationFilter to validate the GET request.
The AntiForgeryAuthorizationFilter then checks for this attribute on the current action, and if found it will search the request query string for the __RequestVerificationToken and use a hack to inject it into the HttpContext as a form field before validating
with the core System.Web.Mmvc.ValidateAntiForgeryTokenAttribute.
Concern 1. If you're performing a POST (from AJAX), even if you include the __RequestVerificationToken in the query string, the code in this filter will not look for it unless you explicitly adorn your action with ValidateAntiForgeryTokenOrchardAttribute.
This is a little counter-intuitive because if you have AntiForgery enabled in your module, then the assumption (at least my assumption) is that all POST requests are validated, and that the ValidateAntiForgeryTokenOrchardAttribute is only needed to force GET
requests to be validated.
Concern 2. The first block of code in the filter's OnAuthorization method will drop-out (bypass validation) if the request is a POST request AND no user is currently authenticated (when the action is not adorned with ValidateAntiForgeryTokenOrchardAttribute).
This is effectively saying "If it's a POST request and there's no authenticated user then check if the ValidateAntiForgeryTokenOrchardAttribute is on the action, and if it's not, then don't bother validating". I question why the state of the current
user even comes into play here? Don't we want to do anti-forgery checks whether regardless of whether a user is authenticated or anonymous?
Concern 3. With the progession to MVC 4, we can now call System.Web.Helpers.AntiForgery.Validate() directly and pass in the cookie and the value contained in __RequestVerificationToken. This means I think we can do away with the HackHttpContext
work-around in the AntiForgeryAuthorizationFilter alltogether and come up with a much cleaner approach.
Concern 4. Is the AntiForgeryAuthorizationFilter hit if requests are made via the new WebAPI features (which I believe Orchard now supports)? If so, then if someone has a module that exposes WebAPI actions and has AntiForgery enabled on
the module, then the OnAuthorization routine possibly should appply validation to PUT and DELETE methods? I guess it could look for the validation token in the request header?
So here's the approach I've taken to the AntiForgeryAuthorizationFilter class...
- In OnAuthorization, if the request is a GET request and the action is not adorned with ValidateAntiForgeryTokenOrchardAttribute then dropout. So if it's a POST or DELETE or PUT, we'll keep going...
- Then check if AntiForgery is not enabled on the module in which case we'll dropout (unless the ValidateAntiForgeryTokenOrchardAttribute is on the action - in which case we always validate - allows us to selectively validate actions even when anti-forgery
is disabled for the module by default).
- Now check the http request header for a validation token (I think this is a nice clean place to put it, and could also support WebAPI based requests - not that I'm sure there's a need for anti-forgery validation in this case). This will allow us to inject
the token in the header of the http request when using ajax. In the sample code below, I've given the http header key "X-Request-Verification-Token" rather than "__RequestVerificationToken" to indicate the header is non-standard.
- Also check the query string for the validation token - supporting the case where a form has been submitted using GET (if we got past step 1 above).
- If we found a token from either of the checks in step 3/4 above then call System.Web.Helpers.AntiForgery.Validate() passing in the cookie and token, rather than hacking a form field into the HttpContext before using the core System.Web.Mmvc.ValidateAntiForgeryTokenAttribute
method of validation.
- If we didn't find a token, then just call the core System.Web.Mmvc.ValidateAntiForgeryTokenAttribute to validate (this will cover all our standard POST forms) as we do now.
Here's the refactored AntiForgeryAuthorizationFilter class...
using System;
using System.Collections.Specialized;
using System.Web;
using System.Web.Mvc;
using System.Web.Routing;
using JetBrains.Annotations;
using Orchard.Environment.Extensions;
using Orchard.Mvc.Filters;
using Orchard.Security;
using Orchard.Settings;
using Orchard.Mvc.AntiForgery;
namespace Orchard.Mvc.AntiForgery {
[UsedImplicitly]
public class AntiForgeryAuthorizationFilter : FilterProvider, IAuthorizationFilter {
private readonly ISiteService _siteService;
private readonly IAuthenticationService _authenticationService;
private readonly IExtensionManager _extensionManager;
public AntiForgeryAuthorizationFilter(ISiteService siteService, IAuthenticationService authenticationService, IExtensionManager extensionManager)
{
_siteService = siteService;
_authenticationService = authenticationService;
_extensionManager = extensionManager;
}
public void OnAuthorization(AuthorizationContext filterContext) {
var request = filterContext.HttpContext.Request;
// TODO: We might want to add a new attribute to allow validation to be disabled
// for particular actions when the module has AntiForgery enabled in general. For
// example DoNotValidateAntiForgeryTokenOrchardAttribute. If we had this we would
// check here and drop out first.
// If the action is adorned with the ValidateAntiForgeryTokenOrchard attribute then
// we always validate regardless of the general module setting or the http method.
// Otherwise we'll only validate if anti-forgery is enabled for the module and the
// request is not a GET request.
if (!ShouldValidate(filterContext))
{
if (request.HttpMethod == "GET" || !IsAntiForgeryProtectionEnabled(filterContext))
return;
}
// If the request header or the request query string contains the
// verification token, then use this to validate the request.
string requestVerificationToken = request.Headers.Get("X-Request-Verification-Token") ?? request.QueryString["__RequestVerificationToken"];
if (!string.IsNullOrWhiteSpace(requestVerificationToken))
{
var cookie = request.Cookies[System.Web.Helpers.AntiForgeryConfig.CookieName];
System.Web.Helpers.AntiForgery.Validate(cookie != null ? cookie.Value : null, requestVerificationToken);
}
else
{
var validator = new ValidateAntiForgeryTokenAttribute();
validator.OnAuthorization(filterContext);
}
}
private bool IsAntiForgeryProtectionEnabled(ControllerContext context) {
string currentModule = GetArea(context.RouteData);
if (!String.IsNullOrEmpty(currentModule)) {
foreach (var descriptor in _extensionManager.AvailableExtensions()) {
if (String.Equals(descriptor.Id, currentModule, StringComparison.OrdinalIgnoreCase)) {
if (String.Equals(descriptor.AntiForgery, "enabled", StringComparison.OrdinalIgnoreCase)) {
return true;
}
return false;
}
}
}
return false;
}
private static string GetArea(RouteData routeData) {
if (routeData.Values.ContainsKey("area"))
return routeData.Values["area"] as string;
return routeData.DataTokens["area"] as string ?? "";
}
private static bool ShouldValidate(AuthorizationContext context) {
var attributes =
(ValidateAntiForgeryTokenOrchardAttribute[])
context.ActionDescriptor.GetCustomAttributes(typeof (ValidateAntiForgeryTokenOrchardAttribute), false);
if (attributes.Length > 0)
{
return true;
}
return false;
}
}
}
You can then include the following javascript (needs jQuery) in your layout.cshtml to ensure all AJAX POST requests automatically include a validation token in the request header (just grabs the first one it can find in the DOM). Works a treat!
$(document).ready(function () {
// Pass anti-forgery token through in the header of all ajax requests
$.ajaxPrefilter(function (options, originalOptions, jqXHR) {
var verificationTokenField = $("input[name='__RequestVerificationToken']").first();
if (verificationTokenField) {
jqXHR.setRequestHeader("X-Request-Verification-Token", verificationTokenField.val());
}
});
});
I've gone so far as writing a result filter (OnResultExecuting) in my own module to inject the javascript automatically so I don't need to explicitly include it in each theme I write. I think it would be nice if we had a core feature that did this (eg. AjaxAntiForgery
or something) or maybe we could add a setting and this filter to the Orchard.JQuery module "Inject Anti-Forgery Token into AJAX request headers"?
I'd really appreciate any thoughts and feedback on this approach! I'll then submit a change based on the feedback.
|