rate limits: adjust limits for various actions.

* Tie rate limits to both the user's ID and their IP address.

* Make each endpoint have separate rate limits. This means that, for
  example, your post edit rate limit is separate from your post vote
  rate limit. Before all write actions had a shared rate limit.

* Make all write endpoints have rate limits. Before some endpoints, such
  as voting, favoriting, commenting, or forum posting, weren't subject
  to rate limits.

* Add stricter rate limits for some endpoints:

** 1 per 5 minutes for creating new accounts.
** 1 per minute for login attempts, changing your email address, or
   for creating mod reports.
** 1 per minute for sending dmails, creating comments, creating forum
   posts, or creating forum topics.
** 1 per second for voting, favoriting, or disapproving posts.
** These rate limits all have burst factors high enough that they
   shouldn't affect normal, non-automated users.

* Raise the default write rate limit for Gold users from 2 per second to
  4 per second, for all other actions not listed above.

* Raise the default burst factor to 200 for all other actions not listed
  above. Before it was 10 for Members, 30 for Gold, and 60 for Platinum.
This commit is contained in:
evazion
2021-03-05 04:50:37 -06:00
parent 4492610dfe
commit 413cd34c45
15 changed files with 55 additions and 49 deletions

View File

@@ -8,7 +8,7 @@ class ApplicationController < ActionController::Base
before_action :reset_current_user
before_action :set_current_user
before_action :normalize_search
before_action :api_check
before_action :check_rate_limit
before_action :ip_ban_check
before_action :set_variant
before_action :add_headers
@@ -69,18 +69,12 @@ class ApplicationController < ActionController::Base
response.headers["X-Git-Hash"] = Rails.application.config.x.git_hash
end
def api_check
return if CurrentUser.is_anonymous? || request.get? || request.head?
rate_limiter = RateLimiter.new(
"write",
[CurrentUser.user.cache_key],
cost: 1,
rate: CurrentUser.user.api_regen_multiplier,
burst: CurrentUser.user.api_burst_limit
)
def check_rate_limit
return if request.get? || request.head?
rate_limiter = RateLimiter.for_action(controller_name, action_name, CurrentUser.user, CurrentUser.ip_addr)
headers["X-Rate-Limit"] = rate_limiter.to_json
rate_limiter.limit!
end

View File

@@ -1,5 +1,4 @@
class CommentVotesController < ApplicationController
skip_before_action :api_check
respond_to :js, :json, :xml, :html
def index

View File

@@ -1,7 +1,6 @@
class CommentsController < ApplicationController
respond_to :html, :xml, :json, :atom
respond_to :js, only: [:new, :destroy, :undelete]
skip_before_action :api_check
def index
params[:group_by] ||= "comment" if params[:search].present?

View File

@@ -1,6 +1,5 @@
class FavoritesController < ApplicationController
respond_to :html, :xml, :json, :js
skip_before_action :api_check
rescue_with Favorite::Error, status: 422
def index

View File

@@ -1,6 +1,5 @@
class ForumPostsController < ApplicationController
respond_to :html, :xml, :json, :js
skip_before_action :api_check
def new
@forum_post = authorize ForumPost.new_reply(params)

View File

@@ -2,7 +2,6 @@ class ForumTopicsController < ApplicationController
respond_to :html, :xml, :json
respond_to :atom, only: [:index, :show]
before_action :normalize_search, :only => :index
skip_before_action :api_check
def new
@forum_topic = authorize ForumTopic.new

View File

@@ -1,7 +1,6 @@
module Moderator
module Post
class PostsController < ApplicationController
skip_before_action :api_check
respond_to :html, :json, :xml, :js
def confirm_move_favorites

View File

@@ -1,5 +1,4 @@
class PostDisapprovalsController < ApplicationController
skip_before_action :api_check
respond_to :js, :html, :json, :xml
def create

View File

@@ -1,5 +1,4 @@
class PostVotesController < ApplicationController
skip_before_action :api_check
respond_to :js, :json, :xml, :html
def index

View File

