Sites API: Return options for Jetpack blog requests

[creativecoder]

Changes proposed in this Pull Request:

• Returns site options for blog requests
• Removes all site options for user requests when the user doesn't have at least the edit_posts capability (subscriber users)

Because this endpoint is currently using a current_user_can( 'edit_posts' ) capability check to determine whether to return site options, blog requests do not see most site options.

Some site options that were added when decorating the request on wpcom still leaked through to subscriber level users, so this change also removes all site level options for those users.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

See p1571157924035600-slack-jetpack-developers for context.

Testing instructions:

Testing this can be complex, because there are a lot of different ways the code is run on wpcom + Jetpack sites. I'd recommend the following testing methodology to verify the output.

• Copy the JSON responses of each api request and paste into a text file
• Prettify the JSON and save the file
• Make the API requests first against the production API and place all of those responses within a folder
• Make the API requests against against the sandboxed API + Jetpack development site and put those responses in a separate folder, with the same file names corresponding to each request
• Diff the folder to see any differences, and ensure that all response output is as expected

Setup

• Set up your local Jetpack development site so that it has an admin and a subscriber level user
• Make each request 4 times: as an admin user, as a subscriber user, as a logged out user, and as the blog
• For user requests, use developer.wordpress.com/console
• For blog requests, wp shell for your Jetpack development site and run echo wp_remote_retrieve_body( Automattic\Jetpack\Connection\Client::wpcom_json_api_request_as_blog( 'sites/{site_id}?force=wpcom') ); (replacing the site_id with the id of your local Jetpack development site)

Requests

• /sites/{site}?force=wpcom
• /sites/{site}
• Additionally, call these endpoint for a different Jetpack site, to ensure user and blog requests get the same response as a logged out users

Proposed changelog entry for your changes:

• Return site options when making an as_blog request to /site/{site}

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello creativecoder! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D34564-code before merging this PR. Thank you!

[jetpackbot]

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: December 3, 2019.
Scheduled code freeze: November 26, 2019

Generated by 🚫 dangerJS against f56a525

[pablinos]

I haven't properly tested this yet, but it looks good.

I thought I had mentioned this somewhere else, so apologies if I'm repeating myself, but I discovered that because of after_render_options, if you try and restrict the options returned, using the options= query parameter, you still get a certain amount of Jetpack information returned.

It's a separate issue, but we might want to consider whether this information should be returned for these requests.

[creativecoder]

I thought I had mentioned this somewhere else, so apologies if I'm repeating myself, but I discovered that because of after_render_options, if you try and restrict the options returned, using the options= query parameter, you still get a certain amount of Jetpack information returned.

@pablinos Thanks for bringing this up. I'd like to handle that issue separately from this PR, since these changes are a bit challenging to test already, given how many different ways there are to call the endpoint and process the request.

[pablinos]

@pablinos Thanks for bringing this up. I'd like to handle that issue separately from this PR

Yes, I think that's fair enough. My only concern would be if we start leaking extra information, but as we've discussed before, that shouldn't happen unless the blog token has been compromised.

[matticbot]

creativecoder, Your synced wpcom patch D34564-code has been updated.

[jeherve]

This should be good to merge. 👍

[creativecoder]

Thanks for merging this @jeherve! I'm just a little slow in looping back to it. Taking care of the wpcom patch now.