Defer implicit middlewares to run after Routing - #67307
Conversation
…dotnet#67174) The auto-injected CsrfProtectionMiddleware runs in the destination pipeline. When an app calls app.UseRouting() explicitly, routing lives only in the source pipeline and runs after the CSRF check, so HttpContext.GetEndpoint() is null at CSRF time. The endpoint's named CORS policy ([EnableCors(name)] / RequireCors(name)) is therefore lost and a legitimate cross-origin POST is wrongly rejected. Give the middleware an on-demand endpoint matcher (a standalone UseRouting() branch) used only when routing has not yet run. It performs endpoint matching but never execution, and the pre-routing state is restored afterwards so middleware ordered before the app's UseRouting() still observes a null endpoint and the endpoint executes exactly once downstream. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the hand-rolled UseRouting() branch in AddCsrfProtectionMiddleware with the shared RerouteHelper.Reroute used by UsePathBase, UseStatusCodePagesWithReExecute, UseExceptionHandler and UseRewriter, the established precedent for middleware that may run before routing but needs the matched endpoint. Compiles src/Shared/Reroute.cs into the assembly. No behavior change; all DefaultBuilder tests remain green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Record why the auto-injected CSRF protection middleware uses the RerouteHelper precedent to match the endpoint on demand rather than a single-match alternative: CSRF is default-on so it cannot use the skip-when-explicit mechanism that lets UseAuthentication/UseAuthorization run in the source pipeline after the user's UseRouting(); the CanAddMiddlewareBeforeUseRouting contract forces restore-to-null (hence a second route match on the explicit-routing path); and the single-match alternatives either lack a positional-insert API, create a security hole when UseEndpoints() is called explicitly, or require core routing changes. Comment-only change; behavior unchanged. Build 0/0, 74 Csrf tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…f-named-cors-ordering # Conflicts: # src/DefaultBuilder/src/Internal/CsrfProtectionMiddleware.cs # src/DefaultBuilder/src/Microsoft.AspNetCore.csproj # src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/CsrfProtectionIntegrationTests.cs
After merging dotnet#67082 the CSRF middleware records a verdict on IAntiforgeryValidationFeature instead of short-circuiting. Update the in-code rationale to describe the wrong-verdict symptom and that matching the endpoint also sets the CsrfProtection sentinel EndpointMiddleware requires. Comment-only; behavior unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ut ordering (dotnet#67174) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes a pipeline-ordering bug where the default-on CsrfProtectionMiddleware can’t see per-endpoint CORS metadata (e.g., RequireCors("name")) when an app calls app.UseRouting() explicitly, causing allowed cross-origin POSTs to be incorrectly rejected.
Changes:
- Inject CSRF protection via a helper that can provide an on-demand endpoint matcher (via
RerouteHelper) when routing hasn’t run yet. - Update
CsrfProtectionMiddlewareto temporarily match an endpoint (when needed) to resolve per-endpoint metadata, then restore the pre-routing endpoint/route-values state. - Add
TestServerintegration tests covering explicit vs auto routing, allowed vs untrusted origins, and ensuring the endpoint executes only once.
Show a summary per file
| File | Description |
|---|---|
| src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/CsrfProtectionIntegrationTests.cs | Adds integration coverage for named CORS policies with explicit UseRouting() and regression guards. |
| src/DefaultBuilder/src/WebApplicationBuilder.cs | Changes CSRF auto-injection to optionally supply an endpoint-matching delegate when routing hasn’t executed yet. |
| src/DefaultBuilder/src/Microsoft.AspNetCore.csproj | Includes shared Reroute.cs to reuse the existing reroute/matching helper in this assembly. |
| src/DefaultBuilder/src/Internal/CsrfProtectionMiddleware.cs | Adds endpoint-resolution support and refactors verdict recording to work with temporary endpoint matching. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 1
|
I know this might sound a little weird, but since Then it just works everywhere, and not just with
I think the problem with this approach other than only working with Absent integrating the |
Alternative to the reroute approach (dotnet#67307). Instead of having the auto-injected CSRF middleware re-run routing to peek at the matched endpoint, the framework now defers its implicit auth/authz/CSRF middleware into a "post-routing block" when the app calls UseRouting() explicitly. EndpointRoutingMiddleware composes that block into its next delegate, so the implicit middleware run immediately after the endpoint is matched - identical to the order used when the framework injects UseRouting() itself. This dissolves the ordering dependency that caused dotnet#67174: a cross-origin POST to an endpoint with a named CORS policy (RequireCors) is no longer rejected, because CSRF observes the matched endpoint and resolves the named policy. It also makes per-endpoint authorization behave the same with explicit and implicit UseRouting(). - MiddlewareInvokedKeys: add PostRoutingMiddleware key (shared Routing/DefaultBuilder) - WebApplicationBuilder: in the explicit-UseRouting case, store the auth/authz/CSRF block instead of adding it to the destination pipeline - EndpointRoutingMiddleware: consume and compose the block after matching - CsrfProtectionMiddleware: revert to a plain middleware (no reroute/resolver) - Remove now-unused Reroute.cs from the DefaultBuilder build Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…net#67174) Reliability refinement of the deferred post-routing block. Only CSRF protection is deferred to run after the app's explicit UseRouting(); authentication and authorization keep their established position at the head of the destination pipeline, so their ordering is unchanged for every app (including the explicit-UseRouting case). This fully fixes dotnet#67174 - CSRF still observes the matched endpoint and resolves a named CORS policy (RequireCors("name")) - while avoiding any behavior change to auth/authz ordering. The post-routing block helper stays general so other middleware can opt into running after routing in future. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ock (dotnet#67174) Two review refinements: 1. CsrfProtectionMiddleware: revert to the dotnet#67082 baseline exactly. The deferred post-routing block guarantees routing runs before CSRF, so the middleware needs no dotnet#67174-specific changes. Removes the unnecessary RecordVerdictAsync extraction introduced during the earlier reroute exploration; InvokeAsync once again records the verdict and calls next inline. Zero diff versus dotnet#67082. 2. WebApplicationBuilder: the post-routing block carries authentication, authorization, and CSRF together (not CSRF alone). When the app calls UseRouting() explicitly, all three run immediately after the endpoint is matched - identical to the order used when the framework injects UseRouting() itself. This makes explicit and implicit UseRouting() behave the same and is what fixes the per-endpoint named CORS policy case (dotnet#67174). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…#67174) Auth/authz are security-critical and already work correctly in every case, so they keep their established position at the head of the destination pipeline - unchanged from main. Only CSRF protection is deferred into the post-routing block, because CSRF is the single middleware that must observe the matched endpoint (to read a per-endpoint CORS policy such as RequireCors("name"), dotnet#67174). Rationale: a user middleware placed before an explicit UseRouting() still sees auth-populated context.User exactly as today, and the existing loud guard in EndpointMiddleware (throws when an [Authorize] endpoint is hit without authorization having run on it) is preserved. This keeps the fix surgical and avoids any change to security-sensitive auth ordering. The post-routing block helper stays general (authn/authz/csrf parameters) so future middleware can opt into running after routing. CsrfProtectionMiddleware is unchanged from dotnet#67082. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…otnet#67174) Revert the authentication/authorization injection back to main's exact code (no addAuthentication/addAuthorization flags). Those middleware are unchanged by this fix, so their original blocks stay verbatim. The only change to ConfigureApplication is the CSRF injection point: when the app calls UseRouting() explicitly, store CSRF in the post-routing block so it runs after the matched endpoint; otherwise add it inline as before. CreatePostRoutingMiddleware is now CSRF-only (no auth/authz parameters), removing the confusing addAuthentication: false call site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…otnet#67174) Enforce default-on CSRF protection right after an endpoint is matched in EndpointRoutingMiddleware instead of via a separately injected CsrfProtectionMiddleware. This guarantees the matched endpoint is observed, so a per-endpoint CORS policy (e.g. RequireCors("name")) is honored, regardless of whether the app calls UseRouting() explicitly. Fixes dotnet#67174. - Remove CsrfProtectionMiddleware and the WebApplicationBuilder auto-inject block plus the __PostRoutingMiddleware deferral key. - Resolve ICsrfProtection in EndpointRoutingMiddleware; honor the DisableCsrfProtection switch via IConfiguration. - Move CsrfValidationException to src/Shared and compile it into Routing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Can we not make routing the dumping ground for every other thing? The current design is better and cleaner and allows for future additions to just work. |
…lock (dotnet#67174) When an app calls UseRouting() explicitly, the framework skips adding its own UseRouting() to the destination pipeline, so the implicit authentication, authorization and CSRF middleware ran before routing matched an endpoint. For CSRF this lost per-endpoint CORS policies (e.g. RequireCors("name")), causing a cross-origin POST to a named-policy endpoint to be wrongly rejected (dotnet#67174). Defer all three implicit middleware into a block that the app's explicit UseRouting() runs immediately after matching, mirroring the order the framework uses when it adds UseRouting() itself. The block is exposed as a stable, named method (PostRoutingPipeline.CreateMiddleware) rather than a closure, and EndpointRoutingMiddleware validates its method name, declaring assembly and public key token before invoking it, so application code cannot smuggle its own middleware into the framework-reserved __PostRoutingMiddleware slot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ed key, add trust-check coverage - Rename the reserved property key from __PostRoutingMiddleware to __PostRoutingPipeline (MiddlewareInvokedKeys.PostRoutingPipeline) to match the PostRoutingPipeline holder. - Collapse the duplicated auth/authz/CSRF registration into a single configurePostRouting delegate reused by both the direct (framework-owned UseRouting) and deferred (explicit UseRouting) paths; PostRoutingPipeline now takes that delegate instead of three boolean flags. - Add reliability tests for the EndpointRoutingMiddleware trust check: a same-shape delegate from another assembly is rejected, a non-delegate value is rejected, an application closure is rejected, and the framework's own block is accepted and runs the implicit middleware after routing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…thub.com/DeagleGross/aspnetcore into deaglegross/fix-csrf-named-cors-ordering
Compute each addAuthentication/addAuthorization/addCsrfProtection flag in the if condition that records the corresponding *MiddlewareSetKey, removing the separate flag declarations and follow-up if blocks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…thub.com/DeagleGross/aspnetcore into deaglegross/fix-csrf-named-cors-ordering
The delegate configures the framework's implicit authentication, authorization and CSRF middleware; the new name reflects that intent rather than the pipeline position. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…thub.com/DeagleGross/aspnetcore into deaglegross/fix-csrf-named-cors-ordering
|
As agreed offline, following the approach @javiercn suggested. I changed PR name and description to describe that we are not only changing the CSRF middleware mechanics, but auth/authZ as well |
Adds coverage proving the framework's implicit authentication and authorization still work when the app calls UseRouting() explicitly (the deferred post-routing block path), alongside the existing implicit routing coverage: - RegisterAuthMiddlewaresCorrectly_WithExplicitUseRouting - Authorization_IsEnforced_WithImplicitRouting / _WithExplicitUseRouting Adds ClaimsAuthHandler so the authorization deny path yields a real 401. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Renames the framework-reserved IApplicationBuilder.Properties key value from "__PostRoutingPipeline" to "__Internal_PostRoutingPipeline" to make it explicit that the slot is framework-internal and must not be overridden by application code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The default-on CSRF protection wrongly rejected a legitimate cross-origin
POSTto an endpoint with a named CORS policy (.RequireCors("name")/[EnableCors("name")]) — but only when the app calledapp.UseRouting()explicitly. In that case the framework's implicit middleware ran before routing matched, so the endpoint (and its CORS policy) wasn't visible yet.Fix
When the app calls
UseRouting()explicitly, defer the framework's implicit authentication, authorization, and CSRF middleware into a block that runs immediately after routing matches — mirroring the order used when the framework addsUseRouting()itself. With implicit routing, behavior is unchanged.The block is a stable, named framework method validated by
EndpointRoutingMiddleware(method name + assembly + public-key-token) before it runs, so application code can't inject its own middleware into the reserved slot.Tests
Integration tests in
CsrfProtectionIntegrationTests.cscover: explicitUseRouting()+ named policy + allowed origin → 200 (the repro), the implicit-routing regression case, untrusted origin still → 400, and the trust-check (closure / wrong-assembly / non-delegate rejected, framework block accepted).Fixes #67174