From 6be28d29808911eddfa933978f44b366814d24e9 Mon Sep 17 00:00:00 2001
From: Mehmet Emin INAC <minac@gitlab.com>
Date: Wed, 21 Dec 2022 20:47:25 +0000
Subject: [PATCH] Fix creating JIRA issue URL for security findings

With this change, we can create JIRA issue URL for an instance of
`Vulnerability`, `Vulnerabilities::Finding`, or `Security::Finding`.

Changelog: fixed
EE: true
---
 ee/app/helpers/vulnerabilities_helper.rb      |  5 +++
 ee/app/models/security/scan.rb                |  1 +
 .../presenters/security/finding_presenter.rb  |  7 ++++
 .../json_schemas/security_finding_data.json   | 29 +++++++++++++++
 .../jira_issue_description.md.erb             |  2 ++
 ee/spec/factories/security/findings.rb        | 17 +++++++++
 .../helpers/vulnerabilities_helper_spec.rb    | 35 ++++++++++++++++++-
 7 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100644 ee/app/presenters/security/finding_presenter.rb

diff --git a/ee/app/helpers/vulnerabilities_helper.rb b/ee/app/helpers/vulnerabilities_helper.rb
index beda683167b27..dc414c710cc01 100644
--- a/ee/app/helpers/vulnerabilities_helper.rb
+++ b/ee/app/helpers/vulnerabilities_helper.rb
@@ -49,6 +49,11 @@ def new_issue_url_for(vulnerability)
     new_project_issue_path(vulnerability.project, { vulnerability_id: vulnerability.id })
   end
 
+  # This method can be called with an instance of the following models;
+  # - Vulnerability
+  # - Vulnerabilities::Finding
+  # - Security::Finding
+  #
   def create_jira_issue_url_for(vulnerability)
     return unless vulnerability.project.configured_to_create_issues_from_vulnerabilities?
 
diff --git a/ee/app/models/security/scan.rb b/ee/app/models/security/scan.rb
index 756573f56a2fc..7c16a0271a0fd 100644
--- a/ee/app/models/security/scan.rb
+++ b/ee/app/models/security/scan.rb
@@ -54,6 +54,7 @@ class Scan < ApplicationRecord
     scope :with_errors, -> { where("jsonb_array_length(COALESCE(info->'errors', '[]'::jsonb)) > 0") }
 
     delegate :name, to: :build
+    alias_attribute :type, :scan_type
 
     before_save :ensure_project_id_pipeline_id
 
diff --git a/ee/app/presenters/security/finding_presenter.rb b/ee/app/presenters/security/finding_presenter.rb
new file mode 100644
index 0000000000000..8b3cd4180d3e2
--- /dev/null
+++ b/ee/app/presenters/security/finding_presenter.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module Security
+  class FindingPresenter < Vulnerabilities::FindingPresenter
+    presents ::Security::Finding, as: :finding
+  end
+end
diff --git a/ee/app/validators/json_schemas/security_finding_data.json b/ee/app/validators/json_schemas/security_finding_data.json
index 2888cec99be79..eb7b1c82b16c0 100644
--- a/ee/app/validators/json_schemas/security_finding_data.json
+++ b/ee/app/validators/json_schemas/security_finding_data.json
@@ -5,6 +5,35 @@
   "description": "The schema validates the content of the Security::Finding#finding_data attribute",
   "additionalProperties": false,
   "properties": {
+    "name": {
+      "type": "string"
+    },
+    "description": {
+      "type": "string"
+    },
+    "solution": {
+      "type": "string"
+    },
+    "location": {
+      "type": "object"
+    },
+    "identifiers": {
+      "type": "array"
+    },
+    "links": {
+      "type": "array",
+      "items": {
+        "type": "object",
+        "properties": {
+          "name": {
+            "type": "string"
+          },
+          "url": {
+            "type": "string"
+          }
+        }
+      }
+    },
     "remediation_byte_offsets": {
       "type": "array",
       "items": {
diff --git a/ee/app/views/vulnerabilities/jira_issue_description.md.erb b/ee/app/views/vulnerabilities/jira_issue_description.md.erb
index 83de3ce4581fa..1d792dc56aa6b 100644
--- a/ee/app/views/vulnerabilities/jira_issue_description.md.erb
+++ b/ee/app/views/vulnerabilities/jira_issue_description.md.erb
@@ -67,6 +67,7 @@ h3. <%= _("Scanner") %>:
 <% if scan_data.type.present? %>
 * <%= _("Type") %>: <%= scan_data.type %>
 <% end %>
+<% unless vulnerability.is_a?(Security::Finding) %>
 <% if scan_data.status.present? %>
 * <%= _("Status") %>: <%= scan_data.status %>
 <% end %>
@@ -78,3 +79,4 @@ h3. <%= _("Scanner") %>:
 <% end %>
 <% end %>
 <% end %>
+<% end %>
diff --git a/ee/spec/factories/security/findings.rb b/ee/spec/factories/security/findings.rb
index fe8968a2e2592..8a157a8d3fa26 100644
--- a/ee/spec/factories/security/findings.rb
+++ b/ee/spec/factories/security/findings.rb
@@ -9,5 +9,22 @@
     confidence { :high }
     uuid { SecureRandom.uuid }
     project_fingerprint { generate(:project_fingerprint) }
+
+    trait :with_finding_data do
+      finding_data do
+        {
+          name: 'Test finding',
+          description: 'The cipher does not provide data integrity update 1',
+          solution: 'foo',
+          identifiers: [],
+          links: [
+            {
+              name: 'Cipher does not check for integrity first?',
+              url: 'https://crypto.stackexchange.com/questions/31428/pbewithmd5anddes-cipher-does-not-check-for-integrity-first'
+            }
+          ]
+        }
+      end
+    end
   end
 end
diff --git a/ee/spec/helpers/vulnerabilities_helper_spec.rb b/ee/spec/helpers/vulnerabilities_helper_spec.rb
index 4b72aaae93a18..05c54dd878de7 100644
--- a/ee/spec/helpers/vulnerabilities_helper_spec.rb
+++ b/ee/spec/helpers/vulnerabilities_helper_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe VulnerabilitiesHelper do
+RSpec.describe VulnerabilitiesHelper, feature_category: :vulnerability_management do
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project, :repository, :public) }
   let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project) }
@@ -328,6 +328,39 @@
           subject
         end
       end
+
+      context 'when the given object is a Security::Finding' do
+        let(:pipeline) { create(:ci_pipeline, project: project) }
+        let(:scan) { create(:security_scan, pipeline: pipeline) }
+        let(:vulnerability) { create(:security_finding, :with_finding_data, scan: scan) }
+        let(:expected_jira_issue_description) do
+          <<~TEXT
+            h3. Description:
+
+            The cipher does not provide data integrity update 1
+
+            * Severity: critical
+            * Confidence: high
+
+
+            h3. Links:
+
+            * [Cipher does not check for integrity first?|https://crypto.stackexchange.com/questions/31428/pbewithmd5anddes-cipher-does-not-check-for-integrity-first]
+
+
+            h3. Scanner:
+
+            * Name: Find Security Bugs
+            * Type: dast
+          TEXT
+        end
+
+        it 'delegates rendering URL to Integrations::Jira' do
+          expect(jira_integration).to receive(:new_issue_url_with_predefined_fields).with("Investigate vulnerability: #{vulnerability.name}", expected_jira_issue_description)
+
+          subject
+        end
+      end
     end
 
     context 'with jira vulnerabilities integration disabled' do
-- 
GitLab