diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb
index fcedd1c1c8da3bbb5a6deb414d85bab4908d8d7b..fa8d380404b8e11a58c3af936b5ecaa5193614bd 100644
--- a/app/services/issues/create_service.rb
+++ b/app/services/issues/create_service.rb
@@ -6,7 +6,7 @@ class CreateService < Issues::BaseService
prepend RateLimitedService
rate_limit key: :issues_create,
- opts: { scope: [:project, :current_user], users_allowlist: -> { [User.support_bot.username] } }
+ opts: { scope: [:project, :current_user, :external_author] }
# NOTE: For Issues::CreateService, we require the spam_params and do not default it to nil, because
# spam_checking is likely to be necessary. However, if there is not a request available in scope
@@ -25,6 +25,10 @@ def execute(skip_system_notes: false)
create(@issue, skip_system_notes: skip_system_notes)
end
+ def external_author
+ params[:external_author] # present when creating an issue using service desk (email: from)
+ end
+
def before_create(issue)
Spam::SpamActionService.new(
spammable: issue,
diff --git a/spec/fixtures/emails/service_desk_forwarded.eml b/spec/fixtures/emails/service_desk_forwarded.eml
index 569879728087b829158055e6179d4ff4a36c76e8..ab509cf55afe0995ee00c8f5c1601d3474debdcc 100644
--- a/spec/fixtures/emails/service_desk_forwarded.eml
+++ b/spec/fixtures/emails/service_desk_forwarded.eml
@@ -1,11 +1,11 @@
Delivered-To: incoming+email-test-project_id-issue-@appmail.adventuretime.ooo
-Return-Path: <jake@adventuretime.ooo>
+Return-Path: <jake.g@adventuretime.ooo>
Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400
Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400
Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming+email-test-project_id-issue-@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700
Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700
Date: Thu, 13 Jun 2013 17:03:48 -0400
-From: Jake the Dog <jake@adventuretime.ooo>
+From: Jake the Dog <jake.g@adventuretime.ooo>
To: support@adventuretime.ooo
Delivered-To: support@adventuretime.ooo
Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
diff --git a/spec/lib/gitlab/application_rate_limiter_spec.rb b/spec/lib/gitlab/application_rate_limiter_spec.rb
index ab6ac024af7d2b82338c9fab9b63574db4bafd9a..0ce7ae8d3aaefe1095800d17d0cabaa71b03a811 100644
--- a/spec/lib/gitlab/application_rate_limiter_spec.rb
+++ b/spec/lib/gitlab/application_rate_limiter_spec.rb
@@ -99,6 +99,16 @@
end
it_behaves_like 'action rate limiter'
+
+ context 'when a scope attribute is nil' do
+ let(:scope) { [user, nil] }
+
+ let(:cache_key) do
+ "application_rate_limiter:test_action:user:#{user.id}"
+ end
+
+ it_behaves_like 'action rate limiter'
+ end
end
context 'when the key is a combination of ActiveRecord models and strings' do
@@ -112,6 +122,16 @@
end
it_behaves_like 'action rate limiter'
+
+ context 'when a scope attribute is nil' do
+ let(:scope) { [project, commit, nil] }
+
+ let(:cache_key) do
+ "application_rate_limiter:test_action:project:#{project.id}:commit:#{commit.sha}"
+ end
+
+ it_behaves_like 'action rate limiter'
+ end
end
end
diff --git a/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb b/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb
index 4fb2dc241dc5f055233cc49bcdc61f79c2fd07af..942ac67c39dc38dc28e8776afc9b4701692d9a28 100644
--- a/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb
+++ b/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb
@@ -11,6 +11,7 @@
end
let(:email_raw) { email_fixture('emails/service_desk.eml') }
+ let(:author_email) { 'jake@adventuretime.ooo' }
let_it_be(:group) { create(:group, :private, name: "email") }
let(:expected_description) do
@@ -45,7 +46,7 @@
receiver.execute
new_issue = Issue.last
- expect(new_issue.issue_email_participants.first.email).to eq("jake@adventuretime.ooo")
+ expect(new_issue.issue_email_participants.first.email).to eq(author_email)
end
it 'sends thank you email' do
@@ -256,13 +257,60 @@ def set_template_file(file_name, content)
it_behaves_like 'a new issue request'
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
+ context 'when rate limiting is in effect', :clean_gitlab_redis_cache do
+ let(:receiver) { Gitlab::Email::Receiver.new(email_raw) }
+
+ subject { 2.times { receiver.execute } }
+
+ before do
+ stub_feature_flags(rate_limited_service_issues_create: true)
+ stub_application_setting(issues_create_limit: 1)
+ end
+
+ context 'when too many requests are sent by one user' do
+ it 'raises an error' do
+ freeze_time do
+ expect { subject }.to raise_error(RateLimitedService::RateLimitedError)
+ end
+ end
+
+ it 'creates 1 issue' do
+ freeze_time do
+ expect do
+ subject
+ rescue RateLimitedService::RateLimitedError
+ end.to change { Issue.count }.by(1)
+ end
+ end
+
+ context 'when requests are sent by different users' do
+ let(:email_raw_2) { email_fixture('emails/service_desk_forwarded.eml') }
+ let(:receiver2) { Gitlab::Email::Receiver.new(email_raw_2) }
- expect { 2.times { receiver.execute } }.to change { Issue.count }.by(2)
+ subject do
+ receiver.execute
+ receiver2.execute
+ end
+
+ it 'creates 2 issues' do
+ freeze_time do
+ expect { subject }.to change { Issue.count }.by(2)
+ end
+ end
+ end
+ end
+
+ context 'when limit is higher than sent emails' do
+ before do
+ stub_application_setting(issues_create_limit: 3)
+ end
+
+ it 'creates 2 issues' do
+ freeze_time do
+ expect { subject }.to change { Issue.count }.by(2)
+ end
end
end
end
@@ -336,6 +384,7 @@ def set_template_file(file_name, content)
end
context 'when the email is forwarded through an alias' do
+ let(:author_email) { 'jake.g@adventuretime.ooo' }
let(:email_raw) { email_fixture('emails/service_desk_forwarded.eml') }
it_behaves_like 'a new issue request'
diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb
index 1887be4896ea77595c009e7b4a92f454abb8c184..0e0f52debc3e9893bc946d251b9764bd5ad174b9 100644
--- a/spec/services/issues/create_service_spec.rb
+++ b/spec/services/issues/create_service_spec.rb
@@ -15,8 +15,7 @@
expect(described_class.rate_limiter_scoped_and_keyed).to be_a(RateLimitedService::RateLimiterScopedAndKeyed)
expect(described_class.rate_limiter_scoped_and_keyed.key).to eq(:issues_create)
- expect(described_class.rate_limiter_scoped_and_keyed.opts[:scope]).to eq(%i[project current_user])
- expect(described_class.rate_limiter_scoped_and_keyed.opts[:users_allowlist].call).to eq(%w[support-bot])
+ expect(described_class.rate_limiter_scoped_and_keyed.opts[:scope]).to eq(%i[project current_user external_author])
expect(described_class.rate_limiter_scoped_and_keyed.rate_limiter_klass).to eq(Gitlab::ApplicationRateLimiter)
end
end
@@ -289,6 +288,50 @@
described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
end
+ context 'when rate limiting is in effect', :clean_gitlab_redis_cache do
+ let(:user) { create(:user) }
+
+ before do
+ stub_feature_flags(rate_limited_service_issues_create: true)
+ stub_application_setting(issues_create_limit: 1)
+ end
+
+ subject do
+ 2.times { described_class.new(project: project, current_user: user, params: opts, spam_params: double).execute }
+ end
+
+ context 'when too many requests are sent by one user' do
+ it 'raises an error' do
+ freeze_time do
+ expect do
+ subject
+ end.to raise_error(RateLimitedService::RateLimitedError)
+ end
+ end
+
+ it 'creates 1 issue' do
+ freeze_time do
+ expect do
+ subject
+ rescue RateLimitedService::RateLimitedError
+ end.to change { Issue.count }.by(1)
+ end
+ end
+ end
+
+ context 'when limit is higher than counf of issues being created' do
+ before do
+ stub_application_setting(issues_create_limit: 3)
+ end
+
+ it 'creates 2 issues' do
+ freeze_time do
+ expect { subject }.to change { Issue.count }.by(2)
+ end
+ end
+ end
+ end
+
context 'after_save callback to store_mentions' do
context 'when mentionable attributes change' do
let(:opts) { { title: 'Title', description: "Description with #{user.to_reference}" } }