processwire-requests icon indicating copy to clipboard operation
processwire-requests copied to clipboard

Custom User Types, Page Edit vs. Profile Edit

Open ghost opened this issue 5 years ago • 18 comments

@ethanbeyer commented on Mar 15, 2018, 2:04 PM UTC:

Short description of the issue

Processwire utilizes a Profile Editing screen that differs a lot from the Page Edit screen.

Tab Groups are not visible and there is no "View" link. Several fieldtypes (mainly repeaters from what I have encountered) do not work as expected on Profile Edit.

For users without page-edit permissions that do have profile-edit permissions, the $user->editUrl() points to the Page Edit screen rather than the Profile Edit (which users with these permissions cannot access).

Expected behavior

The Profile Edit and Page Edit should be exactly the same in functionality and structure.

Actual behavior

  • The editUrl sends Users without Page-Edit permissions to a 404 in admin
  • On Profile Edit, Tab Groups do not show up and there is no View Link
  • On Profile Edit, Repeaters loaded with AJAX cannot add new items (unless the User has Page-Edit permissions for the user-template being edited), and this seems to be a bug with InputfieldRepeater.module:627

Optional: Screenshots/Links that demonstrate the issue

I've gone into fine detail here: https://processwire.com/talk/topic/18712-custom-user-types-page-edit-vs-profile-edit/

Optional: Suggestion for a possible fix

I would suggest using the exact same code for Profile Edit and Page Edit. If they are to be seen as different by URL, the editURL() and editable calls need to be able to differentiate between /access/users/edit and /profile/.

Steps to reproduce the issue

  1. Create a new User Type
  2. Create a new role with Page-View and Profile-Edit permissions
  3. Create a new user, give them that new role
  4. Put a call on the frontend to $user->editUrl().
  5. Click the link and you will see the admin equivalent of a 404.
  6. Add a repeater field to the new User Template
  7. Navigate to /admin/profile and try to add a new Repeater Item (without giving the user Page-Edit access)

Setup/Environment

  • ProcessWire version: 3.0.96

This issue was moved by netcarver from processwire/processwire-issues#538.

ghost avatar Mar 09 '19 11:03 ghost

@ryancramerdesign commented on Mar 15, 2018, 3:32 PM UTC:

I think you may be thinking of the profile editor as being equivalent to a page editor, but it's really quite different by intention. The profile editor is a really locked down process, and it's not intended to do everything the page editor can do. At the same time, it offers more security than the page editor could. It is intentionally limited for security reasons, because it can be accessible to non-trusted users. It's primary purpose is to allow password and email address changes, though it supports several other fieldtypes that you can add by name to ProcessProfile module config.

There is no view link because this is not intended as a page editor, just a profile editor. User pages generally aren't viewable anyway. Tabs are not supported in the user profile, though fieldsets should be.

Repeatable page types support (like repeaters) is limited because repeaters are pages and thus require page-edit permission (and access to ProcessPageEdit), which the user may or may not have. A user does not have page-edit permission to any user page unless they have user-admin permission or page-edit permission to the User template. That's because users may have access to edit their profile, that don't have access to edit any pages. So you can have access to edit your profile, without having access to edit your User page. Anything that uses external pages (Repeater, RepeaterMatrix, PageTable, FieldsetPage) is fine to have on a User template, but probably not going to be useful on the profile edit.

Regarding $page->editUrl(), this is not contextual to the current user. It is contextual to the page it is called from, and is often used in things like admin notification emails, log entries, and such. It simply represents the URL to edit the page, and doesn't say anything about whether the user has access to edit the page. There isn't any instance where it would point to the profile editor, because that does not represent the editor for a page.

ghost avatar Mar 09 '19 11:03 ghost

@ethanbeyer commented on Mar 15, 2018, 7:49 PM UTC:

Ah, ryancramerdesign thank you for weighing in!

I appreciate the explanation on the differences between the Profile and Page Edit screens - I see where you're coming from with Security! That makes sense. There are a few places I am losing the plot though, and if you'll allow me these arguments, please do not think I am being hateful in any way. I love Processwire very much and am hoping to help make it better, or at the very least understand how to solve specific problems by using it.

So here are the issues I take with keeping Profile and Page Editors separate in the exact state that they are currently in:

  1. There are times when a User = Content. I don't know how to lock a single page down to be editable by only a single user, outside of the Profile Editor. It's already there, and that's why I chose to use it. If there's a way to do this, where User A can only edit Page A and Page A can only be edited by User A and Directory Admins, I'd probably have no further problem...

  2. The main issue with Repeaters on the Profile page is when they're generated via AJAX. If they're not generated via AJAX, a user could add as many new items as they so chose, save their profile, and then add the info to the new Repeater Pages - so I am arguing in defense of making what's already possible more streamlined.

  3. I wasn't aware of the fact that the editUrl wasn't contextual - because everywhere I've seen or used the $page->editURL(), it's wrapped in a $page->editable check. So I wrongfully assumed that the Permissions check == a Permissions URL. I see that it doesn't - but I now see the need for something like userEditUrl() that would be contextual. Again, I am speaking based on this limited use-case where I am building a directory - but there are plenty of times when I have built sites that displayed Users and there was a need for an Edit link. And if the person is only able to edit their own information, it would make sense to be able to get that Edit URL dynamically, rather than hardcoding $config->urls->admin . 'profile/'.

I am afraid I am rambling, just trying to sort this whole issue out.

ghost avatar Mar 09 '19 11:03 ghost

@ethanbeyer commented on Mar 15, 2018, 8:08 PM UTC:

Is there a way outside of profile-edit to allow a user to edit their own page? It seems like if a User is a Page (outside of /admin/access), giving them the ability to edit their own page with page-edit is only possible if that user also has user-admin.

Am I wrong?

ghost avatar Mar 09 '19 11:03 ghost

@ethanbeyer commented on Mar 27, 2018, 2:50 PM UTC:

Wanted to weigh in again, due to gebeer and I talking about it on the forums.

One of the reasons I don't want to give directory-members Page Edit permissions is because then they have access to the entire Page Tree, which is not desired at all.

ghost avatar Mar 09 '19 11:03 ghost

@ethanbeyer commented on Mar 27, 2018, 5:51 PM UTC:

Continuing to research and tinker, and maybe this course of action would allow for a fix:

