Account for REST API in private site mode
Closed this issue · 9 comments
Private site mode needs to account for the REST API. There's two issues at play:
- Making sure that this is private if the user wants that.
- Not blocking third-party apps or plugins that need the info.
An extra option is probably needed for this like how we handle feeds. So, basically, this should simply come down to finding the right hook in which to disable/block the REST API from working.
I'd like to add that this is not only applicable with the full REST API merged. The already existing oembed endpoint is already leaking information:
- existence of posts
- post titles
- author names
- author email
- author URLs
I also think that in any case the "Private Site" feature as it is needs some more explanation what it protects and what it doesn't. Depending on the use case leaking the information mentioned above might already pose a serious threat. Not even mentioning the full REST API data...
Are the oembed endpoints not a part of the REST API? Will handling this in one place not work for the other? I'm woefully ignorant of how the REST API works right now.
What I want to say is that when the REST-API infrastructure was merged we also got the oembed endpoints. So in contrast to the other endpoints of the REST-API this is already live and leaking information as we speak.
I'm also not an expert on the REST-API. What I know is that it runs pretty early on the parse_request hook.
Okay now that 4.7 is released with the REST-API this is even more pressing as currently people using the "Private Site" feature are unknowingly leaking plenty of data.
The way to disable the API is exactly what this plugin does: https://wordpress.org/plugins/disable-json-api/
It basically boils down to hooking into rest_authentication_errors and returning an error for unauthenticated users.
If you're busy I might find time to create a PR if you'd like.
@kraftner - Can I get you to test the above commit out and confirm that the new option is working?
Sure. I just tried the dev branch - should that be in a working state? I get a fatal error on activation.
Fatal error: Default value for parameters with a class type hint can only be NULL in /srv/www/wordpress-develop/public_html/src/wp-content/plugins/members/inc/class-registry.php on line 142
EDIT: I tested with a fresh VVV setup using PHP 7
Anyway just looking at the code everything looks reasonable.
The only thing I noticed is the fact that with checking empty( $result ) you make it possible to re-enable the API. I guess this is intentional, but then maybe this should be more clear from the wording along the option to disable the API in the backend.
I'm going to go ahead and close this one as fixed. Thanks for the feedback.
I don't think a warning that it can be re-enabled is necessary. Any option can be overwritten in the plugin. That's just the nature of the hook system.
Fair argument. Thx for the fix!