#38526 closed defect (bug) (fixed)
REST Multisite new user is not assigned to any site
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.7 | Priority: | normal |
Severity: | major | Version: | 4.7 |
Component: | REST API | Keywords: | has-patch has-unit-tests commit |
Focuses: | multisite | Cc: |
Description
Hi there
Creating a new user through the API on a multisite results in a user not assigned to any website. I know the focus on the REST API have not focused mainly on the multisite part of things but would the expected behavior not be to assign the user to the site that the request has been passed to?
Example:
POST http://example.com/wp-json/wp/v2/users will add the user to the example.com site
POST http://test example.com/wp-json/wp/v2/users will add the user to the test.example.com site
And so on.
As users are the thing shared across multisite I think that this would be an important change as otherwise creating new users on multisites will be kinda meaningless at the moment.
Attachments (3)
Change History (23)
This ticket was mentioned in Slack in #core by richardtape. View the logs.
8 years ago
#2
@
8 years ago
See the related GH issue: https://github.com/WP-API/WP-API/issues/2327
#3
@
8 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to 4.7
- Severity changed from normal to major
- Type changed from enhancement to defect (bug)
- Version 4.6.1 deleted
Moving to 4.7 because I think that this needs to be fixed. At a glance it looks like a add_user_to_blog()
call is missing in WP_REST_Users_Controller::create_item()
.
Noted that already on Oct 19th, see https://wordpress.slack.com/archives/core-multisite/p1476912979001644.
cc: @jeremyfelt, @flixos90
#4
@
8 years ago
Agreed. I had first thought that it would be confusing to provide add_user_to_blog()
in create_item()
, but after looking closer at delete_item()
, the user is only removed from a site (via wp_delete_user()
and not deleted from the network. It probably makes sense to align the expected behaviors here.
The harder part will be figuring out (in 4.x) the right endpoint to assign existing network users to individual sites. Right now if you use create_item()
on an existing username, wpmu_validate_user_signup()
will kick back an error.
#5
@
8 years ago
In WP_REST_Users_Controller::create_item()
, on single site, a user is first created and then any roles passed with the REST request are assigned.
In add_user_to_blog()
, a single role is expected with the user.
We could shift
$request['roles']
and pass that to add_user_to_blog()
and then let the current loop handle any remaining roles.
#6
follow-up:
↓ 7
@
8 years ago
I think I misunderstood the original issue at some level.
In the current state, if I create a new user via POST src.wordpress-develop.dev/site-two/wp-json/wp/v2/users
, the user is added to the network and the site as expected.
If I attempt to use a create request for a user that already exists on the network, an error is returned.
This seems like something that should then defer to WP_REST_Users_Controller::update_item()
, which can be used to update the user's roles on a site from none to whatever is passed.
So for the client:
- Request 1: Create user, error response - user_name
- Request 2: Update existing user with roles, success.
Though the request is successful, there are some side effects from add_user_to_blog()
never actually firing:
- The
primary_blog
andsource_domain
meta for a user may not be set. - The
add_user_to_blog
action does not fire. - The
users
and_user_count
cache are not cleared.
We can probably make use of add_to_blog()
in both create_item()
and update_item()
without much trouble.
#7
in reply to:
↑ 6
@
8 years ago
Our consensus here was to punt this to 4.8; as @jeremyfelt notes, creating a user does work, it's only adding them to a site that doesn't. This is at least somewhat intentional: the user already exists on the network (with an ID), what you're really doing to trying to grant them a role on a site. That's an update, which needs to be a PUT
to the resource, not a POST
.
Replying to jeremyfelt:
This seems like something that should then defer to
WP_REST_Users_Controller::update_item()
, which can be used to update the user's roles on a site from none to whatever is passed.
This should work, I think? Apart from:
Though the request is successful, there are some side effects from
add_user_to_blog()
never actually firing:
- The
primary_blog
andsource_domain
meta for a user may not be set.- The
add_user_to_blog
action does not fire.- The
users
and_user_count
cache are not cleared.We can probably make use of
add_to_blog()
in bothcreate_item()
andupdate_item()
without much trouble.
I'd be happy to do that here, although we need to consider the API in the context of multisite more thoroughly in the next release. I don't think we'd be locking ourselves down by doing this.
This ticket was mentioned in Slack in #core-multisite by rmccue. View the logs.
8 years ago
#10
@
8 years ago
In 38526.diff:
- In
create_item()
, whenis_multisite()
, fireadd_user_to_blog()
with an empty string for role. This allows the existing logic to add any passed roles. - In
update_item()
, whenis_multisite()
and not a member of the site, fireadd_user_to_blog()
with an empty string for role.
I have not added any tests yet. Is this an appropriate approach?
This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.
8 years ago
#12
@
8 years ago
38526.2.diff adds tests asserting current expectations.
In the process of creating these I ran into a quirk with how get_rest_url()
handles the $path
argument and how WP_Test_REST_Controller_Testcase
filters it on every setup. For now I've set it to skip filtering when is_multisite()
as our tests should be written to include the leading slash anyway.
This ticket was mentioned in Slack in #core-restapi by jeremyfelt. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by helen. View the logs.
8 years ago
#16
@
8 years ago
- Keywords commit added
- Owner changed from jeremyfelt to rmccue
- Status changed from assigned to reviewing
Handing back to @rmccue for review/commit.
#18
@
8 years ago
@jeremyfelt Is there a reason for using a mix of get_site()->id
and get_current_blog_id()
in https://core.trac.wordpress.org/attachment/ticket/38526/38526.2.diff?
new user through rest api on multisite