diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb index ea76771b80abedb749099a2d91a4fec048866378..3e7928f984265c921ebff6894b621dc1e15ccf11 100644 --- a/app/services/ci/retry_build_service.rb +++ b/app/services/ci/retry_build_service.rb @@ -10,6 +10,10 @@ def self.clone_accessors resource_group scheduling_type].freeze end + def self.extra_accessors + [] + end + def execute(build) build.ensure_scheduling_type! diff --git a/db/migrate/20210604032738_create_dast_site_profiles_builds.rb b/db/migrate/20210604032738_create_dast_site_profiles_builds.rb new file mode 100644 index 0000000000000000000000000000000000000000..2e9eb2c7cb74b27eafa9ce34391debc7d564494d --- /dev/null +++ b/db/migrate/20210604032738_create_dast_site_profiles_builds.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CreateDastSiteProfilesBuilds < ActiveRecord::Migration[6.1] + def up + table_comment = { owner: 'group::dynamic analysis', description: 'Join table between DAST Site Profiles and CI Builds' } + + create_table :dast_site_profiles_builds, primary_key: [:dast_site_profile_id, :ci_build_id], comment: table_comment.to_json do |t| + t.bigint :dast_site_profile_id, null: false + t.bigint :ci_build_id, null: false + + t.index :ci_build_id, unique: true, name: :dast_site_profiles_builds_on_ci_build_id + end + end + + def down + drop_table :dast_site_profiles_builds + end +end diff --git a/db/migrate/20210604034158_add_ci_build_id_fk_to_dast_site_profiles_builds.rb b/db/migrate/20210604034158_add_ci_build_id_fk_to_dast_site_profiles_builds.rb new file mode 100644 index 0000000000000000000000000000000000000000..6908da69e089fcacc8d643b512964b50bf800f8e --- /dev/null +++ b/db/migrate/20210604034158_add_ci_build_id_fk_to_dast_site_profiles_builds.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddCiBuildIdFkToDastSiteProfilesBuilds < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :dast_site_profiles_builds, :ci_builds, column: :ci_build_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :dast_site_profiles_builds, column: :ci_build_id + end + end +end diff --git a/db/migrate/20210604034354_add_dast_site_profile_id_fk_to_dast_site_profiles_builds.rb b/db/migrate/20210604034354_add_dast_site_profile_id_fk_to_dast_site_profiles_builds.rb new file mode 100644 index 0000000000000000000000000000000000000000..58fe3090a4fc9009306943433a8c08aea2f6f271 --- /dev/null +++ b/db/migrate/20210604034354_add_dast_site_profile_id_fk_to_dast_site_profiles_builds.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddDastSiteProfileIdFkToDastSiteProfilesBuilds < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :dast_site_profiles_builds, :dast_site_profiles, column: :dast_site_profile_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :dast_site_profiles_builds, column: :dast_site_profile_id + end + end +end diff --git a/db/migrate/20210604051330_create_dast_scanner_profiles_builds.rb b/db/migrate/20210604051330_create_dast_scanner_profiles_builds.rb new file mode 100644 index 0000000000000000000000000000000000000000..f8a5f735f0d3a7ed566f6734cf7db1936f5da162 --- /dev/null +++ b/db/migrate/20210604051330_create_dast_scanner_profiles_builds.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CreateDastScannerProfilesBuilds < ActiveRecord::Migration[6.1] + def up + table_comment = { owner: 'group::dynamic analysis', description: 'Join table between DAST Scanner Profiles and CI Builds' } + + create_table :dast_scanner_profiles_builds, primary_key: [:dast_scanner_profile_id, :ci_build_id], comment: table_comment.to_json do |t| + t.bigint :dast_scanner_profile_id, null: false + t.bigint :ci_build_id, null: false + + t.index :ci_build_id, unique: true, name: :dast_scanner_profiles_builds_on_ci_build_id + end + end + + def down + drop_table :dast_scanner_profiles_builds + end +end diff --git a/db/migrate/20210604051742_add_ci_build_id_fk_to_dast_scanner_profiles_builds.rb b/db/migrate/20210604051742_add_ci_build_id_fk_to_dast_scanner_profiles_builds.rb new file mode 100644 index 0000000000000000000000000000000000000000..cc495c749c59e5c887cf73653fddf12f3fed561b --- /dev/null +++ b/db/migrate/20210604051742_add_ci_build_id_fk_to_dast_scanner_profiles_builds.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddCiBuildIdFkToDastScannerProfilesBuilds < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :dast_scanner_profiles_builds, :ci_builds, column: :ci_build_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :dast_scanner_profiles_builds, column: :ci_build_id + end + end +end diff --git a/db/migrate/20210604051917_add_dast_scanner_profile_id_fk_to_dast_scanner_profiles_builds.rb b/db/migrate/20210604051917_add_dast_scanner_profile_id_fk_to_dast_scanner_profiles_builds.rb new file mode 100644 index 0000000000000000000000000000000000000000..0c14c798da4ec177eb510d48b75f1520a0794832 --- /dev/null +++ b/db/migrate/20210604051917_add_dast_scanner_profile_id_fk_to_dast_scanner_profiles_builds.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddDastScannerProfileIdFkToDastScannerProfilesBuilds < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :dast_scanner_profiles_builds, :dast_scanner_profiles, column: :dast_scanner_profile_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :dast_scanner_profiles_builds, column: :dast_scanner_profile_id + end + end +end diff --git a/db/schema_migrations/20210604032738 b/db/schema_migrations/20210604032738 new file mode 100644 index 0000000000000000000000000000000000000000..1bd62357c71d313c7840917bd61c12f06595b165 --- /dev/null +++ b/db/schema_migrations/20210604032738 @@ -0,0 +1 @@ +fa373e98739d57d829273cfa9246137e2c151be67e97183c1dcdb288150aaeb5 \ No newline at end of file diff --git a/db/schema_migrations/20210604034158 b/db/schema_migrations/20210604034158 new file mode 100644 index 0000000000000000000000000000000000000000..06b047646280337553d14912302466b4596a0dfc --- /dev/null +++ b/db/schema_migrations/20210604034158 @@ -0,0 +1 @@ +c7cf4aad7637d793d1ace8fee02111bc9b0d2eea09efadb0fd616bc5c5e5550c \ No newline at end of file diff --git a/db/schema_migrations/20210604034354 b/db/schema_migrations/20210604034354 new file mode 100644 index 0000000000000000000000000000000000000000..a76242b34126a14f4300eb91f43884c0a9bbc2a1 --- /dev/null +++ b/db/schema_migrations/20210604034354 @@ -0,0 +1 @@ +da868be7c8edefc462110b5b36415870cc0c7c59dba1e3d514348011a9e70642 \ No newline at end of file diff --git a/db/schema_migrations/20210604051330 b/db/schema_migrations/20210604051330 new file mode 100644 index 0000000000000000000000000000000000000000..74140921c2ffb92926e3e3b89f8391c1b56a6160 --- /dev/null +++ b/db/schema_migrations/20210604051330 @@ -0,0 +1 @@ +2d025932dca7a407968e14872ce053461e69550098ca089d4e6ece323d240927 \ No newline at end of file diff --git a/db/schema_migrations/20210604051742 b/db/schema_migrations/20210604051742 new file mode 100644 index 0000000000000000000000000000000000000000..32ed06dafff12698e00167c2a975c6362239d352 --- /dev/null +++ b/db/schema_migrations/20210604051742 @@ -0,0 +1 @@ +7529373266b6c9b179367d5fa8775f5e2ad600008957b3a821d689aec70c7407 \ No newline at end of file diff --git a/db/schema_migrations/20210604051917 b/db/schema_migrations/20210604051917 new file mode 100644 index 0000000000000000000000000000000000000000..0034d9988e7ee5d12e0309aa750cb8daa5281123 --- /dev/null +++ b/db/schema_migrations/20210604051917 @@ -0,0 +1 @@ +3818094a4470ff7d0c105c000655dac4205e8265f78df638df0e2ef3dc6deaf3 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index eee0b920a468c6b9b218c5eaf9ac9fc2bd73342f..d1809c5ad28fd9207d5c267c3a1c2a8d8ff4d918 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -12047,6 +12047,13 @@ CREATE TABLE dast_scanner_profiles ( CONSTRAINT check_568568fabf CHECK ((char_length(name) <= 255)) ); +CREATE TABLE dast_scanner_profiles_builds ( + dast_scanner_profile_id bigint NOT NULL, + ci_build_id bigint NOT NULL +); + +COMMENT ON TABLE dast_scanner_profiles_builds IS '{"owner":"group::dynamic analysis","description":"Join table between DAST Scanner Profiles and CI Builds"}'; + CREATE SEQUENCE dast_scanner_profiles_id_seq START WITH 1 INCREMENT BY 1 @@ -12102,6 +12109,13 @@ CREATE TABLE dast_site_profiles ( CONSTRAINT check_f22f18002a CHECK ((char_length(auth_username) <= 255)) ); +CREATE TABLE dast_site_profiles_builds ( + dast_site_profile_id bigint NOT NULL, + ci_build_id bigint NOT NULL +); + +COMMENT ON TABLE dast_site_profiles_builds IS '{"owner":"group::dynamic analysis","description":"Join table between DAST Site Profiles and CI Builds"}'; + CREATE SEQUENCE dast_site_profiles_id_seq START WITH 1 INCREMENT BY 1 @@ -21092,12 +21106,18 @@ ALTER TABLE ONLY dast_profiles_pipelines ALTER TABLE ONLY dast_profiles ADD CONSTRAINT dast_profiles_pkey PRIMARY KEY (id); +ALTER TABLE ONLY dast_scanner_profiles_builds + ADD CONSTRAINT dast_scanner_profiles_builds_pkey PRIMARY KEY (dast_scanner_profile_id, ci_build_id); + ALTER TABLE ONLY dast_scanner_profiles ADD CONSTRAINT dast_scanner_profiles_pkey PRIMARY KEY (id); ALTER TABLE ONLY dast_site_profile_secret_variables ADD CONSTRAINT dast_site_profile_secret_variables_pkey PRIMARY KEY (id); +ALTER TABLE ONLY dast_site_profiles_builds + ADD CONSTRAINT dast_site_profiles_builds_pkey PRIMARY KEY (dast_site_profile_id, ci_build_id); + ALTER TABLE ONLY dast_site_profiles_pipelines ADD CONSTRAINT dast_site_profiles_pipelines_pkey PRIMARY KEY (dast_site_profile_id, ci_pipeline_id); @@ -22343,6 +22363,10 @@ CREATE INDEX commit_id_and_note_id_index ON commit_user_mentions USING btree (co CREATE INDEX composer_cache_files_index_on_deleted_at ON packages_composer_cache_files USING btree (delete_at, id); +CREATE UNIQUE INDEX dast_scanner_profiles_builds_on_ci_build_id ON dast_scanner_profiles_builds USING btree (ci_build_id); + +CREATE UNIQUE INDEX dast_site_profiles_builds_on_ci_build_id ON dast_site_profiles_builds USING btree (ci_build_id); + CREATE UNIQUE INDEX design_management_designs_versions_uniqueness ON design_management_designs_versions USING btree (design_id, version_id); CREATE INDEX design_user_mentions_on_design_id_and_note_id_index ON design_user_mentions USING btree (design_id, note_id); @@ -25705,6 +25729,9 @@ ALTER TABLE ONLY vulnerability_feedback ALTER TABLE ONLY deploy_keys_projects ADD CONSTRAINT fk_58a901ca7e FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY dast_scanner_profiles_builds + ADD CONSTRAINT fk_5d46286ad3 FOREIGN KEY (dast_scanner_profile_id) REFERENCES dast_scanner_profiles(id) ON DELETE CASCADE; + ALTER TABLE ONLY issue_assignees ADD CONSTRAINT fk_5e0c8d9154 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; @@ -25864,6 +25891,9 @@ ALTER TABLE ONLY ci_pipeline_schedules ALTER TABLE ONLY todos ADD CONSTRAINT fk_91d1f47b13 FOREIGN KEY (note_id) REFERENCES notes(id) ON DELETE CASCADE; +ALTER TABLE ONLY dast_site_profiles_builds + ADD CONSTRAINT fk_94e80df60e FOREIGN KEY (dast_site_profile_id) REFERENCES dast_site_profiles(id) ON DELETE CASCADE; + ALTER TABLE ONLY vulnerability_feedback ADD CONSTRAINT fk_94f7c8a81e FOREIGN KEY (comment_author_id) REFERENCES users(id) ON DELETE SET NULL; @@ -25927,6 +25957,9 @@ ALTER TABLE ONLY ci_builds ALTER TABLE ONLY ci_pipelines ADD CONSTRAINT fk_a23be95014 FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; +ALTER TABLE ONLY dast_site_profiles_builds + ADD CONSTRAINT fk_a325505e99 FOREIGN KEY (ci_build_id) REFERENCES ci_builds(id) ON DELETE CASCADE; + ALTER TABLE ONLY bulk_import_entities ADD CONSTRAINT fk_a44ff95be5 FOREIGN KEY (parent_id) REFERENCES bulk_import_entities(id) ON DELETE CASCADE; @@ -26137,6 +26170,9 @@ ALTER TABLE ONLY gitlab_subscriptions ALTER TABLE ONLY ci_triggers ADD CONSTRAINT fk_e3e63f966e FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY dast_scanner_profiles_builds + ADD CONSTRAINT fk_e4c49200f8 FOREIGN KEY (ci_build_id) REFERENCES ci_builds(id) ON DELETE CASCADE; + ALTER TABLE ONLY merge_requests ADD CONSTRAINT fk_e719a85f8a FOREIGN KEY (author_id) REFERENCES users(id) ON DELETE SET NULL; diff --git a/ee/app/models/concerns/app_sec/dast/buildable.rb b/ee/app/models/concerns/app_sec/dast/buildable.rb new file mode 100644 index 0000000000000000000000000000000000000000..5d422207f37debaa464699cd66a65ac5ed69c35b --- /dev/null +++ b/ee/app/models/concerns/app_sec/dast/buildable.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module AppSec + module Dast + module Buildable + extend ::ActiveSupport::Concern + + included do + extend SuppressCompositePrimaryKeyWarning + + validate :project_ids_match + end + + def project_ids_match + return if ci_build.nil? || profile.nil? + + unless ci_build.project_id == profile.project_id + errors.add(:ci_build_id, "project_id must match #{profile.class.underscore}.project_id") + end + end + end + end +end diff --git a/ee/app/models/dast/scanner_profiles_build.rb b/ee/app/models/dast/scanner_profiles_build.rb new file mode 100644 index 0000000000000000000000000000000000000000..ede2e3850a40a1930c3696752f60b51edf34c7fe --- /dev/null +++ b/ee/app/models/dast/scanner_profiles_build.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Dast + class ScannerProfilesBuild < ApplicationRecord + include AppSec::Dast::Buildable + + self.table_name = 'dast_scanner_profiles_builds' + + belongs_to :ci_build, class_name: 'Ci::Build', optional: false, inverse_of: :dast_scanner_profiles_build + belongs_to :dast_scanner_profile, class_name: 'DastScannerProfile', optional: false, inverse_of: :dast_scanner_profiles_builds + + validates :ci_build_id, :dast_scanner_profile_id, presence: true + + alias_attribute :profile, :dast_scanner_profile + end +end diff --git a/ee/app/models/dast/site_profiles_build.rb b/ee/app/models/dast/site_profiles_build.rb new file mode 100644 index 0000000000000000000000000000000000000000..5747bdb3749beca178bf74b08d69d505ab99832a --- /dev/null +++ b/ee/app/models/dast/site_profiles_build.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Dast + class SiteProfilesBuild < ApplicationRecord + include AppSec::Dast::Buildable + + self.table_name = 'dast_site_profiles_builds' + + belongs_to :ci_build, class_name: 'Ci::Build', optional: false, inverse_of: :dast_site_profiles_build + belongs_to :dast_site_profile, class_name: 'DastSiteProfile', optional: false, inverse_of: :dast_site_profiles_builds + + validates :ci_build_id, :dast_site_profile_id, presence: true + + alias_attribute :profile, :dast_site_profile + end +end diff --git a/ee/app/models/dast_scanner_profile.rb b/ee/app/models/dast_scanner_profile.rb index 22c4a8256d572e20d0458ead686796d66327a9da..3d599afbbe079b9a8beaded891ddab0d04d76adb 100644 --- a/ee/app/models/dast_scanner_profile.rb +++ b/ee/app/models/dast_scanner_profile.rb @@ -3,6 +3,9 @@ class DastScannerProfile < ApplicationRecord belongs_to :project + has_many :dast_scanner_profiles_builds, class_name: 'Dast::ScannerProfilesBuild', foreign_key: :dast_scanner_profile_id, inverse_of: :dast_scanner_profile + has_many :ci_builds, class_name: 'Ci::Build', through: :dast_scanner_profiles_builds + validates :project_id, presence: true validates :name, length: { maximum: 255 }, uniqueness: { scope: :project_id }, presence: true diff --git a/ee/app/models/dast_site_profile.rb b/ee/app/models/dast_site_profile.rb index d3056e1233880c4a12827c320b16a9bf84d046f4..1b061d690ddfc04c7b94839377da3bf13c549d34 100644 --- a/ee/app/models/dast_site_profile.rb +++ b/ee/app/models/dast_site_profile.rb @@ -9,6 +9,9 @@ class DastSiteProfile < ApplicationRecord has_many :dast_site_profiles_pipelines, class_name: 'Dast::SiteProfilesPipeline', foreign_key: :dast_site_profile_id, inverse_of: :dast_site_profile has_many :ci_pipelines, class_name: 'Ci::Pipeline', through: :dast_site_profiles_pipelines + has_many :dast_site_profiles_builds, class_name: 'Dast::SiteProfilesBuild', foreign_key: :dast_site_profile_id, inverse_of: :dast_site_profile + has_many :ci_builds, class_name: 'Ci::Build', through: :dast_site_profiles_builds + validates :excluded_urls, length: { maximum: 25 } validates :auth_url, addressable_url: true, length: { maximum: 1024 }, allow_nil: true validates :auth_username_field, :auth_password_field, :auth_username, length: { maximum: 255 } diff --git a/ee/app/models/ee/ci/build.rb b/ee/app/models/ee/ci/build.rb index eea46ef67bd7e840cce7edb0267a24bff2570daa..e117908b7a5a11938df4a9ab035b1e2b67a2220c 100644 --- a/ee/app/models/ee/ci/build.rb +++ b/ee/app/models/ee/ci/build.rb @@ -31,6 +31,12 @@ module Build has_many :security_scans, class_name: 'Security::Scan' + has_one :dast_site_profiles_build, class_name: 'Dast::SiteProfilesBuild', foreign_key: :ci_build_id, inverse_of: :ci_build + has_one :dast_site_profile, class_name: 'DastSiteProfile', through: :dast_site_profiles_build + + has_one :dast_scanner_profiles_build, class_name: 'Dast::ScannerProfilesBuild', foreign_key: :ci_build_id, inverse_of: :ci_build + has_one :dast_scanner_profile, class_name: 'DastScannerProfile', through: :dast_scanner_profiles_build + after_commit :track_ci_secrets_management_usage, on: :create delegate :service_specification, to: :runner_session, allow_nil: true @@ -47,12 +53,8 @@ module Build def variables strong_memoize(:variables) do super.tap do |collection| - if pipeline.triggered_for_ondemand_dast_scan? - # Subject to change. Please see gitlab-org/gitlab#330950 for more info. - profile = pipeline.dast_profile || pipeline.dast_site_profile - - collection.concat(profile.secret_ci_variables(pipeline.user)) - end + collection.concat(dast_on_demand_variables) + collection.concat(dast_configuration_variables) end end end @@ -169,6 +171,33 @@ def variables_hash end end + def dast_on_demand_variables + ::Gitlab::Ci::Variables::Collection.new.tap do |collection| + break collection unless pipeline.triggered_for_ondemand_dast_scan? + + # Subject to change. Please see gitlab-org/gitlab#330950 for more info. + profile = pipeline.dast_profile || pipeline.dast_site_profile + + collection.concat(profile.secret_ci_variables(pipeline.user)) + end + end + + def dast_configuration_variables + ::Gitlab::Ci::Variables::Collection.new.tap do |collection| + break collection unless ::Feature.enabled?(:dast_configuration_ui, project) + break collection unless (dast_configuration = options[:dast_configuration]) + + if dast_configuration[:site_profile] && dast_site_profile + collection.concat(dast_site_profile.ci_variables) + collection.concat(dast_site_profile.secret_ci_variables(user)) + end + + if dast_configuration[:scanner_profile] && dast_scanner_profile + collection.concat(dast_scanner_profile.ci_variables) + end + end + end + def parse_security_artifact_blob(security_report, blob) report_clone = security_report.clone_as_blank signatures_enabled = ::Feature.enabled?(:vulnerability_finding_tracking_signatures, project) && project.licensed_feature_available?(:vulnerability_finding_signatures) diff --git a/ee/app/services/ee/ci/retry_build_service.rb b/ee/app/services/ee/ci/retry_build_service.rb index 6513e63e2868878ef1fb738981f4e2f1f21dcecb..a546964aa526ebcc7ef7afe1d2fb64b15f3815e4 100644 --- a/ee/app/services/ee/ci/retry_build_service.rb +++ b/ee/app/services/ee/ci/retry_build_service.rb @@ -11,7 +11,12 @@ module RetryBuildService override :clone_accessors def clone_accessors - (super + %i[secrets]).freeze + (super + extra_accessors).freeze + end + + override :extra_accessors + def extra_accessors + %i[dast_site_profile dast_scanner_profile secrets].freeze end end diff --git a/ee/spec/factories/dast/scanner_profiles_builds.rb b/ee/spec/factories/dast/scanner_profiles_builds.rb new file mode 100644 index 0000000000000000000000000000000000000000..f8b58539970708f372e1b1b295332686c3d17332 --- /dev/null +++ b/ee/spec/factories/dast/scanner_profiles_builds.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :dast_scanner_profiles_build, class: 'Dast::ScannerProfilesBuild' do + dast_scanner_profile + + ci_build { association :ci_build, project: dast_scanner_profile.project } + end +end diff --git a/ee/spec/factories/dast/site_profiles_builds.rb b/ee/spec/factories/dast/site_profiles_builds.rb new file mode 100644 index 0000000000000000000000000000000000000000..17a74809c76da25b5753f6cb73f7acdce323cbbe --- /dev/null +++ b/ee/spec/factories/dast/site_profiles_builds.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :dast_site_profiles_build, class: 'Dast::SiteProfilesBuild' do + dast_site_profile + + ci_build { association :ci_build, project: dast_site_profile.project } + end +end diff --git a/ee/spec/models/ci/build_spec.rb b/ee/spec/models/ci/build_spec.rb index f4a8d299271b3c7ed33eadd862e9d7f4cfb725cd..d89969f8099e650301605728ba29ea0320852368 100644 --- a/ee/spec/models/ci/build_spec.rb +++ b/ee/spec/models/ci/build_spec.rb @@ -129,59 +129,182 @@ end end - context 'when there is a dast_profile associated with the pipeline' do + context 'dast' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user, developer_projects: [project]) } - let_it_be(:dast_profile) { create(:dast_profile, project: project) } - let_it_be(:dast_site_profile_secret_variable) { create(:dast_site_profile_secret_variable, key: 'DAST_PASSWORD_BASE64', dast_site_profile: dast_profile.dast_site_profile) } - - let(:pipeline) { create(:ci_pipeline, pipeline_params.merge!(project: project, dast_profile: dast_profile, user: user) ) } - - let(:key) { dast_site_profile_secret_variable.key } - let(:value) { dast_site_profile_secret_variable.value } + let_it_be(:dast_site_profile) { create(:dast_site_profile, project: project) } + let_it_be(:dast_scanner_profile) { create(:dast_scanner_profile, project: project) } + let_it_be(:dast_profile) { create(:dast_profile, project: project, dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile) } + let_it_be(:dast_site_profile_secret_variable) { create(:dast_site_profile_secret_variable, key: 'DAST_PASSWORD_BASE64', dast_site_profile: dast_site_profile) } + let_it_be(:options) { { dast_configuration: { site_profile: dast_site_profile.name, scanner_profile: dast_scanner_profile.name } } } before do stub_licensed_features(security_on_demand_scans: true) end - shared_examples 'a pipeline with no dast on-demand variables' do - it 'does not include variables associated with the profile' do - keys = subject.to_runner_variables.map { |var| var[:key] } - - expect(keys).not_to include(key) + shared_examples 'it includes variables' do + it 'includes variables from the profile' do + expect(subject.to_runner_variables).to include(*expected_variables.to_runner_variables) end end - it_behaves_like 'a pipeline with no dast on-demand variables' do - let(:pipeline_params) { { config_source: :parameter_source } } + shared_examples 'it excludes variables' do + it 'excludes variables from the profile' do + expect(subject.to_runner_variables).not_to include(*expected_variables.to_runner_variables) + end end - it_behaves_like 'a pipeline with no dast on-demand variables' do - let(:pipeline_params) { { source: :ondemand_dast_scan } } + context 'when there is a dast_site_profile associated with the job' do + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:job) { create(:ci_build, :running, pipeline: pipeline, dast_site_profile: dast_site_profile, user: user, options: options) } + + context 'when feature is enabled' do + it_behaves_like 'it includes variables' do + let(:expected_variables) { dast_site_profile.ci_variables } + end + + context 'when user has permission' do + it_behaves_like 'it includes variables' do + let(:expected_variables) { dast_site_profile.secret_ci_variables(user) } + end + end + + context 'when user does not have permission' do + let_it_be(:user) { create(:user) } + + before do + project.add_guest(user) + end + + it_behaves_like 'it excludes variables' do + let(:expected_variables) { dast_site_profile.secret_ci_variables(user) } + end + end + end + + context 'when feature is disabled' do + before do + stub_feature_flags(dast_configuration_ui: false) + end + + it_behaves_like 'it excludes variables' do + let(:expected_variables) { dast_site_profile.ci_variables.concat(dast_site_profile.secret_ci_variables(user)) } + end + end end - context 'when the dast on-demand pipeline is correctly configured' do - let(:pipeline_params) { { source: :ondemand_dast_scan, config_source: :parameter_source } } + context 'when there is a dast_scanner_profile associated with the job' do + let(:pipeline) { create(:ci_pipeline, project: project, user: user) } + let(:job) { create(:ci_build, :running, pipeline: pipeline, dast_scanner_profile: dast_scanner_profile, options: options) } - it 'includes variables associated with the profile' do - expect(subject.to_runner_variables).to include(key: key, value: value, public: false, masked: true) + context 'when feature is enabled' do + it_behaves_like 'it includes variables' do + let(:expected_variables) { dast_scanner_profile.ci_variables } + end end - context 'when user cannot read secrets' do + context 'when feature is disabled' do before do - stub_licensed_features(security_on_demand_scans: false) + stub_feature_flags(dast_configuration_ui: false) end - it 'does not include variables associated with the profile' do - expect(subject.to_runner_variables).not_to include(key: key, value: value, public: false, masked: true) + it_behaves_like 'it excludes variables' do + let(:expected_variables) { dast_scanner_profile.ci_variables } end end + end + + context 'when there are profiles associated with the job' do + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:job) { create(:ci_build, :running, pipeline: pipeline, dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile, user: user, options: options) } - context 'when there is no user associated with the pipeline' do - let_it_be(:user) { nil } + context 'when dast_configuration is absent from the options' do + let(:options) { {} } + it 'does not attempt look up any dast profiles', :aggregate_failures do + expect(job).not_to receive(:dast_site_profile) + expect(job).not_to receive(:dast_scanner_profile) + + subject + end + end + + context 'when site_profile is absent from the dast_configuration' do + let(:options) { { dast_configuration: { scanner_profile: dast_scanner_profile.name } } } + + it 'does not attempt look up the site profile' do + expect(job).not_to receive(:dast_site_profile) + + subject + end + end + + context 'when scanner_profile is absent from the dast_configuration' do + let(:options) { { dast_configuration: { site_profile: dast_site_profile.name } } } + + it 'does not attempt look up the scanner profile' do + expect(job).not_to receive(:dast_scanner_profile) + + subject + end + end + + context 'when both profiles are present in the dast_configuration' do + it 'attempts look up dast profiles', :aggregate_failures do + expect(job).to receive(:dast_site_profile).and_call_original.at_least(:once) + expect(job).to receive(:dast_scanner_profile).and_call_original.at_least(:once) + + subject + end + end + end + + context 'when there is a dast_profile associated with the pipeline' do + let(:pipeline) { create(:ci_pipeline, pipeline_params.merge!(project: project, dast_profile: dast_profile, user: user) ) } + let(:key) { dast_site_profile_secret_variable.key } + let(:value) { dast_site_profile_secret_variable.value } + + shared_examples 'a record with no associated dast variables' do it 'does not include variables associated with the profile' do - expect(subject.to_runner_variables).not_to include(key: key, value: value, public: false, masked: true) + keys = subject.to_runner_variables.map { |var| var[:key] } + + expect(keys).not_to include(key) + end + end + + context 'when the on-demand pipeline is incorrectly configured' do + it_behaves_like 'a record with no associated dast variables' do + let(:pipeline_params) { { config_source: :parameter_source } } + end + + it_behaves_like 'a record with no associated dast variables' do + let(:pipeline_params) { { source: :ondemand_dast_scan } } + end + end + + context 'when the dast on-demand pipeline is correctly configured' do + let(:pipeline_params) { { source: :ondemand_dast_scan, config_source: :parameter_source } } + + it 'includes variables associated with the profile' do + expect(subject.to_runner_variables).to include(key: key, value: value, public: false, masked: true) + end + + context 'when user cannot read secrets' do + before do + stub_licensed_features(security_on_demand_scans: false) + end + + it 'does not include variables associated with the profile' do + expect(subject.to_runner_variables).not_to include(key: key, value: value, public: false, masked: true) + end + end + + context 'when there is no user associated with the pipeline' do + let_it_be(:user) { nil } + + it 'does not include variables associated with the profile' do + expect(subject.to_runner_variables).not_to include(key: key, value: value, public: false, masked: true) + end end end end diff --git a/ee/spec/models/dast/scanner_profiles_build_spec.rb b/ee/spec/models/dast/scanner_profiles_build_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..c3da11b87ab2870530b17e1f3ee58a5a737fb537 --- /dev/null +++ b/ee/spec/models/dast/scanner_profiles_build_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Dast::ScannerProfilesBuild, type: :model do + subject { create(:dast_scanner_profiles_build) } + + describe 'associations' do + it { is_expected.to belong_to(:ci_build).class_name('Ci::Build').inverse_of(:dast_scanner_profiles_build).required } + it { is_expected.to belong_to(:dast_scanner_profile).class_name('DastScannerProfile').inverse_of(:dast_scanner_profiles_builds).required } + end + + describe 'validations' do + it { is_expected.to be_valid } + it { is_expected.to validate_presence_of(:ci_build_id) } + it { is_expected.to validate_presence_of(:dast_scanner_profile_id) } + + context 'when the ci_build.project_id and dast_scanner_profile.project_id do not match' do + let(:ci_build) { build(:ci_build, project_id: 1) } + let(:scanner_profile) { build(:dast_scanner_profile, project_id: 2) } + + subject { build(:dast_scanner_profiles_build, ci_build: ci_build, dast_scanner_profile: scanner_profile) } + + it 'is not valid', :aggregate_failures do + expect(subject).not_to be_valid + expect(subject.errors.full_messages).to include('Ci build project_id must match dast_scanner_profile.project_id') + end + end + end +end diff --git a/ee/spec/models/dast/site_profiles_build_spec.rb b/ee/spec/models/dast/site_profiles_build_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..110a41b83a1c8ab48d04dfc129f74398b2998152 --- /dev/null +++ b/ee/spec/models/dast/site_profiles_build_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Dast::SiteProfilesBuild, type: :model do + subject { create(:dast_site_profiles_build) } + + describe 'associations' do + it { is_expected.to belong_to(:ci_build).class_name('Ci::Build').inverse_of(:dast_site_profiles_build).required } + it { is_expected.to belong_to(:dast_site_profile).class_name('DastSiteProfile').inverse_of(:dast_site_profiles_builds).required } + end + + describe 'validations' do + it { is_expected.to be_valid } + it { is_expected.to validate_presence_of(:ci_build_id) } + it { is_expected.to validate_presence_of(:dast_site_profile_id) } + + context 'when the ci_build.project_id and dast_site_profile.project_id do not match' do + let(:ci_build) { build(:ci_build, project_id: 1) } + let(:site_profile) { build(:dast_site_profile, project_id: 2) } + + subject { build(:dast_site_profiles_build, ci_build: ci_build, dast_site_profile: site_profile) } + + it 'is not valid', :aggregate_failures do + expect(subject).not_to be_valid + expect(subject.errors.full_messages).to include('Ci build project_id must match dast_site_profile.project_id') + end + end + end +end diff --git a/ee/spec/models/dast_scanner_profile_spec.rb b/ee/spec/models/dast_scanner_profile_spec.rb index d2dbcb0862eb624adfc184065f9053e37ec92850..5563859c3ecc9a2147cc311a4509d3e1d8911214 100644 --- a/ee/spec/models/dast_scanner_profile_spec.rb +++ b/ee/spec/models/dast_scanner_profile_spec.rb @@ -7,6 +7,8 @@ describe 'associations' do it { is_expected.to belong_to(:project) } + it { is_expected.to have_many(:dast_scanner_profiles_builds).class_name('Dast::ScannerProfilesBuild').with_foreign_key(:dast_scanner_profile_id).inverse_of(:dast_scanner_profile) } + it { is_expected.to have_many(:ci_builds).class_name('Ci::Build').through(:dast_scanner_profiles_builds) } end describe 'validations' do diff --git a/ee/spec/models/dast_site_profile_spec.rb b/ee/spec/models/dast_site_profile_spec.rb index d4176bbc3dbb2869022ae7edb2eae566b2e431e2..da4b98485dd78693def5e88e3ef8bbb398ce8c8b 100644 --- a/ee/spec/models/dast_site_profile_spec.rb +++ b/ee/spec/models/dast_site_profile_spec.rb @@ -11,6 +11,8 @@ it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:dast_site) } it { is_expected.to have_many(:secret_variables).class_name('Dast::SiteProfileSecretVariable') } + it { is_expected.to have_many(:dast_site_profiles_builds).class_name('Dast::SiteProfilesBuild').with_foreign_key(:dast_site_profile_id).inverse_of(:dast_site_profile) } + it { is_expected.to have_many(:ci_builds).class_name('Ci::Build').through(:dast_site_profiles_builds) } end describe 'validations' do diff --git a/ee/spec/services/ci/retry_build_service_spec.rb b/ee/spec/services/ci/retry_build_service_spec.rb index 8fec4066c70566094962e1b8588e2e8427bd4c6e..a79c27f78141f68612bc6fc0067a77f1aec57dde 100644 --- a/ee/spec/services/ci/retry_build_service_spec.rb +++ b/ee/spec/services/ci/retry_build_service_spec.rb @@ -28,6 +28,20 @@ project.add_developer(user) end + context 'dast' do + let(:dast_site_profile) { create(:dast_site_profile, project: project) } + let(:dast_scanner_profile) { create(:dast_scanner_profile, project: project) } + + before do + build.update!(dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile) + end + + it 'clones the profile associations', :aggregate_failures do + expect(new_build.dast_site_profile).to eq(dast_site_profile) + expect(new_build.dast_scanner_profile).to eq(dast_scanner_profile) + end + end + context 'when build has secrets' do let(:secrets) do { diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index c71bec31984de1d2d8f9aa353e40d873f0895237..87893f11ed542e4603f61389f27b27e1e85f7f3b 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -30,7 +30,7 @@ project.add_reporter(reporter) end - clone_accessors = described_class.clone_accessors + clone_accessors = described_class.clone_accessors.without(described_class.extra_accessors) reject_accessors = %i[id status user token token_encrypted coverage trace runner @@ -98,7 +98,7 @@ end clone_accessors.each do |attribute| - it "clones #{attribute} build attribute" do + it "clones #{attribute} build attribute", :aggregate_failures do expect(attribute).not_to be_in(forbidden_associations), "association #{attribute} must be `belongs_to`" expect(build.send(attribute)).not_to be_nil expect(new_build.send(attribute)).not_to be_nil @@ -134,7 +134,7 @@ end end - it 'has correct number of known attributes' do + it 'has correct number of known attributes', :aggregate_failures do processed_accessors = clone_accessors + reject_accessors known_accessors = processed_accessors + ignore_accessors @@ -146,9 +146,10 @@ Ci::Build.attribute_names.map(&:to_sym) + Ci::Build.attribute_aliases.keys.map(&:to_sym) + Ci::Build.reflect_on_all_associations.map(&:name) + - [:tag_list, :needs_attributes] - - current_accessors << :secrets if Gitlab.ee? + [:tag_list, :needs_attributes] - + # ee-specific accessors should be tested in ee/spec/services/ci/retry_build_service_spec.rb instead + described_class.extra_accessors - + [:dast_site_profiles_build, :dast_scanner_profiles_build] # join tables current_accessors.uniq!