Add a new Core permission: profile-page-edit. Why?

  • Right now a Member that I want to give Page Edit access to their own profile has to have page-edit and user-admin permissions, which is too broad (full page tree + other user's profiles), but
  • profile-edit permissions don't give the full level of access I'd like.

Right now I'm stuck in the middle, and a lot of seems to be as a result of ProcessPageEdit::___loadPage(). Specifically this bit:

if($page instanceof User) {
    // special case when page is a User

    $userAdmin = $user->hasPermission('user-admin');
    $field = $input->get('field') ? $this->wire('fields')->get($input->get->fieldName('field')) : null;

    if($userAdmin && $this->wire('process') != 'ProcessUser') {
        // only allow user pages to be edited from the access section (at least for non-superusers)
        $this->wire('session')->redirect($config->urls->admin . 'access/users/edit/?id=' . $page->id);

    } else if(!$userAdmin && $page->id === $user->id && $field && $config->ajax) {
        // user is editing themself and we're responding to an ajax request for a field
        /** @var PagePermissions $pagePermissions */
        $pagePermissions = $this->wire('modules')->get('PagePermissions');
        $editable = $pagePermissions->userFieldEditable($field); 
        // prevent a later potential redirect to user editor
        if($editable) $this->setEditor = false;
    }
}

If that was rewritten to use the profile-page-edit permission like this, I think this issue would be solved:

if($page instanceof User) {
    // special case when page is a User

    $profilePageEdit = $user->hasPermission('profile-page-edit');
    $userAdmin = $user->hasPermission('user-admin');
    $field = $input->get('field') ? $this->wire('fields')->get($input->get->fieldName('field')) : null;

    if($profilePageEdit && $page->id === $user->id) {
        // user is editing their profile AS A PAGE
        $this->page->setEditor($this->editor ? $this->editor : $this);

    } else if($userAdmin && $this->wire('process') != 'ProcessUser') {
        // only allow user pages to be edited from the access section (at least for non-superusers)
        $this->wire('session')->redirect($config->urls->admin . 'access/users/edit/?id=' . $page->id);

    } else if(!$userAdmin && $page->id === $user->id && $field && $config->ajax) {
        // user is editing themself and we're responding to an ajax request for a field
        /** @var PagePermissions $pagePermissions */
        $pagePermissions = $this->wire('modules')->get('PagePermissions');
        $editable = $pagePermissions->userFieldEditable($field); 
        // prevent a later potential redirect to user editor
        if($editable) $this->setEditor = false;
    }
}

Would that work, ryancramerdesign ?

ghost avatar Mar 09 '19 11:03 ghost

@ethanbeyer commented on Mar 27, 2018, 7:03 PM UTC:

Just realized my directory member users won't be able to use the Focus+Zoom feature on /admin/profile/.

ghost avatar Mar 09 '19 11:03 ghost

@gebeer commented on Mar 28, 2018, 4:32 AM UTC:

ethanbeyer you could implement that as a module that extends ProcessPageEdit. That way you can make things work without the need for changing the core process

ghost avatar Mar 09 '19 11:03 ghost

@ethanbeyer commented on Apr 3, 2018, 1:32 PM UTC:

gebeer late on my reply, sorry!

I will probably have to. I've taken a look at the code you posted in the forums. To be honest, it's all a little over my head to work out how exactly to untangle this, otherwise I'd make much more of an effort to do so.

Thanks for all the work you've put in on this issue though!

ghost avatar Mar 09 '19 11:03 ghost

@netcarver commented on Mar 9, 2019, 11:09 AM UTC:

Moving this one to the request repo, we can continue the discussion over there.

/move to requests

ghost avatar Mar 09 '19 11:03 ghost

~~I wanted to come back to this, as the issue has broken another part of the site this pertains to: image uploads.~~

~~I don't know why exactly it's happening, but when users try to upload an image to their page, the Inputfield spins and spins. I've seen it before for PHP Config issues, but it started (I believe) after bumping PW from 3.0.97 to 3.0.103.~~

~~I was getting the error that jQuery could not parse the JSON returned, and looking at the response from the upload AJAX call, and it's because the upload is loading the homepage of the site. I don't know why.~~

EDIT:

The above is all good and fine, but I recognize this is not a help forum. So if anyone is interested in debugging this weird issue, I'd appreciate it, but I won't overwhelm the thread with that right now.

Instead, I want to again float the request to untangle some of the hardcoded parts of Users in the editing process, as I think that is the root cause of all the workarounds required for making sites like this work with ProcessWire.

Since the ProcessWire docs mention being able to structure a site where Users are not always nested beneath /admin/access/, it seems like the process where they interact with the rest of the site needs to be looked at. I can't think of a system I'd rather use for building a directory than ProcessWire, because everything I need and could need is built in and 95% flawless. It's just the last bit, the organization of Directory Members that need viewable pages (necessitating a front-end-viewable page [ie, not /admin/access as a parent], but also need login access, necessitating their Page Class being a User.

I'm going to revisit some code @gebeer wrote to help, but I do think that a way for this to happen with Core would be worth looking into

ethanbeyer avatar Apr 08 '19 14:04 ethanbeyer

I think the spinner issue was addressed here https://github.com/processwire/processwire/commit/2aa2342e185bf683303ba8ea8e444f84c313ffcb for https://github.com/processwire/processwire-issues/issues/768

You might try following these instructions by Ryan to help in debugging the issue: https://processwire.com/talk/topic/8712-imagefile-upload-problems-without-errors/?do=findComment&comment=96469

gmclelland avatar Apr 08 '19 14:04 gmclelland

Thanks, @gmclelland - I've determined it's not the User Page issue, as I thought. It's server level! The error is a 404 in the response, and it's loading the Home page. Still debugging.

ethanbeyer avatar Apr 08 '19 15:04 ethanbeyer

Just wanted to say that with a lot of hooks I managed to allow users to get automatically redirected to the ProcessUser edit screen and allow them to edit their profile there without being able to edit other users. This lets me have tabs and also use AdminOnSteroid's sidebar feature (which is an essential improvement to all PW page editing interfaces IMO).

This isn't for the faint of heart, so it would be great to improve this - I believe it's especially important when you are using the custom user template / pages approach because users need to be able to edit their page (often viewable on the frontend) with the full page editing interface.

adrianbj avatar May 12 '21 17:05 adrianbj

Just wanted to say that with a lot of hooks I managed to allow users to get automatically redirected to the ProcessUser edit screen and allow them to edit their profile there without being able to edit other users. This lets me have tabs and also use AdminOnSteroid's sidebar feature (which is an essential improvement to all PW page editing interfaces IMO).

This isn't for the faint of heart, so it would be great to improve this - I believe it's especially important when you are using the custom user template / pages approach because users need to be able to edit their page (often viewable on the frontend) with the full page editing interface.

@adrianbj Could you share how you accomplished this? I've been looking at lots of issues related to this but so far haven't been able to find a solution myself either.

daun avatar Jan 07 '22 16:01 daun

Hi @daun - this is what I have in my admin.php file. This relies on a user-admin-staff (or similar) permission that gives users with the staff role the ability to administer users with the staff role, but then in the hook below I take away that permission unless the user has a role (eg user-admin), or is a superuser when we are on the person template (which is my alternate user template for staff with front-end user pages), or they are trying to edit themselves.

// redirect profile editing to the user's staff page
$wire->addHookAfter('ProcessProfile::execute', function($event) {
    $this->wire()->session->redirect($this->wire()->config->urls->admin.'access/users/edit/?id='.$this->wire()->user->id);
});
// prevent normal staff user from being able to edit other staff member profiles
// even though the staff role has the user-admin-staff permission
$wire->addHookAfter('PagePermissions::pageEditable', function($event) {
    if($event->arguments[0]->template->name == 'person' && !$this->wire('user')->hasRole('user-admin') && !$this->wire('user')->isSuperuser() && $this->wire('user')->id !== $event->arguments[0]->id) {
        $event->return = false;
    }
});

Another issue is being able to view the user on frontend. You can end up with a situation where a user can't view their own page, so you might need to give the guest user role user-view-staff. And if you have other staff with the person template (based on my example above), but they don't have user edit rights, then you might need to assign them an alternate role, eg other-person and give the guest role user-view-other-person.

That should be everything to consider - painful, but it works :)

Hope that helps.

adrianbj avatar Jan 07 '22 17:01 adrianbj

@adrianbj Thanks! I got it working with your example. One caveat is that now person users can actually remove the person role from their own profile, but the roles field is easy enough to hide in a hook.

daun avatar Jan 07 '22 19:01 daun

Are you sure they can actually remove the role? For me it looks like they can, but when they save, the role hasn't actually been removed. That said, hiding the roles section for these types of users is probably a good idea anyway.

adrianbj avatar Jan 07 '22 19:01 adrianbj

You're right, I haven't actually tried saving the page 🤡 The roles aren't updated on save. Confusing, still, but less of a security nightmare.

daun avatar Jan 07 '22 19:01 daun