From c493b6f6cee9fd4298d2b6852be429f8778f09bf Mon Sep 17 00:00:00 2001
From: Victor Zagorodny <vzagorodny@gitlab.com>
Date: Wed, 11 Sep 2019 20:58:34 +0300
Subject: [PATCH 1/6] Rename Vulnerabilities API to VulnerabilityFindings

---
 ...abilities.rb => vulnerability_findings.rb} | 88 +++++++++++++------
 ee/lib/ee/api/api.rb                          |  2 +-
 ...spec.rb => vulnerability_findings_spec.rb} | 70 +++++++++++----
 3 files changed, 114 insertions(+), 46 deletions(-)
 rename ee/lib/api/{vulnerabilities.rb => vulnerability_findings.rb} (68%)
 rename ee/spec/requests/api/{vulnerabilities_spec.rb => vulnerability_findings_spec.rb} (69%)

diff --git a/ee/lib/api/vulnerabilities.rb b/ee/lib/api/vulnerability_findings.rb
similarity index 68%
rename from ee/lib/api/vulnerabilities.rb
rename to ee/lib/api/vulnerability_findings.rb
index 12e9dcd3007252..a2c8d766f7cc33 100644
--- a/ee/lib/api/vulnerabilities.rb
+++ b/ee/lib/api/vulnerability_findings.rb
@@ -1,10 +1,26 @@
 # 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', 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
+      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 +32,28 @@ def vulnerability_occurrences_by(params)
 
         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 +65,32 @@ def vulnerability_occurrences_by(params)
     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?(:vulnerability_findings_api)
+          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?(:vulnerability_findings_api)
+          respond_with_vulnerability_findings
+        else
+          not_found!
+        end
       end
     end
   end
diff --git a/ee/lib/ee/api/api.rb b/ee/lib/ee/api/api.rb
index ca946f97de8ca8..bc7de4f1e07a31 100644
--- a/ee/lib/ee/api/api.rb
+++ b/ee/lib/ee/api/api.rb
@@ -35,7 +35,7 @@ module API
         mount ::API::Scim
         mount ::API::ManagedLicenses
         mount ::API::ProjectApprovals
-        mount ::API::Vulnerabilities
+        mount ::API::VulnerabilityFindings
         mount ::API::MergeRequestApprovals
         mount ::API::MergeRequestApprovalRules
         mount ::API::ProjectAliases
diff --git a/ee/spec/requests/api/vulnerabilities_spec.rb b/ee/spec/requests/api/vulnerability_findings_spec.rb
similarity index 69%
rename from ee/spec/requests/api/vulnerabilities_spec.rb
rename to ee/spec/requests/api/vulnerability_findings_spec.rb
index 3d7703efe16778..f2b5cb3d219d07 100644
--- a/ee/spec/requests/api/vulnerabilities_spec.rb
+++ b/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 @@
     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 @@
       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 @@
 
       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 @@
         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)
 
@@ -96,7 +98,7 @@
         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,7 +106,7 @@
         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)
 
@@ -112,7 +114,7 @@
         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)
 
