Merge branch...

Merge branch 'philipcunningham-fix-bug-always-returning-none-from-dast-site-profile-292739' into 'master'

Set dast_site_validation_id on  validation create

See merge request gitlab-org/gitlab!49791

ee/app/models/dast_site_token.rb

@@ -6,4 +6,8 @@ class DastSiteToken < ApplicationRecord
   validates :project_id, presence: true
   validates :token, length: { maximum: 255 }, presence: true
   validates :url, length: { maximum: 255 }, presence: true, public_url: true
+
+  def dast_site
+    @dast_site ||= DastSite.find_by(project_id: project.id, url: url)
+  end
 end

ee/app/models/dast_site_validation.rb

@@ -25,7 +25,7 @@ class DastSiteValidation < ApplicationRecord
 
   enum validation_strategy: { text_file: 0, header: 1 }
 
-  delegate :project, to: :dast_site_token, allow_nil: true
+  delegate :project, :dast_site, to: :dast_site_token, allow_nil: true
 
   def validation_url
     "#{url_base}/#{url_path}"

ee/app/services/dast_site_validations/create_service.rb

@@ -8,6 +8,10 @@ module DastSiteValidations
 
       dast_site_validation = create_validation!
 
+      return ServiceResponse.error(message: 'Site does not exist for profile') unless dast_site_validation.dast_site
+
+      associate_dast_site!(dast_site_validation)
+
       perform_async_validation(dast_site_validation)
     rescue ActiveRecord::RecordInvalid => err
       ServiceResponse.error(message: err.record.errors.full_messages)
@@ -43,6 +47,10 @@ module DastSiteValidations
       @url_base ||= DastSiteValidation.get_normalized_url_base(dast_site_token.url)
     end
 
+    def associate_dast_site!(dast_site_validation)
+      dast_site_validation.dast_site.update!(dast_site_validation_id: dast_site_validation.id)
+    end
+
     def find_latest_successful_dast_site_validation
       DastSiteValidationsFinder.new(
         project_id: container.id,

ee/spec/factories/dast_site_profiles.rb

@@ -2,13 +2,16 @@
 
 FactoryBot.define do
   factory :dast_site_profile do
+    project
+
+    dast_site { association :dast_site, project: project }
+
     sequence :name do |i|
       "#{FFaker::Product.product_name.truncate(200)} - #{i}"
     end
 
-    before(:create) do |dast_site_profile|
-      dast_site_profile.project ||= FactoryBot.create(:project)
-      dast_site_profile.dast_site ||= FactoryBot.create(:dast_site, project: dast_site_profile.project)
+    trait :with_dast_site_validation do
+      dast_site { association :dast_site, :with_dast_site_validation, project: project }
     end
   end
 end

ee/spec/factories/dast_site_tokens.rb

@@ -2,11 +2,10 @@
 
 FactoryBot.define do
   factory :dast_site_token do
+    project
+
     token { SecureRandom.uuid }
-    url { generate(:url) }
 
-    before(:create) do |dast_site_token|
-      dast_site_token.project ||= FactoryBot.create(:project)
-    end
+    url { generate(:url) }
   end
 end

ee/spec/factories/dast_site_validations.rb

@@ -2,11 +2,10 @@
 
 FactoryBot.define do
   factory :dast_site_validation do
+    dast_site_token
+
     validation_strategy { DastSiteValidation.validation_strategies[:text_file] }
-    url_path { 'some/path/GitLab-DAST-Site-Validation.txt' }
 
-    before(:create) do |dast_site_validation|
-      dast_site_validation.dast_site_token ||= FactoryBot.create(:dast_site_token)
-    end
+    url_path { 'some/path/GitLab-DAST-Site-Validation.txt' }
   end
 end

ee/spec/factories/dast_sites.rb

@@ -2,17 +2,14 @@
 
 FactoryBot.define do
   factory :dast_site do
+    project
+
     url { generate(:url) }
 
-    before(:create) do |dast_site|
-      dast_site.project ||= FactoryBot.create(:project)
-      dast_site.dast_site_validation ||= FactoryBot.create(
-        :dast_site_validation,
-        dast_site_token: FactoryBot.create(
-          :dast_site_token,
-          project: dast_site.project
-        )
-      )
+    trait :with_dast_site_validation do
+      dast_site_validation do
+        association(:dast_site_validation, dast_site_token: association(:dast_site_token, project: project))
+      end
     end
   end
 end

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

@@ -7,7 +7,8 @@ RSpec.describe Mutations::DastSiteValidations::Create do
   let(:project) { dast_site_token.project }
   let(:user) { create(:user) }
   let(:full_path) { project.full_path }
-  let(:dast_site_token) { create(:dast_site_token, project: create(:project, group: group)) }
+  let(:dast_site) { create(:dast_site, project: create(:project, group: group)) }
+  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 }
 

ee/spec/graphql/types/dast_site_profile_type_spec.rb

@@ -86,7 +86,7 @@ RSpec.describe GitlabSchema.types['DastSiteProfile'] do
 
     describe 'validation_status field' do
       it 'is the validation status' do
-        expect(first_dast_site_profile['validationStatus']).to eq('PENDING_VALIDATION')
+        expect(first_dast_site_profile['validationStatus']).to eq('NONE')
       end
     end
 

ee/spec/models/dast_site_profile_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe DastSiteProfile, type: :model do
-  subject { create(:dast_site_profile) }
+  subject { create(:dast_site_profile, :with_dast_site_validation) }
 
   describe 'associations' do
     it { is_expected.to belong_to(:project) }

ee/spec/models/dast_site_token_spec.rb

@@ -28,4 +28,20 @@ RSpec.describe DastSiteToken, type: :model do
       end
     end
   end
+
+  describe '#dast_site' do
+    context 'when dast_site exists' do
+      it 'finds the associated dast_site' do
+        dast_site = create(:dast_site, project_id: subject.project_id, url: subject.url)
+
+        expect(subject.dast_site).to eq(dast_site)
+      end
+    end
+
+    context 'when dast_site does not exist' do
+      it 'returns nil' do
+        expect(subject.dast_site).to be_nil
+      end
+    end
+  end
 end

ee/spec/models/dast_site_validation_spec.rb

@@ -87,6 +87,12 @@ RSpec.describe DastSiteValidation, type: :model do
     end
   end
 
+  describe '#dast_site' do
+    it 'returns dast_site through dast_site_token' do
+      expect(subject.dast_site).to eq(subject.dast_site_token.dast_site)
+    end
+  end
+
   describe '#validation_url' do
     it 'formats the url correctly' do
       expect(subject.validation_url).to eq("#{subject.url_base}/#{subject.url_path}")

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

@@ -5,9 +5,10 @@ require 'spec_helper'
 RSpec.describe 'Creating a DAST Site Token' do
   include GraphqlHelpers
 
-  let(:dast_site_token) { create(:dast_site_token, project: project) }
-  let(:dast_site_validation) { DastSiteValidation.find_by!(url_path: validation_path) }
+  let(:dast_site) { create(:dast_site, project: project) }
+  let(:dast_site_token) { create(:dast_site_token, project: project, url: dast_site.url) }
   let(:validation_path) { SecureRandom.hex }
+  let(:dast_site_validation) { DastSiteValidation.find_by!(url_path: validation_path) }
 
   let(:mutation_name) { :dast_site_validation_create }
   let(:mutation) do

ee/spec/requests/api/graphql/project/dast_site_profile_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe 'Query.project(fullPath).dastSiteProfile' do
   include GraphqlHelpers
 
-  let_it_be(:dast_site_profile) { create(:dast_site_profile) }
+  let_it_be(:dast_site_profile) { create(:dast_site_profile, :with_dast_site_validation) }
   let_it_be(:project) { dast_site_profile.project }
   let_it_be(:current_user) { create(:user) }
 

ee/spec/services/dast_site_validations/create_service_spec.rb

@@ -3,14 +3,19 @@
 require 'spec_helper'
 
 RSpec.describe DastSiteValidations::CreateService do
-  let(:project) { dast_site_token.project }
-  let(:dast_site_token) { create(:dast_site_token, project: create(:project)) }
-  let(:url_path) { SecureRandom.hex }
-  let(:params) { { dast_site_token: dast_site_token, url_path: url_path, validation_strategy: :text_file } }
+  let_it_be(:project) { create(: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) }
 
-  subject { described_class.new(container: project, params: params).execute }
+  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 }
+
+  describe 'execute', :clean_gitlab_redis_shared_state do
+    before do
+      project.clear_memoization(:licensed_feature_available)
+    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)
@@ -45,6 +50,10 @@ RSpec.describe DastSiteValidations::CreateService do
         expect(subject.status).to eq(:success)
       end
 
+      it 'associates the dast_site_validation with the dast_site' do
+        expect(subject.payload).to eq(dast_site.reload.dast_site_validation)
+      end
+
       it 'attempts to validate' do
         aggregate_failures do
           expect(DastSiteValidationWorker).to receive(:perform_async)
@@ -83,7 +92,7 @@ RSpec.describe DastSiteValidations::CreateService do
       end
 
       context 'when the dast_site_token.project and container do not match' do
-        let(:project) { create(:project) }
+        let_it_be(:dast_site_token) { create(:dast_site_token, project: create(:project), url: dast_site.url) }
 
         it 'communicates failure' do
           aggregate_failures do
@@ -99,7 +108,7 @@ RSpec.describe DastSiteValidations::CreateService do
         end
 
         let(:dast_site_validation) do
-          DastSiteValidation.find_by!(dast_site_token: dast_site_token, url_path: url_path)
+          DastSiteValidation.find_by!(dast_site_token: dast_site_token, url_path: params[:url_path])
         end
 
         it 'communicates failure' do
@@ -123,6 +132,17 @@ RSpec.describe DastSiteValidations::CreateService do
           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 '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
   end
 end