the comments/[id] endpoints should have the same permissions checks as the comments endpoint

[tharsheblows]

I noticed this because custom comment types don't show up on the comments endpoint but it's possible to access them via their id in comments/[comment_id].

get_item_permissions_check should do the same checks as get_items_permissions_check in WP_REST_Comments_Controller

[TimothyBlynJacobs]

I’m really not sure what to do with this. It seems incredibly plausible that someone is making use of this functionality and as such, locking it down might break their code. However, it also could be considered an information disclosure issue.

Given that we haven't seen more reports, I'm tempted to leave this as is, but I wouldn't really be happy with that. Maybe a Make.Core post to see if people are using this functionality and address it early in the cycle?

Either way, we should make a decision.

[tharsheblows]

To recap: for general users (users who do not have the edit_posts capability) the comments endpoint only lists comments which have been approved.

It will throw an error for the whole list if there is a query on the posts (eg wp-json/wp/v2/comments?post[]=x) and the user doesn’t have permission to read one of them — ie if they can read all but one they’ll still get an error for the list. This should probably be fixed and those checks (read permission on the post and what to do if the post_id === 0) could be added to get_items with the comment simply not listed if it fails it. (It’s irrelevant here but worth noting.)

Users without the edit_posts capability cannot search comments by author or author details which makes total sense. They also cannot search by the comment type and can only view comments of the type ‘comment’ (which is also null if you’re looking in the database, they’re the same) and they can’t search for unapproved comments.

It was the comment_type check that I mentioned above — there’s no such check on the individual comment endpoint in get_item and it’s possible to iterate through them, picking up comments of all types including pingbacks and trackbacks. The attached patch illustrates this.

The absence of a way to register_comment_type makes deciding what to do a bit more difficult. If people are using custom comment types, they’re rolling their own method most likely using wp_insert_comment and, I would assume, extending the comments controller in the REST API — the alternative is iterating through the endpoint to tease out the comments of the proper type.

This seems a bit of edge case — someone using a custom comment type and then not extending the controller, and maybe it’s more correctly an edge case of an edge case, there are not a lot of people using custom comment types.

I think that almost no one would be effectively scraping all of the individual comment endpoints for use in legitimate things and there might be unintended leakage of information by people who don’t realise those are public – we should apply the same permissions to those as to the comments endpoint.

[tharsheblows]

For illustrative purposes only

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

[adamsilverstein]

In 61276:

Comments: ensure unauthenticated users cannot access the single comment endpoint for notes.

Fix an issue where notes could be accessed by unauthenticated users by using the single comment REST API endpoint and passing the comment ID (/wp/v2/comments/<ID>). This fix only affects the note type.

Props adamsilverstein, peterwilsoncc, westonruter.
See #44157.

[adamsilverstein]

Leaving this open to consider further tightening restrictions for the single comment endpoint any custom comment type. Perhaps we can start with a "doing_it_wrong" waring in 7.0, then completely block in 7.1?

Also, we should consider backporting this fix to the 6.9 branch @peterwilsoncc.

[adamsilverstein]

Thanks for the bug report @tharsheblows - I inadvertently left you off the Props line in my commit message so I have added your props manually in our tool.

@peterwilsoncc I went ahead and committed what we have. I would be in favor of backporting this to 6.9, what do you think?

FWIW I support backporting to 6.9, but defer to a proper committer review/approval there.

[peterwilsoncc]

@adamsilverstein Approved. This looks good for backport and tests well.

[adamsilverstein]

Thanks @peterwilsoncc - I will get this backported.

[johnbillion]

@adamsilverstein Should this be milestoned for 6.9?

[adamsilverstein]

@adamsilverstein Should this be milestoned for 6.9?

I'm not sure - we are fixing this for Notes only in 6.9 and planning a more compete fix in a future release, maybe over two releases 7.0 and 7.1 - since changing the default behavior for other custom comment types could break plugin implementations that expect the current behavior to continue working.

[adamsilverstein]

@johnbillion if its helpful to have a ticket milestoned 6.9 associated with this fix, I can create a new ticket for just the Notes specific issue.

[peterwilsoncc]

In 61297:

Comments: ensure unauthenticated users cannot access the single comment endpoint for notes.

Fix an issue where notes could be accessed by unauthenticated users by using the single comment REST API endpoint and passing the comment ID (/wp/v2/comments/<ID>). This fix only affects the note type.

Reviewed by peterwilsoncc.
Merges [61276] to the 6.9 branch.

Props adamsilverstein, peterwilsoncc, westonruter, tharsheblows.
See #44157.

Committed in r61276.

Backported to 6.9 branch in r61297.