Merge branch 'philipcunningham-dast-site-validation-runner-324990' into 'master'

Use runner for DAST Site Validation

See merge request gitlab-org/gitlab!65794

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

@@ -40,6 +40,7 @@ module Mutations
 
         response = ::DastSiteValidations::CreateService.new(
           container: project,
+          current_user: current_user,
           params: {
             dast_site_token: dast_site_token,
             url_path: validation_path,

ee/app/models/dast_site_validation.rb

@@ -36,7 +36,7 @@ class DastSiteValidation < ApplicationRecord
 
   state_machine :state, initial: INITIAL_STATE.to_sym do
     event :start do
-      transition pending: :inprogress
+      transition any => :inprogress
     end
 
     event :retry do

ee/app/models/ee/ci/build.rb

@@ -204,6 +204,8 @@ module EE
           # Subject to change. Please see gitlab-org/gitlab#330950 for more info.
           profile = pipeline.dast_profile || pipeline.dast_site_profile
 
+          break collection unless profile
+
           collection.concat(profile.secret_ci_variables(pipeline.user))
         end
       end

ee/app/services/app_sec/dast/site_validations/runner_service.rb

+# frozen_string_literal: true
+
+module AppSec
+  module Dast
+    module SiteValidations
+      class RunnerService < BaseProjectService
+        def execute
+          return ServiceResponse.error(message: _('Insufficient permissions')) unless allowed?
+
+          service = Ci::CreatePipelineService.new(project, current_user, ref: project.default_branch_or_main)
+          result = service.execute(:ondemand_dast_scan, content: ci_configuration.to_yaml, variables_attributes: dast_site_validation_variables)
+
+          if result.success?
+            ServiceResponse.success(payload: dast_site_validation)
+          else
+            dast_site_validation.fail_op
+
+            result
+          end
+        end
+
+        private
+
+        def allowed?
+          can?(current_user, :create_on_demand_dast_scan, project) &&
+            ::Feature.enabled?(:dast_runner_site_validation, project, default_enabled: :yaml)
+        end
+
+        def dast_site_validation
+          @dast_site_validation ||= params[:dast_site_validation]
+        end
+
+        def ci_configuration
+          { 'include' => [{ 'template' => 'DAST-Runner-Validation.gitlab-ci.yml' }] }
+        end
+
+        def dast_site_validation_variables
+          [
+            { key: 'DAST_SITE_VALIDATION_ID', secret_value: String(dast_site_validation.id) },
+            { key: 'DAST_SITE_VALIDATION_HEADER', secret_value: ::DastSiteValidation::HEADER },
+            { key: 'DAST_SITE_VALIDATION_STRATEGY', secret_value: dast_site_validation.validation_strategy },
+            { key: 'DAST_SITE_VALIDATION_TOKEN', secret_value: dast_site_validation.dast_site_token.token },
+            { key: 'DAST_SITE_VALIDATION_URL', secret_value: dast_site_validation.validation_url }
+          ]
+        end
+      end
+    end
+  end
+end

ee/app/services/dast_site_validations/create_service.rb

@@ -22,7 +22,7 @@ module DastSiteValidations
     private
 
     def allowed?
-      container.feature_available?(:security_on_demand_scans) &&
+      can?(current_user, :create_on_demand_dast_scan, container) &&
         dast_site_token.project == container
     end
 
@@ -67,6 +67,14 @@ module DastSiteValidations
     end
 
     def perform_async_validation(dast_site_validation)
+      if Feature.enabled?(:dast_runner_site_validation, dast_site_validation.project, default_enabled: :yaml)
+        runner_validation(dast_site_validation)
+      else
+        worker_validation(dast_site_validation)
+      end
+    end
+
+    def worker_validation(dast_site_validation)
       jid = DastSiteValidationWorker.perform_async(dast_site_validation.id)
 
       unless jid.present?
@@ -79,5 +87,13 @@ module DastSiteValidations
 
       ServiceResponse.success(payload: dast_site_validation)
     end
+
+    def runner_validation(dast_site_validation)
+      AppSec::Dast::SiteValidations::RunnerService.new(
+        project: dast_site_validation.project,
+        current_user: current_user,
+        params: { dast_site_validation: dast_site_validation }
+      ).execute
+    end
   end
 end

ee/lib/api/internal/app_sec/dast/site_validations.rb

@@ -45,7 +45,9 @@ module API
 
                   bad_request!('Could not update DAST site validation') unless success
 
-                  status 200, { state: validation.state }
+                  status 200
+
+                  { state: validation.state }
                 end
               end
             end

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

@@ -3,11 +3,10 @@
 require 'spec_helper'
 
 RSpec.describe Mutations::DastSiteValidations::Create do
-  let(:group) { create(:group) }
-  let(:project) { dast_site_token.project }
+  let(:project) { create(:project, :repository) }
   let(:user) { create(:user) }
   let(:full_path) { project.full_path }
-  let(:dast_site) { create(:dast_site, project: create(:project, group: group)) }
+  let(:dast_site) { create(:dast_site, project: project) }
   let(:dast_site_token) { create(:dast_site_token, project: dast_site.project, url: dast_site.url) }
   let(:dast_site_validation) { DastSiteValidation.find_by!(url_path: validation_path) }
   let(:validation_path) { SecureRandom.hex }
@@ -30,28 +29,42 @@ RSpec.describe Mutations::DastSiteValidations::Create do
       )
     end
 
-    context 'when on demand scan feature is enabled' do
-      context 'when the project does not exist' do
-        let(:full_path) { SecureRandom.hex }
+    shared_examples 'a validation mutation' do
+      context 'when on demand scan feature is enabled' do
+        context 'when the project does not exist' do
+          let(:full_path) { SecureRandom.hex }
 
-        it 'raises an exception' do
-          expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
+          it 'raises an exception' do
+            expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
+          end
         end
-      end
 
-      context 'when the user can run a dast scan' do
-        before do
-          project.add_developer(user)
-        end
+        context 'when the user can run a dast scan' do
+          before do
+            project.add_developer(user)
+          end
 
-        it 'returns the dast_site_validation id' do
-          expect(subject[:id]).to eq(dast_site_validation.to_global_id)
-        end
+          it 'returns the dast_site_validation id' do
+            expect(subject[:id]).to eq(dast_site_validation.to_global_id)
+          end
 
-        it 'returns the dast_site_validation status' do
-          expect(subject[:status]).to eq(dast_site_validation.state)
+          it 'returns the dast_site_validation status' do
+            expect(subject[:status]).to eq(dast_site_validation.state)
+          end
         end
       end
     end
+
+    context 'worker validation' do
+      before do
+        stub_feature_flags(dast_runner_site_validation: false)
+      end
+
+      it_behaves_like 'a validation mutation'
+    end
+
+    context 'runner validation' do
+      it_behaves_like 'a validation mutation'
+    end
   end
 end

ee/spec/lib/gitlab/ci/templates/dast_runner_validation_gitlab_ci_yaml_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Secure-Binaries.gitlab-ci.yml' do
+  subject(:template) { Gitlab::Template::GitlabCiYmlTemplate.find('DAST-Runner-Validation') }
+
+  specify { expect(template).not_to be_nil }
+
+  describe 'the created pipeline' do
+    let_it_be(:project) { create(:project, :custom_repo, files: { 'README.txt' => '' }) }
+
+    let(:default_branch) { project.default_branch_or_main }
+    let(:pipeline_branch) { default_branch }
+    let(:user) { project.owner }
+    let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) }
+    let(:pipeline) { service.execute!(:push).payload }
+    let(:build_names) { pipeline.builds.pluck(:name) }
+
+    before do
+      stub_ci_pipeline_yaml_file(template.content)
+
+      allow_next_instance_of(Ci::BuildScheduleWorker) do |worker|
+        allow(worker).to receive(:perform).and_return(true)
+      end
+
+      allow(project).to receive(:default_branch).and_return(default_branch)
+    end
+
+    describe 'validation' do
+      let_it_be(:build_name) { 'validation' }
+
+      it 'creates a validation job' do
+        expect(build_names).to include(build_name)
+      end
+
+      it 'sets DAST_RUNNER_VALIDATION_VERSION to the correct version' do
+        build = pipeline.builds.find_by(name: build_name)
+
+        expect(build.variables.to_hash).to include('DAST_RUNNER_VALIDATION_VERSION' => '1')
+      end
+    end
+  end
+end

