diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 25f1b8bd2..3dc40255f 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -56,7 +56,7 @@ class CommentsController < ApplicationController # POST /comments # POST /comments.json def create - rate_limit!(:comment_creation) + rate_limit!(rate_limit_to_use) @comment = Comment.new(permitted_attributes(Comment)) @comment.user_id = current_user.id @@ -283,4 +283,12 @@ class CommentsController < ApplicationController flash[:error] = "Something went wrong; Comment NOT deleted." redirect_to "#{@comment.path}/mod" end + + def rate_limit_to_use + if current_user.created_at.before?(3.days.ago.beginning_of_day) + :comment_creation + else + :comment_antispam_creation + end + end end diff --git a/app/helpers/rate_limit_checker_helper.rb b/app/helpers/rate_limit_checker_helper.rb index 514fa257c..7edad726a 100644 --- a/app/helpers/rate_limit_checker_helper.rb +++ b/app/helpers/rate_limit_checker_helper.rb @@ -10,7 +10,7 @@ module RateLimitCheckerHelper min: 0, placeholder: 1, title: "Limit number of posts created by a new member", - description: "How many posts can a 3-day-old member create within any 5 minute period?" + description: "How many posts can a new member (3 days or less) create within any 5 minute period?" }, rate_limit_article_update: { min: 1, @@ -54,6 +54,12 @@ module RateLimitCheckerHelper title: "Limit number of comments created", description: "How many comments can someone create within any 30 second period?" }, + rate_limit_comment_antispam_creation: { + min: 0, + placeholder: 1, + title: "Limit number of comments created by a new member", + description: "How many comments can a new member (3 days or less) create within any 5 minute period?" + }, rate_limit_listing_creation: { min: 1, placeholder: 1, diff --git a/app/models/site_config.rb b/app/models/site_config.rb index 0a2a86989..6a167e562 100644 --- a/app/models/site_config.rb +++ b/app/models/site_config.rb @@ -170,6 +170,7 @@ class SiteConfig < RailsSettings::Base # 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_comment_antispam_creation, type: :integer, default: 1 field :rate_limit_listing_creation, type: :integer, default: 1 field :rate_limit_published_article_creation, type: :integer, default: 9 field :rate_limit_published_article_antispam_creation, type: :integer, default: 1 diff --git a/app/services/rate_limit_checker.rb b/app/services/rate_limit_checker.rb index 2c186c35b..187bb124a 100644 --- a/app/services/rate_limit_checker.rb +++ b/app/services/rate_limit_checker.rb @@ -13,7 +13,8 @@ class RateLimitChecker reaction_creation: { retry_after: 30 }, send_email_confirmation: { retry_after: 120 }, user_subscription_creation: { retry_after: 30 }, - user_update: { retry_after: 30 } + user_update: { retry_after: 30 }, + comment_antispam_creation: { retry_after: 300 } }.with_indifferent_access.freeze def initialize(user = nil) @@ -97,6 +98,12 @@ class RateLimitChecker SiteConfig.rate_limit_published_article_antispam_creation end + def check_comment_antispam_creation_limit + # TODO: We should make this time frame configurable. + user.comments.where(created_at: 5.minutes.ago...).size > + SiteConfig.rate_limit_comment_antispam_creation + end + def check_follow_account_limit user_today_follow_count > SiteConfig.rate_limit_follow_count_daily end diff --git a/spec/requests/comments_create_spec.rb b/spec/requests/comments_create_spec.rb index 9503366d9..e86527902 100644 --- a/spec/requests/comments_create_spec.rb +++ b/spec/requests/comments_create_spec.rb @@ -33,15 +33,32 @@ RSpec.describe "CommentsCreate", type: :request do expect(NotificationSubscription.last.notifiable).to eq(Comment.last) end - it "returns 429 Too Many Requests when a user reaches their rate limit" do - allow(RateLimitChecker).to receive(:new).and_return(rate_limit_checker) - allow(rate_limit_checker).to receive(:limit_by_action) - .with(:comment_creation) - .and_return(true) + context "when users hit their rate limits" do + before do + allow(RateLimitChecker).to receive(:new).and_return(rate_limit_checker) + end - post comments_path, params: comment_params + it "returns 429 Too Many Requests when a user reaches their rate limit" do + # avoid hitting new user rate limit check + allow(user).to receive(:created_at).and_return(1.week.ago) + allow(rate_limit_checker).to receive(:limit_by_action) + .with(:comment_creation) + .and_return(true) - expect(response).to have_http_status(:too_many_requests) + post comments_path, params: comment_params + + expect(response).to have_http_status(:too_many_requests) + end + + it "returns 429 Too Many Requests when a new user reaches their rate limit" do + allow(rate_limit_checker).to receive(:limit_by_action) + .with(:comment_antispam_creation) + .and_return(true) + + post comments_path, params: comment_params + + expect(response).to have_http_status(:too_many_requests) + end end context "when user is posting on an author that blocks user" do diff --git a/spec/services/rate_limit_checker_spec.rb b/spec/services/rate_limit_checker_spec.rb index eefd03b5d..da14fba9c 100644 --- a/spec/services/rate_limit_checker_spec.rb +++ b/spec/services/rate_limit_checker_spec.rb @@ -26,10 +26,11 @@ RSpec.describe RateLimitChecker, type: :service do expect { limiter.limit_by_action(action) }.to raise_error("Invalid Cache Key: no unique component present") end - # We check published_article_creation + :published_article_antispam_creation - # limit against database, rather than our cache. - described_class::ACTION_LIMITERS.except(:published_article_creation, - :published_article_antispam_creation).each do |action, _options| + # We check the excepted limits against the database, rather than our cache. + described_class::ACTION_LIMITERS + .except(:published_article_creation, + :published_article_antispam_creation, + :comment_antispam_creation).each do |action, _options| it "returns true if #{action} limit has been reached" do allow(Rails.cache).to receive(:read).with( cache_key(action), raw: true diff --git a/spec/system/comments/user_fills_out_comment_spec.rb b/spec/system/comments/user_fills_out_comment_spec.rb index 8f665d361..3c99773e2 100644 --- a/spec/system/comments/user_fills_out_comment_spec.rb +++ b/spec/system/comments/user_fills_out_comment_spec.rb @@ -29,6 +29,8 @@ RSpec.describe "Creating Comment", type: :system, js: true do let(:rate_limit_checker) { RateLimitChecker.new(user) } before do + # avoid hitting new user rate limit check + allow(user).to receive(:created_at).and_return(1.week.ago) allow(RateLimitChecker).to receive(:new).and_return(rate_limit_checker) allow(rate_limit_checker).to receive(:limit_by_action) .with(:comment_creation)