Merge branch 'add-dast-site-validation-worker-245209' into 'master'

Add ability to validate sites for on-demand DAST

See merge request gitlab-org/gitlab!42198

changelogs/unreleased/add-dast-site-validation-worker-245209.yml

+---
+title: Add state field to DastSiteValidation
+merge_request: 42198
+author:
+type: added

config/sidekiq_queues.yml

@@ -62,6 +62,8 @@
   - 1
 - - cronjob
   - 1
+- - dast_site_validation
+  - 1
 - - default
   - 1
 - - delete_diff_files

db/migrate/20200928095732_add_state_to_dast_site_validation.rb

+# frozen_string_literal: true
+
+class AddStateToDastSiteValidation < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  # rubocop:disable Migration/AddLimitToTextColumns
+  # limit is added in 20200928100408_add_text_limit_to_dast_site_validation_state.rb
+  def change
+    add_column :dast_site_validations, :state, :text, default: :pending, null: false
+  end
+  # rubocop:enable Migration/AddLimitToTextColumns
+end

db/migrate/20200928100408_add_text_limit_to_dast_site_validation_state.rb

+# frozen_string_literal: true
+
+class AddTextLimitToDastSiteValidationState < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_text_limit :dast_site_validations, :state, 255
+  end
+
+  def down
+    remove_text_limit :dast_site_validations, :state
+  end
+end

db/schema_migrations/20200928095732

+5ff9bf6c542f686729abf18f282351d9bb5b895070c6f06c5fc8d125be4a38f7
\ No newline at end of file

db/schema_migrations/20200928100408

+56984f720cfde6ad28b8612e092809252f948797fecb64dfc3b82d5b3b74f7ec
\ No newline at end of file

db/structure.sql

@@ -11232,7 +11232,9 @@ CREATE TABLE dast_site_validations (
     validation_strategy smallint NOT NULL,
     url_base text NOT NULL,
     url_path text NOT NULL,
+    state text DEFAULT 'pending'::text NOT NULL,
     CONSTRAINT check_13b34efe4b CHECK ((char_length(url_path) <= 255)),
+    CONSTRAINT check_283be72e9b CHECK ((char_length(state) <= 255)),
     CONSTRAINT check_cd3b538210 CHECK ((char_length(url_base) <= 255))
 );
 

ee/app/models/dast_site_validation.rb

@@ -11,15 +11,53 @@ class DastSiteValidation < ApplicationRecord
     joins(:dast_site_token).where(dast_site_tokens: { project_id: project_id })
   end
 
-  before_create :set_url_base
+  before_create :set_normalized_url_base
 
   enum validation_strategy: { text_file: 0 }
 
   delegate :project, to: :dast_site_token, allow_nil: true
 
+  def validation_url
+    "#{url_base}/#{url_path}"
+  end
+
+  state_machine :state, initial: :pending do
+    event :start do
+      transition pending: :inprogress
+    end
+
+    event :retry do
+      transition failed: :inprogress
+    end
+
+    event :fail_op do
+      transition any - :failed => :failed
+    end
+
+    event :pass do
+      transition inprogress: :passed
+    end
+
+    before_transition pending: :inprogress do |validation|
+      validation.validation_started_at = Time.now.utc
+    end
+
+    before_transition failed: :inprogress do |validation|
+      validation.validation_last_retried_at = Time.now.utc
+    end
+
+    before_transition any - :failed => :failed do |validation|
+      validation.validation_failed_at = Time.now.utc
+    end
+
+    before_transition inprogress: :passed do |validation|
+      validation.validation_passed_at = Time.now.utc
+    end
+  end
+
   private
 
-  def set_url_base
+  def set_normalized_url_base
     uri = URI(dast_site_token.url)
 
     self.url_base = "%{scheme}://%{host}:%{port}" % { scheme: uri.scheme, host: uri.host, port: uri.port }

ee/app/services/dast_site_validations/validate_service.rb

