Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#7598 closed defect (bug) (fixed)

Incorrect permission check when updating member type

Reported by: meitar's profile meitar Owned by: slaffik's profile slaFFik
Milestone: 3.0 Priority: normal
Severity: normal Version:
Component: Core Keywords:
Cc:

Description

I'm not certain how to describe this bug so apologies for the vagueness.

I am writing a plugin for BuddyPress that uses the built-in WordPress Roles and Capabilities system to allow certain users, and only those users, access to modify a user's BuddyPress Member Type. I have defined a custom role that is a superset of the editor Role and grants the following additional capabilities:

  • edit_users
  • create_users
  • delete_user
  • bp_moderate (this is the important one)

When I assign a user to this role, my understanding is that they should now be able to pass any permission checks that check for the bp_moderate capability. One of these checks is in the bp-members/classes/class-bp-members-admin.php file, which looks like this:

// Permission check.
if ( ! current_user_can( 'bp_moderate' ) && $user_id != bp_loggedin_user_id() ) {
    return;
}

The purpose of this check, as I understand it, is to ensure that a user can only modify the Member Type setting on a given other user if they have the bp_moderate capability and if they are editing their own BP Extended Profile.

My plugin specifically wants to extend this to users of a given role, so that any user account with the edit_users capability can also edit the Member Types of other users. My code does this with the following sequence.

First, during plugin activation, a new role based on the editor role is created:

        // Create a role for the Staff, which need to be similar to
        // the default Editor role but with some user-management, too.
        $editor = get_role( 'editor' );
        $editor_caps = $editor->capabilities;
        add_role( 'staff_editor', esc_html__( 'Staff Editor' ), array_merge( $editor_caps, array(
            'list_users' => true,
            'edit_users' => true,
            'create_users' => true,
            'bp_moderate' => true, // for BuddyPress Extended Profile delegation
        ) ) );

Once a user is assigned to the Staff Editor role, they should inherit the bp_moderate permission as defined here.

However, this alone is not enough to allow a user to edit other users' Extended Profiles, so I also define a filter for the bp_current_user_can hook:

    /**
     * Filters the BuddyPress capability checks.
     *
     * Used for allowing Staff members to be XProfile delegates.
     *
     * @param bool   $retval
     * @param string $capability
     * @param int    $blog_id
     * @param array  $args
     *
     * @return bool
     */
    public static function bp_current_user_can ( $retval, $capability, $blog_id, $args ) {
        if ( current_user_can( 'edit_users' ) && 'bp_moderate' === $capability
            && isset( $_GET['page'] ) && 'bp-profile-edit' === $_GET['page']
            && isset( $_GET['user_id'] )
        ) {
            return true;
        }
        return $retval;
    }

At this point, any user with the edit_users capability can also edit the Extended Profile screens of other users, not just their own.

If my understanding is correct, the above should be all that's necessary for WordPress to permit BuddyPress to show the Extended Profile screen for privileged users (i.e., for "Staff Editors") and also edit the Member Type of those other users, thanks to the bp_moderate capability.

Sadly, however, this is not so.

On my install of BuddyPress (latest version, 2.9.1 as of this writing), the permission check always fails and ends the process_member_type_update() call because the current_user_can( 'bp_moderate' ) function returns false, even though it should return true, right…?

I'm not 100% sure why, but perhaps process_member_type_update() should use bp_current_user_can(), instead of current_user_can()? When I make this small change (patch is attached), my code works as expected, so I believe this is a small bug in BuddyPress.

Thanks for your patience, and I'm sorry I don't have a clearer sense of the exact issue (or the time to dig further into this).

Attachments (1)

7598.patch (592 bytes) - added by meitar 7 years ago.
Patch for permission check when updating a Member Type from the XProfile administration screen.

Download all attachments as: .zip

Change History (4)

@meitar
7 years ago

Patch for permission check when updating a Member Type from the XProfile administration screen.

#1 @DJPaul
7 years ago

  • Component changed from Administration to Core
  • Keywords member type roles capabilities removed
  • Milestone changed from Awaiting Review to 3.0

Thank you for the detailed bug report, @meitar. I am also not sure what's going on here -- I'm hoping @r-a-y or @johnjamesjacoby can explain it to us -- but given we use bp_current_user_can() elsewhere already, and given that you said this fixes it for you (notwithstanding the fact this could be caused by a bug in your code), it seems OK to change all the references of this function where we explicitly check bp_moderate.

#2 @djpaul
7 years ago

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

In 11703:

Core: when checking for bp_moderate capability, use bp_current_user_can().

Fixes #7598

Props meitar for initial patch.

#3 @johnjamesjacoby
7 years ago

If it's not explicitly passed, bp_current_user_can() falls back to the results of bp_get_root_blog_id(). It does this because "per-network" roles and capabilities do not exist, so we leverage the root-site for those settings.

In short, @meitar's filter on bp_current_user_can won't fire when current_user_can() is called alone.

In long, there may be other places where we have conflated these two functions, when we should be checking within the context of the root site vs. the current site (largely in wp-admin but possibly multi-blog mode, etc...)

Note: See TracTickets for help on using tickets.