Update scheduler owner when the owner is not valid
What does this MR do and why?
The problem
Dast::ProfileSchedules belongs to the users. We need to handle the situation when the user is disabled or downgraded.
the solution
The solution was proposed here
This merge request is part of the tasks required to reassign ownership when the scan is updated if the owner is not valid.
The owner should be updated only when it is not valid to avoid confusion. More details on that can be found here.
This MR modifies the AppSec::Dast::Profiles::UpdateService#execute
method to set the scheduler ownership to the current user if the owner is not valid.
To check if the owner is valid, the following ability is used:
Ability.allowed?(owner, :create_on_demand_dast_scan, project)
This check will fail if:
- the owner was deleted
- the owner permission was downgraded
- the owner was removed from the project
Screenshots or screen recordings
These are strongly recommended to assist reviewers and reduce the time to merge your change.
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
rails c
create a new user different than root to be the schedule owner
owner = User.last.dup
owner.email = 'schedule_owner@gitlab.com'
owner.name = 'Schedule Owner'
owner.username = 'schedule_owner'
owner.password = <password>
owner.admin = false
owner.save
project = Project.last
dast_site = DastSite.create(project: project, url: 'http://gitlab.com')
dast_site_profile = DastSiteProfile.create(name: 'Dast Site Profile Owner test', project: project, dast_site: dast_site)
dast_scanner_profile = DastScannerProfile.create(project: project, name: 'dast scanner profile owner test')
dast_profile = Dast::Profile.create(description:'dast profile description for ownership test', name: 'Test profile Ownership Test', project: project, dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile)
schedule = Dast::ProfileSchedule.create!(user_id: owner.id, cron: "*/10 * * * *", next_run_at: Time.zone.now, dast_profile_id: dast_profile.id, project_id: project.id, timezone: "America/New_York", starts_at:Time.zone.now)
project.add_developer(owner)
project.full_path
check the current owner
[15] pry(main)> dast_profile_schedule.user_id
=> 99
Test 1
check that a valid owner is not updated
call the update service
params = {
dast_profile: dast_profile,
name: dast_profile.name,
description: dast_profile.description,
branch_name: nil,
dast_site_profile_id: dast_site_profile.id,
dast_scanner_profile_id: dast_scanner_profile.id,
dast_profile_schedule: {timezone: schedule.timezone},
run_after_update: false
}.compact
::AppSec::Dast::Profiles::UpdateService.new(container: project, current_user: current_user, params: params).execute
Check the current owner. It should be the same
[15] pry(main)> dast_profile_schedule.user_id
=> 99
current_user = User.first
Test 2
when the owner permission is downgraded
Set owner as guest
project.add_guest(owner)
call the update service
params = {
dast_profile: dast_profile,
name: dast_profile.name,
description: dast_profile.description,
branch_name: nil,
dast_site_profile_id: dast_site_profile.id,
dast_scanner_profile_id: dast_scanner_profile.id,
dast_profile_schedule: {timezone: schedule.timezone},
run_after_update: false
}.compact
::AppSec::Dast::Profiles::UpdateService.new(container: project, current_user: current_user, params: params).execute
Check the current owner. It should have been updated to your current user
schedule.reload
[15] pry(main)> dast_profile_schedule.user_id
=> 1
Test 3
when the owner is removed from the project
elevate owner permission again
project.add_developer(owner)
re-assign the scheduler to the owner
schedule.user_id = owner.id
schedule.save
schedule.reload
=> #<Dast::ProfileSchedule:0x00007ff1a4e98728
id: 3,
project_id: 19,
dast_profile_id: 3,
user_id: 99,
next_run_at: Tue, 28 Sep 2021 16:07:00.000000000 UTC +00:00,
created_at: Thu, 23 Sep 2021 16:27:32.538531000 UTC +00:00,
updated_at: Tue, 28 Sep 2021 16:06:43.470109000 UTC +00:00,
active: false,
cron: "* * * * *",
cadence: {}
remove the owner from the project
project.team.truncate
add your current user to the project
project.add_developer(current_user)
call the update service
params = {
dast_profile: dast_profile,
name: dast_profile.name,
description: dast_profile.description,
branch_name: nil,
dast_site_profile_id: dast_site_profile.id,
dast_scanner_profile_id: dast_scanner_profile.id,
dast_profile_schedule: {timezone: schedule.timezone},
run_after_update: false
}.compact
::AppSec::Dast::Profiles::UpdateService.new(container: project, current_user: current_user, params: params).execute
Check the current owner. It should have been updated to your current user
schedule.reload
=> #<Dast::ProfileSchedule:0x00007ff1a4e98728
id: 3,
project_id: 19,
dast_profile_id: 3,
user_id: 1,
next_run_at: Tue, 28 Sep 2021 16:22:00.000000000 UTC +00:00,
created_at: Thu, 23 Sep 2021 16:27:32.538531000 UTC +00:00,
updated_at: Tue, 28 Sep 2021 16:13:08.641549000 UTC +00:00,
active: false,
cron: "* * * * *",
cadence: {},
[17] pry(main)> schedule.user_id
=> 1
Test 4
when the owner is deleted
add owner to the project again
project.add_developer(owner)
re-assign the scheduler to the owner
[20] pry(main)> schedule.user_id = owner.id
=> 99
schedule.save
[26] pry(main)> schedule.reload
Dast::ProfileSchedule Load (0.4ms) SELECT "dast_profile_schedules".* FROM "dast_profile_schedules" WHERE "dast_profile_schedules"."id" = 3 LIMIT 1 /*application:console,db_config_name:main,line:/lib/gitlab/database/load_balancing/connection_proxy.rb:99:in `block in read_using_load_balancer'*/
=> #<Dast::ProfileSchedule:0x00007ff1a4e98728
id: 3,
project_id: 19,
dast_profile_id: 3,
user_id: 99,
delete the owner
owner.destroy!
Check the current owner. It should be nil
schedule.reload
[29] pry(main)> schedule.user_id
=> nil
call the update service
params = {
dast_profile: dast_profile,
name: dast_profile.name,
description: dast_profile.description,
branch_name: nil,
dast_site_profile_id: dast_site_profile.id,
dast_scanner_profile_id: dast_scanner_profile.id,
dast_profile_schedule: {timezone: schedule.timezone},
run_after_update: false
}.compact
::AppSec::Dast::Profiles::UpdateService.new(container: project, current_user: current_user, params: params).execute
Check the current owner. It should have been updated to your current user
schedule.reload
[13] pry(main)> schedule.user_id
=> 1
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.