Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#39944 closed defect (bug) (invalid)

Site wp_X_user_roles option not updated when network settings change

Reported by: sanchothefat's profile sanchothefat Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.5
Component: Networks and Sites Keywords: has-unit-tests
Focuses: multisite Cc:

Description

When a new site is created there is a call to populate_roles() which builds and saves a site option called wp_X_user_roles where X is the blog ID.

If you have "Administrators can add new users" unchecked and create a site, then check it and create another site - the two new sites will have different user roles arrays, the first of which will no longer represent the chosen network settings.

WP should handle updating the wp_X_user_roles option across the network when either the add_new_users or the registration site options change.

The following is an intermediate workaround for this but isn't necessarily very scalable:

<?php
/**
 * Currently when network settings are updated the capabilties for each
 * site stay as they were at the time of creation
 *
 * @action wpmuadminedit Runs before options are updated
 */
add_action( 'wpmuadminedit', function() {

        $do_update_roles = false;

        if ( isset( $_POST['add_new_users'] ) !== (bool) get_site_option( 'add_new_users' ) ) {
                $do_update_roles = true;
        }

        if ( $_POST['registration'] !== get_site_option( 'registration' ) ) {
                $do_update_roles = true;
        }

        if ( ! $do_update_roles ) {
                return;
        }

        /**
         * Runs after settings have been updated
         */
        add_action( 'update_wpmu_options', function() {

                // Need populate_roles() function
                require_once ABSPATH . '/wp-admin/includes/schema.php';

                if ( function_exists( 'get_sites' ) ) {
                        $blog_ids = get_sites( array(
                                'fields' => 'ids'
                        ) );
                } else {
                        $blog_ids = array_map( function( $site ) {
                                return $site['blog_id'];
                        }, wp_get_sites() );
                }

                // Call populate_roles() for each site
                foreach( $blog_ids as $blog_id ) {
                        switch_to_blog( $blog_id );
                        populate_roles();
                        restore_current_blog();
                }

        } );

} );

Any thoughts on the best way to update this option? Eg. when visiting the individual site admin, could we somehow tell if that network setting has been changed since the last run of populate_roles()?

Attachments (1)

test.39944.diff (2.4 KB) - added by sanchothefat 7 years ago.
Unit test as proof of issue

Download all attachments as: .zip

Change History (8)

#1 @sanchothefat
7 years ago

On further discussion with colleagues it seems this *should* be handled in capabilities.php however a bunch of the capabilities listed in schema.php are not reflected in that file - for example list_users.

I guess a patch here will just be a case of going through the extra roles added in schema.php that aren't present in the map_meta_cap() switch statement.

#2 @flixos90
7 years ago

@sanchothefat I might be misunderstanding you, but as far as I can see the behavior controlled by the options is properly applied. The administrator role always has the create_users capability regardless of the value of that option, but then this option is checked in map_meta_cap() and the capability gets adjusted as necessary.

#3 @sanchothefat
7 years ago

@flixos90 look at the list_users capability in particular. It's mentioned in schema.php and added only when installing or creating a site. There's no mention of it in capabilities.php so is never added dynamically from anywhere other than the database.

The list_users cap is used on a bunch of current_user_can() checks in the admin menu for example.

@sanchothefat
7 years ago

Unit test as proof of issue

#4 @sanchothefat
7 years ago

  • Keywords has-unit-tests added

The unit test I just added behaves oddly - eg. not what I saw in the data of an affected site, however it does highlight an error after changing the value of the add_new_users site option that the current admin user still has the list_users cap.

It's not the final unit test for this but should serve as a starting point for exploring the expected behaviour here.

#5 @flixos90
7 years ago

@sanchothefat I think what I'm confused about is why the add_new_users setting should affect the list_users capability. I would expect it to only allow/disallow the capability for creating/adding users.

#6 @sanchothefat
7 years ago

  • Resolution set to invalid
  • Status changed from new to closed

@flixos90 yeah I was talking to @johnbillion about it earlier. You're right - I was under the impression that a site admin without create_users would only see the profile edit option.

It's really weird, the site was deployed on version 3.5 so I'm not sure how the db roles array ended up without list_users and a bunch of other things at all. There's no code referring to map_meta_cap :/

I'll close this out and chalk it up to magic. Definitely can't repro now, sorry to waste your time.

Last edited 7 years ago by sanchothefat (previous) (diff)

#7 @johnbillion
7 years ago

  • Milestone Awaiting Review deleted

Checked this and the setting does affect the create_users cap but not list_users, as expected.

Note: See TracTickets for help on using tickets.