@@ -123,7 +125,7 @@
           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 +134,7 @@
 
           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 +142,7 @@
 
           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 +158,7 @@
       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 +168,7 @@
       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 +176,46 @@
 
     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(vulnerability_findings_api: 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(vulnerability_findings_api: false)
+      end
+
+      it_behaves_like 'not found vulnerabilities endpoint'
+    end
+  end
 end
-- 
GitLab


From a063d85305ff06e9043efd759cbe3d7288a5f9b1 Mon Sep 17 00:00:00 2001
From: Victor Zagorodny <vzagorodny@gitlab.com>
Date: Tue, 17 Sep 2019 10:54:41 +0300
Subject: [PATCH 2/6] Rename vulnerabilities endpoint in pipeline tabs

---
 .../pipelines/_tabs_content.html.haml         |  8 +++-
 .../pipelines/_tabs_content.html.haml_spec.rb | 37 +++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb

diff --git a/ee/app/views/projects/pipelines/_tabs_content.html.haml b/ee/app/views/projects/pipelines/_tabs_content.html.haml
index 2b1902c0b078b2..7a8b7f6872e287 100644
--- a/ee/app/views/projects/pipelines/_tabs_content.html.haml
+++ b/ee/app/views/projects/pipelines/_tabs_content.html.haml
@@ -9,6 +9,12 @@
 - license_management_settings_path = can?(current_user, :admin_software_license_policy, project) ? license_management_settings_path(project) : nil
 - licenses_api_path = licenses_project_pipeline_path(project, pipeline) if project.feature_available?(:license_management)
 
+- vulnerability_endpoint_path_params = { id: project.id, params: { pipeline_id: pipeline.id, scope: 'dismissed' }}
+- if Feature.enabled?(:vulnerability_findings_api)
+  - vulnerabilities_endpoint_path = expose_path(api_v4_projects_vulnerability_findings_path(vulnerability_endpoint_path_params))
+- else
+  - vulnerabilities_endpoint_path = expose_path(api_v4_projects_vulnerabilities_path(vulnerability_endpoint_path_params))
+
 - if pipeline.expose_security_dashboard?
   #js-tab-security.build-security.tab-pane
     - if Feature.enabled?(:pipeline_report_api, default_enabled: true)
@@ -16,7 +22,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: vulnerabilities_endpoint_path,
       vulnerability_feedback_help_path: help_page_path('user/application_security/index') } }
     - else
       #js-security-report-app{ data: { head_blob_path: blob_path,
diff --git a/ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb b/ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb
new file mode 100644
index 00000000000000..99cdf0c4a39bb6
--- /dev/null
+++ b/ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb
@@ -0,0 +1,37 @@
+# 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(vulnerability_findings_api: false)
+    end
+
+    it_behaves_like 'rendering the appropriate API endpoint path' do
+      let(:expected_api_path) { "projects/#{pipeline.project_id}/vulnerabilities" }
+    end
+  end
+end
-- 
GitLab


From 14e4bf6980602343287295637111a402078d1a69 Mon Sep 17 00:00:00 2001
From: Victor Zagorodny <vzagorodny@gitlab.com>
Date: Fri, 4 Oct 2019 15:30:44 +0300
Subject: [PATCH 3/6] Extract vulnerability findings path into a helper

---
 ee/app/helpers/ee/projects_helper.rb          | 10 +++++
 .../pipelines/_tabs_content.html.haml         |  8 +---
 ee/spec/helpers/projects_helper_spec.rb       | 38 +++++++++++++++----
 3 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/ee/app/helpers/ee/projects_helper.rb b/ee/app/helpers/ee/projects_helper.rb
index 199760c8ad5dc5..ee9dc76e36bb8c 100644
--- a/ee/app/helpers/ee/projects_helper.rb
+++ b/ee/app/helpers/ee/projects_helper.rb
@@ -235,5 +235,15 @@ def project_incident_management_setting
     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?(:vulnerability_findings_api)
+               api_v4_projects_vulnerability_findings_path(params)
+             else
+               api_v4_projects_vulnerabilities_path(params)
+             end
+      expose_path(path)
+    end
   end
 end
diff --git a/ee/app/views/projects/pipelines/_tabs_content.html.haml b/ee/app/views/projects/pipelines/_tabs_content.html.haml
index 7a8b7f6872e287..bc402388712b6e 100644
--- a/ee/app/views/projects/pipelines/_tabs_content.html.haml
+++ b/ee/app/views/projects/pipelines/_tabs_content.html.haml
@@ -9,12 +9,6 @@
 - license_management_settings_path = can?(current_user, :admin_software_license_policy, project) ? license_management_settings_path(project) : nil
 - licenses_api_path = licenses_project_pipeline_path(project, pipeline) if project.feature_available?(:license_management)
 
-- vulnerability_endpoint_path_params = { id: project.id, params: { pipeline_id: pipeline.id, scope: 'dismissed' }}
-- if Feature.enabled?(:vulnerability_findings_api)
-  - vulnerabilities_endpoint_path = expose_path(api_v4_projects_vulnerability_findings_path(vulnerability_endpoint_path_params))
-- else
-  - vulnerabilities_endpoint_path = expose_path(api_v4_projects_vulnerabilities_path(vulnerability_endpoint_path_params))
-
 - if pipeline.expose_security_dashboard?
   #js-tab-security.build-security.tab-pane
     - if Feature.enabled?(:pipeline_report_api, default_enabled: true)
