Merge branch '33899-instance-security-dashboard-vulnerabilities' into 'master'

Add vulnerability findings data to instance security dashboard

Closes #33899

See merge request gitlab-org/gitlab!21691

ee/app/controllers/concerns/project_collection_vulnerability_findings_actions.rb

@@ -6,7 +6,7 @@ module ProjectCollectionVulnerabilityFindingsActions
 
   included do
     def history
-      history_count = Gitlab::Vulnerabilities::History.new(vulnerable, filter_params).findings_counter
+      history_count = Gitlab::Vulnerabilities::History.new(vulnerable, params: filter_params).findings_counter
 
       respond_to do |format|
         format.json do

ee/app/controllers/security/vulnerability_findings_controller.rb

+# frozen_string_literal: true
+
+module Security
+  class VulnerabilityFindingsController < ::Security::ApplicationController
+    include ProjectCollectionVulnerabilityFindingsActions
+
+    before_action :remove_invalid_project_ids
+
+    private
+
+    def remove_invalid_project_ids
+      render_empty_response if valid_project_ids.empty?
+
+      params[:project_id] = valid_project_ids
+    end
+
+    def render_empty_response
+      respond_to do |format|
+        format.json do
+          render json: {}
+        end
+      end
+    end
+
+    def vulnerable
+      @vulnerable ||= ApplicationInstance.new
+    end
+
+    def valid_project_ids
+      return security_dashboard_project_ids if request_project_ids.empty?
+
+      security_dashboard_project_ids & request_project_ids
+    end
+
+    def request_project_ids
+      params.fetch(:project_id, []).map(&:to_i)
+    end
+
+    def security_dashboard_project_ids
+      current_user.security_dashboard_project_ids
+    end
+  end
+end

ee/app/helpers/security_helper.rb

+# frozen_string_literal: true
+
+module SecurityHelper
+  def instance_security_dashboard_data
+    {
+      dashboard_documentation: help_page_path('user/application_security/security_dashboard/index', anchor: 'instance-security-dashboard'),
+      empty_dashboard_state_svg_path: image_path('illustrations/security-dashboard-empty-state.svg'),
+      empty_state_svg_path: image_path('illustrations/operations-dashboard_empty.svg'),
+      project_add_endpoint: security_projects_path,
+      project_list_endpoint: security_projects_path,
+      vulnerabilities_count_endpoint: summary_security_vulnerability_findings_path,
+      vulnerabilities_endpoint: security_vulnerability_findings_path,
+      vulnerabilities_history_endpoint: history_security_vulnerability_findings_path,
+      vulnerability_feedback_help_path: help_page_path('user/application_security/index', anchor: 'interacting-with-the-vulnerabilities')
+    }
+  end
+end

ee/app/models/application_instance.rb

+# frozen_string_literal: true
+
+class ApplicationInstance
+  extend ActiveModel::Naming
+  include ::Vulnerable
+
+  def all_pipelines
+    ::Ci::Pipeline.all
+  end
+end

ee/app/models/ee/user.rb

@@ -332,6 +332,14 @@ module EE
       read_attribute(:support_bot)
     end
 
+    def security_dashboard_project_ids
+      if self.can?(:read_all_resources)
+        security_dashboard_projects.ids
+      else
+        security_dashboard_projects.visible_to_user(self).ids
+      end
+    end
+
     protected
 
     override :password_required?

ee/app/views/security/dashboard/show.html.haml

 - page_title _('Security Dashboard')
 - @hide_breadcrumbs = true
 
-#js-security{ data: { vulnerabilities_endpoint: '/groups/gitlab-org/-/security/vulnerabilities',
-  vulnerabilities_count_endpoint: '/groups/gitlab-org/-/security/vulnerabilities/summary',
-  vulnerabilities_history_endpoint: '/groups/gitlab-org/-/security/vulnerabilities/history',
-  project_add_endpoint: security_projects_path,
-  project_list_endpoint: security_projects_path,
-  vulnerability_feedback_help_path: help_page_path('user/application_security/index', anchor: 'interacting-with-the-vulnerabilities'),
-  empty_state_svg_path: image_path('illustrations/operations-dashboard_empty.svg'),
-  empty_dashboard_state_svg_path: image_path('illustrations/security-dashboard-empty-state.svg'),
-  dashboard_documentation: help_page_path('user/application_security/security_dashboard/index', anchor: 'instance-security-dashboard') } }
+#js-security{ data: instance_security_dashboard_data }

ee/config/routes/security.rb

@@ -4,4 +4,11 @@ namespace :security do
   root to: 'dashboard#show'
 
   resources :projects, only: [:index, :create, :destroy]
+
+  resources :vulnerability_findings, only: [:index] do
+    collection do
+      get :summary
+      get :history
+    end
+  end
 end

ee/lib/gitlab/vulnerabilities/history.rb

@@ -8,10 +8,11 @@ module Gitlab
       attr_reader :vulnerable, :filters
 
       HISTORY_RANGE = 3.months
+      NoProjectIDsError = Class.new(StandardError)
 
-      def initialize(vulnerable, filters)
+      def initialize(vulnerable, params:)
         @vulnerable = vulnerable
-        @filters = filters
+        @filters = params
       end
 
       def findings_counter
@@ -57,6 +58,10 @@ module Gitlab
         return filters[:project_id] if filters.key?('project_id')
 
         vulnerable.project_ids_with_security_reports
+      rescue NoMethodError
+        vulnerable_name = vulnerable.model_name.human.downcase
+
+        raise NoProjectIDsError, "A project_id filter must be given with this #{vulnerable_name}"
       end
     end
   end

ee/spec/helpers/security_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe SecurityHelper do
+  describe '#instance_security_dashboard_data' do
+    before do
+      stub_feature_flags(first_class_vulnerabilities: true)
+    end
+
+    subject { instance_security_dashboard_data }
+
+    it 'returns vulnerability, project, feedback, asset, and docs paths for the instance security dashboard' do
+      is_expected.to eq({
+        dashboard_documentation: help_page_path('user/application_security/security_dashboard/index', anchor: 'instance-security-dashboard'),
+        empty_dashboard_state_svg_path: image_path('illustrations/security-dashboard-empty-state.svg'),
+        empty_state_svg_path: image_path('illustrations/operations-dashboard_empty.svg'),
+        project_add_endpoint: security_projects_path,
+        project_list_endpoint: security_projects_path,
+        vulnerabilities_count_endpoint: summary_security_vulnerability_findings_path,
+        vulnerabilities_endpoint: security_vulnerability_findings_path,
+        vulnerabilities_history_endpoint: history_security_vulnerability_findings_path,
+        vulnerability_feedback_help_path: help_page_path('user/application_security/index', anchor: 'interacting-with-the-vulnerabilities')
+      })
+    end
+  end
+end

ee/spec/lib/gitlab/vulnerabilities/history_cache_spec.rb

@@ -3,48 +3,63 @@
 require 'spec_helper'
 
 describe Gitlab::Vulnerabilities::HistoryCache do
-  let(:group) { create(:group) }
-  let(:project) { create(:project, :public, namespace: group) }
-  let(:project_cache_key) { described_class.new(group, project.id).send(:cache_key) }
+  describe '#fetch', :use_clean_rails_memory_store_caching do
+    shared_examples 'the history cache when given an expected Vulnerable' do
+      let(:project) { create(:project, :public, namespace: group) }
+      let(:project_cache_key) { described_class.new(vulnerable, project.id).send(:cache_key) }
 
-  before do
-    create_vulnerabilities(1, project)
-  end
+      before do
+        create_vulnerabilities(1, project)
+      end
 
-  describe '#fetch', :use_clean_rails_memory_store_caching do
-    it 'reads from cache when records are cached' do
-      history_cache = described_class.new(group, project.id)
+      it 'reads from cache when records are cached' do
+        history_cache = described_class.new(vulnerable, project.id)
 
-      expect(Rails.cache.fetch(project_cache_key, raw: true)).to be_nil
+        expect(Rails.cache.fetch(project_cache_key, raw: true)).to be_nil
 
-      control_count = ActiveRecord::QueryRecorder.new { history_cache.fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE) }
+        control_count = ActiveRecord::QueryRecorder.new { history_cache.fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE) }
 
