Commit 83b6f86e authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Freeze time for rate limiter specs

We need to freeze time in these specs so that we know the multiple
requests we're making fall into the same time bucket used by the rate
limiter
parent 29472ba1
...@@ -109,7 +109,7 @@ RSpec.describe Projects::HooksController do ...@@ -109,7 +109,7 @@ RSpec.describe Projects::HooksController do
describe '#test' do describe '#test' do
let(:hook) { create(:project_hook, project: project) } let(:hook) { create(:project_hook, project: project) }
context 'when the endpoint receives requests above the limit' do context 'when the endpoint receives requests above the limit', :freeze_time, :clean_gitlab_redis_rate_limiting do
before do before do
allow(Gitlab::ApplicationRateLimiter).to receive(:rate_limits) allow(Gitlab::ApplicationRateLimiter).to receive(:rate_limits)
.and_return(project_testing_hook: { threshold: 1, interval: 1.minute }) .and_return(project_testing_hook: { threshold: 1, interval: 1.minute })
......
...@@ -522,8 +522,7 @@ module GraphqlHelpers ...@@ -522,8 +522,7 @@ module GraphqlHelpers
end end
end end
# See note at graphql_data about memoization and multiple requests def graphql_errors(body = fresh_response_data)
def graphql_errors(body = json_response)
case body case body
when Hash # regular query when Hash # regular query
body['errors'] body['errors']
......
...@@ -6,39 +6,41 @@ ...@@ -6,39 +6,41 @@
# - request_full_path # - request_full_path
RSpec.shared_examples 'request exceeding rate limit' do RSpec.shared_examples 'request exceeding rate limit' do
before do context 'with rate limiter', :freeze_time, :clean_gitlab_redis_rate_limiting do
stub_application_setting(notes_create_limit: 2) before do
2.times { post :create, params: params } stub_application_setting(notes_create_limit: 2)
end 2.times { post :create, params: params }
end
it 'prevents from creating more notes', :request_store do it 'prevents from creating more notes' do
expect { post :create, params: params } expect { post :create, params: params }
.to change { Note.count }.by(0) .to change { Note.count }.by(0)
expect(response).to have_gitlab_http_status(:too_many_requests) expect(response).to have_gitlab_http_status(:too_many_requests)
expect(response.body).to eq(_('This endpoint has been requested too many times. Try again later.')) expect(response.body).to eq(_('This endpoint has been requested too many times. Try again later.'))
end end
it 'logs the event in auth.log' do it 'logs the event in auth.log' do
attributes = { attributes = {
message: 'Application_Rate_Limiter_Request', message: 'Application_Rate_Limiter_Request',
env: :notes_create_request_limit, env: :notes_create_request_limit,
remote_ip: '0.0.0.0', remote_ip: '0.0.0.0',
request_method: 'POST', request_method: 'POST',
path: request_full_path, path: request_full_path,
user_id: user.id, user_id: user.id,
username: user.username username: user.username
} }
expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once
post :create, params: params post :create, params: params
end end
it 'allows user in allow-list to create notes, even if the case is different' do it 'allows user in allow-list to create notes, even if the case is different' do
user.update_attribute(:username, user.username.titleize) user.update_attribute(:username, user.username.titleize)
stub_application_setting(notes_create_limit_allowlist: ["#{user.username.downcase}"]) stub_application_setting(notes_create_limit_allowlist: ["#{user.username.downcase}"])
post :create, params: params post :create, params: params
expect(response).to have_gitlab_http_status(:found) expect(response).to have_gitlab_http_status(:found)
end
end end
end end
...@@ -66,20 +66,22 @@ RSpec.shared_examples 'a Note mutation when the given resource id is not for a N ...@@ -66,20 +66,22 @@ RSpec.shared_examples 'a Note mutation when the given resource id is not for a N
end end
RSpec.shared_examples 'a Note mutation when there are rate limit validation errors' do RSpec.shared_examples 'a Note mutation when there are rate limit validation errors' do
before do context 'with rate limiter', :freeze_time, :clean_gitlab_redis_rate_limiting do
stub_application_setting(notes_create_limit: 3)
3.times { post_graphql_mutation(mutation, current_user: current_user) }
end
it_behaves_like 'a Note mutation that does not create a Note'
it_behaves_like 'a mutation that returns top-level errors',
errors: ['This endpoint has been requested too many times. Try again later.']
context 'when the user is in the allowlist' do
before do before do
stub_application_setting(notes_create_limit_allowlist: ["#{current_user.username}"]) stub_application_setting(notes_create_limit: 3)
3.times { post_graphql_mutation(mutation, current_user: current_user) }
end end
it_behaves_like 'a Note mutation that creates a Note' it_behaves_like 'a Note mutation that does not create a Note'
it_behaves_like 'a mutation that returns top-level errors',
errors: ['This endpoint has been requested too many times. Try again later.']
context 'when the user is in the allowlist' do
before do
stub_application_setting(notes_create_limit_allowlist: ["#{current_user.username}"])
end
it_behaves_like 'a Note mutation that creates a Note'
end
end end
end end
...@@ -281,7 +281,7 @@ RSpec.shared_examples 'noteable API' do |parent_type, noteable_type, id_name| ...@@ -281,7 +281,7 @@ RSpec.shared_examples 'noteable API' do |parent_type, noteable_type, id_name|
end end
end end
context 'when request exceeds the rate limit' do context 'when request exceeds the rate limit', :freeze_time, :clean_gitlab_redis_rate_limiting do
before do before do
stub_application_setting(notes_create_limit: 1) stub_application_setting(notes_create_limit: 1)
allow(::Gitlab::ApplicationRateLimiter).to receive(:increment).and_return(2) allow(::Gitlab::ApplicationRateLimiter).to receive(:increment).and_return(2)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment