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

Don't use search with user IDs #37

Open
caercam opened this issue May 4, 2020 · 2 comments
Open

Don't use search with user IDs #37

caercam opened this issue May 4, 2020 · 2 comments

Comments

@caercam
Copy link

caercam commented May 4, 2020

If an ID is passed to the fetchUser() function it tries to find the corresponding user using a search query to REST API. If not, it falls back to email and then username.

TL;DR

Don't do that. It's absolutely not reliable and will return the wrong user, is it returns anything at all.

Detailed explanation

WP_REST_Users_Controller uses WP_User_Query which, when the search query is numeric, performs the search on wp_users user_login and ID columns; This means any user with a login matching the searched ID will match the search. Say you have two users with IDs 1234 and 4567. Set user 4567 with '1234' as login and /wp-json/wp/v2/users?search=1234 will get you user 4567 instead of 1234.

Moreover, only users considered as authors are allowed to be accessed through the API. If none of users 1234 and 4567 from the above example has ever posted anything, they won't show at all in the search results. Even worse, authors in this case are users with published posts from REST API readable post types, not users with author role, which means even if user 1234 published something, if that something is not allowed to show in the API, user won't show on the search results.

Solution

If an ID is passed to fetchUser, use the /wp-json/wp/v2/users/{user_id} endpoint.

Light of hope

This should not affect a lot of people, mostly because user login can not be modified directly from the dashboard, you have to go directly through the database; It might affect sites with open user registration, haven't tested that though. I bumped across this while building an app for a site with 10,000+ users with specific login structure that are created through a different API, and notice the user search was not behaving the way I expected it to.

@SachinGanesh
Copy link
Collaborator

Hi, Thank you for the feedback. Is it possible to raise a PR with the fix?

@ellie-me
Copy link

Please see #51 as it's a related issue to this, after investigating these issues more in depth I had to deal with them on my application, I have found that WordPress only has only two options, this fetch method to search for an user or the id method; the search method is unreliable because it does not list all users by default and the search can fail as there are no guarantess that it will work. The same behavior happens if you get the whole list of users and later try to filter the users on the client (Flutter).

Good news is that after doing a test with oauth and jwt authentication I found that there's no other best way rather than using the "me" endpoint to get the logged in user with a token obtained from authentication. I'm working for my PR to use "me" as default when you have JWT authentication enabled, application passwords or Oauth2 user credentials, however, any other method without any sort of authentication will likely route to search user with email or username because WordPress does not provide other methods and without having to do a manual implementation of any Authentication flow (be it application passwords, jwt or oauth).

People should be more aware of what are the implications of using the WordPress REST API for applications, and that it needs an specific security setup in place before getting this plugin to work properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants