Update frequently-asked-questions.md

[BjornW]

Instead of of checking for not empty, we should check for a WP_Error object. This seems more consistent with the code in https://core.trac.wordpress.org/browser/tags/4.9.1/src/wp-includes/rest-api/class-wp-rest-server.php#L320

[rmccue]

This is actually intentional, as authentication methods are expected to return true when they authenticate: https://developer.wordpress.org/reference/hooks/rest_authentication_errors/

We could change this to === true || is_wp_error() though to make that clearer.

Thanks for the PR!

[roytanck]

Hi Ryan. Can you please elaborate a bit? It seems that the only check that WP_REST_Server::check_authentication() does is for the WP_Error type. At first glance, it seems that anything but an error object will result in the request being processed?

If a function hooked into 'rest_authentication_errors' - one that executes early - returns true (or any other non-null, non-WP_error value), then potential following functions written like the FAQ example will skip their (possibly stricter) checks, right? Doesn't this mean that any plugin can effectively disable other plugin's security restrictions?

[rmccue]

That's correct, and intended. true indicates that an authentication scheme was successful, WP_Error indicates that an authentication scheme was not successful, and null indicates that authentication wasn't even attempted.

Take for example OAuth 2. We only want to try OAuth 2 if there's an Authorization header (using the Bearer scheme), otherwise it shouldn't have any effect. OAuth needs to be checked before cookies, and cookies should be ignored if OAuth is being attempted (otherwise, any OAuth request in a browser that already has cookies would basically just ignore your token).

As another plugin, if you get true passed in, that means that another authentication scheme has been attempted and was successful, so you don't need to bother. When OAuth is successful, it returns true which indicates the cookie checks are irrelevant.

I hope that makes it clearer? We should clarify this in the docs, and probably reword it a little better too.

[roytanck]

Thank you for clarifying this. It does still sound to me like a door with four locks that opens if just one key fits. But then again, if a hooked function does not want to be bypassed, it can force its own check, I guess, by not skipping unless the value is already an error.

[BjornW]

@rmccue shall I rework my initial request and include your explanation given here?

[rmccue]

@roytanck The basic algorithm is:

For each authentication method: Attempt authentication If the result is an error: Return the error to the user Otherwise if the result is null: Continue to next authentication method Otherwise: Authentication is successful End loop 

It's mainly just confusing because we're using filters to achieve this.

@BjornW It would be great if you could 👍

[BjornW]

I've created a new pull-request here: #9 which aims to incorporate the discussion between @roytanck and @rmccue.