Merge branch 'most-affected-projects' into 'master'

Add endpoint for a group's vulnerable projects in the API

See merge request gitlab-org/gitlab!15317

changelogs/unreleased/most-affected-projects.yml

+---
+title: Add endpoint for a group's vulnerable projects
+merge_request: 15317
+author:
+type: added

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

+# frozen_string_literal: true
+
+class Groups::Security::VulnerableProjectsController < Groups::ApplicationController
+  include SecurityDashboardsPermissions
+
+  alias_method :vulnerable, :group
+
+  def index
+    projects = group.vulnerable_projects.non_archived.without_deleted.with_route
+
+    vulnerable_projects = projects.map do |project|
+      ::Security::VulnerableProjectPresenter.new(project)
+    end
+
+    render json: VulnerableProjectSerializer.new.represent(vulnerable_projects)
+  end
+end

ee/app/models/ee/group.rb

@@ -118,6 +118,10 @@ module EE
       ip_restrictions.map(&:range).join(",")
     end
 
+    def vulnerable_projects
+      projects.where("EXISTS(?)", ::Vulnerabilities::Occurrence.select(1).undismissed.where('vulnerability_occurrences.project_id = projects.id'))
+    end
+
     def human_ldap_access
       ::Gitlab::Access.options_with_owner.key(ldap_access)
     end

ee/app/models/vulnerabilities/occurrence.rb

@@ -139,6 +139,33 @@ module Vulnerabilities
       end
     end
 
