AntiForgery location

Topics: Customizing Orchard, Writing modules
Oct 16, 2011 at 3:01 PM

I am trying to do html 5 uploading. This works fine, if I relocate the antiforgery token to the forms collection.

 

But how to do it in a nice way, I think would be a change in Orchard itself.

 

In the default authorizer for MVC, the antiforgery token is supposed be on the Forms collection.

 

There is a HAACK in Orchard.Mvc.AntiForgery.AntiForgeryAuthorizationFilter somewhat like this:

  if ((filterContext.HttpContext.Request.HttpMethod != "POST" ||
                 _authenticationService.GetAuthenticatedUser() == null) && !ShouldValidateGet(filterContext)) {
                return;
            }

 

and this can wrap the token in an HackHttpContext.

 

These classes are private, and I cannot reuse them. My solution is as follows:

 [OrchardSuppressDependency("Orchard.Mvc.AntiForgery.AntiForgeryAuthorizationFilter")]
    public class AntiForgeryRelocation :  FilterProvider, IAuthorizationFilter
    {
        AntiForgeryAuthorizationFilter _orgFilter; 
        public AntiForgeryRelocation(ISiteService siteService, IAuthenticationService authenticationService, IExtensionManager extensionManager)
        {
            _orgFilter = new AntiForgeryAuthorizationFilter(siteService, authenticationService, extensionManager);
        }
        public  void OnAuthorization(AuthorizationContext filterContext)
        {
            const string tokenFieldName = "__RequestVerificationToken";
            if (!string.IsNullOrEmpty(filterContext.HttpContext.Request.Headers[tokenFieldName]))
            {
                filterContext.HttpContext = new HackHttpContext(filterContext.HttpContext, (HttpContext)filterContext.HttpContext.Items["originalHttpContext"]);
                ((HackHttpRequest)filterContext.HttpContext.Request).AddFormValue(tokenFieldName, filterContext.HttpContext.Request.Headers[tokenFieldName]);
                _orgFilter.OnAuthorization(filterContext);
                filterContext.HttpContext = ((HackHttpContext)filterContext.HttpContext).OriginalHttpContextBase;
            }
            else
            {
                _orgFilter.OnAuthorization(filterContext);
            }
        }

        #region HackHttpContext

        private class HackHttpContext : HttpContextWrapper
        {
            private readonly HttpContextBase _originalHttpContextBase;
            private readonly HttpContext _originalHttpContext;
            private HttpRequestWrapper _request;

            public HackHttpContext(HttpContextBase httpContextBase, HttpContext httpContext)
                : base(httpContext)
            {
                _originalHttpContextBase = httpContextBase;
                _originalHttpContext = httpContext;
            }

            public HttpContextBase OriginalHttpContextBase
            {
                get { return _originalHttpContextBase; }
            }

            public override HttpRequestBase Request
            {
                get
                {
                    if (_request == null)
                        _request = new HackHttpRequest(_originalHttpContext.Request);

                    return _request;
                }
            }
        }

        #endregion

        #region HackHttpRequest

        private class HackHttpRequest : HttpRequestWrapper
        {
            private readonly HttpRequest _originalHttpRequest;
            private NameValueCollection _form;

            public HackHttpRequest(HttpRequest httpRequest)
                : base(httpRequest)
            {
                _originalHttpRequest = httpRequest;
            }

            public override NameValueCollection Form
            {
                get
                {
                    if (_form == null)
                        _form = new NameValueCollection(_originalHttpRequest.Form);

                    return _form;
                }
            }

            public void AddFormValue(string key, string value)
            {
                Form.Add(key, value);
            }
        }

        #endregion
    }

But this is as a seperate module, which duplicates code (and this is such a small thing to have an entire module for). The solution which  I would prefer is if the original AntiForgeryAuthorizationFilter would relocate the __RequestAntiForgeryToken if it was in the Header, and possible as an addition relocate if the token was a querystring parameter. On the javascript side of things, this would work nice:

    var xhr = new XMLHttpRequest();
                xhr.upload.addEventListener('progress', uploadProgress, false);
                xhr.onreadystatechange = stateChange;
                xhr.open('POST', '/OrchardLocal/UploadFile', true);
                xhr.setRequestHeader('X-FILE-NAME', file.name);
                xhr.setRequestHeader('__RequestVerificationToken','@Html.AntiForgeryTokenValueOrchard()');
                xhr.send(file);

Oct 17, 2011 at 6:03 PM
Edited Oct 17, 2011 at 6:17 PM
morgul77 wrote:

... deleted ... 

There is a HAACK in Orchard.Mvc.AntiForgery.AntiForgeryAuthorizationFilter somewhat like this:

  if ((filterContext.HttpContext.Request.HttpMethod != "POST" ||
                 _authenticationService.GetAuthenticatedUser() == null) && !ShouldValidateGet(filterContext)) {
                return;
            }

 ... deleted ... 

---

I don't get why this haack reads the way it does. Should it not be:
 if (filterContext.HttpContext.Request.HttpMethod != "POST" && !ShouldValidateGet(filterContext)) return;
What does authenticated user has to do with cross site attack? I may not be logged in and I am me?!
Then ShouldValidateGet(filterContext) includes:
if user is authenticated, then it is not a cross site attack. Right? And the cross site attack is much more complex to filter out.

 

Oct 17, 2011 at 8:33 PM
Edited Oct 17, 2011 at 8:34 PM

Not sure what the original hack is meant to do, except perhaps force the user to click from the home page or some entry page, when they are not logged in?

 

I think there is an ActionLink overload which injects the antiforgery token as a querystring parameter, but I need to check the code.

Oct 18, 2011 at 3:12 PM
Edited Oct 18, 2011 at 3:47 PM

I have done some thinking and looking into the Orchard Antiforgery Token. I am not sure if I understand it yet. But I think you have something. Does sending it back in the xhr.setRequestHeader work? The last time that I looked at it, it has to be sent back url encoded.

Oct 18, 2011 at 8:46 PM

My code works for me, and it does not need to be url encoded. (unless I am mistaking something and it happens automagically)

const string tokenFieldName = "__RequestVerificationToken";
            if (!string.IsNullOrEmpty(filterContext.HttpContext.Request.Headers[tokenFieldName]))
            {
                filterContext.HttpContext = new HackHttpContext(filterContext.HttpContext, (HttpContext)filterContext.HttpContext.Items["originalHttpContext"]);
                ((HackHttpRequest)filterContext.HttpContext.Request).AddFormValue(tokenFieldName, filterContext.HttpContext.Request.Headers[tokenFieldName]);
                _orgFilter.OnAuthorization(filterContext);
                filterContext.HttpContext = ((HackHttpContext)filterContext.HttpContext).OriginalHttpContextBase;
            }
            else
            {
                _orgFilter.OnAuthorization(filterContext);
            }

is the piece of code I think would to the trick inside orchard, alongside or instead of the current hack.  As a side note, I first got the idea from here:http://haacked.com/archive/2011/10/10/preventing-csrf-with-ajax.aspx

 

The idea is simple: The token MUST be in forms collection inside mvc3. But the code above wraps the context, and move the token into a temporary wrapper and pass that to mvc3 for validation.

 

It makes the javascript simple and effective, without adding it the JSON object etc (which also works). The upside is html5 file upload will also work, with the setRequestHeader.

 

The existing hack is also somewhat crippled with the fact that html5 file upload WOULD work if it took the validation token regardless of post/get/authenticated user, if the token was called like ?__RequestVerificationToken=UrlEncodedToken.

 

Which would effectively elminated the need for my workaround (although I think mine is nicer), but there is no reason that both locations would be relocated as needed

Oct 18, 2011 at 10:09 PM
Edited Oct 18, 2011 at 10:24 PM

The only way I got the antiforgery token to work was to mimic the media picker example.

The existing hack is crippled because you have to be sending the token back using jquery, which url encodes the token. And futhermore you have to be either logged in or logged off. This makes the logic/behavior difficult to diagnose or understand. If the original intention was to redirect the unauthenticated users to a public area, then I think it should do so independently of a cross-site-attack detection. I say this because if I am a module writer, then I am not suppose to modify Orchard itself, I should only write the module.

And if you look at the token just before jquery sends the token back to the server, it is url encoded, which implies the controller must url decode the token. But this is the Orchard token. Your solution is very interesting, I will have to take a closer look at it soon.

 It is definitely nicer to be able to send it back as xhr.setRequestHeader('__RequestVerificationToken','@Html.AntiForgeryTokenValueOrchard()');

Someone else feel free to comment on the thread, don't let me dominate this conversation :)

Jan 12, 2012 at 5:16 PM
Edited Jan 12, 2012 at 5:16 PM

ref:

http://orchard.codeplex.com/discussions/249394

 

which one is better?

Mar 29, 2012 at 12:18 AM
infofromca wrote:

ref:

http://orchard.codeplex.com/discussions/249394

 

which one is better?

+1 for the this solution by morgul77. I used it in my project and it works just fine with jQuery and $.ajax() if you set the headers member of the config object. The other plugin solution changes the request's POST data from being real serialised JSON to URL-encoded forms key-value pairs, which is not a good way to encode JS objects, particularly complex objects with nested arrays and such.

Using morgul77's solution does not require any change to the data format at all but instead provides an alternate way to pass the required value on to the MVC framework.

I second that this should be included in Orchard as an update to the Orchard.Mvc.AntiForgery.AntiForgeryAuthorizationFilter, possibly in conjunction with another attribute (like how Orchard.Mvc.AntiForgery.ValidateAntiForgeryTokenOrchardAttribute works for GET requests with a QueryString) so you can opt-in. (I know the dev. team is busy so if you want me to submit a patch I'd be more than happy.)

Mar 16, 2013 at 7:17 PM
This hack has helped me post multiple cart items via json. Thanks.

This is a year old and I was wondering if there was a cleaner solution to this yet.