WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 2 years ago

#17981 new defect (bug)

XML-RPC wp.getComments should work for non-admins

Reported by: koke Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.2
Component: XML-RPC Keywords: has-patch needs-testing needs-refresh mobile
Focuses: Cc:

Description

Right now, if the caller doesn't have the moderate_comments permission, the XML-RPC call returns a 401 error.

A more graceful alternative would be to return the approved comments. The user may not be able to moderate, but still should be able to read/reply

Attachments (7)

patch-core-17981.diff (1.9 KB) - added by koke 4 years ago.
patch-core-17981-2.diff (1.9 KB) - added by koke 4 years ago.
Second patch closer to dashboard behavior
commentAPIfixes.patch (3.9 KB) - added by nprasath002 3 years ago.
Fixes cap checks
patch-gp-157-strings-3.diff (3.8 KB) - added by koke 3 years ago.
patch-core-17981-3.diff (3.1 KB) - added by koke 3 years ago.
17981-refresh.diff (3.5 KB) - added by ericmann 3 years ago.
Refresh patch to match latest version of trunk.
patch-core-17981-ut.diff (6.6 KB) - added by koke 3 years ago.
Unit Tests for wp.getComments and wp.editComment

Download all attachments as: .zip

Change History (25)

@koke4 years ago

comment:1 @koke4 years ago

  • Keywords has-patch added; needs-patch removed

Added patch for wp.getComments and wp.getComment. If user can't moderate comments, in only returns approved comments for wp.getComments, and returns an error if comment isn't approved for wp.getComment

@koke4 years ago

Second patch closer to dashboard behavior

comment:2 @koke4 years ago

I should have done a trac search before writing a second patch but I feel it's better ;)

Dashboard also shows unapproved comments to authors, so we check for (edit_posts OR moderate_comments) instead of just moderate_comments. The current checks for edit_comment should take care of the rest.

Also, added better error descriptions.

comment:3 @koke4 years ago

  • Cc jbernal@… added

comment:4 @Anton Torvald4 years ago

Added patch for wp.getComments and wp.getComment

Version 0, edited 4 years ago by Anton Torvald (next)

comment:6 @daniloercoli4 years ago

  • Cc ercoli@… added

comment:7 @josephscott3 years ago

Have you compared this with #19916 yet?

@nprasath0023 years ago

Fixes cap checks

comment:8 @nprasath0023 years ago

The patch fixes the issues in cap checks.
Also added validation for post_id

@koke3 years ago

comment:9 @koke3 years ago

Added a new patch (sorry about the wrong one). Tested with an Author user:

  • wp.getComments shows every comment (as dashboard does)
  • Added a new field 'can_edit' to show if the user has permission to edit/delete that specific comment. That'll allow to customize the UI for it
  • Can edit/delete comments in own posts
  • Can't edit/delete comments in others posts

I think this also solves #19916. Some questions about it:

  • Should wp.deleteComment error be "You are not allowed to delete..." instead of moderate?
  • I was going to change the error in wp.getComments to match wp.getComment, but it's a different error code (401 and 403) and could break something

@ericmann3 years ago

Refresh patch to match latest version of trunk.

comment:10 @ericmann3 years ago

  • Keywords commit added

comment:11 @westi3 years ago

Before we commit these changes I would like us to have some unit tests around the expected behaviour here.

Highest priority are tests for all the cap checks to ensure we have expected behaviour - writing the tests out makes you think a lot about this :)

Second priority are tests for the data returned especially datatypes so we can ensure continuing consistency over time.

I asked koke to see if he could write some of these in IRC today.

@koke3 years ago

Unit Tests for wp.getComments and wp.editComment

comment:12 @c3mdigital2 years ago

  • Keywords needs-testing needs-refresh added; commit removed

comment:13 @jeremyfelt2 years ago

  • Keywords mobile removed

comment:14 follow-up: @markoheijnen2 years ago

Not sure why you remove the mobile keyword. It's used by the mobile team.

comment:15 in reply to: ↑ 14 ; follow-up: @dd322 years ago

  • Keywords mobile added

Replying to markoheijnen:

Not sure why you remove the mobile keyword. It's used by the mobile team.

Since it's not a 'supported' keyword in the drop down, non-privledged users can't set it, so I expect it's probably getting removed without the users intention.

comment:16 in reply to: ↑ 15 ; follow-up: @nacin2 years ago

I'm fine with this patch — anything to make it more like how edit-comments.php actually works.

Replying to dd32:

Since it's not a 'supported' keyword in the drop down, non-privledged users can't set it, so I expect it's probably getting removed without the users intention.

Anyone can click "manual" and set additional keywords, but since it isn't listed, gardeners might not realize it is a pseudo-official keyword and remove it. I'll look into getting it listed.

comment:17 in reply to: ↑ 16 @jeremyfelt2 years ago

Replying to nacin:

Anyone can click "manual" and set additional keywords, but since it isn't listed, gardeners might not realize it is a pseudo-official keyword and remove it. I'll look into getting it listed.

That's my fault. Assumed that mobile was an old tag. :)

comment:18 @dd322 years ago

Anyone can click "manual" and set additional keywords, but since it isn't listed, gardeners might not realize it is a pseudo-official keyword and remove it. I'll look into getting it listed.

oops, I thought the Manual was limited to gardeners. Ignore me :)

Note: See TracTickets for help on using tickets.