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.