WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 6 months ago

#20774 new defect (bug)

Flagging a user with any role on a subsite as spam leads to flagging the site as spam

Reported by: j.conti Owned by:
Milestone: Future Release Priority: normal
Severity: major Version: 3.0
Component: Users Keywords: has-patch needs-testing needs-refresh
Focuses: multisite Cc:

Description

Hi,

Since many weeks, many WangGuard users were contacting me because they say WangGuard were flagging the sub sites as spam.

Thats not a WangGuard bug, is a WordPress issue.

Steps:

  • Create a Subsite, you can add the Super Admin like site admin.
  • Add a user like subscriber.
  • Flag that user as spam in /network/users.php

That site will be flagged as spam although the owner were the Super Admin.

I think WordPress has to look for the user rol and only flag the sub site as spam if the rol is Administrator.

This issue is in WP 3.3.1 and 3.4

Attachments (7)

spam-unspam-user.diff (1.7 KB) - added by wonderboymusic 5 years ago.
20774.diff (2.8 KB) - added by martythornley 4 years ago.
spams site based on user email and adds filter
20774.better.diff (6.1 KB) - added by martythornley 4 years ago.
added quick links
20774.colors.diff (6.1 KB) - added by martythornley 4 years ago.
added span to get link colors
20774.correct.diff (6.1 KB) - added by martythornley 4 years ago.
20774.last.diff (6.8 KB) - added by martythornley 4 years ago.
fixed quick links so they check for blogs too
20774.noquicklinks.diff (4.3 KB) - added by martythornley 4 years ago.
Removed quicklinks. Removed 'unspamming" of sites. added an int_val check on posted user ids

Download all attachments as: .zip

Change History (35)

#1 @SergeyBiryukov
5 years ago

  • Keywords needs-patch added

Confirmed.

#2 @nacin
5 years ago

  • Version changed from 3.4 to 3.0

#3 @wonderboymusic
5 years ago

  • Keywords has-patch added; needs-patch removed

Spamming a site when marking a spammy user makes absolutely no sense. The code in that file is pretty bad, not surprised. Patched. Also added cap check and validates $user.

#4 @wonderboymusic
5 years ago

  • Milestone changed from Awaiting Review to 3.5

#5 @johnjamesjacoby
4 years ago

  • Cc johnjamesjacoby added

Confirmed; happens on BuddyPress.org when I need to spam a user that's also a user on the codex. Assumed it was intentional, but would make sense this is a bug. Either way, it's odd behavior.

#6 @nacin
4 years ago

  • Milestone changed from 3.5 to Future Release

Using the spam tools in WP means your mileage may vary. I'd like to not fix this long-standing bug without including code to spam a site where the spammed user is an "owner" based on role or email address, as it would make existing spam tools in core more difficult to use.

@martythornley
4 years ago

spams site based on user email and adds filter

#7 @martythornley
4 years ago

New filter 'mark_sites_for_spam_user' will toggle whether we spam all sites owned by user. By default it is set to false. If you change to true, it looks for blogs where admin email matches the user email and marks it as spam.

This uncovered a separate issue of sorts... when trying to "unspam" a user, we can no longer find the site that was just spammed. This is because the function get_blogs_of_user cannot find spam sites because it looks for the capabilities user meta but that is no longer there once the spamming action happened. So the second half of it where it looks like it should unspam the user's sites does nothing.

#8 @brianhogg
4 years ago

Might even be a non-issue that 'unspamming' a user doesn't unspam the site, as the site could be marked as spam separately and unspamming the site could be unintended. Removing the loop in the 'notspam' case in the patch should wrap things up?

#9 @martythornley
4 years ago

I take it back... the unspamming works afterall. Must have accidentally done something in the constant testing of the patch.

Still a question of whether unspamming a site is good or bad here. Might be better to just unspam a user but leave blog moderation to a separate task?

@martythornley
4 years ago

added quick links

#10 @martythornley
4 years ago

New patch adds quick links for spam an unspam. They work but not sure how to get the color correct like the spam/unspam links in the sites page.

@martythornley
4 years ago

added span to get link colors

#11 @martythornley
4 years ago

