processwire / processwire-issues

ProcessWire issue reports.
44 stars 2 forks source link

ProcessPageEditLink permission issue on user template #1933

Open adrianbj opened 4 months ago

adrianbj commented 4 months ago

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 commented 4 months ago

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.

image
ryancramerdesign commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

@ryancramerdesign - just in case it helps

image
adrianbj commented 3 months ago

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

adrianbj commented 3 months ago

@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.