ee/spec/models/dast_site_validation_spec.rb

@@ -100,32 +100,26 @@ RSpec.describe DastSiteValidation, type: :model do
   end
 
   describe '#start' do
-    context 'when state=pending' do
-      it 'returns true' do
-        expect(subject.start).to eq(true)
-      end
-
-      it 'records a timestamp' do
-        freeze_time do
-          subject.start
+    it 'is always possible to start over', :aggregate_failures do
+      described_class.state_machine.states.map(&:name).each do |state|
+        subject.state = state
 
-          expect(subject.reload.validation_started_at).to eq(Time.now.utc)
-        end
+        expect(subject.start).to eq(true)
       end
+    end
 
-      it 'transitions to the correct state' do
+    it 'records a timestamp' do
+      freeze_time do
         subject.start
 
-        expect(subject.state).to eq('inprogress')
+        expect(subject.reload.validation_started_at).to eq(Time.now.utc)
       end
     end
 
-    context 'otherwise' do
-      subject { create(:dast_site_validation, state: :failed) }
+    it 'transitions to the correct state' do
+      subject.start
 
-      it 'returns false' do
-        expect(subject.start).to eq(false)
-      end
+      expect(subject.state).to eq('inprogress')
     end
   end
 

ee/spec/requests/api/internal/app_sec/dast/site_validations_spec.rb

