Make WordPress Core

Opened 8 years ago

Last modified 3 years ago

#39544 new task (blessed)

REST API: Improve users endpoint in multisite

Reported by: flixos90's profile flixos90 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: REST API Keywords: ms-roadmap
Focuses: multisite Cc:

Description

As per the discussion that happened during the past two weeks' multisite office-hours, the REST API users endpoint needs to be improved to support multisite behavior.

This ticket is supposed to act as a general task for discussion, and then for the actual implementation smaller spin-off tickets should be opened.

Currently, the four steps (possibly four tickets) we're thinking about are:

  • The users overview at wp-json/wp/v2/users should continue to only show users of that site by default, but a request like wp-json/wp/v2/users?global=true should show all users in the WordPress setup. This parameter must only be available to network administrators though, more specifically users with the manage_network_users capability. In the future a network parameter might also be introduced for support of multi networks, but at this point core does not support querying users per network. Accessing global users should be available from all sites in a setup instead of only from the main site. While this approach makes these endpoints duplicates of each other, it has several benefits like preventing the requirement for cross-domain requests, allowing easier API discovery and not requiring the main site of a setup to be exposed to REST API calls to a sub site.
  • Assigning an existing user to a site and removing a user from a site should generally be only available to network administrators, and the site administrators of the site that is being interacted with.
  • Similarly, editing a user that does not belong to the current site should only be possible for a network administrator. Currently this is available to site administrators as well which is probably wrong.
  • Deleting any user completely should only be available to a network administrator. A good way to handle the reassign parameter needs to be found though.

For background information, please read the posts at https://make.wordpress.org/core/2017/01/09/improving-the-rest-api-users-endpoint-in-multisite/ and https://make.wordpress.org/core/2017/01/11/controlling-access-to-rest-api-user-functionality-for-multisite/ (the latter contains the above list as well)

Change History (26)

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


8 years ago

This ticket was mentioned in Slack in #core by jeremyfelt. View the logs.


8 years ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


8 years ago

#4 @jeremyfelt
8 years ago

Related: #38961, which was opened when partial support existed for adding existing users. We'll need to make sure the correct capabilities are used when this is reintroduced.

#5 @flixos90
8 years ago

While reviewing our previous discussions for my presentation in Torino, I noticed a few problems with our plans. Let me first list all of the functionality and how I understand we're planning it:

  • GET wp/v2/users lists users of the current site
  • GET wp/v2/users?global=true lists all users
  • GET wp/v2/users/<id> shows a user of the current site
  • GET wp/v2/users/<id>?global=true shows any user
  • POST/PUT/PATCH wp/v2/users/<id> updates a user of the current site
  • POST/PUT/PATCH wp/v2/users/<id>?global=true updates any user
  • POST wp/v2/users creates a new user and adds it to the current site
  • POST wp/v2/users?email=<existing-email-address> adds an existing user to the current site
  • DELETE wp/v2/users/<id> removes a user from the current site
  • DELETE wp/v2/users/<id>?global=true deletes any user entirely

If anything from that list is not what we previously discussed, please correct me, as I might have misunderstood it then.

I think the following items need to be reviewed and discussed:

  1. Should POST/PUT/PATCH wp/v2/users/<id> only allow changes of roles and POST/PUT/PATCH wp/v2/users/<id>?global=true only allow changes of everything but roles? Or should we not make this differentiation and handle support based on capabilities (as proposed in #40263)?
  2. Our plans for creating and adding are too vague I think. A problem is that the current POST wp/v2/users is an operation partly in the global, and partly in the site context (a global user is created and then added to the current site). When specifying only an email to add an existing user, this could also be a failed request to create a new user, so there is a lack of clarity. I think it would be best if we had POST wp/v2/users?global=true for creating a global user _without_ adding them to a site, and then POST wp/v2/users to add a user to a site. This solution would perfectly align with our plans for the DELETE route. However, it might be almost impossible to implement things that way now because of backward compatibility. We need to revisit this.

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


8 years ago

#7 @jnylen0
8 years ago

Quoting myself from ticket:40263#comment:2:

By default (in the absence of a special parameter like global=true), the REST API should behave as though multisite does not exist, and clients should not have to care about it.

To me this implies:

  1. We should not make this differentiation and instead handle support based on capabilities.
  2. It's fine if an API operation in "default, non-multisite" mode actually implies some behind-the-scenes global operations. The important thing is that clients don't have to care about that, so I think creating a user via POST /wp/v2/users is fine.

Addressing a couple of the other tricky points of creating a user:

I think it would be best if we had POST wp/v2/users?global=true for creating a global user _without_ adding them to a site

This seems OK to me, but possibly with an extra parameter like add_to_site=false (the default behavior being to add the user to the site currently being operated on).

POST wp/v2/users to add a user to a site

I'm not so sure about this bit, whether it's done by email address by a site administrator, or some other way (username) by a network administrator.

These last couple of points seem closely related to how we will handle displaying and manipulating the list of sites of which a user is a member. I think fleshing out the rest of this will help clarify the best approach to your specific concerns, and it's probably OK to handle those tasks later on after some more basic things (other items in the list of requests/endpoints above).

#8 follow-up: @flixos90
8 years ago

@jnylen0 I generally agree to all points you make.

Only regarding POST wp/v2/users?global=true I don't think we need something like add_to_site there. Calling POST wp/v2/users already creates a new user and adds them to the current site, so that that use-case is covered.
And since the global parameter should indicate a global operation, I think it's sufficient to only create the user under that circumstances. Adding a user to a site is an operation in the site context, so I think it should not happen when passing global.

#9 in reply to: ↑ 8 ; follow-up: @jnylen0
8 years ago

  • Keywords 2nd-opinion removed

Replying to flixos90:

Only regarding POST wp/v2/users?global=true I don't think we need something like add_to_site there. Calling POST wp/v2/users already creates a new user and adds them to the current site, so that that use-case is covered.
And since the global parameter should indicate a global operation, I think it's sufficient to only create the user under that circumstances. Adding a user to a site is an operation in the site context, so I think it should not happen when passing global.

After some further reflection I agree with this: creating a user globally doesn't imply adding them to a specific site.

POST wp/v2/users?email=<existing-email-address> adds an existing user to the current site

For this particular case, it may be better to add a separate endpoint. However, this still seems like a fairly uncommon use case compared to other user management tasks and should be fine to be addressed later on.

#10 in reply to: ↑ 9 ; follow-up: @flixos90
8 years ago

Replying to jnylen0:

POST wp/v2/users?email=<existing-email-address> adds an existing user to the current site

For this particular case, it may be better to add a separate endpoint.

While I certainly don't like POST wp/v2/users?email=<existing-email-address> to handle this, I think it would be best if we could figure out a proper way to allow adding existing users through the same endpoint. This would provide parity with user removal (from a site), as it also happens through the same endpoint that is used to delete a user (entirely).
Alternatively, we should reconsider managing both adding and removing users to/from a site through POST/PUT/PATCH wp/v2/users/<id> in some way instead of using the CREATE and DELETE endpoints.

However, this still seems like a fairly uncommon use case compared to other user management tasks and should be fine to be addressed later on.

Not sure if we should postpone that decision, as I don't consider it an uncommon use-case. In the admin there's UI to add an existing user which is just as prominent as creating a new user and adding them.

Last edited 8 years ago by flixos90 (previous) (diff)

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


8 years ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


8 years ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


8 years ago

#14 in reply to: ↑ 10 @jeremyfelt
8 years ago

We had a good conversation in Slack last week about this.

Some thoughts/assertions from that discussion:

Replying to flixos90:

Should POST/PUT/PATCH wp/v2/users/<id> only allow changes of roles and POST/PUT/PATCH wp/v2/users/<id>?global=true only allow changes of everything but roles?

As @jnylen0 mentioned, we should handle this via capabilities.

Here's what seemed right to me at first glance:

  • POST/PUT/PATCH wp/v2/users/<id> updates data in the site context for a user depending on capabilities.
  • POST/PUT/PATCH wp/v2/users/<id>?global=true updates data in the site and global contexts for a user depending on capabilities.

However, the only current data that lives in a site context is a user's role(s) and a few display settings. We could just assume global=true for both calls and defer to capabilities always.

I'm not sure what position that puts us in for the future if we somehow find reason to add more site context settings. Really, all the data is stored in the same place, so maybe it's all global?

Replying to flixos90:

I think it would be best if we had POST wp/v2/users?global=true for creating a global user _without_ adding them to a site, and then POST wp/v2/users to add a user to a site.

We chatted through this a bit too and came to a conclusion that we can support a combo here.

  • POST wp/v2/users Adds an existing global user to a site.
  • POST wp/v2/users?global=true creates a new global user and does not add them to the site.
  • POST wp/v2/users?global=true&add_to_site=true creates a new global user and adds them to the site.

The add_to_site (or other) parameter adds a little more complexity, but also allows for 1 API call to add a new user to a site rather than 2.

This ticket was mentioned in Slack in #core-restapi by flixos90. View the logs.


8 years ago

#16 @jnylen0
8 years ago

We chatted through this a bit too and came to a conclusion that we can support a combo here.

  • POST wp/v2/users Adds an existing global user to a site.
  • POST wp/v2/users?global=true creates a new global user and does not add them to the site.
  • POST wp/v2/users?global=true&add_to_site=true creates a new global user and adds them to the site.

This isn't making sense to me. Here's what I think it should be instead:

  • POST wp/v2/users - creates a user and adds it to the site. If on multisite, the user will necessarily be created globally, and this is fine. The important thing is that the API client does not need to be multisite-aware.
  • POST wp/v2/users?global=true - creates a new global user and does not add them to the site.
  • POST wp/v2/users?global=true&add_to_site=true - we no longer need this new add_to_site parameter.

IMO, the first case is not broken, even though it is partially a global operation and partially a site-specific operation. This is how I expect the API to behave by default, even in a multisite context. It's still pretty self-consistent this way: we follow the rule that API clients don't need to be multisite-aware, and without the global=true parameter this newly created user doesn't appear on other sites of the same installation. Also, we preserve backwards compatibility, and I think I'm not really OK with breaking something as basic as "POST to create a new user".

From the aforementioned Slack chat, there are three things we want to support:

a) creating a global user and adding them to the current site
b) creating a global user
c) adding an existing global user to the current site
The API can only do a), but it needs to be able to also do b) and c) I think (at least c) for sure).

