Add rate limit for new user comments (#12925)

* Add rate limit for new user comments

* Rename limit, add check to controller

* Add spec

* Fix specs
This commit is contained in:
Michael Kohl 2021-03-16 01:39:31 +00:00 committed by GitHub
parent cf3bde4259
commit 2513de92fb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 56 additions and 14 deletions

View file

@ -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

View file

@ -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,

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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)