Merge branch '10242-1-rename-vulns-to-findings-in-projects-routes' into 'master'

Rename Vulnerabilities to Findings in Projects security routes

See merge request gitlab-org/gitlab!17351

ee/app/controllers/concerns/vulnerabilities_api_feature_gate.rb

+# frozen_string_literal: true
+
+# TODO: remove this module and its usages when :first_class_vulnerabilities feature flag is removed
+# https://gitlab.com/gitlab-org/gitlab/issues/33488
+module VulnerabilitiesApiFeatureGate
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :verify_vulnerabilities_action_enabled!
+
+    def verify_vulnerabilities_action_enabled!
+      access_denied! unless vulnerabilities_action_enabled?
+    end
+
+    def vulnerabilities_action_enabled?
+      raise NotImplementedError('Must be implemented in the including controller class')
+    end
+  end
+end

ee/app/controllers/concerns/vulnerabilities_actions.rb → ee/app/controllers/concerns/vulnerability_findings_actions.rb

 # frozen_string_literal: true
 
-# The VulnerabilitiesActions concern contains actions that are used to populate vulnerabilities
+# The VulnerabilityFindingsActions concern contains actions that are used to populate findings
 # on security dashboards.
 #
 # Note: Consumers of this module will need to define a `def vulnerable` method, which must return
 # an object with an interface that matches the one provided by the Vulnerable model concern.
 
-module VulnerabilitiesActions
+module VulnerabilityFindingsActions
   extend ActiveSupport::Concern
 
   def index
-    vulnerabilities = found_vulnerabilities(:with_sha).ordered.page(params[:page])
+    findings = vulnerability_findings(:with_sha).ordered.page(params[:page])
 
     respond_to do |format|
       format.json do
         render json: Vulnerabilities::OccurrenceSerializer
           .new(current_user: current_user)
           .with_pagination(request, response)
-          .represent(vulnerabilities, preload: true)
+          .represent(findings, preload: true)
       end
     end
   end
 
   def summary
-    vulnerabilities_summary = found_vulnerabilities.counted_by_severity
+    vulnerabilities_summary = vulnerability_findings.counted_by_severity
 
     respond_to do |format|
       format.json do
@@ -39,7 +39,7 @@ module VulnerabilitiesActions
       .merge(hide_dismissed: ::Gitlab::Utils.to_boolean(params[:hide_dismissed]))
   end
 
-  def found_vulnerabilities(collection = :latest)
+  def vulnerability_findings(collection = :latest)
     ::Security::VulnerabilitiesFinder.new(vulnerable, params: filter_params).execute(collection)
   end
 end

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

@@ -2,7 +2,7 @@
 
 class Groups::Security::VulnerabilitiesController < Groups::ApplicationController
   include SecurityDashboardsPermissions
-  include VulnerabilitiesActions
+  include VulnerabilityFindingsActions
 
   alias_method :vulnerable, :group
 

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

 # frozen_string_literal: true
 
 class Projects::Security::VulnerabilitiesController < Projects::ApplicationController
+  include VulnerabilitiesApiFeatureGate # must come first
   include SecurityDashboardsPermissions
-  include VulnerabilitiesActions
+  include VulnerabilityFindingsActions
 
   alias_method :vulnerable, :project
+
+  private
+
+  def vulnerabilities_action_enabled?
+    Feature.disabled?(:first_class_vulnerabilities)
+  end
 end

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

+# frozen_string_literal: true
+
+class Projects::Security::VulnerabilityFindingsController < Projects::ApplicationController
+  include VulnerabilitiesApiFeatureGate # must come first
+  include SecurityDashboardsPermissions
+  include VulnerabilityFindingsActions
+
+  alias_method :vulnerable, :project
+
+  private
+
+  def vulnerabilities_action_enabled?
+    Feature.enabled?(:first_class_vulnerabilities)
+  end
+end

ee/app/helpers/ee/projects_helper.rb

