Merge branch 'reject-active-scans-when-site-not-validated-245210' into 'master'

Reject active on-demand DAST scan when unvalidated

See merge request gitlab-org/gitlab!45423

ee/app/graphql/mutations/dast_on_demand_scans/create.rb

@@ -30,69 +30,55 @@ module Mutations
       def resolve(full_path:, dast_site_profile_id:, **args)
         project = authorized_find_project!(full_path: full_path)
 
-        # TODO: remove explicit coercion once compatibility layer is removed
-        # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
-        dast_site_profile_id = ::Types::GlobalIDType[::DastSiteProfile].coerce_isolated_input(dast_site_profile_id)
+        dast_site_profile = find_dast_site_profile(project, dast_site_profile_id)
+        dast_scanner_profile = find_dast_scanner_profile(project, args[:dast_scanner_profile_id])
+
+        response = create_on_demand_dast_scan(project, dast_site_profile, dast_scanner_profile)
 
-        dast_site_profile = find_dast_site_profile(project: project, dast_site_profile_id: dast_site_profile_id)
-        dast_site = dast_site_profile.dast_site
-        dast_scanner_profile = find_dast_scanner_profile(project: project, dast_scanner_profile_id: args[:dast_scanner_profile_id])
-
-        result = ::Ci::RunDastScanService.new(
-          project, current_user
-        ).execute(
-          branch: project.default_branch,
-          target_url: dast_site.url,
-          spider_timeout: dast_scanner_profile&.spider_timeout,
-          target_timeout: dast_scanner_profile&.target_timeout,
-          full_scan_enabled: dast_scanner_profile&.full_scan_enabled?,
-          use_ajax_spider: dast_scanner_profile&.use_ajax_spider,
-          show_debug_messages: dast_scanner_profile&.show_debug_messages
-        )
-
-        if result.success?
-          success_response(project: project, pipeline: result.payload)
-        else
-          error_response(result)
-        end
+        return { errors: response.errors } if response.error?
+
+        { errors: [], pipeline_url: response.payload.fetch(:pipeline_url) }
       end
 
       private
 
       # rubocop: disable CodeReuse/ActiveRecord
-      def find_dast_site_profile(project:, dast_site_profile_id:)
+      def find_dast_site_profile(project, dast_site_profile_id)
+        # TODO: remove explicit coercion once compatibility layer is removed
+        # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+        dast_site_profile_id = ::Types::GlobalIDType[::DastSiteProfile].coerce_isolated_input(dast_site_profile_id)
+
         DastSiteProfilesFinder.new(project_id: project.id, id: dast_site_profile_id.model_id)
           .execute
           .first!
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
-      def find_dast_scanner_profile(project:, dast_scanner_profile_id:)
+      # rubocop: disable CodeReuse/ActiveRecord
+      def find_dast_scanner_profile(project, dast_scanner_profile_id)
         return unless dast_scanner_profile_id
 
         # TODO: remove explicit coercion once compatibility layer is removed
         # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
         dast_scanner_profile_id = ::Types::GlobalIDType[::DastScannerProfile]
-          .coerce_isolated_input(dast_scanner_profile_id)
+                                    .coerce_isolated_input(dast_scanner_profile_id)
 
-        project
-          .dast_scanner_profiles
-          .find(dast_scanner_profile_id.model_id)
-      end
-
-      def success_response(project:, pipeline:)
-        pipeline_url = Rails.application.routes.url_helpers.project_pipeline_url(
-          project,
-          pipeline
-        )
-        {
-          errors: [],
-          pipeline_url: pipeline_url
-        }
+        DastScannerProfilesFinder.new(
+          project_ids: [project.id],
+          ids: [dast_scanner_profile_id.model_id]
+        ).execute.first!
       end
+      # rubocop: enable CodeReuse/ActiveRecord
 
-      def error_response(result)
-        { errors: result.errors }
+      def create_on_demand_dast_scan(project, dast_site_profile, dast_scanner_profile)
+        ::DastOnDemandScans::CreateService.new(
+          container: project,
+          current_user: current_user,
+          params: {
+            dast_site_profile: dast_site_profile,
+            dast_scanner_profile: dast_scanner_profile
+          }
+        ).execute
       end
     end
   end

ee/app/services/dast_on_demand_scans/create_service.rb

+# frozen_string_literal: true
+
+module DastOnDemandScans
+  class CreateService < BaseContainerService
+    def execute
+      return ServiceResponse.error(message: 'Insufficient permissions') unless allowed?
+      return ServiceResponse.error(message: 'Cannot run active scan against unvalidated target') unless active_scan_allowed?
+
+      create_pipeline
+    rescue KeyError => err
+      ServiceResponse.error(message: err.message.capitalize)
+    end
+
+    private
+
+    def allowed?
+      container.feature_available?(:security_on_demand_scans)
+    end
+
+    def active_scan_allowed?
+      return true unless dast_scanner_profile&.full_scan_enabled?
+
+      dast_site_validation = DastSiteValidationsFinder.new(
+        project_id: container.id,
+        state: :passed,
+        url_base: url_base
+      ).execute.first
+
+      dast_site_validation.present?
+    end
+
+    def dast_site
+      @dast_site ||= params.fetch(:dast_site_profile).dast_site
+    end
+
+    def dast_scanner_profile
+      @dast_scanner_profile ||= params[:dast_scanner_profile]
+    end
+
+    def url_base
+      @url_base ||= DastSiteValidation.get_normalized_url_base(dast_site.url)
+    end
+
+    def default_config
+      {
+        branch: container.default_branch,
+        target_url: dast_site.url
+      }
+    end
+
+    def scanner_profile_config
+      return {} unless dast_scanner_profile
+
+      {
+        spider_timeout: dast_scanner_profile.spider_timeout,
+        target_timeout: dast_scanner_profile.target_timeout,
+        full_scan_enabled: dast_scanner_profile.full_scan_enabled?,
+        use_ajax_spider: dast_scanner_profile.use_ajax_spider,
+        show_debug_messages: dast_scanner_profile.show_debug_messages
+      }
+    end
+
+    def success_response(pipeline)
+      pipeline_url = Rails.application.routes.url_helpers.project_pipeline_url(
+        container,
+        pipeline
+      )
+
+      ServiceResponse.success(
+        payload: {
+          pipeline: pipeline,
+          pipeline_url: pipeline_url
+        }
+      )
+    end
+
+    def create_pipeline
+      params = default_config.merge(scanner_profile_config)
+      result = ::Ci::RunDastScanService.new(container, current_user).execute(params)
+
+      return success_response(result.payload) if result.success?
+
+      result
+    end
+  end
+end

