Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

It should not be possible for an administrator to change their role to "Instructor" #1100

Closed
thomasplevy opened this issue Apr 13, 2020 · 1 comment
Labels
good first issue If you're a first time contributor this is a good issue for you! help wanted Looking for contributors to assist with this issue Type: Bug Bugs and errors
Milestone

Comments

@thomasplevy
Copy link
Contributor

Reproduction Steps

  • Login to admin panel as an administrator
  • Visit Admin->Users
  • Select yourself using the checkbox
  • Select "Instructor" from the bulk role change dropdown on the top of the page
  • Click "Change" button

Expected Behavior

  • The WP core prevents admins from changing their role to any role without the edit_users capability and this behavior would be expected trying to switch yourself to an instructor.
  • This expected can be observed by performing the same steps above but choosing "Editor" instead of "Instructor"

Actual Behavior

  • The role is changed.

More Information

Since instructors are able to edit users (assistant instructors) they kind of have this capability but only for specific users.

We should hook into this bulk edit role change and add additional checks when changing yourself from admin (or lms_manager) to instructor.

@thomasplevy thomasplevy added Type: Bug Bugs and errors help wanted Looking for contributors to assist with this issue good first issue If you're a first time contributor this is a good issue for you! hacktoberfest PRs for this issue count towards Hacktoberfest contributions! labels Apr 13, 2020
@thomasplevy thomasplevy added this to the Future milestone Apr 13, 2020
@github-project-automation github-project-automation bot moved this to Awaiting Triage in Development Apr 3, 2024
@brianhogg brianhogg removed the hacktoberfest PRs for this issue count towards Hacktoberfest contributions! label Apr 3, 2024
@ideadude ideadude moved this from Awaiting Triage to In Progress in Development Apr 3, 2024
@ideadude
Copy link
Member

ideadude commented Apr 3, 2024

Thinking out loud about this issue of admins/managers changing their role via bulk edit.

An issue with this fix (#2569) coded up by @bsetiawan88 is that it stops admins from downgrading other admins or llms_managers from the bulk edit.

I think the real fix here is to stop users from changing their own role via bulk edit.

Unless I'm missing something, I feel WP core should be okay with that. The fix would be fairly straightforward. Remove the current user from the list of user_ids that can be edited: https://github.com/WordPress/WordPress/blob/d6ce792045fa066e82d84c5ed778e943e629eeef/wp-admin/users.php#L138

On the other hand, we know it's hard to get updates like this in core.

We could implement this in general for administrators and llms_manager users only. There is no filter there.

It's a bit hacky, but we could hook into the "editable_roles" filter, leave the array alone, but check if we're bulk editing users and if so adjust the $_REQUEST['user_ids'] to remove the current user. https://github.com/WordPress/WordPress/blob/d6ce792045fa066e82d84c5ed778e943e629eeef/wp-admin/users.php#L122C21-L122C39

Another hacky option is to hook into set_user_roles and if an admin or llms_manager downgrades themselves, undo it. https://github.com/WordPress/WordPress/blob/d6ce792045fa066e82d84c5ed778e943e629eeef/wp-includes/class-wp-user.php#L655

In Slack discussions, we decided to close this issue and wait for either a fix in WP core or an update there (filter on the user_ids included in bulk updates) that allows for a less hacky fix. We will pursue this at WordCamp contributor days.

@ideadude ideadude closed this as completed Apr 3, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Development Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue If you're a first time contributor this is a good issue for you! help wanted Looking for contributors to assist with this issue Type: Bug Bugs and errors
Projects
Archived in project
3 participants