Make WordPress Core

Opened 18 months ago

Closed 3 weeks ago

#59440 closed defect (bug) (fixed)

WP_Comments_List_Table bulk actions do not account for user permissions.

Reported by: snicco's profile snicco Owned by: audrasjb's profile audrasjb
Milestone: 6.8 Priority: normal
Severity: normal Version: 6.3.1
Component: Comments Keywords: good-first-bug has-patch has-unit-tests has-testing-info
Focuses: ui, administration Cc:

Description

Unlike other list tables, the comment list table's get_bulk_actions()
does not check the current user's permissions which leads to a confusing UX if a user can for example, edit comments, but not delete them (due to custom permissions).

<?php
        protected function get_bulk_actions() {
                global $comment_status;

                $actions = array();

                if ( in_array( $comment_status, array( 'all', 'approved' ), true ) ) {
                        $actions['unapprove'] = __( 'Unapprove' );
                }

                if ( in_array( $comment_status, array( 'all', 'moderated' ), true ) ) {
                        $actions['approve'] = __( 'Approve' );
                }

                if ( in_array( $comment_status, array( 'all', 'moderated', 'approved', 'trash' ), true ) ) {
                        $actions['spam'] = _x( 'Mark as spam', 'comment' );
                }

                if ( 'trash' === $comment_status ) {
                        $actions['untrash'] = __( 'Restore' );
                } elseif ( 'spam' === $comment_status ) {
                        $actions['unspam'] = _x( 'Not spam', 'comment' );
                }

                if ( in_array( $comment_status, array( 'trash', 'spam' ), true ) || ! EMPTY_TRASH_DAYS ) {
                        $actions['delete'] = __( 'Delete permanently' );
                } else {
                        $actions['trash'] = __( 'Move to Trash' );
                }

                return $actions;
        }

The correct capability to check for here would be "edit_comment" and return an empty array on permissions mismatch.

There does not seem to be granularity in map_meta_cap for comments - only edit_comment for all actions (I think).

Attachments (5)

full-permissions.png (49.8 KB) - added by snicco 18 months ago.
Here is a screenshot with the user having full permissions. You can see that each comment has individial actions to delete, edit, etc.
Screenshot from 2023-09-25 12-04-49.png (45.6 KB) - added by snicco 18 months ago.
Here is a screenshot with the user having the "edit_comment" permission removed. All individual row level actions are removed, but bulk actions are still shown in the UI
59440.patch (776 bytes) - added by iflairwebtechnologies 2 months ago.
We have hide bulk action dropdown base on user capability
without-patch.png (389.7 KB) - added by shanemuir 2 months ago.
Without patch
with-patch.png (319.2 KB) - added by shanemuir 2 months ago.
With patch applied.

Download all attachments as: .zip

Change History (23)

@snicco
18 months ago

Here is a screenshot with the user having full permissions. You can see that each comment has individial actions to delete, edit, etc.

@snicco
18 months ago

Here is a screenshot with the user having the "edit_comment" permission removed. All individual row level actions are removed, but bulk actions are still shown in the UI

#1 @johnbillion
2 months ago

  • Keywords good-first-bug needs-patch needs-unit-tests added

@iflairwebtechnologies
2 months ago

We have hide bulk action dropdown base on user capability

This ticket was mentioned in Slack in #core by iflairwebtechnologies. View the logs.


2 months ago

#3 @shanemuir
2 months ago

Hey @iflairwebtechnologies

Happy to test this patch if you submit PR :)

UPDATE: just spotted your slack message, looks like you are awaiting a review on your patch.

Last edited 2 months ago by shanemuir (previous) (diff)

#4 @shanemuir
2 months ago

Reproduction Report

Description

This report validates whether the issue can be reproduced.

Environment

  • WordPress: 6.8-alpha-59274-src
  • 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.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. ✅ Error condition occurs (reproduced).

Additional Notes

Comments bulk actions remain regardless of the current users permissions.

Testing roles:
Administrator
Contributor

This ticket was mentioned in PR #8154 on WordPress/wordpress-develop by @iflairwebtechnologies.


2 months ago
#5

  • Keywords has-patch added; needs-patch removed

This ticket was mentioned in Slack in #core by iflairwebtechnologies. View the logs.


2 months ago

#7 @shanemuir
2 months 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/8154.diff

Environment

  • WordPress: 6.8-alpha-59274-src
  • 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.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. ✅ Issue resolved with patch.

Additional Notes

Tested with and without patch as administrator and contributor

Supplemental Artifacts

Added screenshots as attachments.

@shanemuir
2 months ago

Without patch

@shanemuir
2 months ago

With patch applied.

This ticket was mentioned in PR #8264 on WordPress/wordpress-develop by @jignesh.nakrani.


6 weeks ago
#8

  • Keywords has-unit-tests added; needs-unit-tests removed

Core ticket:https://core.trac.wordpress.org/ticket/59440

Added detailed PHPUnit tests to show bulk action drop-down menu for comments table list based on user capability.

Trac ticket:

This ticket was mentioned in Slack in #core by iflairwebtechnologies. View the logs.


5 weeks ago

This ticket was mentioned in Slack in #core by iflairwebtechnologies. View the logs.


4 weeks ago

#11 @shanemuir
4 weeks ago

  • Keywords has-testing-info added

This ticket was mentioned in Slack in #core by iflairwebtechnologies. View the logs.


4 weeks ago

@narenin commented on PR #8264:


4 weeks ago
#13

@jigneshnakrani088 Please fix the PHPCS issues and feedbacks.

#14 @audrasjb
3 weeks ago

  • Milestone changed from Awaiting Review to 6.8
  • Owner set to audrasjb
  • Status changed from new to reviewing

Self assigning to see if we can commit this in 6.8 cycle.

This ticket was mentioned in PR #8415 on WordPress/wordpress-develop by @shanemuir.


3 weeks ago
#15

…s/wordpress-develop/pull/8264)

Trac ticket: https://core.trac.wordpress.org/ticket/59440

Address the feedback and issues from https://github.com/WordPress/wordpress-develop/pull/8264.

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@audrasjb commented on PR #8154:


3 weeks ago
#16

There were still some WPCS issues. I added a commit to fix them.

@audrasjb commented on PR #8154:


3 weeks ago
#17

## Before patch
Logged in as a contributor.
https://github.com/user-attachments/assets/b0d70b71-2d5f-4f35-9e8c-6c303277c65c

## After patch
Logged in as a contributor.
https://github.com/user-attachments/assets/292ec475-a0c7-4fa1-a5c1-36d4824b1433

#18 @audrasjb
3 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 59877:

Comments: Remove bulk action dropdown depending on user caps.

This changeset adds a conditional to show the comments bulk actions dropdown only when the current user has moderate_comments capability.

Props snicco, iflairwebtechnologies, shanemuir, audrasjb.
Fixes #59440.

Note: See TracTickets for help on using tickets.