Currently when the admin pages are visited by anonymous users the admin theme is displayed. I would think this should not be the case as it makes the site look inconsistent to a user who might just have accidentally visited an incorrect URL. To get back to the rest of the site, users now have to fumble around a completely new interface to find any links. I know this has been jarring to me on occasion, even though I am used to the admin theme I setup.

The change would be for user access to be determined before a theme is switched.

I'm classifying this as a bug report, as I think this hurts usability of the site.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gdevlugt’s picture

I added two different patches for Drupal 7.x dev which both will set the theme for access denied pages to either the custom theme of the user, or the sites default theme. While both do the same, the first one fixes the issue by checking in init_theme() whether or not a 403 header is present. The second one solves things in drupal_access_denied() by determining which theme to use.

I'm not sure which one is the best approach. Personally I feel more for the first one since it doesn't duplicate code from init_theme, introduce globals into drupal_access_denied() and simply is smaller.

dvessel’s picture

Status: Needs review » Needs work

This should be done in system_init where the admin theme is initially picked out.

Susurrus’s picture

Would it not make more sense to do a strpos(drupal_get_headers(), '403 Forbidden') instead? I leave out the HTTP/1.1 designator as it's not really needed and may be a little too specific...

Susurrus’s picture

Status: Needs work » Needs review
FileSize
1.02 KB

Just added my suggestions in #3 to 1st patch provided in #1.

dvessel’s picture

Oo, you know, drupal_access_denied actually looks good as long as it's called before system_init. Haven't tested.

dvessel’s picture

Susurrus, I don't think that's a good place to do it. Other modules can set $custom_theme and you're forcing a specific behavior for 403's. Didn't look closely enough to be sure but $custom_theme should be set before init_theme I think.

Susurrus’s picture

I'm not sure where the best place to put this code is, as I'm not familiar with this part of core. I just made a small change to the code that checks for a 403 error and just submitted the patch that you initially suggested. So which one should we go with?

gdevlugt’s picture

The patch, after reading the original issue again and the new comments, I think would need to do the following :

  1. First of all, only apply to the administrative theme set (and the node add and edit theme if set). This basically is the set of pages system_init checks (/admin/* , node/add/* and node/edit/*.
  2. If so, check if the user (anonymous or not) has access.
  3. If not, set the theme back to the user theme or the default site theme.

The patches I provided however were, concerning point 1 a bit less restrictive since it would switch back on every user access denied 403. Whether or not this is good depends on contrib modules setting a custom theme. In some cases you don't want the custom theme to show, but in others perhaps you do want it. What it should be is up to others (not me) to decide.

If you only want it in the admin pages, ideally, system_init like dvessel suggested would be the place since this is the place where the administrative theme gets set. However, at that point it's unclear if the user has access or not. A check for 403 won't work there since it gets set later. Also, a check through user_access isn't an option since there are a lot of core admin permissions, and any contrib module can add admin permissions which also needs to be checked. Unless there's a function which can check if the user has access to a certain path, I'm not sure if its worth checking each permission just to switch a theme back.

I would be happy to provide a patch, but first I'd like some more input on what approach should be taken.

dvessel’s picture

I'm not certain but isn't there a menu function for checking access permissions? If so, then it can be done with that. Wouldn't need to check for 403's specifically in system_init. Chx would know this.

vladimir.dolgopolov’s picture

This is the test for the issue.
Patches #1 pass it.
Patch #4 fails it.


class TestCase221352 extends DrupalTestCase {
  function get_info() {
    return array(
      'name' => t('[221352] Only display administration theme to user\'s with access'),
      'desc' => t('Currently when the admin pages are visited by anonymous users the admin theme is displayed. I would think this should not be the case as it makes the site look inconsistent to a user who might just have accidentally visited an incorrect URL. To get back to the rest of the site, users now have to fumble around a completely new interface to find any links. I know this has been jarring to me on occasion, even though I am used to the admin theme I setup.'),
      'group' => t('Drupal 7 Tests'),
    );
  }

  // The test setup different themes: admin_theme and theme_default.
  // Check for admin_theme for admin. Should pass.
  // Check for theme_default (not admin_theme) for anonymous user. It should pass, but it fails for now.
  // After the test a theme will be switched to $theme_default.
  // It's a bug feature, click any link and things back to normal.
  function testIssue() {
    $theme_default = 'pushbutton';
    $admin_theme = 'garland';
    $admin_path = 'admin/settings/admin'; // admin pages for example

    $this->drupalVariableSet('admin_theme', $admin_theme);
    $this->drupalVariableSet('theme_default', $theme_default);

    // Create a new admin with $admin_theme (differ from $theme_default)
    $admin_user = $this->drupalCreateUserRolePerm(array('access administration pages', 'administer site configuration'));
    user_save($admin_user, array('theme' => $admin_theme));
 
    $this->drupalLoginUser($admin_user);
    $this->drupalGet($admin_path);
    
    // is there $admin_theme? (simple check .css presence)
    $this->assertWantedRaw('<link type="text/css" rel="stylesheet" media="all" href="/themes/' . $admin_theme . '/style.css', 'Check for "Administration theme" for admin');
    $this->drupalGet('logout');

    // anonymous user
    $this->drupalGet($admin_path);

    // is there still $admin_theme? (simple check .css presence)
    $this->assertNoUnwantedRaw('<link type="text/css" rel="stylesheet" media="all" href="/themes/' . $admin_theme . '/style.css', 'Check for "Administration theme" for anonymous user');
    // is there $theme_default? (simple check .css presence)
    $this->assertWantedRaw('<link type="text/css" rel="stylesheet" media="all" href="/themes/' . $theme_default . '/style.css', 'Check for "default" theme for anonymous user');

  }

}

Concerning 403 check this out: Custom 403 page not available to user who doesn't have 'access content' permissions

Robin Monks’s picture

Status: Needs review » Needs work

Based on comments above.

Robin

open-keywords’s picture

Please also consider this discussion http://drupal.org/node/242200 (edit operations and trouble in block module )

effulgentsia’s picture

Status: Needs work » Fixed

If I understand this issue correctly, then I believe it's fixed, in that if an anonymous user goes to /admin, the access denied message is displayed using the default theme, not the admin theme.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

davidwhthomas’s picture

Here's a small module snippet to force the default theme on admin pages to anonymous users:

<?php
/**
 * Implementation of hook_init
 */
function example_init() {
  global $custom_theme;
  // Return 404 for admin pages to anon users - use default site theme
  if ( arg(0) == 'admin' && ! user_is_logged_in() ) {
    $custom_theme = variable_get('theme_default', 'garland');
  }
}
?>

Just a workaround for those looking for an immediate solution.

DT

edgartanaka’s picture

In Drupal 6, my solution was:
- create a page for access denied with PHP formatting type. You want to add the following piece of code in the beggining of the body:

global $custom_theme;
$custom_theme = 'mytheme'; //supposing your site´s theme name is 'mytheme'

- set the URL to the 403 page you created in /admin/settings/error-reporting

edgartanaka’s picture

My solution was:
- create a page for access denied with PHP formatting type. You want to add the following piece of code in the beggining of the body:

global $custom_theme;
$custom_theme = 'mytheme'; //supposing your site´s theme name is 'mytheme'

- set the URL to the 403 page you created in /admin/settings/error-reporting