WordPress.org

Make WordPress Core

Opened 3 years ago

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

Download all attachments as: .zip

Change History (25)

koke3 years ago

comment:1 koke3 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

koke3 years ago

Second patch closer to dashboard behavior

comment:2 koke3 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 koke3 years ago

  • Cc jbernal@… added

comment:4 Anton Torvald2 years ago

Added patch for wp.getComments and wp.getComment

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

comment:6 daniloercoli2 years ago

  • Cc ercoli@… added

comment:7 josephscott2 years ago

Have you compared this with #19916 yet?

nprasath0022 years ago

Fixes cap checks

comment:8 nprasath0022 years ago

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

koke2 years ago

koke2 years ago

comment:9 koke2 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

ericmann2 years ago

Refresh patch to match latest version of trunk.

comment:10 ericmann2 years ago

  • Keywords commit added

comment:11 westi2 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.

koke2 years ago

Unit Tests for wp.getComments and wp.editComment

comment:12 c3mdigital8 months ago

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

comment:13 jeremyfelt8 months ago

  • Keywords mobile removed

comment:14 follow-up: markoheijnen8 months ago

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

comment:15 in reply to: ↑ 14 ; follow-up: dd328 months 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: nacin8 months 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 jeremyfelt8 months 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 dd328 months 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.