Problem/Motivation
GeneratedUrl objects can have cache metadata.
When generating a URL for a route with the _csrf_token requirement, Drupal automatically adds a token GET param containing a CSRF token.
In the context of a non-HTML response the GeneratedUrl object does not have any cache context so it risks being cached incorrectly.
Steps to reproduce
Create JSON controller that returns this:
$url = Url::fromRoute('user.logout')->toString(TRUE);
$response = new CacheableJsonResponse([$url->getGeneratedUrl()]);
$response->addCacheableDependency($url);
return $response;
(Note that you have to call it with ?_format=json if you want the real CSRF token and not a placeholder.)
Proposed resolution
Add the session cache context to the URL object when adding a CSRF token to the URL.
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Comments
Comment #2
prudloff commentedComment #3
prudloff commentedMy bad, just noticed that in HTML responses the token in the URL is a placeholder that is replaced when rendering the page.
However the problem exists in non-HTML responses.
Comment #5
prudloff commentedTests reveal this impacts some JSON/XML config entity routes because they return a Link HTTP header with the enable/disable URLs.
However I don't understand the remaining failures (in BlockJsonBasicAuthTest for example).
It seems simply adding the cache context makes the generated token different:
Comment #7
prudloff commentedThe problem was that the test assumes that the response is cacheable by dynamic page cache so it reuses the headers from the first request to test if they are the same in the seconds request.
But adding the session cache context disables dynamic page cache so this does not work. The test already has some conditions to use a different logic when the response is not cacheable in dynamic page cache so I also did this.
Comment #8
smustgrave commentedThink this needs a CR, since multiple tests had to be updated to pass there is a chance it could break contrib. May seem overkill.
Comment #9
catchIs this the same as #3528434: \Drupal\Core\Access\RouteProcessorCsrf does not add the correct cache context when generating non html responses ?
Comment #10
prudloff commentedYes I think it is a duplicate.
Comment #11
prudloff commentedIt looks like this was fixed in #3587298: RouteProcessorCsrf::processOutbound() should bubble 'session' cache context for non-HTML requests.