Posted by markus_petrux on August 28, 2009 at 3:01pm
7 followers
| Project: | Administration menu |
| Version: | 7.x-3.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | major |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | D7 stable release blocker |
Issue Summary
Admin menu does not change when roles affecting a particular user change. I think it is because the cache identifier for the menu does not take into account user roles. It could also be addressed clearing caches when user roles are updated, for example.
In any case, users may need to use something they have access but it is not visible in the admin menu, or they may see menu items they do not have access after roles have changed.
Comments
#1
Subscribing.
#2
#613668: Empty menu item for admin/user/profile additionally mentions that admin_menu contains stale links after uninstalling a module.
#3
Subscribing, too.
Someone should debug, please.
#4
#5
See possible duplicates
#1322942: "Web Master" elevated role can only see home icon, 0/1, hello [username] and logout.
#1337864: admin menu doesn't take consideration for users' permissions
#6
I think there are two parts to this issue, first is a user's role(s) changing and second is permissions for a role changing. To address the first part we can use the following code.
<?php/**
* Implements hook_user_presave().
*/
function admin_menu_user_presave(&$edit, $account, $category) {
// If a user role changes clear the admin menu cache for that user's session.
if ($category == 'account' && $account->uid != 1) {
// Check for role change
$old_roles = array_keys(array_filter($account->roles));
$new_roles = array_keys(array_filter($edit['roles']));
sort($old_roles);
sort($new_roles);
if ($old_roles != $new_roles) {
// Find the session IDs for that user.
$sessions = db_query("SELECT sid FROM {sessions} s WHERE s.uid = :uid", array(':uid' => $account->uid))->fetchCol(0);
// Clear the cache_menu table for that user.
foreach ($sessions as $session) {
cache_clear_all('admin_menu:' . $session . ':', 'cache_menu', TRUE);
}
// Clear cache_admin_menu table if it exists.
if (db_table_exists('cache_admin_menu')) {
foreach ($sessions as $session) {
cache_clear_all('admin_menu:' . $session . ':', 'cache_admin_menu', TRUE);
}
}
// Reset the static cache.
drupal_static_reset('admin_menu_cache_get');
}
}
}
?>
I think there must be a more efficient way to compare user roles, but this is the best I could come up with. I also wonder if we ought to modify the
admin_menu_flush_caches()function to accept a user id or a session id to perform the bottom half of the above function. Then instead we could simply calladmin_menu_flush_caches($account->uid);when the role change is detected.I also wonder if it's legal in Drupal to manually clear the cache tables with a db_query(), in case the user has many sessions we can just do one query instead of many.
For the second part of the issue, permission changes for a role, I believe we can implement hook_user_role_update(). If not we would probably have to hook_form_alter() the permissions page and add a custom submit handler which sounds a lot more painful.
#7
Ok so some further digging shows that submitting the permissions form already calls cache_clear_all() so we shouldn't have to do anything special in admin_menu when permissions change, only when a user role changes. Therefore I would like some feedback on the approach in my comment above and will then be happy to submit a patch.
#8
Also, the comment in #2 no longer applies to D7 since
drupal_flush_all_caches()is called in thesystem_modules_submit()function therefore #613668: Empty menu item for admin/user/profile could potentially be reopened for D6 only?#9
Attached patch fixes this issue and proves that in a test.
#10
The last submitted patch, admin_menu.perm-changes.9.patch, failed testing.
#11
I can confirm that #9 works for editing a user's roles, editing the permissions for a role and deleting a role.
#12
Ran the tests locally, due to that testbot hiccup - which revealed that the test changes broke another/existing test. Fixed that, and,
Thanks for reporting, reviewing, and testing! Committed to all branches.
A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.
#13
Automatically closed -- issue fixed for 2 weeks with no activity.