REST API: Support reading public settings, implement context handling

[scruffian]

It would be good to make it possible to read and update individual site settings at

/wp/v2/settings/title

for example.

This is needed as part of ​https://github.com/WordPress/gutenberg/pull/18760

[scruffian]

Patch for REST API

[scruffian]

Fix the update method

[scruffian]

Remove the hacks for testing

[scruffian]

I added this Gutenberg PR to change over to this API: ​https://github.com/WordPress/gutenberg/pull/18970

[scruffian]

Patch for REST API

[scruffian]

I updated the patch to expose all "public" settings on the same route, for simplicity.

[pento]

@scruffian: Nice work exploring this!

I'm pondering whether we really need to register a new route here: instead, the existing route can just return particular settings based on whether the current user has read access to all settings, or just those marked public.

48885.diff​ is an experiment along those lines, what do you think of this direction?

@TimothyBlynJacobs, do you have thoughts here? You mentioned concerns over on GB18760 with exposing read-only access to the entire settings API to non-admins, which I agree is a non-starter. Restricting which settings show to non-admins seems like it would cover this problem, however.

[TimothyBlynJacobs]

Yeah this is tricky. Even partial access could be a security concern as I described in #48812. We don't provide a filter to modify the response in the settings controller itself. So if you want to do any modification, you'd have to do it in rest_request_after_callbacks. Something like this for instance.

<?php
add_filter( 'rest_request_after_callbacks', static function ( $response, $route, WP_REST_Request $request ) {
        if ( 0 !== strpos( $request->get_route(), '/wp/v2/settings' ) ) {
                return $response;
        }

        if ( is_wp_error( $response ) || $response->is_error() ) {
                return $response;
        }

        $response->data['do_some'] = 'privileged thing';

        return $response;
}, 10, 3 );

Returning a partial response when the user has no permissions would mean this previously valid mechanism of extending or modifying the response could now potentially reveal sensitive information.

I'm not sure how to evaluate how prevalent this pattern is, but I don't think it is particularly "weird" so to speak. I'm not seeing anything concerning on the ​plugins directory, no idea about custom code.

Making separate routes also doesn't necessarily solve this depending on how a developer is checking the route. strpos vs ===.

---

I think we also still need to figure out how to expose the "rendered" value of the site title to Gutenberg. That is where the bloginfo filter has been applied. I'm not sure if there's a great way of doing that on the existing settings route without breaking backwards compatibility. We couldn't just change the shape of the response to be a raw/rendered object like the title field. That would break anyone reading that field. So we would have to use something like context. Right now there is no context on the settings controller. A couple of possibilities.

1. Introduce a view and edit context. Make edit the default context. When the resource is requested in a view context, change the value to the rendered version. This would work for public access, but doesn't solve the editing requirement. Gutenberg would need to maintain both a view and edit version of the response in the data store.
2. Effectively "version" the endpoint based on a request parameter of some sort. If that request parameter is present, returns an object for the site title that includes the raw and rendered version. When returning the public version of the response, only return the rendered property.
3. Introduce the single option endpoints and use the desired response format there. This would lead to inconsistencies between the collection and single item responses.

---

I'm not wild about any of these options. They all would introduce complexity into the security sensitive settings controller. To me, it doesn't seem like any more of a desirable solution than handling it in the client. It feels strange to expose settings to the public in a manner like this.

[scruffian]

It feels strange to expose settings to the public in a manner like this.

I dont think they would be exposed to the public, only to those who had edit_posts.

[pento]

Replying to TimothyBlynJacobs:

So if you want to do any modification, you'd have to do it in rest_request_after_callbacks.

Adding new data to this response seems like an unsupported edge case: if you want to add data, add a setting. An appropriate dev-note should be published, of course, to let people know it's happening.

1. Introduce a view and edit context. Make edit the default context. When the resource is requested in a view context, change the value to the rendered version. This would work for public access, but doesn't solve the editing requirement. Gutenberg would need to maintain both a view and edit version of the response in the data store.

For Gutenberg to show an accurately rendered preview, and then to be able to edit the data, it probably needs to know how it looks in both the view and edit contexts. So, we need to send that data somehow.

