Problem
Both, the core menu linkset, and the REST menu items module face the same issue: Since Drupal 10.3/11 the user logout link contains a CSRF token. However, the token provided by the API is invalid.
Note that this worked fine for Drupal 10.2 and before, so this is a regression that causes decoupled solutions that fetch the "account" menu to break.
Related issue for REST menu items module: #3484945: Wrong user logout CSRF token
Steps to reproduce
* Access the "account" menu
* Try using the logout link, when the token is valid, it should work without confirmation form.
* The token is not valid, a confirmation form is shown
The token does not work, because it's not the token. The value for the token is the render-placeholder, but the placeholder never gets replaced. API modules rendering to JSON to not use the render system, thus never get placeholders replaced. Still, RouteProcessorCsrf() placeholders the value.
Analysis
The problematic logic is in `\Drupal\Core\Access\RouteProcessorCsrf::processOutbound`.
// Adding this to the parameters means it will get merged into the query
// string when the route is compiled.
if (!$bubbleable_metadata) {
$parameters['token'] = $this->csrfToken->get($path);
}
else {
// Generate a placeholder and a render array to replace it.
$placeholder = Crypt::hashBase64($path);
$placeholder_render_array = [
'#lazy_builder' => ['route_processor_csrf:renderPlaceholderCsrfToken', [$path]],
];
....
However, the assumption "we have cachemetadata, thus we can use placeholders" is wrong. JSON API response should use cache metadata, but cannot use render API placeholders. Not sure what else would be a good indicator on when to placeholder it, but the current one isn't it.
To reproduce, a simple generated URL like
> \Drupal\Core\Url::fromUri('internal:/user/logout')->toString();
will generate an invalid token. (note that you need to use the right session also!)
Proposed solution
Improve the logic of when to generate a placeholder for CSRF tokens. It's safe to assume non-html requests do not render with the render API, thus the proposed solution is to never placeholder tokens on non-html requests. The MR implements the solution and makes sure it works for core-linkset responses + adds test coverage.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | interdiff_7-9.txt | 2.36 KB | arthur_lorenz |
| #9 | 3485174-9.patch | 15.88 KB | arthur_lorenz |
Issue fork drupal-3485174
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
fagoIt's probably safe to assume that every non-html request is not going to use the render sytem and thus a placeholder should not be created. Not perfect, but a better check than it was before.
This works for rest_menu_items and for core-linkset endpoint once it's correctly specified as json. Patch attached. Will work on a test also.
Comment #4
fagoComment #5
smustgrave commentedLeft comments on MR.
Comment #7
arthur_lorenz commentedAdressed @smustgrave's comments.
Comment #8
smustgrave commentedLeft another comment on the MR.
Thanks!
Comment #9
arthur_lorenz commentedThanks, updated the deprecation message and added a test.
Comment #10
fagoThank you! Additions are good, also all comments have been addressed now. So I think this is ready!
Comment #12
quietone commentedThis looks fine. I updated credit.
Comment #13
alexpottCommitted ae69922 and pushed to 11.x. Thanks!
Also backported without the deprecation error to 11.1.x, 10.5.x, and 10.4.x.
Committed and pushed ec91b0d0cd0 to 11.1.x and 438a6e0485d to 10.5.x and eb59afdbf5c to 10.4.x. Thanks!