From eefbe25bd3eeef44a57f5743b727d050247fe0fa Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Tue, 6 Oct 2020 10:10:30 -0400 Subject: [PATCH] [deploy] Add ability for admin to add anti-spam terms (#10615) * Add ability for admin to add anti-spam terms * Fix specs * Fix tests * Fix typo * Fix test --- app/controllers/admin/configs_controller.rb | 1 + app/lib/constants/site_config.rb | 4 +++ app/models/article.rb | 12 +++++++++ app/models/comment.rb | 15 +++++++++++ app/models/reaction.rb | 2 +- app/models/site_config.rb | 4 ++- .../notifications/new_comment/send.rb | 2 ++ app/views/admin/configs/show.html.erb | 10 ++++++- .../feedback_messages/_abuse_reports.html.erb | 3 +++ spec/models/article_spec.rb | 19 ++++++++++++++ spec/models/comment_spec.rb | 26 +++++++++++++++++++ spec/requests/admin/configs_spec.rb | 9 ++++++- .../notifications/new_comment/send_spec.rb | 7 +++++ 13 files changed, 110 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/configs_controller.rb b/app/controllers/admin/configs_controller.rb index 4308a1fed..44f58e9f6 100644 --- a/app/controllers/admin/configs_controller.rb +++ b/app/controllers/admin/configs_controller.rb @@ -56,6 +56,7 @@ module Admin facebook_secret allow_email_password_registration primary_brand_color_hex + spam_trigger_terms ] allowed_params = allowed_params | diff --git a/app/lib/constants/site_config.rb b/app/lib/constants/site_config.rb index f064e3f56..8295ee485 100644 --- a/app/lib/constants/site_config.rb +++ b/app/lib/constants/site_config.rb @@ -222,6 +222,10 @@ module Constants description: "Used as the secondary logo", placeholder: "https://image.url" }, + spam_trigger_terms: { + description: "Individual (case insensitive) phrases that trigger spam alerts, comma separated.", + placeholder: "used cares near you, pokemon go hack" + }, shop_url: { description: "Used as the shop url of the community", placeholder: "https://shop.url" diff --git a/app/models/article.rb b/app/models/article.rb index 14b479c63..546a0b6bd 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -90,6 +90,7 @@ class Article < ApplicationRecord before_save :calculate_base_scores before_save :fetch_video_duration before_save :set_caches + before_save :create_conditional_autovomits before_create :create_password before_destroy :before_destroy_actions, prepend: true @@ -677,6 +678,17 @@ class Article < ApplicationRecord self.spaminess_rating = 0 if new_record? end + def create_conditional_autovomits + return unless SiteConfig.spam_trigger_terms.any? { |term| title.downcase.include?(term.downcase) } + + Reaction.create( + user_id: SiteConfig.mascot_user_id, + reactable_id: id, + reactable_type: "Article", + category: "vomit", + ) + end + def async_bust Articles::BustCacheWorker.perform_async(id) end diff --git a/app/models/comment.rb b/app/models/comment.rb index e181fbde9..3e3acab78 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -25,6 +25,7 @@ class Comment < ApplicationRecord before_validation :evaluate_markdown, if: -> { body_markdown } before_save :set_markdown_character_count, if: :body_markdown + before_save :create_conditional_autovomits before_create :adjust_comment_parent_based_on_depth after_create :after_create_checks after_create :notify_slack_channel_about_warned_users @@ -244,6 +245,20 @@ class Comment < ApplicationRecord Comments::SendEmailNotificationWorker.perform_async(id) end + def create_conditional_autovomits + return unless + SiteConfig.spam_trigger_terms.any? { |term| body_markdown.downcase.include?(term.downcase) } && + user.registered_at > 5.days.ago + + self.score = -1 + Reaction.create( + user_id: SiteConfig.mascot_user_id, + reactable_id: id, + reactable_type: "Comment", + category: "vomit", + ) + end + def should_send_email_notification? parent_exists? && parent_user.class.name != "Podcast" && diff --git a/app/models/reaction.rb b/app/models/reaction.rb index ed08db93e..028cd27a5 100644 --- a/app/models/reaction.rb +++ b/app/models/reaction.rb @@ -173,7 +173,7 @@ class Reaction < ApplicationRecord end def negative_reaction_from_untrusted_user? - return if user&.any_admin? + return if user&.any_admin? || user&.id == SiteConfig.mascot_user_id negative? && !user.trusted end diff --git a/app/models/site_config.rb b/app/models/site_config.rb index a350a9b1e..b00760238 100644 --- a/app/models/site_config.rb +++ b/app/models/site_config.rb @@ -117,7 +117,7 @@ class SiteConfig < RailsSettings::Base field :suggested_tags, type: :array, default: %w[] field :suggested_users, type: :array, default: %w[] - # Rate limits + # Rate limits and spam prevention field :rate_limit_follow_count_daily, type: :integer, default: 500 field :rate_limit_comment_creation, type: :integer, default: 9 field :rate_limit_listing_creation, type: :integer, default: 1 @@ -132,6 +132,8 @@ class SiteConfig < RailsSettings::Base field :rate_limit_user_update, type: :integer, default: 5 field :rate_limit_user_subscription_creation, type: :integer, default: 3 + field :spam_trigger_terms, type: :array, default: [] + # Social Media field :social_media_handles, type: :hash, default: { twitter: nil, diff --git a/app/services/notifications/new_comment/send.rb b/app/services/notifications/new_comment/send.rb index 256b1b70c..04c253311 100644 --- a/app/services/notifications/new_comment/send.rb +++ b/app/services/notifications/new_comment/send.rb @@ -14,6 +14,8 @@ module Notifications end def call + return if comment.score.negative? + user_ids = Set.new(comment_user_ids + subscribed_user_ids + top_level_user_ids + author_subscriber_user_ids) json_data = { diff --git a/app/views/admin/configs/show.html.erb b/app/views/admin/configs/show.html.erb index 68857a23b..9e184400a 100644 --- a/app/views/admin/configs/show.html.erb +++ b/app/views/admin/configs/show.html.erb @@ -834,7 +834,7 @@
<%= render partial: "card_header", locals: { - header: "Rate limits", + header: "Rate limits and anti-spam", state: "collapse", target: "rateLimitsBodyContainer", expanded: "false" @@ -851,6 +851,14 @@
<%= field_hash[:description] %>
<% end %> +
+ <%= admin_config_label :spam_trigger_terms %> + <%= f.text_field :spam_trigger_terms, + class: "form-control", + value: SiteConfig.spam_trigger_terms.to_s, + placeholder: Constants::SiteConfig::DETAILS[:spam_trigger_terms][:placeholder] %> +
<%= Constants::SiteConfig::DETAILS[:spam_trigger_terms][:description] %>
+
diff --git a/app/views/admin/feedback_messages/_abuse_reports.html.erb b/app/views/admin/feedback_messages/_abuse_reports.html.erb index 5988c223b..1758c184a 100644 --- a/app/views/admin/feedback_messages/_abuse_reports.html.erb +++ b/app/views/admin/feedback_messages/_abuse_reports.html.erb @@ -26,6 +26,9 @@
🤢 @<%= reaction.user.username %> + <% if reaction.user_id == SiteConfig.mascot_user_id %> + (auto-generated) + <% end %> <%= reaction.reactable_type %>: diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 15eb025c5..8a649e212 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -776,6 +776,25 @@ RSpec.describe Article, type: :model do end end + describe "spam" do + before do + allow(SiteConfig).to receive(:mascot_user_id).and_return(user.id) + allow(SiteConfig).to receive(:spam_trigger_terms).and_return(["yahoomagoo gogo", "testtestetest"]) + end + + it "creates vomit reaction if possible spam" do + article.body_markdown = article.body_markdown.gsub(article.title, "This post is about Yahoomagoo gogo") + article.save + expect(Reaction.last.category).to eq("vomit") + expect(Reaction.last.user_id).to eq(user.id) + end + + it "does not create vomit reaction if does not have matching title" do + article.save + expect(Reaction.last).to be nil + end + end + describe "async score calc" do it "enqueues Articles::ScoreCalcWorker if published" do sidekiq_assert_enqueued_with(job: Articles::ScoreCalcWorker, args: [article.id]) do diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 76f790798..44ef6b794 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -397,6 +397,32 @@ RSpec.describe Comment, type: :model do end end + describe "spam" do + before do + allow(SiteConfig).to receive(:mascot_user_id).and_return(user.id) + allow(SiteConfig).to receive(:spam_trigger_terms).and_return(["yahoomagoo gogo", "anothertestterm"]) + end + + it "creates vomit reaction if possible spam" do + comment.body_markdown = "This post is about Yahoomagoo gogo" + comment.save + expect(Reaction.last.category).to eq("vomit") + expect(Reaction.last.user_id).to eq(user.id) + end + + it "does not create vomit reaction if user is established in this context" do + user.update_column(:registered_at, 10.days.ago) + comment.body_markdown = "This post is about Yahoomagoo gogo" + comment.save + expect(Reaction.last).to be nil + end + + it "does not create vomit reaction if does not have matching title" do + comment.save + expect(Reaction.last).to be nil + end + end + context "when callbacks are triggered before save" do it "generates character count before saving" do comment.save diff --git a/spec/requests/admin/configs_spec.rb b/spec/requests/admin/configs_spec.rb index 60650400d..3180b2292 100644 --- a/spec/requests/admin/configs_spec.rb +++ b/spec/requests/admin/configs_spec.rb @@ -453,7 +453,7 @@ RSpec.describe "/admin/config", type: :request do end end - describe "Rate Limits" do + describe "Rate Limits and spam" do it "updates rate_limit_follow_count_daily" do expect do post "/admin/config", params: { site_config: { rate_limit_follow_count_daily: 3 }, @@ -544,6 +544,13 @@ RSpec.describe "/admin/config", type: :request do confirmation: confirmation_message } end.to change(SiteConfig, :rate_limit_send_email_confirmation).from(2).to(3) end + + it "updates spam_trigger_terms" do + spam_trigger_terms = "hey, pokemon go hack" + post "/admin/config", params: { site_config: { spam_trigger_terms: spam_trigger_terms }, + confirmation: confirmation_message } + expect(SiteConfig.spam_trigger_terms).to eq(["hey", "pokemon go hack"]) + end end describe "Social Media" do diff --git a/spec/services/notifications/new_comment/send_spec.rb b/spec/services/notifications/new_comment/send_spec.rb index 5c5d9191b..f16b36a77 100644 --- a/spec/services/notifications/new_comment/send_spec.rb +++ b/spec/services/notifications/new_comment/send_spec.rb @@ -27,6 +27,13 @@ RSpec.describe Notifications::NewComment::Send, type: :service do expect(notification.json_data["user"]["username"]).to eq(child_comment.user.username) end + it "does not send if comment has negative score already" do + prior_notification_size = Notification.all.size + child_comment.update_column(:score, -1) + described_class.call(child_comment) + expect(Notification.all.size).to eq prior_notification_size + end + it "creates the correct comment data for the notification" do described_class.call(child_comment)