Mvc: IgnoreAntiforgeryTokenAttribute possibly broken on 1.1.0

[luisgoncalves]

I have the following test application with a global filter for anti-forgery token validation and then a validation bypass on a specific action method:

Startup

    public class Startup
    {
        public void ConfigureServices(IServiceCollection services)
        {
            services.AddMvc(options =>
            {
                options.Filters.Add(new AutoValidateAntiforgeryTokenAttribute());
            });
        }

        public void Configure(IApplicationBuilder app)
        {
            app.UseMvcWithDefaultRoute();
        }
    }

Controller

````
public class TestController : Controller
{
[HttpGet]
public IActionResult Index()
{
return View();
}

    [HttpPost, IgnoreAntiforgeryToken]
    public IActionResult Index(string test)
    {
        return Content("OK");
    }
}

````

Index view

````

Submit

````

Using the 1.0.1 MVC package I can access /Test, submit and get the successful response. However, if I use the 1.1.0 package I get a 400 Bad Request when submitting the form

Am i missing something or did this behavior actually change?

[rynowak]

There's no intended change here. We need to take a look at this /cc @Eilon

[Eilon]

@ryanbrandenburg can you investigate, and also see if perhaps it's patch-worthy? (Is there a workaround?)

[kevinchalet]

FWIW, this bug also impacts Orchard 2's OpenID module, which worked fine with the 1.0.0 bits.

/cc @jersiovic @sebastienros

[rynowak]

The bug here is that this PR https://github.com/aspnet/Mvc/commit/01b237dda9f7320313cbbc881d881d0a26ec432f should have also update the order value of [IgnoreAntiforgery]

If you need a workaround for this, I'd recommend setting [IgnoreAntiforgeryToken(Order = 1000)] - OR using application model to set the order (sample to follow).

[rynowak]

Here's a convention that will fix it.

            services.AddMvc(options =>
            {
                options.Filters.Add(new AutoValidateAntiforgeryTokenAttribute());

                options.Conventions.Add(new FixIgnoreAntiforgeryConvention());
            });

        private class FixIgnoreAntiforgeryConvention : IApplicationModelConvention
        {
            public void Apply(ApplicationModel application)
            {
                foreach (var filter in application.Filters)
                {
                    ProcessFilter(filter);
                }

                foreach (var controller in application.Controllers)
                {
                    foreach (var filter in controller.Filters)
                    {
                        ProcessFilter(filter);
                    }

                    foreach (var action in controller.Actions)
                    {
                        foreach (var filter in action.Filters)
                        {
                            ProcessFilter(filter);
                        }
                    }
                }
            }

            private void ProcessFilter(IFilterMetadata filter)
            {
                var ignoreFilter = filter as IgnoreAntiforgeryTokenAttribute;
                if (ignoreFilter != null)
                {
                    ignoreFilter.Order = 1000;
                }
            }
        }

[Eilon]

Done?

[Eilon]

Re-opening so we can take this for the 1.1.1 branch.

[ryanbrandenburg]

@Eilon I assume this is an oversight but this is patch-approved not just patch-proposed right? I can go ahead with merging this commit into rel/1.1.2?

[Eilon]

Not yet approved, but soon!

[Eilon]

This patch bug is approved. Please use the normal code review process w/ a PR and make sure the fix is in the correct branch, then close the bug and mark it as done.

[Eilon]

Also please remember that this is 1.1.2, so use the right branch! 😄

[viewtance]

A little confuses, did this bug fixed? In asp.net core 1.1.1 or asp.net core mvc 1.1.2?

[Eilon]

@viewtance it's in ASP.NET Core MVC 1.1.2, which was released about a week ago: https://www.nuget.org/packages/Microsoft.AspNetCore.Mvc/1.1.2