[buddypress-trac] [BuddyPress Trac] #7176: Implement user capabilities for Activity component

[buddypress-trac]

#7176: Implement user capabilities for Activity component
-------------------------+-----------------------------
 Reporter:  DJPaul       |       Owner:
     Type:  enhancement  |      Status:  new
 Priority:  normal       |   Milestone:  Future Release
Component:  Activity     |     Version:
 Severity:  normal       |  Resolution:
 Keywords:               |
-------------------------+-----------------------------

Comment (by johnjamesjacoby):

 > WP's Roles system is quite flawed

 This isn't really fair. WordPress's `WP_Role` class is one of best
 examples of proper class usage in WordPress core. And the `$wp_roles`
 global is a relic similar to the others.

 I'd call bbPress's approach a hybrid of dynamic & persistent roles & caps.
 Dynamic in that the roles are registered at run-time (vs. stored in
 `_options`); persistent in that each user continues to have their bbPress
 role saved in `usermeta` along with any role they have already in the rest
 of the site.

 > Most permissions checks are "derived", which means that they're mapped
 to "primitive" caps.

 This isn't really true. Most permissions checks are primitive, meaning
 they're looking for actual capabilities that a user is known to have in
 the database. Some caps are mapped – singles, specifically, like
 `delete_post` with a `$post_id` get mapped to "meta capabilities" as
 defined by the post object itself, like `delete_published_posts`, et
 all...

 > Users are associated with roles based on "capabilities" (bad name!) keys
 stored in usermeta.

 It's not a bad name. [https://en.wikipedia.org/wiki/Role-
 based_access_control (Role)] and [https://en.wikipedia.org/wiki/Role-
 based_access_control (capability)] access control are simple, well known,
 and versatile ways to define a set of allowances and constraints on any
 system. WordPress's hybrid approach to this, coupled with `map_meta_cap`
 allowing JIT overrides, is an incredibly flexible & powerful API.

 > we could use `read` in most cases, but here we needed to cover non-
 logged-in users

 We can't use `read` ever, unless we:

 * Make every user a "Subscriber" to every site
 * Re-init caps for the root-site on every `switch_to_blog()` to check our
 own capability mapping to `read` on root

 > For this reason, I think we should keep the concept of Roles, even if we
 decide not to use the full-fledged, stored-in-the-database version that WP
 has

 We can extend the `WP_Role` base class for user. To extend `WP_Roles`
 would require either:

 * Modifications to allow the `role_key` to be filtered
 * A filter on `get_option( $this->db->get_blog_prefix() . 'user_roles' )`
 which would likely not be very efficient, and kinda difficult for humans
 to unwind

 ----

 Because BuddyPress has the [https://codex.buddypress.org/getting-started
 /installation-in-wordpress-multisite/ (6 horsemen of the installation-type
 apocalypse)], I'm confident (and do agree) that we are forced to off-
 script at least a little bit to build something that's flexible enough to
 work in all environments.

 And we'll need to make sure `bp_moderate` force-allows to maintain
 backwards compatibility.

 I have 2 separate pieces of feedback:

 * About the patch
 * About the general approach

 ----

 '''Patch'''

 * Capabilities are the wrong place to do name-spacing, like
 `edit_bp_activity`. The name-spaces should be: the role the capability
 belongs to, and the more-broad array of roles that belong to BuddyPress.
 * Changes to `bp_add_caps()` and `bp_remove_caps()` look sound, even as a
 separate patch.
 * Adding `true/false` to `bp_get_caps_for_role()` seems OK, but the `Every
 other role` part is scary. bbPress has a "Blocked" role, for example, and
 now BuddyPress is allowing a blocked user in (maybe that's OK?). I think
 it's arguably easier to keep to mapping to WordPress's known roles, since
 they are almost guaranteed to exist in a predictable way.
 * The deprecation (and subsequent abandonment) of
 `bp_get_community_caps()` is sad to see. I think I'd imagined it as the
 funnel where all derived caps could be hooked in by each BuddyPress
 component, but I suppose whatever route we go will probably require a
 different approach than this one.
 * For code like `$activity && $user_id === $activity->user_id` can we
 please do `! empty( $activity ) && ( $user_id === $activity->user_id )` –
 wrapping inline conditionals should be standard practice for all of us by
 now.
 * I think I agree with @boonebgorges, that calling `bp_add_caps()` on
 every site is not a good idea.

 ----

 '''Approach'''

 I think I also agree with @boonebgorges, in that piggy-backing directly
 on-top-of WordPress's per-site role-based capability mapping system is not
 a great way to implement access control in an environment as complex as
 BuddyPress's.

 Which is to say, I think we need to treat each BuddyPress component like
 it is it's own namespace, with it's own roles that a user may-or-may-not
 have, defaulting to a "Subscriber" equivalent where all users are allowed,
 to maintain backwards compatibility.

 I think the way WordPress has a `$wp_roles` global, we should have
 `bp()->roles` as the place where BuddyPress components store their multi-
 dimensional role & capability arrays, completely outside of WordPress's,
 but using a similar & familiar approach.

 We could also then store our own role assignments in `usermeta` in a way
 that doesn't conflict with WordPress's per-site implementation, or pollute
 that space later if BuddyPress is deactivated, but I'm not sure this is
 good or bad etiquette.

 If each component has their own roles with their own caps, imagine having:

 * Activity Administrator
 * Group Administrator
 * Friends Administrator
 * Notifications Administrator
 * Messages Administrator
 * XProfile Administrator
 * Settings Administrator
 * Blogs/Sites Administrator

 Admins being the role that can always see and perform all actions on all
 data for all users. Whether this role is stored in the user's literal
 `wp_ROOT_BLOG_capabilities` meta, or some other meta, doesn't really
 matter very much, so long as it's persistent to the user, and force-
 granted by the presence of `bp_moderate`.

 Similar to bbPress, we could then have "Moderator", "Participant",
 "Spectator", and "Blocked" roles with hard-coded primitive capabilities to
 define what their intentions are, and allow each component to derive from
 them based on whatever the conditions are at the time. (I don't really
 care about the literal names, so much as the spirit of their ranking &
 abilities.)

 I also think, that role & cap checks should not be flat, or part of the
 global namespace. `bp_current_user_can( 'edit_bp_activity' )` for example,
 is going to break-down very quickly as soon as we want to add more crud
 actions to more places for more user-to-user type things.

 Instead, I've always imagined something more like:

 {{{
 $single_activity_check = bp()->activity->current_user_can(
 'edit_activity', $activity_id );
 }}}

 Basically, a way make each component responsible for the registration,
 mapping, checking, and confirming of their own relative caps. Whether
 `bp_current_user_can()` continues to be a funnel or not, will be a funny
 implementation detail we'll have to discover once we get started.

 ----

 @DJPaul thanks or taking a first stab at this. User permissions are
 arguably one of the more difficult concepts to work with, barring maybe
 time & date formats. We will all get this wrong a hundred times before we
 get it right, so I think this is an important first step, and Activity is
 a great component to start with.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/7176#comment:10>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac