Skip to:
Content

BuddyPress.org

Opened 8 years ago

Closed 7 years ago

#4814 closed defect (bug) (fixed)

spammer can create group

Reported by: intimez Owned by: r-a-y
Milestone: 1.8 Priority: normal
Severity: critical Version: 1.7
Component: Groups Keywords: dev-feedback
Cc:

Description

when an admin mark the user as spammer, that user can still create a group and any content created after that point is visible to all.

twenty twelve 1.1
wordpress 3.5.1
trunk-6776

Attachments (1)

4814.01.patch (1.7 KB) - added by r-a-y 8 years ago.

Download all attachments as: .zip

Change History (13)

#1 @r-a-y
8 years ago

  • Keywords reporter-feedback added

I can't duplicate this on latest trunk.

I tried logging in as a spammer and I am not able to authenticate because BP throws up a "Your account has been marked as a spammer" notice.

Are you talking about marking a spammer who is already logged in?

#2 @intimez
8 years ago

Yes when the spammer is caught in the act and banned right away the spammer can continue as normal. I tested with two browsers (one as admin and the other as spammer) and was able to recreate issue each time.

#3 @johnjamesjacoby
8 years ago

  • Component changed from Administration to Groups
  • Milestone changed from Awaiting Review to 1.8

Valid, but not a regression. Moving to 1.8.

#4 @r-a-y
8 years ago

  • Keywords dev-feedback added; reporter-feedback removed

Just duplicated this bug.

intimez - I've created a hotfix plugin for this:
https://github.com/r-a-y/bp-stop-live-spammers

I can port the changes back to BP for 1.7 if the other devs think the approach is sound.

@r-a-y
8 years ago

#5 @r-a-y
8 years ago

  • Keywords has-patch added

Decided to add a patch for this.

If a logged-in user is marked as a spammer, 01.patch checks to see if the user is a spammer. If so, access to the site is killed off by using wp_die().

At first, I thought about redirecting to wp-login.php?reauth=1. This logged the user out, but this didn't deter the user from simply re-registering a new account and spamming again.

Patched against 1.7-bleeding with the hope of moving this into 1.7.

Let me know what you think.

#6 @boonebgorges
8 years ago

  • Keywords commit added

I'm good with this technique. Thanks, r-a-y!

#7 @r-a-y
7 years ago

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

In 6978:

Stop a logged-in spammer from being able to access the site.

When an admin marks a live user as a spammer, that user can still surf
around and cause havoc on the site until that person is logged out.

This commit stops live spammers from accessing the site on the next
page load.

Fixes #4814.

#8 follow-up: @DJPaul
7 years ago

  • Keywords has-patch commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening. If we are going to kill the page like this, we should use the existing bp_is_user_spammer() and bp_is_user_deleted() functions, rather than off-load the logic.

This is also a fairly substantial change in our approach; until now, we've not stopped spam accounts being able to *read* the site, and instead have been preventing them posting to forms, and blocking them from creating new content (this bug not withstanding), etc. I think I prefer this "old" approach, so I wanted to re-open for a bit more discussion.

Sorry for not having seen this patch in the last couple of months to offer this feedback prior to the commit.

#9 in reply to: ↑ 8 @johnjamesjacoby
7 years ago

Replying to DJPaul:

Reopening. If we are going to kill the page like this, we should use the existing bp_is_user_spammer() and bp_is_user_deleted() functions, rather than off-load the logic.

This is also a fairly substantial change in our approach; until now, we've not stopped spam accounts being able to *read* the site, and instead have been preventing them posting to forms, and blocking them from creating new content (this bug not withstanding), etc. I think I prefer this "old" approach, so I wanted to re-open for a bit more discussion.

Sorry for not having seen this patch in the last couple of months to offer this feedback prior to the commit.

Agree with Paul here. Something tells me the approach is probably fine, though it is a drastic behavioral change to make without having discussed it in a dev chat together. The wp_die() approach gets the point across, but maybe it should be a more informative message/page within the theme? Like a 404 but for logged in users.

#10 @r-a-y
7 years ago

In 6983:

In bp_is_user_spammer(), if user is logged-in user or displayed user, use
the already-queried userdata.

Use bp_is_user_spammer() in bp_stop_live_spammer() function.

See #4814.

#11 @r-a-y
7 years ago

Something tells me the approach is probably fine, though it is a drastic behavioral change to make without having discussed it in a dev chat together.

Sorry for not discussing this in a dev chat!

I don't mind how we go about this issue, but we do have to address how a live user that is marked as a spammer is still able to do things as mentioned at the beginning of this ticket.

My two cents can be found above.


The wp_die() approach gets the point across, but maybe it should be a more informative message/page within the theme? Like a 404 but for logged in users.

That could work as well. Perhaps redirect to homepage and use bp_core_add_message() saying the user is a spammer?


If we are going to kill the page like this, we should use the existing bp_is_user_spammer() and bp_is_user_deleted() functions, rather than off-load the logic.

I decided not to use bp_is_user_spammer(), because we've already queried that data for the logged-in user in the $bp->loggedin_user object. Thanks to your feedback, I've added that logic directly in the function now. See r6893.

Version 0, edited 7 years ago by r-a-y (next)

#12 @r-a-y
7 years ago

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

In 6990:

Change logic of bp_stop_live_spammer().

Instead of using wp_die(), reauthorize the user by redirecting the logged-in
spammer to wp-login.php with the 'reauth' parameter.

bp_live_spammer_login_error() adds an error message for the logged-in
spammer on the wp-login.php page.

Fixes #4814.

Note: See TracTickets for help on using tickets.