Use reCaptcha when an issue identified as spam

5 files changed, 289 insertions, 17 deletions

diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 5f27f336f72..4b89381eb96 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -326,7 +326,7 @@ describe Projects::IssuesController do
   end
 
   describe 'POST #create' do
-    def post_new_issue(attrs = {})
+    def post_new_issue(issue_attrs = {}, additional_params = {})
       sign_in(user)
       project = create(:empty_project, :public)
       project.team << [user, :developer]
@@ -334,8 +334,8 @@ describe Projects::IssuesController do
       post :create, {
         namespace_id: project.namespace.to_param,
         project_id: project.to_param,
-        issue: { title: 'Title', description: 'Description' }.merge(attrs)
-      }
+        issue: { title: 'Title', description: 'Description' }.merge(issue_attrs)
+      }.merge(additional_params)
 
       project.issues.first
     end
@@ -378,24 +378,81 @@ describe Projects::IssuesController do
 
     context 'Akismet is enabled' do
       before do
+        stub_application_setting(recaptcha_enabled: true)
         allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true)
-        allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
       end
 
-      def post_spam_issue
-        post_new_issue(title: 'Spam Title', description: 'Spam lives here')
-      end
+      context 'when an issue is not identified as a spam' do
+        before do
+          allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false)
+          allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(false)
+        end
 
-      it 'rejects an issue recognized as spam' do
-        expect{ post_spam_issue }.not_to change(Issue, :count)
-        expect(response).to render_template(:new)
+        it 'does not create an issue' do
+          expect { post_new_issue(title: '') }.not_to change(Issue, :count)
+        end
       end
 
-      it 'creates a spam log' do
-        post_spam_issue
-        spam_logs = SpamLog.all
-        expect(spam_logs.count).to eq(1)
-        expect(spam_logs[0].title).to eq('Spam Title')
+      context 'when an issue is identified as a spam' do
+        before { allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) }
+
+        context 'when captcha is not verified' do
+          def post_spam_issue
+            post_new_issue(title: 'Spam Title', description: 'Spam lives here')
+          end
+
+          before { allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) }
+
+          it 'rejects an issue recognized as a spam' do
+            expect { post_spam_issue }.not_to change(Issue, :count)
+          end
+
+          it 'creates a spam log' do
+            post_spam_issue
+            spam_logs = SpamLog.all
+
+            expect(spam_logs.count).to eq(1)
+            expect(spam_logs.first.title).to eq('Spam Title')
+            expect(spam_logs.first.recaptcha_verified).to be_falsey
+          end
+
+          it 'does not create an issue when it is not valid' do
+            expect { post_new_issue(title: '') }.not_to change(Issue, :count)
+          end
+
+          it 'does not create an issue when recaptcha is not enabled' do
+            stub_application_setting(recaptcha_enabled: false)
+
+            expect { post_spam_issue }.not_to change(Issue, :count)
+          end
+        end
+
+        context 'when captcha is verified' do
+          let!(:spam_logs) { create_list(:spam_log, 2, user: user, title: 'Title') }
+
+          def post_verified_issue
+            post_new_issue({}, { spam_log_id: spam_logs.last.id, recaptcha_verification: true } )
+          end
+
+          before do
+            allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(true)
+          end
+
+          it 'accepts an issue after recaptcha is verified' do
+            expect { post_verified_issue }.to change(Issue, :count)
+          end
+
+          it 'marks spam log as recaptcha_verified' do
+            expect { post_verified_issue }.to change { SpamLog.last.recaptcha_verified }.from(false).to(true)
+          end
+
+          it 'does not mark spam log as recaptcha_verified when it does not belong to current_user' do
+            spam_log = create(:spam_log)
+
+            expect { post_new_issue({}, { spam_log_id: spam_log.id, recaptcha_verification: true } ) }.
+              not_to change { SpamLog.last.recaptcha_verified }
+          end
+        end
       end
     end
 
