diff --git a/app/models/article.rb b/app/models/article.rb index 42f81cece..2ac777f0e 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -809,27 +809,7 @@ class Article < ApplicationRecord end def create_conditional_autovomits - return unless Settings::RateLimit.spam_trigger_terms.any? do |term| - Regexp.new(term.downcase).match?(title.downcase) - end - - Reaction.create( - user_id: Settings::General.mascot_user_id, - reactable_id: id, - reactable_type: "Article", - category: "vomit", - ) - - return unless Reaction.article_vomits.where(reactable_id: user.articles.pluck(:id)).size > 2 - - user.add_role(:suspended) - Note.create( - author_id: Settings::General.mascot_user_id, - noteable_id: user_id, - noteable_type: "User", - reason: "automatic_suspend", - content: "User suspended for too many spammy articles, triggered by autovomit.", - ) + Spam::ArticleHandler.handle!(article: self) end def async_bust diff --git a/app/models/reaction.rb b/app/models/reaction.rb index ad0fbd0eb..d4c15f92f 100644 --- a/app/models/reaction.rb +++ b/app/models/reaction.rb @@ -72,6 +72,14 @@ class Reaction < ApplicationRecord Reaction.where(reactable_id: reactable.id, reactable_type: class_name, user: user, category: category).any? end end + + # @param user [User] the user who might be spamming the system + # + # @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_reactions?(user:) + article_vomits.where(reactable_type: "Article", reactable_id: user.articles.pluck(:id)).size > 2 + end end # no need to send notification if: diff --git a/app/models/settings/rate_limit.rb b/app/models/settings/rate_limit.rb index 4f74e5431..b33e84dda 100644 --- a/app/models/settings/rate_limit.rb +++ b/app/models/settings/rate_limit.rb @@ -21,5 +21,18 @@ module Settings setting :user_considered_new_days, type: :integer, default: 3 setting :user_subscription_creation, type: :integer, default: 3 setting :user_update, type: :integer, default: 15 + + # A helper function to determine if text is spammy. + # + # @param text [String] text to check for "spamminess" + # + # @return [TrueClass] if this is spammy + # @return [FalseClass] if this isn't spammy + def self.trigger_spam_for?(text:) + return false if spam_trigger_terms.empty? + + regexp = Regexp.new("(#{spam_trigger_terms.join('|')})", true) + regexp.match?(text) + end end end diff --git a/app/services/spam/article_handler.rb b/app/services/spam/article_handler.rb new file mode 100644 index 000000000..6e46ed2f6 --- /dev/null +++ b/app/services/spam/article_handler.rb @@ -0,0 +1,42 @@ +module Spam + module ArticleHandler + # Test the article for spamminess. If it's not spammy, don't do anything. + # + # If it is spammy, escalate the situation! + # + # @param article [Article] the article to check for spamminess + # @param attributes [Array] test these attributes of the article. + # + # @note We are not blocking the spammer from submitting the + # article with spam content, but we are instead performing some + # administrative tasks to indicate that we're taking notice and + # even escalating. + def self.handle!(article:, attributes: %i[title body_markdown]) + return :not_spam if attributes.none? do |attr| + Settings::RateLimit.trigger_spam_for?(text: article.public_send(attr)) + end + + Reaction.create!( + user_id: Settings::General.mascot_user_id, + reactable_id: article.id, + reactable_type: "Article", + category: "vomit", + ) + + return unless Reaction.user_has_been_given_too_many_spammy_reactions?(user: article.user) + + # TODO: What should we do with their active session? I think we + # might want to explore later raising an exception that the + # controller rescues and logs the user out. Because, as written + # they might still be logged in but have limited abilities. + article.user.add_role(:suspended) + + Note.create( + author_id: Settings::General.mascot_user_id, + noteable: article.user, + reason: "automatic_suspend", + content: "User suspended for too many spammy articles, triggered by autovomit.", + ) + end + end +end diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index b6c1dfeee..ef538fd8b 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -1033,56 +1033,10 @@ RSpec.describe Article, type: :model do end describe "spam" do - before do - allow(Settings::General).to receive(:mascot_user_id).and_return(user.id) - allow(Settings::RateLimit).to receive(:spam_trigger_terms).and_return( - ["yahoomagoo gogo", "testtestetest", "magoo.+magee"], - ) - end - - it "creates vomit reaction if possible spam" do - article.body_markdown = article.body_markdown.gsub(article.title, "This post is about Yahoomagoo gogo") + it "delegates spam handling to Spam::ArticleHandler" do + allow(Spam::ArticleHandler).to receive(:handle!).with(article: article).and_call_original article.save - expect(Reaction.last.category).to eq("vomit") - expect(Reaction.last.user_id).to eq(user.id) - end - - it "creates vomit reaction if possible spam based on pattern" do - article.body_markdown = article.body_markdown.gsub(article.title, "This post is about magoo to the magee") - article.save - expect(Reaction.last.category).to eq("vomit") - expect(Reaction.last.user_id).to eq(user.id) - end - - it "does not suspend user if only single vomit" do - article.body_markdown = article.body_markdown.gsub(article.title, "This post is about Yahoomagoo gogo") - article.save - expect(article.user.suspended?).to be false - end - - it "suspends user with 3 comment vomits" do - second_article = create(:article, user: article.user) - third_article = create(:article, user: article.user) - article.body_markdown = article.body_markdown.gsub(article.title, "This post is about Yahoomagoo gogo") - second_article.body_markdown = second_article.body_markdown.gsub(second_article.title, "testtestetest") - third_article.body_markdown = third_article.body_markdown.gsub(third_article.title, "yahoomagoo gogo") - - article.save - second_article.save - third_article.save - expect(article.user.suspended?).to be true - expect(Note.last.reason).to eq "automatic_suspend" - end - - it "does not create vomit reaction if does not have matching title" do - article.save - expect(Reaction.last).to be nil - end - - it "does not create vomit reaction if does not have pattern match" do - article.body_markdown = article.body_markdown.gsub(article.title, "This post is about magoo to") - article.save - expect(Reaction.last).to be nil + expect(Spam::ArticleHandler).to have_received(:handle!).with(article: article) end end diff --git a/spec/models/reaction_spec.rb b/spec/models/reaction_spec.rb index 614b7c74c..c72810a23 100644 --- a/spec/models/reaction_spec.rb +++ b/spec/models/reaction_spec.rb @@ -13,6 +13,12 @@ RSpec.describe Reaction, type: :model do it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(%i[reactable_id reactable_type category]) } end + describe ".user_has_been_given_too_many_spammy_reactions?" do + it "performs a valid query for the user" do + expect { described_class.user_has_been_given_too_many_spammy_reactions?(user: user) }.not_to raise_error + end + end + describe "counter_culture" do context "when a reaction is created" do it "increments reaction count on user" do diff --git a/spec/models/settings/rate_limit_spec.rb b/spec/models/settings/rate_limit_spec.rb new file mode 100644 index 000000000..a4511d9b3 --- /dev/null +++ b/spec/models/settings/rate_limit_spec.rb @@ -0,0 +1,33 @@ +require "rails_helper" + +RSpec.describe Settings::RateLimit, type: :model do + describe ".trigger_spam_for?" do + subject { described_class.trigger_spam_for?(text: text) } + + let(:text) { "text to test" } + + context "when it matches a spam trigger term" do + before do + allow(described_class).to receive(:spam_trigger_terms).and_return(["to test"]) + end + + it { is_expected.to be_truthy } + end + + context "when there are no spam trigger terms" do + before do + allow(described_class).to receive(:spam_trigger_terms).and_return([]) + end + + it { is_expected.to be_falsey } + end + + context "when there are spam trigger terms but they don't match" do + before do + allow(described_class).to receive(:spam_trigger_terms).and_return(["in a hole in the ground"]) + end + + it { is_expected.to be_falsey } + end + end +end diff --git a/spec/services/spam/article_handler_spec.rb b/spec/services/spam/article_handler_spec.rb new file mode 100644 index 000000000..c11e85462 --- /dev/null +++ b/spec/services/spam/article_handler_spec.rb @@ -0,0 +1,52 @@ +require "rails_helper" + +RSpec.describe Spam::ArticleHandler, type: :service do + describe ".handle!" do + subject(:handler) { described_class.handle!(article: article) } + + let!(:article) { create(:article) } + let(:mascot_user) { create(:user) } + + before do + allow(Settings::General).to receive(:mascot_user_id).and_return(mascot_user.id) + end + + context "when non-spammy content" do + before do + allow(Settings::RateLimit).to receive(:trigger_spam_for?).with(text: article.title).and_return(false) + allow(Settings::RateLimit).to receive(:trigger_spam_for?).with(text: article.body_markdown).and_return(false) + end + + it { is_expected.to eq(:not_spam) } + end + + context "when first time spammy content" do + before do + allow(Settings::RateLimit).to receive(:trigger_spam_for?).with(text: article.title).and_return(true) + allow(Settings::RateLimit).to receive(:trigger_spam_for?).with(text: article.body_markdown).and_return(false) + allow(Reaction).to receive(:user_has_been_given_too_many_spammy_reactions?) + .with(user: article.user).and_return(false) + end + + it "creates a reaction but does not suspend the user" do + expect { handler }.to change { Reaction.where(reactable: article, category: "vomit").count }.by(1) + expect(article.user.reload).not_to be_suspended + end + end + + context "when multiple offender of spammy" do + before do + allow(Settings::RateLimit).to receive(:trigger_spam_for?).with(text: article.title).and_return(true) + allow(Settings::RateLimit).to receive(:trigger_spam_for?).with(text: article.body_markdown).and_return(false) + allow(Reaction).to receive(:user_has_been_given_too_many_spammy_reactions?) + .with(user: article.user).and_return(true) + end + + it "creates a reaction, suspends the user, and creates a note for the user" do + expect { handler }.to change { Reaction.where(reactable: article, category: "vomit").count }.by(1) + expect(article.user.reload).to be_suspended + expect(Note.where(noteable: article.user, reason: "automatic_suspend").count).to eq(1) + end + end + end +end