Skip to:
Content

BuddyPress.org

Opened 10 years ago

Closed 7 months ago

Last modified 5 months ago

#6123 closed defect (bug) (fixed)

"Signups" are displayed as members on non multisite configs

Reported by: imath's profile imath Owned by: imath's profile imath
Milestone: 14.0.0 Priority: high
Severity: normal Version:
Component: Members Keywords: has-patch
Cc:

Description

On non multisite configs, historically signups were users having a user status set to 2. Since 2.0 we are using the wp_signups table but we keep on creating the user if BP_SIGNUPS_SKIP_USER_CREATION is not set to true (which means by default).

I've just noticed, the profile of this kind of users (which are not yet actually) can be seen by any visitor of the site. I think we shouldn't display these users and simply do a 404. What do you think ?

Attachments (4)

6123.patch (4.4 KB) - added by imath 10 years ago.
6123.02.diff (2.2 KB) - added by imath 10 years ago.
6123.03.patch (2.2 KB) - added by imath 10 years ago.
6123.04.patch (2.2 KB) - added by imath 10 years ago.

Download all attachments as: .zip

Change History (31)

@imath
10 years ago

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


10 years ago

#2 @imath
10 years ago

As DJPaul suggested on slack, i think we can simply use the bp_is_user_deleted() function and avoid creating a new function that is currently doing the same checks. That's what i did in 02.patch.

@imath
10 years ago

#3 @johnjamesjacoby
10 years ago

Using bp_is_user_deleted() for checking if a user isn't active yet, seems incorrect. bp_is_user_active() or some new equivalent function that checks the exact condition we need and returns (bool) would be best.

#4 @imath
10 years ago

The problem with bp_is_user_active() is that it also checks bp_is_user_spammer() in it. so to have the "not yet activated account" the checks would be something like:

if ( bp_is_user_active( bp_displayed_user_id() ) && ! bp_is_user_spammer( bp_displayed_user_id() ) )

#5 @imath
10 years ago

What about 03.patch (bp_is_user_inactive()) ?

@imath
10 years ago

#6 @imath
10 years ago

In 6123.04.patch, i'm improving the canonical stack as i've noticed previous patch was not dealing the right way with it if the logged in user was the super admin.

@imath
10 years ago

#7 @DJPaul
10 years ago

New patch is looking better, but I think you should check with johnjamesjacoby first.

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


10 years ago

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


10 years ago

#10 @johnjamesjacoby
10 years ago

This is going to be tricky. I don't think we have adequate functions for determining a users actual status.

  • bp_is_user_deleted() checks 2 == $user->user_status which conflicts with our more recent sign-ups status. It also checks $user->deleted which appears to be completely unsupported by WordPress core.
  • We should probably have something like bp_is_user_pending() to tell us "This person signed up, but hasn't activated their account."
  • All of our bp_is_user_ status functions truly need to be modernized on top of a user-status API before we go too deep. Then we need bp_get_member_status( $user_id ) that returns a literal string based on that API.

Do the bare minimum to prevent non-moderator level users from seeing inactive accounts, and let's look heavily into this for 2.3.

#11 follow-up: @johnjamesjacoby
10 years ago

I think if a user has not activated their account, we should be 404'ing them in bp_core_set_uri_globals() so no user can ever see their account theme-side.

#13 in reply to: ↑ 11 ; follow-up: @johnjamesjacoby
10 years ago

Replying to johnjamesjacoby:

I think if a user has not activated their account, we should be 404'ing them in bp_core_set_uri_globals() so no user can ever see their account theme-side.

Or maybe this is less unusual than we think. It's not totally unheard of to have an "inactive" account that you can still log into, it's just limited in some capacities until it's verified via the activation key.

Perhaps we leave this as is for 2.2 and address this more completely as a feature enhancement in 2.3?

#14 @imath
10 years ago

Thanks jjj for looking at it. Personaly i think we should try to be consistent between the different configs.
Today if multisite or regular but skip user creation constant on: signups are not displayed because users are not yet created.
If skip user creation is false or undefined on regular configs users are created and so signups are displayed.
It's also weird to be able to add the user as a friend or mention him... although he's not yet a user ;)

#15 in reply to: ↑ 13 @imath
10 years ago

  • Milestone changed from 2.2 to 2.3

Replying to johnjamesjacoby:

Perhaps we leave this as is for 2.2 and address this more completely as a feature enhancement in 2.3?

I agree

#16 @johnjamesjacoby
10 years ago

It's weird, but not completely unnatural.

If I ask you to sign-up for this cool new network, and I see that you do. I can mention you all I want, but you won't know until you activate your account. Basically, these accounts are pending verification, but they do still exist in some capacity, and I guess I'm okay with that for now?

#18 @DJPaul
10 years ago

  • Keywords needs-refresh added; has-patch removed
  • Milestone changed from 2.3 to Future Release

#19 @imath
10 months ago

  • Milestone changed from Awaiting Contributions to 14.0.0

#20 @imath
9 months ago

  • Owner set to imath
  • Status changed from new to assigned

#21 @imath
7 months ago

  • Keywords 2nd-opinion removed

To fix this issue, we simply need to change the default behavior of the check we perform on BP_SIGNUPS_SKIP_USER_CREATION so that by default we're not creating a user anymore.

This ticket was mentioned in PR #271 on buddypress/buddypress by imath.


7 months ago
#22

  • Keywords has-patch added; needs-refresh removed

This backwards compatibility feature was introduced in version 2.0.0, it's causing an issue as the just registered user's profile can already be seen on the front-end. Moreover as it's not yet a user, let's be consistent and only deal with our BP Signups API to manage signups.

The goal of this PR is to stop creating a user on regular WordPress site when a registration is performed.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/6123

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


7 months ago

#24 @espellcaste
7 months ago

I just tested this locally and agree with the overall goal here.

imath commented on PR #271:


7 months ago
#25

Thanks for your suggestions here and your feedback on the Trac ticket @renatonascalves I will update the PR asap.

#26 @imath
7 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 13798:

Stop creating a user when a signup is performed on regular WP configs

Since version 2.0, we are using the $wpdb->signups table to manage pending accounts. We kept on creating a user as well as a user meta to store the activation key for regular WP configs.

This user creation step is now deprecated and by default no user will be created on any WordPress config once a user signs up. In next BP major version we'll fully remove the code but until then you can always use the bp_signups_create_user filter (returning true) or define the deprecated constant BP_SIGNUPS_SKIP_USER_CREATION to false to carry on creating the user on regular WP configs.

This move allows us to fix a 9 years old ticket and is more consistent: signups are becoming users only when they validate their account.

Props johnjamesjacoby, DJPaul, espellcaste

Fixes #6123
Closes https://github.com/buddypress/buddypress/pull/271

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


5 months ago

Note: See TracTickets for help on using tickets.