From 16249946aea0db8a74748909d65c94742482dcb7 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 30 May 2024 14:14:04 +0200 Subject: [PATCH] Merge pull request from GHSA-q3rg-xx5v-4mxh --- config/initializers/rack_attack.rb | 10 ++++++- spec/config/initializers/rack/attack_spec.rb | 31 ++++++++++++++++++-- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 1757ce5df..034fb7444 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -30,13 +30,17 @@ class Rack::Attack end def authenticated_user_id - authenticated_token&.resource_owner_id + authenticated_token&.resource_owner_id || warden_user_id end def authenticated_token_id authenticated_token&.id end + def warden_user_id + @env['warden']&.user&.id + end + def unauthenticated? !authenticated_user_id end @@ -141,6 +145,10 @@ class Rack::Attack req.session[:attempt_user_id] || req.params.dig('user', 'email').presence if req.post? && req.path_matches?('/auth/sign_in') end + throttle('throttle_password_change/account', limit: 10, period: 10.minutes) do |req| + req.authenticated_user_id if req.put? || (req.patch? && req.path_matches?('/auth')) + end + self.throttled_responder = lambda do |request| now = Time.now.utc match_data = request.env['rack.attack.match_data'] diff --git a/spec/config/initializers/rack/attack_spec.rb b/spec/config/initializers/rack/attack_spec.rb index 0a388c2f4..19de48089 100644 --- a/spec/config/initializers/rack/attack_spec.rb +++ b/spec/config/initializers/rack/attack_spec.rb @@ -56,7 +56,7 @@ describe Rack::Attack, type: :request do end def throttle_count - described_class.cache.read("#{counter_prefix}:#{throttle}:#{remote_ip}") || 0 + described_class.cache.read("#{counter_prefix}:#{throttle}:#{discriminator}") || 0 end def counter_prefix @@ -64,11 +64,12 @@ describe Rack::Attack, type: :request do end def increment_counter - described_class.cache.count("#{throttle}:#{remote_ip}", period) + described_class.cache.count("#{throttle}:#{discriminator}", period) end end let(:remote_ip) { '1.2.3.5' } + let(:discriminator) { remote_ip } describe 'throttle excessive sign-up requests by IP address' do context 'when accessed through the website' do @@ -149,4 +150,30 @@ describe Rack::Attack, type: :request do it_behaves_like 'throttled endpoint' end + + describe 'throttle excessive password change requests by account' do + let(:user) { Fabricate(:user, email: 'user@host.example') } + let(:throttle) { 'throttle_password_change/account' } + let(:limit) { 10 } + let(:period) { 10.minutes } + let(:request) { -> { put path, headers: { 'REMOTE_ADDR' => remote_ip } } } + let(:path) { '/auth' } + let(:discriminator) { user.id } + + before do + sign_in user, scope: :user + + # Unfortunately, devise's `sign_in` helper causes the `session` to be + # loaded in the next request regardless of whether it's actually accessed + # by the client code. + # + # So, we make an extra query to clear issue a session cookie instead. + # + # A less resource-intensive way to deal with that would be to generate the + # session cookie manually, but this seems pretty involved. + get '/' + end + + it_behaves_like 'throttled endpoint' + end end