@@ -181,8 +181,8 @@ module EE
       else
         {
           project: { id: project.id, name: project.name },
-          vulnerabilities_endpoint: project_security_vulnerabilities_path(project),
-          vulnerabilities_summary_endpoint: summary_project_security_vulnerabilities_path(project),
+          vulnerabilities_endpoint: project_vulnerabilities_endpoint_path(project),
+          vulnerabilities_summary_endpoint: project_vulnerabilities_summary_endpoint_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'),
@@ -202,6 +202,22 @@ module EE
       end
     end
 
+    def project_vulnerabilities_endpoint_path(project)
+      if ::Feature.enabled?(:first_class_vulnerabilities)
+        project_security_vulnerability_findings_path(project)
+      else
+        project_security_vulnerabilities_path(project)
+      end
+    end
+
+    def project_vulnerabilities_summary_endpoint_path(project)
+      if ::Feature.enabled?(:first_class_vulnerabilities)
+        summary_project_security_vulnerability_findings_path(project)
+      else
+        summary_project_security_vulnerabilities_path(project)
+      end
+    end
+
     def can_create_feedback?(project, feedback_type)
       feedback = Vulnerabilities::Feedback.new(project: project, feedback_type: feedback_type)
       can?(current_user, :create_vulnerability_feedback, feedback)

ee/config/routes/project.rb

@@ -89,11 +89,21 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       namespace :security do
         resources :dependencies, only: [:index]
         resources :licenses, only: [:index]
+        # We have to define both legacy and new routes for Vulnerability Findings
+        # because they are loaded upon application initialization and preloaded by
+        # web server.
+        # TODO: remove this comment and `resources :vulnerabilities` when applicable
+        # see https://gitlab.com/gitlab-org/gitlab/issues/33488
         resources :vulnerabilities, only: [:index] do
           collection do
             get :summary
           end
         end
+        resources :vulnerability_findings, only: [:index] do
+          collection do
+            get :summary
+          end
+        end
       end
     end
   end

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

@@ -6,7 +6,7 @@ describe Groups::Security::VulnerabilitiesController do
   let(:group) { create(:group) }
   let(:user) { create(:user) }
 
-  it_behaves_like VulnerabilitiesActions do
+  it_behaves_like VulnerabilityFindingsActions do
     let(:vulnerable) { group }
     let(:vulnerable_params) { { group_id: group } }
   end

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

@@ -6,13 +6,34 @@ describe Projects::Security::VulnerabilitiesController do
   let(:project) { create(:project) }
   let(:params) { { project_id: project, namespace_id: project.creator } }
 
-  it_behaves_like VulnerabilitiesActions do
+  # when new Vulnerability Findings API is enabled, this controller is not
+  # and its actions are "moved" to Projects::Security::VulnerabilityFindingsController
+
+  it_behaves_like 'VulnerabilityFindingsActions disabled' do
     let(:vulnerable) { project }
     let(:vulnerable_params) { params }
   end
 
-  it_behaves_like SecurityDashboardsPermissions do
+  it_behaves_like 'SecurityDashboardsPermissions disabled' do
     let(:vulnerable) { project }
     let(:security_dashboard_action) { get :index, params: params, format: :json }
   end
+
+  context 'when new Vulnerability Findings API is disabled' do
+    before do
+      stub_feature_flags(first_class_vulnerabilities: false)
+    end
+
+    # when new Vulnerability Findings API is disabled, we fall back to this controller
+
+    it_behaves_like VulnerabilityFindingsActions 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
 end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::Security::VulnerabilityFindingsController do
+  let(:project) { create(:project) }
+  let(:params) { { project_id: project, namespace_id: project.creator } }
+
+  # when new Vulnerability Findings API is enabled, this controller serves it
+
+  it_behaves_like VulnerabilityFindingsActions 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
+
+  context 'when new Vulnerability Findings API is disabled' do
+    before do
+      stub_feature_flags(first_class_vulnerabilities: false)
+    end
+
+    # new Vulnerability Findings API is disabled and we fall back to
+    # Projects::Security::VulnerabilitiesController
+
+    it_behaves_like 'VulnerabilityFindingsActions disabled' do
+      let(:vulnerable) { project }
+      let(:vulnerable_params) { params }
+    end
+
+    it_behaves_like 'SecurityDashboardsPermissions disabled' do
+      let(:vulnerable) { project }
+      let(:security_dashboard_action) { get :index, params: params, format: :json }
+    end
+  end
+end

ee/spec/helpers/projects_helper_spec.rb

@@ -132,6 +132,27 @@ describe ProjectsHelper do
         expect(subject[:security_dashboard_help_path]).to eq '/help/user/application_security/security_dashboard/index'
         expect(subject[:has_pipeline_data]).to eq 'true'
       end
+
+      context 'when new Vulnerability Findings API enabled' do
+        it 'returns new "vulnerability findings" endpoint paths' do
+          expect(subject[:vulnerabilities_endpoint]).to eq project_security_vulnerability_findings_path(project)
+          expect(subject[:vulnerabilities_summary_endpoint]).to(
+            eq(
+              summary_project_security_vulnerability_findings_path(project)
+            ))
+        end
+      end
+
+      context 'when new Vulnerability Findings API disabled' do
+        before do
+          stub_feature_flags(first_class_vulnerabilities: false)
+        end
+
+        it 'returns legacy "vulnerabilities" endpoint paths' do
+          expect(subject[:vulnerabilities_endpoint]).to eq project_security_vulnerabilities_path(project)
+          expect(subject[:vulnerabilities_summary_endpoint]).to eq summary_project_security_vulnerabilities_path(project)
+        end
+      end
     end
   end
 

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+shared_examples 'SecurityDashboardsPermissions disabled' do
+  include ApiHelpers
+
+  let(:security_dashboard_user) { create(:user) }
+
+  before do
+    sign_in(security_dashboard_user)
+  end
+
+  describe 'access for all actions' do
+    context 'when security dashboard feature is enabled' do
+      it 'returns 404' do
+        stub_licensed_features(security_dashboard: true)
+
+        security_dashboard_action
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+  end
+end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+shared_examples 'VulnerabilityFindingsActions disabled' do
+  include ApiHelpers
+  include VulnerableHelpers
+
+  let(:action_params) { vulnerable_params }
+  let(:user) { create(:user) }
+
+  before do
+    vulnerable.add_developer(user)
+
+    sign_in(user)
+    stub_licensed_features(security_dashboard: true)
+  end
+
+  describe 'GET index.json' do
+    subject { get :index, params: action_params, format: :json }
+
+    it 'is disabled and returns "not found"' do
+      subject
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+  end
+
+  describe 'GET summary.json' do
+    subject { get :summary, params: action_params, format: :json }
+
+    before do
+      subject
+    end
+
+    it 'is disabled and returns "not found"' do
+      expect(response).to have_gitlab_http_status(404)
+    end
+  end
+end

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

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-shared_examples VulnerabilitiesActions do
+shared_examples VulnerabilityFindingsActions do
   include ApiHelpers
   include VulnerableHelpers
 
@@ -21,7 +21,7 @@ shared_examples VulnerabilitiesActions do
   describe 'GET index.json' do
     subject { get :index, params: action_params, format: :json }
 
-    it 'returns an ordered list of vulnerabilities' do
+    it 'returns an ordered list of vulnerability findings' do
       critical_vulnerability = create(
         :vulnerabilities_occurrence,
         pipelines: [pipeline],
@@ -53,12 +53,12 @@ shared_examples VulnerabilitiesActions do
         Vulnerabilities::Occurrence.paginates_per Vulnerabilities::Occurrence::OCCURRENCES_PER_PAGE
       end
 
-      it 'returns the list of vulnerabilities that are on the requested page' do
+      it 'returns the list of vulnerability findings that are on the requested page' do
         expect(json_response.length).to eq 1
       end
     end
 
-    context 'when the vulnerabilities have feedback' do
+    context 'when the vulnerability findings have feedback' do
       before do
         vulnerability = create(:vulnerabilities_occurrence, pipelines: [pipeline], project: vulnerable_project, report_type: :sast)
         create(:vulnerability_feedback,
@@ -98,7 +98,7 @@ shared_examples VulnerabilitiesActions do
       context 'with a single report filter' do
         let(:action_params) { vulnerable_params.merge(report_type: ['sast']) }
 
-        it 'returns a list of vulnerabilities for that reporty type only' do
+        it 'returns a list of vulnerability findings for that report type only' do
           expect(json_response.length).to eq 1
           expect(json_response.map { |v| v['report_type'] }.uniq).to contain_exactly('sast')
         end
@@ -107,7 +107,7 @@ shared_examples VulnerabilitiesActions do
       context 'with multiple report filters' do
         let(:action_params) { vulnerable_params.merge(report_type: %w[sast dependency_scanning]) }
 
-        it 'returns a list of vulnerabilities for all filtered upon types' do
+        it 'returns a list of vulnerability findings for all filtered upon types' do
           expect(json_response.length).to eq 2
           expect(json_response.map { |v| v['report_type'] }.uniq).to contain_exactly('sast', 'dependency_scanning')
         end
@@ -131,7 +131,7 @@ shared_examples VulnerabilitiesActions do
       subject
     end
 
-    it 'returns vulnerabilities counts for all report types' do
+    it 'returns vulnerability findings counts for all report types' do
       expect(response).to have_gitlab_http_status(200)
       expect(json_response['high']).to eq(3)
       expect(json_response['low']).to eq(2)
@@ -142,7 +142,7 @@ shared_examples VulnerabilitiesActions do
     context 'with enabled filters' do
       let(:action_params) { vulnerable_params.merge(report_type: %w[sast dast], severity: %[high low]) }
 
-      it 'returns counts for filtered vulnerabilities' do
+      it 'returns counts for filtered vulnerability findings' do
         expect(json_response['high']).to eq(3)
         expect(json_response['low']).to eq(0)
         expect(json_response['medium']).to eq(2)