Merge branch 'add-project-security-dashboard-vulnerabilities-endpoints' into 'master'

Add project security dashboard vulnerabilities endpoints

Closes #12244 and #12381

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

ee/app/controllers/concerns/vulnerabilities_actions.rb

@@ -32,16 +32,6 @@ module VulnerabilitiesActions
     end
   end
 
-  def history
-    history_count = Gitlab::Vulnerabilities::History.new(group, filter_params).vulnerabilities_counter
-
-    respond_to do |format|
-      format.json do
-        render json: history_count
-      end
-    end
-  end
-
   private
 
   def filter_params

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

@@ -5,4 +5,14 @@ class Groups::Security::VulnerabilitiesController < Groups::ApplicationControlle
   include VulnerabilitiesActions
 
   alias_method :vulnerable, :group
+
+  def history
+    history_count = Gitlab::Vulnerabilities::History.new(group, filter_params).vulnerabilities_counter
+
+    respond_to do |format|
+      format.json do
+        render json: history_count
+      end
+    end
+  end
 end

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

+# frozen_string_literal: true
+
+class Projects::Security::VulnerabilitiesController < Projects::ApplicationController
+  include SecurityDashboardsPermissions
+  include VulnerabilitiesActions
+
+  alias_method :vulnerable, :project
+end

ee/app/helpers/ee/projects_helper.rb

@@ -172,9 +172,8 @@ module EE
       else
         {
           project: { id: project.id, name: project.name },
-          vulnerabilities_endpoint: group_security_vulnerabilities_path(project.group),
-          vulnerabilities_summary_endpoint: summary_group_security_vulnerabilities_path(project.group),
-          vulnerabilities_history_endpoint: history_group_security_vulnerabilities_path(project.group),
+          vulnerabilities_endpoint: project_security_vulnerabilities_path(project),
+          vulnerabilities_summary_endpoint: summary_project_security_vulnerabilities_path(project),
           vulnerability_feedback_help_path: help_page_path("user/application_security/index", anchor: "interacting-with-the-vulnerabilities"),
           empty_state_svg_path: image_path('illustrations/security-dashboard-empty-state.svg'),
           dashboard_documentation: help_page_path('user/application_security/security_dashboard/index'),

ee/app/models/ee/project.rb

@@ -20,6 +20,7 @@ module EE
       include EachBatch
       include InsightsFeature
       include IgnorableColumn
+      include Vulnerable
 
       ignore_column :mirror_last_update_at,
         :mirror_last_successful_update_at,

ee/changelogs/unreleased/fix-project-security-dashboard-errors.yml

+---
+title: Fix error fetching project security dashboard data for maintainers with access to a project but not to its group & fix routing error for project security dashboard for projects not in a group
+merge_request: 14896
+author:
+type: fixed

ee/config/routes/project.rb

@@ -82,6 +82,11 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
 
       namespace :security do
         resources :dependencies, only: [:index]
+        resources :vulnerabilities, only: [:index] do
+          collection do
+            get :summary
+          end
+        end
       end
     end
   end

ee/spec/controllers/groups/security/vulnerabilities_controller_spec.rb

@@ -18,13 +18,12 @@ describe Groups::Security::VulnerabilitiesController do
 
   before do
     sign_in(user)
+    stub_licensed_features(security_dashboard: true)
+    group.add_developer(user)
   end
 
   describe 'GET index.json' do
     it 'returns vulnerabilities for all projects in the group' do
-      stub_licensed_features(security_dashboard: true)
-      group.add_developer(user)
-
       # create projects for the group
       2.times do
         project = create(:project, namespace: group)
@@ -43,4 +42,87 @@ describe Groups::Security::VulnerabilitiesController do
       expect(json_response.count).to be(2)
     end
   end
+
+  describe 'GET history.json' do
+    let(:params) { { group_id: group } }
+    let(:project) { create(:project, namespace: group) }
+    let(:pipeline) { create(:ci_pipeline, :success, project: project) }
+
+    subject { get :history, params: params, format: :json }
+
+    before do
+      travel_to(Time.zone.parse('2018-11-10')) do
+        create(:vulnerabilities_occurrence,
+                pipelines: [pipeline],
+                project: project,
+                report_type: :sast,
+                severity: :critical)
+
+        create(:vulnerabilities_occurrence,
+                pipelines: [pipeline],
+                project: project,
+                report_type: :dependency_scanning,
+                severity: :low)
+      end
+
+      travel_to(Time.zone.parse('2018-11-12')) do
+        create(:vulnerabilities_occurrence,
+                pipelines: [pipeline],
+                project: project,
+                report_type: :sast,
+                severity: :critical)
+
+        create(:vulnerabilities_occurrence,
+                pipelines: [pipeline],
+                project: project,
+                report_type: :dependency_scanning,
+                severity: :low)
+      end
+    end
+
+    it 'returns vulnerability history within last 90 days' do
+      travel_to(Time.zone.parse('2019-02-11')) do
+        subject
+      end
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(json_response['total']).to eq({ '2018-11-12' => 2 })
+      expect(json_response['critical']).to eq({ '2018-11-12' => 1 })
+      expect(json_response['low']).to eq({ '2018-11-12' => 1 })
+      expect(response).to match_response_schema('vulnerabilities/history', dir: 'ee')
+    end
+
+    it 'returns empty history if there are no vulnerabilities within last 90 days' do
+      travel_to(Time.zone.parse('2019-02-13')) do
+        subject
+      end
+
+      expect(json_response).to eq({
+        "undefined" => {},
+        "info" => {},
+        "unknown" => {},
+        "low" => {},
+        "medium" => {},
+        "high" => {},
+        "critical" => {},
+        "total" => {}
+      })
+    end
+
+    context 'with a report type filter' do
+      let(:params) { { group_id: group, report_type: %w[sast] } }
+
+      before do
+        travel_to(Time.zone.parse('2019-02-11')) do
+          subject
+        end
+      end
+
+      it 'returns filtered history if filters are enabled' do
+        expect(json_response['total']).to eq({ '2018-11-12' => 1 })
+        expect(json_response['critical']).to eq({ '2018-11-12' => 1 })
+        expect(json_response['low']).to eq({})
+      end
+    end
+  end
 end

ee/spec/controllers/projects/security/vulnerabilities_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::Security::VulnerabilitiesController do
+  let(:project) { create(:project) }
+  let(:params) { { project_id: project, namespace_id: project.creator } }
+
+  it_behaves_like VulnerabilitiesActions do
+    let(:vulnerable) { project }
+    let(:vulnerable_params) { params }
+  end
+
+  it_behaves_like SecurityDashboardsPermissions do
+    let(:vulnerable) { project }
+    let(:security_dashboard_action) { get :index, params: params, format: :json }
+  end
+end

ee/spec/models/project_spec.rb

@@ -7,6 +7,12 @@ describe Project do
   include ::EE::GeoHelpers
   using RSpec::Parameterized::TableSyntax
 
+  let(:project) { create(:project) }
+
+  it_behaves_like Vulnerable do
+    let(:vulnerable) { project }
+  end
+
   describe 'associations' do
     it { is_expected.to delegate_method(:shared_runners_minutes).to(:statistics) }
     it { is_expected.to delegate_method(:shared_runners_seconds).to(:statistics) }

ee/spec/support/helpers/vulnerable_helpers.rb

+# frozen_string_literal: true
+
+module VulnerableHelpers
+  class BadVulnerableError < StandardError
+    def message
+      'The given vulnerable must be either `Project` or `Namespace`'
+    end
+  end
+
+  def as_vulnerable_project(vulnerable)
+    case vulnerable
+    when Project
+      vulnerable
+    when Namespace
+      create(:project, namespace: vulnerable)
+    else
+      raise BadVulnerableError
+    end
+  end
+end

ee/spec/support/shared_examples/controllers/concerns/vulnerabilities_actions_shared_examples.rb

@@ -4,16 +4,12 @@ require 'spec_helper'
 
 shared_examples VulnerabilitiesActions do
   include ApiHelpers
+  include VulnerableHelpers
 
-  let(:params) { vulnerable_params }
+  let(:action_params) { vulnerable_params }
   let(:user) { create(:user) }
-  let(:pipeline) { create(:ci_pipeline, :success, project: project) }
-
-  def project
-    return vulnerable if vulnerable.is_a?(Project)
-
-    @project ||= create(:project, namespace: vulnerable)
-  end
+  let(:pipeline) { create(:ci_pipeline, :success, project: vulnerable_project) }
+  let(:vulnerable_project) { as_vulnerable_project(vulnerable) }
 
   before do
     vulnerable.add_developer(user)
@@ -23,16 +19,16 @@ shared_examples VulnerabilitiesActions do
   end
 
   describe 'GET index.json' do
-    subject { get :index, params: params, format: :json }
+    subject { get :index, params: action_params, format: :json }
 
     it 'returns an ordered list of vulnerabilities' do
       critical_vulnerability = create(
         :vulnerabilities_occurrence,
         pipelines: [pipeline],
-        project: project,
+        project: vulnerable_project,
         severity: :critical
       )
-      create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project, severity: :high)
+      create(:vulnerabilities_occurrence, pipelines: [pipeline], project: vulnerable_project, severity: :high)
 
       subject
 
@@ -43,12 +39,12 @@ shared_examples VulnerabilitiesActions do
     end
 
     context 'when a specific page is requested' do
-      let(:params) { vulnerable_params.merge(page: 2) }
+      let(:action_params) { vulnerable_params.merge(page: 2) }
 
       before do
         Vulnerabilities::Occurrence.paginates_per 2
 
-        create_list(:vulnerabilities_occurrence, 3, pipelines: [pipeline], project: project)
+        create_list(:vulnerabilities_occurrence, 3, pipelines: [pipeline], project: vulnerable_project)
 
         subject
       end
@@ -64,26 +60,26 @@ shared_examples VulnerabilitiesActions do
 
     context 'when the vulnerabilities have feedback' do
       before do
-        vulnerability = create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project, report_type: :sast)
+        vulnerability = create(:vulnerabilities_occurrence, pipelines: [pipeline], project: vulnerable_project, report_type: :sast)
         create(:vulnerability_feedback,
                 :sast,
                 :issue,
                 pipeline: pipeline,
-                issue: create(:issue, project: project),
-                project: project,
+                issue: create(:issue, project: vulnerable_project),
+                project: vulnerable_project,
                 project_fingerprint: vulnerability.project_fingerprint)
       end
 
       it 'avoids N+1 queries', :with_request_store do
         control_count = ActiveRecord::QueryRecorder.new { subject }
 
-        vulnerability = create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project, report_type: :sast)
+        vulnerability = create(:vulnerabilities_occurrence, pipelines: [pipeline], project: vulnerable_project, report_type: :sast)
         create(:vulnerability_feedback,
                 :sast,
                 :issue,
                 pipeline: pipeline,
-                issue: create(:issue, project: project),
-                project: project,
+                issue: create(:issue, project: vulnerable_project),
+                project: vulnerable_project,
                 project_fingerprint: vulnerability.project_fingerprint)
 
         expect { subject }.not_to exceed_all_query_limit(control_count)
@@ -92,15 +88,15 @@ shared_examples VulnerabilitiesActions do
 
     context 'with multiple report types' do
       before do
-        create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project, report_type: :sast)
-        create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project, report_type: :dast)
-        create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project, report_type: :dependency_scanning)
+        create(:vulnerabilities_occurrence, pipelines: [pipeline], project: vulnerable_project, report_type: :sast)
+        create(:vulnerabilities_occurrence, pipelines: [pipeline], project: vulnerable_project, report_type: :dast)
+        create(:vulnerabilities_occurrence, pipelines: [pipeline], project: vulnerable_project, report_type: :dependency_scanning)
 
         subject
       end
 
       context 'with a single report filter' do
