Merge branch 'expose-blob-path-for-group-security-dashboard' into 'master'

Expose blob_path for vulnerability occurrences

See merge request gitlab-org/gitlab-ee!10602

ee/app/controllers/groups/security/vulnerabilities_controller.rb

@@ -4,7 +4,7 @@ class Groups::Security::VulnerabilitiesController < Groups::Security::Applicatio
   HISTORY_RANGE = 3.months
 
   def index
-    vulnerabilities = found_vulnerabilities.ordered.page(params[:page])
+    vulnerabilities = found_vulnerabilities(:with_sha).ordered.page(params[:page])
 
     respond_to do |format|
       format.json do

ee/app/finders/security/vulnerabilities_finder.rb

@@ -54,7 +54,13 @@ module Security
     end
 
     def init_collection(scope)
-      scope == :all ? group.all_vulnerabilities : group.latest_vulnerabilities
+      if scope == :all
+        group.all_vulnerabilities
+      elsif scope == :with_sha
+        group.latest_vulnerabilities_with_sha
+      else
+        group.latest_vulnerabilities
+      end
     end
   end
 end

ee/app/models/ee/group.rb

@@ -111,6 +111,11 @@ module EE
         .for_pipelines(all_pipelines.with_vulnerabilities.latest_successful_ids_per_project)
     end
 
+    def latest_vulnerabilities_with_sha
+      Vulnerabilities::Occurrence
+        .for_pipelines_with_sha(all_pipelines.with_vulnerabilities.latest_successful_ids_per_project)
+    end
+
     def all_vulnerabilities
       Vulnerabilities::Occurrence
         .for_pipelines(all_pipelines.with_vulnerabilities.success)

ee/app/models/vulnerabilities/occurrence.rb

@@ -4,6 +4,7 @@ module Vulnerabilities
   class Occurrence < ApplicationRecord
     include ShaAttribute
     include ::Gitlab::Utils::StrongMemoize
+    include Presentable
 
     self.table_name = "vulnerability_occurrences"
 
@@ -83,6 +84,12 @@ module Vulnerabilities
       preload(:scanner, :identifiers, project: [:namespace, :project_feature])
     end
 
+    def self.for_pipelines_with_sha(pipelines)
+      for_pipelines(pipelines)
+        .joins(:pipelines)
+        .select("vulnerability_occurrences.*, ci_pipelines.sha")
+    end
+
     def self.for_pipelines(pipelines)
       joins(:occurrence_pipelines)
         .where(vulnerability_occurrence_pipelines: { pipeline_id: pipelines })

ee/app/presenters/vulnerabilities/occurrence_presenter.rb

+# frozen_string_literal: true
+
+module Vulnerabilities
+  class OccurrencePresenter < Gitlab::View::Presenter::Delegated
+    presents :occurrence
+
+    def blob_path
+      return '' unless respond_to?(:sha)
+
+      add_line_numbers(location['start_line'], location['end_line'])
+    end
+
+    private
+
+    def add_line_numbers(start_line, end_line)
+      return vulnerability_path unless start_line
+
+      vulnerability_path.tap do |complete_path|
+        complete_path << "#L#{start_line}"
+        complete_path << "-#{end_line}" if end_line
+      end
+    end
+
+    def vulnerability_path
+      @vulnerability_path ||= project_blob_path(project, File.join(sha, location['file']))
+    end
+  end
+end

ee/app/serializers/vulnerabilities/occurrence_entity.rb

@@ -23,6 +23,10 @@ class Vulnerabilities::OccurrenceEntity < Grape::Entity
     expose :solution
   end
 
+  expose :blob_path do |occurrence|
+    occurrence.present.blob_path
+  end
+
   alias_method :occurrence, :object
 
   private

ee/spec/models/group_spec.rb

@@ -288,7 +288,7 @@ describe Group do
     end
   end
 
-  describe '#latest_vulnerabilities' do
+  describe 'Vulnerabilities::Occurrence collection methods' do
     let(:project) { create(:project, namespace: group) }
     let(:external_project) { create(:project) }
     let(:failed_pipeline) { create(:ci_pipeline, :failed, project: project) }
@@ -298,58 +298,70 @@ describe Group do
     let!(:external_vuln) { create_vulnerability(external_project) }
     let!(:failed_vuln) { create_vulnerability(project, failed_pipeline) }
 
-    subject { group.latest_vulnerabilities }
-
     def create_vulnerability(project, pipeline = nil)
       pipeline ||= create(:ci_pipeline, :success, project: project)
       create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project)
     end
 
-    it 'returns vulns only for the latest successful pipelines of projects belonging to the group' do
-      is_expected.to contain_exactly(new_vuln)
-    end
+    describe '#latest_vulnerabilities' do
+      subject { group.latest_vulnerabilities }
+
+      it 'returns vulns only for the latest successful pipelines of projects belonging to the group' do
+        is_expected.to contain_exactly(new_vuln)
+      end
 
-    context 'with vulnerabilities from other branches' do
-      let!(:branch_pipeline) { create(:ci_pipeline, :success, project: project, ref: 'feature-x') }
-      let!(:branch_vuln) { create(:vulnerabilities_occurrence, pipelines: [branch_pipeline], project: project) }
+      context 'with vulnerabilities from other branches' do
+        let!(:branch_pipeline) { create(:ci_pipeline, :success, project: project, ref: 'feature-x') }
+        let!(:branch_vuln) { create(:vulnerabilities_occurrence, pipelines: [branch_pipeline], project: project) }
 
-      # TODO: This should actually fail and we must scope vulns
-      # per branch as soon as we store them for other branches
-      it 'includes vulnerabilities from all branches' do
-        is_expected.to contain_exactly(branch_vuln)
+        # TODO: This should actually fail and we must scope vulns
+        # per branch as soon as we store them for other branches
+        # Dependent on https://gitlab.com/gitlab-org/gitlab-ee/issues/9524
+        it 'includes vulnerabilities from all branches' do
+          is_expected.to contain_exactly(branch_vuln)
+        end
       end
     end
-  end
 
-  describe '#all_vulnerabilities' do
-    let(:project) { create(:project, namespace: group) }
-    let(:external_project) { create(:project) }
-    let(:failed_pipeline) { create(:ci_pipeline, :failed, project: project) }
+    describe '#latest_vulnerabilities_with_sha' do
+      subject { group.latest_vulnerabilities_with_sha }
 
-    let!(:old_vuln) { create_vulnerability(project) }
-    let!(:new_vuln) { create_vulnerability(project) }
-    let!(:external_vuln) { create_vulnerability(external_project) }
-    let!(:failed_vuln) { create_vulnerability(project, failed_pipeline) }
+      it 'returns vulns only for the latest successful pipelines of projects belonging to the group' do
+        is_expected.to contain_exactly(new_vuln)
+      end
 
-    subject { group.all_vulnerabilities }
+      it { is_expected.to all(respond_to(:sha)) }
 
-    def create_vulnerability(project, pipeline = nil)
-      pipeline ||= create(:ci_pipeline, :success, project: project)
-      create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project)
-    end
+      context 'with vulnerabilities from other branches' do
+        let!(:branch_pipeline) { create(:ci_pipeline, :success, project: project, ref: 'feature-x') }
+        let!(:branch_vuln) { create(:vulnerabilities_occurrence, pipelines: [branch_pipeline], project: project) }
 
-    it 'returns vulns for all successful pipelines of projects belonging to the group' do
-      is_expected.to contain_exactly(old_vuln, new_vuln)
+        # TODO: This should actually fail and we must scope vulns
+        # per branch as soon as we store them for other branches
+        # Dependent on https://gitlab.com/gitlab-org/gitlab-ee/issues/9524
+        it 'includes vulnerabilities from all branches' do
+          is_expected.to contain_exactly(branch_vuln)
+        end
+      end
     end
 