2. Effectively "version" the endpoint based on a request parameter of some sort. If that request parameter is present, returns an object for the site title that includes the raw and rendered version. When returning the public version of the response, only return the rendered property.

I'm fine with versioning. We haven't done that in Core yet, so there will probably need to be a bit of work to come up with a pattern, but it's not a super complex thing to do. Eg, a simple model would look something like this:

<?php
class WP_REST_Settings_v2_1_Controller extends WP_REST_Settings_Controller {
        public function __construct() {
                parent::__construct();
                $this->namespace = 'wp/v2.1';
        }

        public function register_routes() {
                // Register the same routes, with new namespace and read permisions.

        }

        protected function get_registered_options() {
                // Updated logic for getting registered options based on current user permissions.
        }
}

3. Introduce the single option endpoints and use the desired response format there. This would lead to inconsistencies between the collection and single item responses.

This option feels like overkill, and I agree that the inconsistencies would be undesirable.

I'm not wild about any of these options. They all would introduce complexity into the security sensitive settings controller. To me, it doesn't seem like any more of a desirable solution than handling it in the client. It feels strange to expose settings to the public in a manner like this.

The problem is that we have a bunch of obviously public options (eg, blogname, blogdescription, siteurl, timezone_string) mixed in with a few obviously private options (eg, admin_email).

I don't think it's necessary to create an entirely new way of registering public options, just adding the public flag to the register_setting() call is much the same as when the show_in_rest flag was added.

Replying to scruffian:

I dont think they would be exposed to the public, only to those who had edit_posts.

I'd be inclined to expose these options publicly, rather than restricting to those with edit_posts. Otherwise, the public flag doesn't really make sense as it's named. Also, edit_posts is highly specific to the circumstances under which this REST API change is coming into being: it's reasonable to assume that an option marked as "public" should be readable by anyone.

[TimothyBlynJacobs]

Adding new data to this response seems like an unsupported edge case: if you want to add data, add a setting.

It would also be for modifying an option's representation. But I agree, and would classify it as #doingitwrong, but it makes me nervous that that it would lead to a potential information disclosure issue. Particularly since we haven't really said it is a misappropriation of that filter.

I'm fine with versioning. We haven't done that in Core yet, so there will probably need to be a bit of work to come up with a pattern, but it's not a super complex thing to do.

Yeah not technically difficult, but a lot of implications it would be interesting to explore.

The problem is that we have a bunch of obviously public options (eg, blogname, blogdescription, siteurl, timezone_string) mixed in with a few obviously private options (eg, admin_email).

These have been available or made available via the REST API index /wp-json/, WP_REST_Server::get_index().

The question seems to be whether this needs to be addressed in the client, or in the server. The complexity exists either way, I'm still not understanding why it is better to solve it in the server. These feel a lot like entirely separate contexts ( the logged in administrator, versus everyone else ) to me. But I'm just repeating myself at this point 😃. I'll let @kadamwhite and other REST API team members chime in. I may very well be making a mountain out of a mole hill.

[pento]

Historically speaking, the reason for these settings being exposed in / (as opposed to allowing non-admins read access to /wp/v2/settings) is that there wasn't an immediate need for unifying the way these settings were read, so the discussion we're having on this ticket now was punted. 🙂

I agree with @TimothyBlynJacobs's assessment that the primary question is whether the context switching should happen on the client side (read values from /, write values to /wp/v2/settings), or on the server side (alter /wp/v2/settings so all users can read public settings). I find the idea of reading and writing values to two different locations weird, though. As @scruffian mentioned in #48812, this is part of the first step towards full site editing in Gutenberg, which means now is a good time to make the API for that a bit more consistent than it currently is.

The question of how to get the rendered vs raw values is interesting. I don't think returning an object with rendered and raw properties is necessary, since non-admins probably shouldn't have access to the raw value in the first place, regardless of whether the settings is considered public or not. There are plenty of reasons why setting value might be filtered for non-admins before it's shown to them, let's not circumvent that.

In which case, we need to add view and edit context support to the endpoint. Non-admins can only GET the endpoint with view context. Admins will need to GET with view context for previewing the block, and with edit context for editing the block, and then can POST to update the setting value.