-        let(:params) { vulnerable_params.merge(report_type: ['sast']) }
+        let(:action_params) { vulnerable_params.merge(report_type: ['sast']) }
 
         it 'returns a list of vulnerabilities for that reporty type only' do
           expect(json_response.length).to eq 1
@@ -109,7 +105,7 @@ shared_examples VulnerabilitiesActions do
       end
 
       context 'with multiple report filters' do
-        let(:params) { vulnerable_params.merge(report_type: %w[sast dependency_scanning]) }
+        let(:action_params) { vulnerable_params.merge(report_type: %w[sast dependency_scanning]) }
 
         it 'returns a list of vulnerabilities for all filtered upon types' do
           expect(json_response.length).to eq 2
@@ -120,17 +116,17 @@ shared_examples VulnerabilitiesActions do
   end
 
   describe 'GET summary.json' do
-    subject { get :summary, params: params, format: :json }
+    subject { get :summary, params: action_params, format: :json }
 
     before do
       create_list(:vulnerabilities_occurrence, 3,
-        pipelines: [pipeline], project: project, report_type: :sast, severity: :high)
+        pipelines: [pipeline], project: vulnerable_project, report_type: :sast, severity: :high)
       create_list(:vulnerabilities_occurrence, 2,
-        pipelines: [pipeline], project: project, report_type: :dependency_scanning, severity: :low)
+        pipelines: [pipeline], project: vulnerable_project, report_type: :dependency_scanning, severity: :low)
       create_list(:vulnerabilities_occurrence, 1,
-        pipelines: [pipeline], project: project, report_type: :dast, severity: :medium)
+        pipelines: [pipeline], project: vulnerable_project, report_type: :dast, severity: :medium)
       create_list(:vulnerabilities_occurrence, 1,
-        pipelines: [pipeline], project: project, report_type: :sast, severity: :medium)
+        pipelines: [pipeline], project: vulnerable_project, report_type: :sast, severity: :medium)
 
       subject
     end
@@ -144,7 +140,7 @@ shared_examples VulnerabilitiesActions do
     end
 
     context 'with enabled filters' do
-      let(:params) { vulnerable_params.merge(report_type: %w[sast dast], severity: %[high low]) }
+      let(:action_params) { vulnerable_params.merge(report_type: %w[sast dast], severity: %[high low]) }
 
       it 'returns counts for filtered vulnerabilities' do
         expect(json_response['high']).to eq(3)
@@ -153,83 +149,4 @@ shared_examples VulnerabilitiesActions do
       end
     end
   end
-
-  describe 'GET history.json' do
-    subject { get :history, params: params, format: :json }
-
-    before do
-      travel_to(Time.zone.parse('2018-11-10')) do
-        create(:vulnerabilities_occurrence,
-                pipelines: [pipeline],
-                project: project,
-                report_type: :sast,
-                severity: :critical)
-
-        create(:vulnerabilities_occurrence,
-                pipelines: [pipeline],
-                project: project,
-                report_type: :dependency_scanning,
-                severity: :low)
-      end
-
-      travel_to(Time.zone.parse('2018-11-12')) do
-        create(:vulnerabilities_occurrence,
-                pipelines: [pipeline],
-                project: project,
-                report_type: :sast,
-                severity: :critical)
-
-        create(:vulnerabilities_occurrence,
-                pipelines: [pipeline],
-                project: project,
-                report_type: :dependency_scanning,
-                severity: :low)
-      end
-    end
-
-    it 'returns vulnerability history within last 90 days' do
-      travel_to(Time.zone.parse('2019-02-11')) do
-        subject
-      end
-
-      expect(response).to have_gitlab_http_status(200)
-      expect(json_response['total']).to eq({ '2018-11-12' => 2 })
-      expect(json_response['critical']).to eq({ '2018-11-12' => 1 })
-      expect(json_response['low']).to eq({ '2018-11-12' => 1 })
-      expect(response).to match_response_schema('vulnerabilities/history', dir: 'ee')
-    end
-
-    it 'returns empty history if there are no vulnerabilities within last 90 days' do
-      travel_to(Time.zone.parse('2019-02-13')) do
-        subject
-      end
-
-      expect(json_response).to eq({
-        "undefined" => {},
-        "info" => {},
-        "unknown" => {},
-        "low" => {},
-        "medium" => {},
-        "high" => {},
-        "critical" => {},
-        "total" => {}
-      })
-    end
-
-    context 'with a report type filter' do
-      let(:params) { vulnerable_params.merge(report_type: %w[sast]) }
-
-      before do
-        travel_to(Time.zone.parse('2019-02-11')) do
-          subject
-        end
-      end
-
-      it 'returns filtered history if filters are enabled' do
-        expect(json_response['total']).to eq({ '2018-11-12' => 1 })
-        expect(json_response['critical']).to eq({ '2018-11-12' => 1 })
-        expect(json_response['low']).to eq({})
-      end
-    end
-  end
 end

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

