diff --git a/config/gitlab_loose_foreign_keys.yml b/config/gitlab_loose_foreign_keys.yml index cf473270d6129257e0b4b2b63a0a85f2ed4387f7..c4be4cc1800601a5029d0e1d840a16b9291e4a4f 100644 --- a/config/gitlab_loose_foreign_keys.yml +++ b/config/gitlab_loose_foreign_keys.yml @@ -196,6 +196,10 @@ dast_profiles_pipelines: - table: ci_pipelines column: ci_pipeline_id on_delete: async_delete +dast_profiles_tags: + - table: tags + column: tag_id + on_delete: async_delete dast_scanner_profiles_builds: - table: ci_builds column: ci_build_id diff --git a/db/docs/dast_profiles_tags.yml b/db/docs/dast_profiles_tags.yml new file mode 100644 index 0000000000000000000000000000000000000000..8210b93f3afa1a88e3adc399440d4e31a4a9df7d --- /dev/null +++ b/db/docs/dast_profiles_tags.yml @@ -0,0 +1,10 @@ +--- +table_name: dast_profiles_tags +classes: + - Dast::ScannerProfileTag +feature_categories: + - dynamic_application_security_testing +description: Join Table for Runner tags and DAST Profiles +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108371 +milestone: '15.8' +gitlab_schema: gitlab_main diff --git a/db/migrate/20230106184809_create_dast_profiles_tags.rb b/db/migrate/20230106184809_create_dast_profiles_tags.rb new file mode 100644 index 0000000000000000000000000000000000000000..f31eaea5fa869704b3601148b0ddcbca010135fe --- /dev/null +++ b/db/migrate/20230106184809_create_dast_profiles_tags.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CreateDastProfilesTags < Gitlab::Database::Migration[2.1] + def up + create_table :dast_profiles_tags do |t| + t.references :dast_profile, null: false, foreign_key: { on_delete: :cascade }, + index: { name: 'i_dast_profiles_tags_on_scanner_profiles_id' } + + t.bigint :tag_id, null: false + + t.index :tag_id, name: :index_dast_profiles_tags_on_tag_id + end + end + + def down + drop_table :dast_profiles_tags + end +end diff --git a/db/schema_migrations/20230106184809 b/db/schema_migrations/20230106184809 new file mode 100644 index 0000000000000000000000000000000000000000..95318b9ea027c2243d2e8678e3bdf1ed7165e688 --- /dev/null +++ b/db/schema_migrations/20230106184809 @@ -0,0 +1 @@ +dad6e8972db3829dc6c02013ee87b08aa9bf4c50e58b35b0dbd67935ee4c266a \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 1434b3898bac810b197a9e201235afbf898930c5..d06c33ec2b21193e128d954009681e0d4784d150 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14545,6 +14545,21 @@ CREATE TABLE dast_profiles_pipelines ( COMMENT ON TABLE dast_profiles_pipelines IS '{"owner":"group::dynamic analysis","description":"Join table between DAST Profiles and CI Pipelines"}'; +CREATE TABLE dast_profiles_tags ( + id bigint NOT NULL, + dast_profile_id bigint NOT NULL, + tag_id bigint NOT NULL +); + +CREATE SEQUENCE dast_profiles_tags_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE dast_profiles_tags_id_seq OWNED BY dast_profiles_tags.id; + CREATE TABLE dast_scanner_profiles ( id bigint NOT NULL, created_at timestamp with time zone NOT NULL, @@ -24056,6 +24071,8 @@ ALTER TABLE ONLY dast_profile_schedules ALTER COLUMN id SET DEFAULT nextval('das ALTER TABLE ONLY dast_profiles ALTER COLUMN id SET DEFAULT nextval('dast_profiles_id_seq'::regclass); +ALTER TABLE ONLY dast_profiles_tags ALTER COLUMN id SET DEFAULT nextval('dast_profiles_tags_id_seq'::regclass); + ALTER TABLE ONLY dast_scanner_profiles ALTER COLUMN id SET DEFAULT nextval('dast_scanner_profiles_id_seq'::regclass); ALTER TABLE ONLY dast_scanner_profiles_tags ALTER COLUMN id SET DEFAULT nextval('dast_scanner_profiles_tags_id_seq'::regclass); @@ -25926,6 +25943,9 @@ ALTER TABLE ONLY dast_profiles_pipelines ALTER TABLE ONLY dast_profiles ADD CONSTRAINT dast_profiles_pkey PRIMARY KEY (id); +ALTER TABLE ONLY dast_profiles_tags + ADD CONSTRAINT dast_profiles_tags_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); @@ -28249,6 +28269,8 @@ CREATE INDEX i_compliance_frameworks_on_id_and_created_at ON compliance_manageme CREATE INDEX i_dast_pre_scan_verification_steps_on_pre_scan_verification_id ON dast_pre_scan_verification_steps USING btree (dast_pre_scan_verification_id); +CREATE INDEX i_dast_profiles_tags_on_scanner_profiles_id ON dast_profiles_tags USING btree (dast_profile_id); + CREATE INDEX i_dast_scanner_profiles_tags_on_scanner_profiles_id ON dast_scanner_profiles_tags USING btree (dast_scanner_profile_id); CREATE UNIQUE INDEX i_pm_licenses_on_spdx_identifier ON pm_licenses USING btree (spdx_identifier); @@ -29229,6 +29251,8 @@ CREATE UNIQUE INDEX index_dast_profiles_on_project_id_and_name ON dast_profiles CREATE UNIQUE INDEX index_dast_profiles_pipelines_on_ci_pipeline_id ON dast_profiles_pipelines USING btree (ci_pipeline_id); +CREATE INDEX index_dast_profiles_tags_on_tag_id ON dast_profiles_tags USING btree (tag_id); + CREATE UNIQUE INDEX index_dast_scanner_profiles_on_project_id_and_name ON dast_scanner_profiles USING btree (project_id, name); CREATE INDEX index_dast_scanner_profiles_tags_on_tag_id ON dast_scanner_profiles_tags USING btree (tag_id); @@ -35269,6 +35293,9 @@ ALTER TABLE ONLY merge_request_user_mentions ALTER TABLE ONLY x509_commit_signatures ADD CONSTRAINT fk_rails_ab07452314 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY dast_profiles_tags + ADD CONSTRAINT fk_rails_ab9e643cd8 FOREIGN KEY (dast_profile_id) REFERENCES dast_profiles(id) ON DELETE CASCADE; + ALTER TABLE ONLY resource_iteration_events ADD CONSTRAINT fk_rails_abf5d4affa FOREIGN KEY (issue_id) REFERENCES issues(id) ON DELETE CASCADE; diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index fd91a63574f8abdbc364a3a6fd1931682dcabb82..0a8d99a201c3be235d42d5565b18aafcd3eb60d5 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -1961,6 +1961,7 @@ Input type: `DastProfileCreateInput` | <a id="mutationdastprofilecreatefullpath"></a>`fullPath` | [`ID!`](#id) | Project the profile belongs to. | | <a id="mutationdastprofilecreatename"></a>`name` | [`String!`](#string) | Name of the profile. | | <a id="mutationdastprofilecreaterunaftercreate"></a>`runAfterCreate` | [`Boolean`](#boolean) | Run scan using profile after creation. Defaults to false. | +| <a id="mutationdastprofilecreatetaglist"></a>`tagList` | [`[String!]`](#string) | Indicates the runner tags associated with the profile. | #### Fields @@ -2027,6 +2028,7 @@ Input type: `DastProfileUpdateInput` | <a id="mutationdastprofileupdateid"></a>`id` | [`DastProfileID!`](#dastprofileid) | ID of the profile to be deleted. | | <a id="mutationdastprofileupdatename"></a>`name` | [`String`](#string) | Name of the profile. | | <a id="mutationdastprofileupdaterunafterupdate"></a>`runAfterUpdate` | [`Boolean`](#boolean) | Run scan using profile after update. Defaults to false. | +| <a id="mutationdastprofileupdatetaglist"></a>`tagList` | [`[String!]`](#string) | Indicates the runner tags associated with the profile. | #### Fields @@ -2051,7 +2053,7 @@ Input type: `DastScannerProfileCreateInput` | <a id="mutationdastscannerprofilecreatescantype"></a>`scanType` | [`DastScanTypeEnum`](#dastscantypeenum) | Indicates the type of DAST scan that will run. Either a Passive Scan or an Active Scan. | | <a id="mutationdastscannerprofilecreateshowdebugmessages"></a>`showDebugMessages` | [`Boolean`](#boolean) | Indicates if debug messages should be included in DAST console output. True to include the debug messages. | | <a id="mutationdastscannerprofilecreatespidertimeout"></a>`spiderTimeout` | [`Int`](#int) | Maximum number of minutes allowed for the spider to traverse the site. | -| <a id="mutationdastscannerprofilecreatetaglist"></a>`tagList` | [`[String!]`](#string) | Indicates the runner tags associated with the scanner profile. | +| <a id="mutationdastscannerprofilecreatetaglist"></a>`tagList` **{warning-solid}** | [`[String!]`](#string) | **Deprecated:** Moved to DastProfile. Deprecated in 15.8. | | <a id="mutationdastscannerprofilecreatetargettimeout"></a>`targetTimeout` | [`Int`](#int) | Maximum number of seconds allowed for the site under test to respond to a request. | | <a id="mutationdastscannerprofilecreateuseajaxspider"></a>`useAjaxSpider` | [`Boolean`](#boolean) | Indicates if the AJAX spider should be used to crawl the target site. True to run the AJAX spider in addition to the traditional spider, and false to run only the traditional spider. | @@ -2098,7 +2100,7 @@ Input type: `DastScannerProfileUpdateInput` | <a id="mutationdastscannerprofileupdatescantype"></a>`scanType` | [`DastScanTypeEnum`](#dastscantypeenum) | Indicates the type of DAST scan that will run. Either a Passive Scan or an Active Scan. | | <a id="mutationdastscannerprofileupdateshowdebugmessages"></a>`showDebugMessages` | [`Boolean`](#boolean) | Indicates if debug messages should be included in DAST console output. True to include the debug messages. | | <a id="mutationdastscannerprofileupdatespidertimeout"></a>`spiderTimeout` | [`Int!`](#int) | Maximum number of minutes allowed for the spider to traverse the site. | -| <a id="mutationdastscannerprofileupdatetaglist"></a>`tagList` | [`[String!]`](#string) | Indicates the runner tags associated with the scanner profile. | +| <a id="mutationdastscannerprofileupdatetaglist"></a>`tagList` **{warning-solid}** | [`[String!]`](#string) | **Deprecated:** Moved to DastProfile. Deprecated in 15.8. | | <a id="mutationdastscannerprofileupdatetargettimeout"></a>`targetTimeout` | [`Int!`](#int) | Maximum number of seconds allowed for the site under test to respond to a request. | | <a id="mutationdastscannerprofileupdateuseajaxspider"></a>`useAjaxSpider` | [`Boolean`](#boolean) | Indicates if the AJAX spider should be used to crawl the target site. True to run the AJAX spider in addition to the traditional spider, and false to run only the traditional spider. | @@ -12001,6 +12003,7 @@ Represents a DAST Profile. | <a id="dastprofileeditpath"></a>`editPath` | [`String`](#string) | Relative web path to the edit page of a profile. | | <a id="dastprofileid"></a>`id` | [`DastProfileID!`](#dastprofileid) | ID of the profile. | | <a id="dastprofilename"></a>`name` | [`String`](#string) | Name of the profile. | +| <a id="dastprofiletaglist"></a>`tagList` | [`[String!]`](#string) | Runner tags associated with the profile. | ### `DastProfileBranch` @@ -12055,7 +12058,7 @@ Represents a DAST scanner profile. | <a id="dastscannerprofilescantype"></a>`scanType` | [`DastScanTypeEnum`](#dastscantypeenum) | Indicates the type of DAST scan that will run. Either a Passive Scan or an Active Scan. | | <a id="dastscannerprofileshowdebugmessages"></a>`showDebugMessages` | [`Boolean!`](#boolean) | Indicates if debug messages should be included in DAST console output. True to include the debug messages. | | <a id="dastscannerprofilespidertimeout"></a>`spiderTimeout` | [`Int`](#int) | Maximum number of minutes allowed for the spider to traverse the site. | -| <a id="dastscannerprofiletaglist"></a>`tagList` | [`[String!]`](#string) | Runner tags associated with the scanner profile. | +| <a id="dastscannerprofiletaglist"></a>`tagList` **{warning-solid}** | [`[String!]`](#string) | **Deprecated** in 15.8. Moved to DastProfile. | | <a id="dastscannerprofiletargettimeout"></a>`targetTimeout` | [`Int`](#int) | Maximum number of seconds allowed for the site under test to respond to a request. | | <a id="dastscannerprofileuseajaxspider"></a>`useAjaxSpider` | [`Boolean!`](#boolean) | Indicates if the AJAX spider should be used to crawl the target site. True to run the AJAX spider in addition to the traditional spider, and false to run only the traditional spider. | diff --git a/ee/app/graphql/mutations/dast/profiles/create.rb b/ee/app/graphql/mutations/dast/profiles/create.rb index d0d6b720a4ee2f49e7eaf9b71d6a4db40ecd23fe..c1e194a003758a3f0abb67e45e34610b63173be0 100644 --- a/ee/app/graphql/mutations/dast/profiles/create.rb +++ b/ee/app/graphql/mutations/dast/profiles/create.rb @@ -50,27 +50,24 @@ class Create < BaseMutation description: 'Run scan using profile after creation. Defaults to false.', default_value: false + argument :tag_list, [GraphQL::Types::String], + required: false, + description: 'Indicates the runner tags associated with the profile.' + authorize :create_on_demand_dast_scan - def resolve(full_path:, name:, dast_site_profile_id:, dast_scanner_profile_id:, description: '', branch_name: nil, run_after_create: false, dast_profile_schedule: nil) - project = authorized_find!(full_path) + def resolve(**args) + project = authorized_find!(args.delete(:full_path)) + + dast_site_profile = project.dast_site_profiles.find(args.delete(:dast_site_profile_id).model_id) + dast_scanner_profile = project.dast_scanner_profiles.find(args.delete(:dast_scanner_profile_id).model_id) - dast_site_profile = project.dast_site_profiles.find(dast_site_profile_id.model_id) - dast_scanner_profile = project.dast_scanner_profiles.find(dast_scanner_profile_id.model_id) + args.delete(:tag_list) unless Feature.enabled?(:on_demand_scans_runner_tags, project) response = ::AppSec::Dast::Profiles::CreateService.new( - container: project, + project: project, current_user: current_user, - params: { - project: project, - name: name, - description: description, - branch_name: branch_name, - dast_site_profile: dast_site_profile, - dast_scanner_profile: dast_scanner_profile, - run_after_create: run_after_create, - dast_profile_schedule: dast_profile_schedule - } + params: args.merge({ dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile }) ).execute return { errors: response.errors } if response.error? diff --git a/ee/app/graphql/mutations/dast/profiles/update.rb b/ee/app/graphql/mutations/dast/profiles/update.rb index 5669c74b0ad6f835832ca3b6bac3252a5f8e775a..2ab779f1df7895e22ce50a0265eb074b0842d1d8 100644 --- a/ee/app/graphql/mutations/dast/profiles/update.rb +++ b/ee/app/graphql/mutations/dast/profiles/update.rb @@ -61,6 +61,10 @@ class Update < BaseMutation description: 'Run scan using profile after update. Defaults to false.', default_value: false + argument :tag_list, [GraphQL::Types::String], + required: false, + description: 'Indicates the runner tags associated with the profile.' + authorize :create_on_demand_dast_scan def resolve(id:, name:, description:, full_path: nil, branch_name: nil, dast_scanner_profile_id: nil, run_after_update: false, **args) @@ -77,8 +81,10 @@ def resolve(id:, name:, description:, full_path: nil, branch_name: nil, dast_sca run_after_update: run_after_update }.compact + params[:tag_list] = args[:tag_list] if Feature.enabled?(:on_demand_scans_runner_tags, dast_profile.project) + response = ::AppSec::Dast::Profiles::UpdateService.new( - container: dast_profile.project, + project: dast_profile.project, current_user: current_user, params: params ).execute diff --git a/ee/app/graphql/mutations/dast_scanner_profiles/create.rb b/ee/app/graphql/mutations/dast_scanner_profiles/create.rb index 06b8113ff27bc3404ee2426925c06730137f984f..12647e2c6e6932932c66b94d1678fbc51ea2e711 100644 --- a/ee/app/graphql/mutations/dast_scanner_profiles/create.rb +++ b/ee/app/graphql/mutations/dast_scanner_profiles/create.rb @@ -49,9 +49,11 @@ class Create < BaseMutation description: 'Indicates if debug messages should be included in DAST console output. ' \ 'True to include the debug messages.', default_value: false + argument :tag_list, [GraphQL::Types::String], required: false, - description: 'Indicates the runner tags associated with the scanner profile.' + description: 'Indicates the runner tags associated with the scanner profile.', + deprecated: { reason: 'Moved to DastProfile', milestone: '15.8' } authorize :create_on_demand_dast_scan diff --git a/ee/app/graphql/mutations/dast_scanner_profiles/update.rb b/ee/app/graphql/mutations/dast_scanner_profiles/update.rb index 9ccf62709de1457043e76fe33276b408b483f332..97936ac099bf0d8f5bc14265102c1759b192204d 100644 --- a/ee/app/graphql/mutations/dast_scanner_profiles/update.rb +++ b/ee/app/graphql/mutations/dast_scanner_profiles/update.rb @@ -53,19 +53,17 @@ class Update < BaseMutation required: false, description: 'Indicates if debug messages should be included in DAST console output. ' \ 'True to include the debug messages.' + argument :tag_list, [GraphQL::Types::String], required: false, - description: 'Indicates the runner tags associated with the scanner profile.' + description: 'Indicates the runner tags associated with the scanner profile.', + deprecated: { reason: 'Moved to DastProfile', milestone: '15.8' } authorize :create_on_demand_dast_scan def resolve(id:, full_path: nil, **service_args) dast_scanner_profile = authorized_find!(id) - unless Feature.enabled?(:on_demand_scans_runner_tags, dast_scanner_profile.project) - service_args.delete(:tag_list) - end - params = { **service_args, id: dast_scanner_profile.id } service = ::AppSec::Dast::ScannerProfiles::UpdateService.new(project: dast_scanner_profile.project, current_user: current_user, params: params) result = service.execute diff --git a/ee/app/graphql/types/dast/profile_type.rb b/ee/app/graphql/types/dast/profile_type.rb index 1883c1c2571bb915d6416e0efaaf263a54c81157..648f6ac679d63604be62ea94b1ce5d3307798e60 100644 --- a/ee/app/graphql/types/dast/profile_type.rb +++ b/ee/app/graphql/types/dast/profile_type.rb @@ -40,6 +40,9 @@ class ProfileType < BaseObject description: 'DAST Pre Scan Verification associated with the site profile. Will always return `null` ' \ 'if `dast_on_demand_scans_scheduler` feature flag is disabled.' + field :tag_list, [GraphQL::Types::String], + null: true, description: 'Runner tags associated with the profile.' + def edit_path Gitlab::Routing.url_helpers.edit_project_on_demand_scan_path(object.project, object) end diff --git a/ee/app/graphql/types/dast_scanner_profile_type.rb b/ee/app/graphql/types/dast_scanner_profile_type.rb index acfc6c81714e0bca2aeab691f532422c73388308..47ea1d76ace8be8767f3333ed8d1539c7442d07c 100644 --- a/ee/app/graphql/types/dast_scanner_profile_type.rb +++ b/ee/app/graphql/types/dast_scanner_profile_type.rb @@ -43,7 +43,9 @@ class DastScannerProfileType < BaseObject null: true, calls_gitaly: true, description: 'List of security policy names that are referencing given project.' - field :tag_list, [GraphQL::Types::String], null: true, description: 'Runner tags associated with the scanner profile.' + field :tag_list, [GraphQL::Types::String], null: true, + description: 'Runner tags associated with the scanner profile.', + deprecated: { reason: 'Moved to DastProfile', milestone: '15.8' } def edit_path Rails.application.routes.url_helpers.edit_project_security_configuration_profile_library_dast_scanner_profile_path(object.project, object) @@ -55,5 +57,9 @@ def referenced_in_security_policies object ) end + + def tag_list + [] + end end end diff --git a/ee/app/models/dast/profile.rb b/ee/app/models/dast/profile.rb index d1537bf0f28b0e7ba91703938957f4531b082ae5..f40f35def94e1f63c2d5fc084a7751f817a192d3 100644 --- a/ee/app/models/dast/profile.rb +++ b/ee/app/models/dast/profile.rb @@ -15,6 +15,9 @@ class Profile < ApplicationRecord has_one :dast_profile_schedule, class_name: 'Dast::ProfileSchedule', foreign_key: :dast_profile_id, inverse_of: :dast_profile has_one :dast_pre_scan_verification, class_name: 'Dast::PreScanVerification', foreign_key: :dast_profile_id, inverse_of: :dast_profile + has_many :profile_runner_tags, class_name: 'Dast::ProfileTag', foreign_key: :dast_profile_id, inverse_of: :dast_profile + has_many :tags, through: :profile_runner_tags, disable_joins: true + validates :description, length: { maximum: 255 } validates :name, length: { maximum: 255 }, uniqueness: { scope: :project_id }, presence: true validates :branch_name, length: { maximum: 255 } @@ -46,6 +49,10 @@ def branch Dast::Branch.new(self) end + def tag_list + tags.map(&:name) + end + private def project_ids_match diff --git a/ee/app/models/dast/profile_tag.rb b/ee/app/models/dast/profile_tag.rb new file mode 100644 index 0000000000000000000000000000000000000000..7d624c9a4cbd99befd5797bc169bf77670c1b6d7 --- /dev/null +++ b/ee/app/models/dast/profile_tag.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Dast + class ProfileTag < ApplicationRecord + self.table_name = 'dast_profiles_tags' + + belongs_to :tag, class_name: 'ActsAsTaggableOn::Tag', optional: false + belongs_to :dast_profile, class_name: 'Dast::Profile', optional: false + end +end diff --git a/ee/app/models/dast/scanner_profile_tag.rb b/ee/app/models/dast/scanner_profile_tag.rb deleted file mode 100644 index a8e7a6cd2ccc468f95fa0d4cbd645169815289b5..0000000000000000000000000000000000000000 --- a/ee/app/models/dast/scanner_profile_tag.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -module Dast - class ScannerProfileTag < ApplicationRecord - self.table_name = 'dast_scanner_profiles_tags' - - belongs_to :tag, class_name: 'ActsAsTaggableOn::Tag', optional: false - belongs_to :dast_scanner_profile, class_name: 'DastScannerProfile', optional: false - end -end diff --git a/ee/app/models/dast_scanner_profile.rb b/ee/app/models/dast_scanner_profile.rb index 208f455a8a3ca2f480502e0f7af4837a7423d348..bbd1b5b6fcc21548614e80d1df82f404d6597a56 100644 --- a/ee/app/models/dast_scanner_profile.rb +++ b/ee/app/models/dast_scanner_profile.rb @@ -4,8 +4,6 @@ class DastScannerProfile < ApplicationRecord include Sanitizable belongs_to :project - has_many :scanner_profile_runner_tags, class_name: 'Dast::ScannerProfileTag' - has_many :tags, through: :scanner_profile_runner_tags, disable_joins: true validates :project_id, presence: true validates :name, length: { maximum: 255 }, uniqueness: { scope: :project_id }, presence: true @@ -54,8 +52,4 @@ def referenced_in_security_policies .inject(&:merge) .to_a end - - def tag_list - tags.map(&:name) - end end diff --git a/ee/app/services/app_sec/dast/profiles/base_service.rb b/ee/app/services/app_sec/dast/profiles/base_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..b3bb88ef7d8bf33839b8b20f8bbdf65ccf9e9a1a --- /dev/null +++ b/ee/app/services/app_sec/dast/profiles/base_service.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module AppSec + module Dast + module Profiles + class BaseService < BaseProjectService + private + + def valid_tags? + return true unless tag_list? + + tag_list.size == params[:tag_list].size + end + + def tag_list? + Feature.enabled?(:on_demand_scans_runner_tags, project) && params[:tag_list].present? + end + + def tag_list + @tag_list ||= ActsAsTaggableOn::Tag.named_any(params[:tag_list]) + end + + def tags + if tag_list? + tag_list + else + [] + end + end + + def error(message, opts = {}) + ServiceResponse.error(message: message, **opts) + end + end + end + end +end diff --git a/ee/app/services/app_sec/dast/profiles/create_service.rb b/ee/app/services/app_sec/dast/profiles/create_service.rb index 7d7fb06603f76027c57fb2724b988c0cc049e24a..d3a26f10b979418e7c14b40da8fdbb87cd8bedf9 100644 --- a/ee/app/services/app_sec/dast/profiles/create_service.rb +++ b/ee/app/services/app_sec/dast/profiles/create_service.rb @@ -3,9 +3,11 @@ module AppSec module Dast module Profiles - class CreateService < BaseContainerService + class CreateService < BaseService def execute - return ServiceResponse.error(message: _('Insufficient permissions')) unless allowed? + return error(_('Insufficient permissions')) unless allowed? + + return error(_('Invalid tags')) unless valid_tags? ApplicationRecord.transaction do @dast_profile = create_profile @@ -38,21 +40,26 @@ def execute private def create_profile - ::Dast::Profile.create!( - project: container, + ::Dast::Profile.create!(create_params) + end + + def create_params + { + project: project, name: params.fetch(:name), description: params.fetch(:description), branch_name: params[:branch_name], dast_site_profile: dast_site_profile, - dast_scanner_profile: dast_scanner_profile - ) + dast_scanner_profile: dast_scanner_profile, + tags: tags + } end def create_schedule(dast_profile) ::Dast::ProfileSchedule.create!( owner: current_user, dast_profile: dast_profile, - project_id: container.id, + project_id: project.id, cadence: dast_profile_schedule[:cadence], timezone: dast_profile_schedule[:timezone], starts_at: dast_profile_schedule[:starts_at] @@ -61,14 +68,14 @@ def create_schedule(dast_profile) def create_on_demand_scan(dast_profile) ::AppSec::Dast::Scans::CreateService.new( - container: container, + container: project, current_user: current_user, params: { dast_profile: dast_profile } ).execute end def allowed? - container.licensed_feature_available?(:security_on_demand_scans) + project.licensed_feature_available?(:security_on_demand_scans) end def dast_site_profile @@ -87,7 +94,7 @@ def create_audit_event(dast_profile, schedule) ::Gitlab::Audit::Auditor.audit( name: 'dast_profile_create', author: current_user, - scope: container, + scope: project, target: dast_profile, message: 'Added DAST profile' ) @@ -96,7 +103,7 @@ def create_audit_event(dast_profile, schedule) ::Gitlab::Audit::Auditor.audit( name: 'dast_profile_schedule_create', author: current_user, - scope: container, + scope: project, target: schedule, message: 'Added DAST profile schedule' ) diff --git a/ee/app/services/app_sec/dast/profiles/update_service.rb b/ee/app/services/app_sec/dast/profiles/update_service.rb index a48c0e00fd3ff2a66949fb86c05e3fb1f5b00a61..22a682117538f292556b0d60606cb3757fae72a6 100644 --- a/ee/app/services/app_sec/dast/profiles/update_service.rb +++ b/ee/app/services/app_sec/dast/profiles/update_service.rb @@ -3,17 +3,19 @@ module AppSec module Dast module Profiles - class UpdateService < BaseContainerService + class UpdateService < BaseService include Gitlab::Utils::StrongMemoize def execute return unauthorized unless allowed? return error(_('Profile parameter missing')) unless dast_profile + return error(_('Invalid tags')) unless valid_tags? + build_auditors! ApplicationRecord.transaction do - dast_profile.update!(dast_profile_params) + dast_profile.update!(update_params) update_or_create_schedule! if schedule_input_params end @@ -43,11 +45,17 @@ def execute private + def update_params + update_params = dast_profile_params + update_params[:tags] = tags if tag_list? + update_params + end + attr_reader :auditors, :create_schedule_audit def allowed? - container.licensed_feature_available?(:security_on_demand_scans) && - can?(current_user, :create_on_demand_dast_scan, container) + project.licensed_feature_available?(:security_on_demand_scans) && + can?(current_user, :create_on_demand_dast_scan, project) end def update_or_create_schedule! @@ -57,7 +65,7 @@ def update_or_create_schedule! ::Dast::ProfileSchedule.new( dast_profile: dast_profile, owner: current_user, - project: container + project: project ).tap do |dast_schedule| dast_schedule.update!(schedule_input_params) end @@ -70,10 +78,6 @@ def schedule dast_profile.dast_profile_schedule end - def error(message, opts = {}) - ServiceResponse.error(message: message, **opts) - end - def success(payload) ServiceResponse.success(payload: payload) end @@ -106,7 +110,7 @@ def build_schedule_input_params def build_auditors! @auditors = [ - AppSec::Dast::Profiles::Audit::UpdateService.new(container: container, current_user: current_user, params: { + AppSec::Dast::Profiles::Audit::UpdateService.new(container: project, current_user: current_user, params: { dast_profile: dast_profile, new_params: dast_profile_params, old_params: dast_profile.attributes.symbolize_keys @@ -115,7 +119,7 @@ def build_auditors! if schedule_input_params && schedule @auditors << - AppSec::Dast::ProfileSchedules::Audit::UpdateService.new(project: container, current_user: current_user, params: { + AppSec::Dast::ProfileSchedules::Audit::UpdateService.new(project: project, current_user: current_user, params: { dast_profile_schedule: schedule, new_params: schedule_input_params, old_params: schedule.attributes.symbolize_keys @@ -130,7 +134,7 @@ def execute_auditors! ::Gitlab::Audit::Auditor.audit( name: 'dast_profile_schedule_create', author: current_user, - scope: container, + scope: project, target: schedule, message: 'Added DAST profile schedule' ) @@ -139,7 +143,7 @@ def execute_auditors! def create_scan(dast_profile) ::AppSec::Dast::Scans::CreateService.new( - container: container, + container: project, current_user: current_user, params: { dast_profile: dast_profile } ).execute diff --git a/ee/app/services/app_sec/dast/scan_configs/build_service.rb b/ee/app/services/app_sec/dast/scan_configs/build_service.rb index 493397dfb6b63c30a88c7ccaac289445e8fb929b..159b8922e882fce44f8c982daaa4f030c8024b24 100644 --- a/ee/app/services/app_sec/dast/scan_configs/build_service.rb +++ b/ee/app/services/app_sec/dast/scan_configs/build_service.rb @@ -50,8 +50,8 @@ def ci_configuration } } - if Feature.enabled?(:on_demand_scans_runner_tags, container) && dast_scanner_profile&.tags.present? - ci_config['dast']['tags'] = dast_scanner_profile.tag_list + if Feature.enabled?(:on_demand_scans_runner_tags, container) && dast_profile&.tags.present? + ci_config['dast']['tags'] = dast_profile.tag_list end ci_config.to_yaml diff --git a/ee/app/services/app_sec/dast/scanner_profiles/base_service.rb b/ee/app/services/app_sec/dast/scanner_profiles/base_service.rb index de756c2cc06169611ad63f3a8070d97f3a8e9d92..941bc6765f2196490a24a47bb42cdb7bb53bc59a 100644 --- a/ee/app/services/app_sec/dast/scanner_profiles/base_service.rb +++ b/ee/app/services/app_sec/dast/scanner_profiles/base_service.rb @@ -6,28 +6,6 @@ module ScannerProfiles class BaseService < BaseProjectService private - def valid_tags? - return true unless tag_list? - - tag_list.size == params[:tag_list].size - end - - def tag_list? - Feature.enabled?(:on_demand_scans_runner_tags, project) && params[:tag_list].present? - end - - def tag_list - @tag_list ||= ActsAsTaggableOn::Tag.named_any(params[:tag_list]) - end - - def tags - if tag_list? - tag_list - else - [] - end - end - def base_params { target_timeout: params[:target_timeout], diff --git a/ee/app/services/app_sec/dast/scanner_profiles/create_service.rb b/ee/app/services/app_sec/dast/scanner_profiles/create_service.rb index 9e9eb66b42f94a5f5fc7f7116cf965f7be4b9860..2ebc51c2ddb97029124708551a45b2d390dc9f51 100644 --- a/ee/app/services/app_sec/dast/scanner_profiles/create_service.rb +++ b/ee/app/services/app_sec/dast/scanner_profiles/create_service.rb @@ -7,8 +7,6 @@ class CreateService < BaseService def execute return ServiceResponse.error(message: 'Insufficient permissions') unless allowed? - return ServiceResponse.error(message: 'Invalid tag_list') unless valid_tags? - dast_scanner_profile = DastScannerProfile.create(create_params) if dast_scanner_profile.valid? @@ -37,7 +35,7 @@ def create_audit_event(profile) end def create_params - base_params.merge({ name: params[:name], project: project, tags: tags }) + base_params.merge({ name: params[:name], project: project }) end end end diff --git a/ee/app/services/app_sec/dast/scanner_profiles/update_service.rb b/ee/app/services/app_sec/dast/scanner_profiles/update_service.rb index 20aa1e7d00a0312ea804f4fdac661cad89942425..3a4028c15c9dd3aaae932a24c4961e181ad9aa85 100644 --- a/ee/app/services/app_sec/dast/scanner_profiles/update_service.rb +++ b/ee/app/services/app_sec/dast/scanner_profiles/update_service.rb @@ -13,8 +13,6 @@ def execute return ServiceResponse.error(message: _('Scanner profile not found for given parameters')) unless dast_scanner_profile return ServiceResponse.error(message: _('Cannot modify %{profile_name} referenced in security policy') % { profile_name: dast_scanner_profile.name }) if referenced_in_security_policy?(dast_scanner_profile) - return ServiceResponse.error(message: 'Invalid tag_list') unless valid_tags? - old_params = dast_scanner_profile.attributes.symbolize_keys update_params = update_params(params[:profile_name]) @@ -46,8 +44,6 @@ def find_dast_scanner_profile(id) end def create_audit_events(profile, params, old_params) - params.delete(:tags) if tag_list? - params.each do |property, new_value| old_value = old_params[property] @@ -66,7 +62,6 @@ def create_audit_events(profile, params, old_params) def update_params(profile_name) params = base_params params[:name] = profile_name - params[:tags] = tags if tag_list? params.compact end end diff --git a/ee/spec/graphql/mutations/dast/profiles/create_spec.rb b/ee/spec/graphql/mutations/dast/profiles/create_spec.rb index 2f6f3df890416348630dfc138b85e172eccfd503..fdbf696376302dd9217597ebb6d1adb6a52336e5 100644 --- a/ee/spec/graphql/mutations/dast/profiles/create_spec.rb +++ b/ee/spec/graphql/mutations/dast/profiles/create_spec.rb @@ -15,10 +15,13 @@ let(:run_after_create) { false } let(:dast_profile) { Dast::Profile.find_by(project: project, name: name) } let(:dast_profile_schedule) { nil } + let(:tag_list) { %w[ruby postgres] } subject(:mutation) { described_class.new(object: nil, context: { current_user: developer }, field: nil) } before do + ActsAsTaggableOn::Tag.create!(name: 'ruby') + ActsAsTaggableOn::Tag.create!(name: 'postgres') stub_licensed_features(security_on_demand_scans: true) end @@ -34,7 +37,8 @@ dast_site_profile_id: dast_site_profile.to_global_id, dast_scanner_profile_id: dast_scanner_profile.to_global_id, run_after_create: run_after_create, - dast_profile_schedule: dast_profile_schedule + dast_profile_schedule: dast_profile_schedule, + tag_list: tag_list ) end diff --git a/ee/spec/graphql/mutations/dast/profiles/update_spec.rb b/ee/spec/graphql/mutations/dast/profiles/update_spec.rb index 891f69e54ab0398a5566c13fa4c9c7d3efa2c525..bfde91327cfbd27f2151e9265a7d47e7a1a6a537 100644 --- a/ee/spec/graphql/mutations/dast/profiles/update_spec.rb +++ b/ee/spec/graphql/mutations/dast/profiles/update_spec.rb @@ -2,15 +2,23 @@ require 'spec_helper' -RSpec.describe Mutations::Dast::Profiles::Update do +RSpec.describe Mutations::Dast::Profiles::Update, :dynamic_analysis, + feature_category: :dynamic_application_security_testing do include GraphqlHelpers 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(:old_tags) do + [ActsAsTaggableOn::Tag.create!(name: 'ruby'), ActsAsTaggableOn::Tag.create!(name: 'postgres')] + end + + let_it_be(:dast_profile, reload: true) { create(:dast_profile, project: project, tags: old_tags) } 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_it_be(:new_tags) { [ActsAsTaggableOn::Tag.create!(name: 'rails'), ActsAsTaggableOn::Tag.create!(name: 'docker')] } + let_it_be(:new_tag_list) { new_tags.map(&:name) } + let(:dast_profile_schedule_attrs) { nil } let(:dast_profile_gid) { dast_profile.to_global_id } @@ -25,7 +33,8 @@ 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, - dast_profile_schedule: dast_profile_schedule_attrs + dast_profile_schedule: dast_profile_schedule_attrs, + tag_list: new_tag_list } end @@ -81,6 +90,7 @@ expect(updated_dast_profile.name).to eq(params[:name]) expect(updated_dast_profile.description).to eq(params[:description]) expect(updated_dast_profile.branch_name).to eq(params[:branch_name]) + expect(updated_dast_profile.tags).to match_array(new_tags) end end @@ -175,6 +185,20 @@ expect(subject[:errors]).to include('Profile failed to update') end end + + context 'when feature flag on_demand_scans_runner_tags is disabled' do + before do + stub_feature_flags(on_demand_scans_runner_tags: false) + end + + it 'does not update the tag_list' do + subject + + updated_dast_profile = dast_profile.reload + + expect(updated_dast_profile.tags).to match_array(old_tags) + end + end end end end diff --git a/ee/spec/graphql/mutations/dast_scanner_profiles/create_spec.rb b/ee/spec/graphql/mutations/dast_scanner_profiles/create_spec.rb index b7a533974881dfdcdf84375caf56e148584f2d15..a19b8bd98bf092fb67130b4ef88be575f3de0415 100644 --- a/ee/spec/graphql/mutations/dast_scanner_profiles/create_spec.rb +++ b/ee/spec/graphql/mutations/dast_scanner_profiles/create_spec.rb @@ -9,14 +9,11 @@ let(:user) { create(:user) } let(:full_path) { project.full_path } let(:profile_name) { SecureRandom.hex } - let(:tag_list) { %w[ruby postgres] } let(:dast_scanner_profile) { DastScannerProfile.find_by(project: project, name: profile_name) } subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } before do - ActsAsTaggableOn::Tag.create!(name: 'ruby') - ActsAsTaggableOn::Tag.create!(name: 'postgres') stub_licensed_features(security_on_demand_scans: true) end @@ -29,8 +26,7 @@ profile_name: profile_name, scan_type: DastScannerProfile.scan_types[:passive], use_ajax_spider: false, - show_debug_messages: false, - tag_list: tag_list + show_debug_messages: false ) end @@ -66,8 +62,7 @@ name: profile_name, scan_type: DastScannerProfile.scan_types[:passive], show_debug_messages: false, - use_ajax_spider: false, - tag_list: tag_list + use_ajax_spider: false } } @@ -78,34 +73,6 @@ subject end - context 'when feature flag on_demand_scans_runner_tags is disabled' do - before do - stub_feature_flags(on_demand_scans_runner_tags: false) - end - - it 'calls the dast_scanner_profile creation service without the tag_list' do - service = double(described_class) - result = double('result', success?: false, errors: []) - - args = { - project: project, - current_user: user, - params: { - name: profile_name, - scan_type: DastScannerProfile.scan_types[:passive], - show_debug_messages: false, - use_ajax_spider: false - } - } - - expect(::AppSec::Dast::ScannerProfiles::CreateService).to receive(:new).with(args).and_return(service) - - expect(service).to receive(:execute).and_return(result) - - subject - end - end - context 'when the dast_scanner_profile already exists' do it 'returns an error' do subject diff --git a/ee/spec/graphql/mutations/dast_scanner_profiles/update_spec.rb b/ee/spec/graphql/mutations/dast_scanner_profiles/update_spec.rb index 78dcc5be0e000cfe63b7285e445fbaa71c7e22cd..2ae73f9e69f2a71786eedcdc8bf4c9b5ef4298d6 100644 --- a/ee/spec/graphql/mutations/dast_scanner_profiles/update_spec.rb +++ b/ee/spec/graphql/mutations/dast_scanner_profiles/update_spec.rb @@ -9,8 +9,7 @@ let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } let_it_be(:user) { create(:user) } - let_it_be(:old_tags) { [ActsAsTaggableOn::Tag.create!(name: 'ruby'), ActsAsTaggableOn::Tag.create!(name: 'postgres')] } - let_it_be(:dast_scanner_profile) { create(:dast_scanner_profile, project: project, target_timeout: 200, spider_timeout: 5000, tags: old_tags) } + let_it_be(:dast_scanner_profile) { create(:dast_scanner_profile, project: project, target_timeout: 200, spider_timeout: 5000) } let_it_be(:new_profile_name) { SecureRandom.hex } let_it_be(:new_target_timeout) { dast_scanner_profile.target_timeout + 1 } @@ -18,8 +17,6 @@ let_it_be(:new_scan_type) { (DastScannerProfile.scan_types.keys - [DastScannerProfile.last.scan_type]).first } let_it_be(:new_use_ajax_spider) { !dast_scanner_profile.use_ajax_spider } let_it_be(:new_show_debug_messages) { !dast_scanner_profile.show_debug_messages } - let_it_be(:new_tags) { [ActsAsTaggableOn::Tag.create!(name: 'rails'), ActsAsTaggableOn::Tag.create!(name: 'docker')] } - let_it_be(:new_tag_list) { new_tags.map(&:name) } subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } @@ -38,8 +35,7 @@ spider_timeout: new_spider_timeout, scan_type: new_scan_type, use_ajax_spider: new_use_ajax_spider, - show_debug_messages: new_show_debug_messages, - tag_list: new_tag_list + show_debug_messages: new_show_debug_messages ) end @@ -82,7 +78,6 @@ expect(dast_scanner_profile.scan_type).to eq(new_scan_type) expect(dast_scanner_profile.use_ajax_spider).to eq(new_use_ajax_spider) expect(dast_scanner_profile.show_debug_messages).to eq(new_show_debug_messages) - expect(dast_scanner_profile.tags).to match_array(new_tags) end end @@ -90,18 +85,6 @@ expect(subject[:dast_scanner_profile]).to eq(dast_scanner_profile) end - context 'when feature flag on_demand_scans_runner_tags is disabled' do - before do - stub_feature_flags(on_demand_scans_runner_tags: false) - end - - it 'does not update the tag_list' do - dast_scanner_profile = subject[:id].find - - expect(dast_scanner_profile.tags).to match_array(old_tags) - end - end - context 'when dast scanner profile does not exist' do let(:scanner_profile_id) { global_id_of(model_name: 'DastScannerProfile', id: 'does_not_exist') } diff --git a/ee/spec/graphql/types/dast/profile_type_spec.rb b/ee/spec/graphql/types/dast/profile_type_spec.rb index 8094787df12c9176b3522aa24def72fb4394965d..a382da7e5e70ab49619f491fb7b72bea2a6c77a3 100644 --- a/ee/spec/graphql/types/dast/profile_type_spec.rb +++ b/ee/spec/graphql/types/dast/profile_type_spec.rb @@ -10,15 +10,19 @@ let_it_be(:object) { create(:dast_profile, project: project) } let_it_be(:dast_pre_scan_verification) { create(:dast_pre_scan_verification, dast_profile: object) } let_it_be(:user) { create(:user, developer_projects: [project]) } + let_it_be(:tag_list) { %w[ruby postgres] } + let_it_be(:fields) do %i[id name description dastSiteProfile dastScannerProfile dastProfileSchedule branch editPath - dastPreScanVerification] + dastPreScanVerification tagList] end specify { expect(described_class.graphql_name).to eq('DastProfile') } specify { expect(described_class).to require_graphql_authorizations(:read_on_demand_dast_scan) } before do + ActsAsTaggableOn::Tag.create!(name: 'ruby') + ActsAsTaggableOn::Tag.create!(name: 'postgres') stub_licensed_features(security_on_demand_scans: true) end @@ -63,4 +67,10 @@ end end end + + describe 'tagList field' do + it 'correctly resolves the field' do + expect(resolve_field(:tag_list, object, current_user: user)).to eq(object.tag_list) + end + end end diff --git a/ee/spec/graphql/types/dast_scanner_profile_type_spec.rb b/ee/spec/graphql/types/dast_scanner_profile_type_spec.rb index 34139704bc11725bf0262c991b430ab761b9b389..70d532e4edf8864e01005061aca3b8f1d4a174ee 100644 --- a/ee/spec/graphql/types/dast_scanner_profile_type_spec.rb +++ b/ee/spec/graphql/types/dast_scanner_profile_type_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' -RSpec.describe GitlabSchema.types['DastScannerProfile'] do +RSpec.describe GitlabSchema.types['DastScannerProfile'], :dynamic_analysis, + feature_category: :dynamic_application_security_testing do include RepoHelpers let_it_be(:dast_scanner_profile) { create(:dast_scanner_profile) } @@ -52,7 +53,6 @@ useAjaxSpider showDebugMessages referencedInSecurityPolicies - tagList } } } diff --git a/ee/spec/models/dast/profile_spec.rb b/ee/spec/models/dast/profile_spec.rb index 906338f4a5394a0d2f18ff976801d46538ba89cc..64c9a8a482a023807f1c59922e6f89bbf495a36d 100644 --- a/ee/spec/models/dast/profile_spec.rb +++ b/ee/spec/models/dast/profile_spec.rb @@ -16,6 +16,8 @@ it { is_expected.to have_many(:secret_variables).through(:dast_site_profile).class_name('Dast::SiteProfileSecretVariable') } it { is_expected.to have_one(:dast_profile_schedule).class_name('Dast::ProfileSchedule').with_foreign_key(:dast_profile_id).inverse_of(:dast_profile) } it { is_expected.to have_one(:dast_pre_scan_verification).class_name('Dast::PreScanVerification').with_foreign_key(:dast_profile_id).inverse_of(:dast_profile) } + it { is_expected.to have_many(:profile_runner_tags) } + it { is_expected.to have_many(:tags) } end describe 'validations' do diff --git a/ee/spec/models/dast_scanner_profile_spec.rb b/ee/spec/models/dast_scanner_profile_spec.rb index 72250f4030403b98868c40d819ce31bcfdd09747..a721d9bc146eba967e1673bfd747b34c3accd453 100644 --- a/ee/spec/models/dast_scanner_profile_spec.rb +++ b/ee/spec/models/dast_scanner_profile_spec.rb @@ -9,8 +9,6 @@ describe 'associations' do it { is_expected.to belong_to(:project) } - it { is_expected.to have_many(:scanner_profile_runner_tags) } - it { is_expected.to have_many(:tags) } end describe 'validations' do diff --git a/ee/spec/services/app_sec/dast/profiles/create_service_spec.rb b/ee/spec/services/app_sec/dast/profiles/create_service_spec.rb index 0283129828927f1bbf2e85b1608dcf3c76e6ad96..969d3df02d494a329d8ec635dd553e758a96c30c 100644 --- a/ee/spec/services/app_sec/dast/profiles/create_service_spec.rb +++ b/ee/spec/services/app_sec/dast/profiles/create_service_spec.rb @@ -2,9 +2,10 @@ require 'spec_helper' -RSpec.describe AppSec::Dast::Profiles::CreateService do +RSpec.describe AppSec::Dast::Profiles::CreateService, :dynamic_analysis, + feature_category: :dynamic_application_security_testing do let_it_be(:project) { create(:project, :repository) } - let_it_be(:developer) { create(:user, developer_projects: [project] ) } + let_it_be(:developer) { create(:user, developer_projects: [project]) } 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(:time_zone) { Time.zone.tzinfo.name } @@ -21,7 +22,7 @@ let(:params) { default_params } - subject { described_class.new(container: project, current_user: developer, params: params).execute } + subject { described_class.new(project: project, current_user: developer, params: params).execute } describe 'execute' do context 'when on demand scan licensed feature is not available' do @@ -139,7 +140,7 @@ it 'rollback the transaction' do expect { subject }.to change { ::Dast::ProfileSchedule.count }.by(0) - .and change { ::Dast::Profile.count }.by(0) + .and change { ::Dast::Profile.count }.by(0) end it 'returns the error service response' do @@ -158,6 +159,50 @@ end end end + + context 'when param tag_list is present' do + let_it_be(:tags) do + [ActsAsTaggableOn::Tag.create!(name: 'ruby'), ActsAsTaggableOn::Tag.create!(name: 'postgres')] + end + + let(:tag_list) { tags.map(&:name) } + + let(:params) { default_params.merge(tag_list: tag_list) } + + it 'creates a dast_profile with tags' do + expect(subject.payload[:dast_profile].tags).to match_array(tags) + end + + context 'when there is a invalid tag' do + let(:tag_list) { %w[invalid_tag] } + + it 'does not create a new dast_profile' do + expect { subject }.not_to change { Dast::Profile.count } + end + + it 'returns an error status' do + expect(subject.status).to eq(:error) + end + + it 'populates message' do + expect(subject.message).to eq('Invalid tags') + end + end + + context 'when feature flag on_demand_scans_runner_tags is disabled' do + before do + stub_feature_flags(on_demand_scans_runner_tags: false) + end + + it 'returns a success status' do + expect(subject.status).to eq(:success) + end + + it 'creates a dast_profile ignoring the tags' do + expect(subject.payload[:dast_profile].tags).to be_empty + end + end + end end end end diff --git a/ee/spec/services/app_sec/dast/profiles/update_service_spec.rb b/ee/spec/services/app_sec/dast/profiles/update_service_spec.rb index 712c556b7c23ccdd7926def5ca8d14b958afd5f4..c928a04653e1134b474a2cb5bd4c414a47414dfb 100644 --- a/ee/spec/services/app_sec/dast/profiles/update_service_spec.rb +++ b/ee/spec/services/app_sec/dast/profiles/update_service_spec.rb @@ -2,15 +2,20 @@ require 'spec_helper' -RSpec.describe AppSec::Dast::Profiles::UpdateService do +RSpec.describe AppSec::Dast::Profiles::UpdateService, :dynamic_analysis, + feature_category: :dynamic_application_security_testing 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, branch_name: 'orphaned-branch') } + let_it_be(:old_tags) { [ActsAsTaggableOn::Tag.create!(name: 'ruby'), ActsAsTaggableOn::Tag.create!(name: 'postgres')] } + let_it_be(:dast_profile, reload: true) { create(:dast_profile, project: project, branch_name: 'orphaned-branch', tags: old_tags) } 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(:plan_limits) { create(:plan_limits, :default_plan) } let_it_be(:scheduler_owner) { create(:user, name: 'Scheduler Owner') } + let_it_be(:new_tags) { [ActsAsTaggableOn::Tag.create!(name: 'rails'), ActsAsTaggableOn::Tag.create!(name: 'docker')] } + let_it_be(:new_tag_list) { new_tags.map(&:name) } + let(:default_params) do { name: SecureRandom.hex, @@ -26,7 +31,7 @@ subject do described_class.new( - container: project, + project: project, current_user: user, params: params ).execute @@ -277,6 +282,44 @@ end end end + + context 'with tag_list param' do + let(:params) { default_params.merge(tag_list: new_tag_list) } + + it 'updates the tags' do + subject + + expect(dast_profile.tags).to match_array(new_tags) + end + + context 'when there is a invalid tag' do + let(:new_tag_list) { %w[invalid_tag] } + + it 'returns an error status' do + expect(subject.status).to eq(:error) + end + + it 'populates message' do + expect(subject.message).to eq('Invalid tags') + end + end + + context 'when feature flag on_demand_scans_runner_tags is disabled' do + before do + stub_feature_flags(on_demand_scans_runner_tags: false) + end + + it 'returns a success status' do + expect(subject.status).to eq(:success) + end + + it 'does not update the tags' do + updated_profile = dast_profile.reload + + expect(updated_profile.tag_list).to match_array(old_tags.map(&:name)) + end + end + end end end end diff --git a/ee/spec/services/app_sec/dast/scan_configs/build_service_spec.rb b/ee/spec/services/app_sec/dast/scan_configs/build_service_spec.rb index 2556fed0a4fd30f4437b342241cdf78a2b22c825..b26d7a165973fdd2b1b2dca33b0abb860ac1f2bb 100644 --- a/ee/spec/services/app_sec/dast/scan_configs/build_service_spec.rb +++ b/ee/spec/services/app_sec/dast/scan_configs/build_service_spec.rb @@ -45,16 +45,74 @@ shared_examples 'build service execute tests' do context 'when a dast_profile is provided' do let(:params) { { dast_profile: dast_profile } } - - it 'returns a dast_profile, branch and YAML configuration' do - expected_payload = { + let(:expected_payload) do + { dast_profile: dast_profile, branch: dast_profile.branch_name, ci_configuration: expected_yaml_configuration } + end + + shared_examples 'a payload with a dast_profile' do + it 'returns a branch and YAML configuration' do + expected_payload = { + dast_profile: dast_profile, + branch: dast_profile.branch_name, + ci_configuration: expected_yaml_configuration + } + + expect(subject.payload).to eq(expected_payload) + end + end + it 'returns a dast_profile, branch and YAML configuration' do expect(subject.payload).to eq(expected_payload) end + + context 'when the dast_profile has tag_list' do + context 'when feature flag on_demand_scans_runner_tags is disabled' do + before do + stub_feature_flags(on_demand_scans_runner_tags: false) + end + + it_behaves_like 'a payload with a dast_profile' + end + + context 'when feature flag on_demand_scans_runner_tags is enabled' do + let_it_be(:tags) { [ActsAsTaggableOn::Tag.create!(name: 'ruby'), ActsAsTaggableOn::Tag.create!(name: 'postgres')] } + let_it_be(:dast_profile) do + create(:dast_profile, + project: project, + dast_site_profile: dast_site_profile, + dast_scanner_profile: dast_scanner_profile, + branch_name: 'master', + tags: tags) + end + + let(:expected_yaml_configuration) do + <<~YAML + --- + stages: + - dast + include: + - template: #{template} + dast: + dast_configuration: + site_profile: #{dast_site_profile.name} + scanner_profile: #{dast_scanner_profile.name} + tags: + - ruby + - postgres + YAML + end + + it_behaves_like 'a payload with a dast_profile' + end + end + + context 'when the scanner profile has no runner tags' do + it_behaves_like 'a payload with a dast_profile' + end end context 'when a dast_site_profile is provided' do @@ -85,44 +143,6 @@ expect(subject.message).to eq('Cannot run active scan against unvalidated target') end end - - context 'when the dast_scanner_profile has tag_list' do - context 'when feature flag on_demand_scans_runner_tags is disabled' do - before do - stub_feature_flags(on_demand_scans_runner_tags: false) - end - - it_behaves_like 'a payload without a dast_profile' - end - - context 'when feature flag on_demand_scans_runner_tags is enabled' do - let_it_be(:tags) { [ActsAsTaggableOn::Tag.create!(name: 'ruby'), ActsAsTaggableOn::Tag.create!(name: 'postgres')] } - let_it_be(:dast_scanner_profile) { create(:dast_scanner_profile, project: project, tags: tags) } - - let(:expected_yaml_configuration) do - <<~YAML - --- - stages: - - dast - include: - - template: #{template} - dast: - dast_configuration: - site_profile: #{dast_site_profile.name} - scanner_profile: #{dast_scanner_profile.name} - tags: - - ruby - - postgres - YAML - end - - it_behaves_like 'a payload without a dast_profile' - end - end - - context 'when the scanner profile has no runner tags' do - it_behaves_like 'a payload without a dast_profile' - end end context 'when a dast_scanner_profile is not provided' do @@ -163,6 +183,12 @@ end end + context 'when the target_type is NOT api' do + let(:template) { on_demand_scan_template } + + it_behaves_like 'build service execute tests' + end + context 'when the target_type is api' do before do dast_site_profile.target_type = 'api' @@ -172,11 +198,5 @@ it_behaves_like 'build service execute tests' end - - context 'when the target_type is NOT api' do - let(:template) { on_demand_scan_template } - - it_behaves_like 'build service execute tests' - end end end diff --git a/ee/spec/services/app_sec/dast/scanner_profiles/create_service_spec.rb b/ee/spec/services/app_sec/dast/scanner_profiles/create_service_spec.rb index f3bcf86061bdf11e186e98f4b6f46f5ab3625d04..683fc2d0d33c9170f2b20e46916f9aa7a2d5ba9a 100644 --- a/ee/spec/services/app_sec/dast/scanner_profiles/create_service_spec.rb +++ b/ee/spec/services/app_sec/dast/scanner_profiles/create_service_spec.rb @@ -12,8 +12,7 @@ let(:scan_type) { 1 } let(:use_ajax_spider) { true } let(:show_debug_messages) { true } - let(:tags) { [ActsAsTaggableOn::Tag.create!(name: 'ruby'), ActsAsTaggableOn::Tag.create!(name: 'postgres')] } - let(:tag_list) { tags.map(&:name) } + let(:params) do { name: name, @@ -21,8 +20,7 @@ spider_timeout: spider_timeout, scan_type: scan_type, use_ajax_spider: use_ajax_spider, - show_debug_messages: show_debug_messages, - tag_list: tag_list + show_debug_messages: show_debug_messages } end @@ -86,7 +84,6 @@ expect(DastScannerProfile.scan_types[payload.scan_type]).to eq(scan_type) expect(payload.use_ajax_spider).to eq(use_ajax_spider) expect(payload.show_debug_messages).to eq(show_debug_messages) - expect(payload.tags).to match_array(tags) end end @@ -147,36 +144,6 @@ expect(message).to eq('Insufficient permissions') end end - - context 'when there is a invalid tag' do - let(:tag_list) { %w[invalid_tag] } - - it 'does not create a new dast_scanner_profile' do - expect { subject }.not_to change(DastScannerProfile, :count) - end - - it 'returns an error status' do - expect(status).to eq(:error) - end - - it 'populates message' do - expect(message).to eq('Invalid tag_list') - end - end - - context 'when feature flag on_demand_scans_runner_tags is disabled' do - before do - stub_feature_flags(on_demand_scans_runner_tags: false) - end - - it 'returns a success status' do - expect(status).to eq(:success) - end - - it 'creates a dast_scanner_profile ignoring the tags' do - expect(payload.tags).to be_empty - end - end end end end diff --git a/ee/spec/services/app_sec/dast/scanner_profiles/update_service_spec.rb b/ee/spec/services/app_sec/dast/scanner_profiles/update_service_spec.rb index 3acfc744848e8bc2c8ef6e853502f0a899a5a73a..a1fa1bf461624e5571a32975bd064725c9c3e052 100644 --- a/ee/spec/services/app_sec/dast/scanner_profiles/update_service_spec.rb +++ b/ee/spec/services/app_sec/dast/scanner_profiles/update_service_spec.rb @@ -5,8 +5,7 @@ RSpec.describe AppSec::Dast::ScannerProfiles::UpdateService, :dynamic_analysis, feature_category: :dynamic_application_security_testing do let_it_be(:user) { create(:user) } - let_it_be(:old_tags) { [ActsAsTaggableOn::Tag.create!(name: 'ruby'), ActsAsTaggableOn::Tag.create!(name: 'postgres')] } - let_it_be(:dast_profile, reload: true) { create(:dast_scanner_profile, target_timeout: 200, spider_timeout: 5000, tags: old_tags) } + let_it_be(:dast_profile, reload: true) { create(:dast_scanner_profile, target_timeout: 200, spider_timeout: 5000) } let_it_be(:dast_profile_2, reload: true) { create(:dast_scanner_profile) } let(:project) { dast_profile.project } let(:project_2) { dast_profile_2.project } @@ -19,9 +18,6 @@ let(:new_use_ajax_spider) { !dast_profile.use_ajax_spider } let(:new_show_debug_messages) { !dast_profile.show_debug_messages } - let_it_be(:new_tags) { [ActsAsTaggableOn::Tag.create!(name: 'rails'), ActsAsTaggableOn::Tag.create!(name: 'docker')] } - let_it_be(:new_tag_list) { new_tags.map(&:name) } - let(:params) do { id: dast_scanner_profile_id, @@ -30,8 +26,7 @@ spider_timeout: new_spider_timeout, scan_type: new_scan_type, use_ajax_spider: new_use_ajax_spider, - show_debug_messages: new_show_debug_messages, - tag_list: new_tag_list + show_debug_messages: new_show_debug_messages } end @@ -71,8 +66,7 @@ spider_timeout: new_spider_timeout, scan_type: new_scan_type, use_ajax_spider: new_use_ajax_spider, - show_debug_messages: new_show_debug_messages, - tag_list: new_tag_list + show_debug_messages: new_show_debug_messages } end @@ -163,7 +157,6 @@ expect(updated_dast_scanner_profile.scan_type).to eq(new_scan_type) expect(updated_dast_scanner_profile.use_ajax_spider).to eq(new_use_ajax_spider) expect(updated_dast_scanner_profile.show_debug_messages).to eq(new_show_debug_messages) - expect(updated_dast_scanner_profile.tags).to match_array(new_tags) end end @@ -259,34 +252,6 @@ end include_examples 'restricts modification if referenced by policy', :modify - - context 'when there is a invalid tag' do - let(:new_tag_list) { %w[invalid_tag] } - - it 'returns an error status' do - expect(status).to eq(:error) - end - - it 'populates message' do - expect(message).to eq('Invalid tag_list') - end - end - - context 'when feature flag on_demand_scans_runner_tags is disabled' do - before do - stub_feature_flags(on_demand_scans_runner_tags: false) - end - - it 'returns a success status' do - expect(status).to eq(:success) - end - - it 'does not update the tags' do - updated_dast_scanner_profile = payload.reload - - expect(updated_dast_scanner_profile.tag_list).to match_array(old_tags.map(&:name)) - end - end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 729011c59cc6a76feda220f84fc2de2893f90dda..d0a0c961f56109332130f7e861b4b895b141ae7d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -22832,6 +22832,9 @@ msgstr "" msgid "Invalid status" msgstr "" +msgid "Invalid tags" +msgstr "" + msgid "Invalid two-factor code." msgstr ""