[TimothyBlynJacobs]

Admins will need to GET with view context for previewing the block, and with edit context for editing the block, and then can POST to update the setting value.

At that point, is there a benefit to consolidating everything into a single endpoint? As I understand it, the purpose of centralizing it into one endpoint is to make it easy for core-data. In this model, the data store would need to maintain two copies of the resource that are fetched differently.

Switching how we format a value based on the context would be a new pattern I believe. I can't think of an example of us doing that in core, and couldn't find any doing a brief search. Currently you can request the "maximum" level of context available to you, and retrieve access to all data. I'm not opposed to it, but I wanted to document that.

[pento]

Replying to TimothyBlynJacobs:

At that point, is there a benefit to consolidating everything into a single endpoint?

Yup, that's the benefit: one endpoint, rather than two endpoints. This isn't specific to Gutenberg (which has WordPress experts working on it), this is about ensuring the data is available in the most logical location for any developer who wants to use this endpoint.

Switching how we format a value based on the context would be a new pattern I believe. I can't think of an example of us doing that in core, and couldn't find any doing a brief search. Currently you can request the "maximum" level of context available to you, and retrieve access to all data. I'm not opposed to it, but I wanted to document that.

It's not quite the same, but this is an example of a value changing based on the context. Given a post with a post password set on it, and the password is not sent in the GET request for that post, content.rendered on the returned post object will be empty in the view context, and populated correctly in the edit context.

[scruffian]

new patch with context

[scruffian]

I added a new patch which also takes into account the context parameter.

