Site Admins cannot edit BP emails

[shanebp]

As the only site admin, I can edit BP Emails at ...wp-admin/edit.php?post_type=bp-email.
If I create another site admin, that user can only view BP Emails.

Forum Topic:
​https://buddypress.org/support/topic/admins-can-not-edit-buddypress-emails/

[imath]

@shanebp I confirm, thanks for reporting the issue. Let's try to fix it during the 7.0.0 dev cycle.

[shanebp]

I took a look, but the only permissions check I found for the BP Emails admin page was for user > can manage options which should be true for all site admins.
So I am curious as to where the issue lies.

[imath]

So here's the situation, WordPress uses 3 meta capabilities (edit_post, read_post, and delete_post) that are mapped to corresponding primitive capabilities depending on the context (for our case it's edit_others_posts & edit_published_posts).
See ​https://developer.wordpress.org/reference/functions/register_post_type/#capabilities.

Although we map primitive caps to bp_moderate, this cap is not part of the Administrator's all caps but is mapped to manage_options into _bp_enforce_bp_moderate_cap_for_admins().

When WordPress checks for the edit_post cap, it adds the bp_moderate cap to the caps to check, but the $cap that is passed to _bp_enforce_bp_moderate_cap_for_admins() is still edit_post and not bp_moderate.

So _bp_enforce_bp_moderate_cap_for_admins() receives:

• $cap = 'edit_post',
• $caps = array( 'bp_moderate', 'edit_published_posts' ),
• The user ID,
• and $args = array( $post_id ).

and doesn't return array( 'manage_options' ), but the unchanged $caps. As the first admin is the author of the bp_email post_type, he can edit it, but other administrators cannot because it's not edit_other_posts that is checked against the Administrator's role database capabilities but bp_moderate, which is not into the Administrator's role database capabilities 🤪

A possible way to fix this, is to use the capability_type argument of the register_post_type() function instead of the capabilities one and to edit _bp_enforce_bp_moderate_cap_for_admins() to check some BP Administrator's caps which includes bp_moderate and the BP Email post type capabilities.

That's what I did in 8355.patch to fix the issue. I'd be happy to have a second thought about it 🤔 What do you think about it @boonebgorges ?

[shanebp]

Yikes, nasty... thanks for unearthing this issue.

[boonebgorges]

8355.patch​ frightens me :) I'll take some time next week to try understanding this issue a bit better.

For clarification, "site admin" in this context means "Administrator on the BP root blog", not "super admin", right?

[imath]

Ahah 😆 Thanks Boone. Absolutely, site admin is the Administrator of the root blog.

[boonebgorges]

This bug shows the weakness in our _bp_enforce_bp_moderate_cap_for_admins() approach. Our approach assumes that WP will always check the bp_moderate check directly, but the CPT capability mapping is a case where this assumption is incorrect.

The strategy in 8355.patch​ seems to work. But I worry that it creates more complexity that will provide difficult to maintain down the line. Will we have to add more caps to the 'administrator' role in this way in the future? Especially as we leverage more CPTs and taxonomies in the future.

Our current approach was always meant to be a temporary workaround. See #4296, [6185]. Can we revisit to see if there is a proper fix? Specifically, is it now possible in WP to dynamically filter a role's capabilities? If we were able to add 'bp_moderate' to the Administrator role, then this problem - and future problems like it - would go away. cc @johnjamesjacoby

[imath]

Well since WordPress 4.7 there's a new action wp_roles_init inside the WP_Roles->init_roles() method which was added in WordPress 4.9.

I've tested to use this action and it seems possible to add the bp_moderate cap to the Administrator role without having it saved into the database. It's an easier fix for this issue.

Whether it's available since 4.7 (action) or 4.9 (method), it's anyway available for the BP 7.0.0 required WP version.

See 8355.2.patch

[boonebgorges]

