Contrib.social

Faulty toolbar subtree hash breaks asynchonous loading of admin menu content

Open on Drupal.org →
Created on 23 April 2020, over 4 years ago
Updated 5 August 2024, 3 months ago

Problem/Motivation

The toolbar subtrees use a hash to determine if access to the subtree should be allowed in ToolbarController::checkSubTreeAccess(). In certain situations the hash does not match, resulting in a JavaScript error and the subtrees not dynamically loading. Temporarily commenting out the hash check from ToolbarController::checkSubTreeAccess() makes the toggles appear again.

The toolbar module relies on _toolbar_get_subtrees_hash function to generate hash based on admin menu subtrees' length. This information is not same when you call it via `hook_toolbar` and via /toolbar/subtrees Ajax route. This difference is due to multiple reasons, few are:
1. If you are on admin pages, subtree uses stable template to generate html while Ajax uses toolbar module template
2. If there is a customization like adding destination to any of the link then hook_toolbar attaches current page url while Ajax attaches /toolbar/subtrees as destination

Steps to reproduce

  1. Set the admin menu in vertical mode.
  2. Make sure to have a site with 2 languages.
  3. Enable the Account administration pages on Language negotiation > Detection and selection.
  4. Set the default language to something non-english.
  5. Set an admin language in your user profile other than the default site language (e.g. english).
  6. Go to a content page in your content language.
  7. Refresh the cache (I did this from the content page with devel module).
  8. Navigate to an admin page.
  9. Observe that the toolbar submenu toggles (blue arrows) are not shown and there is a JavaScript error.

Proposed resolution

The hash is calculated from the rendered subtrees, but if the rendered content changes, the hash no longer matches with the sent one and the access check fails.

Proposed solution is to use $hash = Crypt::hashBase64(serialize(array_keys($subtrees))); instead of $hash = Crypt::hashBase64(serialize($subtrees));. So that the hash is based on structure, rather than content.

Remaining tasks

  1. Understand the purpose of the hash(?). Is it related to CSRF, MITM, permissions?
  2. Agree on an alternative hash input.

User interface changes

Hopefully none

API changes

Probably none

Data model changes

Release notes snippet

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Toolbar  →

Last updated 10 days ago

  • Maintained by
  • 🇫🇷France @nod_
Created by

🇮🇳India ash2303

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024