@@ -405,7 +462,7 @@ describe Projects::IssuesController do
       end
 
       it 'creates a user agent detail' do
-        expect{ post_new_issue }.to change(UserAgentDetail, :count).by(1)
+        expect { post_new_issue }.to change(UserAgentDetail, :count).by(1)
       end
     end
 
diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index 42fbfe89368..8cc216445eb 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -44,7 +44,7 @@ describe RegistrationsController do
         post(:create, user_params)
 
         expect(response).to render_template(:new)
-        expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please re-solve the reCAPTCHA.'
+        expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
       end
 
       it 'redirects to the dashboard when the recaptcha is solved' do
diff --git a/spec/features/issues/spam_issues_spec.rb b/spec/features/issues/spam_issues_spec.rb
new file mode 100644
index 00000000000..4bc9b49f889
--- /dev/null
+++ b/spec/features/issues/spam_issues_spec.rb
@@ -0,0 +1,66 @@
+require 'rails_helper'
+
+describe 'New issue', feature: true do
+  include StubENV
+
+  let(:project) { create(:project, :public) }
+  let(:user)    { create(:user)}
+
+  before do
+    stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+
+    current_application_settings.update!(
+      akismet_enabled: true,
+      akismet_api_key: 'testkey',
+      recaptcha_enabled: true,
+      recaptcha_site_key: 'test site key',
+      recaptcha_private_key: 'test private key'
+    )
+
+    project.team << [user, :master]
+    login_as(user)
+  end
+
+  context 'when identified as a spam' do
+    before do
+      WebMock.stub_request(:any, /.*akismet.com.*/).to_return(body: "true", status: 200)
+
+      visit new_namespace_project_issue_path(project.namespace, project)
+    end
+
+    it 'creates an issue after solving reCaptcha' do
+      fill_in 'issue_title', with: 'issue title'
+      fill_in 'issue_description', with: 'issue description'
+
+      click_button 'Submit issue'
+
+      # it is impossible to test recaptcha automatically and there is no possibility to fill in recaptcha
+      # recaptcha verification is skipped in test environment and it always returns true
+      expect(page).not_to have_content('issue title')
+      expect(page).to have_css('.recaptcha')
+
+      click_button 'Submit issue'
+
+      expect(page.find('.issue-details h2.title')).to have_content('issue title')
+      expect(page.find('.issue-details .description')).to have_content('issue description')
+    end
+  end
+
+  context 'when not identified as a spam' do
+    before do
+      WebMock.stub_request(:any, /.*akismet.com.*/).to_return(body: 'false', status: 200)
+
+      visit new_namespace_project_issue_path(project.namespace, project)
+    end
+
+    it 'creates an issue' do
+      fill_in 'issue_title', with: 'issue title'
+      fill_in 'issue_description', with: 'issue description'
+
+      click_button 'Submit issue'
+
+      expect(page.find('.issue-details h2.title')).to have_content('issue title')
+      expect(page.find('.issue-details .description')).to have_content('issue description')
+    end
+  end
+end
diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb
index ac3834c32ff..30578ee4c7d 100644
--- a/spec/services/issues/create_service_spec.rb
+++ b/spec/services/issues/create_service_spec.rb
@@ -181,5 +181,107 @@ describe Issues::CreateService, services: true do
         expect(issue.title).to be_nil
       end
     end
