Aspnetcore: Detect AntiforgeryValidationException without throwing

[davidfowl]

Seems like a first chance exception here. Can we avoid it by adding a new API to the IAntiforgery interface (or another interface)?

https://github.com/aspnet/Mvc/blob/760c8f38678118734399c58c2dac981ea6e47046/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ValidateAntiforgeryTokenAuthorizationFilter.cs#L43

[Eilon]

We could presumably add a new interface with that capability and check for it and call the new method.

e.g.

c# public interface IAntiforgeryWithoutException { bool DoValidationThingy(out string message); }

Or something like that. And then check if the _antiforgery instance implements it, and if so, call the new API.

Is that what you had in mind?

[benaadams]

Perhaps more:

bool TryValidateRequestAsync(HttpContext context, ILogger logger)

So?

if (!await _antiforgery.TryValidateRequestAsync(context.HttpContext, _logger))
{
    context.Result = new BadRequestResult();
}

[rynowak]

Or maybe we make it an abstract class that implements the old interface. Then we don't have to keep doing this dance. IAntiforgery is already a facade for convenience.

[davidfowl]

Sounds good.

[dougbu]

I'm missing something here. Isn't the fix just to use the IsRequestValidAsync(...) method instead of ValidateRequestAsync(...)? That is, IAntiForgery and our default implementation already what we need to avoid first-chance exceptions. Can just change ValidateAntiforgeryTokenAuthorizationFilter.

Of course, should look for other non-test ValidateRequestAsync(...) calls.

[dougbu]

@rynowak let me know what I was missing: Changing to IsRequestValidAsync(...) would ignore GET and other "safe" requests and it would lose the message which ValidateAntiforgeryTokenAuthorizationFilter uses to log detailed information about a failure.

[pranavkm]

As @dougbu commented, there are slight variances in the behavior for IsRequestValidAsync and ValidateRequestAsync that make it nearly, but not 100% compatible. However, given that in ordinary code paths you wouldn't encounter this exception, I'm not sure the suggested work here is warranted. Closing this as won't fix.