Commit e5bd19af authored by Ethan Urie's avatar Ethan Urie

Fix result logic in SpamVerdictService#spamcheck_verdict

The logic now mirrors the logic for Akismet in #akismet_verdict.
Essentially, if Recaptcha is enabled and Spamcheck returns anything but
ALLOW, we return CONDITIONAL_ALLOW. If Recaptcha is disabled, we return
the result as-is.
parent 4457201d
......@@ -66,8 +66,9 @@ module Spam
# @TODO log if error is not nil https://gitlab.com/gitlab-org/gitlab/-/issues/329545
if Gitlab::Recaptcha.enabled? && result == CONDITIONAL_ALLOW
ALLOW
# Duplicate logic with Akismet logic in #akismet_verdict
if Gitlab::Recaptcha.enabled? && result != ALLOW
CONDITIONAL_ALLOW
else
result
end
......
......@@ -183,26 +183,45 @@ RSpec.describe Spam::SpamVerdictService do
end
end
context 'the verdict is CONDITIONAL_ALLOW' do
let(:verdict) { CONDITIONAL_ALLOW }
context 'when recaptcha is enabled' do
before do
allow(Gitlab::Recaptcha).to receive(:enabled?).and_return(true)
end
context 'when recaptcha is enabled' do
before do
allow(Recaptcha).to receive(:enabled?).and_return(true)
end
using RSpec::Parameterized::TableSyntax
it 'returns CONDITIONAL_ALLOW' do
expect(subject).to eq CONDITIONAL_ALLOW
end
# rubocop: disable Lint/BinaryOperatorWithIdenticalOperands
where(:verdict_value, :expected) do
::Spam::SpamConstants::ALLOW | ::Spam::SpamConstants::ALLOW
::Spam::SpamConstants::CONDITIONAL_ALLOW | ::Spam::SpamConstants::CONDITIONAL_ALLOW
::Spam::SpamConstants::DISALLOW | ::Spam::SpamConstants::CONDITIONAL_ALLOW
::Spam::SpamConstants::BLOCK_USER | ::Spam::SpamConstants::CONDITIONAL_ALLOW
end
# rubocop: enable Lint/BinaryOperatorWithIdenticalOperands
with_them do
let(:verdict) { verdict_value }
context 'when recaptcha is disabled' do
before do
allow(Recaptcha).to receive(:enabled?).and_return(false)
it "returns expected spam constant" do
expect(subject).to eq(expected)
end
end
end
context 'when recaptcha is disabled' do
before do
allow(Gitlab::Recaptcha).to receive(:enabled?).and_return(false)
end
[::Spam::SpamConstants::ALLOW,
::Spam::SpamConstants::CONDITIONAL_ALLOW,
::Spam::SpamConstants::DISALLOW,
::Spam::SpamConstants::BLOCK_USER].each do |verdict_value|
let(:verdict) { verdict_value }
let(:expected) { verdict_value }
it 'returns the verdict' do
expect(subject).to eq CONDITIONAL_ALLOW
it "returns expected spam constant" do
expect(subject).to eq(expected)
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment