Skip to content

Commit

Permalink
Avoid re-running routing for implicit middlewares and remove implicit…
Browse files Browse the repository at this point in the history
… anti-forgery (#50864)

## Description

Avoid running routing eagerly in implicit middlewares to prevents the `EndpointFeature` from being set and causing unexpected reactions in other middlewares, like the static file middleware.

We also remove the implicit registration of the anti-forgery middleware to avoid unintended collisions with authentication in Blazor.

Fixes #50818, #50815, #50844

## Customer Impact

Without this change, the anti-forgery middleware in Blazor apps runs too early and is not able to examine authentication state in the application. Requiring the middleware to be registered explicitly ensures that the correct ordering is applied.

Without this change, users will run into difficult to resolve issues with building applications that include forms with Blazor web apps.

## Regression?

- [X] Yes
- [ ] No

This is a regression that was introduced to middleware routing in .NET 8 Preview 7.

## Risk

- [ ] High
- [X] Medium
- [ ] Low

**Medium risk** because:
- We are reverting a change that was originally applied to resolve #49654. This means that the original bug will impact users, specifically those who are calling `UseRouting` explicitly without calling `UseAuthentication` and `UseAuthorization` if they are not available. There is a workaround that we plan to document this behavior for users.
- Apps deployed in .NET 8 RC 1 will break because we no longer automatically enable the anti-forgery middleware. Users will receive an exception at startup notifying them of the code changes to make in order to get things working correctly.

## Verification

- [X] Manual (required)
- [X] Automated

## Packaging changes reviewed?

- [ ] Yes
- [ ] No
- [X] N/A
  • Loading branch information
captainsafia authored Sep 25, 2023
1 parent f269696 commit 3dbfa81
Show file tree
Hide file tree
Showing 11 changed files with 7 additions and 438 deletions.
15 changes: 0 additions & 15 deletions src/Antiforgery/src/AntiforgeryApplicationBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@

using Microsoft.AspNetCore.Antiforgery;
using Microsoft.AspNetCore.Antiforgery.Internal;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;

namespace Microsoft.AspNetCore.Builder;

Expand All @@ -26,19 +24,6 @@ public static IApplicationBuilder UseAntiforgery(this IApplicationBuilder builde
builder.VerifyAntiforgeryServicesAreRegistered();

builder.Properties[AntiforgeryMiddlewareSetKey] = true;

// The anti-forgery middleware adds annotations to HttpContext.Items to indicate that it has run
// that will be validated by the EndpointsRoutingMiddleware later. To do this, we need to ensure
// that routing has run and set the endpoint feature on the HttpContext associated with the request.
if (builder.Properties.TryGetValue(RerouteHelper.GlobalRouteBuilderKey, out var routeBuilder) && routeBuilder is not null)
{
return builder.Use(next =>
{
var newNext = RerouteHelper.Reroute(builder, routeBuilder, next);
var antiforgery = builder.ApplicationServices.GetRequiredService<IAntiforgery>();
return new AntiforgeryMiddleware(antiforgery, newNext).Invoke;
});
}
builder.UseMiddleware<AntiforgeryMiddleware>();

return builder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,5 @@

<ItemGroup>
<Compile Include="$(SharedSourceRoot)HttpExtensions.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)Reroute.cs" LinkBase="Shared"/>
</ItemGroup>
</Project>
11 changes: 0 additions & 11 deletions src/DefaultBuilder/src/WebApplicationBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using Microsoft.AspNetCore.Antiforgery;
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Hosting;
Expand All @@ -25,7 +24,6 @@ public sealed class WebApplicationBuilder : IHostApplicationBuilder
private const string EndpointRouteBuilderKey = "__EndpointRouteBuilder";
private const string AuthenticationMiddlewareSetKey = "__AuthenticationMiddlewareSet";
private const string AuthorizationMiddlewareSetKey = "__AuthorizationMiddlewareSet";
private const string AntiforgeryMiddlewareSetKey = "__AntiforgeryMiddlewareSet";
private const string UseRoutingKey = "__UseRouting";

private readonly HostApplicationBuilder _hostApplicationBuilder;
Expand Down Expand Up @@ -453,15 +451,6 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui
}
}

if (serviceProviderIsService?.IsService(typeof(IAntiforgery)) is true)
{
if (!_builtApplication.Properties.ContainsKey(AntiforgeryMiddlewareSetKey))
{
_builtApplication.Properties[AntiforgeryMiddlewareSetKey] = true;
app.UseAntiforgery();
}
}

// Wire the source pipeline to run in the destination pipeline
var wireSourcePipeline = new WireSourcePipeline(_builtApplication);
app.Use(wireSourcePipeline.CreateMiddleware);
Expand Down
Loading

0 comments on commit 3dbfa81

Please sign in to comment.