From 34d76ecb851039bbb674fe3430fab6cbe23889e5 Mon Sep 17 00:00:00 2001 From: atiwari71 Date: Wed, 11 Aug 2021 15:52:41 +0530 Subject: [PATCH] Update Graphql dastProfileUpdate mutation to include Schedule Graphql update mutation will take a schedule and perform update operation on it. Changelog: changed --- doc/api/graphql/reference/index.md | 1 + .../graphql/mutations/dast/profiles/update.rb | 21 ++++-- .../profile_schedules/audit/update_service.rb | 27 +++++++ .../app_sec/dast/profiles/update_service.rb | 74 ++++++++++++++++--- .../mutations/dast/profiles/update_spec.rb | 60 ++++++++++++++- .../audit/update_service_spec.rb | 34 +++++++++ .../dast/profiles/update_service_spec.rb | 46 ++++++++++++ 7 files changed, 248 insertions(+), 15 deletions(-) create mode 100644 ee/app/services/app_sec/dast/profile_schedules/audit/update_service.rb create mode 100644 ee/spec/services/app_sec/dast/profile_schedules/audit/update_service_spec.rb diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index da2133e1f37b4c..41584278305479 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -1502,6 +1502,7 @@ Input type: `DastProfileUpdateInput` | ---- | ---- | ----------- | | `branchName` | [`String`](#string) | Associated branch. | | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `dastProfileSchedule` | [`DastProfileScheduleInput`](#dastprofilescheduleinput) | Represents a DAST profile schedule. Results in an error if `dast_on_demand_scans_scheduler` feature flag is disabled. | | `dastScannerProfileId` | [`DastScannerProfileID`](#dastscannerprofileid) | ID of the scanner profile to be associated. | | `dastSiteProfileId` | [`DastSiteProfileID`](#dastsiteprofileid) | ID of the site profile to be associated. | | `description` | [`String`](#string) | Description of the profile. Defaults to an empty string. | diff --git a/ee/app/graphql/mutations/dast/profiles/update.rb b/ee/app/graphql/mutations/dast/profiles/update.rb index c0a266351ad885..dc695eb4fafe5d 100644 --- a/ee/app/graphql/mutations/dast/profiles/update.rb +++ b/ee/app/graphql/mutations/dast/profiles/update.rb @@ -30,6 +30,10 @@ class Update < BaseMutation required: true, description: 'Project the profile belongs to.' + argument :dast_profile_schedule, ::Types::Dast::ProfileScheduleInputType, + required: false, + description: 'Represents a DAST profile schedule. Results in an error if `dast_on_demand_scans_scheduler` feature flag is disabled.' + argument :name, GraphQL::Types::String, required: false, description: 'Name of the profile.' @@ -58,9 +62,9 @@ class Update < BaseMutation authorize :create_on_demand_dast_scan - def resolve(full_path:, id:, name:, description:, branch_name: nil, dast_site_profile_id: nil, dast_scanner_profile_id: nil, run_after_update: false) + def resolve(full_path:, id:, name:, description:, branch_name: nil, dast_scanner_profile_id: nil, run_after_update: false, **args) project = authorized_find!(full_path) - raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless allowed?(project) + raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless allowed?(args[:dast_profile_schedule], project) dast_profile = find_dast_profile(project.id, id) authorize!(dast_profile) @@ -70,8 +74,9 @@ def resolve(full_path:, id:, name:, description:, branch_name: nil, dast_site_pr name: name, description: description, branch_name: branch_name, - dast_site_profile_id: as_model_id(SiteProfileID, dast_site_profile_id), + dast_site_profile_id: as_model_id(SiteProfileID, args[:dast_site_profile_id]), dast_scanner_profile_id: as_model_id(ScannerProfileID, dast_scanner_profile_id), + dast_profile_schedule: args[:dast_profile_schedule], run_after_update: run_after_update }.compact @@ -86,8 +91,14 @@ def resolve(full_path:, id:, name:, description:, branch_name: nil, dast_site_pr private - def allowed?(project) - project.feature_available?(:security_on_demand_scans) + def allowed?(dast_profile_schedule, project) + scheduler_flag_enabled?(dast_profile_schedule, project) + end + + def scheduler_flag_enabled?(dast_profile_schedule, project) + return true unless dast_profile_schedule + + Feature.enabled?(:dast_on_demand_scans_scheduler, project, default_enabled: :yaml) end def as_model_id(klass, value) diff --git a/ee/app/services/app_sec/dast/profile_schedules/audit/update_service.rb b/ee/app/services/app_sec/dast/profile_schedules/audit/update_service.rb new file mode 100644 index 00000000000000..59f533a68210e9 --- /dev/null +++ b/ee/app/services/app_sec/dast/profile_schedules/audit/update_service.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module AppSec + module Dast + module ProfileSchedules + module Audit + class UpdateService < BaseProjectService + def execute + params[:new_params].each do |property, new_value| + old_value = params[:old_params][property] + + next if old_value == new_value + + ::Gitlab::Audit::Auditor.audit( + name: 'dast_profile_schedule_update', + author: current_user, + scope: project, + target: params[:dast_profile_schedule], + message: "Changed DAST profile schedule #{property} from #{old_value} to #{new_value}" + ) + end + end + end + end + end + end +end diff --git a/ee/app/services/app_sec/dast/profiles/update_service.rb b/ee/app/services/app_sec/dast/profiles/update_service.rb index 4b4cb9e7d127c8..93638826aa55ac 100644 --- a/ee/app/services/app_sec/dast/profiles/update_service.rb +++ b/ee/app/services/app_sec/dast/profiles/update_service.rb @@ -9,33 +9,60 @@ class UpdateService < BaseContainerService def execute return unauthorized unless allowed? return error('Profile parameter missing') unless dast_profile + return error('Dast Profile Schedule not found') if update_schedule? && !schedule - auditor = AppSec::Dast::Profiles::Audit::UpdateService.new(container: container, current_user: current_user, params: { - dast_profile: dast_profile, - new_params: dast_profile_params, - old_params: dast_profile.attributes.symbolize_keys - }) + build_auditors! + + ApplicationRecord.transaction do + dast_profile.update!(dast_profile_params) - return error(dast_profile.errors.full_messages) unless dast_profile.update(dast_profile_params) + update_schedule if update_schedule? + end - auditor.execute + execute_auditors! - return success(dast_profile: dast_profile, pipeline_url: nil) unless params[:run_after_update] + unless params[:run_after_update] + return success( + dast_profile: dast_profile, + pipeline_url: nil, + dast_profile_schedule: schedule + ) + end response = create_scan(dast_profile) return error(response.message) if response.error? - success(dast_profile: dast_profile, pipeline_url: response.payload.fetch(:pipeline_url)) + success( + dast_profile: dast_profile, + pipeline_url: response.payload.fetch(:pipeline_url), + dast_profile_schedule: schedule + ) + rescue ActiveRecord::RecordInvalid => err + error(err.record.errors.full_messages) end private + attr_reader :auditors + def allowed? container.licensed_feature_available?(:security_on_demand_scans) && can?(current_user, :create_on_demand_dast_scan, container) end + def update_schedule? + schedule_input_params.present? + end + + def update_schedule + schedule.update!(schedule_input_params) + end + + def schedule + @schedule ||= dast_profile.dast_profile_schedule + end + def error(message, opts = {}) ServiceResponse.error(message: message, **opts) end @@ -56,6 +83,35 @@ def dast_profile_params params.slice(:dast_site_profile_id, :dast_scanner_profile_id, :name, :description, :branch_name) end + def schedule_input_params + # params[:dast_profile_schedule] is `Types::Dast::ProfileScheduleInputType` object. + # Using to_h method to convert object into equivalent hash. + @schedule_input_params ||= params[:dast_profile_schedule]&.to_h + end + + def build_auditors! + @auditors = [ + AppSec::Dast::Profiles::Audit::UpdateService.new(container: container, current_user: current_user, params: { + dast_profile: dast_profile, + new_params: dast_profile_params, + old_params: dast_profile.attributes.symbolize_keys + }) + ] + + if schedule_input_params + @auditors << + AppSec::Dast::ProfileSchedules::Audit::UpdateService.new(project: container, current_user: current_user, params: { + dast_profile_schedule: schedule, + new_params: schedule_input_params, + old_params: schedule.attributes.symbolize_keys + }) + end + end + + def execute_auditors! + auditors.map(&:execute) + end + def create_scan(dast_profile) ::DastOnDemandScans::CreateService.new( container: container, diff --git a/ee/spec/graphql/mutations/dast/profiles/update_spec.rb b/ee/spec/graphql/mutations/dast/profiles/update_spec.rb index 534bdf32f8064c..b5a7eedc121286 100644 --- a/ee/spec/graphql/mutations/dast/profiles/update_spec.rb +++ b/ee/spec/graphql/mutations/dast/profiles/update_spec.rb @@ -11,6 +11,8 @@ let_it_be(:new_dast_site_profile) { create(:dast_site_profile, project: project) } let_it_be(:new_dast_scanner_profile) { create(:dast_scanner_profile, project: project) } + let(:dast_profile_schedule_attrs) { nil } + let(:dast_profile_gid) { dast_profile.to_global_id } let(:run_after_update) { false } @@ -22,7 +24,8 @@ branch_name: project.default_branch, dast_site_profile_id: global_id_of(new_dast_site_profile), dast_scanner_profile_id: global_id_of(new_dast_scanner_profile), - run_after_update: run_after_update + run_after_update: run_after_update, + dast_profile_schedule: dast_profile_schedule_attrs } end @@ -81,6 +84,61 @@ end end + context 'when associated dast profile schedule is present' do + before do + create(:dast_profile_schedule, dast_profile: dast_profile) + end + + context 'when dast_profile_schedule param is present' do + let(:new_dast_profile_schedule) { attributes_for(:dast_profile_schedule) } + + subject do + mutation.resolve(**params.merge( + full_path: project.full_path, + dast_profile_schedule: new_dast_profile_schedule + )) + end + + context 'when dast_on_demand_scans_scheduler feature is enabled' do + it 'updates the profile schedule' do + subject + + updated_schedule = dast_profile.reload.dast_profile_schedule + + aggregate_failures do + expect(updated_schedule.timezone).to eq(new_dast_profile_schedule[:timezone]) + expect(updated_schedule.starts_at.to_i).to eq(new_dast_profile_schedule[:starts_at].to_i) + expect(updated_schedule.cadence).to eq(new_dast_profile_schedule[:cadence].stringify_keys) + end + end + end + + context 'when dast_on_demand_scans_scheduler feature is disabled' do + let(:dast_profile_schedule_attrs) { attributes_for(:dast_profile_schedule) } + + before do + stub_feature_flags(dast_on_demand_scans_scheduler: false) + end + + it 'returns the dast_profile_schedule' do + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end + end + end + + context 'when dast_profile_schedule param is not passed' do + context 'when dast_on_demand_scans_scheduler feature is enabled' do + it 'does not updates the profile schedule' do + schedule_before_update = dast_profile.dast_profile_schedule + + subject + + expect(schedule_before_update).to eq(dast_profile.dast_profile_schedule.reload) + end + end + end + end + context 'when run_after_update=true' do let(:run_after_update) { true } diff --git a/ee/spec/services/app_sec/dast/profile_schedules/audit/update_service_spec.rb b/ee/spec/services/app_sec/dast/profile_schedules/audit/update_service_spec.rb new file mode 100644 index 00000000000000..afdfc472ccb957 --- /dev/null +++ b/ee/spec/services/app_sec/dast/profile_schedules/audit/update_service_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe AppSec::Dast::ProfileSchedules::Audit::UpdateService do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:dast_profile_schedule) { create(:dast_profile_schedule, owner: user) } + + describe '#execute' do + it 'creates audit events for the changed properties', :aggregate_failures do + auditor = described_class.new(project: project, current_user: user, params: { + dast_profile_schedule: dast_profile_schedule, + new_params: { starts_at: Date.tomorrow }, + old_params: { starts_at: Date.today } + }) + + auditor.execute + + audit_event = AuditEvent.find_by(author_id: user.id) + expect(audit_event.author).to eq(user) + expect(audit_event.entity).to eq(project) + expect(audit_event.target_id).to eq(dast_profile_schedule.id) + expect(audit_event.target_type).to eq('Dast::ProfileSchedule') + expect(audit_event.details).to eq({ + author_name: user.name, + custom_message: "Changed DAST profile schedule starts_at from #{Date.today} to #{Date.tomorrow}", + target_id: dast_profile_schedule.id, + target_type: 'Dast::ProfileSchedule', + target_details: user.name + }) + end + end +end diff --git a/ee/spec/services/app_sec/dast/profiles/update_service_spec.rb b/ee/spec/services/app_sec/dast/profiles/update_service_spec.rb index 0c25dc06c44fb8..ea9b3e1d21a119 100644 --- a/ee/spec/services/app_sec/dast/profiles/update_service_spec.rb +++ b/ee/spec/services/app_sec/dast/profiles/update_service_spec.rb @@ -76,6 +76,52 @@ end end + context 'with dast_profile_schedule param' do + let_it_be(:time_zone) { Time.zone.tzinfo.name } + + let(:params) do + default_params.merge( + dast_profile_schedule: { + active: false, + starts_at: Time.zone.now + 10.days, + timezone: time_zone, + cadence: { unit: 'month', duration: 1 } + } + ) + end + + context 'when associated schedule is not present' do + it 'communicates failure for dast_profile_schedule' do + aggregate_failures do + expect(dast_profile.dast_profile_schedule).to be nil + expect(subject.status).to eq(:error) + expect(subject.message).to include('Dast Profile Schedule not found') + end + end + end + + context 'when associated schedule is present' do + before do + create(:dast_profile_schedule, dast_profile: dast_profile) + end + + it 'updates the dast profile schedule' do + updated_schedule = subject.payload[:dast_profile_schedule].reload + + aggregate_failures do + expect(updated_schedule.active).to eq(params[:dast_profile_schedule][:active]) + expect(updated_schedule.starts_at.to_i).to eq(params[:dast_profile_schedule][:starts_at].to_i) + expect(updated_schedule.timezone).to eq(params[:dast_profile_schedule][:timezone]) + expect(updated_schedule.cadence).to eq(params[:dast_profile_schedule][:cadence].stringify_keys) + end + end + + it 'creates the audit event' do + expect { subject }.to change { AuditEvent.where(target_id: dast_profile.dast_profile_schedule.id).count } + end + end + end + it 'audits the update', :aggregate_failures do old_profile_attrs = { description: dast_profile.description, -- GitLab