Merge branch 'philipcunningham-long-lived-tokens-dast-validation-337932' into 'master'

Reuse existing DastSiteToken if it already exists

See merge request gitlab-org/gitlab!67933

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

@@ -32,8 +32,8 @@ module Mutations
       def resolve(full_path:, target_url:)
         project = authorized_find!(full_path)
 
-        response = ::DastSiteTokens::CreateService.new(
-          container: project,
+        response = ::AppSec::Dast::SiteTokens::FindOrCreateService.new(
+          project: project,
           params: { target_url: target_url }
         ).execute
 

ee/app/models/dast_site_token.rb

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

ee/app/services/app_sec/dast/site_tokens/find_or_create_service.rb

+# frozen_string_literal: true
+
+module AppSec
+  module Dast
+    module SiteTokens
+      class FindOrCreateService < BaseProjectService
+        def execute
+          return ServiceResponse.error(message: 'Insufficient permissions') unless allowed?
+
+          existing_validation = find_dast_site_validation
+
+          return success_response(existing_validation.dast_site_token, existing_validation.state) if existing_validation
+
+          find_or_create_dast_site_token
+        rescue URI::InvalidURIError
+          error_response('Invalid target_url')
+        end
+
+        private
+
+        def allowed?
+          project.licensed_feature_available?(:security_on_demand_scans)
+        end
+
+        def error_response(message)
+          ServiceResponse.error(message: message)
+        end
+
+        def success_response(dast_site_token, status)
+          ServiceResponse.success(payload: { dast_site_token: dast_site_token, status: status })
+        end
+
+        def find_or_create_dast_site_token
+          existing_token = DastSiteToken.find_by(project: project, url: params[:target_url]) # rubocop: disable CodeReuse/ActiveRecord
+
+          return success_response(existing_token, DastSiteValidation::INITIAL_STATE) if existing_token
+
+          new_token = DastSiteToken.create(project: project, token: SecureRandom.uuid, url: params[:target_url])
+
+          return error_response(new_token.errors.full_messages) unless new_token.valid?
+
+          success_response(new_token, DastSiteValidation::INITIAL_STATE)
+        end
+
+        def find_dast_site_validation
+          url_base = DastSiteValidation.get_normalized_url_base(params[:target_url])
+
+          DastSiteValidationsFinder.new(project_id: project.id, url_base: url_base)
+            .execute
+            .first
+        end
+      end
+    end
+  end
+end

ee/app/services/dast_site_tokens/create_service.rb

-# frozen_string_literal: true
-
-module DastSiteTokens
-  class CreateService < BaseContainerService
-    def execute
-      return ServiceResponse.error(message: 'Insufficient permissions') unless allowed?
-
-      target_url = params[:target_url]
-      url_base = normalize_target_url(target_url)
-
-      dast_site_token = DastSiteToken.create!(
-        project: container,
-        token: SecureRandom.uuid,
-        url: target_url
-      )
-
-      dast_site_validation = find_dast_site_validation(url_base)
-      status = calculate_status(dast_site_validation)
-
-      ServiceResponse.success(
-        payload: { dast_site_token: dast_site_token, status: status }
-      )
-    rescue ActiveRecord::RecordInvalid => err
-      ServiceResponse.error(message: err.record.errors.full_messages)
-    rescue URI::InvalidURIError
-      ServiceResponse.error(message: 'Invalid target_url')
-    end
-
-    private
-
-    def allowed?
-      container.feature_available?(:security_on_demand_scans)
-    end
-
-    def normalize_target_url(target_url)
-      DastSiteValidation.get_normalized_url_base(target_url)
-    end
-
-    def find_dast_site_validation(url_base)
-      DastSiteValidationsFinder.new(project_id: container.id, url_base: url_base)
-        .execute
-        .first
-    end
-
-    def calculate_status(dast_site_validation)
-      dast_site_validation&.state || DastSiteValidation::INITIAL_STATE
-    end
-  end
-end

ee/spec/models/dast_site_token_spec.rb

@@ -16,6 +16,8 @@ RSpec.describe DastSiteToken, type: :model do
     it { is_expected.to validate_length_of(:url).is_at_most(255) }
     it { is_expected.to validate_presence_of(:token) }
     it { is_expected.to validate_presence_of(:url) }
+    it { is_expected.to validate_uniqueness_of(:token) }
+    it { is_expected.to validate_uniqueness_of(:url).scoped_to(:project_id) }
 
     context 'when the url is not public' do
       subject { build(:dast_site_token, url: 'http://127.0.0.1') }

ee/spec/services/dast_site_tokens/create_service_spec.rb → ee/spec/services/app_sec/dast/site_tokens/find_or_create_service_spec.rb

@@ -2,13 +2,13 @@
 
 require 'spec_helper'
 
-RSpec.describe DastSiteTokens::CreateService do
-  let(:project) { create(:project) }
-  let(:target_url) { generate(:url) }
+RSpec.describe AppSec::Dast::SiteTokens::FindOrCreateService do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:target_url) { generate(:url) }
 
   subject do
     described_class.new(
-      container: project,
+      project: project,
       params: { target_url: target_url }
     ).execute
   end
@@ -18,10 +18,7 @@ RSpec.describe DastSiteTokens::CreateService 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
+        expect(subject).to have_attributes(status: :error, message: 'Insufficient permissions')
       end
     end
 
@@ -30,26 +27,39 @@ RSpec.describe DastSiteTokens::CreateService do
         stub_licensed_features(security_on_demand_scans: true)
       end
 
+      it 'creates a new token' do
+        expect { subject }.to change { DastSiteToken.count }.by(1)
+      end
+
       it 'communicates success' do
-        expect(subject.status).to eq(:success)
+        expect(subject).to have_attributes(status: :success, payload: { dast_site_token: instance_of(DastSiteToken), status: 'pending' })
+      end
+
+      context 'when the token already exists' do
+        let_it_be(:dast_site_token) { create(:dast_site_token, project: project, url: target_url) }
+
+        it 'does not create a new token' do
+          expect { subject }.not_to change { DastSiteToken.count }
         end
 
-      it 'contains a dast_site_validation' do
-        expect(subject.payload[:dast_site_token]).to be_a(DastSiteToken)
+        it 'includes it in the payload' do
+          expect(subject).to have_attributes(status: :success, payload: hash_including(dast_site_token: dast_site_token))
         end
 
-      it 'contains a status' do
-        expect(subject.payload[:status]).to eq('pending')
+        context 'when an existing validation exists' do
+          let_it_be(:dast_site_validation) { create(:dast_site_validation, dast_site_token: dast_site_token, state: :passed) }
+
+          it 'includes its status in the payload' do
+            expect(subject).to have_attributes(status: :success, payload: hash_including(status: dast_site_validation.state))
+          end
+        end
       end
 
       context 'when an invalid target_url is supplied' do
-        let(:target_url) { 'http://bogus:broken' }
+        let_it_be(:target_url) { 'http://bogus:broken' }
 
         it 'communicates failure' do
-          aggregate_failures do
-            expect(subject.status).to eq(:error)
-            expect(subject.message).to eq('Invalid target_url')
-          end
+          expect(subject).to have_attributes(status: :error, message: 'Invalid target_url')
         end
 
         it 'does not create a dast_site_validation' do