@@ -22,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: vulnerabilities_endpoint_path,
+      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,
diff --git a/ee/spec/helpers/projects_helper_spec.rb b/ee/spec/helpers/projects_helper_spec.rb
index 0182eccdafb7cb..be1672c53763d8 100644
--- a/ee/spec/helpers/projects_helper_spec.rb
+++ b/ee/spec/helpers/projects_helper_spec.rb
@@ -97,18 +97,24 @@
     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 @@
       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(vulnerability_findings_api: false)
+      end
+
+      it { is_expected.to include("projects/#{project.id}/vulnerabilities") }
+    end
+  end
 end
-- 
GitLab


From c0c9dde56ecba496d212377b8546e7c4eacb1dd4 Mon Sep 17 00:00:00 2001
From: Victor Zagorodny <vzagorodny@gitlab.com>
Date: Sun, 6 Oct 2019 18:48:17 +0300
Subject: [PATCH 4/6] Fix Rubocop offenses

---
 ee/app/helpers/ee/projects_helper.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ee/app/helpers/ee/projects_helper.rb b/ee/app/helpers/ee/projects_helper.rb
index ee9dc76e36bb8c..e063578b1e6ada 100644
--- a/ee/app/helpers/ee/projects_helper.rb
+++ b/ee/app/helpers/ee/projects_helper.rb
@@ -237,12 +237,14 @@ def can_import_members?
     end
 
     def api_projects_vulnerability_findings_path(project, pipeline)
-      params = { id: project.id, params: { pipeline_id: pipeline.id, scope: 'dismissed' }}
+      params = { id: project.id, params: { pipeline_id: pipeline.id, scope: 'dismissed' } }
+
       path = if ::Feature.enabled?(:vulnerability_findings_api)
                api_v4_projects_vulnerability_findings_path(params)
              else
                api_v4_projects_vulnerabilities_path(params)
              end
+
       expose_path(path)
     end
   end
-- 
GitLab


From 59036634c63435d2f01955b3fe3215348749c901 Mon Sep 17 00:00:00 2001
From: Victor Zagorodny <vzagorodny@gitlab.com>
Date: Mon, 7 Oct 2019 21:17:12 +0300
Subject: [PATCH 5/6] Rename feature flag to first_class_vulnerabilities

---
 ee/app/helpers/ee/projects_helper.rb                          | 2 +-
 ee/lib/api/vulnerability_findings.rb                          | 4 ++--
 ee/spec/helpers/projects_helper_spec.rb                       | 2 +-
 ee/spec/requests/api/vulnerability_findings_spec.rb           | 4 ++--
 .../views/projects/pipelines/_tabs_content.html.haml_spec.rb  | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/ee/app/helpers/ee/projects_helper.rb b/ee/app/helpers/ee/projects_helper.rb
index e063578b1e6ada..fdd8c2a523338c 100644
--- a/ee/app/helpers/ee/projects_helper.rb
+++ b/ee/app/helpers/ee/projects_helper.rb
@@ -239,7 +239,7 @@ def can_import_members?
     def api_projects_vulnerability_findings_path(project, pipeline)
       params = { id: project.id, params: { pipeline_id: pipeline.id, scope: 'dismissed' } }
 
-      path = if ::Feature.enabled?(:vulnerability_findings_api)
+      path = if ::Feature.enabled?(:first_class_vulnerabilities)
                api_v4_projects_vulnerability_findings_path(params)
              else
                api_v4_projects_vulnerabilities_path(params)
diff --git a/ee/lib/api/vulnerability_findings.rb b/ee/lib/api/vulnerability_findings.rb
index a2c8d766f7cc33..419446dad0d1ff 100644
--- a/ee/lib/api/vulnerability_findings.rb
+++ b/ee/lib/api/vulnerability_findings.rb
@@ -72,7 +72,7 @@ def respond_with_vulnerability_findings
         success ::Vulnerabilities::OccurrenceEntity
       end
       get ':id/vulnerabilities' do
-        if Feature.enabled?(:vulnerability_findings_api)
+        if Feature.enabled?(:first_class_vulnerabilities)
           respond_with_vulnerabilities
         else
           respond_with_vulnerability_findings
@@ -86,7 +86,7 @@ def respond_with_vulnerability_findings
         success ::Vulnerabilities::OccurrenceEntity
       end
       get ':id/vulnerability_findings' do
-        if Feature.enabled?(:vulnerability_findings_api)
+        if Feature.enabled?(:first_class_vulnerabilities)
           respond_with_vulnerability_findings
         else
           not_found!
diff --git a/ee/spec/helpers/projects_helper_spec.rb b/ee/spec/helpers/projects_helper_spec.rb
index be1672c53763d8..70d601dc97e1a0 100644
--- a/ee/spec/helpers/projects_helper_spec.rb
+++ b/ee/spec/helpers/projects_helper_spec.rb
@@ -146,7 +146,7 @@
 
     context 'when the Vulnerability Findings API is disabled' do
       before do
-        stub_feature_flags(vulnerability_findings_api: false)
+        stub_feature_flags(first_class_vulnerabilities: false)
       end
 
       it { is_expected.to include("projects/#{project.id}/vulnerabilities") }
diff --git a/ee/spec/requests/api/vulnerability_findings_spec.rb b/ee/spec/requests/api/vulnerability_findings_spec.rb
index f2b5cb3d219d07..0203d1e5a7a355 100644
--- a/ee/spec/requests/api/vulnerability_findings_spec.rb
+++ b/ee/spec/requests/api/vulnerability_findings_spec.rb
@@ -198,7 +198,7 @@
 
     context 'when vulnerability findings API is disabled' do
       before do
-        stub_feature_flags(vulnerability_findings_api: false)
+        stub_feature_flags(first_class_vulnerabilities: false)
       end
 
       it_behaves_like 'getting list of vulnerability findings'
@@ -212,7 +212,7 @@
 
     context 'when vulnerability findings API is disabled' do
       before do
-        stub_feature_flags(vulnerability_findings_api: false)
+        stub_feature_flags(first_class_vulnerabilities: false)
       end
 
       it_behaves_like 'not found vulnerabilities endpoint'
diff --git a/ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb b/ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb
index 99cdf0c4a39bb6..b8e66a973c77bb 100644
--- a/ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb
+++ b/ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb
@@ -27,7 +27,7 @@
 
   context 'when the Vulnerability Findings API is disabled' do
     before do
-      stub_feature_flags(vulnerability_findings_api: false)
+      stub_feature_flags(first_class_vulnerabilities: false)
     end
 
     it_behaves_like 'rendering the appropriate API endpoint path' do
-- 
GitLab


From 8b1b9a2ec4fc4e2b9ecdfe719b20458691d0800b Mon Sep 17 00:00:00 2001
From: Victor Zagorodny <vzagorodny@gitlab.com>
Date: Mon, 7 Oct 2019 22:09:34 +0300
Subject: [PATCH 6/6] Add missing values validation w/ tests for it

---
 ee/lib/api/vulnerability_findings.rb           | 16 ++++++++++++----
 .../api/vulnerability_findings_spec.rb         | 18 ++++++++++++++++++
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/ee/lib/api/vulnerability_findings.rb b/ee/lib/api/vulnerability_findings.rb
index 419446dad0d1ff..5322d1cafce016 100644
--- a/ee/lib/api/vulnerability_findings.rb
+++ b/ee/lib/api/vulnerability_findings.rb
@@ -6,15 +6,23 @@ class VulnerabilityFindings < Grape::API
 
     helpers do
       params :vulnerability_findings_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 :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',
+                 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',
+                 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'
 
diff --git a/ee/spec/requests/api/vulnerability_findings_spec.rb b/ee/spec/requests/api/vulnerability_findings_spec.rb
index 0203d1e5a7a355..d10c3a712342ff 100644
--- a/ee/spec/requests/api/vulnerability_findings_spec.rb
+++ b/ee/spec/requests/api/vulnerability_findings_spec.rb
@@ -95,6 +95,12 @@
           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
 
@@ -113,6 +119,12 @@
           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(project_vulnerabilities_path, user), params: { per_page: 40, confidence: 'high' }
 
@@ -121,6 +133,12 @@
           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
-- 
GitLab