Add latest changes from gitlab-org/gitlab@13-0-stable-ee

2 files changed, 131 insertions, 21 deletions

diff --git a/spec/services/spam/spam_check_service_spec.rb b/spec/services/spam/spam_action_service_spec.rb
index 3d0cb1447bd..560833aba97 100644
--- a/spec/services/spam/spam_check_service_spec.rb
+++ b/spec/services/spam/spam_action_service_spec.rb
@@ -2,7 +2,9 @@
 
 require 'spec_helper'
 
-describe Spam::SpamCheckService do
+describe Spam::SpamActionService do
+  include_context 'includes Spam constants'
+
   let(:fake_ip) { '1.2.3.4' }
   let(:fake_user_agent) { 'fake-user-agent' }
   let(:fake_referrer) { 'fake-http-referrer' }
@@ -15,7 +17,7 @@ describe Spam::SpamCheckService do
 
   let_it_be(:project) { create(:project, :public) }
   let_it_be(:user) { create(:user) }
-  let_it_be(:issue) { create(:issue, project: project, author: user) }
+  let(:issue) { create(:issue, project: project, author: user) }
 
   before do
     issue.spam = false
@@ -51,7 +53,7 @@ describe Spam::SpamCheckService do
 
   shared_examples 'only checks for spam if a request is provided' do
     context 'when request is missing' do
-      let(:request) { nil }
+      subject { described_class.new(spammable: issue, request: nil) }
 
       it "doesn't check as spam" do
         subject
@@ -70,21 +72,28 @@ describe Spam::SpamCheckService do
 
   describe '#execute' do
     let(:request) { double(:request, env: env) }
+    let(:fake_verdict_service) { double(:spam_verdict_service) }
+    let(:allowlisted) { false }
 
     let_it_be(:existing_spam_log) { create(:spam_log, user: user, recaptcha_verified: false) }
 
     subject do
       described_service = described_class.new(spammable: issue, request: request)
-      described_service.execute(user_id: user.id, api: nil, recaptcha_verified: recaptcha_verified, spam_log_id: existing_spam_log.id)
+      allow(described_service).to receive(:allowlisted?).and_return(allowlisted)
+      described_service.execute(user: user, api: nil, recaptcha_verified: recaptcha_verified, spam_log_id: existing_spam_log.id)
     end
 
-    context 'when recaptcha was already verified' do
+    before do
+      allow(Spam::SpamVerdictService).to receive(:new).and_return(fake_verdict_service)
+    end
+
+    context 'when reCAPTCHA was already verified' do
       let(:recaptcha_verified) { true }
 
-      it "updates spam log and doesn't check Akismet" do
+      it "doesn't check with the SpamVerdictService" do
         aggregate_failures do
-          expect(SpamLog).not_to receive(:create!)
-          expect(an_instance_of(described_class)).not_to receive(:check)
+          expect(SpamLog).to receive(:verify_recaptcha!)
+          expect(fake_verdict_service).not_to receive(:execute)
         end
 
         subject
@@ -95,18 +104,12 @@ describe Spam::SpamCheckService do
       end
     end
 
-    context 'when recaptcha was not verified' do
+    context 'when reCAPTCHA was not verified' do
       let(:recaptcha_verified) { false }
 
       context 'when spammable attributes have not changed' do
         before do
           issue.closed_at = Time.zone.now
-
-          allow(Spam::AkismetService).to receive(:new).and_return(double(spam?: true))
-        end
-
-        it 'returns false' do
-          expect(subject).to be_falsey
         end
 
         it 'does not create a spam log' do
@@ -120,9 +123,19 @@ describe Spam::SpamCheckService do
           issue.description = 'SPAM!'
         end
 
-        context 'when indicated as spam by Akismet' do
+        context 'if allowlisted' do
+          let(:allowlisted) { true }
+
+          it 'does not perform spam check' do
+            expect(Spam::SpamVerdictService).not_to receive(:new)
+
+            subject
+          end
+        end
+
+        context 'when disallowed by the spam verdict service' do
           before do
