Skip to:
Content

Opened 3 years ago

Last modified 3 years ago

#6123 new defect (bug)

"Signups" are displayed as members on non multisite configs

Reported by: imath Owned by:
Milestone: Future Release Priority: high
Severity: normal Version:
Component: Members Keywords: 2nd-opinion needs-refresh
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 3 years ago.
6123.02.diff (2.2 KB) - added by imath 3 years ago.
6123.03.patch (2.2 KB) - added by imath 3 years ago.
6123.04.patch (2.2 KB) - added by imath 3 years ago.

Download all attachments as: .zip

Change History (22)

@imath
3 years ago

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


3 years ago

#2 @imath
3 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
3 years ago

#3 @johnjamesjacoby
3 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
3 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
3 years ago

What about 03.patch (bp_is_user_inactive()) ?

@imath
3 years ago

#6 @imath
3 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
3 years ago

#7 @DJPaul
3 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.


3 years ago

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


3 years ago

#10 @johnjamesjacoby
3 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
3 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
3 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
3 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
3 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
3 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
3 years ago

  • Keywords needs-refresh added; has-patch removed
  • Milestone changed from 2.3 to Future Release
Note: See TracTickets for help on using tickets.