diff options
Diffstat (limited to 'spec/services/spam')
-rw-r--r-- | spec/services/spam/spam_action_service_spec.rb (renamed from spec/services/spam/spam_check_service_spec.rb) | 87 | ||||
-rw-r--r-- | spec/services/spam/spam_verdict_service_spec.rb | 65 |
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 |