#7598 closed defect (bug) (fixed)
Incorrect permission check when updating member type
Reported by: |
|
Owned by: |
|
---|---|---|---|
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)
Change History (4)
#1
@
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
.
#3
@
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...)
Patch for permission check when updating a Member Type from the XProfile administration screen.