The more I think about it, the more I'm realizing that the thing is tripping us up is (c). Everywhere else in the REST API, the following rule holds true: "a POST to a resource without an ID is a CREATE operation". Let's not break it here.

So, it seems like trying to fit (c) into our existing endpoints is not going to be very RESTful no matter what we do. Also, (c) is necessarily a multisite-aware operation, so we can assume that clients know exactly what they are doing here. These two points suggest that (c) is a fundamentally different task than (a) and (b); also, that we have a good deal more freedom in how we approach it.

IMO this will probably need to include either a separate API endpoint or a newly added feature of another API endpoint.

I had a couple other ideas here that don't feel as fully-formed, but maybe they can lead to some good discussion at least.

  1. We could add a new endpoint like /wp/v2/users/:id/sites. Here, a POST operation would make more sense: we're "creating" a "site membership resource" for this user. This endpoint would also naturally support pagination, which would solve the problem of a user being a member of an extremely large number of sites.
  1. We could also allow GET /wp/v2/users?global=true&email=something@example.com to function as a search that returns limited data including a user ID (for users with the correct capabilities). This would require two separate requests to perform the operation of adding a user by email address, but I think I'm probably OK with that trade-off, and there are other benefits to this approach (the client can fetch user data and confirm the intended result).

This ticket was mentioned in Slack in #core-multisite by jnylen. View the logs.


8 years ago

#18 @flixos90
8 years ago

@jnylen0

we follow the rule that API clients don't need to be multisite-aware