-      expect { 2.times { history_cache.fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE) } }.not_to exceed_query_limit(control_count)
-    end
+        expect { 2.times { history_cache.fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE) } }.not_to exceed_query_limit(control_count)
+      end
 
-    it 'returns the proper format for uncached history' do
-      Timecop.freeze do
-        fetched_history = described_class.new(group, project.id).fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE)
+      it 'returns the proper format for uncached history' do
+        Timecop.freeze do
+          fetched_history = described_class.new(vulnerable, project.id).fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE)
 
-        expect(fetched_history[:total]).to eq( Date.today => 1 )
-        expect(fetched_history[:high]).to eq( Date.today => 1 )
+          expect(fetched_history[:total]).to eq( Date.today => 1 )
+          expect(fetched_history[:high]).to eq( Date.today => 1 )
+        end
       end
-    end
 
-    it 'returns the proper format for cached history' do
-      Timecop.freeze do
-        described_class.new(group, project.id).fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE)
-        fetched_history = described_class.new(group, project.id).fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE)
+      it 'returns the proper format for cached history' do
+        Timecop.freeze do
+          described_class.new(vulnerable, project.id).fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE)
+          fetched_history = described_class.new(vulnerable, project.id).fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE)
 
-        expect(fetched_history[:total]).to eq( Date.today => 1 )
-        expect(fetched_history[:high]).to eq( Date.today => 1 )
+          expect(fetched_history[:total]).to eq( Date.today => 1 )
+          expect(fetched_history[:high]).to eq( Date.today => 1 )
+        end
+      end
+
+      def create_vulnerabilities(count, project, options = {})
+        report_type = options[:report_type] || :sast
+        pipeline = create(:ci_pipeline, :success, project: project)
+        create_list(:vulnerabilities_occurrence, count, report_type: report_type, pipelines: [pipeline], project: project)
+      end
+    end
+
+    context 'when given a Group' do
+      it_behaves_like 'the history cache when given an expected Vulnerable' do
+        let(:group) { create(:group) }
+        let(:vulnerable) { group }
       end
     end
 
-    def create_vulnerabilities(count, project, options = {})
-      report_type = options[:report_type] || :sast
-      pipeline = create(:ci_pipeline, :success, project: project)
-      create_list(:vulnerabilities_occurrence, count, report_type: report_type, pipelines: [pipeline], project: project)
+    context 'when given an ApplicationInstance' do
+      it_behaves_like 'the history cache when given an expected Vulnerable' do
+        let(:group) { create(:group) }
+        let(:vulnerable) { ApplicationInstance.new }
+      end
     end
   end
 end

ee/spec/lib/gitlab/vulnerabilities/history_spec.rb

@@ -3,65 +3,99 @@
 require 'spec_helper'
 
 describe Gitlab::Vulnerabilities::History do
-  let(:group) { create(:group) }
-  let(:project1) { create(:project, :public, namespace: group) }
-  let(:project2) { create(:project, :public, namespace: group) }
-  let(:filters) { {} }
-
-  before do
-    create_vulnerabilities(1, project1, { severity: :medium, report_type: :sast })
-    create_vulnerabilities(2, project2, { severity: :high, report_type: :sast })
-  end
-
   describe '#findings_counter', :use_clean_rails_memory_store_caching do
-    subject(:counter) { described_class.new(group, filters).findings_counter }
+    shared_examples 'the history cache when given an expected Vulnerable' do
+      let(:filters) { project_ids }
+      let(:today) { Date.parse('20191031') }
+
+      before do
+        Timecop.freeze(today) do
+          create_vulnerabilities(1, project1, { severity: :medium, report_type: :sast })
+          create_vulnerabilities(2, project2, { severity: :high, report_type: :sast })
+        end
+      end
+
+      subject(:counter) { described_class.new(vulnerable, params: filters).findings_counter }
 
-    context 'filters are passed' do
-      let(:filters) { { report_type: :sast } }
+      context 'when filters are passed' do
+        let(:filters) { project_ids.merge(report_type: :sast) }
 
-      it 'does not call Gitlab::Vulnerabilities::HistoryCache' do
-        expect(Gitlab::Vulnerabilities::HistoryCache).not_to receive(:new)
+        it 'does not call Gitlab::Vulnerabilities::HistoryCache' do
+          expect(Gitlab::Vulnerabilities::HistoryCache).not_to receive(:new)
+
+          counter
+        end
+      end
+
+      it 'calls Gitlab::Vulnerabilities::HistoryCache' do
+        expect(Gitlab::Vulnerabilities::HistoryCache).to receive(:new).twice.and_call_original
 
         counter
       end
-    end
 
-    it 'calls Gitlab::Vulnerabilities::HistoryCache' do
-      expect(Gitlab::Vulnerabilities::HistoryCache).to receive(:new).twice.and_call_original
+      it 'returns the proper format for the history' do
+        Timecop.freeze(today) do
+          expect(counter[:total]).to eq({ today => 3 })
+          expect(counter[:high]).to eq({ today => 2 })
+        end
+      end
+
+      context 'when there are multiple projects with vulnerabilities' do
+        before do
+          Timecop.freeze(today - 1) do
+            create_vulnerabilities(1, project1, { severity: :high })
+          end
+          Timecop.freeze(today - 4) do
+            create_vulnerabilities(1, project2, { severity: :high })
+          end
+        end
+
+        it 'sorts by date for each key' do
+          Timecop.freeze(today) do
+            expect(counter[:high].keys).to eq([(today - 4), (today - 1), today])
+          end
+        end
+      end
 
-      counter
+      def create_vulnerabilities(count, project, options = {})
+        report_type = options[:report_type] || :sast
+        severity = options[:severity] || :high
+        pipeline = create(:ci_pipeline, :success, project: project)
+        created_at = options[:created_at] || today
+        create_list(:vulnerabilities_occurrence, count, report_type: report_type, severity: severity, pipelines: [pipeline], project: project, created_at: created_at)
+      end
     end
 
-    it 'returns the proper format for the history' do
-      Timecop.freeze do
-        expect(counter[:total]).to eq({ Date.today => 3 })
-        expect(counter[:high]).to eq({ Date.today => 2 })
+    context 'when the given vulnerable is a Group' do
+      it_behaves_like 'the history cache when given an expected Vulnerable' do
+        let(:group) { create(:group) }
+        let(:project1) { create(:project, :public, namespace: group) }
+        let(:project2) { create(:project, :public, namespace: group) }
+        let(:project_ids) { {} }
+        let(:vulnerable) { group }
       end
     end
 
-    context 'multiple projects with vulnerabilities' do
-      before do
-        Timecop.freeze(Date.today - 1) do
-          create_vulnerabilities(1, project1, { severity: :high })
-        end
-        Timecop.freeze(Date.today - 4) do
-          create_vulnerabilities(1, project2, { severity: :high })
+    context 'when given an ApplicationInstance' do
+      let(:vulnerable) { ApplicationInstance.new }
+
+      context 'and a project_id filter' do
+        it_behaves_like 'the history cache when given an expected Vulnerable' do
+          let(:group) { create(:group) }
+          let(:project1) { create(:project, :public, namespace: group) }
+          let(:project2) { create(:project, :public, namespace: group) }
+          let(:project_ids) { ActionController::Parameters.new({ 'project_id' => [project1, project2] }) }
         end
       end
 
-      it 'sorts by date for each key' do
-        Timecop.freeze do
-          expect(counter[:high].keys).to eq([(Date.today - 4), (Date.today - 1), Date.today])
+      context 'and no project_id filter' do
+        it 'throws an error saying that the filter must be given' do
+          expect { described_class.new(vulnerable, params: {}).findings_counter }.to raise_error(
+            Gitlab::Vulnerabilities::History::NoProjectIDsError,
+            "A project_id filter must be given with this #{vulnerable.model_name.human.downcase}"
+          )
         end
       end
     end
-
-    def create_vulnerabilities(count, project, options = {})
-      report_type = options[:report_type] || :sast
-      severity = options[:severity] || :high
-      pipeline = create(:ci_pipeline, :success, project: project)
-      created_at = options[:created_at] || Date.today
-      create_list(:vulnerabilities_occurrence, count, report_type: report_type, severity: severity, pipelines: [pipeline], project: project, created_at: created_at)
-    end
   end
 end

