Fix: 絵文字リアクションに厳しいレートリミットが適用される問題 (#752)
This commit is contained in:
parent
14820ee52d
commit
51155d6bd8
2 changed files with 70 additions and 17 deletions
|
@ -141,8 +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
|
||||||
|
|
||||||
|
API_CREATE_EMOJI_REACTION_REGEX = %r{\A/api/v1/statuses/\d+/emoji_reactions}
|
||||||
|
|
||||||
throttle('throttle_password_change/account', limit: 10, period: 10.minutes) do |req|
|
throttle('throttle_password_change/account', limit: 10, period: 10.minutes) do |req|
|
||||||
req.warden_user_id if req.put? || (req.patch? && req.path_matches?('/auth'))
|
req.warden_user_id if (req.put? && !req.path.match?(API_CREATE_EMOJI_REACTION_REGEX)) || (req.patch? && req.path_matches?('/auth'))
|
||||||
end
|
end
|
||||||
|
|
||||||
self.throttled_responder = lambda do |request|
|
self.throttled_responder = lambda do |request|
|
||||||
|
|
|
@ -7,7 +7,7 @@ describe Rack::Attack, type: :request do
|
||||||
Rails.application
|
Rails.application
|
||||||
end
|
end
|
||||||
|
|
||||||
shared_examples 'throttled endpoint' do
|
shared_context 'with throttled endpoint base' do
|
||||||
before do
|
before do
|
||||||
# Rack::Attack periods are not rolling, so avoid flaky tests by setting the time in a way
|
# Rack::Attack periods are not rolling, so avoid flaky tests by setting the time in a way
|
||||||
# to avoid crossing period boundaries.
|
# to avoid crossing period boundaries.
|
||||||
|
@ -19,6 +19,30 @@ describe Rack::Attack, type: :request do
|
||||||
travel_to Time.zone.at(counter_prefix * period.seconds)
|
travel_to Time.zone.at(counter_prefix * period.seconds)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def below_limit
|
||||||
|
limit - 1
|
||||||
|
end
|
||||||
|
|
||||||
|
def above_limit
|
||||||
|
limit * 2
|
||||||
|
end
|
||||||
|
|
||||||
|
def throttle_count
|
||||||
|
described_class.cache.read("#{counter_prefix}:#{throttle}:#{discriminator}") || 0
|
||||||
|
end
|
||||||
|
|
||||||
|
def counter_prefix
|
||||||
|
(Time.now.to_i / period.seconds).to_i
|
||||||
|
end
|
||||||
|
|
||||||
|
def increment_counter
|
||||||
|
described_class.cache.count("#{throttle}:#{discriminator}", period)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
shared_examples 'throttled endpoint' do
|
||||||
|
include_examples 'with throttled endpoint base'
|
||||||
|
|
||||||
context 'when the number of requests is lower than the limit' do
|
context 'when the number of requests is lower than the limit' do
|
||||||
before do
|
before do
|
||||||
below_limit.times { increment_counter }
|
below_limit.times { increment_counter }
|
||||||
|
@ -46,25 +70,32 @@ describe Rack::Attack, type: :request do
|
||||||
expect(response).to_not have_http_status(429)
|
expect(response).to_not have_http_status(429)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def below_limit
|
shared_examples 'does not throttle endpoint' do
|
||||||
limit - 1
|
include_examples 'with throttled endpoint base'
|
||||||
|
|
||||||
|
context 'when the number of requests is lower than the limit' do
|
||||||
|
before do
|
||||||
|
below_limit.times { increment_counter }
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'does not change the request status' do
|
||||||
|
expect { request.call }.to change { throttle_count }.by(0)
|
||||||
|
|
||||||
|
expect(response).to_not have_http_status(429)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def above_limit
|
context 'when the number of requests is higher than the limit' do
|
||||||
limit * 2
|
before do
|
||||||
end
|
above_limit.times { increment_counter }
|
||||||
|
end
|
||||||
|
|
||||||
def throttle_count
|
it 'returns http too many requests after limit and returns to normal status after period' do
|
||||||
described_class.cache.read("#{counter_prefix}:#{throttle}:#{discriminator}") || 0
|
expect { request.call }.to change { throttle_count }.by(0)
|
||||||
end
|
expect(response).to_not have_http_status(429)
|
||||||
|
end
|
||||||
def counter_prefix
|
|
||||||
(Time.now.to_i / period.seconds).to_i
|
|
||||||
end
|
|
||||||
|
|
||||||
def increment_counter
|
|
||||||
described_class.cache.count("#{throttle}:#{discriminator}", period)
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -176,4 +207,24 @@ describe Rack::Attack, type: :request do
|
||||||
|
|
||||||
it_behaves_like 'throttled endpoint'
|
it_behaves_like 'throttled endpoint'
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe 'throttle excessive emoji reaction 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(:status) { Fabricate(:status) }
|
||||||
|
let(:emoji) { Fabricate(:custom_emoji) }
|
||||||
|
let(:path) { "/api/v1/statuses/#{status.id}/emoji_reactions/#{emoji.shortcode}" }
|
||||||
|
let(:discriminator) { user.id }
|
||||||
|
|
||||||
|
before do
|
||||||
|
sign_in user, scope: :user
|
||||||
|
|
||||||
|
get '/'
|
||||||
|
end
|
||||||
|
|
||||||
|
it_behaves_like 'does not throttle endpoint'
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue