Authorization-invoked marker is not endpoint-specific
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 290
Description
## Summary
The internal marker that records "authorization middleware observed an endpoint" is a shared, non-endpoint-specific sentinel, so a later safety check can only confirm that authorization ran against *some* endpoint during the request, not the endpoint that is about to execute.
## What is wrong
* `AuthorizationMiddleware` stores a single shared object under a well-known `HttpContext.Items` key whenever it observes any endpoint, rather than recording which endpoint it evaluated.
* The corresponding safety check in `EndpointMiddleware` only tests whether that key is present, not whether the recorded value corresponds to the endpoint currently executing.
## Why it matters (defense in depth)
* This weakens an existing fail-closed safeguard into a fail-open one for any code path where the executing endpoint can change after authorization has already run once during the same request. The safeguard's intent is to catch endpoints whose authorization requirements were never evaluated; a non-endpoint-specific marker cannot fully honor that intent.
* Hardening this independently of any specific triggering scenario reduces reliance on every current and future code path that can change the executing endpoint mid-request to correctly replay authorization middleware on its own.
## Affected code
* src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs:29-104 - stores a shared sentinel object rather than the observed `Endpoint`
* src/Http/Routing/src/EndpointMiddleware.cs:13-56 - safety check compares only key presence, not the recorded value against the endpoint about to execute
* src/Http/Routing/test/UnitTests/EndpointMiddlewareTest.cs:225-260 - existing test arranges the marker as a boxed `bool`, not an `Endpoint`, and will need updating for the stricter check
## Recommended fix
Store the actual `Endpoint` reference under the marker key instead of a shared sentinel, and change the safety check to a reference-equality comparison against the endpoint that is about to execute, so the check verifies "authorization ran against this endpoint" instead of "authorization ran against some endpoint."
Alternatives considered:
* Leaving the marker as-is and relying entirely on every code path that can change the executing endpoint mid-request to independently replay authorization middleware correctly — rejected, since it provides no independent safety net if any current or future such path gets this wrong.
Compatibility: internal-only change; the marker key, sentinel, and comparison logic are internal/private on both sides, so there is no public API impact. One existing internal unit test needs its arrange step updated to store an `Endpoint` instead of a boxed `bool`.
## Acceptance criteria
* [ ] The safety check throws when an endpoint with authorization metadata executes without authorization having run against that specific endpoint, even if authorization ran against a different endpoint earlier in the same request.
* [ ] Existing behavior is unchanged for the common case where authorization ran against the endpoint that is now executing.
* [ ] Existing tests continue to pass, with arrange steps updated as needed.
## Related
* #68581 - root-cause fix so authorization re-runs after a mid-pipeline reroute in implicit-routing minimal hosting
Contributor guide
Research direction
Start with src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs and src/Http/Routing/src/EndpointMiddleware.cs to trace how the internal marker is stored and checked. Update the related arrange step in src/Http/Routing/test/UnitTests/EndpointMiddlewareTest.cs, then run the EndpointMiddleware tests. Done means the check distinguishes the currently executing endpoint from a different endpoint previously observed while preserving the common case.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- api, backend, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100