ee/changelogs/unreleased/reject-active-scans-when-site-not-validated-245210.yml

+---
+title: Reject active on-demand DAST scan when unvalidated
+merge_request: 45423
+author:
+type: changed

ee/spec/graphql/mutations/dast_on_demand_scans/create_spec.rb

@@ -103,7 +103,7 @@ RSpec.describe Mutations::DastOnDemandScans::Create do
         end
 
         context 'when dast_scanner_profile_id is provided' do
-          let(:dast_scanner_profile) { create(:dast_scanner_profile, project: project, target_timeout: 200, spider_timeout: 5000, use_ajax_spider: true, show_debug_messages: true, scan_type: 'active') }
+          let(:dast_scanner_profile) { create(:dast_scanner_profile, project: project, target_timeout: 200, spider_timeout: 5000, use_ajax_spider: true, show_debug_messages: true, scan_type: 'passive') }
           let(:dast_scanner_profile_id) { dast_scanner_profile.to_global_id }
 
           subject do
@@ -133,6 +133,24 @@ RSpec.describe Mutations::DastOnDemandScans::Create do
 
             subject
           end
+
+          context 'when scan_type=active' do
+            let(:dast_scanner_profile) { create(:dast_scanner_profile, project: project, scan_type: 'active') }
+
+            context 'when target is not validated' do
+              it 'communicates failure' do
+                expect(subject[:errors]).to include('Cannot run active scan against unvalidated target')
+              end
+            end
+
+            context 'when target is validated' do
+              it 'has no errors' do
+                create(:dast_site_validation, state: :passed, dast_site_token: create(:dast_site_token, project: project, url: dast_site_profile.dast_site.url))
+
+                expect(subject[:errors]).to be_empty
+              end
+            end
+          end
         end
 
         context 'when on demand scan licensed feature is not available' do

ee/spec/services/dast_on_demand_scans/create_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe DastOnDemandScans::CreateService do
+  let(:project) { create(:project, :repository) }
+  let(:user) { create(:user) }
+  let(:dast_site_profile) { create(:dast_site_profile, project: project) }
+  let(:dast_scanner_profile) { create(:dast_scanner_profile, project: project) }
+
+  subject do
+    described_class.new(
+      container: project,
+      current_user: user,
+      params: {
+        dast_site_profile: dast_site_profile,
+        dast_scanner_profile: dast_scanner_profile
+      }
+    ).execute
+  end
+
+  describe 'execute' do
+    context 'when on demand scan licensed feature is not available' do
+      context 'when the user cannot run an on demand scan' do
+        it 'communicates failure' do
+          stub_licensed_features(security_on_demand_scans: false)
+
+          aggregate_failures do
+            expect(subject.status).to eq(:error)
+            expect(subject.message).to eq('Insufficient permissions')
+          end
+        end
+      end
+    end
+
+    context 'when the feature is enabled' do
+      before do
+        stub_licensed_features(security_on_demand_scans: true)
+      end
+
+      context 'when user can run an on demand scan' do
+        before do
+          project.add_developer(user)
+        end
+
+        it 'communicates success' do
+          expect(subject.status).to eq(:success)
+        end
+
+        it 'returns a pipeline and pipeline_url' do
+          aggregate_failures do
+            expect(subject.payload[:pipeline]).to be_a(Ci::Pipeline)
+            expect(subject.payload[:pipeline_url]).to be_a(String)
+          end
+        end
+
+        it 'delegates pipeline creation to Ci::RunDastScanService' do
+          expected_params = {
+            branch: 'master',
+            full_scan_enabled: false,
+            show_debug_messages: false,
+            spider_timeout: nil,
+            target_timeout: nil,
+            target_url: dast_site_profile.dast_site.url,
+            use_ajax_spider: false
+          }
+
+          service = double(Ci::RunDastScanService)
+          response = ServiceResponse.error(message: 'Stubbed response')
+
+          aggregate_failures do
+            expect(Ci::RunDastScanService).to receive(:new).and_return(service)
+            expect(service).to receive(:execute).with(expected_params).and_return(response)
+          end
+
+          subject
+        end
+
+        context 'when dast_scanner_profile is nil' do
+          let(:dast_scanner_profile) { nil }
+
+          it 'communicates success' do
+            expect(subject.status).to eq(:success)
+          end
+        end
+
+        context 'when target is not validated and an active scan is requested' do
+          let(:dast_scanner_profile) { create(:dast_scanner_profile, project: project, scan_type: 'active') }
+
+          it 'communicates failure' do
+            aggregate_failures do
+              expect(subject.status).to eq(:error)
+              expect(subject.message).to eq('Cannot run active scan against unvalidated target')
+            end
+          end
+        end
+      end
+    end
+  end
+end