Skip to:
Content

Opened 12 months ago

Last modified 11 months ago

#4991 new defect (bug)

manage_options capability required for non-super admin xprofile editing

Reported by: danzigism Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 1.7
Component: XProfile Keywords: has-patch dev-feedback
Cc: sam3dus@…

Description

Prior to 1.7.1 the only requirement for non-super admins to edit other users' extended profiles is that they only needed the "edit_users" capability as defined in bp-members-functions.php

Since the release of 1.7.1 user roles with the "edit_users" capability cannot edit other users' extended profiles and are taken to a standard "You do not have sufficient privileges to view this page" error, despite the code in bp-members-functions.php only asking for "edit_users" or (bp_current_user_can('bp_moderate' ) capabilities.

It appears that as of 1.7.1 only user roles with the "manage_options" capability are able to edit extended profiles other than their own. As we know, this capability gives user roles access to all of the WordPress Settings.

Tested with the "Editor" user role. Tried changing the conditional statement in bp-members-functions.php but it has no effect.

Attachments (1)

4999.01.patch (1.0 KB) - added by r-a-y 11 months ago.

Download all attachments as: .zip

Change History (5)

comment:1 shanebp12 months ago

  • Cc sam3dus@… added

r-a-y11 months ago

comment:2 r-a-y11 months ago

  • Component changed from Core to XProfile
  • Keywords has-patch dev-feedback added
  • Milestone changed from Awaiting Review to 1.8

Prior to 1.7.1 the only requirement for non-super admins to edit other users' extended profiles is that they only needed the "edit_users" capability as defined in bp-members-functions.php

In older versions of BP (1.5 and below), you had to explicitly be a super admin in order to edit a user's profile and avatar. 1.5 example.

In BP 1.6, you could give non-super admins the "bp_moderate" capability (not "edit_users") to edit the profile and avatar. 1.6 example.

In BP 1.7, this appears to be more restricting as of r6844. Read lines 233-234 of the inline doc.

I'm only basing this off the inline doc, but it appears that if a user has the "bp_moderate" cap, you would also need the "manage_options" cap in order to edit a user's profile. This is probably what you meant in your original report, danzigism. The phpDoc for that function also says that this is temporary, so I'm not sure what the end goal is.

With that being said, 4999.01.patch (disregard the wrong ticket number!) will allow you to check the "edit_users" capability as well, but I'm not sure if the other devs intended for this to happen. I don't believe this will do any harm though.

Devs: Are there places where bp_current_user_can( 'bp_moderate' ) should be replaced with bp_core_can_edit_settings()? I can see a couple of instances where we could do this such as activity deletion and spamming.

comment:3 boonebgorges11 months ago

The phpDoc for that function also says that this is temporary, so I'm not sure what the end goal is.

The standard treatment of caps in WP is that they are mapped to roles "permanently" - that is, they're stored in the database. For 1.6, we weren't prepared to do that, so we put in place a temporary hack. r6844 looks like it was meant to fix a legitimate bug, but in the process it actually changed the implementation of bp_moderate.

This is a big mess. The idea was that, in the long run, we were going to use real WP caps. But this hasn't materialized. So instead, we have a hodge-podge of current_user_can( 'bp_moderate' ) and function-specific functions like bp_core_can_edit_settings(). Ideally (again, in the long run) we should pick one of these strategies and stick with it.

For now, I think that r-a-y's patch is wise. It has two virtues: 1. it fixes the bug, and 2. it does so in a very specific way, using a tool (bp_core_can_edit_settings()) that we already have, instead of mucking around with the way that the bp_moderate map works.

Are there places where bp_current_user_can( 'bp_moderate' ) should be replaced with bp_core_can_edit_settings()? I can see a couple of instances where we could do this such as activity deletion and spamming.

Activity deletion and spamming are not the same thing as editing other people's profiles, and really don't neatly fall under the rubric of "can_edit_settings". I think that the general strategy should be to move toward greater specificity/fine-grainedness with our caps, while lumping all this stuff under "can edit settings" is moving in the wrong direction. So let's hold off a larger audit for now.

comment:4 boonebgorges11 months ago

  • Milestone changed from 1.8 to Future Release
Note: See TracTickets for help on using tickets.