Merge branch 'jwanjohi-project-allowlist' into 'master'

Add project fields to Spamcheck client message & create verdict NOOP [RUN ALL RSPEC][RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!60902

Gemfile

@@ -478,7 +478,7 @@ group :ed25519 do
 end
 
 # Spamcheck GRPC protocol definitions
-gem 'spamcheck', '~> 0.0.5'
+gem 'spamcheck', '~> 0.1.0'
 
 # Gitaly GRPC protocol definitions
 gem 'gitaly', '~> 13.12.0.pre.rc1'

Gemfile.lock

@@ -519,8 +519,8 @@ GEM
     google-cloud-env (1.4.0)
       faraday (>= 0.17.3, < 2.0)
     google-protobuf (3.14.0)
-    googleapis-common-protos-types (1.0.5)
-      google-protobuf (~> 3.11)
+    googleapis-common-protos-types (1.0.6)
+      google-protobuf (~> 3.14)
     googleauth (0.14.0)
       faraday (>= 0.17.3, < 2.0)
       jwt (>= 1.4, < 3.0)
@@ -1211,7 +1211,7 @@ GEM
       thor (~> 1.0)
       tilt (~> 2.0)
       yard (~> 0.9, >= 0.9.24)
-    spamcheck (0.0.5)
+    spamcheck (0.1.0)
       grpc (~> 1.0)
     spring (2.1.1)
     spring-commands-rspec (1.0.4)
@@ -1608,7 +1608,7 @@ DEPENDENCIES
   slack-messenger (~> 2.3.4)
   snowplow-tracker (~> 0.6.1)
   solargraph (~> 0.40.4)
-  spamcheck (~> 0.0.5)
+  spamcheck (~> 0.1.0)
   spring (~> 2.1.0)
   spring-commands-rspec (~> 1.0.4)
   sprockets (~> 3.7.0)

app/services/spam/spam_action_service.rb

@@ -130,6 +130,9 @@ module Spam
           create_spam_log(api)
         when ALLOW
           target.clear_spam_flags!
+        when NOOP
+          # spamcheck is not explicitly rendering a verdict & therefore can't make a decision
+          target.clear_spam_flags!
         end
       end
     end

app/services/spam/spam_constants.rb

@@ -6,6 +6,7 @@ module Spam
     DISALLOW = "disallow"
     ALLOW = "allow"
     BLOCK_USER = "block"
+    NOOP = "noop"
 
     SUPPORTED_VERDICTS = {
       BLOCK_USER => {
@@ -19,6 +20,9 @@ module Spam
       },
       ALLOW => {
         priority: 4
+      },
+      NOOP => {
+        priority: 5
       }
     }.freeze
   end

lib/gitlab/spamcheck/client.rb

@@ -11,7 +11,8 @@ module Gitlab
         ::Spamcheck::SpamVerdict::Verdict::ALLOW => ALLOW,
         ::Spamcheck::SpamVerdict::Verdict::CONDITIONAL_ALLOW => CONDITIONAL_ALLOW,
         ::Spamcheck::SpamVerdict::Verdict::DISALLOW => DISALLOW,
-        ::Spamcheck::SpamVerdict::Verdict::BLOCK => BLOCK_USER
+        ::Spamcheck::SpamVerdict::Verdict::BLOCK => BLOCK_USER,
+        ::Spamcheck::SpamVerdict::Verdict::NOOP => NOOP
       }.freeze
 
       ACTION_MAPPING = {
@@ -60,6 +61,7 @@ module Gitlab
         issue_pb.created_at = convert_to_pb_timestamp(issue.created_at) if issue.created_at
         issue_pb.updated_at = convert_to_pb_timestamp(issue.updated_at) if issue.updated_at
         issue_pb.user_in_project = user.authorized_project?(issue.project)
+        issue_pb.project = build_project_protobuf(issue)
         issue_pb.action = ACTION_MAPPING.fetch(context.fetch(:action)) if context.has_key?(:action)
         issue_pb.user = build_user_protobuf(user)
         issue_pb
@@ -87,6 +89,13 @@ module Gitlab
         email_pb
       end
 
+      def build_project_protobuf(issue)
+        project_pb = ::Spamcheck::Project.new
+        project_pb.project_id = issue.project_id
+        project_pb.project_path = issue.project.full_path
+        project_pb
+      end
+
       def convert_to_pb_timestamp(ar_timestamp)
         Google::Protobuf::Timestamp.new(seconds: ar_timestamp.to_time.to_i,
                                         nanos: ar_timestamp.to_time.nsec)

spec/lib/gitlab/spamcheck/client_spec.rb

@@ -38,6 +38,7 @@ RSpec.describe Gitlab::Spamcheck::Client do
       ::Spamcheck::SpamVerdict::Verdict::CONDITIONAL_ALLOW    | Spam::SpamConstants::CONDITIONAL_ALLOW
       ::Spamcheck::SpamVerdict::Verdict::DISALLOW             | Spam::SpamConstants::DISALLOW
       ::Spamcheck::SpamVerdict::Verdict::BLOCK                | Spam::SpamConstants::BLOCK_USER
+      ::Spamcheck::SpamVerdict::Verdict::NOOP                 | Spam::SpamConstants::NOOP
     end
 
     with_them do
@@ -58,6 +59,7 @@ RSpec.describe Gitlab::Spamcheck::Client do
       expect(issue_pb.title).to eq issue.title
       expect(issue_pb.description).to eq issue.description
       expect(issue_pb.user_in_project). to be false
+      expect(issue_pb.project.project_id).to eq issue.project_id
       expect(issue_pb.created_at).to eq timestamp_to_protobuf_timestamp(issue.created_at)
       expect(issue_pb.updated_at).to eq timestamp_to_protobuf_timestamp(issue.updated_at)
       expect(issue_pb.action).to be ::Spamcheck::Action.lookup(::Spamcheck::Action::CREATE)
@@ -83,7 +85,7 @@ RSpec.describe Gitlab::Spamcheck::Client do
         user.emails << secondary_email
       end
 
-      it 'adds emsils to the user pb object' do
+      it 'adds emails to the user pb object' do
         user_pb = described_class.new.send(:build_user_protobuf, user)
         expect(user_pb.emails.count).to eq 2
         expect(user_pb.emails.first.email).to eq user.email
@@ -94,6 +96,14 @@ RSpec.describe Gitlab::Spamcheck::Client do
     end
   end
 
+  describe "#build_project_protobuf", :aggregate_failures do
+    it 'builds the expected protobuf object' do
+      project_pb = described_class.new.send(:build_project_protobuf, issue)
+      expect(project_pb.project_id).to eq issue.project_id
+      expect(project_pb.project_path).to eq issue.project.full_path
+    end
+  end
+
   private
 
   def timestamp_to_protobuf_timestamp(timestamp)

spec/services/spam/spam_action_service_spec.rb

@@ -313,6 +313,22 @@ RSpec.describe Spam::SpamActionService do
           end
         end
 
+        context 'when spam verdict service returns noop' do
+          before do
+            allow(fake_verdict_service).to receive(:execute).and_return(NOOP)
+          end
+
+          it 'does not create a spam log' do
+            expect { subject }.not_to change(SpamLog, :count)
+          end
+
+          it 'clears spam flags' do
+            expect(issue).to receive(:clear_spam_flags!)
+
+            subject
+          end
+        end
+
         context 'with spam verdict service options' do
           before do
             allow(fake_verdict_service).to receive(:execute).and_return(ALLOW)

spec/support/shared_contexts/spam_constants.rb

@@ -6,5 +6,6 @@ RSpec.shared_context 'includes Spam constants' do
     stub_const('DISALLOW', Spam::SpamConstants::DISALLOW)
     stub_const('ALLOW', Spam::SpamConstants::ALLOW)
     stub_const('BLOCK_USER', Spam::SpamConstants::BLOCK_USER)
+    stub_const('NOOP', Spam::SpamConstants::NOOP)
   end
 end