Adding configurations to tighten spam handling (#18195)

This is a draft highlighting a proof of concept to allow for each Forem
to add tighter enforcement of spam.

Related to forem/forem-internal-eng#453
This commit is contained in:
Jeremy Friesen 2022-08-01 11:21:34 -04:00 committed by GitHub
parent dcc669de47
commit 5296b6c4b4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 63 additions and 9 deletions

View file

@ -87,20 +87,33 @@ class Reaction < ApplicationRecord
end
# @param user [User] the user who might be spamming the system
# @param threshold [Integer] the number of strikes before they are spam
# @param include_user_profile [Boolean] do we include the user's profile as part of the "check
# for spamminess"
#
# @return [TrueClass] yup, they're spamming the system.
# @return [FalseClass] they're not (yet) spamming the system
def user_has_been_given_too_many_spammy_article_reactions?(user:, threshold: 2)
def user_has_been_given_too_many_spammy_article_reactions?(user:, threshold: 2, include_user_profile: false)
threshold -= 1 if include_user_profile && user_has_spammy_profile_reaction?(user: user)
article_vomits.where(reactable_id: user.articles.ids).size > threshold
end
# @param user [User] the user who might be spamming the system
# @param threshold [Integer] the number of strikes before they are spam
# @param include_user_profile [Boolean] do we include the user's profile as part of the "check
# for spamminess"
#
# @return [TrueClass] yup, they're spamming the system.
# @return [FalseClass] they're not (yet) spamming the system
def user_has_been_given_too_many_spammy_comment_reactions?(user:, threshold: 2)
def user_has_been_given_too_many_spammy_comment_reactions?(user:, threshold: 2, include_user_profile: false)
threshold -= 1 if include_user_profile && user_has_spammy_profile_reaction?(user: user)
comment_vomits.where(reactable_id: user.comments.ids).size > threshold
end
# @param user [User] the user who might be spamming the system
def user_has_spammy_profile_reaction?(user:)
user_vomits.exists?(reactable_id: user.id)
end
end
# no need to send notification if:

View file

@ -3,6 +3,18 @@ module Spam
#
# @note We may not immediately block spam but instead slowly escalate our response.
module Handler
# @return [TrueClass] if we are going to try to use more rigorous spam handling
# @return [FalseClass] if we are using less rigorous spam handling
def self.more_rigorous_user_profile_spam_checking?
FeatureFlag.enabled?(:more_rigorous_user_profile_spam_checking)
end
# @return [TrueClass] if we are going to unpublish articles when we auto-suspend
# @return [FalseClass] if we are not going to unpublish articles when we auto-suspend
def self.unpublish_all_posts_when_user_auto_suspended?
FeatureFlag.enabled?(:unpublish_all_posts_when_user_auto_suspended)
end
# Test the article for spamminess. If it's not spammy, don't do anything.
#
# If it is spammy, escalate the situation!
@ -15,7 +27,10 @@ module Spam
issue_spam_reaction_for!(reactable: article)
return unless Reaction.user_has_been_given_too_many_spammy_article_reactions?(user: article.user)
return unless Reaction.user_has_been_given_too_many_spammy_article_reactions?(
user: article.user,
include_user_profile: more_rigorous_user_profile_spam_checking?,
)
suspend!(user: article.user)
end
@ -37,7 +52,10 @@ module Spam
issue_spam_reaction_for!(reactable: comment)
return unless Reaction.user_has_been_given_too_many_spammy_comment_reactions?(user: comment.user)
return unless Reaction.user_has_been_given_too_many_spammy_comment_reactions?(
user: comment.user,
include_user_profile: more_rigorous_user_profile_spam_checking?,
)
suspend!(user: comment.user)
end
@ -50,7 +68,7 @@ module Spam
def self.handle_user!(user:)
text = [user.name]
if FeatureFlag.enabled?(:more_rigorous_user_profile_spam_checking)
if more_rigorous_user_profile_spam_checking?
text += [
user.email,
user.github_username,
@ -79,6 +97,9 @@ module Spam
# written they might still be logged in but have limited
# abilities.
def self.suspend!(user:)
# TODO: Should we send an email when we auto-suspend? As a matter of practice, whenever we
# suspend someone should we notify. Note, this is not the only place that we suspend
# someone.
user.add_role(:suspended)
Note.create(
@ -87,6 +108,10 @@ module Spam
reason: "automatic_suspend",
content: I18n.t("models.comment.suspended_too_many"),
)
return unless unpublish_all_posts_when_user_auto_suspended?
user.articles.update_all(published: false)
end
private_class_method :suspend!

View file

@ -17,6 +17,13 @@ RSpec.describe Reaction, type: :model do
it "performs a valid query for the user" do
expect { described_class.user_has_been_given_too_many_spammy_article_reactions?(user: user) }.not_to raise_error
end
it "performs a valid query for the user with the include_user_profile logic" do
expect do
described_class.user_has_been_given_too_many_spammy_article_reactions?(user: user,
include_user_profile: true)
end.not_to raise_error
end
end
describe "counter_culture" do

View file

@ -23,7 +23,7 @@ RSpec.describe Spam::Handler, type: :service do
before do
allow(Settings::RateLimit).to receive(:trigger_spam_for?).and_return(true)
allow(Reaction).to receive(:user_has_been_given_too_many_spammy_article_reactions?)
.with(user: article.user).and_return(false)
.with(user: article.user, include_user_profile: false).and_return(false)
end
it "creates a reaction but does not suspend the user" do
@ -36,7 +36,7 @@ RSpec.describe Spam::Handler, type: :service do
before do
allow(Settings::RateLimit).to receive(:trigger_spam_for?).and_return(true)
allow(Reaction).to receive(:user_has_been_given_too_many_spammy_article_reactions?)
.with(user: article.user).and_return(true)
.with(user: article.user, include_user_profile: false).and_return(true)
end
it "creates a reaction, suspends the user, and creates a note for the user" do
@ -44,6 +44,15 @@ RSpec.describe Spam::Handler, type: :service do
expect(article.user.reload).to be_suspended
expect(Note.where(noteable: article.user, reason: "automatic_suspend").count).to eq(1)
end
it "creates a reaction, notes, suspends, and unpublishes all posts when applicable" do
allow(described_class).to receive(:unpublish_all_posts_when_user_auto_suspended?).and_return(true)
expect(article).to be_published
expect { handler }.to change { Reaction.where(reactable: article, category: "vomit").count }.by(1)
expect(article.user.reload).to be_suspended
expect(article.reload).not_to be_published
expect(Note.where(noteable: article.user, reason: "automatic_suspend").count).to eq(1)
end
end
end
@ -70,7 +79,7 @@ RSpec.describe Spam::Handler, type: :service do
before do
allow(Settings::RateLimit).to receive(:trigger_spam_for?).and_return(true)
allow(Reaction).to receive(:user_has_been_given_too_many_spammy_article_reactions?)
.with(user: comment.user).and_return(false)
.with(user: comment.user, include_user_profile: false).and_return(false)
end
it "creates a reaction but does not suspend the user" do
@ -83,7 +92,7 @@ RSpec.describe Spam::Handler, type: :service do
before do
allow(Settings::RateLimit).to receive(:trigger_spam_for?).and_return(true)
allow(Reaction).to receive(:user_has_been_given_too_many_spammy_comment_reactions?)
.with(user: comment.user).and_return(true)
.with(user: comment.user, include_user_profile: false).and_return(true)
end
it "creates a reaction, suspends the user, and creates a note for the user" do