diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2017-02-07 17:21:13 +0300 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2017-02-07 17:21:13 +0300 |
commit | 38cb0347b2ae8dc3d8bd61dd81f5f10af688562b (patch) | |
tree | f668cf3725116b72e1cdbc3c95be76a77be6b922 /spec | |
parent | 8d5ae0d99f9194b3c6c97f5f90883fe1f07298f8 (diff) | |
parent | 3d2954e4570d236a080b0d46698d96a28fd9acec (diff) |
Merge branch '21518_recaptcha_spam_issues' into 'master'
Use reCaptcha when an issue identified as spam
Closes #21518
See merge request !8846
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/issues_controller_spec.rb | 89 | ||||
-rw-r--r-- | spec/controllers/registrations_controller_spec.rb | 2 | ||||
-rw-r--r-- | spec/features/issues/spam_issues_spec.rb | 66 | ||||
-rw-r--r-- | spec/services/issues/create_service_spec.rb | 102 | ||||
-rw-r--r-- | spec/services/spam_service_spec.rb | 47 |
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 |