Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect and show a notice when REST API is disabled #8549

Closed
Clorith opened this issue Aug 5, 2018 · 18 comments
Closed

Detect and show a notice when REST API is disabled #8549

Clorith opened this issue Aug 5, 2018 · 18 comments
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability Framework Issues related to broader framework topics, especially as it relates to javascript REST API Interaction Related to REST API [Type] Enhancement A suggestion for improvement.

Comments

@Clorith
Copy link
Member

Clorith commented Aug 5, 2018

Some users disable the REST API, this means that even though post types etc enable it with show_in_rest set to true Gutenberg will load up and remain blank/un-usable.

I saw someone mention they did not have the REST API enabled, and that reminded me that security plugins (I will not say if they all do it, because I've not tested, there are just so many...) also turn the API off. If I recall correctly, some of them even default to disabling the REST API, so the users won't even know it's a thing.

When Gutenberg is opened to edit a post or page we already start then by doing REST requests, we should be able to detect that we're not getting a response for content we know should exist (editing) and redirect the editor to the classic one with a notice informing the user what is going on as we just moved them.

@youknowriad
Copy link
Contributor

Definitely, longer term we should also deprecate "disabling the REST API" globally IMO.

@danielbachhuber danielbachhuber added [Type] Enhancement A suggestion for improvement. Backwards Compatibility Issues or PRs that impact backwards compatability labels Aug 6, 2018
@danielbachhuber danielbachhuber changed the title Handling of sites that disable the REST API Detect and show a notice when REST API is disabled Aug 6, 2018
@danielbachhuber danielbachhuber added Framework Issues related to broader framework topics, especially as it relates to javascript REST API Interaction Related to REST API labels Aug 6, 2018
@danielbachhuber
Copy link
Member

I've updated the title to "Detect and show a notice when REST API is disabled" for clarity.

@Ipstenu
Copy link
Contributor

Ipstenu commented Aug 7, 2018

As much as it pains me to say this, we can’t just stop people from disabling the REST API even though it breaks things. We allow them to break things. People have built some wild workflows around this already. Showing a notice (and recommending the install of the Classic Editor) is going to be a necessary evil 😕

I would recommend making it hookable, so if someone disabled it they could change the messgae. Like “You’re trying to edit posts on the wrong site, go over to our sandbox”

@youknowriad
Copy link
Contributor

youknowriad commented Aug 7, 2018

I don't expect Gutenberg to be the only part of WordPress using the REST API, so I really think we should clarify that the REST API is not evil and it shouldn't be disabled. The more we move towards JavaScript UIs, the more this issue will be visible.

Why do people disable the REST API and don't disable the AJAX actions used in All WordPress, these are the exact same thing, just not "normalized"? It just doesn't make sense to disable the REST API since it's not only for plugin usage.

@TimothyBJacobs
Copy link
Member

As far as I am aware, disabling the rest api globally was already completely removed. The filter just adds a notice but doesn’t disable anything. The recommended way to “kinda” disable the rest api is to add additional permission checks so the data isn’t publicly accessible. If implemented properly, Gutenberg should still work ( the current user would have permission anyways ).

Note, that is completely separate from show_in_rest which there is a core ticket for 42785.

@Ipstenu
Copy link
Contributor

Ipstenu commented Aug 7, 2018

If implemented properly, Gutenberg should still work ( the current user would have permission anyways ).

Right that's the problem. Also as proven with Cloudflare, hosting does weird things. So a failsafe either in Gutenberg or on the Rest API itself feels like a prudent, user aware choice here.

@pbiron
Copy link

pbiron commented Aug 7, 2018

Some users disable the REST API, this means that even though post types etc enable it with show_in_rest set to true Gutenberg will load up and remain blank/un-usable.

I am one of those.

A while back I had a particularly malicious bot trying to hack one of my sites via SQL Injections. Luckily, my security plugin's WAF blocked those attempts (thank you Wordfence!) . I then noticed the same malicious bot was hitting various REST API endpoints! Since I didn't have bandwidth at the time to investigate how vulnerable those endpoints were and the "normal" operation of the site did not need the REST API, I simply added a WAF rule to block all REST API access. I fully intended to go back at some point and be a little more discriminating about which REST API accesses to block...of course, I never did 😊

A few days ago I installed GB on the site and the second I tried to edit a post I remembered I was still blocking the REST API 😖

I removed the WAF rule I had added and a little bit of code to restrict REST API access to logged in users, and now GB is happy.

Definitely, longer term we should also deprecate "disabling the REST API" globally IMO.

Until some basic aspect of core requires the REST API, forbidding (or even recommending) users from disabling/block it would be bad, IMHO.

@TimothyBJacobs
Copy link
Member

That’s kinda the issue :) is that plugins aren’t using any “correct” way of disabling the REST API, but rather outright blocking requests to the path or adding some global block somewhere. Which means I’m not sure how Gutenberg distinguishes between an error from someone “disabling” the rest API and an actual error or intermittent error.

As more and more code uses the REST API in core, I think disabling the rest api in that manner should be akin to disabling admin-ajax.php where we should certainly not crash the page, but just show an error message as opposed to trying to intelligently redirect the user back to the classic editor.

@danieltj27
Copy link
Contributor

I'm coming along from issue #8960 where I raised this flag.

Irrespective of whether the REST API is disabled or not, that's not the point @Clorith was making (I don't think anyway). The issue I raised above was because the permalink cache hadn't been flushed/built.

I created a new site (locally) and the first thing I did was login and install Gutenberg from the callout. From there I then went to the Gutenberg demo page (although any Gutenberg page would be fine in this example) and it was just.... white, blank.

I didn't disable the REST API. Granted, the REST API was in a 'disabled-like' state, but nonetheless it wasn't working and so showing nothing to me is very extreme.

There needs to be a message when it's not loadable because there's any number of potential issues which may mean Gutenberg can't connect to the API and as such can't show anything. Not showing anything at all and expecting the API to always work is the worst UX.

There needs to be a hookable notice stating why there is a white, blank page.

@youknowriad
Copy link
Contributor

Can we flush the permalinks when we install WordPress? Or more exactly, why can't we?

@danieltj27
Copy link
Contributor

It'd be a good idea to do so, but edge cases where the REST API isn't available for whatever reason (disabled or not), it's not a very good UX to not show anything at all because at that point your site is pretty useless if you don't have the Classic Editor.

@Ipstenu
Copy link
Contributor

Ipstenu commented Aug 15, 2018

So basically what we're saying is that there are edge cases where, for whatever reason, Gutenberg won't run due to the rest API being 'down.' Which yes, will pose a problem down the line similar to how jacked up WP gets when people disable jquery (look, I'm just the messenger, don't ask me WHY, I have no idea). Which means in some ways this should be on the CORE pre-flight checklist. Can WP do what it needs?

Options:

  1. Gutenberg checks for what it needs, like a proper plugin
  2. Someone patches core to add the rest API on to it's own pre-flight checks

Additionally we should probably pitch a core patch to flush permalink post install.

Does that sounds about right?

@Clorith
Copy link
Member Author

Clorith commented Aug 15, 2018

Yup, sounds right. I'd put a dime in on upping the likelihood one notch from "edge case" to "unlikely but not unexpected" though, looking at how things are handled by plugins, hosts, etc.

@pento
Copy link
Member

pento commented Aug 16, 2018

@Ipstenu is correct, y'all should always listen to what she says. 🙂

For Gutenberg:

  • On plugin activate, do a hard flush of the rewrite rules.
  • On page load, confirm we can actually reach the REST API.

For Core:

@Hanewali
Copy link

In #9150 problem might be different. I just had the same issue (Macbook, Gutenberg not working on Chrome&Safari (Worked on Firefox, thou)). Did a total restart (i mean with cleaning cache of website) and started working, after stopping after update to 3.6.2.

@danielbachhuber danielbachhuber added this to the WordPress 5.0 milestone Oct 3, 2018
@danielbachhuber
Copy link
Member

Hm. The REST API works without pretty permalinks, so that shouldn't be a dependency on Gutenberg's end.

@Ipstenu
Copy link
Contributor

Ipstenu commented Oct 3, 2018

Possibly the permalinks 'fix' is just the same as flushing the rewrite rules? That is if WP somehow got stuck in a weird state with them off but not really?

@danielbachhuber
Copy link
Member

Closing in favor of #10492

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability Framework Issues related to broader framework topics, especially as it relates to javascript REST API Interaction Related to REST API [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

9 participants