Merge branch 'on-demand-dast-validation-model-layer-245208' into 'master'

DAST Site validation - Model Layer

See merge request gitlab-org/gitlab!41639

changelogs/unreleased/on-demand-dast-validation-model-layer-245208.yml

+---
+title: DAST Site validation - Model Layer
+merge_request: 41639
+author:
+type: added

db/migrate/20200907021256_create_dast_site_tokens.rb

+# frozen_string_literal: true
+
+class CreateDastSiteTokens < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    unless table_exists?(:dast_site_tokens)
+      with_lock_retries do
+        create_table :dast_site_tokens do |t|
+          t.references :project, foreign_key: { on_delete: :cascade }, null: false, index: true
+
+          t.timestamps_with_timezone null: false
+          t.datetime_with_timezone :expired_at
+
+          t.text :token, null: false, unique: true
+          t.text :url, null: false
+        end
+      end
+    end
+
+    add_text_limit :dast_site_tokens, :token, 255
+    add_text_limit :dast_site_tokens, :url, 255
+  end
+
+  def down
+    with_lock_retries do
+      drop_table :dast_site_tokens
+    end
+  end
+end

db/migrate/20200907062101_create_dast_site_validations.rb

+# frozen_string_literal: true
+
+class CreateDastSiteValidations < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    unless table_exists?(:dast_site_validations)
+      with_lock_retries do
+        create_table :dast_site_validations do |t|
+          t.references :dast_site_token, foreign_key: { on_delete: :cascade }, null: false, index: true
+
+          t.timestamps_with_timezone null: false
+          t.datetime_with_timezone :validation_started_at
+          t.datetime_with_timezone :validation_passed_at
+          t.datetime_with_timezone :validation_failed_at
+          t.datetime_with_timezone :validation_last_retried_at
+
+          t.integer :validation_strategy, null: false, limit: 2
+
+          t.text :url_base, null: false
+          t.text :url_path, null: false
+        end
+      end
+    end
+
+    add_concurrent_index :dast_site_validations, :url_base
+    add_text_limit :dast_site_validations, :url_base, 255
+    add_text_limit :dast_site_validations, :url_path, 255
+  end
+
+  def down
+    with_lock_retries do
+      drop_table :dast_site_validations
+    end
+  end
+end

db/migrate/20200908033805_add_dast_site_validation_id_to_dast_site.rb

+# frozen_string_literal: true
+
+class AddDastSiteValidationIdToDastSite < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  TABLE_NAME = :dast_sites
+  RELATION_NAME = :dast_site_validations
+  FK_NAME = :dast_site_validation_id
+  INDEX_NAME = "index_dast_sites_on_#{FK_NAME}"
+
+  disable_ddl_transaction!
+
+  def up
+    unless column_exists?(TABLE_NAME, FK_NAME)
+      with_lock_retries do
+        add_column TABLE_NAME, FK_NAME, :bigint
+      end
+    end
+
+    add_concurrent_index TABLE_NAME, FK_NAME, name: INDEX_NAME
+    add_concurrent_foreign_key TABLE_NAME, RELATION_NAME, column: FK_NAME, on_delete: :nullify
+  end
+
+  def down
+    remove_foreign_key_if_exists TABLE_NAME, RELATION_NAME
+    remove_concurrent_index_by_name TABLE_NAME, INDEX_NAME
+
+    with_lock_retries do
+      remove_column TABLE_NAME, FK_NAME
+    end
+  end
+end

db/schema_migrations/20200907021256

+5fba5213226186a1506f672eb3eab2d07f58b019c4ba13760663cb119f62d4e2
\ No newline at end of file

db/schema_migrations/20200907062101

+002c92f830762d97dcbdbcf8a0287ebbb576edc27f4f76f4bb18d043e956ba7a
\ No newline at end of file

db/schema_migrations/20200908033805

+5f932b8a3503fc275ba6d09436115999b32f6438700e3b719f53730c5527a354
\ No newline at end of file

db/structure.sql

@@ -11202,12 +11202,59 @@ CREATE SEQUENCE public.dast_site_profiles_id_seq
 ALTER SEQUENCE public.dast_site_profiles_id_seq OWNED BY public.dast_site_profiles.id;
+CREATE TABLE public.dast_site_tokens (
+    id bigint NOT NULL,
+    project_id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    expired_at timestamp with time zone,
+    token text NOT NULL,
+    url text NOT NULL,
+    CONSTRAINT check_02a6bf20a7 CHECK ((char_length(token) <= 255)),
+    CONSTRAINT check_69ab8622a6 CHECK ((char_length(url) <= 255))
+);
+
+CREATE SEQUENCE public.dast_site_tokens_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE public.dast_site_tokens_id_seq OWNED BY public.dast_site_tokens.id;
+
+CREATE TABLE public.dast_site_validations (
+    id bigint NOT NULL,
+    dast_site_token_id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    validation_started_at timestamp with time zone,
+    validation_passed_at timestamp with time zone,
+    validation_failed_at timestamp with time zone,
+    validation_last_retried_at timestamp with time zone,
+    validation_strategy smallint NOT NULL,
+    url_base text NOT NULL,
+    url_path text NOT NULL,
+    CONSTRAINT check_13b34efe4b CHECK ((char_length(url_path) <= 255)),
+    CONSTRAINT check_cd3b538210 CHECK ((char_length(url_base) <= 255))
+);
+
+CREATE SEQUENCE public.dast_site_validations_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE public.dast_site_validations_id_seq OWNED BY public.dast_site_validations.id;
+
 CREATE TABLE public.dast_sites (
     id bigint NOT NULL,
     project_id bigint NOT NULL,
     created_at timestamp with time zone NOT NULL,
     updated_at timestamp with time zone NOT NULL,
     url text NOT NULL,
+    dast_site_validation_id bigint,
     CONSTRAINT check_46df8b449c CHECK ((char_length(url) <= 255))
 );
@@ -17166,6 +17213,10 @@ ALTER TABLE ONLY public.dast_scanner_profiles ALTER COLUMN id SET DEFAULT nextva
 ALTER TABLE ONLY public.dast_site_profiles ALTER COLUMN id SET DEFAULT nextval('public.dast_site_profiles_id_seq'::regclass);
+ALTER TABLE ONLY public.dast_site_tokens ALTER COLUMN id SET DEFAULT nextval('public.dast_site_tokens_id_seq'::regclass);
+
+ALTER TABLE ONLY public.dast_site_validations ALTER COLUMN id SET DEFAULT nextval('public.dast_site_validations_id_seq'::regclass);
+
 ALTER TABLE ONLY public.dast_sites ALTER COLUMN id SET DEFAULT nextval('public.dast_sites_id_seq'::regclass);
 ALTER TABLE ONLY public.dependency_proxy_blobs ALTER COLUMN id SET DEFAULT nextval('public.dependency_proxy_blobs_id_seq'::regclass);
@@ -18195,6 +18246,12 @@ ALTER TABLE ONLY public.dast_scanner_profiles
 ALTER TABLE ONLY public.dast_site_profiles
     ADD CONSTRAINT dast_site_profiles_pkey PRIMARY KEY (id);
+ALTER TABLE ONLY public.dast_site_tokens
+    ADD CONSTRAINT dast_site_tokens_pkey PRIMARY KEY (id);
+
+ALTER TABLE ONLY public.dast_site_validations
+    ADD CONSTRAINT dast_site_validations_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY public.dast_sites
     ADD CONSTRAINT dast_sites_pkey PRIMARY KEY (id);
@@ -19785,6 +19842,14 @@ CREATE INDEX index_dast_site_profiles_on_dast_site_id ON public.dast_site_profil
 CREATE UNIQUE INDEX index_dast_site_profiles_on_project_id_and_name ON public.dast_site_profiles USING btree (project_id, name);
+CREATE INDEX index_dast_site_tokens_on_project_id ON public.dast_site_tokens USING btree (project_id);
+
+CREATE INDEX index_dast_site_validations_on_dast_site_token_id ON public.dast_site_validations USING btree (dast_site_token_id);
+
+CREATE INDEX index_dast_site_validations_on_url_base ON public.dast_site_validations USING btree (url_base);
+
+CREATE INDEX index_dast_sites_on_dast_site_validation_id ON public.dast_sites USING btree (dast_site_validation_id);
+
 CREATE UNIQUE INDEX index_dast_sites_on_project_id_and_url ON public.dast_sites USING btree (project_id, url);
 CREATE INDEX index_dependency_proxy_blobs_on_group_id_and_file_name ON public.dependency_proxy_blobs USING btree (group_id, file_name);
