[deploy] Limit articles created by newer users (#10686)
* Limit articles created by newer users * Fix typo in test
This commit is contained in:
parent
98b600da5c
commit
52fd043c33
8 changed files with 61 additions and 5 deletions
|
|
@ -165,6 +165,7 @@ module Admin
|
|||
rate_limit_follow_count_daily
|
||||
rate_limit_image_upload
|
||||
rate_limit_published_article_creation
|
||||
rate_limit_published_article_antispam_creation
|
||||
rate_limit_organization_creation
|
||||
rate_limit_user_subscription_creation
|
||||
rate_limit_article_update
|
||||
|
|
|
|||
|
|
@ -33,7 +33,12 @@ module RateLimitCheckerHelper
|
|||
rate_limit_published_article_creation: {
|
||||
min: 0,
|
||||
placeholder: 9,
|
||||
description: "The number of articles a user can create within 30 seconds"
|
||||
description: "The number of articles that a user can create within 30 seconds"
|
||||
},
|
||||
rate_limit_published_article_antispam_creation: {
|
||||
min: 0,
|
||||
placeholder: 1,
|
||||
description: "The number of articles that a 3-day-old user can create within 5 minutes"
|
||||
},
|
||||
rate_limit_image_upload: {
|
||||
min: 0,
|
||||
|
|
|
|||
|
|
@ -121,6 +121,7 @@ class SiteConfig < RailsSettings::Base
|
|||
field :rate_limit_comment_creation, type: :integer, default: 9
|
||||
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
|
||||
field :rate_limit_organization_creation, type: :integer, default: 1
|
||||
field :rate_limit_reaction_creation, type: :integer, default: 10
|
||||
field :rate_limit_image_upload, type: :integer, default: 9
|
||||
|
|
|
|||
|
|
@ -31,7 +31,13 @@ module Articles
|
|||
attr_reader :user, :article_params, :event_dispatcher
|
||||
|
||||
def rate_limit!
|
||||
user.rate_limiter.check_limit!(:published_article_creation)
|
||||
rate_limit_to_use = if user.created_at > 3.days.ago.beginning_of_day
|
||||
:published_article_antispam_creation
|
||||
else
|
||||
:published_article_creation
|
||||
end
|
||||
|
||||
user.rate_limiter.check_limit!(rate_limit_to_use)
|
||||
end
|
||||
|
||||
def dispatch_event(article)
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ class RateLimitChecker
|
|||
listing_creation: { retry_after: 60 },
|
||||
organization_creation: { retry_after: 300 },
|
||||
published_article_creation: { retry_after: 30 },
|
||||
published_article_antispam_creation: { retry_after: 300 },
|
||||
reaction_creation: { retry_after: 30 },
|
||||
send_email_confirmation: { retry_after: 120 },
|
||||
user_subscription_creation: { retry_after: 30 },
|
||||
|
|
@ -85,10 +86,17 @@ class RateLimitChecker
|
|||
end
|
||||
|
||||
def check_published_article_creation_limit
|
||||
# TODO: Vaidehi Joshi - We should make this time frame configurable.
|
||||
user.articles.published.where("created_at > ?", 30.seconds.ago).size >
|
||||
SiteConfig.rate_limit_published_article_creation
|
||||
end
|
||||
|
||||
def check_published_article_antispam_creation_limit
|
||||
# TODO: Vaidehi Joshi - We should make this time frame configurable.
|
||||
user.articles.published.where("created_at > ?", 5.minutes.ago).size >
|
||||
SiteConfig.rate_limit_published_article_antispam_creation
|
||||
end
|
||||
|
||||
def check_follow_account_limit
|
||||
user_today_follow_count > SiteConfig.rate_limit_follow_count_daily
|
||||
end
|
||||
|
|
|
|||
|
|
@ -468,6 +468,13 @@ RSpec.describe "/admin/config", type: :request do
|
|||
end.to change(SiteConfig, :rate_limit_published_article_creation).from(9).to(3)
|
||||
end
|
||||
|
||||
it "updates rate_limit_published_article_antispam_creation" do
|
||||
expect do
|
||||
post "/admin/config", params: { site_config: { rate_limit_published_article_antispam_creation: 3 },
|
||||
confirmation: confirmation_message }
|
||||
end.to change(SiteConfig, :rate_limit_published_article_antispam_creation).from(1).to(3)
|
||||
end
|
||||
|
||||
it "updates rate_limit_organization_creation" do
|
||||
expect do
|
||||
post "/admin/config", params: { site_config: { rate_limit_organization_creation: 3 },
|
||||
|
|
|
|||
|
|
@ -104,13 +104,29 @@ RSpec.describe "ArticlesCreate", type: :request do
|
|||
end
|
||||
|
||||
context "when creation limit is reached" do
|
||||
it "returns a too_many_requests response if rate limit is reached" do
|
||||
it "returns a too_many_requests response if antispam rate limit is reached" do
|
||||
rate_limit_checker = RateLimitChecker.new(user)
|
||||
allow(RateLimitChecker).to receive(:new).and_return(rate_limit_checker)
|
||||
allow(rate_limit_checker).to receive(:limit_by_action).and_return(true)
|
||||
|
||||
post articles_path, params: { article: { body_markdown: "123" } }
|
||||
|
||||
expect(response).to have_http_status(:too_many_requests)
|
||||
expected_retry_after = RateLimitChecker::ACTION_LIMITERS.dig(:published_article_antispam_creation, :retry_after)
|
||||
expect(response.headers["Retry-After"]).to eq(expected_retry_after)
|
||||
end
|
||||
|
||||
it "returns a too_many_requests response if rate limit is reached" do
|
||||
# Explicitly create this user more than 3.days.ago, since we
|
||||
# check for this in Articles::Creator#rate_limit!
|
||||
user.update!(created_at: 4.days.ago)
|
||||
|
||||
rate_limit_checker = RateLimitChecker.new(user)
|
||||
allow(RateLimitChecker).to receive(:new).and_return(rate_limit_checker)
|
||||
allow(rate_limit_checker).to receive(:limit_by_action).and_return(true)
|
||||
|
||||
post articles_path, params: { article: { body_markdown: "123 i love to spam" } }
|
||||
|
||||
expect(response).to have_http_status(:too_many_requests)
|
||||
expected_retry_after = RateLimitChecker::ACTION_LIMITERS.dig(:published_article_creation, :retry_after)
|
||||
expect(response.headers["Retry-After"]).to eq(expected_retry_after)
|
||||
|
|
|
|||
|
|
@ -26,8 +26,10 @@ RSpec.describe RateLimitChecker, type: :service do
|
|||
expect { limiter.limit_by_action(action) }.to raise_error("Invalid Cache Key: no unique component present")
|
||||
end
|
||||
|
||||
# published_article_creation limit we check against the database rather than our cache
|
||||
described_class::ACTION_LIMITERS.except(:published_article_creation).each do |action, _options|
|
||||
# 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|
|
||||
it "returns true if #{action} limit has been reached" do
|
||||
allow(Rails.cache).to receive(:read).with(
|
||||
cache_key(action), raw: true
|
||||
|
|
@ -60,6 +62,12 @@ RSpec.describe RateLimitChecker, type: :service do
|
|||
end
|
||||
end
|
||||
|
||||
it "returns true if too many published articles at once and potentially spammy" do
|
||||
allow(SiteConfig).to receive(:rate_limit_published_article_antispam_creation).and_return(1)
|
||||
create_list(:article, 2, user_id: user.id, published: true)
|
||||
expect(rate_limit_checker.limit_by_action("published_article_antispam_creation")).to be(true)
|
||||
end
|
||||
|
||||
it "returns true if too many published articles at once" do
|
||||
allow(SiteConfig).to receive(:rate_limit_published_article_creation).and_return(1)
|
||||
create_list(:article, 2, user_id: user.id, published: true)
|
||||
|
|
@ -90,6 +98,10 @@ RSpec.describe RateLimitChecker, type: :service do
|
|||
expect(rate_limit_checker.limit_by_action("follow_account")).to be(false)
|
||||
end
|
||||
|
||||
it "returns false if published articles antispam limit has not been reached" do
|
||||
expect(described_class.new(user).limit_by_action("published_article_antispam_creation")).to be(false)
|
||||
end
|
||||
|
||||
it "returns false if published articles limit has not been reached" do
|
||||
expect(described_class.new(user).limit_by_action("published_article_creation")).to be(false)
|
||||
end
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue