In the latest version of Panelizer, there's a mismatch in permissions between what Panelizer provides and what the IPE provides. Some access configurations can cause the IPE buttons to appear but not work (they generate the "You are not authorized to access this page" alert). I think this is because Panelizer's implementation of hook_panels_ipe_access is too broad. If a certain role has edit access to a content type, but the Panelizer permissions that are required to use the IPE on it are not granted, the buttons will appear when they shouldn't.

Versions in use:
CTools 7.x-1.11+0-dev
Panels 7.x-3.8+0-dev
Panelizer 7.x-3.4+15-dev

To reproduce:

Setup

  1. Install latest Drupal core.
  2. Download and install the latest dev versions of CTools, Panels, and Panelizer, including Page Manager and the Panels IPE.
  3. Log in as an administrator.
  4. Enable "node_view" page in Page Manager.
  5. For both the Basic page and Article content types, enable Panelizer.
  6. Check these boxes:
    • Panelize
    • Full page override
    • Provide an initial display named Default
  7. In the Panelizer settings page for both Article and Basic page, set the full page override renderer to In-place-editor.
  8. Create an Article node.
  9. Create a Basic page node.

Roles and permissions

  1. Grant all permissions to the administrator role
  2. Create a role called "editor" and grant it some permissions:
    • Node: Grant all node creating, editing, and deleting permissions for Articles and Basic pages to the Editor role.
    • Panels: Grant Use the Panels IPE and Change layouts with the Panels IPE to the Editor role.
    • Panelizer: Grant the Administer Panelizer layout and content permissions for Article nodes to the Editor role. Do not grant panelizer permissions to Basic pages.
  3. Create a user with the editor role.

Demonstrate the issue

  1. Log in as the editor.
  2. Visit the Article node.
  3. Note the IPE buttons appear at the bottom of the screen.
  4. Click the Customize this page button.
  5. Note the IPE interface displays.
  6. Click the Cancel button.
  7. Visit the Basic page node.
  8. Note the IPE buttons appear at the bottom of the screen. Expected behavior: Because editors are not granted the Panelizer Administer Content or Layout permissions, the buttons should not appear.
  9. Click the Customize this page button.
  10. Note you see the JS alert "You are not authorized to access this page."

The code in question is here:

/**
 * Implements hook_panels_ipe_access().
 */
function panelizer_panels_ipe_access($display) {
  // We only care about Panels displays from panelizer.
  if (isset($display->context['panelizer'])) {
    // The type array contains 3 elements, where the first is the full context
    // type (ie. 'entity:ENTITY_TYPE'), and the remaining two are the parts
    // seperated by ':', so 'entity' and the entity type name.
    $entity_type = $display->context['panelizer']->type[2];
    if ($handler = panelizer_entity_plugin_get_handler($entity_type)) {
      // Only allow access to use the IPE if the user has 'update' access to
      // the underlying entity.
      $entity = $display->context['panelizer']->data;
      return $handler->entity_access('update', $entity);
    }
  }
}

What this does is allow or deny access to the IPE strictly based on whether the current user has edit access to the node. If the current user has edit access but not the Administer Panelizer Content and/or Layout permissions, this hook causes the buttons to be displayed. But the Panelizer permissions kick in when you click the buttons, thus you get MENU_ACCESS_DENIED as a result of the page callback and you see the alert.

Instead, the hook should probably return FALSE if you don't have edit access, and NULL in all other cases.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cboyden created an issue. See original summary.

cboyden’s picture

Assigned: cboyden » Unassigned

This looks more complicated. It looks like Panelizer will have to add checks for Administer content and layout permissions into its hook_panels_ipe_access implementation, because otherwise Panels IPE will just add the buttons anywhere anything is rendered with the IPE pipeline based on the permissions it provides itself.

cboyden’s picture

Status: Active » Needs review
FileSize
1.53 KB

Here's a patch that updates the existing check so that all it does is deny IPE access if the current user doesn't have edit access to the entity, and also adds a check to deny IPE access if the current user doesn't have either the content or layout IPE permission for the entity type and bundle. So this hook won't ever return TRUE, it just checks to see if it should deny access, and if not, passes that decision off to the next hook or to Panels.

It's not granular enough to handle showing ONLY the Customize... or Change layout button if the current user has the overall Panels permissions, but only has one of the Panelizer permissions (content or layout).

dsnopek’s picture