-    context 'with vulnerabilities from other branches' do
-      let!(:branch_pipeline) { create(:ci_pipeline, :success, project: project, ref: 'feature-x') }
-      let!(:branch_vuln) { create(:vulnerabilities_occurrence, pipelines: [branch_pipeline], project: project) }
+    describe '#all_vulnerabilities' do
+      subject { group.all_vulnerabilities }
+
+      it 'returns vulns for all successful pipelines of projects belonging to the group' do
+        is_expected.to contain_exactly(old_vuln, new_vuln)
+      end
+
+      context 'with vulnerabilities from other branches' do
+        let!(:branch_pipeline) { create(:ci_pipeline, :success, project: project, ref: 'feature-x') }
+        let!(:branch_vuln) { create(:vulnerabilities_occurrence, pipelines: [branch_pipeline], project: project) }
 
-      # TODO: This should actually fail and we must scope vulns
-      # per branch as soon as we store them for other branches
-      it 'includes vulnerabilities from all branches' do
-        is_expected.to contain_exactly(old_vuln, new_vuln, branch_vuln)
+        # TODO: This should actually fail and we must scope vulns
+        # per branch as soon as we store them for other branches
+        # Dependent on https://gitlab.com/gitlab-org/gitlab-ee/issues/9524
+        it 'includes vulnerabilities from all branches' do
+          is_expected.to contain_exactly(old_vuln, new_vuln, branch_vuln)
+        end
       end
     end
   end

ee/spec/models/vulnerabilities/occurrence_spec.rb

@@ -82,6 +82,21 @@ describe Vulnerabilities::Occurrence do
     end
   end
 
+  describe '.for_pipelines_with_sha' do
+    let(:project) { create(:project) }
+    let(:pipeline) { create(:ci_pipeline, :success, project: project) }
+
+    before do
+      create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project)
+    end
+
+    subject(:occurrences) { described_class.for_pipelines_with_sha([pipeline]) }
+
+    it 'sets the sha' do
+      expect(occurrences.first.sha).to eq(pipeline.sha)
+    end
+  end
+
   describe '.count_by_day_and_severity' do
     let(:project) { create(:project) }
     let(:date_1) { Time.zone.parse('2018-11-11') }

ee/spec/presenters/vulnerabilities/occurrence_presenter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Vulnerabilities::OccurrencePresenter do
+  let(:presenter) { described_class.new(occurrence) }
+  let(:occurrence) { build_stubbed(:vulnerabilities_occurrence) }
+
+  describe '#blob_path' do
+    subject { presenter.blob_path }
+
+    context 'without a sha' do
+      it { is_expected.to be_blank }
+    end
+
+    context 'with a sha' do
+      before do
+        allow_any_instance_of(Vulnerabilities::Occurrence).to receive(:sha)
+          .and_return('abc')
+      end
+
+      context 'without start_line or end_line' do
+        before do
+          allow(presenter).to receive(:location)
+            .and_return({ 'file' => 'a.txt' })
+        end
+
+        it { is_expected.to end_with('a.txt') }
+      end
+
+      context 'with start_line only' do
+        before do
+          allow(presenter).to receive(:location)
+            .and_return({ 'file' => 'a.txt', 'start_line' => 1 })
+        end
+
+        it { is_expected.to end_with('#L1') }
+      end
+
+      context 'with start_line and end_line' do
+        before do
+          allow(presenter).to receive(:location)
+            .and_return({ 'file' => 'a.txt', 'start_line' => 1, 'end_line' => 2 })
+        end
+
+        it { is_expected.to end_with('#L1-2') }
+      end
+    end
+  end
+end

ee/spec/serializers/vulnerabilities/occurrence_entity_spec.rb

@@ -53,6 +53,7 @@ describe Vulnerabilities::OccurrenceEntity do
       expect(subject).to include(:scanner, :project, :identifiers)
       expect(subject).to include(:dismissal_feedback, :issue_feedback)
       expect(subject).to include(:description, :links, :location, :remediations, :solution)
+      expect(subject).to include(:blob_path)
     end
 
     context 'when not allowed to admin vulnerability feedback' do