Merge branch '10242-1-rename-vulns-to-vuln-findings-api' into 'master'

Rename Vulnerabilities API to Vulnerability Findings API

See merge request gitlab-org/gitlab!16468

ee/app/helpers/ee/projects_helper.rb

@@ -235,5 +235,17 @@ module EE
     def can_import_members?
       super && !membership_locked?
     end
+
+    def api_projects_vulnerability_findings_path(project, pipeline)
+      params = { id: project.id, params: { pipeline_id: pipeline.id, scope: 'dismissed' } }
+
+      path = if ::Feature.enabled?(:first_class_vulnerabilities)
+               api_v4_projects_vulnerability_findings_path(params)
+             else
+               api_v4_projects_vulnerabilities_path(params)
+             end
+
+      expose_path(path)
+    end
   end
 end

ee/app/views/projects/pipelines/_tabs_content.html.haml

@@ -16,7 +16,7 @@
       empty_state_svg_path: image_path('illustrations/security-dashboard-empty-state.svg'),
       pipeline_id: pipeline.id,
       project_id: project.id,
-      vulnerabilities_endpoint: expose_path(api_v4_projects_vulnerabilities_path(id: project.id, params: { pipeline_id: pipeline.id, scope: 'dismissed' })),
+      vulnerabilities_endpoint: api_projects_vulnerability_findings_path(project, pipeline),
       vulnerability_feedback_help_path: help_page_path('user/application_security/index') } }
     - else
       #js-security-report-app{ data: { head_blob_path: blob_path,

ee/lib/api/vulnerabilities.rb → ee/lib/api/vulnerability_findings.rb

 # frozen_string_literal: true
 
 module API
-  class Vulnerabilities < Grape::API
+  class VulnerabilityFindings < Grape::API
     include PaginationParams
 
     helpers do
+      params :vulnerability_findings_params do
+        optional :report_type, type: Array[String], desc: 'The type of report vulnerability belongs to',
+                 values: ::Vulnerabilities::Occurrence.report_types.keys,
+                 default: ::Vulnerabilities::Occurrence.report_types.keys
+        optional :scope, type: String, desc: 'Return vulnerabilities for the given scope: `dismissed` or `all`',
+                 default: 'dismissed', values: %w[all dismissed]
+        optional :severity,
+                 type: Array[String],
+                 desc: 'Returns issues belonging to specified severity level: '\
+                       '`undefined`, `info`, `unknown`, `low`, `medium`, `high`, or `critical`. Defaults to all',
+                 values: ::Vulnerabilities::Occurrence.severities.keys,
+                 default: ::Vulnerabilities::Occurrence.severities.keys
+        optional :confidence,
+                 type: Array[String],
+                 desc: 'Returns vulnerabilities belonging to specified confidence level: '\
+                       '`undefined`, `ignore`, `unknown`, `experimental`, `low`, `medium`, `high`, or `confirmed`. '\
+                       'Defaults to all',
+                 values: ::Vulnerabilities::Occurrence.confidences.keys,
+                 default: ::Vulnerabilities::Occurrence.confidences.keys
+        optional :pipeline_id, type: String, desc: 'The ID of the pipeline'
+
+        use :pagination
+      end
+
       def vulnerability_occurrences_by(params)
         pipeline = if params[:pipeline_id]
                      params[:project].all_pipelines.find_by(id: params[:pipeline_id]) # rubocop:disable CodeReuse/ActiveRecord
@@ -16,6 +40,28 @@ module API
 
         Security::PipelineVulnerabilitiesFinder.new(pipeline: pipeline, params: params).execute
       end
+
+      def respond_with_vulnerabilities
+        # TODO: implement the "Get a list of project's Vulnerabilities" step
+        # of https://gitlab.com/gitlab-org/gitlab-ee/issues/10242#status
+        not_found!
+      end
+
+      def respond_with_vulnerability_findings
+        authorize! :read_project_security_dashboard, user_project
+
+        vulnerability_occurrences = paginate(
+          Kaminari.paginate_array(
+            vulnerability_occurrences_by(declared_params.merge(project: user_project))
+          )
+        )
+
+        Gitlab::Vulnerabilities::OccurrencesPreloader.preload_feedback!(vulnerability_occurrences)
+
+        present vulnerability_occurrences,
+                with: ::Vulnerabilities::OccurrenceEntity,
+                request: GrapeRequestProxy.new(request, current_user)
+      end
     end
 
     before do
@@ -27,40 +73,32 @@ module API
     end
 
     resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
+      params do
+        use :vulnerability_findings_params
+      end
       desc 'Get a list of project vulnerabilities' do
         success ::Vulnerabilities::OccurrenceEntity
       end
+      get ':id/vulnerabilities' do
+        if Feature.enabled?(:first_class_vulnerabilities)
+          respond_with_vulnerabilities
+        else
+          respond_with_vulnerability_findings
+        end
+      end
 
       params do
-        optional :report_type, type: Array[String], desc: 'The type of report vulnerability belongs to', default: ::Vulnerabilities::Occurrence.report_types.keys
-        optional :scope, type: String, desc: 'Return vulnerabilities for the given scope: `dismissed` or `all`', default: 'dismissed', values: %w[all dismissed]
-        optional :severity,
-                 type: Array[String],
-                 desc: 'Returns issues belonging to specified severity level: `undefined`, `info`, `unknown`, `low`, `medium`, `high`, or `critical`. Defaults to all',
-                 default: ::Vulnerabilities::Occurrence.severities.keys
-        optional :confidence,
-                 type: Array[String],
-                 desc: 'Returns vulnerabilities belonging to specified confidence level: `undefined`, `ignore`, `unknown`, `experimental`, `low`, `medium`, `high`, or `confirmed`. Defaults to all',
-                 default: ::Vulnerabilities::Occurrence.confidences.keys
-        optional :pipeline_id, type: String, desc: 'The ID of the pipeline'
-
-        use :pagination
+        use :vulnerability_findings_params
       end
-
-      get ':id/vulnerabilities' do
-        authorize! :read_project_security_dashboard, user_project
-
-        vulnerability_occurrences = paginate(
-          Kaminari.paginate_array(
-            vulnerability_occurrences_by(declared_params.merge(project: user_project))
-          )
-        )
-
-        Gitlab::Vulnerabilities::OccurrencesPreloader.preload_feedback!(vulnerability_occurrences)
-
-        present vulnerability_occurrences,
-          with: ::Vulnerabilities::OccurrenceEntity,
-          request: GrapeRequestProxy.new(request, current_user)
+      desc 'Get a list of project vulnerability findings' do
+        success ::Vulnerabilities::OccurrenceEntity
+      end
+      get ':id/vulnerability_findings' do
+        if Feature.enabled?(:first_class_vulnerabilities)
+          respond_with_vulnerability_findings
+        else
+          not_found!
+        end
       end
     end
   end

ee/lib/ee/api/api.rb

@@ -35,7 +35,7 @@ module EE
         mount ::API::Scim
         mount ::API::ManagedLicenses
         mount ::API::ProjectApprovals
-        mount ::API::Vulnerabilities
+        mount ::API::VulnerabilityFindings
         mount ::API::MergeRequestApprovals
         mount ::API::MergeRequestApprovalRules
         mount ::API::ProjectAliases

ee/spec/helpers/projects_helper_spec.rb

@@ -97,18 +97,24 @@ describe ProjectsHelper do
     end
   end
 
-  describe '#project_security_dashboard_config' do
+  shared_context 'project with owner and pipeline' do
     let(:user) { create(:user) }
     let(:group) { create(:group).tap { |g| g.add_owner(user) } }
-    let(:project) { create(:project, :repository, group: group) }
     let(:pipeline) do
       create(:ee_ci_pipeline,
-      :with_sast_report,
-      user: user,
-      project: project,
-      ref: project.default_branch,
-      sha: project.commit.sha)
+             :with_sast_report,
+             user: user,
+             project: project,
+             ref: project.default_branch,
+             sha: project.commit.sha)
     end
+    let(:project) { create(:project, :repository, group: group) }
+  end
+
+  describe '#project_security_dashboard_config' do
+    include_context 'project with owner and pipeline'
+
+    let(:project) { create(:project, :repository, group: group) }
 
     context 'project without pipeline' do
       subject { helper.project_security_dashboard_config(project, nil) }
@@ -128,4 +134,22 @@ describe ProjectsHelper do
       end
     end
   end
+
+  describe '#api_projects_vulnerability_findings_path' do
+    include_context 'project with owner and pipeline'
+
+    subject { helper.api_projects_vulnerability_findings_path(project, pipeline) }
+
+    context 'when Vulnerability Findings API enabled' do
+      it { is_expected.to include("projects/#{project.id}/vulnerability_findings") }
+    end
+
+    context 'when the Vulnerability Findings API is disabled' do
+      before do
+        stub_feature_flags(first_class_vulnerabilities: false)
+      end
+
+      it { is_expected.to include("projects/#{project.id}/vulnerabilities") }
+    end
+  end
 end

ee/spec/requests/api/vulnerabilities_spec.rb → ee/spec/requests/api/vulnerability_findings_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe API::Vulnerabilities do
+describe API::VulnerabilityFindings do
   set(:project) { create(:project, :public) }
   set(:user) { create(:user) }
 
@@ -32,7 +32,9 @@ describe API::Vulnerabilities do
     dismissal
   end
 
-  describe "GET /projects/:id/vulnerabilities" do
+  shared_examples 'getting list of vulnerability findings' do
+    let(:project_vulnerabilities_path) { "/projects/#{project.id}/#{api_resource_name}" }
+
     context 'with an authorized user with proper permissions' do
       before do
         project.add_developer(user)
@@ -41,7 +43,7 @@ describe API::Vulnerabilities do
       it 'returns all non-dismissed vulnerabilities' do
         occurrence_count = (sast_report.occurrences.count + ds_report.occurrences.count - 1).to_s
 
-        get api("/projects/#{project.id}/vulnerabilities?per_page=40", user)
+        get api(project_vulnerabilities_path, user), params: { per_page: 40 }
 
         expect(response).to have_gitlab_http_status(200)
         expect(response).to include_pagination_headers
@@ -54,17 +56,17 @@ describe API::Vulnerabilities do
 
       it 'does not have N+1 queries' do
         control_count = ActiveRecord::QueryRecorder.new do
-          get api("/projects/#{project.id}/vulnerabilities", user), params: { report_type: 'dependency_scanning' }
+          get api(project_vulnerabilities_path, user), params: { report_type: 'dependency_scanning' }
         end.count
 
-        expect { get api("/projects/#{project.id}/vulnerabilities", user) }.not_to exceed_query_limit(control_count)
+        expect { get api(project_vulnerabilities_path, user) }.not_to exceed_query_limit(control_count)
       end
 
       describe 'filtering' do
         it 'returns vulnerabilities with sast report_type' do
           occurrence_count = (sast_report.occurrences.count - 1).to_s
 
-          get api("/projects/#{project.id}/vulnerabilities", user), params: { report_type: 'sast' }
+          get api(project_vulnerabilities_path, user), params: { report_type: 'sast' }
 
           expect(response).to have_gitlab_http_status(200)
 
@@ -80,7 +82,7 @@ describe API::Vulnerabilities do
         it 'returns vulnerabilities with dependency_scanning report_type' do
           occurrence_count = ds_report.occurrences.count.to_s
 
-          get api("/projects/#{project.id}/vulnerabilities", user), params: { report_type: 'dependency_scanning' }
+          get api(project_vulnerabilities_path, user), params: { report_type: 'dependency_scanning' }
 
           expect(response).to have_gitlab_http_status(200)
 
@@ -93,10 +95,16 @@ describe API::Vulnerabilities do
           expect(json_response.first['name']).to eq 'ruby-ffi DDL loading issue on Windows OS'
         end
 
+        it 'returns a "bad request" response for an unknown report type' do
+          get api(project_vulnerabilities_path, user), params: { report_type: 'blah' }
+
+          expect(response).to have_gitlab_http_status(400)
+        end
+
         it 'returns dismissed vulnerabilities with `all` scope' do
           occurrence_count = (sast_report.occurrences.count + ds_report.occurrences.count).to_s
 
-          get api("/projects/#{project.id}/vulnerabilities", user), params: { per_page: 40, scope: 'all' }
+          get api(project_vulnerabilities_path, user), params: { per_page: 40, scope: 'all' }
 
           expect(response).to have_gitlab_http_status(200)
 
@@ -104,26 +112,38 @@ describe API::Vulnerabilities do
         end
 
         it 'returns vulnerabilities with low severity' do
-          get api("/projects/#{project.id}/vulnerabilities", user), params: { per_page: 40, severity: 'low' }
+          get api(project_vulnerabilities_path, user), params: { per_page: 40, severity: 'low' }
 
           expect(response).to have_gitlab_http_status(200)
 
           expect(json_response.map { |v| v['severity'] }.uniq).to eq %w[low]
         end
 
+        it 'returns a "bad request" response for an unknown severity value' do
+          get api(project_vulnerabilities_path, user), params: { severity: 'foo' }
+
+          expect(response).to have_gitlab_http_status(400)
+        end
+
         it 'returns vulnerabilities with high confidence' do
-          get api("/projects/#{project.id}/vulnerabilities", user), params: { per_page: 40, confidence: 'high' }
+          get api(project_vulnerabilities_path, user), params: { per_page: 40, confidence: 'high' }
 
           expect(response).to have_gitlab_http_status(200)
 
           expect(json_response.map { |v| v['confidence'] }.uniq).to eq %w[high]
         end
 
+        it 'returns a "bad request" response for an unknown confidence value' do
+          get api(project_vulnerabilities_path, user), params: { confidence: 'qux' }
+
+          expect(response).to have_gitlab_http_status(400)
+        end
+
         context 'when pipeline_id is supplied' do
           it 'returns vulnerabilities from supplied pipeline' do
             occurrence_count = (sast_report.occurrences.count + ds_report.occurrences.count - 1).to_s
 
-            get api("/projects/#{project.id}/vulnerabilities", user), params: { per_page: 40, pipeline_id: pipeline.id }
+            get api(project_vulnerabilities_path, user), params: { per_page: 40, pipeline_id: pipeline.id }
 
             expect(response).to have_gitlab_http_status(200)
 
@@ -132,7 +152,7 @@ describe API::Vulnerabilities do
 
           context 'pipeline has no reports' do
             it 'returns empty results' do
-              get api("/projects/#{project.id}/vulnerabilities", user), params: { per_page: 40, pipeline_id: pipeline_without_vulnerabilities.id }
+              get api(project_vulnerabilities_path, user), params: { per_page: 40, pipeline_id: pipeline_without_vulnerabilities.id }
 
               expect(json_response).to eq []
             end
@@ -140,7 +160,7 @@ describe API::Vulnerabilities do
 
           context 'with unknown pipeline' do
             it 'returns empty results' do
-              get api("/projects/#{project.id}/vulnerabilities", user), params: { per_page: 40, pipeline_id: 0 }
+              get api(project_vulnerabilities_path, user), params: { per_page: 40, pipeline_id: 0 }
 
               expect(json_response).to eq []
             end
@@ -156,7 +176,7 @@ describe API::Vulnerabilities do
       end
 
       it 'responds with 403 Forbidden' do
-        get api("/projects/#{project.id}/vulnerabilities", user)
+        get api(project_vulnerabilities_path, user)
 
         expect(response).to have_gitlab_http_status(403)
       end
@@ -166,7 +186,7 @@ describe API::Vulnerabilities do
       it 'responds with 404 Not Found' do
         private_project = create(:project)
 
-        get api("/projects/#{private_project.id}/vulnerabilities", user)
+        get api("/projects/#{private_project.id}/#{api_resource_name}", user)
 
         expect(response).to have_gitlab_http_status(404)
       end
@@ -174,10 +194,46 @@ describe API::Vulnerabilities do
 
     context 'with unknown project' do
       it 'responds with 404 Not Found' do
-        get api("/projects/0/vulnerabilities", user)
+        get api("/projects/0/#{api_resource_name}", user)
 
         expect(response).to have_gitlab_http_status(404)
       end
     end
   end
+
+  shared_examples 'not found vulnerabilities endpoint' do
+    it do
+      get api("/projects/#{project.id}/#{api_resource_name}?", user), params: { per_page: 40 }
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+  end
+
+  describe "GET /projects/:id/vulnerabilities" do
+    let(:api_resource_name) { 'vulnerabilities' }
+
+    it_behaves_like 'not found vulnerabilities endpoint'
+
+    context 'when vulnerability findings API is disabled' do
+      before do
+        stub_feature_flags(first_class_vulnerabilities: false)
+      end
+
+      it_behaves_like 'getting list of vulnerability findings'
+    end
+  end
+
+  describe "GET /projects/:id/vulnerability_findings" do
+    let(:api_resource_name) { 'vulnerability_findings' }
+
+    it_behaves_like 'getting list of vulnerability findings'
+
+    context 'when vulnerability findings API is disabled' do
+      before do
+        stub_feature_flags(first_class_vulnerabilities: false)
+      end
+
+      it_behaves_like 'not found vulnerabilities endpoint'
+    end
+  end
 end

ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'projects/pipelines/_tabs_content' do
+  set(:user) { create(:user) }
+  let(:pipeline) { create(:ci_pipeline).present(current_user: user) }
+  let(:locals) { { pipeline: pipeline, project: pipeline.project } }
+
+  before do
+    allow(pipeline).to receive(:expose_security_dashboard?).and_return(true)
+  end
+
+  shared_examples 'rendering the appropriate API endpoint path' do
+    it do
+      render partial: 'projects/pipelines/tabs_content', locals: locals
+
+      expect(rendered).to include expected_api_path
+    end
+  end
+
+  context 'when Vulnerability Findings API enabled' do
+    it_behaves_like 'rendering the appropriate API endpoint path' do
+      let(:expected_api_path) { "projects/#{pipeline.project_id}/vulnerability_findings" }
+    end
+  end
+
+  context 'when the Vulnerability Findings API is disabled' do
+    before do
+      stub_feature_flags(first_class_vulnerabilities: false)
+    end
+
+    it_behaves_like 'rendering the appropriate API endpoint path' do
+      let(:expected_api_path) { "projects/#{pipeline.project_id}/vulnerabilities" }
+    end
+  end
+end