IMHO 8355.2.patch​ is a much more elegant solution to the current ticket, as well as to the overall issue of how to implement 'bp_moderate'. (Even better would be if there were proper filters in place so that we didn't have to manipulate globals, but it looks like this is not possible at the moment.) @imath are you happy with the solution?

@johnjamesjacoby I know you have put a great deal of thought into the WP role system, and it would be really valuable to have your thoughts on the approach in 8355.2.patch​ before we go forward with it. I feel like it's fairly safe, but 'bp_moderate' touches everything in BP, so I'd like as many eyes on it as possible.

[imath]

@boonebgorges I'm happy with it: "less code for the same result" 👍. I haven't tested on multisite: I believe we should probably improve .2.patch to only add this cap to Administrator(s) of the BP Root blog.

[johnjamesjacoby]

Remember that there is no guarantee that the administrator role exists, as well as its capabilities array key. I would add some isset() checks to avoid unintentionally invoking a role that wasn’t there previously.

_bp_enforce_bp_moderate_cap_for_admins() makes a similar assumption, which is not ideal but apparently hasn’t been an issue for anyone yet. It doesn’t really seem right, but I don’t have a better idea right now.

The idea with bp_moderate was that it replaced all of the is_super_admin() calls, so it is intentional for it to be a higher than normal type of privilege. If it’s not mapping correctly, is it possible that WordPress core is checking the incorrect capability in this location?

[imath]

8355.tests.patch contains unit tests for regular an multisite WordPress config.

8355.3.patch is improving 8355.2.patch adding isset checks for the administrator role and making sure only Super Admins can bp_moderate when BuddyPress is network activated. With 8355.3.patch applied tests are all successful, without 1 test is failing (Administrator should be able to edit others emails)

is it possible that WordPress core is checking the incorrect capability in this location?

I don't think so, WordPress checks for edit_post and the bp_moderate capability is added to caps to check the user against. The specific problem here is _bp_enforce_bp_moderate_cap_for_admins() checks for the requested cap and not for the allowed caps. 8355.patch is a way to fix this into _bp_enforce_bp_moderate_cap_for_admins() including specific emails caps, I guess another easier way could be to do:

$caps = array_unique( array_merge( $caps, array( $cap ) ) );

if ( ! in_array( 'bp_moderate', $caps, true ) ) {
    return $caps.
}

[boonebgorges]

Thanks, all. John's point about the 'administrator' role is a good one, both that it should be checked for, and that it must not be a common issue if it's never been reported in all these years :) 8355.3.patch​ still feels like the most natural solution to me, since it's a way of adding the 'bp_moderate' cap to certain roles, rather than using (abusing?) the map_meta_cap logic.

[imath]

I'm fine with 8355.3.patch, I believe there can be custom roles containing the bp_moderate cap but I doubt they would contain the manage_options cap. But if we're worried about custom roles having the manage_options cap, we could also check for the manage_options cap instead of the role. That's what I've tested in 8355.4.patch.

[imath]

Let's fix this in 7.0.0. The last step we need to make is whether to commit:

• 8355.3.patch (Administrator Role check)
• 8355.4.patch (manage_options capability check)

[imath]

In 12740:

Core: improve the way BuddyPress adds the bp_moderate cap to Admins

This capability is dynamically added to the users having the manage_options one on regular configurations of WordPress (non multisite).

The private function _bp_enforce_bp_moderate_cap_for_admins() has been deprecated and we are now using the private function _bp_roles_init() to do this capability mapping.

This need for improvement has been revealed by an issue about the incapacity for Admins to edit BP Emails if they weren't their authors.

Props shanebp, boonebgorges, johnjamesjacoby

Fixes #8355

[bharadhan]

When will the version 7.0.0 will be released , can you please let us know

[imath]

@bharadhan 7.0.0 is slated to December 1st. You can follow development updates from this site:
​https://bpdevel.wordpress.com/

You've very welcome to help us testing the 7.0.0-beta1 that was released a few days ago:
​https://buddypress.org/2020/10/buddypress-7-0-0-beta1/