+# frozen_string_literal: true
+
+module DastSiteValidations
+  class ValidateService < BaseContainerService
+    PermissionsError = Class.new(StandardError)
+    TokenNotFound = Class.new(StandardError)
+
+    def execute!
+      raise PermissionsError.new('Insufficient permissions') unless allowed?
+
+      return if dast_site_validation.passed?
+
+      if dast_site_validation.pending?
+        dast_site_validation.start
+      else
+        dast_site_validation.retry
+      end
+
+      response = make_http_request!
+
+      validate!(response)
+    end
+
+    private
+
+    def allowed?
+      container.feature_available?(:security_on_demand_scans) &&
+        Feature.enabled?(:security_on_demand_scans_site_validation, container)
+    end
+
+    def dast_site_validation
+      @dast_site_validation ||= params.fetch(:dast_site_validation)
+    end
+
+    def make_http_request!
+      uri, _ = Gitlab::UrlBlocker.validate!(dast_site_validation.validation_url)
+      Gitlab::HTTP.get(uri)
+    end
+
+    def token_found?(response)
+      response.body.include?(dast_site_validation.dast_site_token.token)
+    end
+
+    def validate!(response)
+      raise TokenNotFound.new('Could not find token in response body') unless token_found?(response)
+
+      dast_site_validation.pass
+    end
+  end
+end

ee/app/workers/all_queues.yml

@@ -571,6 +571,14 @@
   :weight: 2
   :idempotent:
   :tags: []
+- :name: dast_site_validation
+  :feature_category: :dynamic_application_security_testing
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: elastic_commit_indexer
   :feature_category: :global_search
   :has_external_dependencies:

ee/app/workers/dast_site_validation_worker.rb

+# frozen_string_literal: true
+
+class DastSiteValidationWorker
+  include ApplicationWorker
+
+  idempotent!
+
+  sidekiq_options retry: 3, dead: false
+
+  sidekiq_retry_in { 25 }
+
+  feature_category :dynamic_application_security_testing
+
+  sidekiq_retries_exhausted do |job|
+    dast_site_validation = DastSiteValidation.find(job['args'][0])
+    dast_site_validation.fail_op
+  end
+
+  def perform(dast_site_validation_id)
+    dast_site_validation = DastSiteValidation.find(dast_site_validation_id)
+    project = dast_site_validation.project
+
+    DastSiteValidations::ValidateService.new(
+      container: project,
+      params: { dast_site_validation: dast_site_validation }
+    ).execute!
+  end
+end

ee/changelogs/unreleased/add-dast-site-validation-worker-245209.yml

+---
+title: Add ability to validate sites for on-demand DAST
+merge_request: 42198
+author:
+type: added

ee/spec/models/dast_site_validation_spec.rb

@@ -16,10 +16,21 @@ RSpec.describe DastSiteValidation, type: :model do
   end
 
   describe 'before_create' do
-    it 'sets normalises the dast_site_token url' do
-      uri = URI(subject.dast_site_token.url)
+    describe '#set_normalized_url_base' do
+      subject do
+        dast_site_token = create(
+          :dast_site_token,
+          url: generate(:url) + '/' + SecureRandom.hex + '?' + { param: SecureRandom.hex }.to_query
+        )
 
-      expect(subject.url_base).to eq("#{uri.scheme}://#{uri.host}:#{uri.port}")
+        create(:dast_site_validation, dast_site_token: dast_site_token)
+      end
+
+      it 'normalizes the dast_site_token url' do
+        uri = URI(subject.dast_site_token.url)
+
+        expect(subject.url_base).to eq("#{uri.scheme}://#{uri.host}:#{uri.port}")
+      end
     end
   end
 
@@ -51,4 +62,130 @@ RSpec.describe DastSiteValidation, type: :model do
       expect(subject.project).to eq(subject.dast_site_token.project)
     end
   end
