Commit 6747c6bc authored by Avielle Wolfe's avatar Avielle Wolfe Committed by Robert Speicher

Extract the VulnerableProjectsFinder

Now let's use it!
parent 8b6e2f23
...@@ -6,12 +6,18 @@ class Groups::Security::VulnerableProjectsController < Groups::ApplicationContro ...@@ -6,12 +6,18 @@ class Groups::Security::VulnerableProjectsController < Groups::ApplicationContro
alias_method :vulnerable, :group alias_method :vulnerable, :group
def index def index
projects = group.vulnerable_projects.non_archived.without_deleted.with_route vulnerable_projects = ::Security::VulnerableProjectsFinder.new(projects).execute
vulnerable_projects = projects.map do |project| presented_projects = vulnerable_projects.map do |project|
::Security::VulnerableProjectPresenter.new(project) ::Security::VulnerableProjectPresenter.new(project)
end end
render json: VulnerableProjectSerializer.new.represent(vulnerable_projects) render json: VulnerableProjectSerializer.new.represent(presented_projects)
end
private
def projects
::Project.for_group_and_its_subgroups(group).non_archived.without_deleted.with_route
end end
end end
# frozen_string_literal: true
module Security
class VulnerableProjectsFinder
def initialize(projects)
@projects = projects
end
def execute
projects.where("EXISTS(?)", vulnerabilities) # rubocop:disable CodeReuse/ActiveRecord
end
private
attr_reader :projects
def vulnerabilities
::Vulnerabilities::Occurrence
.select(1)
.undismissed
.scoped_project
end
end
end
...@@ -150,15 +150,6 @@ module EE ...@@ -150,15 +150,6 @@ module EE
ip_restrictions.map(&:range).join(",") ip_restrictions.map(&:range).join(",")
end end
def vulnerable_projects
vulnerabilities = ::Vulnerabilities::Occurrence
.select(1)
.undismissed
.where('vulnerability_occurrences.project_id = projects.id')
::Project.for_group_and_its_subgroups(self).where("EXISTS(?)", vulnerabilities)
end
def human_ldap_access def human_ldap_access
::Gitlab::Access.options_with_owner.key(ldap_access) ::Gitlab::Access.options_with_owner.key(ldap_access)
end end
......
...@@ -43,6 +43,18 @@ describe Groups::Security::VulnerableProjectsController do ...@@ -43,6 +43,18 @@ describe Groups::Security::VulnerableProjectsController do
expect(json_response.first['critical_vulnerability_count']).to eq(2) expect(json_response.first['critical_vulnerability_count']).to eq(2)
end end
it 'includes projects in subgroups' do
subgroup = create(:group, parent: group)
project = create(:project, namespace: subgroup)
create(:vulnerabilities_occurrence, project: project)
subject
expect(response).to have_gitlab_http_status(:ok)
expect(json_response.count).to be(1)
expect(json_response.first['id']).to eq(project.id)
end
it 'does not include archived or deleted projects' do it 'does not include archived or deleted projects' do
archived_project = create(:project, :archived, namespace: group) archived_project = create(:project, :archived, namespace: group)
deleted_project = create(:project, namespace: group, pending_delete: true) deleted_project = create(:project, namespace: group, pending_delete: true)
......
...@@ -66,5 +66,11 @@ FactoryBot.define do ...@@ -66,5 +66,11 @@ FactoryBot.define do
project_fingerprint: finding.project_fingerprint) project_fingerprint: finding.project_fingerprint)
end end
end end
::Vulnerabilities::Occurrence::REPORT_TYPES.keys.each do |security_report_type|
trait security_report_type do
report_type { security_report_type }
end
end
end end
end end
# frozen_string_literal: true
require 'spec_helper'
describe Security::VulnerableProjectsFinder do
describe '#execute' do
let(:projects) { Project.all }
let!(:safe_project) { create(:project) }
let(:vulnerable_project) { create(:project) }
let!(:vulnerability) { create(:vulnerabilities_occurrence, project: vulnerable_project) }
subject { described_class.new(projects).execute }
it 'returns the projects that have vulnerabilities from the collection of projects given to it' do
expect(subject).to contain_exactly(vulnerable_project)
end
it 'does not include projects that only have dismissed vulnerabilities' do
create(:vulnerabilities_occurrence, :dismissed, project: safe_project)
expect(subject).to contain_exactly(vulnerable_project)
end
it 'only uses 1 query' do
another_project = create(:project)
create(:vulnerabilities_occurrence, :dismissed, project: another_project)
expect { subject }.not_to exceed_query_limit(1)
expect(subject).to contain_exactly(vulnerable_project)
end
end
end
...@@ -261,60 +261,6 @@ describe Group do ...@@ -261,60 +261,6 @@ describe Group do
end end
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 'includes projects in subgroups' do
subgroup = create(:group, parent: group)
project = create(:project, namespace: subgroup)
create(:vulnerabilities_occurrence, project: project)
vulnerable_projects = group.vulnerable_projects
expect(vulnerable_projects.count).to be(1)
expect(vulnerable_projects.first).to eq(project)
end
it 'does not include projects that only have dismissed vulnerabilities' do
project = create(:project, namespace: group)
vulnerability = create(:vulnerabilities_occurrence, report_type: :dast, project: project)
create(
:vulnerability_feedback,
category: :dast,
feedback_type: :dismissal,
project: project,
project_fingerprint: vulnerability.project_fingerprint
)
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 describe '#mark_ldap_sync_as_failed' do
it 'sets the state to failed' do it 'sets the state to failed' do
group.start_ldap_sync group.start_ldap_sync
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment