Commit 08cd9b3c authored by Marcos Rocha's avatar Marcos Rocha Committed by Mayra Cabrera

Add owner_valid to Dast::ProfileScheduleType

Changelog: added
MR:
EE: true
parent 6c7980a7
# frozen_string_literal: true
module Preloaders
# This class preloads the max access level (role) for the users within the given projects and
# stores the values in requests store via the ProjectTeam class.
class UsersMaxAccessLevelInProjectsPreloader
def initialize(projects:, users:)
@projects = projects
@users = users
end
def execute
return unless @projects.present? && @users.present?
access_levels.each do |(project_id, user_id), access_level|
project = projects_by_id[project_id]
project.team.write_member_access_for_user_id(user_id, access_level)
end
end
private
def access_levels
ProjectAuthorization
.where(project_id: project_ids, user_id: user_ids)
.group(:project_id, :user_id)
.maximum(:access_level)
end
# Use reselect to override the existing select to prevent
# the error `subquery has too many columns`
# NotificationsController passes in an Array so we need to check the type
def project_ids
@projects.is_a?(ActiveRecord::Relation) ? @projects.reselect(:id) : @projects
end
def user_ids
@users.is_a?(ActiveRecord::Relation) ? @users.reselect(:id) : @users
end
def projects_by_id
@projects_by_id ||= @projects.index_by(&:id)
end
end
end
......@@ -9675,6 +9675,7 @@ Represents a DAST profile schedule.
| <a id="dastprofileschedulecadence"></a>`cadence` | [`DastProfileCadence`](#dastprofilecadence) | Cadence of the DAST profile schedule. |
| <a id="dastprofilescheduleid"></a>`id` | [`DastProfileScheduleID!`](#dastprofilescheduleid) | ID of the DAST profile schedule. |
| <a id="dastprofileschedulenextrunat"></a>`nextRunAt` | [`Time`](#time) | Next run time of the DAST profile schedule in the given timezone. |
| <a id="dastprofilescheduleownervalid"></a>`ownerValid` | [`Boolean`](#boolean) | Status of the current owner of the DAST profile schedule. |
| <a id="dastprofileschedulestartsat"></a>`startsAt` | [`Time`](#time) | Start time of the DAST profile schedule in the given timezone. |
| <a id="dastprofilescheduletimezone"></a>`timezone` | [`String`](#string) | Time zone of the start time of the DAST profile schedule. |
......@@ -21,11 +21,9 @@ module Dast
attr_reader :params
# rubocop: disable CodeReuse/ActiveRecord
def default_relation
Dast::Profile.limit(100)
Dast::Profile.limit(100).with_schedule_owner
end
# rubocop: enable CodeReuse/ActiveRecord
def by_id(relation)
return relation if params[:id].nil?
......
......@@ -74,6 +74,8 @@ module EE
field :dast_profiles,
::Types::Dast::ProfileType.connection_type,
null: true,
extras: [:lookahead],
late_extensions: [::Gitlab::Graphql::Project::DastProfileConnectionExtension],
resolver: ::Resolvers::AppSec::Dast::ProfileResolver,
description: 'DAST Profiles associated with the project.'
......
......@@ -21,19 +21,35 @@ module Resolvers
end
def resolve_with_lookahead(**args)
apply_lookahead(find_dast_profiles(args))
end
profiles = apply_lookahead(find_dast_profiles(args)).to_a
profiles.each do |profile|
profile.project = project
if node_selection&.selects?(:dast_profile_schedule) && profile.dast_profile_schedule
profile.dast_profile_schedule.project = project
end
end
context[:project_dast_profiles] ||= []
context[:project_dast_profiles] += profiles
DAST_PROFILE_PRELOAD = {
dast_site_profile: [{ dast_site_profile: [:dast_site, :secret_variables] }],
dast_scanner_profile: [:dast_scanner_profile],
dast_profile_schedule: [:dast_profile_schedule]
}.freeze
# If we are querying a single profile, we should return the profile
# because the late_extensions won't be called
return profiles if single?
# We want to avoid resolving any fields on these profiles until all
# leaves at the name level have been resolved.
# See: DastProfileConnectionExtension for where this batch is consumed.
::Gitlab::Graphql::Lazy.new { profiles }
end
private
def preloads
DAST_PROFILE_PRELOAD
{
dast_site_profile: [{ dast_site_profile: [:dast_site, :secret_variables] }],
dast_scanner_profile: [:dast_scanner_profile],
dast_profile_schedule: [{ dast_profile_schedule: [:owner] }]
}
end
def find_dast_profiles(args)
......
......@@ -26,6 +26,9 @@ module Types
field :next_run_at, Types::TimeType, null: true,
description: 'Next run time of the DAST profile schedule in the given timezone.'
field :owner_valid, GraphQL::Types::Boolean, null: true,
description: 'Status of the current owner of the DAST profile schedule.', method: :owner_valid?
def starts_at
return unless object.starts_at && object.timezone
......
......@@ -31,6 +31,10 @@ module Dast
has_dast_profile_schedule ? joins(:dast_profile_schedule) : where.missing(:dast_profile_schedule)
end
scope :with_schedule_owner, -> do
eager_load(dast_profile_schedule: [:owner])
end
delegate :secret_ci_variables, to: :dast_site_profile
sanitizes! :name, :description
......
......@@ -84,6 +84,7 @@ module EE
has_many :dast_site_profiles
has_many :dast_site_tokens
has_many :dast_sites
has_many :dast_profiles, class_name: 'Dast::Profile'
has_many :protected_environments
has_many :software_license_policies, inverse_of: :project, class_name: 'SoftwareLicensePolicy'
......
......@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe GitlabSchema.types['DastProfileSchedule'] do
include GraphqlHelpers
let_it_be(:fields) { %i[id active startsAt timezone nextRunAt cadence] }
let_it_be(:fields) { %i[id active startsAt timezone nextRunAt cadence ownerValid] }
let_it_be(:project) { create(:project) }
let_it_be(:user) { create(:user, developer_projects: [project]) }
let_it_be(:object) { create(:dast_profile_schedule, project: project, owner: user) }
......@@ -29,4 +29,10 @@ RSpec.describe GitlabSchema.types['DastProfileSchedule'] do
expect(resolve_field(:next_run_at, object, current_user: user)).to eq(object.next_run_at.in_time_zone(object.timezone))
end
end
describe 'ownerValid' do
it 'returns if the owner is valid' do
expect(resolve_field(:owner_valid, object, current_user: user)).to eq(object.owner_valid?)
end
end
end
......@@ -254,4 +254,34 @@ RSpec.describe Dast::ProfileSchedule, type: :model do
is_expected.not_to include(inactive_dast_profile_schedule)
end
end
describe '#owner_valid?' do
let_it_be(:owner) { create(:user) }
let(:dast_profile_schedule_with_owner) { create(:dast_profile_schedule, project: project, user_id: owner.id) }
subject { dast_profile_schedule_with_owner }
context 'when the feature is enabled' do
before do
stub_licensed_features(security_on_demand_scans: true)
end
context 'when the scheduler owner is not null and has the ability to create_on_demand_dast_scan' do
before do
project.add_developer(owner)
end
it { is_expected.to be_owner_valid }
end
context 'when the user_id is nil' do
let(:dast_profile_schedule_nil_owner) { create(:dast_profile_schedule, project: project, user_id: nil) }
subject { dast_profile_schedule_nil_owner }
it { is_expected.not_to be_owner_valid }
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Query.group(fullPath).projects.dastProfiles.dastProfileSchedule' do
include GraphqlHelpers
let_it_be(:group) { create(:group) }
let_it_be(:current_user) { create(:user) }
let_it_be(:project) { create(:project, :repository, group: group) }
let_it_be(:dast_profile) { create(:dast_profile, project: project) }
let_it_be(:dast_profile_schedule) { create(:dast_profile_schedule, project: project, dast_profile: dast_profile, owner: current_user) }
let_it_be(:project2) { create(:project, :repository, group: group) }
let_it_be(:dast_profile2) { create(:dast_profile, project: project2) }
let_it_be(:dast_profile_schedule2) { create(:dast_profile_schedule, project: project2, dast_profile: dast_profile2, owner: current_user) }
let(:query) do
%(
query {
group(fullPath: "#{group.full_path}") {
projects {
edges {
node {
__typename
id
dastProfiles {
edges {
node {
dastProfileSchedule {
#{all_graphql_fields_for('DastProfileSchedule')}
}
}
}
}
}
}
}
}
}
)
end
def run_query(query)
run_with_clean_state(query,
context: { current_user: current_user },
variables: {})
end
before do
stub_licensed_features(security_on_demand_scans: true)
end
context 'when a user has access to dast_profile_schedule' do
let_it_be(:plan_limits) { create(:plan_limits, :default_plan) }
before do
project.add_developer(current_user)
end
it 'returns a dast_profile_schedule' do
r = run_query(query).to_h
schedule = graphql_dig_at(r, :data,
:group,
:projects, :edges, :node,
:dast_profiles, :edges, :node,
:dast_profile_schedule, 0)
expect(schedule).to include('ownerValid' => eq(true))
end
it_behaves_like 'query dastProfiles.dastProfileSchedule shared examples', :avoids_n_plus_1_queries, create_new_project: true
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Query.project(fullPath).dastProfiles.dastProfileSchedule' do
include GraphqlHelpers
let_it_be(:plan_limits) { create(:plan_limits, :default_plan) }
let_it_be(:project) { create(:project, :repository) }
let_it_be(:current_user) { create(:user) }
let_it_be(:dast_profile) { create(:dast_profile, project: project) }
let_it_be(:dast_profile_schedule) { create(:dast_profile_schedule, project: project, dast_profile: dast_profile, owner: current_user) }
let(:query) do
%(
query {
project(fullPath: "#{project.full_path}") {
dastProfiles {
edges {
node {
dastProfileSchedule { #{all_graphql_fields_for('DastProfileSchedule')} }
}
}
}
}
}
)
end
def run_query(query)
run_with_clean_state(query,
context: { current_user: current_user },
variables: {})
end
subject { post_graphql(query, current_user: current_user) }
let(:project_data) { graphql_data_at(:project) }
let(:dast_profile_data) { graphql_data_at(:project, :dast_profiles, :edges) }
before do
stub_licensed_features(security_on_demand_scans: true)
end
context 'when a user does not have access to the project' do
it 'returns a null project' do
subject
expect(project_data).to be_nil
end
end
context 'when a user does not have access to dast_profile' do
before do
project.add_guest(current_user)
end
it 'returns an empty dast_profile' do
subject
expect(dast_profile_data).to be_empty
end
end
context 'when a user has access to dast_profile_schedule' do
before do
project.add_developer(current_user)
end
it 'returns a dast_profile_schedule' do
subject
expect(dast_profile_data[0].dig('node', 'dastProfileSchedule', 'ownerValid')).to eq(true)
end
it_behaves_like 'query dastProfiles.dastProfileSchedule shared examples', :avoids_n_plus_1_queries
end
end
# frozen_string_literal: true
RSpec.shared_examples 'query dastProfiles.dastProfileSchedule shared examples' do |create_new_project: false|
it 'avoids N+1 queries' do
profile_project = if create_new_project
create(:project, :repository, group: group)
else
project
end
profile_project.add_developer(current_user)
extra_users = create_list(:user, 6)
extra_users.each do |user|
profile_project.add_developer(user)
end
control = ActiveRecord::QueryRecorder.new(query_recorder_debug: true) do
run_query(query)
end
extra_users.each do |extra_user|
create(:dast_profile_schedule,
project: profile_project,
dast_profile: create(:dast_profile, project: profile_project), owner: extra_user)
end
expect { run_query(query) }.not_to exceed_query_limit(control)
end
end
# frozen_string_literal: true
module Gitlab
module Graphql
module Project
class DastProfileConnectionExtension < GraphQL::Schema::Field::ConnectionExtension
def after_resolve(value:, object:, context:, **rest)
preload_authorizations(context[:project_dast_profiles])
context[:project_dast_profiles] = nil
value
end
def preload_authorizations(dast_profiles)
return unless dast_profiles
projects = dast_profiles.map(&:project)
users = dast_profiles.filter_map { |dast_profile| dast_profile.dast_profile_schedule&.owner }
Preloaders::UsersMaxAccessLevelInProjectsPreloader.new(projects: projects, users: users).execute
end
end
end
end
end
......@@ -529,6 +529,7 @@ project:
- vulnerability_feedback
- vulnerability_identifiers
- vulnerability_scanners
- dast_profiles
- dast_site_profiles
- dast_scanner_profiles
- dast_sites
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Preloaders::UsersMaxAccessLevelInProjectsPreloader do
let_it_be(:user1) { create(:user) }
let_it_be(:user2) { create(:user) }
let_it_be(:project_1) { create(:project) }
let_it_be(:project_2) { create(:project) }
let_it_be(:project_3) { create(:project) }
let(:projects) { [project_1, project_2, project_3] }
let(:users) { [user1, user2] }
before do
project_1.add_developer(user1)
project_1.add_developer(user2)
project_2.add_developer(user1)
project_2.add_developer(user2)
project_3.add_developer(user1)
project_3.add_developer(user2)
end
context 'preload maximum access level to avoid querying project_authorizations', :request_store do
it 'avoids N+1 queries', :request_store do
Preloaders::UsersMaxAccessLevelInProjectsPreloader.new(projects: projects, users: users).execute
expect(count_queries).to eq(0)
end
it 'runs N queries without preloading' do
query_count_without_preload = count_queries
Preloaders::UsersMaxAccessLevelInProjectsPreloader.new(projects: projects, users: users).execute
count_queries_with_preload = count_queries
expect(count_queries_with_preload).to be < query_count_without_preload
end
end
def count_queries
ActiveRecord::QueryRecorder.new do
projects.each do |project|
user1.can?(:read_project, project)
user2.can?(:read_project, project)
end
end.count
end
end
......@@ -28,4 +28,4 @@ excluded_attributes:
- :iid
project:
- :id
- :created_at
\ No newline at end of file
- :created_at
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