+
+    context 'checking spam' do
+      let(:opts) do
+        {
+          title: 'Awesome issue',
+          description: 'please fix',
+          request: double(:request, env: {})
+        }
+      end
+
+      before do
+        allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true)
+      end
+
+      context 'when recaptcha was verified' do
+        let(:log_user)  { user }
+        let(:spam_logs) { create_list(:spam_log, 2, user: log_user, title: 'Awesome issue') }
+
+        before do
+          opts[:recaptcha_verified] = true
+          opts[:spam_log_id]        = spam_logs.last.id
+
+          expect(AkismetService).not_to receive(:new)
+        end
+
+        it 'does no mark an issue as a spam ' do
+          expect(issue).not_to be_spam
+        end
+
+        it 'an issue is valid ' do
+          expect(issue.valid?).to be_truthy
+        end
+
+        it 'does not assign a spam_log to an issue' do
+          expect(issue.spam_log).to be_nil
+        end
+
+        it 'marks related spam_log as recaptcha_verified' do
+          expect { issue }.to change{SpamLog.last.recaptcha_verified}.from(false).to(true)
+        end
+
+        context 'when spam log does not belong to a user' do
+          let(:log_user) { create(:user) }
+
+          it 'does not mark spam_log as recaptcha_verified' do
+            expect { issue }.not_to change{SpamLog.last.recaptcha_verified}
+          end
+        end
+
+        context 'when spam log title does not match the issue title' do
+          before do
+            opts[:title] = 'Another issue'
+          end
+
+          it 'does not mark spam_log as recaptcha_verified' do
+            expect { issue }.not_to change{SpamLog.last.recaptcha_verified}
+          end
+        end
+      end
+
+      context 'when recaptcha was not verified' do
+        context 'when akismet detects spam' do
+          before do
+            allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
+          end
+
+          it 'marks an issue as a spam ' do
+            expect(issue).to be_spam
+          end
+
+          it 'an issue is not valid ' do
+            expect(issue.valid?).to be_falsey
+          end
+
+          it 'creates a new spam_log' do
+            expect{issue}.to change{SpamLog.count}.from(0).to(1)
+          end
+
+          it 'assigns a spam_log to an issue' do
+            expect(issue.spam_log).to eq(SpamLog.last)
+          end
+        end
+
+        context 'when akismet does not detect spam' do
+          before do
+            allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(false)
+          end
+
+          it 'does not mark an issue as a spam ' do
+            expect(issue).not_to be_spam
+          end
+
+          it 'an issue is valid ' do
+            expect(issue.valid?).to be_truthy
+          end
+
+          it 'does not assign a spam_log to an issue' do
+            expect(issue.spam_log).to be_nil
+          end
+        end
+      end
+    end
   end
 end
diff --git a/spec/services/spam_service_spec.rb b/spec/services/spam_service_spec.rb
new file mode 100644
index 00000000000..271c17dd8c0
--- /dev/null
+++ b/spec/services/spam_service_spec.rb
@@ -0,0 +1,47 @@
+require 'spec_helper'
+
+describe SpamService, services: true do
+  describe '#check' do
+    let(:project) { create(:project, :public) }
+    let(:issue)   { create(:issue, project: project) }
+    let(:request) { double(:request, env: {}) }
+
+    def check_spam(issue, request)
+      described_class.new(issue, request).check
+    end
+
+    context 'when indicated as spam by akismet' do
+      before { allow(AkismetService).to receive(:new).and_return(double(is_spam?: true)) }
+
+      it 'returns false when request is missing' do
+        expect(check_spam(issue, nil)).to be_falsey
+      end
+
+      it 'returns false when issue is not public' do
+        issue = create(:issue, project: create(:project, :private))
+
+        expect(check_spam(issue, request)).to be_falsey
+      end
+
+      it 'returns true' do
+        expect(check_spam(issue, request)).to be_truthy
+      end
+
+      it 'creates a spam log' do
+        expect { check_spam(issue, request) }.to change { SpamLog.count }.from(0).to(1)
+      end
+    end
+
+    context 'when not indicated as spam by akismet' do
+      before { allow(AkismetService).to receive(:new).and_return(double(is_spam?: false)) }
+
+      it 'returns false' do
+        expect(check_spam(issue, request)).to be_falsey
+      end
+
+      it 'does not create a spam log' do
+        expect { check_spam(issue, request) }.not_to change { SpamLog.count }
+      end
+    end
+  end
+end