Sorry... Last couple had the filter set to true for testing. This has it back to false how it should be.

@martythornley
4 years ago

fixed quick links so they check for blogs too

#12 @brianhogg
4 years ago

  • Cc brian@… added

#13 @jeremyfelt
4 years ago

  • Keywords needs-testing added
  • Milestone changed from Future Release to 3.7

#14 @nacin
4 years ago

The approach in #24771 was to decouple the actions — spamming a user should not affect a site, and vice versa. Maybe we should replicate that here.

I am not sure we need "Spam" and "Unspam" as action links. The vast majority of networks won't benefit from this, and could add them with a plugin. Thoughts?

#15 @brianhogg
4 years ago

I don't think we should be unmarking blogs as spam when a user is marked as not spam, ie. line 166

update_blog_status( $details->userblog_id, 'spam', '0' );

Since we don't know if the blog was marked as spam separately from the user. Really marking a user as spam should never automatically mark the blog as spam by default IMO, the action could be used to do that for wordpress.com and others that need it.

#16 @martythornley
4 years ago

I agree with decoupling the actions but providing a filter to allow us to join them if we want. Right now it is setup to false so that it does not spam a site as you spam a user. I believe that is inline with what we talked about at WCSF contributor day as we did the patch? Right now they are decoupled and you would need to filter it to turn it on. That way you can activate it for .com.

For now, I left in the functions to unspam a site to act in a similar way but definitely agree that there is no use for unspamming a site as a user is unspammed. I can remove if that is the consensus?

For the quick links, maybe add a filter there? Put the standard links back to how it was but that way if I use spam/unspam a lot, I could add them back in with a plugin. Either way, there should be a filter as there is no way to manipulate those links right now.

@martythornley
4 years ago

Removed quicklinks. Removed 'unspamming" of sites. added an int_val check on posted user ids

#17 @SergeyBiryukov
4 years ago

  • Summary changed from Flag an user as spam with any rol become a sub site flagged as spam (Multisite) to Flagging a user with any role on a subsite as spam leads to flagging the site as spam

#18 @nacin
3 years ago

  • Milestone changed from 3.7 to 3.8

This appears good but needs further review and testing.

#19 @nacin
3 years ago

  • Milestone changed from 3.8 to Future Release

#20 @jeremyfelt
3 years ago

  • Component changed from Multisite to Users
  • Focuses multisite added

This ticket was mentioned in Slack in #core by martythornley. View the logs.


2 years ago

This ticket was mentioned in Slack in #core-multisite by martythornley. View the logs.


2 years ago

#23 @jeremyfelt
2 years ago

  • Milestone changed from Future Release to 4.2
  • Let's stick to modifying bulk action only and not add action links for Spam and Unspam.
  • It almost feels like the filter should be true by default as it more matches current behavior. Though some of the current behavior is what we're trying to fix. It looks like there may have been an offline discussion about this previously. Any details on that?
  • Is mark_sites_for_spam_user the right filter name? We should have full docs for the filter as well before this goes in.

#24 @MartyThornley
2 years ago

Basically everything comment 7 - comment 16 or so was at contributor day at WCSF 2013 with offline discussion and work between @brianhogg and I with input from @nacin.

There was some discussion around proper filter name. That was the best we came up with but since it is new, it could change for sure.

Defaulting to false/true, the question is should a user marked as spam mean that any site they are an admin on be spammed. One example is that that user might be spammy but they might be an admin on another site where other admins are there as well and the site as a whole is fine. Should deciding a user is spam mean all their SITES are spam as well?

#25 @jeremyfelt
2 years ago

  • Keywords needs-refresh added

Ok, that makes sense. Let's stick with a default of false and mark_sites_for_spam_user as the filter name for now.

I think all we need here is refreshed patch against trunk that only handles bulk actions and filter documentation.

#26 @jeremyfelt
2 years ago

  • Milestone changed from 4.2 to Future Release

We need an updated patch with some filter docs. Let's try for 4.3 on this.

#27 @SergeyBiryukov
8 months ago

#37538 was marked as a duplicate.

#28 @ocean90
6 months ago

#38124 was marked as a duplicate.

Note: See TracTickets for help on using tickets.