Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

AntiforgeryMiddleware needs an escape-hatch for troubleshooting and logging, and customizing the final response #68054

Copy link
Copy link

Description

@daiplusplus
Issue body actions

Is there an existing issue for this?

  • I have searched the existing issues

Searching existing issues found these which are somewhat relevant, but ultimately unhelpful for this specific problem:

Is your feature request related to a problem? Please describe the problem.

When AntiforgeryMiddleware is active, it becomes impossible to inspect HttpRequest.Form for logging or troubleshooting purposes, which frustrates attempts to resolve (non-malicious) inadvertent CSRF failures. Additionally, there are plenty of locations within AspNetCore itself where HttpRequest.Form or HasFormContentType is used such that HandleUncheckedAntiforgeryValidationFeature() will throw.

Relatedly, I noticed that there doesn't seem to be a way to return a user-friendly error-page or non-500 response when there is a CSRF failure.

For example, because Microsoft.AspNetCore.Mvc.ModelBinding.FormValueProviderFactory accesses HttpRequest.HasFormContentType it will throw-up the InvalidOperationException caused by the AntiforgeryMiddleware:

[12:56:19 Error] Microsoft.AspNetCore.Server.Kestrel
Connection id "0HNNC321O3812", Request id "0HNNC321O3812:00000035": An unhandled exception was thrown by the application.
System.InvalidOperationException: This form is being accessed with an invalid anti-forgery token. Validate the `IAntiforgeryValidationFeature` on the request before reading from the form.
   at Microsoft.AspNetCore.Http.Features.FormFeature.HandleUncheckedAntiforgeryValidationFeature()
   at Microsoft.AspNetCore.Http.Features.FormFeature.get_HasFormContentType()
   at Microsoft.AspNetCore.Mvc.ModelBinding.FormValueProviderFactory.CreateValueProviderAsync(ValueProviderFactoryContext context)
   at Microsoft.AspNetCore.Mvc.ModelBinding.CompositeValueProvider.CreateAsync(ActionContext actionContext, IList`1 factories)
   at Microsoft.AspNetCore.Mvc.ModelBinding.CompositeValueProvider.TryCreateAsync(ActionContext actionContext, IList`1 factories)
   at Microsoft.AspNetCore.Mvc.Controllers.ControllerBinderDelegateProvider.<>c__DisplayClass0_0.<<CreateBinderDelegate>g__Bind|0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeInnerFilterAsync>g__Awaited|13_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResourceFilter>g__Awaited|25_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResourceExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeFilterPipelineAsync()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)

Furthermore, because it throws the generic exception-type, InvalidOperationException with no specific information carried with it, it's impractical to intercept it in middleware and return a more appropriate response - and as said, it's impossible to log the actual parsed Request.Form values (if any) when they may aid troubleshooting.

Describe the solution you'd like

IAntiforgeryValidationFeature should expose RequestHasFormContentType and the IFormCollection that HttpRequest.Form would otherwise return without throwing InvalidOperationException, for use by logging/debugging/troubleshooters.

Workaround

This is what I'm currently doing as a hack workaround to enable logging of Request.Form, though it uses an undocumented HttpContext.Items key - and I'm unsure how robust or reliable it is under other CSRF scenarios:

		private static String GetRequestFormForLogScope( HttpRequest req )
		{
			if( !"POST".EqualsIns( req.Method ) ) return "(Non-POST request)";

			//

			// AspNetCore's AntiForgery feature blocks access to `HttpRequest.Form`, including even checking `HasFormContentType` - which means we can't log those details (grrr).
			// I filed this issue concerning this problem: https://github.com/dotnet/aspnetcore/issues/68054

			IAntiforgeryValidationFeature? csrfFeature = req.HttpContext.Features.Get<IAntiforgeryValidationFeature>();
			if( csrfFeature is null || csrfFeature.IsValid )
			{
				if( req.HasFormContentType ) // <-- Accessing this property will throw if `csrfFeature.IsValid == false`
				{
					return StringBuilderLoggingExtensions.FlattenRequestForm( req.Form );
				}
				else
				{
					return KnownSerilogProperties.AspNetRequestForm_InvalidContentType;
				}
			}
			else
			{
				HttpContext httpContext = req.HttpContext;

				// HACK:
				const String FLAG_ITEM_KEY = @"__AntiforgeryMiddlewareWithEndpointInvoked";
				if( httpContext.Items.ContainsKey( FLAG_ITEM_KEY ) )
				{
					Object? item = httpContext.Items[ FLAG_ITEM_KEY ];
					_ = httpContext.Items.Remove( FLAG_ITEM_KEY );
					try
					{
						if( req.HasFormContentType )
						{
							return StringBuilderLoggingExtensions.FlattenRequestForm( req.Form );
						}
						else
						{
							return KnownSerilogProperties.AspNetRequestForm_InvalidContentType;
						}
					}
					finally
					{ 
						httpContext.Items[ FLAG_ITEM_KEY ] = item;
					}
				}
				else
				{
					return "CSRF Antiforgery error: " + csrfFeature.Error?.ToString();
				}
			}
		}

Additional context

No response

Reactions are currently unavailable

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

    Morty Proxy This is a proxified and sanitized view of the page, visit original site.