Base xprofile field Name(primary) and visibility when logged in as superadmin

[imath]

If i'm logged in as superadmin, then in the front end on my profile edit page or profile edit page of members html gives me the impression i can change this field, it actually doesn't change but i think it's a bit confusing. So i opened bp-core/bp-core-caps.php at line 380 function bp_current_user_can. And i saw a comment before the first if statement

// @todo: remove this when implemented
	if ( is_super_admin() )
		return true;

So i first try to remove this condition, but if i do so, then i get a 404 when trying to edit another user's profile on front end..

So i simply added in the condition && $capability != 'bp_xprofile_change_field_visibility' and it seems to work

[boonebgorges]

So i first try to remove this condition, but if i do so, then i get a 404 when trying to edit another user's profile on front end..

Removing this condition is, IMO, the correct fix. (Having a special case for this profile field seems very ugly.) I'm not able to reproduce the 404 after removing the condition. Can you give more details?

[imath]

Replying to boonebgorges:

Can you give more details?

Hi Boone,
you're right very ugly !!
but i've just tested it again, and it's a bit more problematic, because if i comment the two lines under the @todo comment then :
1/ in the backend the Profile Field menu is not showing as a submenu of the WordPress users menu.
2/ if as a superadmin i want to edit the profile from front end of one of my user : for instance buddy16.dev/members/mathchristo/profile/edit/group/1/ then i have a 404

If i leave uncommented the 2 lines under the @todo comment, then it works but it makes it possible for the superadmin to change the visibility of the base xprofile field name... Even if i seems to have no effect on the display of this field, it's a bit confusing

As i'm going on holidays tomorrow morning, i'm sorry i cannot try to find why :(

[boonebgorges]

Broadly, the reason this is happening is because the bp_moderate cap is not 100% implemented. We have functions bp-core-caps.php that are meant to add the cap to the Administrator role at activation, but these functions are not actually called by BP during plugin activation (they were incompletely ported from bbPress). We haven't noticed the issue so much because (1) the temporary is_super_admin() check made it so that bp_moderate worked as expected, and (2) on Multisite, super admins get all caps anyway ​https://core.trac.wordpress.org/browser/tags/3.4.1/wp-includes/capabilities.php#L874 - our homegrown is_super_admin() check only matters on non-MS.

IMO it is too late to fix this in the proper way for BP 1.6 (the "proper" way is to add caps to the database). Activation stuff is way too finicky, and if done wrong, it will break many parts of BuddyPress, as imath notes above.

As a workaround, I suggest the 4296.01.patch. It is a temporary filter on 'user_has_cap', which will add the 'bp_moderate' cap to Administrator-level users on non-MS, unless the 'do_not_allow' cap is also being passed. This serves as a temporary implementation of 'bp_moderate', and also allows our other caps (like 'bp_xprofile_change_field_visibility') to pass through the WP_User::has_cap() logic as expected. My patch is not an elegant solution, but it is effective, and can easily be removed later on when we properly initialize our caps/roles.

This is an important issue, so I'd really like some dev feedback on it. Thanks.

[DJPaul]

I don't quite understand why the is_super_admin check does not work in this case, but the patch looks solid if this is the best way to fix the problem for 1.6. I haven't tested the patch.

[boonebgorges]

I don't quite understand why the is_super_admin check does not work in this case

WP's core system has the following logic: On multisite, if you are super admin, you automatically get all caps, *except when a cap is mapped onto 'do_not_allow'*. That way, it's possible to make current_user_can( 'foo' ) to return false for a given foo, even for super admins. In our case, we want current_user_can( 'bp_xprofile_change_field_visibility' ) to return false for all users, even super admins, for the fullname field. The WP way to do this is to map 'bp_xprofile_change_field_visibility' onto 'do_not_allow'. But the if ( is_super_admin() ) return true; check at the beginning of our bp_current_user_can() was wrecking this, by making every cap return true for SAs. The original intention of this super admin check was, I assume, to ensure that super admins have the catch-all 'bp_moderate' cap; my patch does this, without disrupting the ability to have other instances of bp_current_user_can() return false for SAs.

(It's complicated ;) )

[DJPaul]

Thanks for explanation

[boonebgorges]

(In [6185]) Improves current_user_can( 'bp_moderate' ) implementation for 1.6 release

The newly introduced bp_moderate cap must be manually added to the
Administrator role on order for it to work properly under the current
implementation of BP caps, wherein they are not stored in the database like
other caps.

This is a workaround, which will be reverted for BP 1.7.

Allows other caps passing through bp_current_user_can() to be evaluated
properly, without being overridden for super admins.

Fixes #4296