diff --git a/app/labor/rate_limit_checker.rb b/app/labor/rate_limit_checker.rb index b0458785c..c2da137cd 100644 --- a/app/labor/rate_limit_checker.rb +++ b/app/labor/rate_limit_checker.rb @@ -1,10 +1,6 @@ class RateLimitChecker attr_reader :user, :action - def self.daily_account_follow_limit - SiteConfig.rate_limit_follow_count_daily - end - def initialize(user = nil) @user = user end @@ -13,18 +9,9 @@ class RateLimitChecker class DailyFollowAccountLimitReached < StandardError; end def limit_by_action(action) - result = case action - when "comment_creation" - user.comments.where("created_at > ?", 30.seconds.ago).size > 9 - when "published_article_creation" - user.articles.published.where("created_at > ?", 30.seconds.ago).size > 9 - when "image_upload" - Rails.cache.read("#{user.id}_image_upload").to_i > 9 - when "follow_account" - user_today_follow_count > self.class.daily_account_follow_limit - else - false - end + check_method = "check_#{action}_limit" + result = respond_to?(check_method, true) ? send(check_method) : false + if result @action = action ping_admins @@ -40,19 +27,38 @@ class RateLimitChecker def limit_by_email_recipient_address(address) # This is related to the recipient, not the "user" initiator, like in action. - EmailMessage.where(to: address). - where("sent_at > ?", 2.minutes.ago).size > 5 + EmailMessage.where(to: address).where("sent_at > ?", 2.minutes.ago).size > + SiteConfig.rate_limit_email_recipient + end + + private + + def check_comment_creation_limit + user.comments.where("created_at > ?", 30.seconds.ago).size > + SiteConfig.rate_limit_comment_creation + end + + def check_published_article_creation_limit + user.articles.published.where("created_at > ?", 30.seconds.ago).size > + SiteConfig.rate_limit_published_article_creation + end + + def check_image_upload_limit + Rails.cache.read("#{user.id}_image_upload").to_i > + SiteConfig.rate_limit_image_upload + end + + def check_follow_account_limit + user_today_follow_count > SiteConfig.rate_limit_follow_count_daily end def ping_admins RateLimitCheckerWorker.perform_async(user.id, action) end - private - def user_today_follow_count following_users_count = user.following_users_count - return following_users_count if following_users_count < self.class.daily_account_follow_limit + return following_users_count if following_users_count < SiteConfig.rate_limit_follow_count_daily now = Time.zone.now user.follows.where(created_at: (now.beginning_of_day..now)).size diff --git a/app/models/site_config.rb b/app/models/site_config.rb index 2f2bd182e..9c8110dff 100644 --- a/app/models/site_config.rb +++ b/app/models/site_config.rb @@ -20,6 +20,10 @@ class SiteConfig < RailsSettings::Base # rate limits field :rate_limit_follow_count_daily, type: :integer, default: 500 + field :rate_limit_comment_creation, type: :integer, default: 9 + field :rate_limit_published_article_creation, type: :integer, default: 9 + field :rate_limit_image_upload, type: :integer, default: 9 + field :rate_limit_email_recipient, type: :integer, default: 5 # Google Analytics Reporting API v4 # diff --git a/spec/labor/rate_limit_checker_spec.rb b/spec/labor/rate_limit_checker_spec.rb index 43ea1a79e..aabbb068a 100644 --- a/spec/labor/rate_limit_checker_spec.rb +++ b/spec/labor/rate_limit_checker_spec.rb @@ -4,83 +4,89 @@ RSpec.describe RateLimitChecker, type: :labor do let(:user) { create(:user) } let(:article) { create(:article, user_id: user.id) } - describe "self.daily_account_follow_limit " do - it "returns the value set in SiteConfig.rate_limit_follow_count_daily" do - expect(described_class.daily_account_follow_limit).to eq(SiteConfig.rate_limit_follow_count_daily) - end - end - describe "#limit_by_action" do + let(:rate_limit_checker) { described_class.new(user) } + it "returns false for invalid action" do - expect(described_class.new(user).limit_by_action("random-nothing")).to eq(false) + expect(rate_limit_checker.limit_by_action("random-nothing")).to eq(false) end - it "returns true if too many comments at once" do - create_list(:comment, 10, user_id: user.id, commentable_id: article.id) - expect(described_class.new(user).limit_by_action("comment_creation")).to eq(true) - end + context "when creating comments" do + before do + allow(SiteConfig).to receive(:rate_limit_comment_creation).and_return(1) + end - it "triggers ping admin when too many comments" do - allow(RateLimitCheckerWorker).to receive(:perform_async) - create_list(:comment, 10, user_id: user.id, commentable_id: article.id) - described_class.new(user).limit_by_action("comment_creation") - expect(RateLimitCheckerWorker).to have_received(:perform_async).with(user.id, "comment_creation") - end + it "returns true if too many comments at once" do + create_list(:comment, 2, user_id: user.id, commentable_id: article.id) + expect(rate_limit_checker.limit_by_action("comment_creation")).to eq(true) + end - it "returns false if allowed comment" do - create_list(:comment, 2, user_id: user.id, commentable_id: article.id) - expect(described_class.new(user).limit_by_action("comment_creation")).to eq(false) + it "triggers ping admin when too many comments" do + allow(RateLimitCheckerWorker).to receive(:perform_async) + create_list(:comment, 2, user_id: user.id, commentable_id: article.id) + rate_limit_checker.limit_by_action("comment_creation") + expect(RateLimitCheckerWorker).to have_received(:perform_async).with(user.id, "comment_creation") + end + + it "returns false if allowed comment" do + expect(rate_limit_checker.limit_by_action("comment_creation")).to eq(false) + end end it "returns true if too many published articles at once" do - create_list(:article, 10, user_id: user.id, published: true) - expect(described_class.new(user).limit_by_action("published_article_creation")).to eq(true) + allow(SiteConfig).to receive(:rate_limit_published_article_creation).and_return(1) + create_list(:article, 2, user_id: user.id, published: true) + expect(rate_limit_checker.limit_by_action("published_article_creation")).to eq(true) end it "returns true if a user has followed more than accounts today" do - rate_limit_checker = described_class.new(user) - allow(rate_limit_checker). to receive(:user_today_follow_count). - and_return(described_class.daily_account_follow_limit + 1) + and_return(SiteConfig.rate_limit_follow_count_daily + 1) expect(rate_limit_checker.limit_by_action("follow_account")).to eq(true) end it "returns false if a user's following_users_count is less than " do - rate_limit_checker = described_class.new(user) - allow(user). to receive(:following_users_count). - and_return(described_class.daily_account_follow_limit - 1) + and_return(SiteConfig.rate_limit_follow_count_daily - 1) expect(rate_limit_checker.limit_by_action("follow_account")).to eq(false) end it "returns false if a user has followed less than accounts today" do - rate_limit_checker = described_class.new(user) - allow(rate_limit_checker). to receive(:user_today_follow_count). - and_return(described_class.daily_account_follow_limit - 1) + and_return(SiteConfig.rate_limit_image_upload + 1) expect(rate_limit_checker.limit_by_action("follow_account")).to eq(false) end - it "returns false if published articles comment" do - create_list(:article, 2, user_id: user.id, published: true) + it "returns false if published articles limit has not been reached" do expect(described_class.new(user).limit_by_action("published_article_creation")).to eq(false) end + + it "returns false if a user uploads too many images" do + allow(rate_limit_checker). + to receive(:track_image_uploads). + and_return(SiteConfig.rate_limit_follow_count_daily - 1) + + expect(rate_limit_checker.limit_by_action("image_upload")).to eq(false) + end end describe "#limit_by_email_recipient_address" do - it "returns true if too many published articles at once" do - 10.times { EmailMessage.create(to: user.email, sent_at: Time.current) } + before do + allow(SiteConfig).to receive(:rate_limit_email_recipient).and_return(1) + end + + it "returns true if too many emails are sent to the same recipient" do + 2.times { EmailMessage.create(to: user.email, sent_at: Time.current) } expect(described_class.new.limit_by_email_recipient_address(user.email)).to eq(true) end - it "returns false if published articles comment" do - 2.times { EmailMessage.create(to: user.email, sent_at: Time.current) } + it "returns false if we are below the message limit for this recipient" do expect(described_class.new.limit_by_email_recipient_address(user.email)).to eq(false) end end diff --git a/spec/requests/follows_create_spec.rb b/spec/requests/follows_create_spec.rb index 5d5055dee..3eecab5d2 100644 --- a/spec/requests/follows_create_spec.rb +++ b/spec/requests/follows_create_spec.rb @@ -22,7 +22,7 @@ RSpec.describe "Follows #create", type: :request do allow(rate_limit_checker). to receive(:user_today_follow_count). - and_return(RateLimitChecker.daily_account_follow_limit + 1) + and_return(SiteConfig.rate_limit_follow_count_daily + 1) allow(RateLimitChecker). to receive(:new).