@@ -135,9 +135,20 @@ RSpec.describe API::Internal::AppSec::Dast::SiteValidations do
             end
           end
 
-          shared_examples 'it transitions' do |event|
+          shared_examples 'it transitions' do |event, initial_state|
             let(:event_param) { event }
 
+            before do
+              site_validation.update_column(:state, initial_state)
+            end
+
+            it 'returns 200 and the new state', :aggregate_failures do
+              subject
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(json_response).to eq('state' => site_validation.reload.state)
+            end
+
             it "calls the underlying transition method: ##{event}", :aggregate_failures do
               expect(DastSiteValidation).to receive(:find).with(String(site_validation.id)).and_return(site_validation)
               expect(site_validation).to receive(event).and_call_original
@@ -153,10 +164,10 @@ RSpec.describe API::Internal::AppSec::Dast::SiteValidations do
               expect { subject }.to change { site_validation.reload.state }.from('pending').to('inprogress')
             end
 
-            it_behaves_like 'it transitions', :start
-            it_behaves_like 'it transitions', :fail_op
-            it_behaves_like 'it transitions', :retry
-            it_behaves_like 'it transitions', :pass
+            it_behaves_like 'it transitions', :start, :pending
+            it_behaves_like 'it transitions', :fail_op, :inprogress
+            it_behaves_like 'it transitions', :retry, :failed
+            it_behaves_like 'it transitions', :pass, :inprogress
           end
         end
       end

ee/spec/services/app_sec/dast/site_validations/runner_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AppSec::Dast::SiteValidations::RunnerService do
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:developer) { create(:user, developer_projects: [project] ) }
+  let_it_be(:dast_site_token) { create(:dast_site_token, project: project) }
+  let_it_be(:dast_site_validation) { create(:dast_site_validation, dast_site_token: dast_site_token) }
+
+  subject do
+    described_class.new(project: project, current_user: developer, params: { dast_site_validation: dast_site_validation }).execute
+  end
+
+  describe 'execute' do
+    shared_examples 'a failure' do
+      it 'communicates failure' do
+        aggregate_failures do
+          expect(subject.status).to eq(:error)
+          expect(subject.message).to eq('Insufficient permissions')
+        end
+      end
+    end
+
+    context 'when on demand scan licensed feature is not available' do
+      before do
+        stub_licensed_features(security_on_demand_scans: false)
+      end
+
+      it_behaves_like 'a failure'
+    end
+
+    context 'when the feature is enabled' do
+      before do
+        stub_licensed_features(security_on_demand_scans: true)
+      end
+
+      it 'communicates success' do
+        expect(subject).to have_attributes(status: :success, payload: dast_site_validation)
+      end
+
+      it 'creates a ci_pipeline with ci_pipeline_variables' do
+        expect { subject }.to change { Ci::Pipeline.count }.by(1)
+      end
+
+      it 'makes the correct variables available to the ci_build' do
+        subject
+
+        build = Ci::Pipeline.last.builds.find_by(name: 'validation')
+
+        expected_variables = {
+          'DAST_SITE_VALIDATION_ID' => String(dast_site_validation.id),
+          'DAST_SITE_VALIDATION_HEADER' => ::DastSiteValidation::HEADER,
+          'DAST_SITE_VALIDATION_STRATEGY' => dast_site_validation.validation_strategy,
+          'DAST_SITE_VALIDATION_TOKEN' => dast_site_validation.dast_site_token.token,
+          'DAST_SITE_VALIDATION_URL' => dast_site_validation.validation_url
+        }
+
+        expect(build.variables.to_hash).to include(expected_variables)
+      end
+
+      context 'when pipeline creation fails' do
+        before do
+          allow_next_instance_of(Ci::Pipeline) do |instance|
+            allow(instance).to receive(:created_successfully?).and_return(false)
+            allow(instance).to receive(:full_error_messages).and_return('error message')
+          end
+        end
+
+        it 'transitions the dast_site_validation to a failure state', :aggregate_failures do
+          expect(dast_site_validation).to receive(:fail_op).and_call_original
+
+          expect { subject }.to change { dast_site_validation.state }.from('pending').to('failed')
+        end
+      end
+
+      context 'when feature flag is disabled' do
+        before do
+          stub_feature_flags(dast_runner_site_validation: false)
+        end
+
+        it_behaves_like 'a failure'
+      end
+    end
+  end
+end

ee/spec/services/dast_site_validations/create_service_spec.rb

@@ -3,19 +3,58 @@
 require 'spec_helper'
 
 RSpec.describe DastSiteValidations::CreateService do
-  let_it_be(:project) { create(:project) }
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:developer) { create(:user, developer_projects: [project]) }
   let_it_be(:dast_site) { create(:dast_site, project: project) }
   let_it_be(:dast_site_token) { create(:dast_site_token, project: project, url: dast_site.url) }
 
   let(:params) { { dast_site_token: dast_site_token, url_path: SecureRandom.hex, validation_strategy: :text_file } }
 
-  subject { described_class.new(container: dast_site.project, params: params).execute }
+  subject { described_class.new(container: project, current_user: developer, params: params).execute }
+
+  shared_examples 'the licensed feature is not available' 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
+
+  shared_examples 'the licensed feature is available' do
+    before do
+      stub_licensed_features(security_on_demand_scans: true)
+    end
+
+    it 'communicates success' do
+      expect(subject.status).to eq(:success)
+    end
+
+    it 'creates a new record in the database' do
+      expect { subject }.to change { DastSiteValidation.count }.by(1)
+    end
+
+    it 'associates the dast_site_validation with the dast_site' do
+      expect(subject.payload).to eq(dast_site.reload.dast_site_validation)
+    end
+
+    context 'when a param is missing' do
+      let(:params) { { dast_site_token: dast_site_token, validation_strategy: :text_file } }
 
-  describe 'execute', :clean_gitlab_redis_shared_state do
-    context 'when on demand scan licensed feature is not available' 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('Key not found: :url_path')
+        end
+      end
+    end
 
+    context 'when the dast_site_token.project and container do not match' do
+      let_it_be(:dast_site_token) { create(:dast_site_token, project: create(:project), url: dast_site.url) }
+
+      it 'communicates failure' do
         aggregate_failures do
           expect(subject.status).to eq(:error)
           expect(subject.message).to eq('Insufficient permissions')
@@ -23,105 +62,103 @@ RSpec.describe DastSiteValidations::CreateService do
       end
     end
 
-    context 'when the feature is available' do
-      before do
-        stub_licensed_features(security_on_demand_scans: true)
-      end
+    context 'when the dast_site_token does not have a related dast_site via its url' do
+      let_it_be(:dast_site_token) { create(:dast_site_token, project: project, url: generate(:url)) }
 
-      it 'communicates success' do
-        expect(subject.status).to eq(:success)
+      it 'communicates failure' do
+        aggregate_failures do
+          expect(subject.status).to eq(:error)
+          expect(subject.message).to eq('Site does not exist for profile')
+        end
       end
+    end
+  end
 
-      it 'associates the dast_site_validation with the dast_site' do
-        expect(subject.payload).to eq(dast_site.reload.dast_site_validation)
-      end
+  shared_examples 'a dast_site_validation already exists' do
+    let!(:dast_site_validation) { create(:dast_site_validation, dast_site_token: dast_site_token, state: :passed) }
 
-      it 'attempts to validate' do
-        aggregate_failures do
-          expect(DastSiteValidationWorker).to receive(:perform_async)
+    it 'returns the existing successful dast_site_validation' do
+      expect(subject.payload).to eq(dast_site_validation)
+    end
 
-          expect { subject }.to change { DastSiteValidation.count }.by(1)
-        end
+    it 'does not create a new record in the database' do
+      expect { subject }.not_to change { DastSiteValidation.count }
+    end
+  end
+
+  describe 'execute', :clean_gitlab_redis_shared_state do
+    context 'worker validation' do
+      before do
+        stub_feature_flags(dast_runner_site_validation: false)
       end
 
-      context 'when the associated dast_site_validation has successfully been validated' do
-        it 'returns the existing successful dast_site_validation' do
-          dast_site_validation = create(:dast_site_validation, dast_site_token: dast_site_token, state: :passed)
+      it_behaves_like 'the licensed feature is not available'
+
+      it_behaves_like 'the licensed feature is available' do
+        it 'attempts to validate' do
+          expect(DastSiteValidationWorker).to receive(:perform_async)
 
-          expect(subject.payload).to eq(dast_site_validation)
+          subject
         end
 
-        it 'does not attempt to re-validate' do
-          create(:dast_site_validation, dast_site_token: dast_site_token, state: :passed)
+        context 'when worker does not return a job id' do
+          before do
+            allow(DastSiteValidationWorker).to receive(:perform_async).and_return(nil)
+          end
 
-          aggregate_failures do
-            expect(DastSiteValidationWorker).not_to receive(:perform_async)
+          let(:dast_site_validation) do
+            DastSiteValidation.find_by!(dast_site_token: dast_site_token, url_path: params[:url_path])
+          end
 
-            expect { subject }.not_to change { DastSiteValidation.count }
+          it 'communicates failure' do
+            aggregate_failures do
+              expect(subject.status).to eq(:error)
+              expect(subject.message).to eq('Validation failed')
+            end
           end
-        end
-      end
 
-      context 'when a param is missing' do
-        let(:params) { { dast_site_token: dast_site_token, validation_strategy: :text_file } }
+          it 'sets dast_site_validation.state to failed' do
+            subject
 
-        it 'communicates failure' do
-          aggregate_failures do
-            expect(subject.status).to eq(:error)
-            expect(subject.message).to eq('Key not found: :url_path')
+            expect(dast_site_validation.state).to eq('failed')
           end
-        end
-      end
 
-      context 'when the dast_site_token.project and container do not match' do
-        let_it_be(:dast_site_token) { create(:dast_site_token, project: create(:project), url: dast_site.url) }
+          it 'logs an error' do
+            allow(Gitlab::AppLogger).to receive(:error)
 
-        it 'communicates failure' do
-          aggregate_failures do
-            expect(subject.status).to eq(:error)
-            expect(subject.message).to eq('Insufficient permissions')
-          end
-        end
-      end
+            subject
 
-      context 'when worker does not return a job id' do
-        before do
-          allow(DastSiteValidationWorker).to receive(:perform_async).and_return(nil)
+            expect(Gitlab::AppLogger).to have_received(:error).with(message: 'Unable to validate dast_site_validation', dast_site_validation_id: dast_site_validation.id)
+          end
         end
 
-        let(:dast_site_validation) do
-          DastSiteValidation.find_by!(dast_site_token: dast_site_token, url_path: params[:url_path])
-        end
+        it_behaves_like 'a dast_site_validation already exists' do
+          it 'does not attempt to re-validate' do
+            expect(DastSiteValidationWorker).not_to receive(:perform_async)
 
-        it 'communicates failure' do
-          aggregate_failures do
-            expect(subject.status).to eq(:error)
-            expect(subject.message).to eq('Validation failed')
+            subject
           end
         end
+      end
+    end
 
-        it 'sets dast_site_validation.state to failed' do
-          subject
+    context 'runner validation' do
+      it_behaves_like 'the licensed feature is not available'
 
-          expect(dast_site_validation.state).to eq('failed')
-        end
+      it_behaves_like 'the licensed feature is available' do
+        it 'attempts to validate' do
+          expected_args = { project: project, current_user: developer, params: { dast_site_validation: instance_of(DastSiteValidation) } }
 
-        it 'logs an error' do
-          allow(Gitlab::AppLogger).to receive(:error)
+          expect(AppSec::Dast::SiteValidations::RunnerService).to receive(:new).with(expected_args).and_call_original
 
           subject
-
-          expect(Gitlab::AppLogger).to have_received(:error).with(message: 'Unable to validate dast_site_validation', dast_site_validation_id: dast_site_validation.id)
         end
-      end
 
-      context 'when the dast_site_token does not have a related dast_site via its url' do
-        let_it_be(:dast_site_token) { create(:dast_site_token, project: project, url: generate(:url)) }
+        it_behaves_like 'a dast_site_validation already exists' do
+          it 'does not attempt to re-validate' do
+            expect(AppSec::Dast::SiteValidations::RunnerService).not_to receive(:new)
 
-        it 'communicates failure' do
-          aggregate_failures do
-            expect(subject.status).to eq(:error)
-            expect(subject.message).to eq('Site does not exist for profile')
+            subject
           end
         end
       end

lib/gitlab/ci/templates/Security/DAST-Runner-Validation.gitlab-ci.yml

+# To contribute improvements to CI/CD templates, please follow the Development guide at:
+# https://docs.gitlab.com/ee/development/cicd/templates.html
+# This specific template is located at:
+# https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/DAST-Runner-Validation.gitlab-ci.yml
+
+stages:
+  - build
+  - test
+  - deploy
+  - dast
+
+variables:
+  DAST_RUNNER_VALIDATION_VERSION: 1
+
+validation:
+  stage: dast
+  image:
+    name: "registry.gitlab.com/security-products/dast-runner-validation:$DAST_RUNNER_VALIDATION_VERSION"
+  variables:
+    GIT_STRATEGY: none
+  allow_failure: false
+  script:
+    - ~/validate.sh