Document and guard against known tricky middleware-ordering traps (output caching, diagnostics re-execution)
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 290
Description
## Summary
Several distinct, security-relevant defects in ASP.NET Core apps share a single root cause: **wrong middleware ordering in the request pipeline**. They compile cleanly, produce no error by default, and can lead to information disclosure or authorization bypass. This issue proposes that we (1) document the known traps, (2) consider build-time and/or runtime guardrails, and (3) provide AI-assisted authoring guidance.
The common thread: ordering bugs have **no compiler signal**, are often **security-relevant**, and only **one** ordering rule is enforced today (`ASP0001` for `UseAuthorization` placement). Other risky orderings rely entirely on developer knowledge, and in at least one case the official docs show the risky order.
## Known traps
### 1. `UseOutputCache()` before `UseAuthentication()`/`UseAuthorization()`
```csharp
app.UseRouting();
app.UseOutputCache(); // BEFORE auth
app.UseAuthentication();
app.UseAuthorization();
```
The default output-cache policy decides whether to cache by checking the `Authorization` request header and `HttpContext.User?.Identity?.IsAuthenticated`. Cookie authentication sets no `Authorization` header, and when `UseOutputCache()` runs *before* `UseAuthentication()`, `IsAuthenticated` is still `false`. Both guards pass, so a response for an authenticated user can be cached and replayed to other users.
Contributing factors:
- The Output Caching docs sample shows `UseOutputCache()` **before** `UseAuthorization()` (`UseHttpsRedirection(); UseOutputCache(); UseAuthorization();`).
- The only documented ordering constraints are "after `UseCors`" and "after `UseRouting`" — nothing about authentication/authorization.
- Output caching is **absent from the canonical numbered middleware-order list**.
- There is no analyzer, startup filter, or runtime validation for its placement.
Correct order: `UseOutputCache()` after `UseAuthentication()`/`UseAuthorization()` (and still after `UseRouting`/`UseCors`) for authenticated apps.
### 2. `UseExceptionHandler("/path")` / `UseStatusCodePagesWithReExecute("/path")` after `UseAuthorization()`
```csharp
app.UseRouting();
app.UseAuthentication();
app.UseAuthorization();
app.UseExceptionHandler("/error"); // AFTER auth
app.UseStatusCodePagesWithReExecute("/status"); // AFTER auth
```
Pipeline re-execution only re-runs middleware registered **after** the diagnostics middleware. When the diagnostics middleware is registered *after* `UseAuthorization()`, a re-executed request to a protected error/status endpoint does not flow back through authorization for the newly-selected endpoint, so a `[Authorize]` policy on that endpoint may not be enforced.
The canonical middleware-order list already places exception/error handling **first** (before routing/auth), but the *reason* — re-execution re-runs only downstream middleware — is not stated, so a developer who reorders for any reason can introduce a regression without realizing it. The docs also note that custom error pages should support anonymous access, implying re-execution targets should be anonymous-safe and carry no secrets.
Correct order: register error/diagnostics middleware **early** (before `UseRouting`/`UseAuthentication`/`UseAuthorization`), so auth is downstream and re-runs for the re-executed endpoint.
## Why this is worth addressing centrally
- No compiler signal — reordering two lines compiles and runs.
- Security-relevant and often silent (no exception/log by default).
- Docs sometimes show the risky order (trap 1) or omit the *reason* for the safe order (trap 2).
- Only `ASP0001` enforces any ordering today.
## Proposed options (not mutually exclusive)
**Documentation**
- Add output caching to the canonical middleware-order list with explicit guidance to place it after `UseAuthentication`/`UseAuthorization` for authenticated apps; annotate/fix the sample that shows it before auth.
- Add a callout in the error-handling article explaining *why* re-execution middleware must be early (it only re-runs downstream middleware) and that re-execution targets must support anonymous access and carry no secrets.
**Analyzers** (mirroring `ASP0001` / `UseAuthorizationAnalyzer`)
- Warn when `UseOutputCache()` is registered before `UseAuthentication`/`UseAuthorization` in an app that also calls auth.
- Warn when `UseExceptionHandler(path)` / `UseStatusCodePagesWithReExecute(path)` is registered after `UseAuthorization()`.
**Runtime defensive defaults**
- Consider having the output-cache default policy inspect endpoint authorization metadata (available after `UseRouting`) and refuse to share-cache protected endpoints regardless of middleware order, with an opt-out.
**AI authoring guidance**
- A middleware-order skill / `Program.cs` instructions that flag these orderings during AI-assisted editing, including helper-method indirection that static analyzers may miss.
## References
- Middleware order: https://learn.microsoft.com/aspnet/core/fundamentals/middleware/?view=aspnetcore-10.0#middleware-order
- Output caching: https://learn.microsoft.com/aspnet/core/performance/caching/output
- Handle errors / re-execution: https://learn.microsoft.com/aspnet/core/fundamentals/error-handling?view=aspnetcore-10.0
- ASP0001 analyzer precedent: https://learn.microsoft.com/aspnet/core/diagnostics/asp0001?view=aspnetcore-10.0
Contributor guide
Research direction
Read the linked middleware-order, output-caching, error-handling, and ASP0001 documentation, then inspect the existing UseAuthorizationAnalyzer precedent. Done requires selecting and scoping the documentation, analyzer, runtime, or AI-guidance work and adding the corresponding validation, without leaving the proposed options unresolved.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend-api-design, devtools, documentation, security
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100