backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

[UX] Dashboard: Make sure things look decent for users with lesser permissions (no access to admin theme) #4010

Open klonos opened 5 years ago

klonos commented 5 years ago

This was discovered while testing #3994

...it works fine when viewed by an admin account, but certain issues start coming up when accessing the Dashboard as a user with less permissions...

I created a new user, and gave it just the "authenticated" role. I then logged in, and (as expected), I saw nothing ๐Ÿ‘ ...moving on, I gave the "authenticated" user role the "Access Dashboard" permission, and also the "Access administration bar" permission -> refreshed the page -> clicked the "Dashboard" link in the admin bar -> realised that the Dashboard was not built with other themes in mind, except Seven ๐Ÿ˜…:

2019-09-02-00-25-2836 backdrop backdrop qa backdropcms org

...perhaps we need to make the "Access Dashboard" permission work the same way as the "Access administration bar" permission, where a pop-up suggests that we also provide the "Use the administration pages" + the "View the administration theme" permissions (I believe that @stpaultim has mentioned something similar during his work in additional blocks for the Dashboard)? But that is definitely a separate issue - just mentioning it here so it doesn't get lost in time.

ghost commented 5 years ago

Is there a good reason for not just giving authenticated users the 'view admin theme' permission by default...?

klonos commented 5 years ago

In my quest to see when the view the administration theme permission was first introduced (to better understand what the need for it was), I found this:

/**
 * Allow all users to view the administration theme.
 */
function system_update_7067() {
  // Preserve the site's current behavior of automatically allowing all users
  // to view the administration theme whenever they have access to an
  // administrative page.
  user_role_grant_permissions(DRUPAL_ANONYMOUS_RID, array('view the administration theme'));
  user_role_grant_permissions(DRUPAL_AUTHENTICATED_RID, array('view the administration theme'));
}

...although the current incarnation of that update hook is different:

/**
 * Grant administrators permission to view the administration theme.
 */
function system_update_7067() {
  // Users with access to administration pages already see the administration
  // theme in some places (if one is enabled on the site), so we want them to
  // continue seeing it.
  $admin_roles = user_roles(FALSE, 'access administration pages');
  foreach (array_keys($admin_roles) as $rid) {
    _update_7000_user_role_grant_permissions($rid, array('view the administration theme'), 'system');
  }
  // The above check is not guaranteed to reach all administrative users of the
  // site, so if the site is currently using an administration theme, display a
  // message also.
  if (variable_get('admin_theme')) {
    if (empty($admin_roles)) {
      drupal_set_message('The new "View the administration theme" permission is required in order to view your site\'s administration theme. You can grant this permission to your site\'s administrators on the <a href="' . url('admin/people/permissions', array('fragment' => 'module-system')) . '">permissions page</a>.');
    }
    else {
      drupal_set_message('The new "View the administration theme" permission is required in order to view your site\'s administration theme. This permission has been automatically granted to the following roles: <em>' . check_plain(implode(', ', $admin_roles)) . '</em>. You can grant this permission to other roles on the <a href="' . url('admin/people/permissions', array('fragment' => 'module-system')) . '">permissions page</a>.');
    }
  }
}

Interesting! ๐Ÿค”

Anyway, the d.org issue that seems to have introduced this permissions is https://www.drupal.org/node/669510 ...the commit mentions @quicksketch, so he may have a better understanding of the reason.