I'm not convinced by introducing a separate endpoint for that behavior, as I'm not fully of the opinion that a client shouldn't need to be multisite-aware. After all multisite works differently in some points, particularly user management, so why ignore that in the API? We're introducing a global parameter, so I think it should be honored properly on all wp/v2/users routes, so without specifying that parameter in POST wp/v2/users no global operation should happen, as that would be inconsistent.
Even now, for example you cannot delete users through the REST API in a multisite while you can without multisite. Of course this is only because we couldn't get it ready, but that is particularly because it works so differently.

The more I think about it, the more I'm realizing that the thing is tripping us up is (c). Everywhere else in the REST API, the following rule holds true: "a POST to a resource without an ID is a CREATE operation". Let's not break it here.

My understanding is that, for a single site administrator adding an existing user (multisite) has the same effect as creating a new user (single site). So while from a global point of view it isn't a CREATE operation, it is one when seen in the site context (thus without global parameter).

We could add a new endpoint like /wp/v2/users/:id/sites.

If the consensus ends up not to change the existing single site behavior (POST wp/v2/users adds a global user and adds them to the site), I'd still prefer finding a way to handle adding an existing user to the current site with the infrastructure already present, for the above reasons. For example POST wp/v2/users?email=...&existing=true.

What I wouldn't like in particular is if we didn't support adding existing users to a site in the CREATE route while at the same time supporting removing users from a site in the DELETE route. Either POST wp/v2/users and DELETE wp/v2/users/<id> should be able to add/remove users to/from a site independently from creating/deleting, or neither of them should.

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


8 years ago

This ticket was mentioned in Slack in #core-multisite by jnylen. View the logs.


8 years ago

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


8 years ago

#22 @flixos90
7 years ago

  • Keywords ms-roadmap added

These tickets belong to our planned roadmap (a few of them not per final decision), so flagging with a keyword for better overview.

#23 @paaljoachim
3 years ago

Can we get a status update on this ticket?
@flixos90 @jeremyfelt @jnylen0

Thanks!

Getting this merged seems to be a precursor to:
Include more usermeta fields in the Network Admin's "Add User" view ticket.
https://core.trac.wordpress.org/ticket/18163#comment:24

This ticket was mentioned in Slack in #core-multisite by paaljoachim. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-editor by paaljoachim. View the logs.


3 years ago

#26 @johnjamesjacoby
3 years ago

I have some ideas I have not seen discussed yet, and maybe some variations on existing ones.

  1. I am not a big fan of global=true. It feels bolted on (because it is) instead of an intentional part of the API design. Users in WordPress are always technically global no matter the installation type; Roles and Caps just happen to grant them access to Sites and Networks.
  2. User management for Sites could be part of a /sites route instead of /users. This would sidestep problems with multisite awareness, as the multisite specific routes would not be registered on single-site installations. It also could perhaps provide an alternative route to list the users of specific sites, etc...
  3. User management for Networks could be part of a /networks route. This doesn't exist in WordPress core at all, but perhaps we can incrementally start experimenting with it.
  • GET wp/v2/users always returns all users regardless of site
  • GET wp/v2/sites/<id>/users returns users with Caps on the specified site
  • GET wp/v2/users/<id> returns a user
  • GET wp/v2/sites/<id>/users/<id> returns a user if Capped on the specified site
  • POST/PUT/PATCH wp/v2/users/<id> updates any user
  • POST wp/v2/users creates a new user
  • POST wp/v2/sites/<id>/users/add/<email|id> adds an existing user to the current site
  • DELETE wp/v2/users/<id> deletes any user entirely⭐️
  • DELETE wp/v2/sites/<id>/users/<id> deletes a user from a site

⭐️ Should deleting a user entirely check for authored content and fail if they have any? Could a force=true flag be added or something?

The only obvious problem with that API design is that it exposes site IDs, which may or may not be desirable. It also is perhaps more work, and enables capable users to make requests at one site to affect another one, which is kind of weird and I don't really love either now that I'm typing this.

Oh well. Sharing anyways just to get my thoughts out! 😅

Version 0, edited 3 years ago by johnjamesjacoby (next)
Note: See TracTickets for help on using tickets.