There are currently some limitations to the permissioning of the IPE, often requiring an "all or nothing" approach centering around the "Use the Panels In-Place Editor" permission. The up and coming pipelines magic should game change this, but until then here is a proposal:

-- implement access checking for displaying the IPE to allow users to use the IPE if they have the global "Use the IPE" permission AND they also have authority to edit the node/page in question. i.e. panelizer could check node_access() and page manager can check "Use Page Manager".
-- update the text of the global permissions "Use the Panels In-Place Editor" and "Change layouts with the Panels In-Place Editor" to signify they require you to have access to actually edit the content.
-- update the text of the panelizer permissions (Administer Panelizer layout, Administer Panelizer content) to let people know those are global permissions to change panelizer defaults.

The major change here would be to bring in line the ability to "edit" content/pages and the ability to use the IPE around them. This makes sense IMO since its super wierd you can use the IPE on things you dont have the ability to actually edit.

CommentFileSizeAuthor
#3 1772844-reject-ipe-without-edit-permission.patch1023 bytesmerlinofchaos

Comments

merlinofchaos’s picture

merlinofchaos commented

implement access checking for displaying the IPE to allow users to use the IPE if they have the global "Use the IPE" permission AND they also have authority to edit the node/page in question. i.e. panelizer could check node_access() and page manager can check "Use Page Manager".

The first is absolutely true, but the second actually is not. Not being able to use page manager is not necessarily a restriction from using the IPE to edit that content, and this is totally intentional. For Panelizer, I had actually addressed that as part of #1777264: Use IPE features to allow Panelizer to edit default or overridden panel for an entity where panelizer itself is doing some extra checking of the IPE and forbidding it if the user does not actually have access to edit the entity.

populist’s picture

populist commented

Thanks for the feedback. In terms of reformulating this request I think I am just asking for this now:

-- add the ability for users to be given permission to use the IPE (edit content, change layouts) through Panelizer for nodes they have the ability to edit WITHOUT giving them permissions to use the IPE on the rest of the pages

merlinofchaos’s picture

merlinofchaos commented
Status: Active » Needs review
StatusFileSize
new 1772844-reject-ipe-without-edit-permission.patch1023 bytes

Since #1777264 needs a lot of work, I've extracted that part of that patch and refined it.

This patch should do the trick inside Panelizer nicely.

merlinofchaos’s picture

merlinofchaos commented
Project: Panels » Panelizer (obsolete)
Component: In-Place Editor (IPE) » Code

This is a patch to Panelizer, not Panels.

beeradb’s picture

beeradb commented
Status: Needs review » Needs work

After applying the patch I saw the "customize this page" button as expected. After moving stuff around I clicked save and the page reverted to the original step. I tried this a few times moving different page elements to different regions but always got a revert. I then went through the panelizer tab and was able to move page elements around and save without issues.

I also attempted to edit the body text on the page, and got a "This field has been disabled because you do not have sufficient permissions to edit it." message inside a disabled textarea. The user account I am using has the "Content Page: Edit any content" permission, and I was editing a standard content page.

Let me know if you need any additional details.

merlinofchaos’s picture

merlinofchaos commented

#5: I don't see how this patch could cause that. Does that happen without the patch as well?

beeradb’s picture

beeradb commented

@merlinofchaos:

On the "customize this page" issue I attempted again with a fresh checkout of Panelizer. I attempted to trigger it several times and was only able to re-produce it once. This patch was applied when I triggered it. I have not found a way to reliably reproduce the issue though, so my best guess is that perhaps there is some sort of strange caching issue at play here?

The issue when editing the body field is present with or without this patch, but is most likely a FAPE issue. Editing an image field or other attributes on the page works correctly.

The primary issue here of the IPE showing up for pages the user has the ability to edit, while not showing for pages they don't is working, so I would say if no one else can reproduce the issue I was seeing we should move forward with this, and just keep an eye out for it in the future.

ctyar’s picture

ctyar commented

after applying this patch i see the IPE in any nodes even the ones i can't edit.
i checked the code if i remove the this part:
$this->panelizer_access('content', $entity, $view_mode)
and the $view_mode is page_manager
IPE only shows up in nodes the user can edit
is something wrong with the patch or I'm doing something wrong?

populist’s picture

populist commented

I did a review of the patch this morning and can confirm the following behavior:

  • Setup - Create a normal user which has the ability to "Use the Panels In-Place Editor", "Node TYPE: Administer Panelizer content", and "Edit Own TYPE".
  • Without the Patch - The user is able to edit any node regardless of access control
  • With the patch - The user is only able to edit pages they have control over

In terms of the comments from #5 and #7, the issue there is probably related to the permissions on the input format for the body. I had seen similar issues but saw that it was the problem.

HOWEVER, the major problem that is still there is that granting "Use the Panels In-Place Editor" to the user allows them to edit *any* IPE enabled Page Manager pages. This would mean that on a site that uses Panelizer things would be permissioned right for nodes, but the user could edit any Panels front pages.

In terms of resolution, not sure this is something we can take care of in the Panelizer module. Perhaps a patch to Panels that expands its permission to have an option that can be selected to allow Panelizer IPE but not Panels IPE.

mpotter’s picture

mpotter commented
Status: Needs work » Reviewed & tested by the community

I tested this patch on clean D7 site with Panopoly and Organic Groups. Only the group owner was able to customize the layout via IPE. When the ownership of the group was changed, the user no longer had access to edit the group and no longer had the Customize Layout buttons. So for the original issue case, I think this patch is good.

I agree with populist about the issue in #9. The Group Owner was still able to customize non-node panels pages (like the home page). So I think another permission is probably needed somewhere to control this patch that would deny access if it wasn't a node-based page somehow.

Not sure if that means this specific patch "needs work" since additional permissions probably require a separate patch for Panels itself. So I'm going to mark this specific patch as "reviewed", but feel free to change the status if you disagree.

merlinofchaos’s picture

merlinofchaos commented

The Page Manager solution must be separate from the Panelizer solution. It can probably be similar, and would probably require an additional Page Manager specific IPE permission which can then be checked by page manager when setting the renderer, similar to how Panelizer is doing it.

merlinofchaos’s picture

merlinofchaos commented
Status: Reviewed & tested by the community » Fixed

I have committed and pushed this patch.

I've spun off a Panels patch to address the Page Manager weakness here: http://drupal.org/node/1854374#comment-6792512

If someone could give that a quick test that'd be lovely!

Status: Fixed » Closed (fixed)

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