Module Admin Restrict Branch

[Kiwi Chris]

I'm not sure whether it's relating to having this module installed or Lister/ListerPro by @ryan but I seem to be having issues displaying unpublished pages in Lister/ListerPro, and Ryan did mention to check on any other modules that modify permissions.

Is anyone else using this with custom Listers or ListerPro? If so, are users able to view their own unpublished pages OK?

[adrian]

@Kiwi Chris - even on a site without this module installed, I have to do this for users to see unpublished pages in ListerPro:

Does that help?

[Kiwi Chris]

@adrian I already have that option configured. In lister I get a message: 

Quote

Not all specified templates are editable. Only 'include=hidden' is allowed

I know this issue has come up before, and Ryan has asked people to check if they have anything that is hooking permissions.

With my site structure I have multiple clients who are restricted to what they can edit via this module, however they all have pages they can edit using common templates, so there will be some pages using a given template a user can edit and others they can't.

The template in question in my lister definitely is editable for the user, and published pages for the user show up fine.

I think AdminRestrictBranch is the only module I have installed that does hook permissions, so I thought it would be worth checking whether other people are using it and Lister/ListerPro together successfully. 

[adrian]

4 hours ago, Kiwi Chris said:

Not all specified templates are editable. Only 'include=hidden' is allowed

I see that notice with non-superusers even without this module installed. Have you tested with the module uninstalled, disabled, or with a user with no restricted branches?

[adrian]

@Kiwi Chris

Just looked at one of my sites and this is what I put in ready.php

// lets non superusers view unpublished and hidden pages in listers
// https://processwire.com/talk/topic/9346-not-all-specified-templates-are-editable-only-includehidden-is-allowed/?do=findComment&comment=143068
$this->wire()->addHookBefore('ProcessPageLister::getSelector', function($event) {
    $event->object->allowIncludeAll = true;
});

Does this solve your problems?

[Kiwi Chris]

4 hours ago, adrian said:

@Kiwi Chris

Just looked at one of my sites and this is what I put in ready.php

// lets non superusers view unpublished and hidden pages in listers
// https://processwire.com/talk/topic/9346-not-all-specified-templates-are-editable-only-includehidden-is-allowed/?do=findComment&comment=143068
$this->wire()->addHookBefore('ProcessPageLister::getSelector', function($event) {
    $event->object->allowIncludeAll = true;
});

Does this solve your problems?

Thanks. This provided the result I needed. I actually added it to a custom module that added the hook, but same result.

[adrian]

5 hours ago, Kiwi Chris said:

Thanks. This provided the result I needed. I actually added it to a custom module that added the hook, but same result.

So just to confirm, do you agree that this module isn't having any impacts on the issue?

[Kiwi Chris]

4 hours ago, adrian said:

So just to confirm, do you agree that this module isn't having any impacts on the issue?

Yes, I've now also confirmed on a site that doesn't have this module installed. I'll go back to @ryan and ask him about ListerPro. It has an option to include hidden + unpublished but doesn't actually do anything. Thanks for the workaround. If I get ListerPro working as expected without this module without needing a workaround, I'll recheck again with this module to confirm it has no side effects.

[adrian]

3 minutes ago, Kiwi Chris said:

I'll go back to @ryan and ask him about ListerPro. It has an option to include hidden + unpublished but doesn't actually do anything.

Did you read this thread? 

 

[Kiwi Chris]

On 11/7/2019 at 8:28 AM, Kiwi Chris said:

Yes, I've now also confirmed on a site that doesn't have this module installed. I'll go back to @ryan and ask him about ListerPro. It has an option to include hidden + unpublished but doesn't actually do anything. Thanks for the workaround. If I get ListerPro working as expected without this module without needing a workaround, I'll recheck again with this module to confirm it has no side effects.

I've done some further investigation, and on a site without this module, Lister/Lister Pro will show hidden + unpublished as long as the template itself has access control set.

It won't work if only the parent has access control set. Access control must be explicitly set on the template in question, as Lister (Pro) doesn't appear to look at access control inheritance.

With AdminRestrictBranch installed, even if a template does have explicit access control, and no other modifications to access control, it does seem that the hidden + unpublished option in Lister/Lister Pro doesn't work, although using the include all hook, or doing the equivalent creating the lister via the API, setting allowIncludeAll property to true, and including include=all in initSelector property of the lister does provides provides the expected results.

[adrian]

2 hours ago, Kiwi Chris said:

Lister/Lister Pro will show hidden + unpublished as long as the template itself has access control set.

I see that too but it's weird to me that access permissions aren't inherited from the home template in this case. Why should we have to explicitly set permissions for the specific template for this to work? Do you agree or am I missing something?

2 hours ago, Kiwi Chris said:

With AdminRestrictBranch installed, even if a template does have explicit access control, and no other modifications to access control, it does seem that the hidden + unpublished option in Lister/Lister Pro doesn't work

Thanks, I can also confirm this - will see if it's fixable.

[adrian]

Ok, so I found the source of the problem: https://github.com/processwire/processwire/blob/4e4b3afdcbe9aef27d03170dab64dc6e8e6c5a4d/wire/modules/Process/ProcessPageLister/ProcessPageLister.module#L1100-L1106

Lister (the base version in the core) checks to see if a test page is editable but it doesn't ever set the parent:

// determine how many templates are editable
$test = $this->wire('pages')->newPage();
foreach($templates as $template) {
    $test->template = $template;
    $test->id = 999; // required (any ID number works)
    if($test->editable()) $numEditable++;
}

Without knowing the parent, when AdminRestrictBranch hooks into editable() it has to return false. If you want to test this out, add:

$test->parent = xxxx;

where xxxx is the ID of the branch parent you are restricting to - with that in place, you won't need the ProcessPageLister::getSelector hook.

I feel like that code in Lister is a bit hacky and will cause problems with lots of modules that hook into editable(). 

I am not sure this is a good way for me to workaround this. Perhaps I could have an option in AdminRestrictBranch to automatically add this hook:

$this->wire()->addHookBefore('ProcessPageLister::getSelector', function($event) {
    $event->object->allowIncludeAll = true;
});

rather than having to add it manually.

But I still think it's really weird that for Lister to work as expected you can't rely on inherited template edit permissions in the first place. Although this reason for this is obviously the lack of parent for the $test page - it can't rely on inherited permissions because it doesn't have a template to inherit from.

[xportde]

Hi Adrian,

we use your excellent module for a customer, who wants restrict access to users in exactly that way, that your module gives (by role name). Now, they want to let some users act for more than one role name. This seems to be possible by activating the desired roles in user settings, but isn't working. I.e., the user always has access to the branch of the first limiting role, but not to further one(s). 

So, it looks like it ist not possible to combine two ore more restricted roles for one user?

Greeting, Thomas.

[adrian]

@xportde - hi Thomas.

Unfortunately with the way this module is built, it can only ever work for one branch.

There are other ways to achieve what you are looking for - this thread (https://processwire.com/talk/topic/1176-hiding-uneditable-pages-from-users/) contains lots of useful info, but keep in mind there are lots of hooks you'll need to implement to ensure everything is locked down - it's one thing to hide part of the page tree, but you also need to hide the pages from the top-level nav under Pages > Tree. You also need to prevent direct edit access to pages outside the allowed branches. You may also want to consider modifying the breadcrumbs (like this module allows). 

Honestly I think it's a shame that the core doesn't support this natively.

 

[xportde]

Hi Adrian,

thanks for clearing this out! No problem, if your module is intended for restricting to one branch, then we will suggest our client to use it that way.

Greeting, Thomas.

[Kiwi Chris]

Will the custom PHP code option work with multiple statements? I have a tree structure like:
Members
-Member 1
 -page 1
 -page 2
-Member 2
Competitions
...

I want to restrict logins with role member to just their own member branch, and that works fine currently with: return "/members/" . ($user->name);

What I'd like to do is if a login has the role 'membership-secretary' set their branch to Members, and then any other admin roles to home, as I can use template permissions to prevent them accessing parts of the site they shouldn't, but I'm trying to make it easier on the 'membership-secretary' role so they only have to see the branch of tree they'll work with.

I understand the module doesn't support restricting to multiple branches simultaneously, but I don't think that's what I'm trying to do. I only want to grant access to a single branch to a given user, however depending on their role, they will have access at a different level in the page tree.

I had a go with 

if($user->hasRole('competition-secretary') || $user->hasRole('membership-secretary'){return "/members/";}else if($user->hasRole('member'){return "/members/" . ($user->name);}else{return "/"}

statement, but that caused a 500 error, so I'm just checking whether this kind of use case scenario is possible?

 

[adrian]

I think that should work, but your if statements are missing the closing ")"

You have:

if($user->hasRole('competition-secretary') || $user->hasRole('membership-secretary') {
    return "/members/";
}
else if($user->hasRole('member') {
    return "/members/" . ($user->name);
}
else {
    return "/"
}

instead of:

if($user->hasRole('competition-secretary') || $user->hasRole('membership-secretary')) {
    return "/members/";
}
else if($user->hasRole('member')) {
    return "/members/" . ($user->name);
}
else {
    return "/"
}

The error should go away if you fix the syntax.

I think you might also need a closing slash after $user->name to complete the path.

[Ivan Gretsky]

Good day, @adrian!

I have found your super-module installed on a client's site. And it lives alongside PageEditPerUser. The If no match, give all access or no access? option is set to No Access. For some users we need to apply restrict branch per user limitation via your module. But for others we need to limit page access on a per page basis with PageEditPerUser, leaving the branch option empty. But this does not seem to work. If the branch_parent field is left empty it still rules over other page edit permissions. Could you please explain, why is that happening, can we get around it (like changing the order of hooks applied or something). Or maybe suggest another way to deal with it?

[adrian]

Hi @Ivan Gretsky - I think you are probably on track with it being a hook priority issue. Unfortunately PW's approach to this is not very convenient IMO - I think it would be better if developers could have the option to set the priority in each PW install, as needed because otherwise the module developer has to set it without knowing what other modules might be installed on the site that also hook into the same methods.

I think for now you might have to adjust the priority yourself in both modules to ensure that PageEditPerUser overrides ARB as needed. Please also note this issue (https://github.com/ryancramerdesign/PageEditPerUser/issues/4) in Ryan's module - not sure how much it affects its intended functionality, but worth keeping an eye on, or fixing yourself.

Sorry, I don't have time to investigate properly at the moment but if you have any specific requested changes to the module to make it work as you need, just let me know.

[Ivan Gretsky]

Thanks! I remember reading about the possibility to change the module load priority, but cannot find it( Could you direct me there to make the answer complete for someone finding it later?

[adrian]

2 minutes ago, Ivan Gretsky said:

Thanks! I remember reading about the possibility to change the module load priority, but cannot find it( Could you direct me there to make the answer complete for someone finding it later?

Here's the thread that explains it: 

 

[Ivan Gretsky]

Cool! I was sure it has made it's way into the core though, so was looking in the blog posts... Or was it just your Tracy Debugger that got privileged))

[adrian]

Just now, Ivan Gretsky said:

Or was it just your Tracy Debugger that got privileged))

You're confusing module load priority with hook priority. The link above is to set hook priority. The module load priority was introduced quite recently and is what Tracy uses to make sure it is loaded before other modules, but what you want to adjust is the hook priority.

[Ivan Gretsky]

Oh, I see. I've indeed confused the two.

So looking at the thread it seems I need to modify the modules code, right? No way to sort'em afterwards? Sorry for asking the questions in the wrong thread))

[adrian]

Just now, Ivan Gretsky said:

So looking at the thread it seems I need to modify the modules code, right? No way to sort'em afterwards?

Correct - that is why I was saying that I don't think it's an ideal setup.