This patch assumes that the name of settings in the rest api match the values of $show in get_bloginfo (​https://developer.wordpress.org/reference/functions/get_bloginfo/) which they do in the cases we have made public so far, but won't for all options. Should we add a new key for this mapping?

[TimothyBlynJacobs]

We discussed this in #core-restapi and decided to take another go at this as a Gutenberg PR. We're going to go with a public registration option tentatively combined with the WP_REST_Request context inspection to determine the response format. Adding support for public options alone does not solve GB's issue.

Slack reference: ​https://wordpress.slack.com/archives/C02RQC26G/p1579198597407600
PR: ​https://github.com/WordPress/gutenberg/pull/18760

[TimothyBlynJacobs]

Chatting with @scruffian I think we can move this out of the 5.4 milestone since we should be able to make the changes Gutenberg needs in the plugin itself, and it is for Full Site Editing which I don't think is landing in WP 5.4? Pinging @jorgefilipecosta as the 5.4 GB tech lead for confirmation.

[jorgefilipecosta]

Hi @TimothyBlynJacobs, thank you for bringing this to my attention.
Your assessment seems correct. Full Site Editing is not part of WordPress 5.4, and it is technically possible to implement the required endpoint in the plugin.

[TimothyBlynJacobs]

Thanks for the confirmation @jorgefilipecosta!

[TimothyBlynJacobs]

@spacedmonkey expressed an interest in working on this for 5.6. I think our tentative plan is to add support for a permission_callback and prepare_callback to register_setting.

[TimothyBlynJacobs]

Something to keep in mind with permission callbacks is that reading permission would be different than writing permission. And do we want to support a custom write permission callback? And if we do, do we need to update options.php to use that capability instead of manage_options.

Working on #48885. Decided to go a different direction than the ticket name. Use the existing endpoint but add some new callbacks.

To use this PR, here is the example I use to test.

add_action('init', function(){
	register_setting(
		'general',
		'wibble',
		array(
			'show_in_rest' => array(
				'name' => 'wibble',
				'edit_permission_callback' => '__return_true',
				'read_permission_callback' => '__return_true',
			),
			'type'         => 'string',
			'description'  => __( 'Site title.' ),
		)
	);
});

This make a setting called wibble, that will now be public to read and edit. You never do this, but it gives you an idea of how the code works.

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

@TimothyBJacobs This PR is a work in progress, but gives you an idea of what i am thinking. If you are happy with the way I did this, I will add tests.

This path looks good to me!

We should make sure that if the user passed an invalid callback the option isn't disclosed.

[spacedmonkey]

Something to keep in mind with permission callbacks is that reading permission would be different than writing permission. And do we want to support a custom write permission callback? And if we do, do we need to update options.php to use that capability instead of manage_options.

As discussed in office hours, I do not believe we should make permission callbacks apply to other parts of core like options.php. Read and edit callbacks, should be limited to the REST API. The name of the ticket and original purpose are clear. To make values from wp_options available to read and update via the REST API. To add a whole permission model to register_settings, is massively out of scope for this ticket and introduces edge cases issues we simple do not need to tackle to complete this ticket's goal.

I would love @johnjamesjacoby thoughts on the matter, as he is the maintainer of options and meta.

[spacedmonkey]

I also think that changing the scope of this ticket, this late into the 5.6 cycle, would mean we would miss the 5.6 beta 1 cut off in just under two weeks. I think we should scope the change to the smallest we can, an MVP if you will and iterate in other releases.

Thoughts

[TimothyBlynJacobs]

Based off @johnjamesjacoby's feedback, I wanted to restate what the direct needs are from the Gutenberg team out of this ticket that have gotten lost thru the various Slack and GitHub conversations.

Gutenberg makes use of a number of settings in Full Site Editing such as the website's title, posts per page, etc... This data is typically things that are or would be exposed via the REST API index at wp-json. Additionally, some of these values have a different "rendered" form and "raw" form like the site title and description.

Right now as I understand it Gutenberg either reads these values from the index, or bootstraps them in the JS settings. When interacting with them in an editable fashion, Gutenberg then uses the settings endpoint.

Architecturally, things would be simpler if Gutenberg could use the same API endpoint for both cases. To accomplish this, there are three changes we must make.

1. Allow for marking settings as public in some fashion, so that they are available in the settings endpoint to unauthorized users.
2. Add support for context to allow for settings with a raw and rendered form ( or similar patterns ).
3. Introduce a prepare_callback to allow for settings to customize their output.

We also need to make sure that the title and description settings that currently are exposed can change to outputting a raw and rendered object while maintaining backward compatibility. This could be done by only using that format when the context value is explicitly defined, or we'll have to bite the bullet and version the endpoint.

This doesn't mean it has to be done using a public flag. Allowing for callbacks to implement more specific handling makes sense to me, but it isn't mandatory. As stated in my earlier comment, my worry about implementing an edit_permission_callback is that we need to make sure it isn't bypassable via options.php. This could be done by checking for the callback specifically before the individual update_option calls, or we could introduce a manage_option meta capability perhaps.

I think if we don't take the permission callback into account at all, it will be misleading to developers who think they can use it to safely secure an option.

---

Based on the additional work required before Beta 1 in two weeks, I think we should move this to 5.7. Full Site Editing isn't shipping in 5.6, so we'll have more time to give this the refinement it needs.

[hellofromTonya]

Per Timothy's comment above, punting this ticket to Future Release to allow more soak and refinement time.

Based on the additional work required before Beta 1 in two weeks, I think we should move this to 5.7. Full Site Editing isn't shipping in 5.6, so we'll have more time to give this the refinement it needs.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

[TimothyBlynJacobs]

#38731 was marked as a duplicate.

[TimothyBlynJacobs]

Related #43210.

[spacedmonkey]

@TimothyBlynJacobs I have updated ​#534 with unit tests. Can you review again?

I think there is a good chance we could get this in WordPress 5.9.

One question is, should we make some of the none sensitive settings public. Things you can easily workout.

Examples could be public

• name
• description
• url
• home
• gmt_offset
• timezone_string
• language
• posts_per_page

Example that could behind edit_posts capabilty

• default_category
• default_post_format

The tests are all failing unfortunately :(

Working on #48885. Decided to go a different direction than the ticket name. Use the existing endpoint but add some new callbacks.

To use this PR, here is the example I use to test.

add_action('init', function(){
	register_setting(
		'general',
		'wibble',
		array(
			'show_in_rest' => array(
				'name' => 'wibble',
				'edit_permission_callback' => '__return_true',
				'read_permission_callback' => '__return_true',
			),
			'type'         => 'string',
			'description'  => __( 'Site title.' ),
		)
	);
});

This make a setting called wibble, that will now be public to read and edit. You never do this, but it gives you an idea of how the code works.

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