From 2e9f4dc2f445281022a0e5bff219c59ff5040f3a Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 9 Dec 2021 21:37:09 -0600 Subject: [PATCH] controllers: refactor rate limits. Refactor controllers so that endpoint rate limits are declared locally, with the endpoint, instead of globally, in a single method in ApplicationController. This way an endpoint's rate limit is declared in the same file as the endpoint itself. This is so we can add fine-grained rate limits for certain GET requests. Before rate limits were only for non-GET requests. --- app/controllers/application_controller.rb | 33 +++++++++++++++-- app/controllers/comment_votes_controller.rb | 3 ++ app/controllers/comments_controller.rb | 2 + app/controllers/dmails_controller.rb | 2 + app/controllers/emails_controller.rb | 2 + app/controllers/favorites_controller.rb | 3 ++ app/controllers/forum_posts_controller.rb | 2 + app/controllers/forum_topics_controller.rb | 2 + .../moderation_reports_controller.rb | 2 + .../post_disapprovals_controller.rb | 2 + app/controllers/post_votes_controller.rb | 3 ++ app/controllers/sessions_controller.rb | 2 + app/controllers/users_controller.rb | 2 + app/logical/rate_limiter.rb | 37 ++++++------------- 14 files changed, 67 insertions(+), 30 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 22273a652..2dc7d87af 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -8,7 +8,7 @@ class ApplicationController < ActionController::Base before_action :reset_current_user before_action :set_current_user before_action :normalize_search - before_action :check_rate_limit + before_action :check_default_rate_limit before_action :ip_ban_check before_action :set_variant before_action :add_headers @@ -25,6 +25,23 @@ class ApplicationController < ActionController::Base end end + # Define a rate limit for the given controller action. + # + # @example + # rate_limit :index, 1.0/1.minute, 50, if: -> { request.format.atom? } + def self.rate_limit(action, rate:, burst:, key: "#{controller_name}:#{action}", if: nil) + if_proc = binding.local_variable_get(:if) + + before_action(only: action, if: if_proc) do + key = "#{controller_name}:#{action}" + rate_limiter = RateLimiter.build(action: key, rate: rate, burst: burst, user: CurrentUser.user, ip_addr: CurrentUser.ip_addr) + headers["X-Rate-Limit"] = rate_limiter.to_json + rate_limiter.limit! + end + + skip_before_action :check_default_rate_limit, only: action, if: if_proc + end + private def respond_with(subject, *args, model: model_name, **options, &block) @@ -69,12 +86,20 @@ class ApplicationController < ActionController::Base response.headers["X-Git-Hash"] = Rails.application.config.x.git_hash end - def check_rate_limit + # Apply the default rate limit to all update actions (POST, PUT, or DELETE), unless the + # endpoint already declared a more specific rate limit using the `rate_limit` macro above. + def check_default_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 = RateLimiter.build( + action: "#{controller_name}:#{action_name}", + rate: CurrentUser.user.api_regen_multiplier, + burst: 200, + user: CurrentUser.user, + ip_addr: CurrentUser.ip_addr, + ) + headers["X-Rate-Limit"] = rate_limiter.to_json rate_limiter.limit! end diff --git a/app/controllers/comment_votes_controller.rb b/app/controllers/comment_votes_controller.rb index 02881d920..e45620257 100644 --- a/app/controllers/comment_votes_controller.rb +++ b/app/controllers/comment_votes_controller.rb @@ -1,6 +1,9 @@ class CommentVotesController < ApplicationController respond_to :js, :json, :xml, :html + rate_limit :create, rate: 1.0/1.second, burst: 200 + rate_limit :destroy, rate: 1.0/1.second, burst: 200 + def index @comment_votes = authorize CommentVote.visible(CurrentUser.user).paginated_search(params, count_pages: true) @comment_votes = @comment_votes.includes(:user, comment: [:creator, { post: [:uploader, :media_asset] }]) if request.format.html? diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 2001b9bf8..e03440c3b 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -2,6 +2,8 @@ class CommentsController < ApplicationController respond_to :html, :xml, :json, :atom respond_to :js, only: [:new, :update, :destroy, :undelete] + rate_limit :create, rate: 1.0/1.minute, burst: 50 + def index params[:group_by] ||= "comment" if params[:search].present? diff --git a/app/controllers/dmails_controller.rb b/app/controllers/dmails_controller.rb index 73499707b..8df9ecef1 100644 --- a/app/controllers/dmails_controller.rb +++ b/app/controllers/dmails_controller.rb @@ -1,6 +1,8 @@ class DmailsController < ApplicationController respond_to :html, :xml, :js, :json + rate_limit :create, rate: 1.0/1.minute, burst: 50 + def new if params[:respond_to_id] parent = authorize Dmail.find(params[:respond_to_id]), :show? diff --git a/app/controllers/emails_controller.rb b/app/controllers/emails_controller.rb index 429392048..d22a8c050 100644 --- a/app/controllers/emails_controller.rb +++ b/app/controllers/emails_controller.rb @@ -2,6 +2,8 @@ class EmailsController < ApplicationController before_action :requires_reauthentication, only: [:edit, :update] respond_to :html, :xml, :json + rate_limit :update, rate: 1.0/1.minute, burst: 10 + def index @email_addresses = authorize EmailAddress.visible(CurrentUser.user).paginated_search(params, count_pages: true) @email_addresses = @email_addresses.includes(:user) diff --git a/app/controllers/favorites_controller.rb b/app/controllers/favorites_controller.rb index 40378df79..486791eb5 100644 --- a/app/controllers/favorites_controller.rb +++ b/app/controllers/favorites_controller.rb @@ -1,6 +1,9 @@ class FavoritesController < ApplicationController respond_to :js, :json, :html, :xml + rate_limit :create, rate: 1.0/1.second, burst: 200 + rate_limit :destroy, rate: 1.0/1.second, burst: 200 + def index post_id = params[:post_id] || params[:search][:post_id] user_id = params[:user_id] || params[:search][:user_id] diff --git a/app/controllers/forum_posts_controller.rb b/app/controllers/forum_posts_controller.rb index 0a239591c..b1ecf47e9 100644 --- a/app/controllers/forum_posts_controller.rb +++ b/app/controllers/forum_posts_controller.rb @@ -1,6 +1,8 @@ class ForumPostsController < ApplicationController respond_to :html, :xml, :json, :js + rate_limit :create, rate: 1.0/1.minute, burst: 50 + def new @forum_post = authorize ForumPost.new_reply(params) respond_with(@forum_post) diff --git a/app/controllers/forum_topics_controller.rb b/app/controllers/forum_topics_controller.rb index ef127a025..1582f83b1 100644 --- a/app/controllers/forum_topics_controller.rb +++ b/app/controllers/forum_topics_controller.rb @@ -3,6 +3,8 @@ class ForumTopicsController < ApplicationController respond_to :atom, only: [:index, :show] before_action :normalize_search, :only => :index + rate_limit :create, rate: 1.0/1.minute, burst: 50 + def new @forum_topic = authorize ForumTopic.new @forum_topic.original_post = ForumPost.new diff --git a/app/controllers/moderation_reports_controller.rb b/app/controllers/moderation_reports_controller.rb index a36f3e13c..95d395d1c 100644 --- a/app/controllers/moderation_reports_controller.rb +++ b/app/controllers/moderation_reports_controller.rb @@ -1,6 +1,8 @@ class ModerationReportsController < ApplicationController respond_to :html, :xml, :json, :js + rate_limit :create, rate: 1.0/1.minute, burst: 10 + def new @moderation_report = authorize ModerationReport.new(permitted_attributes(ModerationReport)) respond_with(@moderation_report) diff --git a/app/controllers/post_disapprovals_controller.rb b/app/controllers/post_disapprovals_controller.rb index e27853ee9..e470dbccf 100644 --- a/app/controllers/post_disapprovals_controller.rb +++ b/app/controllers/post_disapprovals_controller.rb @@ -1,6 +1,8 @@ class PostDisapprovalsController < ApplicationController respond_to :js, :html, :json, :xml + rate_limit :destroy, rate: 1.0/1.second, burst: 200 + def create @post_disapproval = authorize PostDisapproval.new(user: CurrentUser.user, **permitted_attributes(PostDisapproval)) @post_disapproval.save diff --git a/app/controllers/post_votes_controller.rb b/app/controllers/post_votes_controller.rb index a1cd76f1d..0ef9f8f84 100644 --- a/app/controllers/post_votes_controller.rb +++ b/app/controllers/post_votes_controller.rb @@ -1,6 +1,9 @@ class PostVotesController < ApplicationController respond_to :js, :json, :xml, :html + rate_limit :create, rate: 1.0/1.second, burst: 200 + rate_limit :destroy, rate: 1.0/1.second, burst: 200 + def index @post_votes = authorize PostVote.visible(CurrentUser.user).paginated_search(params) @post_votes = @post_votes.includes(:user, post: [:uploader, :media_asset]) if request.format.html? diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 09b008457..2481ff538 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -2,6 +2,8 @@ class SessionsController < ApplicationController respond_to :html, :json skip_forgery_protection only: :create, if: -> { !request.format.html? } + rate_limit :create, rate: 1.0/1.minute, burst: 10 + def new @user = User.new end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index ca95cdb4e..1d5ba9b47 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,6 +1,8 @@ class UsersController < ApplicationController respond_to :html, :xml, :json + rate_limit :create, rate: 1.0/5.minutes, burst: 10 + def new @user = authorize User.new @user.email_address = EmailAddress.new diff --git a/app/logical/rate_limiter.rb b/app/logical/rate_limiter.rb index 82e5636a2..bde280be3 100644 --- a/app/logical/rate_limiter.rb +++ b/app/logical/rate_limiter.rb @@ -21,34 +21,19 @@ class RateLimiter @burst = burst end - # Create a RateLimiter object for the current controller, action, user, and - # IP. A RateLimiter usually has two RateLimits, one for the user and one for - # their IP. The action is limited if either the user or their IP are limited. + # Create a RateLimiter object for the given action. A RateLimiter usually has + # two RateLimits, one for the user and one for their IP. The action is + # limited if either the user or their IP are limited. # - # @param controller_name [String] the current controller - # @param action_name [String] the current controller action - # @param user [User] the current user - # @param ip_addr [String] the user's IP address - # @return [RateLimit] the rate limit for the action - def self.for_action(controller_name, action_name, user, ip_addr) - action = "#{controller_name}:#{action_name}" + # @param action [String] An identifier for the action being rate limited. + # @param rate [Float] The rate limit, in actions per second. + # @param burst [Float] The burst limit (the maximum number of actions you can + # perform in one burst before being rate limited). + # @param user [User] The current user. + # @param ip_addr [String] The user's IP address. + # @return [RateLimit] The rate limit for the action. + def self.build(action:, rate:, burst:, user:, ip_addr:) 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