+
+  describe '#validation_url' do
+    it 'formats the url correctly' do
+      expect(subject.validation_url).to eq("#{subject.url_base}/#{subject.url_path}")
+    end
+  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
+
+          expect(subject.reload.validation_started_at).to eq(Time.now.utc)
+        end
+      end
+
+      it 'transitions to the correct state' do
+        subject.start
+
+        expect(subject.state).to eq('inprogress')
+      end
+    end
+
+    context 'otherwise' do
+      subject { create(:dast_site_validation, state: :failed) }
+
+      it 'returns false' do
+        expect(subject.start).to eq(false)
+      end
+    end
+  end
+
+  describe '#retry' do
+    context 'when state=failed' do
+      subject { create(:dast_site_validation, state: :failed) }
+
+      it 'returns true' do
+        expect(subject.retry).to eq(true)
+      end
+
+      it 'records a timestamp' do
+        freeze_time do
+          subject.retry
+
+          expect(subject.reload.validation_last_retried_at).to eq(Time.now.utc)
+        end
+      end
+
+      it 'transitions to the correct state' do
+        subject.retry
+
+        expect(subject.state).to eq('inprogress')
+      end
+    end
+
+    context 'otherwise' do
+      it 'returns false' do
+        expect(subject.retry).to eq(false)
+      end
+    end
+  end
+
+  describe '#fail_op' do
+    context 'when state=failed' do
+      subject { create(:dast_site_validation, state: :failed) }
+
+      it 'returns false' do
+        expect(subject.fail_op).to eq(false)
+      end
+    end
+
+    context 'otherwise' do
+      it 'returns true' do
+        expect(subject.fail_op).to eq(true)
+      end
+
+      it 'records a timestamp' do
+        freeze_time do
+          subject.fail_op
+
+          expect(subject.reload.validation_failed_at).to eq(Time.now.utc)
+        end
+      end
+
+      it 'transitions to the correct state' do
+        subject.fail_op
+
+        expect(subject.state).to eq('failed')
+      end
+    end
+  end
+
+  describe '#pass' do
+    context 'when state=inprogress' do
+      subject { create(:dast_site_validation, state: :inprogress) }
+
+      it 'returns true' do
+        expect(subject.pass).to eq(true)
+      end
+
+      it 'records a timestamp' do
+        freeze_time do
+          subject.pass
+
+          expect(subject.reload.validation_passed_at).to eq(Time.now.utc)
+        end
+      end
+
+      it 'transitions to the correct state' do
+        subject.pass
+
+        expect(subject.state).to eq('passed')
+      end
+    end
+
+    context 'otherwise' do
+      it 'returns false' do
+        expect(subject.pass).to eq(false)
+      end
+    end
+  end
 end

ee/spec/services/dast_site_validations/validate_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe DastSiteValidations::ValidateService do
+  let(:dast_site_validation) { create(:dast_site_validation) }
+
+  subject do
+    described_class.new(
+      container: dast_site_validation.project,
+      params: { dast_site_validation: dast_site_validation }
+    ).execute!
+  end
+
+  describe 'execute!' do
+    context 'when on demand scan feature is disabled' do
+      it 'communicates failure' do
+        stub_licensed_features(security_on_demand_scans: true)
+        stub_feature_flags(security_on_demand_scans_site_validation: false)
+
+        expect { subject }.to raise_error(DastSiteValidations::ValidateService::PermissionsError)
+      end
+    end
+
+    context 'when on demand scan licensed feature is not available' do
+      it 'communicates failure' do
+        stub_licensed_features(security_on_demand_scans: false)
+        stub_feature_flags(security_on_demand_scans_site_validation: true)
+
+        expect { subject }.to raise_error(DastSiteValidations::ValidateService::PermissionsError)
+      end
+    end
+
+    context 'when the feature is enabled' do
+      before do
+        stub_licensed_features(security_on_demand_scans: true)
+        stub_feature_flags(security_on_demand_scans_site_validation: true)
+        stub_request(:get, dast_site_validation.validation_url).to_return(body: response_body)
+      end
+
+      let(:response_body) do
+        dast_site_validation.dast_site_token.token
+      end
+
+      it 'validates the url before making an http request' do
+        uri = double('uri')
+
+        aggregate_failures do
+          expect(Gitlab::UrlBlocker).to receive(:validate!).and_return([uri, nil])
+          expect(Gitlab::HTTP).to receive(:get).with(uri).and_return(double('response', body: dast_site_validation.dast_site_token.token))
+        end
+
+        subject
+      end
+
+      context 'when the request body contains the token' do
+        it 'calls dast_site_validation#start' do
+          expect(dast_site_validation).to receive(:start)
+
+          subject
+        end
+
+        it 'calls dast_site_validation#pass' do
+          expect(dast_site_validation).to receive(:pass)
+
+          subject
+        end
+
+        it 'marks the validation successful' do
+          subject
+
+          expect(dast_site_validation.reload.state).to eq('passed')
+        end
+
+        context 'when validation has already started' do
+          let(:dast_site_validation) { create(:dast_site_validation, state: :inprogress) }
+
+          it 'does not call dast_site_validation#pass' do
+            expect(dast_site_validation).not_to receive(:start)
+
+            subject
+          end
+        end
+
+        context 'when validation is already complete' do
+          let(:dast_site_validation) { create(:dast_site_validation, state: :passed) }
+
+          it 'does not re-validate' do
+            expect(Gitlab::HTTP).not_to receive(:get)
+
+            subject
+          end
+        end
+      end
+
+      context 'when the request body does not contain the token' do
+        let(:response_body) do
+          SecureRandom.hex
+        end
+
+        it 'raises an exception' do
+          expect { subject }.to raise_error(DastSiteValidations::ValidateService::TokenNotFound)
+        end
+      end
+
+      context 'when validation has already been attempted' do
+        let_it_be(:dast_site_validation) { create(:dast_site_validation, state: :failed) }
+
+        it 'marks the validation as a retry' do
+          freeze_time do
+            subject
+
+            expect(dast_site_validation.reload.validation_last_retried_at).to eq(Time.now.utc)
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/workers/dast_site_validation_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe DastSiteValidationWorker do
+  let_it_be(:dast_site_validation) { create(:dast_site_validation) }
+
+  subject do
+    described_class.new.perform(dast_site_validation.id)
+  end
+
+  describe '#perform' do
+    it 'calls DastSiteValidations::ValidateService' do
+      double = double(DastSiteValidations::ValidateService, "execute!" => true)
+      args = { container: dast_site_validation.project, params: { dast_site_validation: dast_site_validation } }
+
+      expect(double).to receive(:execute!)
+      expect(DastSiteValidations::ValidateService).to receive(:new).with(args).and_return(double)
+
+      subject
+    end
+
+    context 'when the feature is enabled' do
+      let(:response_body) do
+        dast_site_validation.dast_site_token.token
+      end
+
+      before do
+        stub_licensed_features(security_on_demand_scans: true)
+        stub_feature_flags(security_on_demand_scans_site_validation: true)
+        stub_request(:get, dast_site_validation.validation_url).to_return(body: response_body)
+      end
+
+      context 'when the request body contains the token' do
+        include_examples 'an idempotent worker' do
+          subject do
+            perform_multiple([dast_site_validation.id], worker: described_class.new)
+          end
+        end
+      end
+    end
+  end
+
+  describe '.sidekiq_retries_exhausted' do
+    it 'calls find with the correct id' do
+      job = { 'args' => [dast_site_validation.id], 'jid' => '1' }
+
+      expect(dast_site_validation.class).to receive(:find).with(dast_site_validation.id).and_call_original
+
+      described_class.sidekiq_retries_exhausted_block.call(job)
+    end
+
+    it 'marks validation failed' do
+      job = { 'args' => [dast_site_validation.id], 'jid' => '1' }
+
+      freeze_time do
+        described_class.sidekiq_retries_exhausted_block.call(job)
+
+        aggregate_failures do
+          obj = dast_site_validation.reload
+
+          expect(obj.state).to eq('failed')
+          expect(obj.validation_failed_at).to eq(Time.now.utc)
+        end
+      end
+    end
+  end
+end