-            allow(Spam::AkismetService).to receive(:new).and_return(double(spam?: true))
+            allow(fake_verdict_service).to receive(:execute).and_return(DISALLOW)
           end
 
           context 'when allow_possible_spam feature flag is false' do
@@ -150,13 +163,45 @@ describe Spam::SpamCheckService do
           end
         end
 
-        context 'when not indicated as spam by Akismet' do
+        context 'when spam verdict service requires reCAPTCHA' do
           before do
-            allow(Spam::AkismetService).to receive(:new).and_return(double(spam?: false))
+            allow(fake_verdict_service).to receive(:execute).and_return(REQUIRE_RECAPTCHA)
+          end
+
+          context 'when allow_possible_spam feature flag is false' do
+            before do
+              stub_feature_flags(allow_possible_spam: false)
+            end
+
+            it_behaves_like 'only checks for spam if a request is provided'
+
+            it 'does not mark as spam' do
+              subject
+
+              expect(issue).not_to be_spam
+            end
+
+            it 'marks as needing reCAPTCHA' do
+              subject
+
+              expect(issue.needs_recaptcha?).to be_truthy
+            end
           end
 
-          it 'returns false' do
-            expect(subject).to be_falsey
+          context 'when allow_possible_spam feature flag is true' do
+            it_behaves_like 'only checks for spam if a request is provided'
+
+            it 'does not mark as needing reCAPTCHA' do
+              subject
+
+              expect(issue.needs_recaptcha).to be_falsey
+            end
+          end
+        end
+
+        context 'when spam verdict service allows creation' do
+          before do
+            allow(fake_verdict_service).to receive(:execute).and_return(ALLOW)
           end
 
           it 'does not create a spam log' do
diff --git a/spec/services/spam/spam_verdict_service_spec.rb b/spec/services/spam/spam_verdict_service_spec.rb
new file mode 100644
index 00000000000..93460a5e7d7
--- /dev/null
+++ b/spec/services/spam/spam_verdict_service_spec.rb
@@ -0,0 +1,65 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Spam::SpamVerdictService do
+  include_context 'includes Spam constants'
+
+  let(:fake_ip) { '1.2.3.4' }
+  let(:fake_user_agent) { 'fake-user-agent' }
+  let(:fake_referrer) { 'fake-http-referrer' }
+  let(:env) do
+    { 'action_dispatch.remote_ip' => fake_ip,
+      'HTTP_USER_AGENT' => fake_user_agent,
+      'HTTP_REFERRER' => fake_referrer }
+  end
+  let(:request) { double(:request, env: env) }
+
+  let(:check_for_spam) { true }
+  let(:issue) { build(:issue) }
+  let(:service) do
+    described_class.new(target: issue, request: request, options: {})
+  end
+
+  describe '#execute' do
+    subject { service.execute }
+
+    before do
+      allow_next_instance_of(Spam::AkismetService) do |service|
+        allow(service).to receive(:spam?).and_return(spam_verdict)
+      end
+    end
+
+    context 'if Akismet considers it spam' do
+      let(:spam_verdict) { true }
+
+      context 'if reCAPTCHA is enabled' do
+        before do
+          stub_application_setting(recaptcha_enabled: true)
+        end
+
+        it 'requires reCAPTCHA' do
+          expect(subject).to eq REQUIRE_RECAPTCHA
+        end
+      end
+
+      context 'if reCAPTCHA is not enabled' do
+        before do
+          stub_application_setting(recaptcha_enabled: false)
+        end
+
+        it 'disallows the change' do
+          expect(subject).to eq DISALLOW
+        end
+      end
+    end
+
+    context 'if Akismet does not consider it spam' do
+      let(:spam_verdict) { false }
+
+      it 'allows the change' do
+        expect(subject).to eq ALLOW
+      end
+    end
+  end
+end