ProcessPageEditLink permission issue on user template

[adrianbj]

Short description of the issue

If you have an RTE field on the user template non-superusers can't insert in link because of these lines: https://github.com/processwire/processwire/blob/e78ada885469406a9e61314265d7ce693346e8d4/wire/modules/Process/ProcessPageEditLink/ProcessPageEditLink.module#L127-L129

Expected behavior

If they can edit users they should be able to insert a link.

Actual behavior

They get a server error.

Steps to reproduce the issue

1. Add RTE field to user template
2. Be logged in as a user with user admin permissions (but not superuser)
3. Try to insert a link into the RTE field

[adrianbj]

Sorry, a bit of a somewhat related followup. I was thinking that changing that conditional from page-view to page-edit might fix things, but users with user-admin don't have page-edit permissions on users. See the following screenshot where I am logged in as a user with user-admin and editing a user page, but a check on page-edit returns false.

[ryancramerdesign]

@adrianbj They won't have page-edit permission because user-admin permission only provides page-edit permission with ProcessUser module as the gateway. Plus, no need to have edit permission just for linking to something. I think that the page-view check here is likely just as much about making sure we're linking to something that actually renders output rather than a 404, and we need a better error message. But the check seems like it should be broader $page->viewable() rather than the more limited $user->hasPermission('page-view', $page);. Do you find that changing it to viewable() helps?

[adrianbj]

Hi @ryancramerdesign - sorry about the edit / view confusion - it is confusing with user pages :)

$page->viewable() still returns false when a non-superuser is editing a user page so unfortunately that won't help. Anything else you suggest I try?

[adrianbj]

@ryancramerdesign - just in case it helps

[adrianbj]

@ryancramerdesign - is there anything else you need from me on this?

[adrianbj]

@ryancramerdesign - unfortunately I am not sure there is even a way I can fix this via a hook because hasPermission isn't hookable. I am not suggesting that it should be, but I was just looking for a way to fix this without bothering you further. A lot of the user related permission issues I've reported I have been able to fix with hooks, but not sure there is an option for this one without hacking the core.

[Toutouwai]

I'm guessing that the reason for the test here is that ProcessPageEditLink could reveal information about files that exist on $this->page when the user may not be allowed to know anything about $this->page. Because by itself ProcessPageEditLink can't make any changes to $this->page - that can only happen through a page editor, e.g. ProcessPageEdit.

It would make more sense to me if the test was on $page->editable() rather than $page->viewable() because technically users may be able to edit pages that they can't view.

But for this particular issue there would need to be a solution to this: https://github.com/processwire/processwire-issues/issues/1322

Maybe along the lines of my comment:

how about making ProcessUser::getEditableRoles() public so that it's a bit easier to answer the question "Is the current user allowed to edit the given user in ProcessUser?" outside the context of ProcessUser?

[Toutouwai]

Actually ProcessUser::getEditableRoles() doesn't need to be public.

How about the following as a replacement for https://github.com/processwire/processwire/blob/2361b907392e92056681ef1da31b109153b7f165/wire/modules/Process/ProcessPageEditLink/ProcessPageEditLink.module#L128-L130 ?

if($this->page && $this->page->id) {
    $editable = $this->page->editable();
    if(!$editable && $this->page instanceof User) {
        $user = $this->wire()->user;
        if($this->page === $user && $user->hasPermission('profile-edit')) {
            $editable = true;
        } else {
            $pu = $this->wire()->modules->get('ProcessUser');
            $editableRoles = $pu->getEditableRoles($this->page);
            foreach($editableRoles as $role) {
                if($this->page->hasRole($role)) {
                    $editable = true;
                    break;
                }
            }
        }
    }
    if(!$editable) throw new WireException($this->_('Page is not editable'));
}

[edit: added some code to handle editing own profile]

Does that work for you @adrianbj ?

[adrianbj]

Thanks @ryancramerdesign for https://github.com/processwire/processwire/commit/68fa2b47f6d135a9015eac4fc54038dd80f21d06 which looks to have solved this as well.