Multisite signups do not have a WordPress role

[r-a-y]

When a user registers on a multisite install, after the user activates their account, that user does not have a WordPress role on the BP root blog.

To duplicate:

• Set up a multisite install and enable user registration.
• Go to the BP signup page (example.com/register) and register a new user.
• Activate the new user.
• Now, login as an administrator to the root blog's admin dashboard "Users" page.
• You'll notice that the user is not listed because the new user has no WordPress role on the root blog.

The problem is during the activation process, wpmu_create_user() (via wpmu_activate_signup()) does not add a role:
​https://github.com/WordPress/WordPress/blob/4.8-branch/wp-includes/ms-functions.php#L1116

However, we do add a role for single site signups, but not multisite:
​https://buddypress.trac.wordpress.org/browser/tags/2.8.2/src/bp-members/bp-members-functions.php?marks=2001-2002#L1991

Attached patch will add a default role regardless of the WP install after activation. Also includes a unit test.

[needle]

@r-a-y Heh, well it seems I've been labouring under the misapprehension that this was by design (i.e. leaving it up to the developer to decide how and when to assign a role to new users) and I've always included code along the lines of your patch in each of my BuddyPress installs. However I tend to use BP Registration Options (or similar) to moderate signups, so I hook into later actions provided by BPRO when moderation has taken place. Doing so will mean BPRO (and similar plugins) will need to unhook this to prevent unmoderated users from receiving the default role.

tl;dr Worth publicising this before committing?

[r-a-y]

I would say that the role addition is currently inconsistent. If we add the default role on single site, then why don't we add the default role on multisite?

I understand that plugins such as Multisite User Management have had to workaround this issue by adding their own role, so there is a potential conflict here. However, WordPress allows a user to have multiple roles, but adding the default role might be unwanted like what you mention above.

Maybe we do nothing here?

[DJPaul]

Let's do this.

@r-a-y Your patch: why the is_numeric check? Is that not plastering over some other bug in BP elsewhere?

Otherwise looks fine.

[espellcaste]

@r-a-y Could you take another look at your patch before 3.0?

[r-a-y]

This change might affect existing multisite installs that have already conjured up a workaround to add in a default role or are using some form of fine-grained role permissions. For example, member subscription sites or those using authentication via LDAP or some other means.

Adding in the default role might be unwanted. I think it might be better that we do nothing in this case.

[espellcaste]

I see your point but still, people using BuddyPress with scenarios different from those mentioned might still have members with no role.

So it seems the bug continues. :/

[r-a-y]

I'll leave it to a lead dev to decide.

[DJPaul]

I like the patch and think the behaviour makes sense to maintain and keep for the majority of our users.

[boonebgorges]

To play devil's advocate: WP site roles do some special tasks in Multisite. For example, they cause a site to show up in the My Sites toolbar dropdown and wp-admin/my-sites.php. Adding all users to the BP site as Subscribers may obscure this somewhat.

That being said, I think on balance that we should go with the patch. The inconsistency is annoying. Also, bbPress, when used with BP on Multisite, defaults to adding users to the root site as Participants, which causes weird and mysterious voodoo bugs when those users don't also have a WP role on the site. (Semi-related: ​https://bbpress.trac.wordpress.org/ticket/2210)

[r-a-y]

Now that I've had more time to think about it, I actually think this is more of a multisite upstream issue. See ​#WP18025, ​#WP38526 for some examples. Note that for the latter ticket, in the WP REST API, WP has decided to add a role for a multisite user.

However, they use add_user_to_blog(), which is probably a better option for multisite rather than $member->set_role( get_option( 'default_role' ) );. We should probably do what WP does for multisite if we decide to go forward with this patch.

I guess the question is do we create a ticket upstream to get some feedback or do we impose what we think is correct, even though this might affect existing sites?

bbPress works about the same as what we are proposing, however it adds its own role while we would be adding the WP default role of subscriber.

[boonebgorges]

I have my doubts about whether this would ever be fixed upstream, for backward compatibility reasons. The argument would be that REST API requests are always site-specific, while wp-signup.php could be an entry point to the entire network. Not that I think this is *right*, but I don't think it's a battle worth fighting.

Agreed with your point about add_user_to_blog().

I guess the question is do we create a ticket upstream to get some feedback or do we impose what we think is correct, even though this might affect existing sites?

What's the worst that will happen to existing sites? There seems little harm in adding people as Subscribers. Is it possible that someone may have configured their site in such a way that the default role is something higher than Subscriber, so that BP would then suddenly be adding privileged users?

[DJPaul]

I've been thinking about this and I think we should / and it's ok for BuddyPress to give the new user the network's default role on the BP root site. We are really just making our behaviour consistent.

(This is not going to work with BP_ENABLE_MULTIBLOG, but that's an extreme edge case, and I bet there's more stuff inconsistent on that sort of configuration, anyway. So I'm not worried about that.)

From a user's perspective, I think this is the correct thing to do because, as @boonebgorges said previously, they'll see the community site in the Toolbar's "My Sites" dropdown.

Do we have sufficient consensus to proceed?

[espellcaste]

I agree, as before! I feel adding the default role is the correct approach.

[r-a-y]

Patch is refreshed to use add_user_to_blog() for multisite.

One thing I also looked at was what ​#WP38526 did, leaving the role blank for multisite -- add_user_to_blog( $user_id, $blog_id, '' ).

When this is done, the user will still show up under the BP root blog's admin Users dashboard on multisite, but with no role attached under the "Site Role" column. This isn't in the current patch, but thought it was worth noting.

We might want to consider registering users with no role on multisite instead, but leave the current behavior for single-site alone.

[DJPaul]

Looks good, let's get it in please.

[r-a-y]

In 11829:

Activation: Add default WordPress role for new signups.

Props r-a-y, needle, espellcaste, DJPaul, boonebgorges.

Fixes #7565.

[r-a-y]

After r11829, there's a failing unit test due to #5834.

Since we now add a user role on multisite, that test shouldn't be necessary anymore.

Any qualms with removing it? See remove_unit_test.patch.

[DJPaul]

Of course not, go for it. Thanks!

[r-a-y]

In 11832:

Activation: After r11829, remove failing unit test for new user and blog signup.

r11829 adds the default user role for new signups, which makes the unit
test added in #5834 no longer necessary.

See #7565.