+    def self.undismissed
+      where(
+        "NOT EXISTS (?)",
+        Feedback.select(1)
+        .where("ENCODE(#{table_name}.project_fingerprint, 'HEX') = vulnerability_feedback.project_fingerprint") # rubocop:disable GitlabSecurity/SqlInjection
+        .for_dismissal
+      )
+    end
+
+    def self.batch_count_by_project_and_severity(project_id, severity)
+      BatchLoader.for(project_id: project_id, severity: severity).batch(default_value: 0) do |items, loader|
+        project_ids = items.map { |i| i[:project_id] }.uniq
+        severities = items.map { |i| i[:severity] }.uniq
+
+        counts = undismissed
+          .by_severities(severities)
+          .by_projects(project_ids)
+          .group(:project_id, :severity)
+          .count
+
+        counts.each do |(found_project_id, found_severity), count|
+          loader_key = { project_id: found_project_id, severity: found_severity }
+          loader.call(loader_key, count)
+        end
+      end
+    end
+
     def feedback(feedback_type:)
       params = {
         project_id: project_id,

ee/app/presenters/security/vulnerable_project_presenter.rb

+# frozen_string_literal: true
+
+module Security
+  class VulnerableProjectPresenter < ::Gitlab::View::Presenter::Delegated
+    SEVERITY_LEVELS = ::Vulnerabilities::Occurrence::SEVERITY_LEVELS.keys
+
+    presents :project
+
+    def initialize(project)
+      super(project, counts_for_project(project))
+    end
+
+    private
+
+    def counts_for_project(project)
+      SEVERITY_LEVELS.each_with_object({}) do |severity, counts|
+        counts["#{severity}_vulnerability_count".to_sym] = ::Vulnerabilities::Occurrence.batch_count_by_project_and_severity(project.id, severity)
+      end
+    end
+  end
+end

ee/app/serializers/vulnerable_project_entity.rb

+# frozen_string_literal: true
+
+class VulnerableProjectEntity < ProjectEntity
+  ::Vulnerabilities::Occurrence::SEVERITY_LEVELS.each_key do |severity_level|
+    expose "#{severity_level}_vulnerability_count"
+  end
+end

ee/app/serializers/vulnerable_project_serializer.rb

+# frozen_string_literal: true
+
+class VulnerableProjectSerializer < BaseSerializer
+  entity VulnerableProjectEntity
+end

ee/config/routes/group.rb

@@ -110,6 +110,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
 
     namespace :security do
       resource :dashboard, only: [:show], controller: :dashboard
+      resources :vulnerable_projects, 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.

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Groups::Security::VulnerableProjectsController do
+  let(:group) { create(:group) }
+  let(:user) { create(:user) }
+
+  it_behaves_like SecurityDashboardsPermissions do
+    let(:vulnerable) { group }
+    let(:security_dashboard_action) { get :index, params: { group_id: group }, format: :json }
+  end
+
+  describe '#index' do
+    before do
+      stub_licensed_features(security_dashboard: true)
+
+      group.add_developer(user)
+      sign_in(user)
+    end
+
+    subject { get :index, params: { group_id: group }, format: :json }
+
+    it "responds with a list of the group's most vulnerable projects" do
+      _ungrouped_project = create(:project)
+      _safe_project = create(:project, namespace: group)
+      vulnerable_project = create(:project, namespace: group)
+      create_list(:vulnerabilities_occurrence, 2, project: vulnerable_project, severity: :critical)
+
+      subject
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(json_response.count).to be(1)
+      expect(json_response.first['id']).to eq(vulnerable_project.id)
+      expect(json_response.first['full_path']).to eq(project_path(vulnerable_project))
+      expect(json_response.first['critical_vulnerability_count']).to eq(2)
+    end
+
+    it 'does not include archived or deleted projects' do
+      archived_project = create(:project, :archived, namespace: group)
+      deleted_project = create(:project, namespace: group, pending_delete: true)
+      create(:vulnerabilities_occurrence, project: archived_project)
+      create(:vulnerabilities_occurrence, project: deleted_project)
+
+      subject
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(json_response).to be_empty
+    end
+  end
+end

ee/spec/models/group_spec.rb

@@ -142,6 +142,47 @@ describe Group do
     end
   end
 
+  describe '#vulnerable_projects' do
+    it "fetches the group's projects that have vulnerabilities" do
+      vulnerable_project = create(:project, namespace: group)
+      _safe_project = create(:project, namespace: group)
+      create(:vulnerabilities_occurrence, project: vulnerable_project)
+
+      vulnerable_projects = group.vulnerable_projects
+
+      expect(vulnerable_projects.count).to be(1)
+      expect(vulnerable_projects.first).to eq(vulnerable_project)
+    end
+
+    it 'does not include projects that only have dismissed vulnerabilities' do
+      project = create(:project, namespace: group)
+      vulnerability = create(:vulnerabilities_occurrence, project: project)
+      create(
+        :vulnerability_feedback,
+        project_fingerprint: vulnerability.project_fingerprint,
+        feedback_type: :dismissal
+      )
+
+      vulnerable_projects = group.vulnerable_projects
+
+      expect(vulnerable_projects).to be_empty
+    end
+
+    it 'only uses 1 query' do
+      project_one = create(:project, namespace: group)
+      project_two = create(:project, namespace: group)
+      create(:vulnerabilities_occurrence, project: project_one)
+      dismissed_vulnerability = create(:vulnerabilities_occurrence, project: project_two)
+      create(
+        :vulnerability_feedback,
+        project_fingerprint: dismissed_vulnerability.project_fingerprint,
+        feedback_type: :dismissal
+      )
+
+      expect { group.vulnerable_projects }.not_to exceed_query_limit(1)
+    end
+  end
+
   describe '#mark_ldap_sync_as_failed' do
     it 'sets the state to failed' do
       group.start_ldap_sync

ee/spec/models/vulnerabilities/occurrence_spec.rb

@@ -280,6 +280,71 @@ describe Vulnerabilities::Occurrence do
     end
   end
 
+  describe '.undismissed' do
+    it 'returns occurrences that do not have a corresponding dismissal feedback' do
+      undismissed_occurrence = create(:vulnerabilities_occurrence)
+      dismissed_occurrence = create(:vulnerabilities_occurrence)
+      create(:vulnerability_feedback, project_fingerprint: dismissed_occurrence.project_fingerprint)
+
+      expect(described_class.undismissed).to contain_exactly(undismissed_occurrence)
+    end
+  end
+
+  describe '.batch_count_by_project_and_severity' do
+    let(:project) { create(:project) }
+
+    it 'fetches a vulnerability count for the given project and severity' do
+      create(:vulnerabilities_occurrence, project: project, severity: :high)
+
+      count = described_class.batch_count_by_project_and_severity(project.id, 'high')
+
+      expect(count).to be(1)
+    end
+
+    it 'returns 0 when there are no vulnerabilities for that severity level' do
+      count = described_class.batch_count_by_project_and_severity(project.id, 'high')
+
+      expect(count).to be(0)
+    end
+
+    it 'batch loads the counts' do
+      projects = create_list(:project, 2)
+
+      projects.each do |project|
+        create(:vulnerabilities_occurrence, project: project, severity: :high)
+        create(:vulnerabilities_occurrence, project: project, severity: :low)
+      end
+
+      projects_and_severities = [
+        [projects.first, 'high'],
+        [projects.first, 'low'],
+        [projects.second, 'high'],
+        [projects.second, 'low']
+      ]
+
+      counts = projects_and_severities.map do |(project, severity)|
+        described_class.batch_count_by_project_and_severity(project.id, severity)
+      end
+
+      expect { expect(counts).to all(be 1) }.not_to exceed_query_limit(1)
+    end
+
+    it 'does not include dismissed vulnerabilities in the counts' do
+      create(:vulnerabilities_occurrence, project: project, severity: :high)
+      dismissed_vulnerability = create(:vulnerabilities_occurrence, project: project, severity: :high)
+      create(
+        :vulnerability_feedback,
+        project: project,
+        project_fingerprint: dismissed_vulnerability.project_fingerprint,
+        feedback_type: :dismissal
+      )
+
+      count = described_class.batch_count_by_project_and_severity(project.id, 'high')
+
+      expect(count).to be(1)
+    end
+  end
+
   describe 'feedback' do
     set(:project) { create(:project) }
     let(:occurrence) do

ee/spec/presenters/security/vulnerable_project_presenter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Security::VulnerableProjectPresenter do
+  SEVERITY_LEVELS = ::Vulnerabilities::Occurrence::SEVERITY_LEVELS.keys
+
+  let(:project) { create(:project) }
+
+  before do
+    allow(::Vulnerabilities::Occurrence).to receive(:batch_count_by_project_and_severity).and_return(1)
+  end
+
+  subject { described_class.new(project) }
+
+  it 'presents the given project' do
+    expect(subject.id).to be(project.id)
+  end
+
+  SEVERITY_LEVELS.each do |severity_level|
+    it "exposes a vulnerability count attribute for #{severity_level} vulnerabilities" do
+      expect(subject.public_send("#{severity_level}_vulnerability_count")).to be(1)
+    end
+  end
+end

ee/spec/requests/groups/security/vulnerable_projects_index_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'GET /groups/*group_id/-/security/projects' do
+  let(:group) { create(:group) }
+  let(:user) { create(:user) }
+
+  before do
+    stub_licensed_features(security_dashboard: true)
+    login_as(user)
+
+    group.add_developer(user)
+  end
+
+  it 'does not use N+1 queries' do
+    control_project = create(:project, namespace: group)
+    create(:vulnerabilities_occurrence, project: control_project)
+
+    control_count = ActiveRecord::QueryRecorder.new do
+      get group_security_vulnerable_projects_path(group, format: :json)
+    end
+
+    projects = create_list(:project, 2, namespace: group)
+
+    projects.each do |project|
+      ::Vulnerabilities::Occurrence::SEVERITY_LEVELS.keys.each do |severity|
+        create(:vulnerabilities_occurrence, severity: severity, project: project)
+      end
+    end
+
+    expect do
+      get group_security_vulnerable_projects_path(group, format: :json)
+    end.not_to exceed_query_limit(control_count)
+
+    expect(response).to have_gitlab_http_status(200)
+    expect(json_response.size).to be(3)
+  end
+end

ee/spec/serializers/vulnerable_project_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe VulnerableProjectEntity do
+  SEVERITY_LEVELS = ::Vulnerabilities::Occurrence::SEVERITY_LEVELS.keys
+
+  let(:project) { create(:project) }
+  let(:vulnerable_project) { ::Security::VulnerableProjectPresenter.new(project) }
+
+  before do
+    allow(::Vulnerabilities::Occurrence).to receive(:batch_count_by_project_and_severity).and_return(2)
+  end
+
+  subject { described_class.new(vulnerable_project) }
+
+  SEVERITY_LEVELS.each do |severity_level|
+    it "exposes a vulnerability count attribute for #{severity_level} vulnerabilities" do
+      expect(subject.as_json["#{severity_level}_vulnerability_count".to_sym]).to be(2)
+    end
+  end
+end

ee/spec/serializers/vulnerable_project_serializer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe VulnerableProjectSerializer do
+  let(:project) { create(:project) }
+  let(:serializer) { described_class.new(project: project, current_user: user) }
+  let(:user) { create(:user) }
+  let(:vulnerable_project) { ::Security::VulnerableProjectPresenter.new(project) }
+
+  before do
+    project.add_developer(user)
+
+    allow(::Vulnerabilities::Occurrence).to receive(:batch_count_by_project_and_severity)
+  end
+
+  describe '#represent' do
+    subject { serializer.represent(vulnerable_project) }
+
+    it 'includes counts for each severity of vulnerability' do
+      ::Vulnerabilities::Occurrence::SEVERITY_LEVELS.keys.each do |severity_level|
+        expect(subject).to include("#{severity_level}_vulnerability_count".to_sym)
+      end
+    end
+  end
+end