Commit fd7a09ee authored by Ash McKenzie's avatar Ash McKenzie

Catch and handle RateLimitedError exception

RateLimitedError is thrown when rate limit
throttling is being applied.
parent 42c74b25
...@@ -108,6 +108,12 @@ class ApplicationController < ActionController::Base ...@@ -108,6 +108,12 @@ class ApplicationController < ActionController::Base
head :forbidden, retry_after: Gitlab::Auth::UniqueIpsLimiter.config.unique_ips_limit_time_window head :forbidden, retry_after: Gitlab::Auth::UniqueIpsLimiter.config.unique_ips_limit_time_window
end end
rescue_from RateLimitedService::RateLimitedError do |e|
e.log_request(request, current_user)
response.headers.merge!(e.headers)
render plain: e.message, status: :too_many_requests
end
def redirect_back_or_default(default: root_path, options: {}) def redirect_back_or_default(default: root_path, options: {})
redirect_back(fallback_location: default, **options) redirect_back(fallback_location: default, **options)
end end
......
...@@ -124,6 +124,11 @@ module API ...@@ -124,6 +124,11 @@ module API
handle_api_exception(exception) handle_api_exception(exception)
end end
rescue_from RateLimitedService::RateLimitedError do |exception|
exception.log_request(context.request, context.current_user)
rack_response({ 'message' => { 'error' => exception.message } }.to_json, 429, exception.headers)
end
format :json format :json
formatter :json, Gitlab::Json::GrapeFormatter formatter :json, Gitlab::Json::GrapeFormatter
content_type :json, 'application/json' content_type :json, 'application/json'
......
...@@ -1411,39 +1411,42 @@ RSpec.describe Projects::IssuesController do ...@@ -1411,39 +1411,42 @@ RSpec.describe Projects::IssuesController do
stub_application_setting(issues_create_limit: 5) stub_application_setting(issues_create_limit: 5)
end end
it 'prevents from creating more issues', :request_store do context 'when issue creation limits imposed' do
5.times { post_new_issue } it 'prevents from creating more issues', :request_store do
5.times { post_new_issue }
expect { post_new_issue }
.to change { Gitlab::GitalyClient.get_request_count }.by(1) # creates 1 projects and 0 issues
post_new_issue
expect(response.body).to eq(_('This endpoint has been requested too many times. Try again later.'))
expect(response).to have_gitlab_http_status(:too_many_requests)
end
it 'logs the event on auth.log' do
attributes = {
message: 'Application_Rate_Limiter_Request',
env: :issues_create_request_limit,
remote_ip: '0.0.0.0',
request_method: 'POST',
path: "/#{project.full_path}/-/issues",
user_id: user.id,
username: user.username
}
expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once expect { post_new_issue }
.to change { Gitlab::GitalyClient.get_request_count }.by(1) # creates 1 projects and 0 issues
project.add_developer(user) post_new_issue
sign_in(user)
6.times do expect(response.body).to eq(_('This endpoint has been requested too many times. Try again later.'))
post :create, params: { expect(response).to have_gitlab_http_status(:too_many_requests)
namespace_id: project.namespace.to_param, end
project_id: project,
issue: { title: 'Title', description: 'Description' } it 'logs the event on auth.log' do
attributes = {
message: 'Application_Rate_Limiter_Request',
env: :issues_create_request_limit,
remote_ip: '0.0.0.0',
request_method: 'POST',
path: "/#{project.full_path}/-/issues",
user_id: user.id,
username: user.username
} }
expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once
project.add_developer(user)
sign_in(user)
6.times do
post :create, params: {
namespace_id: project.namespace.to_param,
project_id: project,
issue: { title: 'Title', description: 'Description' }
}
end
end end
end end
end end
......
...@@ -53,7 +53,11 @@ RSpec.describe Mutations::Issues::Create do ...@@ -53,7 +53,11 @@ RSpec.describe Mutations::Issues::Create do
stub_spam_services stub_spam_services
end end
subject { mutation.resolve(**mutation_params) } def resolve
mutation.resolve(**mutation_params)
end
subject { resolve }
context 'when the user does not have permission to create an issue' do context 'when the user does not have permission to create an issue' do
it 'raises an error' do it 'raises an error' do
...@@ -61,6 +65,15 @@ RSpec.describe Mutations::Issues::Create do ...@@ -61,6 +65,15 @@ RSpec.describe Mutations::Issues::Create do
end end
end end
context 'when the user has exceeded the rate limit' do
it 'raises an error' do
allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
project.add_developer(user)
expect { resolve }.to raise_error(RateLimitedService::RateLimitedError, _('This endpoint has been requested too many times. Try again later.'))
end
end
context 'when the user can create an issue' do context 'when the user can create an issue' do
context 'when creating an issue a developer' do context 'when creating an issue a developer' do
before do before do
......
...@@ -136,6 +136,36 @@ RSpec.describe Gitlab::Email::Handler::CreateIssueHandler do ...@@ -136,6 +136,36 @@ RSpec.describe Gitlab::Email::Handler::CreateIssueHandler do
expect { handler.execute }.to raise_error(Gitlab::Email::ProjectNotFound) expect { handler.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
end end
end end
context 'rate limiting' do
let(:rate_limited_service_feature_enabled) { nil }
before do
stub_feature_flags(rate_limited_service_issues_create: rate_limited_service_feature_enabled)
end
context 'when :rate_limited_service Feature is disabled' do
let(:rate_limited_service_feature_enabled) { false }
it 'does not attempt to throttle' do
expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?)
setup_attachment
receiver.execute
end
end
context 'when :rate_limited_service Feature is enabled' do
let(:rate_limited_service_feature_enabled) { true }
it 'raises a RateLimitedService::RateLimitedError' do
allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
setup_attachment
expect { receiver.execute }.to raise_error(RateLimitedService::RateLimitedError, _('This endpoint has been requested too many times. Try again later.'))
end
end
end
end end
def email_fixture(path) def email_fixture(path)
......
...@@ -243,6 +243,15 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do ...@@ -243,6 +243,15 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do
end end
end end
end end
context 'when rate limiting is in effect' do
it 'allows unlimited new issue creation' do
stub_application_setting(issues_create_limit: 1)
setup_attachment
expect { 2.times { receiver.execute } }.to change { Issue.count }.by(2)
end
end
end end
describe '#can_handle?' do describe '#can_handle?' do
......
...@@ -399,16 +399,15 @@ RSpec.describe API::Issues do ...@@ -399,16 +399,15 @@ RSpec.describe API::Issues do
end end
context 'when request exceeds the rate limit' do context 'when request exceeds the rate limit' do
before do it 'prevents users from creating more issues' do
allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true) allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
end
it 'prevents users from creating more issues' do
post api("/projects/#{project.id}/issues", user), post api("/projects/#{project.id}/issues", user),
params: { title: 'new issue', labels: 'label, label2', weight: 3, assignee_ids: [user2.id] } params: { title: 'new issue', labels: 'label, label2', weight: 3, assignee_ids: [user2.id] }
expect(response).to have_gitlab_http_status(:too_many_requests)
expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.') expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.')
expect(response).to have_gitlab_http_status(:too_many_requests)
end end
end end
end end
......
...@@ -80,6 +80,21 @@ RSpec.describe EmailReceiverWorker, :mailer do ...@@ -80,6 +80,21 @@ RSpec.describe EmailReceiverWorker, :mailer do
expect(email).to be_nil expect(email).to be_nil
end end
end end
context 'when the error is RateLimitedService::RateLimitedError' do
let(:error) { RateLimitedService::RateLimitedError.new(key: :issues_create, rate_limiter: Gitlab::ApplicationRateLimiter) }
it 'does not report the error to the sender' do
expect(Gitlab::ErrorTracking).to receive(:track_exception).with(error).and_call_original
perform_enqueued_jobs do
described_class.new.perform(raw_message)
end
email = ActionMailer::Base.deliveries.last
expect(email).to be_nil
end
end
end end
end end
......
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