Fix rate limiting for paths with formats (#20675)
This commit is contained in:
		
							
								
								
									
										3
									
								
								Gemfile
									
									
									
									
									
								
							
							
						
						
									
										3
									
								
								Gemfile
									
									
									
									
									
								
							| @@ -122,6 +122,7 @@ group :test do | ||||
|   gem 'simplecov', '~> 0.21', require: false | ||||
|   gem 'webmock', '~> 3.18' | ||||
|   gem 'rspec_junit_formatter', '~> 0.6' | ||||
|   gem 'rack-test', '~> 2.0' | ||||
| end | ||||
|  | ||||
| group :development do | ||||
| @@ -152,7 +153,5 @@ end | ||||
|  | ||||
| gem 'concurrent-ruby', require: false | ||||
| gem 'connection_pool', require: false | ||||
|  | ||||
| gem 'xorcist', '~> 1.1' | ||||
|  | ||||
| gem 'cocoon', '~> 1.2' | ||||
|   | ||||
| @@ -815,6 +815,7 @@ DEPENDENCIES | ||||
|   rack (~> 2.2.4) | ||||
|   rack-attack (~> 6.6) | ||||
|   rack-cors (~> 1.1) | ||||
|   rack-test (~> 2.0) | ||||
|   rails (~> 6.1.7) | ||||
|   rails-controller-testing (~> 1.0) | ||||
|   rails-i18n (~> 6.0) | ||||
|   | ||||
| @@ -17,6 +17,18 @@ class Rack::Attack | ||||
|       @remote_ip ||= (@env["action_dispatch.remote_ip"] || ip).to_s | ||||
|     end | ||||
|  | ||||
|     def throttleable_remote_ip | ||||
|       @throttleable_remote_ip ||= begin | ||||
|         ip = IPAddr.new(remote_ip) | ||||
|  | ||||
|         if ip.ipv6? | ||||
|           ip.mask(64) | ||||
|         else | ||||
|           ip | ||||
|         end | ||||
|       end.to_s | ||||
|     end | ||||
|  | ||||
|     def authenticated_user_id | ||||
|       authenticated_token&.resource_owner_id | ||||
|     end | ||||
| @@ -29,6 +41,10 @@ class Rack::Attack | ||||
|       path.start_with?('/api') | ||||
|     end | ||||
|  | ||||
|     def path_matches?(other_path) | ||||
|       /\A#{Regexp.escape(other_path)}(\..*)?\z/ =~ path | ||||
|     end | ||||
|  | ||||
|     def web_request? | ||||
|       !api_request? | ||||
|     end | ||||
| @@ -51,19 +67,19 @@ class Rack::Attack | ||||
|   end | ||||
|  | ||||
|   throttle('throttle_unauthenticated_api', limit: 300, period: 5.minutes) do |req| | ||||
|     req.remote_ip if req.api_request? && req.unauthenticated? | ||||
|     req.throttleable_remote_ip if req.api_request? && req.unauthenticated? | ||||
|   end | ||||
|  | ||||
|   throttle('throttle_api_media', limit: 30, period: 30.minutes) do |req| | ||||
|     req.authenticated_user_id if req.post? && req.path.match?('^/api/v\d+/media') | ||||
|     req.authenticated_user_id if req.post? && req.path.match?(/\A\/api\/v\d+\/media\z/i) | ||||
|   end | ||||
|  | ||||
|   throttle('throttle_media_proxy', limit: 30, period: 10.minutes) do |req| | ||||
|     req.remote_ip if req.path.start_with?('/media_proxy') | ||||
|     req.throttleable_remote_ip if req.path.start_with?('/media_proxy') | ||||
|   end | ||||
|  | ||||
|   throttle('throttle_api_sign_up', limit: 5, period: 30.minutes) do |req| | ||||
|     req.remote_ip if req.post? && req.path == '/api/v1/accounts' | ||||
|     req.throttleable_remote_ip if req.post? && req.path == '/api/v1/accounts' | ||||
|   end | ||||
|  | ||||
|   throttle('throttle_authenticated_paging', limit: 300, period: 15.minutes) do |req| | ||||
| @@ -71,39 +87,34 @@ class Rack::Attack | ||||
|   end | ||||
|  | ||||
|   throttle('throttle_unauthenticated_paging', limit: 300, period: 15.minutes) do |req| | ||||
|     req.remote_ip if req.paging_request? && req.unauthenticated? | ||||
|     req.throttleable_remote_ip if req.paging_request? && req.unauthenticated? | ||||
|   end | ||||
|  | ||||
|   API_DELETE_REBLOG_REGEX = /\A\/api\/v1\/statuses\/[\d]+\/unreblog/.freeze | ||||
|   API_DELETE_STATUS_REGEX = /\A\/api\/v1\/statuses\/[\d]+/.freeze | ||||
|   API_DELETE_REBLOG_REGEX = /\A\/api\/v1\/statuses\/[\d]+\/unreblog\z/.freeze | ||||
|   API_DELETE_STATUS_REGEX = /\A\/api\/v1\/statuses\/[\d]+\z/.freeze | ||||
|  | ||||
|   throttle('throttle_api_delete', limit: 30, period: 30.minutes) do |req| | ||||
|     req.authenticated_user_id if (req.post? && req.path.match?(API_DELETE_REBLOG_REGEX)) || (req.delete? && req.path.match?(API_DELETE_STATUS_REGEX)) | ||||
|   end | ||||
|  | ||||
|   throttle('throttle_sign_up_attempts/ip', limit: 25, period: 5.minutes) do |req| | ||||
|     if req.post? && req.path == '/auth' | ||||
|       addr = req.remote_ip | ||||
|       addr = IPAddr.new(addr) if addr.is_a?(String) | ||||
|       addr = addr.mask(64) if addr.ipv6? | ||||
|       addr.to_s | ||||
|     end | ||||
|     req.throttleable_remote_ip if req.post? && req.path_matches?('/auth') | ||||
|   end | ||||
|  | ||||
|   throttle('throttle_password_resets/ip', limit: 25, period: 5.minutes) do |req| | ||||
|     req.remote_ip if req.post? && req.path == '/auth/password' | ||||
|     req.throttleable_remote_ip if req.post? && req.path_matches?('/auth/password') | ||||
|   end | ||||
|  | ||||
|   throttle('throttle_password_resets/email', limit: 5, period: 30.minutes) do |req| | ||||
|     req.params.dig('user', 'email').presence if req.post? && req.path == '/auth/password' | ||||
|     req.params.dig('user', 'email').presence if req.post? && req.path_matches?('/auth/password') | ||||
|   end | ||||
|  | ||||
|   throttle('throttle_email_confirmations/ip', limit: 25, period: 5.minutes) do |req| | ||||
|     req.remote_ip if req.post? && %w(/auth/confirmation /api/v1/emails/confirmations).include?(req.path) | ||||
|     req.throttleable_remote_ip if req.post? && (req.path_matches?('/auth/confirmation') || req.path == '/api/v1/emails/confirmations') | ||||
|   end | ||||
|  | ||||
|   throttle('throttle_email_confirmations/email', limit: 5, period: 30.minutes) do |req| | ||||
|     if req.post? && req.path == '/auth/password' | ||||
|     if req.post? && req.path_matches?('/auth/password') | ||||
|       req.params.dig('user', 'email').presence | ||||
|     elsif req.post? && req.path == '/api/v1/emails/confirmations' | ||||
|       req.authenticated_user_id | ||||
| @@ -111,11 +122,11 @@ class Rack::Attack | ||||
|   end | ||||
|  | ||||
|   throttle('throttle_login_attempts/ip', limit: 25, period: 5.minutes) do |req| | ||||
|     req.remote_ip if req.post? && req.path == '/auth/sign_in' | ||||
|     req.throttleable_remote_ip if req.post? && req.path_matches?('/auth/sign_in') | ||||
|   end | ||||
|  | ||||
|   throttle('throttle_login_attempts/email', limit: 25, period: 1.hour) do |req| | ||||
|     req.session[:attempt_user_id] || req.params.dig('user', 'email').presence if req.post? && req.path == '/auth/sign_in' | ||||
|     req.session[:attempt_user_id] || req.params.dig('user', 'email').presence if req.post? && req.path_matches?('/auth/sign_in') | ||||
|   end | ||||
|  | ||||
|   self.throttled_responder = lambda do |request| | ||||
|   | ||||
| @@ -73,7 +73,7 @@ Rails.application.routes.draw do | ||||
|     end | ||||
|   end | ||||
|  | ||||
|   devise_for :users, path: 'auth', controllers: { | ||||
|   devise_for :users, path: 'auth', format: false, controllers: { | ||||
|     omniauth_callbacks: 'auth/omniauth_callbacks', | ||||
|     sessions:           'auth/sessions', | ||||
|     registrations:      'auth/registrations', | ||||
| @@ -216,7 +216,7 @@ Rails.application.routes.draw do | ||||
|   resource :relationships, only: [:show, :update] | ||||
|   resource :statuses_cleanup, controller: :statuses_cleanup, only: [:show, :update] | ||||
|  | ||||
|   get '/media_proxy/:id/(*any)', to: 'media_proxy#show', as: :media_proxy | ||||
|   get '/media_proxy/:id/(*any)', to: 'media_proxy#show', as: :media_proxy, format: false | ||||
|  | ||||
|   resource :authorize_interaction, only: [:show, :create] | ||||
|   resource :share, only: [:show, :create] | ||||
| @@ -404,7 +404,7 @@ Rails.application.routes.draw do | ||||
|  | ||||
|   get '/admin', to: redirect('/admin/dashboard', status: 302) | ||||
|  | ||||
|   namespace :api do | ||||
|   namespace :api, format: false do | ||||
|     # OEmbed | ||||
|     get '/oembed', to: 'oembed#show', as: :oembed | ||||
|  | ||||
|   | ||||
							
								
								
									
										82
									
								
								spec/config/initializers/rack_attack_spec.rb
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										82
									
								
								spec/config/initializers/rack_attack_spec.rb
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,82 @@ | ||||
| require 'rails_helper' | ||||
|  | ||||
| describe Rack::Attack do | ||||
|   include Rack::Test::Methods | ||||
|  | ||||
|   def app | ||||
|     Rails.application | ||||
|   end | ||||
|  | ||||
|   shared_examples 'throttled endpoint' do | ||||
|     context 'when the number of requests is lower than the limit' do | ||||
|       it 'does not change the request status' do | ||||
|         limit.times do | ||||
|           request.call | ||||
|           expect(last_response.status).to_not eq(429) | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|  | ||||
|     context 'when the number of requests is higher than the limit' do | ||||
|       it 'returns http too many requests' do | ||||
|         (limit * 2).times do |i| | ||||
|           request.call | ||||
|           expect(last_response.status).to eq(429) if i > limit | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|   end | ||||
|  | ||||
|   let(:remote_ip) { '1.2.3.5' } | ||||
|  | ||||
|   describe 'throttle excessive sign-up requests by IP address' do | ||||
|     context 'through the website' do | ||||
|       let(:limit) { 25 } | ||||
|       let(:request) { ->() { post path, {}, 'REMOTE_ADDR' => remote_ip } } | ||||
|  | ||||
|       context 'for exact path' do | ||||
|         let(:path)  { '/auth' } | ||||
|         it_behaves_like 'throttled endpoint' | ||||
|       end | ||||
|  | ||||
|       context 'for path with format' do | ||||
|         let(:path)  { '/auth.html' } | ||||
|         it_behaves_like 'throttled endpoint' | ||||
|       end | ||||
|     end | ||||
|  | ||||
|     context 'through the API' do | ||||
|       let(:limit) { 5 } | ||||
|       let(:request) { ->() { post path, {}, 'REMOTE_ADDR' => remote_ip } } | ||||
|  | ||||
|       context 'for exact path' do | ||||
|         let(:path)  { '/api/v1/accounts' } | ||||
|         it_behaves_like 'throttled endpoint' | ||||
|       end | ||||
|  | ||||
|       context 'for path with format' do | ||||
|         let(:path)  { '/api/v1/accounts.json' } | ||||
|  | ||||
|         it 'returns http not found' do | ||||
|           request.call | ||||
|           expect(last_response.status).to eq(404) | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|   end | ||||
|  | ||||
|   describe 'throttle excessive sign-in requests by IP address' do | ||||
|     let(:limit) { 25 } | ||||
|     let(:request) { ->() { post path, {}, 'REMOTE_ADDR' => remote_ip } } | ||||
|  | ||||
|     context 'for exact path' do | ||||
|       let(:path)  { '/auth/sign_in' } | ||||
|       it_behaves_like 'throttled endpoint' | ||||
|     end | ||||
|  | ||||
|     context 'for path with format' do | ||||
|       let(:path)  { '/auth/sign_in.html' } | ||||
|       it_behaves_like 'throttled endpoint' | ||||
|     end | ||||
|   end | ||||
| end | ||||
		Reference in New Issue
	
	Block a user