#38820 closed defect (bug) (fixed)
REST API: Clients should not be allowed to set arbitrary comment_type's
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Comments | Keywords: | has-patch has-unit-tests |
Focuses: | rest-api | Cc: |
Description
The Comments API currently allows unauthed users to create comments with any comment_type
, this includes 'comment'
(which internally is sanitized to ''
), 'pingback'
, and 'foobar'
/'custom_internal_comment_type'
.
The comments API should only allow a user to create a standard comment, anything other than that should be done through another specialised endpoint, or by a plugin enabling the extra comment_type
ability.
Attachments (3)
Change History (12)
#2
in reply to:
↑ 1
;
follow-up:
↓ 4
@
7 years ago
Replying to dd32:
I'll also add that I'm not sure a user with
moderate_comments
capability should be allowed to set this field either, but followed that inline with what the other fields in the API endpoint require. I'd be all for rejecting all requests which attempted to set it (unless a plugin had allowed it somehow).
This seems right to me with respect to updates. I don't think there's precedent elsewhere in core for allowing 'moderate_comments' users, or anyone, to change comment types. And in fact, update_item()
already disallows the changing of comment types. If this isn't about permissions (and I don't think it is) then the check probably belongs in create_item()
. See 38820.2.diff.
Is the intent to support 'trackback' and 'pingback' creation via the core endpoint? @dd32 your patch hardcoded 'comment' only, but my patch includes all three core types.
This ticket was mentioned in Slack in #core by helen. View the logs.
7 years ago
#4
in reply to:
↑ 2
@
7 years ago
Replying to boonebgorges:
Replying to dd32:
I'll also add that I'm not sure a user with
moderate_comments
capability should be allowed to set this field either, but followed that inline with what the other fields in the API endpoint require. I'd be all for rejecting all requests which attempted to set it (unless a plugin had allowed it somehow).
This seems right to me with respect to updates. I don't think there's precedent elsewhere in core for allowing 'moderate_comments' users, or anyone, to change comment types. And in fact,
update_item()
already disallows the changing of comment types. If this isn't about permissions (and I don't think it is) then the check probably belongs increate_item()
. See 38820.2.diff.
Yep, it prevents you changing the comment type after-the-fact (which sounds correct to me). I put the checks into the comment creation permission check function as that's where permission checks in the rest api belong AFAIK.
My comment about moderate_comments
was that currently the rest of the API allows a user with that cap from overriding most of the checks in place, so I went with that for this one too - although I think that should probably be reviewed too. I'm all for removing that check from my patch.
Is the intent to support 'trackback' and 'pingback' creation via the core endpoint? @dd32 your patch hardcoded 'comment' only, but my patch includes all three core types.
It currently allows that, but I don't think that's correct - a trackback/pingback shouldn't be able to be created via a comment creation api, that should only be created via the trackback/pingback entry points (and their validation/sanitization of the pingback/trackback).
@
7 years ago
Only allow comments to be created with a comment type of comment
, otherwise return error.
#6
@
7 years ago
In 38820.3.diff only a comment type of comment
is allowed during comment create. Otherwise, a WP_Error
is returned.
Also includes a test for comment
being the default value in the schema for the type
property.
I'll also add that I'm not sure a user with
moderate_comments
capability should be allowed to set this field either, but followed that inline with what the other fields in the API endpoint require. I'd be all for rejecting all requests which attempted to set it (unless a plugin had allowed it somehow).