@@ -21754,6 +21819,9 @@ ALTER TABLE ONLY public.merge_requests
 ALTER TABLE ONLY public.user_interacted_projects
     ADD CONSTRAINT fk_0894651f08 FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE;
+ALTER TABLE ONLY public.dast_sites
+    ADD CONSTRAINT fk_0a57f2271b FOREIGN KEY (dast_site_validation_id) REFERENCES public.dast_site_validations(id) ON DELETE SET NULL;
+
 ALTER TABLE ONLY public.web_hooks
     ADD CONSTRAINT fk_0c8ca6d9d1 FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
@@ -22537,6 +22605,9 @@ ALTER TABLE ONLY public.lfs_file_locks
 ALTER TABLE ONLY public.project_alerting_settings
     ADD CONSTRAINT fk_rails_27a84b407d FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
+ALTER TABLE ONLY public.dast_site_validations
+    ADD CONSTRAINT fk_rails_285c617324 FOREIGN KEY (dast_site_token_id) REFERENCES public.dast_site_tokens(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.resource_state_events
     ADD CONSTRAINT fk_rails_29af06892a FOREIGN KEY (issue_id) REFERENCES public.issues(id) ON DELETE CASCADE;
@@ -23464,6 +23535,9 @@ ALTER TABLE ONLY public.merge_request_metrics
 ALTER TABLE ONLY public.draft_notes
     ADD CONSTRAINT fk_rails_e753681674 FOREIGN KEY (merge_request_id) REFERENCES public.merge_requests(id) ON DELETE CASCADE;
+ALTER TABLE ONLY public.dast_site_tokens
+    ADD CONSTRAINT fk_rails_e84f721a8e FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.group_deploy_keys_groups
     ADD CONSTRAINT fk_rails_e87145115d FOREIGN KEY (group_id) REFERENCES public.namespaces(id) ON DELETE CASCADE;

ee/app/models/dast_site.rb

@@ -2,8 +2,20 @@
 class DastSite < ApplicationRecord
   belongs_to :project
+  belongs_to :dast_site_validation
   has_many :dast_site_profiles
   validates :url, length: { maximum: 255 }, uniqueness: { scope: :project_id }, public_url: true
   validates :project_id, presence: true
+  validate :dast_site_validation_project_id_fk
+
+  private
+
+  def dast_site_validation_project_id_fk
+    return unless dast_site_validation_id
+
+    if project_id != dast_site_validation.project.id
+      errors.add(:project_id, 'does not match dast_site_validation.project')
+    end
+  end
 end

ee/app/models/dast_site_token.rb

+# frozen_string_literal: true
+
+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
+end

ee/app/models/dast_site_validation.rb

+# frozen_string_literal: true
+
+class DastSiteValidation < ApplicationRecord
+  belongs_to :dast_site_token
+  has_many :dast_sites
+
+  validates :dast_site_token_id, presence: true
+  validates :validation_strategy, presence: true
+
+  scope :by_project_id, -> (project_id) do
+    joins(:dast_site_token).where(dast_site_tokens: { project_id: project_id })
+  end
+
+  before_create :set_url_base
+
+  enum validation_strategy: { text_file: 0 }
+
+  delegate :project, to: :dast_site_token, allow_nil: true
+
+  private
+
+  def set_url_base
+    uri = URI(dast_site_token.url)
+
+    self.url_base = "%{scheme}://%{host}:%{port}" % { scheme: uri.scheme, host: uri.host, port: uri.port }
+  end
+end

ee/app/models/ee/project.rb

@@ -81,6 +81,7 @@ module EE
       has_many :vulnerability_exports, class_name: 'Vulnerabilities::Export'
       has_many :dast_site_profiles
+      has_many :dast_site_tokens
       has_many :dast_sites
       has_many :protected_environments

ee/spec/factories/dast_site_tokens.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :dast_site_token do
+    token { SecureRandom.uuid }
+    url { FFaker::Internet.uri(:https) }
+
+    before(:create) do |dast_site_token|
+      dast_site_token.project ||= FactoryBot.create(:project)
+    end
+  end
+end

ee/spec/factories/dast_site_validations.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :dast_site_validation do
+    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
+  end
+end

ee/spec/models/dast_site_spec.rb

@@ -7,6 +7,7 @@ RSpec.describe DastSite, type: :model do
   describe 'associations' do
     it { is_expected.to belong_to(:project) }
+    it { is_expected.to belong_to(:dast_site_validation) }
     it { is_expected.to have_many(:dast_site_profiles) }
   end
@@ -16,6 +17,20 @@ RSpec.describe DastSite, type: :model do
     it { is_expected.to validate_uniqueness_of(:url).scoped_to(:project_id) }
     it { is_expected.to validate_presence_of(:project_id) }
+    context 'when the project_id and dast_site_token.project_id do not match' do
+      let(:project) { create(:project) }
+      let(:dast_site_validation) { create(:dast_site_validation) }
+
+      subject { build(:dast_site, project: project, dast_site_validation: dast_site_validation) }
+
+      it 'is not valid' do
+        aggregate_failures do
+          expect(subject.valid?).to eq(false)
+          expect(subject.errors.full_messages).to include('Project does not match dast_site_validation.project')
+        end
+      end
+    end
+
     context 'when the url is not public' do
       subject { build(:dast_site, url: 'http://127.0.0.1') }

ee/spec/models/dast_site_token_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe DastSiteToken, type: :model do
+  subject { create(:dast_site_token) }
+
+  describe 'associations' do
+    it { is_expected.to belong_to(:project) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to be_valid }
+    it { is_expected.to validate_presence_of(:project_id) }
+    it { is_expected.to validate_length_of(:token).is_at_most(255) }
+    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) }
+
+    context 'when the url is not public' do
+      subject { build(:dast_site_token, url: 'http://127.0.0.1') }
+
+      it 'is not valid' do
+        aggregate_failures do
+          expect(subject.valid?).to eq(false)
+          expect(subject.errors.full_messages).to include('Url is blocked: Requests to localhost are not allowed')
+        end
+      end
+    end
+  end
+end

ee/spec/models/dast_site_validation_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe DastSiteValidation, type: :model do
+  subject { create(:dast_site_validation) }
+
+  describe 'associations' do
+    it { is_expected.to belong_to(:dast_site_token) }
+    it { is_expected.to have_many(:dast_sites) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to be_valid }
+    it { is_expected.to validate_presence_of(:dast_site_token_id) }
+  end
+
+  describe 'before_create' do
+    it 'sets normalises 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
+
+  describe 'scopes' do
+    describe 'by_project_id' do
+      let(:another_dast_site_validation) { create(:dast_site_validation) }
+
+      it 'includes the correct records' do
+        result = described_class.by_project_id(subject.dast_site_token.project_id)
+
+        aggregate_failures do
+          expect(result).to include(subject)
+          expect(result).not_to include(another_dast_site_validation)
+        end
+      end
+    end
+  end
+
+  describe 'enums' do
+    let(:validation_strategies) do
+      { text_file: 0 }
+    end
+
+    it { is_expected.to define_enum_for(:validation_strategy).with_values(validation_strategies) }
+  end
+
+  describe '#project' do
+    it 'returns project through dast_site_token' do
+      expect(subject.project).to eq(subject.dast_site_token.project)
+    end
+  end
+end

ee/spec/models/project_spec.rb

@@ -35,6 +35,7 @@ RSpec.describe Project do
     it { is_expected.to have_many(:vulnerability_exports) }
     it { is_expected.to have_many(:vulnerability_scanners) }
     it { is_expected.to have_many(:dast_site_profiles) }
+    it { is_expected.to have_many(:dast_site_tokens) }
     it { is_expected.to have_many(:dast_sites) }
     it { is_expected.to have_many(:audit_events).dependent(false) }
     it { is_expected.to have_many(:protected_environments) }

spec/lib/gitlab/import_export/all_models.yml

@@ -482,6 +482,8 @@ project:
 - dast_site_profiles
 - dast_scanner_profiles
 - dast_sites
+- dast_site_tokens
+- dast_site_validations
 - operations_feature_flags
 - operations_feature_flags_client
 - operations_feature_flags_user_lists