Merge pull request from GHSA-q3rg-xx5v-4mxh
This commit is contained in:
parent
e292a28933
commit
020228ddba
3 changed files with 41 additions and 10 deletions
|
@ -30,13 +30,17 @@ class Rack::Attack
|
||||||
end
|
end
|
||||||
|
|
||||||
def authenticated_user_id
|
def authenticated_user_id
|
||||||
authenticated_token&.resource_owner_id
|
authenticated_token&.resource_owner_id || warden_user_id
|
||||||
end
|
end
|
||||||
|
|
||||||
def authenticated_token_id
|
def authenticated_token_id
|
||||||
authenticated_token&.id
|
authenticated_token&.id
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def warden_user_id
|
||||||
|
@env['warden']&.user&.id
|
||||||
|
end
|
||||||
|
|
||||||
def unauthenticated?
|
def unauthenticated?
|
||||||
!authenticated_user_id
|
!authenticated_user_id
|
||||||
end
|
end
|
||||||
|
@ -137,6 +141,10 @@ class Rack::Attack
|
||||||
req.session[:attempt_user_id] || req.params.dig('user', 'email').presence if req.post? && req.path_matches?('/auth/sign_in')
|
req.session[:attempt_user_id] || req.params.dig('user', 'email').presence if req.post? && req.path_matches?('/auth/sign_in')
|
||||||
end
|
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|
|
self.throttled_responder = lambda do |request|
|
||||||
now = Time.now.utc
|
now = Time.now.utc
|
||||||
match_data = request.env['rack.attack.match_data']
|
match_data = request.env['rack.attack.match_data']
|
||||||
|
|
|
@ -1,8 +1,6 @@
|
||||||
require 'rails_helper'
|
require 'rails_helper'
|
||||||
|
|
||||||
describe Rack::Attack do
|
describe Rack::Attack, type: :request do
|
||||||
include Rack::Test::Methods
|
|
||||||
|
|
||||||
def app
|
def app
|
||||||
Rails.application
|
Rails.application
|
||||||
end
|
end
|
||||||
|
@ -12,7 +10,7 @@ describe Rack::Attack do
|
||||||
it 'does not change the request status' do
|
it 'does not change the request status' do
|
||||||
limit.times do
|
limit.times do
|
||||||
request.call
|
request.call
|
||||||
expect(last_response.status).to_not eq(429)
|
expect(response.status).to_not eq(429)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -21,7 +19,7 @@ describe Rack::Attack do
|
||||||
it 'returns http too many requests' do
|
it 'returns http too many requests' do
|
||||||
(limit * 2).times do |i|
|
(limit * 2).times do |i|
|
||||||
request.call
|
request.call
|
||||||
expect(last_response.status).to eq(429) if i > limit
|
expect(response.status).to eq(429) if i > limit
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -32,7 +30,7 @@ describe Rack::Attack do
|
||||||
describe 'throttle excessive sign-up requests by IP address' do
|
describe 'throttle excessive sign-up requests by IP address' do
|
||||||
context 'through the website' do
|
context 'through the website' do
|
||||||
let(:limit) { 25 }
|
let(:limit) { 25 }
|
||||||
let(:request) { ->() { post path, {}, 'REMOTE_ADDR' => remote_ip } }
|
let(:request) { ->() { post path, headers: { 'REMOTE_ADDR' => remote_ip } } }
|
||||||
|
|
||||||
context 'for exact path' do
|
context 'for exact path' do
|
||||||
let(:path) { '/auth' }
|
let(:path) { '/auth' }
|
||||||
|
@ -47,7 +45,7 @@ describe Rack::Attack do
|
||||||
|
|
||||||
context 'through the API' do
|
context 'through the API' do
|
||||||
let(:limit) { 5 }
|
let(:limit) { 5 }
|
||||||
let(:request) { ->() { post path, {}, 'REMOTE_ADDR' => remote_ip } }
|
let(:request) { ->() { post path, headers: { 'REMOTE_ADDR' => remote_ip } } }
|
||||||
|
|
||||||
context 'for exact path' do
|
context 'for exact path' do
|
||||||
let(:path) { '/api/v1/accounts' }
|
let(:path) { '/api/v1/accounts' }
|
||||||
|
@ -59,7 +57,7 @@ describe Rack::Attack do
|
||||||
|
|
||||||
it 'returns http not found' do
|
it 'returns http not found' do
|
||||||
request.call
|
request.call
|
||||||
expect(last_response.status).to eq(404)
|
expect(response.status).to eq(404)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -67,7 +65,7 @@ describe Rack::Attack do
|
||||||
|
|
||||||
describe 'throttle excessive sign-in requests by IP address' do
|
describe 'throttle excessive sign-in requests by IP address' do
|
||||||
let(:limit) { 25 }
|
let(:limit) { 25 }
|
||||||
let(:request) { ->() { post path, {}, 'REMOTE_ADDR' => remote_ip } }
|
let(:request) { ->() { post path, headers: { 'REMOTE_ADDR' => remote_ip } } }
|
||||||
|
|
||||||
context 'for exact path' do
|
context 'for exact path' do
|
||||||
let(:path) { '/auth/sign_in' }
|
let(:path) { '/auth/sign_in' }
|
||||||
|
@ -79,4 +77,28 @@ describe Rack::Attack do
|
||||||
it_behaves_like 'throttled endpoint'
|
it_behaves_like 'throttled endpoint'
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe 'throttle excessive password change requests by account' do
|
||||||
|
let(:user) { Fabricate(:user, email: 'user@host.example') }
|
||||||
|
let(:limit) { 10 }
|
||||||
|
let(:period) { 10.minutes }
|
||||||
|
let(:request) { -> { put path, headers: { 'REMOTE_ADDR' => remote_ip } } }
|
||||||
|
let(:path) { '/auth' }
|
||||||
|
|
||||||
|
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
|
end
|
||||||
|
|
|
@ -41,6 +41,7 @@ RSpec.configure do |config|
|
||||||
|
|
||||||
config.include Devise::Test::ControllerHelpers, type: :controller
|
config.include Devise::Test::ControllerHelpers, type: :controller
|
||||||
config.include Devise::Test::ControllerHelpers, type: :view
|
config.include Devise::Test::ControllerHelpers, type: :view
|
||||||
|
config.include Devise::Test::IntegrationHelpers, type: :request
|
||||||
config.include Paperclip::Shoulda::Matchers
|
config.include Paperclip::Shoulda::Matchers
|
||||||
config.include ActiveSupport::Testing::TimeHelpers
|
config.include ActiveSupport::Testing::TimeHelpers
|
||||||
config.include Redisable
|
config.include Redisable
|
||||||
|
|
Loading…
Reference in a new issue