ee/spec/models/application_instance_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ApplicationInstance do
+  it_behaves_like Vulnerable do
+    let(:vulnerable) { described_class.new }
+  end
+
+  describe '#all_pipelines' do
+    it 'returns all CI pipelines for the instance' do
+      allow(::Ci::Pipeline).to receive(:all)
+
+      described_class.new.all_pipelines
+
+      expect(::Ci::Pipeline).to have_received(:all)
+    end
+  end
+end

ee/spec/models/user_spec.rb

@@ -700,4 +700,32 @@ describe User do
       end
     end
   end
+
+  describe '#security_dashboard_project_ids' do
+    let(:project) { create(:project) }
+
+    context 'when the user can read all resources' do
+      it "returns the ids for all of the user's security dashboard projects" do
+        admin = create(:admin)
+        auditor = create(:auditor)
+
+        admin.security_dashboard_projects << project
+        auditor.security_dashboard_projects << project
+
+        expect(admin.security_dashboard_project_ids).to eq([project.id])
+        expect(auditor.security_dashboard_project_ids).to eq([project.id])
+      end
+    end
+
+    context 'when the user cannot read all resources' do
+      it 'returns the ids for security dashboard projects visible to the user' do
+        user = create(:user)
+        member_project = create(:project)
+        member_project.add_developer(user)
+        user.security_dashboard_projects << [project, member_project]
+
+        expect(user.security_dashboard_project_ids).to eq([member_project.id])
+      end
+    end
+  end
 end

ee/spec/support/helpers/vulnerable_helpers.rb

@@ -3,7 +3,7 @@
 module VulnerableHelpers
   class BadVulnerableError < StandardError
     def message
-      'The given vulnerable must be either `Project` or `Namespace`'
+      'The given vulnerable must be either `Project`, `Namespace`, or `ApplicationInstance`'
     end
   end
 
@@ -13,6 +13,21 @@ module VulnerableHelpers
       vulnerable
     when Namespace
       create(:project, namespace: vulnerable)
+    when ApplicationInstance
+      create(:project)
+    else
+      raise BadVulnerableError
+    end
+  end
+
+  def as_external_vulnerable_project(vulnerable)
+    case vulnerable
+    when Project
+      create(:project)
+    when Namespace
+      create(:project)
+    when ApplicationInstance
+      nil
     else
       raise BadVulnerableError
     end

ee/spec/support/shared_examples/models/concerns/vulnerable_shared_examples.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 shared_examples_for Vulnerable do
   include VulnerableHelpers
 
-  let(:external_project) { create(:project) }
+  let(:external_project) { as_external_vulnerable_project(vulnerable) }
   let(:failed_pipeline) { create(:ci_pipeline, :failed, project: vulnerable_project) }
 
   let!(:old_vuln) { create_vulnerability(vulnerable_project) }
@@ -20,8 +20,10 @@ shared_examples_for Vulnerable do
   end
 
   def create_vulnerability(project, pipeline = nil)
-    pipeline ||= create(:ci_pipeline, :success, project: project)
-    create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project)
+    if project
+      pipeline ||= create(:ci_pipeline, :success, project: project)
+      create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project)
+    end
   end
 
   describe '#latest_vulnerabilities' do

ee/spec/support/shared_examples/requests/security/instance_security_dashboard_json_endpoint_shared_examples.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+shared_examples 'instance security dashboard JSON endpoint' do
+  context 'when the user is authenticated' do
+    let(:security_application_controller_user) { create(:user) }
+
+    before do
+      stub_licensed_features(security_dashboard: true)
+
+      login_as(security_application_controller_user)
+    end
+
+    it 'responds with success' do
+      security_dashboard_request
+
+      expect(response).to have_gitlab_http_status(:ok)
+    end
+
+    context 'and the instance does not have an Ultimate license' do
+      it '404s' do
+        stub_licensed_features(security_dashboard: false)
+
+        security_dashboard_request
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'and the security dashboard feature is disabled' do
+      it '404s' do
+        stub_feature_flags(security_dashboard: false)
+
+        security_dashboard_request
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+  end
+
+  context 'when the user is not authenticated' do
+    it 'responds with a 401' do
+      security_dashboard_request
+
+      expect(response).to have_gitlab_http_status(:unauthorized)
+    end
+  end
+end