Deduplicate findings by comparing the UUIDs

Some SAST scanners are generating findings only with CWE and WASC
identifiers which can not be used in comparison as they are type
identifiers. Therefore, we also need to check the UUIDs to make sure
there will be no duplicate findings.

Changelog: fixed
EE: true

ee/spec/lib/gitlab/ci/reports/security/finding_spec.rb

@@ -361,6 +361,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
     let(:finding) { build(:ci_reports_security_finding, identifiers: [identifier_1, identifier_2], location: location, vulnerability_finding_signatures_enabled: true, signatures: [signature]) }
     let(:expected_keys) do
       [
+        finding.uuid,
         build(:ci_reports_security_finding_key, location_fingerprint: location.fingerprint, identifier_fingerprint: identifier_1.fingerprint),
         build(:ci_reports_security_finding_key, location_fingerprint: location.fingerprint, identifier_fingerprint: identifier_2.fingerprint),
         build(:ci_reports_security_finding_key, location_fingerprint: signature.signature_hex, identifier_fingerprint: identifier_1.fingerprint),

lib/gitlab/ci/reports/security/finding.rb

@@ -126,7 +126,7 @@ module Gitlab
               location_fingerprints.map do |location_fingerprint|
                 FindingKey.new(location_fingerprint: location_fingerprint, identifier_fingerprint: identifier.fingerprint)
               end
-            end
+            end.push(uuid)
           end
 
           def primary_identifier_fingerprint

lib/gitlab/ci/reports/security/finding_key.rb

@@ -11,6 +11,8 @@ module Gitlab
           end
 
           def ==(other)
+            return false unless other.is_a?(self.class)
+
             has_fingerprints? && other.has_fingerprints? &&
               location_fingerprint == other.location_fingerprint &&
                 identifier_fingerprint == other.identifier_fingerprint

spec/lib/gitlab/ci/reports/security/finding_key_spec.rb

@@ -6,36 +6,47 @@ RSpec.describe Gitlab::Ci::Reports::Security::FindingKey do
   using RSpec::Parameterized::TableSyntax
 
   describe '#==' do
-    where(:location_fp_1, :location_fp_2, :identifier_fp_1, :identifier_fp_2, :equals?) do
-      nil           | 'different location fp' | 'identifier fp' | 'different identifier fp' | false
-      'location fp' | nil                     | 'identifier fp' | 'different identifier fp' | false
-      'location fp' | 'different location fp' | nil             | 'different identifier fp' | false
-      'location fp' | 'different location fp' | 'identifier fp' | nil                       | false
-      nil           | nil                     | 'identifier fp' | 'identifier fp'           | false
-      'location fp' | 'location fp'           | nil             | nil                       | false
-      nil           | nil                     | nil             | nil                       | false
-      'location fp' | 'different location fp' | 'identifier fp' | 'different identifier fp' | false
-      'location fp' | 'different location fp' | 'identifier fp' | 'identifier fp'           | false
-      'location fp' | 'location fp'           | 'identifier fp' | 'different identifier fp' | false
-      'location fp' | 'location fp'           | 'identifier fp' | 'identifier fp'           | true
-    end
-
-    with_them do
-      let(:finding_key_1) do
-        build(:ci_reports_security_finding_key,
-              location_fingerprint: location_fp_1,
-              identifier_fingerprint: identifier_fp_1)
+    context 'when the comparison is done between FindingKey instances' do
+      where(:location_fp_1, :location_fp_2, :identifier_fp_1, :identifier_fp_2, :equals?) do
+        nil           | 'different location fp' | 'identifier fp' | 'different identifier fp' | false
+        'location fp' | nil                     | 'identifier fp' | 'different identifier fp' | false
+        'location fp' | 'different location fp' | nil             | 'different identifier fp' | false
+        'location fp' | 'different location fp' | 'identifier fp' | nil                       | false
+        nil           | nil                     | 'identifier fp' | 'identifier fp'           | false
+        'location fp' | 'location fp'           | nil             | nil                       | false
+        nil           | nil                     | nil             | nil                       | false
+        'location fp' | 'different location fp' | 'identifier fp' | 'different identifier fp' | false
+        'location fp' | 'different location fp' | 'identifier fp' | 'identifier fp'           | false
+        'location fp' | 'location fp'           | 'identifier fp' | 'different identifier fp' | false
+        'location fp' | 'location fp'           | 'identifier fp' | 'identifier fp'           | true
       end
 
-      let(:finding_key_2) do
-        build(:ci_reports_security_finding_key,
-              location_fingerprint: location_fp_2,
-              identifier_fingerprint: identifier_fp_2)
+      with_them do
+        let(:finding_key_1) do
+          build(:ci_reports_security_finding_key,
+                location_fingerprint: location_fp_1,
+                identifier_fingerprint: identifier_fp_1)
+        end
+
+        let(:finding_key_2) do
+          build(:ci_reports_security_finding_key,
+                location_fingerprint: location_fp_2,
+                identifier_fingerprint: identifier_fp_2)
+        end
+
+        subject { finding_key_1 == finding_key_2 }
+
+        it { is_expected.to be(equals?) }
       end
+    end
+
+    context 'when the comparison is not done between FindingKey instances' do
+      let(:finding_key) { build(:ci_reports_security_finding_key) }
+      let(:uuid) { SecureRandom.uuid }
 
-      subject { finding_key_1 == finding_key_2 }
+      subject { finding_key == uuid }
 
-      it { is_expected.to be(equals?) }
+      it { is_expected.to be_falsey }
     end
   end
 end