Make WordPress Core

Opened 12 months ago

Closed 9 months ago

#57952 closed defect (bug) (fixed)

Able to delete multiple users with the Change button - STRANGE

Reported by: haritpanchal's profile haritpanchal Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.3 Priority: normal
Severity: major Version:
Component: Users Keywords: has-patch has-testing-info commit
Focuses: administration Cc:

Description

While deleting multiple users, I faced this strange behavior. I was able to delete multiple users using the Change button instead Apply

Steps to recreate this issue.

  1. Navigate to the Users screen.
  2. Select 2-3 Users.
  3. Change Bulk actions dropdown to Delete.
  4. Click Change button instead Apply.
  5. WordPress will ask you to confirm deleting those users.

Video: https://www.awesomescreenshot.com/video/15740825?key=2abd8945655f6f2c558e56650eb9fa8a

Attachments (2)

57952.diff (497 bytes) - added by costdev 12 months ago.
Force action to promote when $_REQUEST['changeit'] is set.
57952.2.diff (536 bytes) - added by SergeyBiryukov 12 months ago.

Download all attachments as: .zip

Change History (10)

@costdev
12 months ago

Force action to promote when $_REQUEST['changeit'] is set.

#1 @costdev
12 months ago

  • Keywords has-patch needs-testing has-testing-info added; needs-patch removed
  • Milestone changed from Awaiting Review to 6.3

Testing Instructions

These steps define how to reproduce the issue, and indicate the expected behavior.

Steps to Reproduce

  1. Navigate to Users > All Users.
  2. Ensure there are at least two users.
  3. Check the box beside one of the users (not admin).
  4. In Bulk actions, select Delete. Do not click Apply
  5. In Change role to, Do not select a role
  6. Click Change.
  7. 🐞 Bug occurs.

Expected Results

When reproducing a bug:

  • ❌ The user is deleted (or the delete confirmation screen appears).

When testing a patch to validate it works as expected:

  • ✅ You should see "Sorry, you are not allowed to give users that role."
Last edited 12 months ago by costdev (previous) (diff)

#2 @costdev
12 months ago

Reproduction Report

This report validates that the issue can be reproduced.

Environment

  • WordPress: 6.3-alpha-55505
  • PHP: 7.4.33
  • Server: Apache/2.4.56 (Ubuntu)
  • Database: mysqli (Server: 5.7.41-0ubuntu0.18.04.1 / Client: mysqlnd 7.4.33)
  • Browser: Chrome 111.0.0.0 (Windows 10/11)
  • Theme: Twenty Twenty-Three 1.1
  • MU-Plugins: None activated
  • Plugins: None activated

Actual Results

  1. ❌ The user is deleted (or the delete confirmation screen appears). Issue reproduced.

#3 @costdev
12 months ago

57962.diff forces the action to promote when $_REQUEST['changeit'] is set.

#4 @SergeyBiryukov
12 months ago

Thanks for the patch!

It looks like there is already some logic in WP_Users_List_Table::current_action() to set the action to promote if $_REQUEST['changeit'] is set.

The reason it doesn't work in this case is because it also checks that $_REQUEST['new_role'] is not empty, which is not the case if we did not select a role.

So I think a better solution would be to adjust that logic instead of duplicating it elsewhere, see 57952.2.diff.

#5 @Ankit K Gupta
11 months ago

Test Report ✅

Env

  • WordPress - 6.3-alpha-55615
  • Chrome Version - Version 111.0.5563.110
  • OS - macOS Monterey V12.3.1
  • Theme: Storefront Version: 4.2.0
  • PHP - 7.4.0
  • Web Server - Apache
  • Database - MySQL 5.7.28

Steps to test

  1. Access the WordPress Dashboard.
  2. Navigate to the 'Users' section.
  3. Select 2-3 Users.
  4. Change the Bulk actions dropdown to Delete.
  5. Click the Change button instead Apply.

Test result

The Change button functionality has been fixed and is now working as expected. Users are no longer being deleted when the Change button is clicked.

Screencast:

Before Fix -

https://screencast-o-matic.com/watch/c0fV6zVaryp

After Fix-

https://somup.com/c0fV6B4fsT

Actual Results

✅ Expected result- works as expected with the patch https://core.trac.wordpress.org/attachment/ticket/57952/57952.2.diff

#6 @Ankit K Gupta
11 months ago

  • Keywords needs-testing removed

#7 @SergeyBiryukov
11 months ago

  • Keywords commit added

Thanks for testing! Marking for commit consideration.

#8 @SergeyBiryukov
9 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 55864:

Users: Make sure bulk actions are only executed with the Apply button, not Change.

The Change button is supposed to perform the “Change role to...” action only, but could unintentionally be used for other bulk actions if the role was not selected.

This commit removes an extra check and ensures the correct error message is displayed in that case:

Sorry, you are not allowed to give users that role.

Follow-up to [6990], [8691], [9028], [15576], [15642], [34636], [49944].

Props haritpanchal, costdev, ankit-k-gupta, SergeyBiryukov.
Fixes #57952.

Note: See TracTickets for help on using tickets.