[buddypress-trac] [BuddyPress Trac] #7391: Can 'change' visibility on registration form even for fields marked "Enforce field visibility"

[buddypress-trac]

#7391: Can 'change' visibility on registration form even for fields marked
"Enforce field visibility"
------------------------------------+--------------------
 Reporter:  maccast                 |       Owner:
     Type:  defect (bug)            |      Status:  new
 Priority:  normal                  |   Milestone:  2.7.4
Component:  Extended Profile        |     Version:  2.7.2
 Severity:  normal                  |  Resolution:
 Keywords:  dev-feedback has-patch  |
------------------------------------+--------------------

Comment (by boonebgorges):

 Thanks for your patches and thoughts here, @hnla.

 I know that it seems cumbersome to move these sorts of checks into
 capability checks instead of just using template functions. But using
 proper cap checks becomes increasingly important as we move toward REST
 API endpoints, because we need consistent ways of checking permissions
 regardless of client (theme templates, REST request, etc). So the approach
 in [attachment:7391-update-xprofile-caps.patch] seems preferable to me.
 [attachment:7391.diff] is a slightly cleaned up version.

 The fact that we have a special case for non-logged-in users in this case
 is a result of a stupid quirk in the way that BP interacts with WP. In the
 long run, we may need a better solution. See
 https://buddypress.trac.wordpress.org/ticket/7298#comment:5 and follow-up
 conversation.

 Looking at this, I see that there are some other bugs in the way this
 capability is mapped. First, we should adopt a policy of always passing
 the `field_id` to the `bp_current_user_can()` check, and only use the
 `bp_get_the_profile_field_id()` value as a fallback. This helps to prevent
 security-related bugs. However, this isn't a simple change to make: At
 some point in the past, we started requiring `$args` (as in
 `bp_current_user_can( 'cap_name', $args )`) to be an associative array. (I
 think this had something to do with multisite support - I haven't looked
 through the logs.) So the checks should look like this:

 {{{
 if ( bp_current_user_can( 'bp_xprofile_change_field_visibility', array(
 'field_id' => $field_id ) ) ) {
 }}}

 But the way we check `$args` in `bp_xprofile_map_meta_caps()` is not
 compatible with this: we look for a numerically-indexed `$args` array.
 This problem is not something we necessarily need to fix for 2.7.x (I
 don't think it's new to 2.7) but we should address it for 2.8.

 @hnla Could you have a glance at [attachment:7391.diff] to see if my
 updates look OK to you? If so, please go ahead and commit, and I can open
 a new ticket to address the other bugs.

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