Deprecate the rest_enabled filter

[pento]

With the introduction of Core functionality that relies on the REST API existing (#38342 and #38343), being able to turn off the REST API is roughly the equivalent of being able to turn off admin-ajax.php - something that will break your site.

As more Dashboard (and plugin!) functionality comes to rely on the REST API, it's irresponsible of us to provide a method for disabling the REST API entirely.

I'd also be inclined to throw a _doing_it_wrong if anything is actually attached to the rest_enabled filter.

[johnbillion]

Devil's advocate for the purposes of considering the impact of removing this filter: What will people who really want to disable the REST API start doing instead in order to disable it? What will break in the process? Will this cause other headaches to core or site owners?

[pento]

There are still options to disable it, but it will hopefully encourage a more considered approach. For example, returning an error on the rest_authentication_errors filter will prevent all REST API calls, but would hopefully encourage a little bit of nuance - returning an error when ! is_user_logged_in(), for example.

I'm inclined to think that an off switch like rest_enabled is too much of a blunt instrument.

[jorbin]

First pass at some language and removal of code. Still needs some unit tests.

[pento]

@dmchale: Hi there! It looks like yours is the only WordPress.org plugin that makes use of this filter - as the REST API is going to become a dependency for wp-admin, disabling the REST API won't work anymore.

I'd recommend replacing it with a check that the user is logged in, something like:

function only_allow_logged_in_rest_access( $access ) {
        if( ! is_user_logged_in() ) {
                return new WP_Error( 'rest_cannot_access', __( 'Only authenticated users can access the REST API.', 'disable-json-api' ), array( 'status' => rest_authorization_required_code() ) );
        }

        return $access;
}
add_filter( 'rest_authentication_errors', 'only_allow_logged_in_rest_access' );

Ticket note: I'm inclined to do a make/core dev note for this change, too, as there are ​several snippets floating around to do the same thing.

We can document rest_authentication_errors as the alternative, and include this example on docs page.

[dmchale]

@pento thanks for the heads up. Is it merely being deprecated during the 4.7 cycle, or will the filter already be removed entirely then? I agree a make/core post would be great regardless since I know I've seen Ryan post it as a solution for people numerous times, so I'm sure there's many installs out there NOT using my plugin to achieve the same thing.

[pento]

The current plan is 38446.diff​ - the filter will run, but the return value will be ignored.

I'm waiting for input from the REST API team before we go ahead with any changes, though.

[rmccue]

I am -1 on removing this, for a few reasons.

1. I think we should give people enough rope to hang themselves if they really want. The fact is that the REST API does introduce a new attack surface (the Flash XSS exploit for example).
2. The admin should be designed to work without JavaScript, and hence without the REST API. For people that disable the API, the progressive enhancement should drop back to standard interactions.

That said, there is a way to remove the endpoints without needing to use this filter:

remove_action( 'rest_api_init', 'create_initial_rest_routes', 99 );

I think we should start publicising this method rather than the sledgehammer that is rest_enabled.

@dmchale Maybe you could switch your plugin to that method? Unsure if that's changing the functionality too much.

[pento]

Replying to rmccue:

1. I think we should give people enough rope to hang themselves if they really want.

Conversely, they already have plenty of ways to explode their site, they don't really need one more.

The fact is that the REST API does introduce a new attack surface (the Flash XSS exploit for example).

Everything we do introduces a new attack surface, but we don't add an off button for each feature. If a REST API vulnerability is discovered, a Core auto update is far more efficient at protecting sites than a filter to disable the REST API.

2. The admin should be designed to work without JavaScript, and hence without the REST API. For people that disable the API, the progressive enhancement should drop back to standard interactions.

There are already significant parts that either require JS (the Customizer), or the no-js fallback is barely usable (the post editor). As more development moves to a JS first (or JS only) model, I expect the balance to tip more in a Customizer direction. Indeed, as the Customizer moves to using the REST API, disabling the API (or particular endpoints) will cause all sorts of exciting behaviour.

That said, there is a way to remove the endpoints without needing to use this filter:

remove_action( 'rest_api_init', 'create_initial_rest_routes', 99 );

I think we should start publicising this method rather than the sledgehammer that is rest_enabled.

Per discussion on #38339, removing the core endpoints is just as bad, we shouldn't be encouraging that.

[dmchale]

I am fine with whatever updates are needed for the plugin. I will probably need to add a Settings page no matter what happens, so that site admins can configure whatever solution is deemed best practice.

I'll keep an eye on things from the background until a decision has been made, and figure out the plugin's best reaction to the ecosystem from there.

[jorbin]

This decision is less of an enhancement, and more of a task for us to decide if we want to do it, changing the type accordingly.

The admin should be designed to work without JavaScript, and hence without the REST API. For people that disable the API, the progressive enhancement should drop back to standard interactions.

There are parts of the admin that need to always work without JavaScript, namely the ability to change themes and the ability to activate/deactivate plugins. Beyond that, JavaScript is a foundational part of the web that we all should learn. deeply. And it's something that features can and should use.

[pento]

38446.2.diff​ updates the deprecated message, and adds a unit test.

[pento]

In 38947:

REST API: Deprecate the rest_enabled filter.

As the REST API becomes more integral to WordPress Core, turning it off will cause a... suboptimal experience. If we don't want it to be turned off, the off switch needs to be removed.

Props jorbin, pento.
Fixes #38446.

[jnylen0]

Noting that rest_request_before_callbacks (​ref) is probably a better filter to use than rest_authentication_errors, because it gets access to the $request object.

This makes it really easy to only restrict, for example, the built-in wp/v2 endpoints by checking the beginning of $request->get_route().

[SergeyBiryukov]

In 40038:

REST API: After [38947], improve the wording of the message to clarify that rest_authentication_errors is a filter.

See #38446.

[wturrell]

Any case for revisiting this, given recent events?

[dmchale]

Not sure what needs to be revisited? If you'd like to disable the REST API to forward-facing site visitors you can use the code that @pento posted above which is nearly identical to the ​example found in the REST API documentation, or you can install the ​plugin that does it for you. You just can't use the old rest_enabled filter anymore.

[SergeyBiryukov]

In 47044:

I18N: Replace rest_authentication_errors filter name with a placeholder in a translatable string.

Follow-up to [38947] and [40038].

See #38446.

[desrosj]

As far as I can tell, this never received a dev note. I'm going to remove the needs-dev-note keyword as 3 years have already passed.