Commit 50244ebd authored by Philip Cunningham's avatar Philip Cunningham Committed by Tiger Watson

Support DAST secret delivery using DastSiteProfile

parent f78ea95c
---
title: Create database table dast_site_profiles_pipelines
merge_request: 60090
author:
type: added
# frozen_string_literal: true
class CreateDastSiteProfilesPipelines < ActiveRecord::Migration[6.0]
def up
table_comment = { owner: 'group::dynamic analysis', description: 'Join table between DAST Site Profiles and CI Pipelines' }
create_table :dast_site_profiles_pipelines, primary_key: [:dast_site_profile_id, :ci_pipeline_id], comment: table_comment.to_json do |t|
t.bigint :dast_site_profile_id, null: false
t.bigint :ci_pipeline_id, null: false
t.index :ci_pipeline_id, unique: true, name: :index_dast_site_profiles_pipelines_on_ci_pipeline_id
end
end
def down
drop_table :dast_site_profiles_pipelines
end
end
# frozen_string_literal: true
class AddDastSiteProfileIdFkToDastSiteProfilesPipelines < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_concurrent_foreign_key :dast_site_profiles_pipelines, :dast_site_profiles, column: :dast_site_profile_id, on_delete: :cascade
end
def down
with_lock_retries do
remove_foreign_key :dast_site_profiles_pipelines, column: :dast_site_profile_id
end
end
end
# frozen_string_literal: true
class AddCiPipelineIdFkToDastSiteProfilesPipelines < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_concurrent_foreign_key :dast_site_profiles_pipelines, :ci_pipelines, column: :ci_pipeline_id, on_delete: :cascade
end
def down
with_lock_retries do
remove_foreign_key :dast_site_profiles_pipelines, column: :ci_pipeline_id
end
end
end
84f7b631c9017b286665beca42fb30e064c852d5a21c2f82a8bee6f0d5e62c25
\ No newline at end of file
0ea5c328f9d15d73744f8847c4b1071e2a360cd52ce0da1216ca6acc768050e5
\ No newline at end of file
013106237f73a94606f962f54c740af23deac637c8e075471ba03ef5d6c1b953
\ No newline at end of file
......@@ -12050,6 +12050,13 @@ CREATE SEQUENCE dast_site_profiles_id_seq
ALTER SEQUENCE dast_site_profiles_id_seq OWNED BY dast_site_profiles.id;
CREATE TABLE dast_site_profiles_pipelines (
dast_site_profile_id bigint NOT NULL,
ci_pipeline_id bigint NOT NULL
);
COMMENT ON TABLE dast_site_profiles_pipelines IS '{"owner":"group::dynamic analysis","description":"Join table between DAST Site Profiles and CI Pipelines"}';
CREATE TABLE dast_site_tokens (
id bigint NOT NULL,
project_id bigint NOT NULL,
......@@ -20878,6 +20885,9 @@ ALTER TABLE ONLY dast_scanner_profiles
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_pipelines
ADD CONSTRAINT dast_site_profiles_pipelines_pkey PRIMARY KEY (dast_site_profile_id, ci_pipeline_id);
ALTER TABLE ONLY dast_site_profiles
ADD CONSTRAINT dast_site_profiles_pkey PRIMARY KEY (id);
......@@ -22865,6 +22875,8 @@ CREATE INDEX index_dast_site_profiles_on_dast_site_id ON dast_site_profiles USIN
CREATE UNIQUE INDEX index_dast_site_profiles_on_project_id_and_name ON dast_site_profiles USING btree (project_id, name);
CREATE UNIQUE INDEX index_dast_site_profiles_pipelines_on_ci_pipeline_id ON dast_site_profiles_pipelines USING btree (ci_pipeline_id);
CREATE INDEX index_dast_site_tokens_on_project_id ON dast_site_tokens USING btree (project_id);
CREATE INDEX index_dast_site_validations_on_dast_site_token_id ON dast_site_validations USING btree (dast_site_token_id);
......@@ -25362,6 +25374,9 @@ ALTER TABLE ONLY alert_management_alerts
ALTER TABLE ONLY path_locks
ADD CONSTRAINT fk_5265c98f24 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
ALTER TABLE ONLY dast_site_profiles_pipelines
ADD CONSTRAINT fk_53849b0ad5 FOREIGN KEY (ci_pipeline_id) REFERENCES ci_pipelines(id) ON DELETE CASCADE;
ALTER TABLE ONLY clusters_applications_prometheus
ADD CONSTRAINT fk_557e773639 FOREIGN KEY (cluster_id) REFERENCES clusters(id) ON DELETE CASCADE;
......@@ -25719,6 +25734,9 @@ ALTER TABLE ONLY experiment_subjects
ALTER TABLE ONLY todos
ADD CONSTRAINT fk_ccf0373936 FOREIGN KEY (author_id) REFERENCES users(id) ON DELETE CASCADE;
ALTER TABLE ONLY dast_site_profiles_pipelines
ADD CONSTRAINT fk_cf05cf8fe1 FOREIGN KEY (dast_site_profile_id) REFERENCES dast_site_profiles(id) ON DELETE CASCADE;
ALTER TABLE ONLY geo_event_log
ADD CONSTRAINT fk_cff7185ad2 FOREIGN KEY (reset_checksum_event_id) REFERENCES geo_reset_checksum_events(id) ON DELETE CASCADE;
# frozen_string_literal: true
module Dast
class SiteProfilesPipeline < ApplicationRecord
extend SuppressCompositePrimaryKeyWarning
self.table_name = 'dast_site_profiles_pipelines'
belongs_to :ci_pipeline, class_name: 'Ci::Pipeline', optional: false, inverse_of: :dast_site_profiles_pipeline
belongs_to :dast_site_profile, class_name: 'DastSiteProfile', optional: false, inverse_of: :dast_site_profiles_pipelines
validates :ci_pipeline_id, :dast_site_profile_id, presence: true
validate :project_ids_match
private
def project_ids_match
return if ci_pipeline.nil? || dast_site_profile.nil?
unless ci_pipeline.project_id == dast_site_profile.project_id
errors.add(:ci_pipeline_id, 'project_id must match dast_site_profile.project_id')
end
end
end
end
......@@ -6,6 +6,9 @@ class DastSiteProfile < ApplicationRecord
has_many :secret_variables, class_name: 'Dast::SiteProfileSecretVariable'
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
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 }
......@@ -25,6 +28,10 @@ class DastSiteProfile < ApplicationRecord
delegate :dast_site_validation, to: :dast_site, allow_nil: true
def ci_variables
::Gitlab::Ci::Variables::Collection.new(secret_variables)
end
def status
return DastSiteValidation::NONE_STATE unless dast_site_validation
......
......@@ -48,8 +48,11 @@ module EE
def variables
strong_memoize(:variables) do
super.tap do |collection|
if pipeline.triggered_for_ondemand_dast_scan? && pipeline.dast_profile
collection.concat(pipeline.dast_profile.ci_variables)
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.ci_variables)
end
end
end
......
......@@ -22,6 +22,11 @@ module EE
has_one :dast_profiles_pipeline, class_name: 'Dast::ProfilesPipeline', foreign_key: :ci_pipeline_id, inverse_of: :ci_pipeline
has_one :dast_profile, class_name: 'Dast::Profile', through: :dast_profiles_pipeline
# Temporary location to be moved in the future. Please see gitlab-org/gitlab#330950 for more info.
has_one :dast_site_profiles_pipeline, class_name: 'Dast::SiteProfilesPipeline', foreign_key: :ci_pipeline_id, inverse_of: :ci_pipeline
has_one :dast_site_profile, class_name: 'DastSiteProfile', through: :dast_site_profiles_pipeline
has_one :source_project, class_name: 'Ci::Sources::Project', foreign_key: :pipeline_id
# Legacy way to fetch security reports based on job name. This has been replaced by the reports feature.
......
......@@ -2,13 +2,18 @@
module Ci
class RunDastScanService < BaseService
def execute(branch:, dast_profile: nil, **args)
def execute(branch:, dast_profile: nil, dast_site_profile: nil, **args)
return ServiceResponse.error(message: 'Insufficient permissions') unless allowed?
service = Ci::CreatePipelineService.new(project, current_user, ref: branch)
pipeline = service.execute(:ondemand_dast_scan, content: ci_yaml(args)) do |pipeline|
pipeline.dast_profile = dast_profile
if dast_profile
pipeline.dast_profile = dast_profile
else
# Subject to change. Please see gitlab-org/gitlab#330950 for more info.
pipeline.dast_site_profile = dast_site_profile
end
end
if pipeline.created_successfully?
......
......@@ -62,7 +62,7 @@ module DastOnDemandScans
end
def default_config
{ dast_profile: dast_profile, branch: branch }
{ dast_profile: dast_profile, dast_site_profile: dast_site_profile, branch: branch }
end
def target_config
......
# frozen_string_literal: true
FactoryBot.define do
factory :dast_site_profiles_pipeline, class: 'Dast::SiteProfilesPipeline' do
dast_site_profile
ci_pipeline { association :ci_pipeline, project: dast_site_profile.project }
end
end
......@@ -8,6 +8,8 @@ RSpec.describe Mutations::Dast::Profiles::Update do
let_it_be(:project) { create(:project, :repository) }
let_it_be(:user) { create(:user) }
let_it_be(:dast_profile, reload: true) { create(:dast_profile, project: project) }
let_it_be(:new_dast_site_profile) { create(:dast_site_profile, project: project) }
let_it_be(:new_dast_scanner_profile) { create(:dast_scanner_profile, project: project) }
let(:dast_profile_gid) { dast_profile.to_global_id }
let(:run_after_update) { false }
......@@ -18,8 +20,8 @@ RSpec.describe Mutations::Dast::Profiles::Update do
name: SecureRandom.hex,
description: SecureRandom.hex,
branch_name: project.default_branch,
dast_site_profile_id: global_id_of(create(:dast_site_profile, project: project)),
dast_scanner_profile_id: global_id_of(create(:dast_scanner_profile, project: project)),
dast_site_profile_id: global_id_of(new_dast_site_profile),
dast_scanner_profile_id: global_id_of(new_dast_scanner_profile),
run_after_update: run_after_update
}
end
......
......@@ -20,6 +20,8 @@ RSpec.describe Ci::Pipeline do
it { is_expected.to have_many(:vulnerabilities_finding_pipelines).class_name('Vulnerabilities::FindingPipeline') }
it { is_expected.to have_one(:dast_profiles_pipeline).class_name('Dast::ProfilesPipeline').with_foreign_key(:ci_pipeline_id).inverse_of(:ci_pipeline) }
it { is_expected.to have_one(:dast_profile).class_name('Dast::Profile').through(:dast_profiles_pipeline) }
it { is_expected.to have_one(:dast_site_profiles_pipeline).class_name('Dast::SiteProfilesPipeline').with_foreign_key(:ci_pipeline_id).inverse_of(:ci_pipeline) }
it { is_expected.to have_one(:dast_site_profile).class_name('DastSiteProfile').through(:dast_site_profiles_pipeline) }
end
describe '.failure_reasons' do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Dast::SiteProfilesPipeline, type: :model do
subject { create(:dast_site_profiles_pipeline) }
describe 'associations' do
it { is_expected.to belong_to(:ci_pipeline).class_name('Ci::Pipeline').inverse_of(:dast_site_profiles_pipeline).required }
it { is_expected.to belong_to(:dast_site_profile).class_name('DastSiteProfile').inverse_of(:dast_site_profiles_pipelines).required }
end
describe 'validations' do
it { is_expected.to be_valid }
it { is_expected.to validate_presence_of(:ci_pipeline_id) }
it { is_expected.to validate_presence_of(:dast_site_profile_id) }
context 'when the ci_pipeline.project_id and dast_site_profile.project_id do not match' do
let(:pipeline) { build(:ci_pipeline, project_id: 1) }
let(:site_profile) { build(:dast_site_profile, project_id: 2) }
subject { build(:dast_site_profiles_pipeline, ci_pipeline: pipeline, 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 pipeline project_id must match dast_site_profile.project_id')
end
end
end
end
......@@ -193,5 +193,21 @@ RSpec.describe DastSiteProfile, type: :model do
end
end
end
describe '#ci_variables' do
context 'when there are no secret_variables' do
it 'returns an empty collection' do
expect(subject.ci_variables.size).to be_zero
end
end
context 'when there are secret_variables' do
it 'returns a collection containing that variable' do
variable = create(:dast_site_profile_secret_variable, dast_site_profile: subject)
expect(subject.ci_variables.to_runner_variables).to include(key: variable.key, value: variable.value, public: false, masked: true)
end
end
end
end
end
......@@ -6,6 +6,7 @@ RSpec.describe Ci::RunDastScanService do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :repository, creator: user) }
let_it_be(:dast_profile) { create(:dast_profile) }
let_it_be(:dast_site_profile) { dast_profile.dast_site_profile }
let_it_be(:branch) { project.default_branch }
let_it_be(:spider_timeout) { 42 }
let_it_be(:target_timeout) { 21 }
......@@ -35,7 +36,8 @@ RSpec.describe Ci::RunDastScanService do
auth_username_field: 'session[username]',
auth_password_field: 'session[password]',
auth_username: 'tanuki',
dast_profile: dast_profile
dast_profile: dast_profile,
dast_site_profile: dast_site_profile
)
end
......@@ -175,23 +177,37 @@ RSpec.describe Ci::RunDastScanService do
expect(build.yaml_variables).to contain_exactly(*expected_variables)
end
it 'associates the dast_profile with the pipeline' do
expect(pipeline.dast_profile).to eq(dast_profile)
shared_examples 'transactional creation' do
let_it_be(:type_mismatch) { build(:dast_scanner_profile) }
it 'does not create a Ci::Pipeline' do
expect { subject }.to raise_error(ActiveRecord::AssociationTypeMismatch).and change { Ci::Pipeline.count }.by(0)
end
end
context 'when no dast_profile is provided' do
let(:dast_profile) { nil }
context 'when the dast_profile and dast_site_profile are provided' do
it 'associates the dast_profile with the pipeline' do
expect(pipeline.dast_profile).to eq(dast_profile)
end
it 'does not create a new association' do
expect(pipeline.dast_profile).to be_nil
it 'does associate the dast_site_profile with the pipeline' do
expect(pipeline.dast_site_profile).to be_nil
end
it_behaves_like 'transactional creation' do
let_it_be(:dast_profile) { type_mismatch }
end
end
context 'when creating the association betweeen the dast_profile and the pipeline fails' do
let_it_be(:dast_profile) { build(:dast_site_profile) }
context 'when the dast_site_profile is provided' do
let(:dast_profile) { nil }
it 'does not create a Ci::Pipeline' do
expect { subject }.to raise_error(ActiveRecord::AssociationTypeMismatch).and change { Ci::Pipeline.count }.by(0)
it 'associates the dast_site_profile with the pipeline' do
expect(pipeline.dast_site_profile).to eq(dast_site_profile)
end
it_behaves_like 'transactional creation' do
let_it_be(:dast_site_profile) { type_mismatch }
end
end
......
......@@ -70,6 +70,7 @@ RSpec.describe DastOnDemandScans::CreateService do
auth_username_field: dast_site_profile.auth_username_field,
branch: project.default_branch,
dast_profile: nil,
dast_site_profile: dast_site_profile,
excluded_urls: dast_site_profile.excluded_urls.join(','),
full_scan_enabled: false,
show_debug_messages: false,
......
......@@ -48,6 +48,7 @@ RSpec.describe DastOnDemandScans::ParamsCreateService do
auth_url: dast_site_profile.auth_url,
branch: project.default_branch,
dast_profile: nil,
dast_site_profile: dast_site_profile,
excluded_urls: excluded_urls,
target_url: target_url
)
......@@ -65,6 +66,7 @@ RSpec.describe DastOnDemandScans::ParamsCreateService do
auth_url: dast_site_profile.auth_url,
branch: project.default_branch,
dast_profile: nil,
dast_site_profile: dast_site_profile,
excluded_urls: excluded_urls,
full_scan_enabled: false,
show_debug_messages: false,
......@@ -102,6 +104,7 @@ RSpec.describe DastOnDemandScans::ParamsCreateService do
expect(subject.payload).to eq(
branch: project.default_branch,
dast_profile: nil,
dast_site_profile: dast_site_profile,
excluded_urls: excluded_urls,
full_scan_enabled: false,
show_debug_messages: false,
......@@ -125,29 +128,38 @@ RSpec.describe DastOnDemandScans::ParamsCreateService do
expect(subject.payload).to match(expected_payload).and exclude(:target_url)
end
end
context 'when the dast_profile is provided' do
let_it_be(:dast_profile) { create(:dast_profile, project: project, dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile, branch_name: project.default_branch) }
let(:params) { { dast_profile: dast_profile } }
it 'returns prepared scanner params in the payload' do
expect(subject.payload).to eq(
auth_password_field: dast_site_profile.auth_password_field,
auth_username: dast_site_profile.auth_username,
auth_username_field: dast_site_profile.auth_username_field,
branch: dast_profile.branch_name,
auth_url: dast_site_profile.auth_url,
dast_profile: dast_profile,
dast_site_profile: dast_profile.dast_site_profile,
excluded_urls: excluded_urls,
full_scan_enabled: false,
show_debug_messages: false,
spider_timeout: nil,
target_timeout: nil,
target_url: target_url,
use_ajax_spider: false
)
end
end
end
context 'when the dast_profile is provided' do
let_it_be(:dast_profile) { create(:dast_profile, project: project, dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile, branch_name: project.default_branch) }
let(:params) { { dast_profile: dast_profile } }
it 'returns prepared scanner params in the payload' do
expect(subject.payload).to eq(
auth_password_field: dast_site_profile.auth_password_field,
auth_username: dast_site_profile.auth_username,
auth_username_field: dast_site_profile.auth_username_field,
branch: dast_profile.branch_name,
auth_url: dast_site_profile.auth_url,
dast_profile: dast_profile,
excluded_urls: excluded_urls,
full_scan_enabled: false,
show_debug_messages: false,
spider_timeout: nil,
target_timeout: nil,
target_url: target_url,
use_ajax_spider: false
)
context 'when the branch is provided' do
let(:params) { { dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile, branch: 'hello-world' } }
it 'returns the branch in the payload' do
expect(subject.payload[:branch]).to match('hello-world')
end
end
end
......
......@@ -55,6 +55,7 @@ RSpec.describe Security::SecurityOrchestrationPolicies::OnDemandScanPipelineConf
auth_username: site_profile.auth_username,
auth_username_field: site_profile.auth_username_field,
dast_profile: nil,
dast_site_profile: site_profile,
branch: project.default_branch,
excluded_urls: site_profile.excluded_urls.join(','),
full_scan_enabled: false,
......
......@@ -260,6 +260,8 @@ ci_pipelines:
- latest_statuses
- dast_profile
- dast_profiles_pipeline
- dast_site_profile
- dast_site_profiles_pipeline
ci_refs:
- project
- ci_pipelines
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment