Output Caching DefaultPolicy allows bypassing cache with any Authorization header even if there's no [Authorize] attribute specified
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 290
Description
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Describe the bug
The title pretty much says it all.
There is related issue https://github.com/dotnet/aspnetcore/issues/45469 which pretty much describes this bug/feature but I'd like to focus on the other aspect of the problem.
Output caching was introduced in .net7 as more reliable alternative to response caching. One of it's core features was that unlike response caching it couldn't be disabled by sending `max-age=0` or `no-cache` in `Cache-control` header.
Default policy for output caching has this check that seems a bit doubtful:
https://github.com/dotnet/aspnetcore/blob/e0e1d582a993feddf346e7224c7fbd712376d91f/src/Middleware/OutputCaching/src/Policies/DefaultPolicy.cs#L76
This check passes if there's `Authorization` header present **regardles of it's content and of controller's requirements**, which means any output cache can be effectively bypassed by adding an `Authorization` header to any request.
The question is: should it behave like this **by default?**
Yes, I know that
> authenticated requests are not cached by design
_Originally posted by @sebastienros in https://github.com/dotnet/aspnetcore/issues/45469#issuecomment-1338323610_
but the presence of a single specific header does not make the request authenticated!
Yes, I know that I can configure my own caching policy but shouldn't something like this be a default?
### Expected Behavior
In my humble opinion the correct condition for this chek should be something like:
```csharp
if (request.HttpContext.User?.Identity?.IsAuthenticated == true)
{
return false;
}
```
In case of someone putting the correct Bearer Token in a header by mistake. It works only if `[Authorize]` attribute is present on a controller action. Otherwise `HttpContext.User` is not authenticated. However this solution has a downside as it will only work with minimal API or if the api controller itself does not have the `[Authorize]` attribute.
But regardless of `[Authorize]` attribute this also prevents the malicious use of invalid `Authorization` header to purposely perform a Thundering Herd attack.
### Steps To Reproduce
1. Create a controller action or a minimal API endpoint with Output caching configured. (The controller **and** the action should **not** have `[Authorize]` attribute if you plan to use valid jwt authentication bearer in the next step).
2. Make a request to an endpoint with the Authorization header present (the value of header doesn't matter). Observe that the response is not cached.
3. Make the same request to the same endpoint without the Authorization header present. Observe that it will be cached.
### Exceptions (if any)
_No response_
### .NET Version
8.0.100
### Anything else?
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.