Merge branch 'mc_rocha-add-scan-method-to-dast-profile-site-327070' into 'master'

Add scan method to dast site profile

See merge request gitlab-org/gitlab!78745

db/migrate/20220119220620_add_scan_method_to_dast_site_profile.rb

+# frozen_string_literal: true
+
+class AddScanMethodToDastSiteProfile < Gitlab::Database::Migration[1.0]
+  def up
+    add_column :dast_site_profiles, :scan_method, :integer, limit: 2, default: 0, null: false
+  end
+
+  def down
+    remove_column :dast_site_profiles, :scan_method
+  end
+end

db/migrate/20220208171826_update_default_scan_method_of_dast_site_profile.rb

+# frozen_string_literal: true
+
+# See https://docs.gitlab.com/ee/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class UpdateDefaultScanMethodOfDastSiteProfile < Gitlab::Database::Migration[1.0]
+  BATCH_SIZE = 500
+
+  disable_ddl_transaction!
+
+  def up
+    each_batch_range('dast_site_profiles', scope: ->(table) { table.where(target_type: 1) }, of: BATCH_SIZE) do |min, max|
+      execute <<~SQL
+        UPDATE dast_site_profiles
+        SET scan_method = 1
+        WHERE id BETWEEN #{min} AND #{max}
+      SQL
+    end
+  end
+
+  def down
+    # noop
+  end
+end

db/schema_migrations/20220119220620

+535f476a358dcb3f3472f1e0ec1afef738f995197b5d1f4fcd61e58a9c9e8e75
\ No newline at end of file

db/schema_migrations/20220208171826

+e48473172d7561fb7474e16e291e555843c0ec4543300b007f86cd4a5923db85
\ No newline at end of file

db/structure.sql

@@ -13414,6 +13414,7 @@ CREATE TABLE dast_site_profiles (
     auth_password_field text,
     auth_username text,
     target_type smallint DEFAULT 0 NOT NULL,
+    scan_method smallint DEFAULT 0 NOT NULL,
     CONSTRAINT check_5203110fee CHECK ((char_length(auth_username_field) <= 255)),
     CONSTRAINT check_6cfab17b48 CHECK ((char_length(name) <= 255)),
     CONSTRAINT check_c329dffdba CHECK ((char_length(auth_password_field) <= 255)),

ee/app/models/dast_site_profile.rb

@@ -27,10 +27,14 @@ class DastSiteProfile < ApplicationRecord
 
   enum target_type: { website: 0, api: 1 }
 
+  enum scan_method: { site: 0, openapi: 1, har: 2, postman: 3 }, _prefix: true
+
   delegate :dast_site_validation, to: :dast_site, allow_nil: true
 
   sanitizes! :name
 
+  before_save :ensure_scan_method
+
   def self.names(site_profile_ids)
     find(*site_profile_ids).pluck(:name)
   rescue ActiveRecord::RecordNotFound
@@ -114,4 +118,14 @@ class DastSiteProfile < ApplicationRecord
 
     errors.add(:excluded_urls, message % { urls: invalid.join(', ') })
   end
+
+  # This callback is necessary to avoid discrepancy between the scan_method and target_type
+  # before we enable the dast_api_scanner feature flag by default.
+  # More context can be found here:
+  # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78745#note_837953465
+  def ensure_scan_method
+    if api? && scan_method_site?
+      self.scan_method = 'openapi'
+    end
+  end
 end

ee/spec/models/dast_site_profile_spec.rb

@@ -129,7 +129,13 @@ RSpec.describe DastSiteProfile, type: :model do
       { website: 0, api: 1 }
     end
 
+    let(:scan_methods) do
+      { site: 0, openapi: 1, har: 2, postman: 3 }
+    end
+
     it { is_expected.to define_enum_for(:target_type).with_values(**target_types) }
+
+    it { is_expected.to define_enum_for(:scan_method).with_values(**scan_methods).with_prefix }
   end
 
   describe '.names' do
@@ -314,5 +320,26 @@ RSpec.describe DastSiteProfile, type: :model do
         end
       end
     end
+
+    describe '#ensure_scan_method' do
+      let(:target_type) { 'website' }
+      let(:scan_method) { 'site' }
+
+      subject { create(:dast_site_profile, scan_method: scan_method, target_type: target_type) }
+
+      context 'when the target_type is website' do
+        it 'does not change the scan_method' do
+          expect(subject.scan_method).to eq('site')
+        end
+      end
+
+      context 'when the target type is api' do
+        let(:target_type) { 'api' }
+
+        it 'does set the scan_method to openapi' do
+          expect(subject.scan_method).to eq('openapi')
+        end
+      end
+    end
   end
 end

spec/migrations/update_default_scan_method_of_dast_site_profile_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe UpdateDefaultScanMethodOfDastSiteProfile do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:dast_sites) { table(:dast_sites) }
+  let(:dast_site_profiles) { table(:dast_site_profiles) }
+
+  before do
+    namespace = namespaces.create!(name: 'test', path: 'test')
+    project = projects.create!(id: 12, namespace_id: namespace.id, name: 'gitlab', path: 'gitlab')
+    dast_site = dast_sites.create!(id: 1, url: 'https://www.gitlab.com', project_id: project.id)
+
+    dast_site_profiles.create!(id: 1, project_id: project.id, dast_site_id: dast_site.id,
+                               name: "#{FFaker::Product.product_name.truncate(192)} #{SecureRandom.hex(4)} - 0",
+                               scan_method: 0, target_type: 0)
+
+    dast_site_profiles.create!(id: 2, project_id: project.id, dast_site_id: dast_site.id,
+                               name: "#{FFaker::Product.product_name.truncate(192)} #{SecureRandom.hex(4)} - 1",
+                               scan_method: 0, target_type: 1)
+  end
+
+  it 'updates the scan_method to 1 for profiles with target_type 1' do
+    migrate!
+
+    expect(dast_site_profiles.where(scan_method: 1).count).to eq 1
+    expect(dast_site_profiles.where(scan_method: 0).count).to eq 1
+  end
+end