Use bp_user_can for some group-related permissions.

[dcavins]

In many places in the groups component, we're applying checks to determine if a user can do something. These checks can be fairly complicated and are duplicated wherever they are needed. I'd like to move to a more centralized approach using bp_user_can() when possible. In addition to standardizing the permission checks, this will add a filter point that devs can use.

This also helps lay the groundwork for future changes making group statuses more flexible.

Thanks for your feedback!

[dcavins]

Use bp_current_user_can() for permission checks in the groups component.

[DJPaul]

Hey @dcavins
I haven't checked you've implemented the logic as per the original, but assuming that's good, the patch is pretty good. Some minor feedback:

• in_array() third parameter should be true, for type strictness.
• Use type strict logical operators, e.g. === instead of ==.
• The break indentation in the switch statements is a little off in places, I think. Worth double-checking.
• The test names should be human readable. Take test "user_can_groups_see_group_private_logged_in_user" -- I can see it's built off the capability name, but it should be readable if you see it in the terminal output. e.g. "user_can_join_private_groups" (or whatever).
• The test names also currently read positive, if that makes sense. If a test uses assertFalse, then the test name should make it clear that it is testing something that should not happen. e.g. "anonymous_user_cannot_join_private_groups".

[DJPaul]

Putting this in 3.0 assuming that mr @dcavins will get around to improving this patch in the next couple of months. :)

[dcavins]

Here's an updated version with strictness and renaming/organizing/cleaning up of the tests. Thanks again for the feedback.

[dcavins]

Now with more strictness!

[DJPaul]

Much better. :)

[dcavins]

In 11776:

Use bp_user_can for group-related permissions.

Centralize and de-duplicate permissions checks for group-related functions, like checking whether a user can join a group or send invitations from a group.

Props dcavins, djpaul.

Fixes #7610.

[r-a-y]

Encountered a bug where the "Request Membership" tab was showing up on public group pages for a multisite install.

To duplicate:

1. Ensure BuddyPress is installed on a multisite network.
2. Login as a super admin. This is key.
3. Navigate to any public group page.
4. You'll see the "Request Membership" nav item in the group nav.

The reason why this is happening is super admins, by default, are allowed to do anything and thus the bp_current_user_can() capability check returns true.

By the time, we check for groups_request_membership when registering the nav item, the return value is already true for super admins:
​https://buddypress.trac.wordpress.org/browser/tags/3.0.0-beta1/src/bp-groups/classes/class-bp-groups-component.php#L619
​https://buddypress.trac.wordpress.org/browser/tags/3.0.0-beta1/src/bp-groups/bp-groups-filters.php#L229

To fix this, we should set the return value to false before doing our request membership check.

I've also written a unit test that duplicates the problem:
phpunit -c tests/phpunit/multisite.xml --group BP7610

We might need to audit the rest of the group capability checks in this ticket to ensure the default values are working as we expect them to.

[dcavins]

@r-a-y Can you share the unit test you wrote? I'm not seeing it in trunk. Thanks!

[r-a-y]

@dcavins - The unit test is part of groups_request_membership.patch. Sorry for not being clear!

[dcavins]

Add some special logic for site admins.

[dcavins]

Thanks, @r-a-y. I took another spin through the original patch (finding a logic inversion from the original) and fixed up two more cases where site admins should clear additional hurdles before being allowed to do something. These three capabilities have been updated (including your earlier work):
groups_receive_invitation, groups_request_membership and groups_join_group

Thanks for finding this issue and also for writing new tests. (It's so great to have tests covering these capabilities.)

If no one has any further comments, let's get these changes committed. :)

[r-a-y]

Looks good, @dcavins!

[dcavins]

In 11970:

Fix admin user cases in bp_groups_user_can_filter().

If a user is an admin, he or she is pretty much allowed to do anything, so the capability has been approved before bp_groups_user_can_filter() filters the value. In a few cases, even admins need to satisfy a few other requirements before being allowed to do something, like request membership in a group, where even site admins need to not be members of the group.

Props r-a-y, dcavins.

See #7610.

[dcavins]

In 11971:

Correct bp_user_can() logic error.

In r11776, I introduced a logic error in groups_action_join_group(). The logic should be that if a user can't simply join a group, then we check for outstadning invitations.

See #7610.

[r-a-y]

In 11972:

Unit Tests: After r11970, fix fatal error when running WordPress 4.5.

grant_super_admin() doesn't exist for single-site on WP 4.5.

Thanks Travis!

See #7610.