Combining HttpPost and ValidateAntiForgeryToken

Thursday, September 8, 2016

I’ve been kicking around the idea of combining [HttpPost] and [ValidateAntiForgeryToken] in an application using authentication cookies. Both attributes typically appear together to prevent cross-site request forgeries in MVC applications using cookie based authentication. The result looks like the following.

[HttpPostWithValidAntiForgeryToken]
public IActionResult Edit(Input model)
{
    // ... 
}

And the attribute definition is as follows.

public class HttpPostWithValidAntiForgeryToken
    : Attribute, IActionHttpMethodProvider, IFilterFactory
{
    private readonly HttpPostAttribute _postAttribute;
    private readonly ValidateAntiForgeryTokenAttribute _antiForgeryAttribute;

    public HttpPostWithValidAntiForgeryToken()
    {
        _postAttribute = new HttpPostAttribute();
        _antiForgeryAttribute = new ValidateAntiForgeryTokenAttribute();
    }

    public IEnumerable<string> HttpMethods => _postAttribute.HttpMethods;

    public IFilterMetadata CreateInstance(IServiceProvider serviceProvider)
    {
        return _antiForgeryAttribute.CreateInstance(serviceProvider);
    }

    public bool IsReusable => _antiForgeryAttribute.IsReusable;
}

With the new attribute, a plain [HttpPost] should never appear in the application, and a unit test using reflection could enforce the rule.


Comments
gravatar JT Thursday, September 8, 2016
When implementing AF tokens across our large project, I didn't want to require changes to all the controllers. Instead, I used Phil Haack's conditional filter provider and via that globally added the AntiForgeryTokenAttribute to all POST requests. I did add a couple of exceptions, but you get the idea. Simpler than a different attribute for a POST IMO, but you may prefer the explicit approach. For me, we have a large team and a large project. Details like this were likely to be lost without trace sometimes so I felt the implicit strategy was good for us.
gravatar Anthony Mills Thursday, September 8, 2016
I use [HttpPost, ValidateAntiForgeryToken] to emphasize they should be together.
gravatar Matthew Wills Thursday, September 8, 2016
Another approach is to keep them separate, and add a unit test to confirm that all methods with [HttpPost] also have the anti forgery attribute.
Kurt Dowswell Thursday, September 8, 2016
I really like the idea of validating methods are decorated properly with unit tests. I use this approach to validate my WCF service endpoints and objects are decorated with the proper attributes.
gravatar Chris Chilvers Friday, September 9, 2016
Another interesting idea would be to create global CSRF validation filter that applies it self automatically to any post request. Then have a suppress validation attribute similar to how you can set AuthorizeAttribute as a global filter, then suppress it using the AllowAnonymousAttribute. Thus changing the CRSF validation to opt-out.
gravatar JT Saturday, September 10, 2016
@Chris Chilvers: see my comment above. That's exactly what I did, and IMO it's better in a team setting to just make it work all the time than try to enforce it with unit tests. I also ensured that there was JS hooks to enforce sending the token on the other side.
gravatar TE Monday, October 3, 2016
There may be situations where one can POST without needing the token or where the token is needed without POST. Internally generated computations that have cascaded from vetted user input might be one area, although I hesitate to go beyond being leery of always linking the two. I like the separation, but also the convenience of an attribute that will do both. Concerning the unit testing, if there is a stipulation that no POST should exist without an AFT, that is a compelling reason right there for a test. And there is still a possible requirement for testing that the AFT is not shirking. Creating a combo attribute would probably generate more tests than going with the out of the box attribute and AFT. And people will add bells and whistles and stripes and so forth, generating more testing. And then, in considering your interesting attribute, I think of what other marvelous things we might combine. Hideous and amazing chimeras arise that best not see the light of day. Let them lurk in the darkness of untested and unmanaged code, "Far, far below the deepest delvings of the Dwarves, the world is gnawed by nameless things...but I will bring no report to darken the light of day." LOTR Book III Ch 5 "The White Rider". THAT is what I am talking about.
Comments are closed.

My Pluralsight Courses

K.Scott Allen OdeToCode by K. Scott Allen
What JavaScript Developers Should Know About ECMAScript 2015
The Podcast!