@@ -1,6 +1,5 @@
class UsersController < ApplicationController
respond_to :html, :xml, :json
skip_before_action :api_check
def new
@user = authorize User.new

View File

@@ -11,6 +11,28 @@ class RateLimiter
@burst = burst
end
def self.for_action(controller_name, action_name, user, ip_addr)
action = "#{controller_name}:#{action_name}"
keys = [(user.cache_key unless user.is_anonymous?), "ip/#{ip_addr.to_s}"].compact
case action
when "users:create"
rate, burst = 1.0/5.minutes, 10
when "emails:update", "sessions:create", "moderation_reports:create"
rate, burst = 1.0/1.minute, 10
when "dmails:create", "comments:create", "forum_posts:create", "forum_topics:create"
rate, burst = 1.0/1.minute, 50
when "comment_votes:create", "comment_votes:destroy", "post_votes:create",
"post_votes:destroy", "favorites:create", "favorites:destroy", "post_disapprovals:create"
rate, burst = 1.0/1.second, 200
else
rate = user.api_regen_multiplier
burst = 200
end
RateLimiter.new(action, keys, rate: rate, burst: burst)
end
def limit!
raise RateLimitError if limited?
end

View File

@@ -464,26 +464,12 @@ class User < ApplicationRecord
# regen this amount per second
def api_regen_multiplier(level)
if level >= User::Levels::PLATINUM
if level >= User::Levels::GOLD
4
elsif level == User::Levels::GOLD
2
else
1
end
end
# can make this many api calls at once before being bound by
# api_regen_multiplier refilling your pool
def api_burst_limit(level)
if level >= User::Levels::PLATINUM
60
elsif level == User::Levels::GOLD
30
else
10
end
end
end
def max_saved_searches
@@ -534,10 +520,6 @@ class User < ApplicationRecord
User.api_regen_multiplier(level)
end
def api_burst_limit
User.api_burst_limit(level)
end
def statement_timeout
User.statement_timeout(level)
end

View File

@@ -61,10 +61,8 @@ class UserPolicy < ApplicationPolicy
updated_at last_logged_in_at last_forum_read_at
comment_threshold default_image_size
favorite_tags blacklisted_tags time_zone per_page
custom_style favorite_count api_regen_multiplier
api_burst_limit statement_timeout
favorite_group_limit favorite_limit tag_query_limit
max_saved_searches theme
custom_style favorite_count statement_timeout favorite_group_limit
favorite_limit tag_query_limit max_saved_searches theme
]
end

View File

@@ -72,6 +72,31 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest
assert_equal(0, @ip_ban.reload.hit_count)
assert_nil(@ip_ban.last_hit_at)
end
should "rate limit logins to 10 per minute per IP" do
freeze_time
11.times do
post session_path, params: { name: @user.name, password: "password" }, headers: { REMOTE_ADDR: "1.2.3.4" }
assert_redirected_to posts_path
assert_equal(@user.id, session[:user_id])
delete_auth session_path, @user
end
post session_path, params: { name: @user.name, password: "password" }, headers: { REMOTE_ADDR: "1.2.3.4" }
assert_response 429
assert_not_equal(@user.id, session[:user_id])
travel 59.seconds
post session_path, params: { name: @user.name, password: "password" }, headers: { REMOTE_ADDR: "1.2.3.4" }
assert_response 429
assert_not_equal(@user.id, session[:user_id])
travel 10.seconds
post session_path, params: { name: @user.name, password: "password" }, headers: { REMOTE_ADDR: "1.2.3.4" }
assert_redirected_to posts_path
assert_equal(@user.id, session[:user_id])
end
end
context "destroy action" do

View File

@@ -48,11 +48,5 @@ class ApiKeyTest < ActiveSupport::TestCase
should "not authenticate with the wrong name" do
assert_equal(false, create(:user).authenticate_api_key(@api_key.key))
end
should "have the same limits whether or not they have an api key" do
assert_no_difference(["@user.reload.api_regen_multiplier", "@user.reload.api_burst_limit"]) do
@api_key.destroy
end
end
end
end