Skip to:
Content

BuddyPress.org

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#7553 closed defect (bug) (fixed)

Xprofile field default visibility not honored during POST processing of registration form

Reported by: uscore713's profile uscore713 Owned by: r-a-y's profile r-a-y
Milestone: 2.9 Priority: high
Severity: normal Version: 1.6
Component: Registration Keywords: has-patch
Cc:

Description

I have an application requirement whereby the standard profile field visibility controls are NOT to be presented on the registration page, so I cloned the standard members/register.php template and simply removed all the visibility-related elements.

This of course has the effect of no visibility-related fields being posted on form submission, and when the POST (usermeta) data is processed, the net effect is that each field's visibility is set to public, as the processing does not take into account the field's (potentially defined) default visibility.

This raises a bit of a security problem in our application, as several profile fields are for User and Admin use. (I elected to not post this report in the WordPress Hacker One program as I don't believe it ranks up there as a typical "security issue".)

This problem exists in the BP_Signup class (buddypress/bp-members/classes/class-bp-signup.php), in the add_backcompat() method, at lines 392-393:

// Save the visibility level.
$visibility_level = ! empty( $usermeta['field_' . $field_id . '_visibility'] ) ? $usermeta['field_' . $field_id . '_visibility'] : 'public';
xprofile_set_field_visibility_level( $field_id, $user_id, $visibility_level );

The fix for this security problem is something like the following (the mod we use to get past this problem for now):

// Save the visibility level.
// Use the field's default visibility if not present, and use 'public' IFF a default visibility is not defined
$key = 'field_' . $field_id . '_visibility';
if ( isset( $usermeta[ $key ] ) ) {
    $visibility_level = $usermeta[ $key ];
} else {
    $vfield           = xprofile_get_field( $field_id );
    $visibility_level = isset( $vfield->default_visibility ) ? $vfield->default_visibility : 'public';
}
xprofile_set_field_visibility_level( $field_id, $user_id, $visibility_level );

This problem exists in BP 2.8.2; no other versions have been examined. Hopefully this can be fixed in the next point release.

Attachments (2)

7553.01.patch (2.2 KB) - added by r-a-y 8 years ago.
7553.02.patch (3.6 KB) - added by r-a-y 8 years ago.

Download all attachments as: .zip

Change History (12)

This ticket was mentioned in Slack in #buddypress by offereins. View the logs.


8 years ago

#2 @DJPaul
8 years ago

  • Component changed from Core to Registration
  • Milestone changed from Awaiting Review to 2.9

Hi @uscore713 - thanks for the report. We think you're right, and will look at for our next release, 2.9 (which has just entered beta).

#3 @r-a-y
8 years ago

  • Keywords needs-unit-tests added
  • Version changed from 2.8.2 to 1.6

I've patched what @uscore713 recommends, as well as another spot that uses xprofile_set_field_visibility_level() in bp_core_activate_signup().

Needs unit tests.

@r-a-y
8 years ago

#4 @Offereins
8 years ago

Saw the patch. Looks good.

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.


8 years ago

@r-a-y
8 years ago

#6 @r-a-y
8 years ago

  • Keywords needs-unit-tests removed

02.patch includes a unit test exhibiting the problem.

I think this is ready for commit.

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.


8 years ago

#8 @r-a-y
8 years ago

  • Owner set to r-a-y
  • Resolution set to fixed
  • Status changed from new to closed

In 11629:

Activation: Ensure we use the XProfile field's default visibility level during activation.

Previously, if the visibility option for a XProfile field was removed from
the registration template, the visibility level for the user would always
fallback to 'public' instead of what the actual XProfile field's default
visibility level was.

Commit includes a unit test and rectifies the problem.

Props uscore713.

Fixes #7553 (2.8-branch)

#9 @r-a-y
8 years ago

In 11630:

Activation: Ensure we use the XProfile field's default visibility level during activation.

Previously, if the visibility option for a XProfile field was removed from
the registration template, the visibility level for the user would always
fallback to 'public' instead of what the actual XProfile field's default
visibility level was.

Commit includes a unit test and rectifies the problem.

Props uscore713.

Fixes #7553 (trunk)

#10 @r-a-y
8 years ago

In 11639:

Activation: After r11630, fix issues with multisite unit test.

The unit test written for #7553 can cause a database error when
wpmu_create_blog() fires when running the multisite test suite.

To workaround this, we remove the 'domain' and 'path' from the signup
user to prevent wpmu_create_blog() from firing.

Props netweb.

See #7553.

Note: See TracTickets for help on using tickets.