@@ -3,17 +3,19 @@
 require 'spec_helper'
 
 shared_examples_for Vulnerable do
-  let(:project) { create(:project, namespace: vulnerable) }
+  include VulnerableHelpers
+
   let(:external_project) { create(:project) }
-  let(:failed_pipeline) { create(:ci_pipeline, :failed, project: project) }
+  let(:failed_pipeline) { create(:ci_pipeline, :failed, project: vulnerable_project) }
 
-  let!(:old_vuln) { create_vulnerability(project) }
-  let!(:new_vuln) { create_vulnerability(project) }
+  let!(:old_vuln) { create_vulnerability(vulnerable_project) }
+  let!(:new_vuln) { create_vulnerability(vulnerable_project) }
   let!(:external_vuln) { create_vulnerability(external_project) }
-  let!(:failed_vuln) { create_vulnerability(project, failed_pipeline) }
+  let!(:failed_vuln) { create_vulnerability(vulnerable_project, failed_pipeline) }
+  let(:vulnerable_project) { as_vulnerable_project(vulnerable) }
 
   before do
-    pipeline_ran_against_new_sha = create(:ci_pipeline, :success, project: project, sha: '123')
+    pipeline_ran_against_new_sha = create(:ci_pipeline, :success, project: vulnerable_project, sha: '123')
     new_vuln.pipelines << pipeline_ran_against_new_sha
   end
 
@@ -30,8 +32,8 @@ shared_examples_for Vulnerable do
     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) }
+      let!(:branch_pipeline) { create(:ci_pipeline, :success, project: vulnerable_project, ref: 'feature-x') }
+      let!(:branch_vuln) { create(:vulnerabilities_occurrence, pipelines: [branch_pipeline], project: vulnerable_project) }
 
       # TODO: This should actually fail and we must scope vulns
       # per branch as soon as we store them for other branches
@@ -52,8 +54,8 @@ shared_examples_for Vulnerable do
     it { is_expected.to all(respond_to(:sha)) }
 
     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) }
+      let!(:branch_pipeline) { create(:ci_pipeline, :success, project: vulnerable_project, ref: 'feature-x') }
+      let!(:branch_vuln) { create(:vulnerabilities_occurrence, pipelines: [branch_pipeline], project: vulnerable_project) }
 
       # TODO: This should actually fail and we must scope vulns
       # per branch as soon as we store them for other branches
@@ -72,8 +74,8 @@ shared_examples_for Vulnerable do
     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) }
+      let!(:branch_pipeline) { create(:ci_pipeline, :success, project: vulnerable_project, ref: 'feature-x') }
+      let!(:branch_vuln) { create(:vulnerabilities_occurrence, pipelines: [branch_pipeline], project: vulnerable_project) }
 
       # TODO: This should actually fail and we must scope vulns
       # per branch as soon as we store them for other branches