[6.x] Run custom API middleware before the response cache - #15385
[6.x] Run custom API middleware before the response cache#15385duncanmcclean wants to merge 5 commits into
Conversation
the `statamic.api.middleware` group and graphql schema middleware now run before `HandleAuthentication` and the cache middleware, so cached responses can no longer bypass custom authentication. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015MAdoF7RTBQ4mUTaki7k2e
|
Test failures are unrelated. Fixed in #15387 |
jasonvarga
left a comment
There was a problem hiding this comment.
Nice catch on the underlying bug — a cached response being served before a middleware-based auth layer runs is a genuine hole, and the direction here is right. Two things to sort before this goes in.
Separately, worth calling out in the release notes: because Cache is now the innermost middleware, cache hits also run SubstituteBindings, so every cached REST request now resolves its route bindings (Collection::findByHandle, taxonomies, asset containers, globals, forms, sites — see RouteServiceProvider) against the Stache before the cached response is returned. That isn't in the behaviour-change list in the description, and it does eat into the point of the response cache.
only the custom middleware needs to run before `Cache`. moving `HandleAuthentication` inside the configured group meant `HandleToken` ran before the `auth_token` check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01624MAUMx1KeFxpnqwivkiA
only `CacheResponse` needs to move behind the custom middleware. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01624MAUMx1KeFxpnqwivkiA
…areTest` the config test was pointed at the `GraphQL::addMiddleware()` environment, leaving `addCustomMiddlewareThroughConfig` unused. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01624MAUMx1KeFxpnqwivkiA
This pull request fixes an issue where custom middleware configured via
statamic.api.middleware(orstatamic.graphql.middleware) couldn't be used to authenticate requests, because a cached response would be returned before the custom middleware ran. Once an authenticated request had populated the cache, unauthenticated requests to the same endpoint received the cached response.This was happening because the configured middleware group was nested inside the
Cachemiddleware in the REST API route group, and the GraphQL schema appended custom middleware afterCacheResponse.This PR fixes it by running the configured middleware after
HandleAuthenticationbut before the cache middleware in both the REST API and GraphQL. The$middlewarePriorityworkaround from the issue continues to work.Behaviour changes
apigroup includesThrottleRequests, so cached responses now count towards the rate limit.GraphQL::addMiddleware()as well asstatamic.graphql.middleware.Cacheis now the innermost middleware, cache hits also runSubstituteBindingsfrom theapigroup, so route bindings (collections, taxonomies, asset containers, globals, forms, sites) are resolved against the Stache before the cached response is returned.I don't think any of these are unreasonable, since the config file describes
statamic.api.middlewareas a place to "configure a middleware-based authentication layer", which wasn't possible before. But feel free to close if you consider this breaking.Fixes #10010