Spam protection tools - #2138
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| def post_types | ||
| @post_types = @category.top_level_post_types | ||
| if @post_types.one? | ||
| if !user_signed_in? |
There was a problem hiding this comment.
Apparently one of our system tests was relying on this redirecting, which was order-dependent, so I've moved it to always redirect when not signed in.
| @@ -0,0 +1,34 @@ | |||
| module UserTestHelpers | |||
There was a problem hiding this comment.
This whole file is just the existing helpers extracted from users_controller_test.rb to allow for a restructure.
| @@ -0,0 +1,252 @@ | |||
| require 'test_helper' | |||
There was a problem hiding this comment.
This file is the existing tests relating to user mod tools extracted into their own file, because the users_controller_test.rb was getting excessively long.
|
A couple initial questions (haven't started testing/reviewing yet):
|
They don't find out specifically what blocked the post - in line with the obscurity in the rest of our spam tools, we spit out a "useful" error message and leave it at that. I don't see many (any?) legitimate reasons for a new user to post identical post content either multiple times on one site, or several times across the network, so I'm reasonably comfortable with that. There's more of a chance of an established user/mod/admin posting identical posts across the network (I think we've done it as staff), but anyone with >=10 (configurable) non-deleted posts network-wide is exempt from this completely.
Yes.
Same feedback - a "useful" error message. They also wouldn't be able to post on Meta so would have to contact support for help. However:
The theory is that a potential spammer would be blocked from posting until a moderator reviews the spam flag. On review, they'd then mark the flag as helpful and nuke the user, obviating the need for a post block, and the new deletion job will apply a STAT block. A legitimate user who was wrongly spam-flagged would also be blocked until review, but would then be released once the flag is declined. Likewise, users with >=10 (configurable) non-deleted posts network-wide are completely exempt.
Pending drafts aren't saved as posts (they're actually not in the DB at all, but in Redis), so won't interact with this at all unless the user tries to post it a second time without modification - which would be rejected. |
|
That all sounds good to me. Thanks for explaining! |
|
Duplicate posts: I tested with a user who had exactly 10 posts, 3 of them deleted. I copied the content of an existing post and was able to repost it. (Tried with both questions and answers.) I then tried with a user with no posts -- added an answer (ok), then cut/pasted that on a different question and got the error as expected. Does this mean we're only checking against posts created after this change is in place? (That's fine, but I want to check that it's intentional and not a bug.) I tested duplicate posts across communities; worked as expected. Adding a carriage return was enough to make the posts non-identical (post succeeded); do we want to normalize or are we not worried about this case? -- Spam flags: Tested with a user with < 10 posts and an active spam flag, both local community and elsewhere on the network. Both produced errors as expected, and declining the flag enabled posting again. Note: I tested this with the same user from my first paragraph: the flag block worked but the duplicate content block didn't. Are we accidentally counting deleted posts in one place but not the other? Tested with a user with > 10 posts and an active spam flag. Posting permitted, as expected. -- Error message: The actual error message looks like one of our 418s -- "The heisenbug vanished when we opened the logs. Please look away and try again." I hadn't followed your "useful" link before; while your comment seemed fine when I read it, seeing the cryptic error on the post page "live" feels different. I'd be more comfortable with a vague error than a cryptic one, especially in the case of spam flags because sometimes people are wrong about that. Can we just say something like "post rejected"? A real person would then come to meta to ask, and meanwhile a spammer doesn't get useful information. -- Deleted spammers: I'm a little afraid to IP-block myself on my dev environment, so I haven't tested deleting spammers. |
Does "rejected" here mean that an accidental second posting will not work, leaving the user unharmed, or that they will be flagged and blocked from posting anything? Some users on seeing their draft question or answer still present after posting (due to #1545) assume that posting failed, and post again. Previously this has led to them noticing the unintended duplicate soon after and raising it on Meta (or commenting under an existing Meta post). Will this still be possible with this pull request or will such a user be blocked from posting on Meta? I can imagine a new user caught by this might not realise that emailing support is even an option. |
@trichoplax the former - the identical post check just blocks that post, and doesn't apply any ongoing consequences to the account. |
No repro - I have a user with 10 posts, 2 deleted. I copied a post's Markdown and got the expected error. Did you copy the Markdown or the post's rendered appearance? The latter might have introduced changes in the Markdown, which would mean the post passes the check.
Done.
If you want to test it, you can always go into the DB manually afterwards to unblock yourself - |
I did copy the markdown (went into edit to get it), but now I can't repro either with the same user and same post -- maybe I missed a terminal newline and that made the difference?
Thank you. |
cellio
left a comment
There was a problem hiding this comment.
LGTM. Someone else should review code (particularly the queries and CIDR). I didn't test CIDR/auto-escalated-flag.
Uh oh!
There was an error while loading. Please reload this page.