Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

easy way to have a custom AntiforgeryValidationException Error Page #3616

Closed
schmitch opened this issue Oct 11, 2018 · 7 comments
Closed

easy way to have a custom AntiforgeryValidationException Error Page #3616

schmitch opened this issue Oct 11, 2018 · 7 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Done This issue has been fixed
Milestone

Comments

@schmitch
Copy link

schmitch commented Oct 11, 2018

Hello, currently I have a .net core app (some parts are Razor Pages), currently we also share the same cookie with an old webforms app (thanks owin).

Currently if I try to Login with the same browser window in two tabs (i.e. same happens inside the old app), I will get an 400 response without a body, which is really really ugly.
I actually tried everything to get rid of it (even disabling AntiForgery for Login, which I guess does not work?!) but what I actually wanted to do is having a nice looking error page.

Currently that is what is happening inside my logs:

Microsoft.AspNetCore.Mvc.ViewFeatures.Internal.AutoValidateAntiforgeryTokenAuthorizationFilter[1]
      Antiforgery token validation failed. The provided antiforgery token was meant for a different claims-based user than the current user.
Microsoft.AspNetCore.Antiforgery.AntiforgeryValidationException: The provided antiforgery token was meant for a different claims-based user than the current user.
   at Microsoft.AspNetCore.Antiforgery.Internal.DefaultAntiforgery.ValidateTokens(HttpContext httpContext, AntiforgeryTokenSet antiforgeryTokenSet)
   at Microsoft.AspNetCore.Antiforgery.Internal.DefaultAntiforgery.ValidateRequestAsync(HttpContext httpContext)
   at Microsoft.AspNetCore.Mvc.ViewFeatures.Internal.ValidateAntiforgeryTokenAuthorizationFilter.OnAuthorizationAsync(AuthorizationFilterContext context)
info: Microsoft.AspNetCore.Mvc.RazorPages.Internal.PageActionInvoker[3]
      Authorization failed for the request at filter 'Microsoft.AspNetCore.Mvc.ViewFeatures.Internal.AutoValidateAntiforgeryTokenAuthorizationFilter'.

it would be great if there was a way to have an event where I can actually handle the exception and post a nice looking error page to the user, i.e. something like "you probably already logged in inside another tab" or if it is not login based (do something different).

however the issue is either one of the following:

  1. it's not possible in an easy way to show/redirect a razor view view nice looking error details
  2. the easy way is not documented in any way

i.e. and no it's not a programmer error if a user tries to login twice. (aspnet/Mvc#8276)

@poke
Copy link
Contributor

poke commented Oct 11, 2018

Have you tried creating your own error handler as explained in the error handling docs? You should be able to catch the AntiforgeryValidationException there and e.g. invoke some nice looking error page that way.

@schmitch
Copy link
Author

schmitch commented Oct 11, 2018

sadly, AntiforgeryValidationException will get converted into a error 400 somewhere in the Antiforgery Filter chain.

i.e.:

public async Task OnAuthorizationAsync(AuthorizationFilterContext context)
    {
      ValidateAntiforgeryTokenAuthorizationFilter authorizationFilter = this;
      if (context == null)
        throw new ArgumentNullException(nameof (context));
      if (!context.IsEffectivePolicy<IAntiforgeryPolicy>((IAntiforgeryPolicy) authorizationFilter))
      {
        authorizationFilter._logger.NotMostEffectiveFilter(typeof (IAntiforgeryPolicy));
      }
      else
      {
        if (!authorizationFilter.ShouldValidate(context))
          return;
        try
        {
          await authorizationFilter._antiforgery.ValidateRequestAsync(context.HttpContext);
        }
        catch (AntiforgeryValidationException ex)
        {
          authorizationFilter._logger.AntiforgeryTokenInvalid(ex.Message, (Exception) ex);
          context.Result = (IActionResult) new BadRequestResult();
        }
      }
    }

the best thing I can do is to overwrite ValidateAntiforgeryTokenAuthorizationFilter completly, somehow.

@poke
Copy link
Contributor

poke commented Oct 12, 2018

Hmm, you are right, the ValidateAntiforgeryTokenAuthorizationFilter does that and there is unfortunately no way to hook into that.

Allowing to hook into that, using some event system (like with the authentication stack) or by splitting up the validation, is probably too complicated and unfortunately, authorization filters are also called before exception filters (so you cannot just throw from authorization filters and have an exception filter handle it).

Also unfortunately, returning a result from an authorization filter short-circuits the MVC filter pipeline, so one couldn’t even inspect the result to handle the BadRequestResult differently in some way.

But what we could do maybe is create a custom InvalidAntiforgeryResult that also just produced a 400 but also allowed some hook when it is being executed. So one could maybe configure it through the AntiforgeryOptions and specify a custom InvalidAntiforgeryResult handler? I think I’ll give that idea a try and see how it feels.

@schmitch
Copy link
Author

well having something in context.Items which says "this 400 result was caused by xyz" would be enough since there is a app.UseStatusCodePages however at the moment it's not easy to know if the framework itself caused the error 400.

@poke
Copy link
Contributor

poke commented Oct 15, 2018

I just pushed a WIP PR for this over at aspnet/Mvc#8604 that implements the idea outlined above. You would use it like this to configure a redirect for example:

services.AddAntiforgery(options =>
{
    options.ValidationFailedResultHandler = ctx =>
    {
        ctx.Response.Redirect("/anti-forgery-error");
        return Task.CompletedTask;
    };
});

@pranavkm pranavkm added the Done This issue has been fixed label Oct 19, 2018
@pranavkm pranavkm added this to the 2.2.0 milestone Oct 19, 2018
@pranavkm
Copy link
Contributor

As part of the PR @poke sent, Antiforgery filter returns an IActionResult with a well known type - IAntiforgeryValidationFailedResult. The guidance is to write a IAlwaysRuntResultFilter that tests for this type. See https://github.com/aspnet/Mvc/blob/ddbe0fef2617f94a42312f002860f8f45145f11d/test/WebSites/BasicWebSite/Filters/RedirectAntiforgeryValidationFailedResultFilter.cs

@schmitch
Copy link
Author

well the outcome of the PR is really good, thanks @poke! and Asp.Net Core Team!

@Eilon Eilon added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed repo:Antiforgery labels Nov 7, 2018
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Done This issue has been fixed
Projects
None yet
Development

No branches or pull requests

4 participants