Status: Needs review » Needs work
  1. +++ b/panelizer.module
    @@ -170,15 +170,27 @@ function panelizer_panels_ipe_access($display) {
    +      // Also deny access to the IPE if the user doesn't have either the
    +      // Administer Panelizer content or Administer Panelizer layout permission
    +      // for this entity type.
    +      $bundle = $entity->type;
    +      $layout_perm = "administer panelizer $entity_type $bundle layout";
    +      $content_perm = "administer panelizer $entity_type $bundle content";
    +      if (!user_access($layout_perm) && !user_access($content_perm)) {
    +        return FALSE;
    

    Rather than assembling these permissions manually, let's use the Panelizer API to check the permissions.

    I think that's something like:

    if (!$handler->panelizer_access("layout", $bundle, $view_mode) || !$handler->panelizer_access("content", $bundle, $view_mode)) {
      return FALSE;
    }
    

    ... although, I haven't actually tested that. But it would allow OG permission checks and hook_panelizer_access() to continue working.

  2. +++ b/panelizer.module
    @@ -170,15 +170,27 @@ function panelizer_panels_ipe_access($display) {
    +  return NULL;
    

    We're now only returning FALSE and NULL, but never TRUE. Although, the way the Panels IPE uses the result of this hook doesn't really do anything with TRUE, I still think we should return it in a way that makes sense for the API of this hook.

    So, I think in the "if (isset($display->context['panelizer'])) { ... }" block, at the end, if we haven't denied access, we should return TRUE.

    That would mean only returning NULL on a non-Panelizer displays, which I think is correct, because we have no opinion about Panelizer displays.

cboyden’s picture

Status: Needs work » Needs review
FileSize
1.51 KB
940 bytes

Thanks for the review, I've updated the patch to use panelizer_access and to return TRUE at the end of the "if $display->context['panelizer']" block.

dsnopek’s picture

Status: Needs review » Needs work

Thanks! The changes look great, but there was something else I didn't notice initially:

+++ b/panelizer.module
@@ -170,15 +170,27 @@ function panelizer_panels_ipe_access($display) {
+      $bundle = $entity->type;

This will work for nodes, but not all entity types generally.

Instead, this should be:

list (,, $bundle) = entity_extract_ids($entity_type, $entity);

Or, actually, we can pass the $entity to $handler->panelizer_access("layout", $entity, $view_mode) as an alternate signature. That's probably the better way to go because we have the entity object available...

Sorry for not leading you this way originally!

cboyden’s picture

Status: Needs work » Needs review
FileSize
1.47 KB
769 bytes

Thanks, I've updated the patch to pass the $entity object instead.

dsnopek’s picture

Assigned: Unassigned » DamienMcKenna
Status: Needs review » Reviewed & tested by the community

Thanks! This looks good to me - RTBC'ing and assigning to Damien to check out :-)

cboyden’s picture

I was testing this in conjunction with Fieldable Panels Panes on Panopoly and ran into a problem where less-privileged users couldn't add any panes to the page after the patch had been applied.

Notice: Undefined property: stdClass::$panelizer_view_mode in panelizer_panels_ipe_access() (line 214 of panelizer.module)

The access hook was being invoked on an entity that did not have the panelizer_view_mode property set, and was causing problems. The updated patch fixes this error so less-privileged users who have IPE access (and the necessary permissions on FPPs) can add FPPs successfully.

I'm not sure if there are any other effects I should be worried about. And should the hook return NULL instead of TRUE if the entity does not have a Panelizer view mode?

dsnopek’s picture

Status: Needs review » Reviewed & tested by the community

Checking if $panelizer_view_mode is set is a good idea in any case!

However, I'm a little befuddled as to how that could cause the problem you describe. This should be called on the entity we're Panelizing, not anything to do with an entity that's being placed. So, this might point to another problem somewhere else, where panelizer_panels_ipe_access() is getting called in an unexpected way?

That would be a problem for another issue, though. The code on this patch looks good in isolation and still solves the problem described here, so, RTBC'ing again. :-)

DamienMcKenna’s picture

Status: Reviewed & tested by the community » Fixed
Parent issue: » #2787147: Plan for Panelizer 7.x-3.5 release

Committed. Thanks cboyden, and dsnopek for the reviews!

DamienMcKenna’s picture

I'd suggest creating another issue to deal with the regression you found and writing some tests to cover the use case, we can then dig into where the bug comes from.

Status: Fixed » Closed (fixed)

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

DamienMcKenna’s picture

Assigned: DamienMcKenna » Unassigned