Opened 11 months ago
Last modified 11 hours ago
#61100 assigned defect (bug)
Fix capability error in bulk role removal when editing site users in network admin
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.9 | Priority: | normal |
Severity: | normal | Version: | 3.1 |
Component: | Networks and Sites | Keywords: | has-testing-info needs-unit-tests needs-refresh dev-feedback |
Focuses: | administration, multisite | Cc: |
Description
when selecting the option '-No role for this site-' on page wp-admin/network/site-users.php?id=xxx triggers an error. This should not be the case. This field is also available on wp-admin/users.php but this doesnt trigger an error.
Bug
See wp-admin/network/site-users.php @ line 140-183. This case 'promote' handles this request. The issue is the value=none of '-No role for this site-', which is not found in $edible_roles. The if-statement on line 145 will cause the error.
Solution
The case 'promote' of wp-admin/users.php @ line 110-170 is almost identical to that of wp-admin/network/site-users.php?id=xxx. But this statement took the 'none' value into account.
wp-admin/network/site-users.php line 145 - 147 should be replaced by wp-admin/users.php lines 125-136
Attachments (3)
Change History (36)
#1
@
11 months ago
- Keywords needs-patch good-first-bug added
- Milestone changed from Awaiting Review to 6.6
This ticket was mentioned in PR #6470 on WordPress/wordpress-develop by @mi5t4n.
11 months ago
#2
- Keywords has-patch added; needs-patch removed
#3
@
10 months ago
Here is my suggestion, To fix this, we should modify the case 'promote' section in wp-admin/network/site-users.php to handle the 'none' role similarly to how it's dealt with in wp-admin/users.php.
Here are the steps to fix this issue:
- Add a mock none-role to $editable_roles:
$editable_roles['none'] = array( 'name' => __( '— No role for this site —' ), );
- Update the if-statement to handle the 'none' role properly:
if ( ! $role || empty( $editable_roles[ $role ] ) ) { wp_die( __( 'Sorry, you are not allowed to give users that role.' ), 403 ); } if ( 'none' === $role ) { $role = ''; }
The error will be resolved by implementing these changes, and selecting "-No role for this site-" will work correctly without triggering an error. This solution aligns the behavior of wp-admin/network/site-users.php with wp-admin/users.php.
This ticket was mentioned in Slack in #core by oglekler. View the logs.
9 months ago
#6
follow-up:
↓ 7
@
9 months ago
This issue is only for the network site? I've tested on a regular WP, and I don't think there is an issue. https://prnt.sc/ODEyd_h4Kozo
#7
in reply to:
↑ 6
@
9 months ago
Replying to hmbashar:
This issue is only for the network site? I've tested on a regular WP, and I don't think there is an issue. https://prnt.sc/ODEyd_h4Kozo
Mate, I provided all links (wp-admin/network/site-users.php) to the exact page. Given instructions on how to duplicate the issues. And here you are completely ignoring all the details of the issue. I suggest you follow instructions more carefully. Its a multisite issue which is blatantly clear in the post details.
#8
@
9 months ago
Test Report
Description
I have added patch in my local and it is working fine
Environment
- WordPress: 6.6-beta2-58420
- PHP: 8.1.23
- Server: nginx/1.16.0
- Database: mysqli (Server: 8.0.16 / Client: mysqlnd 8.1.23)
- Browser: Chrome 126.0.0.0 (macOS)
- Theme: Twenty Nineteen 2.8
Steps to Reproduce
- Go to Users
- Select user
- Click on "Chang role to..." and Select "No role for this site"
Expected Results
If you select "No role for this site" for user role, then it should be changed successfully
Actual Results
User Role is changed sucessfully when i change role to "No role for this site"
https://app.screencast.com/yIj1xAwxUxgxq
Patched applied - https://app.screencast.com/7ePeVO35Hw9Wp
#10
@
9 months ago
@spacedmonkey are we good to go with this one, or should we need to reschedule it?
@sudipatel007 only one test report is not usually enough.
This ticket was mentioned in Slack in #core by nhrrob. View the logs.
9 months ago
#15
@
6 months ago
- Owner set to jeremyfelt
- Status changed from new to assigned
- Summary changed from 'Change role' field on wp-admin/network/site-users.php?id=xxx contains a bug to Fix capability error in bulk role removal when editing site users in network admin
- Version set to 3.1
This was likely introduced in or around [16560].
6 months ago
#16
@jeremyfelt Thank you for the suggestion. I have implemented your requested changes.
This ticket was mentioned in Slack in #core by stoyangeorgiev. View the logs.
5 months ago
#18
@
5 months ago
- Milestone changed from 6.7 to 6.8
This one was discussed at a bug scrub. With RC1 tomorrow and some additional checks and tests needed, we are moving this one to 6.8.
Cheers!
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
6 weeks ago
#21
@
6 weeks ago
- Keywords has-testing-info added
Test Report
Description
This report validates whether the indicated patch works as expected.
Patch tested: https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/6470.diff
Environment
- WordPress: 6.8-alpha-59817
- PHP: 8.3.15
- Server: Apache/2.4.62 (Unix) OpenSSL/3.4.0 PHP/8.3.15
- Database: mysqli (Server: 8.0.39 / Client: mysqlnd 8.3.15)
- Browser: Chrome 132.0.0.0
- OS: macOS
- Theme: Twenty Twenty-Five 1.1
- MU Plugins: None activated
- Plugins:
- Test Reports 1.2.0
Actual Results
- ✅ Issue resolved with patch.
Additional Notes
- Any additional details worth mentioning.
Supplemental Artifacts
without patch: https://core.trac.wordpress.org/attachment/ticket/61100/61100-without-patch.mov
with patch: https://core.trac.wordpress.org/attachment/ticket/61100/61100-with-patch.mov
I will leave needs-testing tag in case more testing is required if not should get removed on next core-test scrub.
This ticket was mentioned in Slack in #core-test by krupajnanda. View the logs.
5 weeks ago
#23
@
5 weeks ago
Test Report
Description
This report validates whether the indicated patch works as expected.
Patch tested: https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/6470.diff
Environment
- WordPress: 6.8-alpha-20250215.103545
- PHP: 8.1.29
- Server: nginx/1.26.1
- Database: mysqli (Server: 8.0.35 / Client: mysqlnd 8.1.29)
- Browser: Firefox 135.0
- OS: macOS
- Theme: Twenty Twenty 2.8
- MU Plugins: None activated
- Plugins:
- Test Reports 1.2.0
Actual Results
- ✅ After applying the patch, no error was shown when the role was set to
none
- the user's role was set to
none
successfully
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
11 days ago
#26
@
11 days ago
Fix capability error in bulk role removal when editing site users in network admin
Description
when selecting the option '-No role for this site-' on page wp-admin/network/site-users.php?id=xxx triggers an error. This should not be the case. This field is also available on wp-admin/users.php but this doesnt trigger an error.
Bug
See wp-admin/network/site-users.php @ line 140-183. This case 'promote' handles this request. The issue is the value=none of '-No role for this site-', which is not found in $edible_roles. The if-statement on line 145 will cause the error.
Solution
The case 'promote' of wp-admin/users.php @ line 110-170 is almost identical to that of wp-admin/network/site-users.php?id=xxx. But this statement took the 'none' value into account.
wp-admin/network/site-users.php line 145 - 147 should be replaced by wp-admin/users.php lines 125-136
Patch tested: https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/6470.diff
Steps to Reproduce
Go to Users
Select user
Click on "Change role to..." and Select "No role for this site"
Environment
- WordPress: 6.8-beta2
- PHP: 8.4.1
- Server: nginx/1.21.4
- Database: mysqli (Server: 5.7.44-log / Client: mysqlnd 8.4.1)
- Browser: Chrome 134.0.0.0
- OS: macOS
- Theme: Twenty Twenty-Five 1.1
- MU Plugins: None activated
- Plugins:
- Test Reports 1.2.0
Actual Results
- ✅ Issue resolved with patch.
#27
@
5 days ago
- Focuses administration added
- Keywords has-unit-tests added
Test Environment:
WordPress Version: 6.8
Test Scenario
Navigate to Network Admin → Sites → Edit Site → Users (wp-admin/network/site-users.php?id=xxx).
Select "-No role for this site-" from the dropdown for a user.
Click Change to update the user’s role.
Observe if an error occurs.
Repeat the same test in Users (wp-admin/users.php) to compare behavior.
Expected Result
Selecting "-No role for this site-" should remove the user’s role without triggering an error.
Actual Result (Before Patch)
An error occurs when selecting "-No role for this site-" on wp-admin/network/site-users.php.
No error occurs when performing the same action on wp-admin/users.php.
Root Cause:
The value "none" is not present in $editable_roles.
The if-statement at line 145 fails, causing the error.
Result (After Applying Patch)
✅ Issue Resolved when selecting "-No role for this site-" for a non-admin user.
❌ Error still occurs when removing the main admin’s role.
The issue persists when attempting to remove the network administrator role.
Attachments
Test Conclusion
Status: 🟡 Partially Fixed
Impact: Medium (Affects user role management in Multisite, particularly for admins)
Recommendation:
Investigate the issue when removing the main admin’s role.
Modify the patch to handle cases where the admin role is being modified.
Ensure consistency between network admin user management and single-site user management.
#28
follow-up:
↓ 30
@
4 days ago
- Keywords needs-unit-tests added; needs-testing has-unit-tests removed
@tusharaddweb I can't reproduce your error with the patch PR6470, following your exact instructions.
Attaching video to show this
Also, this patch doesn't have unit-tests yet, so that tag is not applicable, but this definitely needs some units tests, so I'm adding the needs-unit-tests
tag.
I've tested like the rest and ✅ Issue resolved with the patch.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
4 days ago
#30
in reply to:
↑ 28
@
4 days ago
Replying to SirLouen:
@tusharaddweb I can't reproduce your error with the patch PR6470, following your exact instructions.
Attaching video to show this
Also, this patch doesn't have unit-tests yet, so that tag is not applicable, but this definitely needs some units tests, so I'm adding the
needs-unit-tests
tag.
I've tested like the rest and ✅ Issue resolved with the patch.
Please bulk select all user including admin account
See the video : https://cap.so/s/h7j5bdy2dv5jbm0
#31
@
4 days ago
- Keywords needs-refresh dev-feedback added; has-patch removed
Test Report
Description
This report can't validate that the indicated patch is working as expected, thanks to the great catch of @tusharaddweb. Providing additional information.
Environment
- WordPress: 6.8-beta2-59971-src
- PHP: 8.2.28
- Server: nginx/1.27.4
- Database: mysqli (Server: 8.4.4 / Client: mysqlnd 8.2.28)
- Browser: Chrome 134.0.0.0
- OS: Windows 10/11
- Theme: Twenty Twenty-Five 1.1
- MU Plugins: None activated
- Plugins:
- Test Reports 1.2.0
Steps to Reproduce with the Patch PR 6470
- Create a Multisite
- Go to Users>Add User in one of the network blogs and create one extra user
- Go to Users>All Users in one of the same network blog
- Select admin user and remove role
- 🐞 Bug occurs >
Expected Results
- Error should arise. Admin role, should not be able to remove his own roles.
If you check a regular WordPress site (a non Network/multi site), and try to remove the role from admin with admin user, it will throw the error https://i.imgur.com/2w2Na1o.png
Actual Results
- ❌ It doesnt throw an error. It simply remove the role
- ❌ It only triggers the error, when removing all users Role at the same time
Additional Notes
Instead of an error, a Notice message should appear saying something like: "You cannot remove Admin user privileges".
But I think this is something for another report, not for this one. This one technically is solving the simple user role editing, but, I'm not confident if it’s bringing a new bug (being able to remove capabilities of Admin account)
Some additional opinions are required to evaluate how this should behave.
Supplemental Artifacts
Video: Removing all roles at once
Removing just admin role
This pull request resolves the issue encountered when attempting to change a user's role to 'None' using the 'Change Role To...' bulk option within multisite, specifically at the URL
wp-admin/network/site-users.php?id=xxx
.Trac ticket: https://core.trac.wordpress.org/ticket/61100