diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-08-30 22:45:17 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-08-30 22:45:17 +0300 |
commit | 1cad287a7b40174786cadaecea9c91a68e49fcba (patch) | |
tree | 7cdc2447c143cec003eb7c0e42a324f26902bc5d | |
parent | 1fb0bae24e6686b3571fc1c44cbf239d8563e0d7 (diff) |
Add latest changes from gitlab-org/security/gitlab@16-3-stable-ee
17 files changed, 334 insertions, 131 deletions
diff --git a/app/controllers/groups/labels_controller.rb b/app/controllers/groups/labels_controller.rb index 57bca5ebc52..f927cae90b1 100644 --- a/app/controllers/groups/labels_controller.rb +++ b/app/controllers/groups/labels_controller.rb @@ -4,7 +4,8 @@ class Groups::LabelsController < Groups::ApplicationController include ToggleSubscriptionAction before_action :label, only: [:edit, :update, :destroy] - before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update, :destroy] + before_action :authorize_group_for_admin_labels!, only: [:new, :create, :edit, :update, :destroy] + before_action :authorize_label_for_admin_label!, only: [:edit, :update, :destroy] before_action :save_previous_label_path, only: [:edit] respond_to :html @@ -75,10 +76,14 @@ class Groups::LabelsController < Groups::ApplicationController protected - def authorize_admin_labels! + def authorize_group_for_admin_labels! return render_404 unless can?(current_user, :admin_label, @group) end + def authorize_label_for_admin_label! + return render_404 unless can?(current_user, :admin_label, @label) + end + def authorize_read_labels! return render_404 unless can?(current_user, :read_label, @group) end diff --git a/app/models/bulk_imports/entity.rb b/app/models/bulk_imports/entity.rb index 4f50a112141..644673e249e 100644 --- a/app/models/bulk_imports/entity.rb +++ b/app/models/bulk_imports/entity.rb @@ -41,19 +41,15 @@ class BulkImports::Entity < ApplicationRecord validates :project, absence: true, if: :group validates :group, absence: true, if: :project validates :source_type, presence: true - validates :source_full_path, presence: true, format: { - with: Gitlab::Regex.bulk_import_source_full_path_regex, - message: Gitlab::Regex.bulk_import_source_full_path_regex_message - } - + validates :source_full_path, presence: true validates :destination_name, presence: true, if: -> { group || project } validates :destination_namespace, exclusion: [nil], if: :group validates :destination_namespace, presence: true, if: :project? validate :validate_parent_is_a_group, if: :parent validate :validate_imported_entity_type - validate :validate_destination_namespace_ascendency, if: :group_entity? + validate :validate_source_full_path_format enum source_type: { group_entity: 0, project_entity: 1 } @@ -236,4 +232,15 @@ class BulkImports::Entity < ApplicationRecord ) end end + + def validate_source_full_path_format + validator = group? ? NamespacePathValidator : ProjectPathValidator + + return if validator.valid_path?(source_full_path) + + errors.add( + :source_full_path, + Gitlab::Regex.bulk_import_source_full_path_regex_message + ) + end end diff --git a/lib/api/validations/validators/bulk_imports.rb b/lib/api/validations/validators/bulk_imports.rb index f8ad5ed6d14..67dc084cc12 100644 --- a/lib/api/validations/validators/bulk_imports.rb +++ b/lib/api/validations/validators/bulk_imports.rb @@ -32,8 +32,7 @@ module API class DestinationNamespacePath < Grape::Validations::Validators::Base def validate_param!(attr_name, params) return if params[attr_name].blank? - - return if params[attr_name] =~ Gitlab::Regex.bulk_import_destination_namespace_path_regex + return if NamespacePathValidator.valid_path?(params[attr_name]) raise Grape::Exceptions::Validation.new( params: [@scope.full_name(attr_name)], @@ -44,7 +43,10 @@ module API class SourceFullPath < Grape::Validations::Validators::Base def validate_param!(attr_name, params) - return if params[attr_name] =~ Gitlab::Regex.bulk_import_source_full_path_regex + full_path = params[attr_name] + + return if params['source_type'] == 'group_entity' && NamespacePathValidator.valid_path?(full_path) + return if params['source_type'] == 'project_entity' && ProjectPathValidator.valid_path?(full_path) raise Grape::Exceptions::Validation.new( params: [@scope.full_name(attr_name)], diff --git a/lib/gitlab/regex/bulk_imports.rb b/lib/gitlab/regex/bulk_imports.rb index e9ec24b831f..65c23b9d2e6 100644 --- a/lib/gitlab/regex/bulk_imports.rb +++ b/lib/gitlab/regex/bulk_imports.rb @@ -3,28 +3,6 @@ module Gitlab module Regex module BulkImports - def bulk_import_destination_namespace_path_regex - # This regexp validates the string conforms to rules for a destination_namespace path: - # i.e does not start with a non-alphanumeric character, - # contains only alphanumeric characters, forward slashes, periods, and underscores, - # does not end with a period or forward slash, and has a relative path structure - # with no http protocol chars or leading or trailing forward slashes - # eg 'source/full/path' or 'destination_namespace' not 'https://example.com/destination/namespace/path' - # the regex also allows for an empty string ('') to be accepted as this is allowed in - # a bulk_import POST request - @bulk_import_destination_namespace_path_regex ||= %r/((\A\z)|(\A[0-9a-z]*(-_.)?[0-9a-z])(\/?[0-9a-z]*[-_.]?[0-9a-z])+\z)/i - end - - def bulk_import_source_full_path_regex - # This regexp validates the string conforms to rules for a source_full_path path: - # i.e does not start with a non-alphanumeric character except for periods or underscores, - # contains only alphanumeric characters, forward slashes, periods, and underscores, - # does not end with a period or forward slash, and has a relative path structure - # with no http protocol chars or leading or trailing forward slashes - # eg 'source/full/path' or 'destination_namespace' not 'https://example.com/source/full/path' - @bulk_import_source_full_path_regex ||= %r/\A([.]?)[^\W](\/?([-_.+]*)*[0-9a-z][-_]*)+\z/i - end - def bulk_import_source_full_path_regex_message bulk_import_destination_namespace_path_regex_message end diff --git a/spec/controllers/groups/labels_controller_spec.rb b/spec/controllers/groups/labels_controller_spec.rb index ebe379d948c..f9f1fc21538 100644 --- a/spec/controllers/groups/labels_controller_spec.rb +++ b/spec/controllers/groups/labels_controller_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Groups::LabelsController, feature_category: :team_planning do let_it_be(:group) { create(:group) } let_it_be(:user) { create(:user) } + let_it_be(:another_user) { create(:user) } let_it_be(:project) { create(:project, namespace: group) } before do @@ -66,6 +67,46 @@ RSpec.describe Groups::LabelsController, feature_category: :team_planning do end end + shared_examples 'when current_user does not have ability to modify the label' do + before do + sign_in(another_user) + end + + it 'responds with status 404' do + group_request + + expect(response).to have_gitlab_http_status(:not_found) + end + + # No matter what permissions you have in a sub-group, you need the proper + # permissions in the group in order to modify a group label + # See https://gitlab.com/gitlab-org/gitlab/-/issues/387531 + context 'when trying to edit a parent group label from inside a subgroup' do + it 'responds with status 404' do + sub_group.add_owner(another_user) + sub_group_request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'GET #edit' do + let_it_be(:label) { create(:group_label, group: group) } + + it 'shows the edit page' do + get :edit, params: { group_id: group.to_param, id: label.to_param } + + expect(response).to have_gitlab_http_status(:ok) + end + + it_behaves_like 'when current_user does not have ability to modify the label' do + let_it_be(:sub_group) { create(:group, parent: group) } + let(:group_request) { get :edit, params: { group_id: group.to_param, id: label.to_param } } + let(:sub_group_request) { get :edit, params: { group_id: sub_group.to_param, id: label.to_param } } + end + end + describe 'POST #toggle_subscription' do it 'allows user to toggle subscription on group labels' do label = create(:group_label, group: group) @@ -106,19 +147,20 @@ RSpec.describe Groups::LabelsController, feature_category: :team_planning do end end - context 'when current_user does not have ability to destroy the label' do - let(:another_user) { create(:user) } - - before do - sign_in(another_user) - end - - it 'responds with status 404' do - label = create(:group_label, group: group) - delete :destroy, params: { group_id: group.to_param, id: label.to_param } + it_behaves_like 'when current_user does not have ability to modify the label' do + let_it_be(:label) { create(:group_label, group: group) } + let_it_be(:sub_group) { create(:group, parent: group) } + let(:group_request) { delete :destroy, params: { group_id: group.to_param, id: label.to_param } } + let(:sub_group_request) { delete :destroy, params: { group_id: sub_group.to_param, id: label.to_param } } + end + end - expect(response).to have_gitlab_http_status(:not_found) - end + describe 'PUT #update' do + it_behaves_like 'when current_user does not have ability to modify the label' do + let_it_be(:label) { create(:group_label, group: group) } + let_it_be(:sub_group) { create(:group, parent: group) } + let(:group_request) { put :update, params: { group_id: group.to_param, id: label.to_param, label: { title: 'Test' } } } + let(:sub_group_request) { put :update, params: { group_id: sub_group.to_param, id: label.to_param, label: { title: 'Test' } } } end end end diff --git a/spec/factories/bulk_import/entities.rb b/spec/factories/bulk_import/entities.rb index a662d42c7c9..04407947f14 100644 --- a/spec/factories/bulk_import/entities.rb +++ b/spec/factories/bulk_import/entities.rb @@ -18,6 +18,7 @@ FactoryBot.define do trait(:project_entity) do source_type { :project_entity } + sequence(:source_full_path) { |n| "root/source-path-#{n}" } end trait :created do diff --git a/spec/lib/api/validations/validators/bulk_imports/destination_namespace_path_spec.rb b/spec/lib/api/validations/validators/bulk_imports/destination_namespace_path_spec.rb new file mode 100644 index 00000000000..39432c724a0 --- /dev/null +++ b/spec/lib/api/validations/validators/bulk_imports/destination_namespace_path_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Validations::Validators::BulkImports::DestinationNamespacePath, feature_category: :importers do + include ApiValidatorsHelpers + + subject do + described_class.new(['test'], {}, false, scope.new) + end + + context 'when destination namespace param is valid' do + it 'raises a validation error', :aggregate_failures do + expect_validation_error('test' => '?gitlab') + expect_validation_error('test' => "Users's something") + expect_validation_error('test' => '/source') + expect_validation_error('test' => 'http:') + expect_validation_error('test' => 'https:') + expect_validation_error('test' => 'example.com/?stuff=true') + expect_validation_error('test' => 'example.com:5000/?stuff=true') + expect_validation_error('test' => 'http://gitlab.example/gitlab-org/manage/import/gitlab-migration-test') + expect_validation_error('test' => 'good_for_me!') + expect_validation_error('test' => 'good_for+you') + expect_validation_error('test' => 'source/') + expect_validation_error('test' => '.source/full./path') + end + end + + context 'when destination namespace param is invalid' do + it 'does not raise a validation error', :aggregate_failures do + expect_no_validation_error('') + expect_no_validation_error('test' => '') + expect_no_validation_error('test' => 'source') + expect_no_validation_error('test' => 'source/full') + expect_no_validation_error('test' => 'source/full/path') + expect_no_validation_error('test' => 'sou_rce/fu-ll/pa.th') + expect_no_validation_error('test' => 'domain_namespace') + expect_no_validation_error('test' => 'gitlab-migration-test') + expect_no_validation_error('test' => '1-project-path') + expect_no_validation_error('test' => 'e-project-path') + end + end +end diff --git a/spec/lib/api/validations/validators/bulk_imports/source_full_path_validator_spec.rb b/spec/lib/api/validations/validators/bulk_imports/source_full_path_validator_spec.rb new file mode 100644 index 00000000000..f0d2121b0d6 --- /dev/null +++ b/spec/lib/api/validations/validators/bulk_imports/source_full_path_validator_spec.rb @@ -0,0 +1,168 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Validations::Validators::BulkImports::SourceFullPath, feature_category: :importers do + include ApiValidatorsHelpers + using RSpec::Parameterized::TableSyntax + + subject do + described_class.new(['test'], {}, false, scope.new) + end + + let(:source_type_params) { { 'source_type' => source_type } } + + context 'when source_type is group_entity' do + let(:source_type) { 'group_entity' } + + context 'when source_full_path param is invalid' do + where(:invalid_param) do + [ + '', + '?gitlab', + "Users's something", + '/source', + 'http:', + 'https:', + 'example.com/?stuff=true', + 'example.com:5000/?stuff=true', + 'http://gitlab.example/gitlab-org/manage/import/gitlab-migration-test', + 'good_for_me!', + 'good_for+you', + 'source/', + '.source/full/path.' + ] + end + + with_them do + it 'raises a validation error' do + params = source_type_params.merge('test' => invalid_param) + + expect_validation_error(params) + end + end + end + + context 'when source_full_path param is valid' do + where(:valid_param) do + [ + 'source', + 'source/full', + 'source/full/path', + 'sou_rce/fu-ll/pa.th', + 'source/full/path---', + 'source/full/..path', + 'domain_namespace', + 'gitlab-migration-test', + '1-project-path', + 'e-project-path' + ] + end + + with_them do + it 'does not raise a validation error' do + params = source_type_params.merge('test' => valid_param) + + expect_no_validation_error(params) + end + end + end + end + + context 'when source_type is project_entity' do + let(:source_type) { 'project_entity' } + + context 'when source_full_path param is invalid' do + where(:invalid_param) do + [ + '', + '?gitlab', + "Users's something", + '/source', + 'http:', + 'https:', + 'example.com/?stuff=true', + 'example.com:5000/?stuff=true', + 'http://gitlab.example/gitlab-org/manage/import/gitlab-migration-test', + 'good_for_me!', + 'good_for+you', + 'source/', + 'source', + '.source/full./path', + 'domain_namespace', + 'gitlab-migration-test', + '1-project-path', + 'e-project-path' + ] + end + + with_them do + it 'raises a validation error' do + params = source_type_params.merge('test' => invalid_param) + + expect_validation_error(params) + end + end + end + + context 'when source_full_path param is valid' do + where(:valid_param) do + [ + 'source/full', + 'source/full/path', + 'sou_rce/fu-ll/pa.th', + 'source/full/path---', + 'source/full/..path' + ] + end + + with_them do + it 'does not raise a validation error' do + params = source_type_params.merge('test' => valid_param) + + expect_no_validation_error(params) + end + end + end + end + + context 'when source_type is invalid' do + let(:source_type) { '' } + + context 'when source_full_path param is invalid' do + where(:invalid_param) do + [ + '', + '?gitlab', + "Users's something", + '/source', + 'http:', + 'https:', + 'example.com/?stuff=true', + 'example.com:5000/?stuff=true', + 'http://gitlab.example/gitlab-org/manage/import/gitlab-migration-test', + 'good_for_me!', + 'good_for+you', + 'source/', + '.source/full./path', + 'source', + 'source/full', + 'source/full/path', + 'sou_rce/fu-ll/pa.th', + 'domain_namespace', + 'gitlab-migration-test', + '1-project-path', + 'e-project-path' + ] + end + + with_them do + it 'raises a validation error' do + params = source_type_params.merge('test' => invalid_param) + + expect_validation_error(params) + end + end + end + end +end diff --git a/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb b/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb index 50640e1e4ba..c91b031de30 100644 --- a/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb +++ b/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb @@ -7,7 +7,7 @@ RSpec.describe BulkImports::Common::Pipelines::LfsObjectsPipeline, feature_categ let_it_be(:oid) { 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' } let(:tmpdir) { Dir.mktmpdir } - let(:entity) { create(:bulk_import_entity, :project_entity, project: portable, source_full_path: 'test', source_xid: nil) } + let(:entity) { create(:bulk_import_entity, :project_entity, project: portable, source_xid: nil) } let(:tracker) { create(:bulk_import_tracker, entity: entity) } let(:context) { BulkImports::Pipeline::Context.new(tracker) } let(:lfs_dir_path) { tmpdir } @@ -53,7 +53,7 @@ RSpec.describe BulkImports::Common::Pipelines::LfsObjectsPipeline, feature_categ .to receive(:new) .with( configuration: context.configuration, - relative_url: "/#{entity.pluralized_name}/test/export_relations/download?relation=lfs_objects", + relative_url: "/#{entity.pluralized_name}/#{CGI.escape(entity.source_full_path)}/export_relations/download?relation=lfs_objects", tmpdir: tmpdir, filename: 'lfs_objects.tar.gz') .and_return(download_service) diff --git a/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb b/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb index 09c8c7b92c2..675ae935c1c 100644 --- a/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb +++ b/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb @@ -79,7 +79,7 @@ RSpec.describe BulkImports::Common::Pipelines::UploadsPipeline, feature_category .to receive(:new) .with( configuration: context.configuration, - relative_url: "/#{entity.pluralized_name}/test/export_relations/download?relation=uploads", + relative_url: "/#{entity.pluralized_name}/#{CGI.escape(entity.source_full_path)}/export_relations/download?relation=uploads", tmpdir: tmpdir, filename: 'uploads.tar.gz') .and_return(download_service) @@ -178,14 +178,14 @@ RSpec.describe BulkImports::Common::Pipelines::UploadsPipeline, feature_category context 'when importing to group' do let(:portable) { group } - let(:entity) { create(:bulk_import_entity, :group_entity, group: group, source_full_path: 'test', source_xid: nil) } + let(:entity) { create(:bulk_import_entity, :group_entity, group: group, source_xid: nil) } include_examples 'uploads import' end context 'when importing to project' do let(:portable) { project } - let(:entity) { create(:bulk_import_entity, :project_entity, project: project, source_full_path: 'test', source_xid: nil) } + let(:entity) { create(:bulk_import_entity, :project_entity, project: project, source_xid: nil) } include_examples 'uploads import' end diff --git a/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb b/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb index 87efad92131..e65339ffdd0 100644 --- a/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb +++ b/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb @@ -9,7 +9,7 @@ RSpec.describe BulkImports::Projects::Pipelines::DesignBundlePipeline, feature_c let(:tmpdir) { Dir.mktmpdir } let(:design_bundle_path) { File.join(tmpdir, 'design.bundle') } let(:entity) do - create(:bulk_import_entity, :project_entity, project: portable, source_full_path: 'test', source_xid: nil) + create(:bulk_import_entity, :project_entity, project: portable, source_xid: nil) end let(:tracker) { create(:bulk_import_tracker, entity: entity) } @@ -52,7 +52,8 @@ RSpec.describe BulkImports::Projects::Pipelines::DesignBundlePipeline, feature_c .to receive(:new) .with( configuration: context.configuration, - relative_url: "/#{entity.pluralized_name}/test/export_relations/download?relation=design", + relative_url: "/#{entity.pluralized_name}/#{CGI.escape(entity.source_full_path)}" \ + '/export_relations/download?relation=design', tmpdir: tmpdir, filename: 'design.tar.gz') .and_return(download_service) diff --git a/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb b/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb index 5aae8c959fa..2865215823a 100644 --- a/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb +++ b/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb @@ -9,7 +9,7 @@ RSpec.describe BulkImports::Projects::Pipelines::RepositoryBundlePipeline, featu let(:tmpdir) { Dir.mktmpdir } let(:bundle_path) { File.join(tmpdir, 'repository.bundle') } let(:entity) do - create(:bulk_import_entity, :project_entity, project: portable, source_full_path: 'test', source_xid: nil) + create(:bulk_import_entity, :project_entity, project: portable, source_xid: nil) end let(:tracker) { create(:bulk_import_tracker, entity: entity) } @@ -62,7 +62,8 @@ RSpec.describe BulkImports::Projects::Pipelines::RepositoryBundlePipeline, featu .to receive(:new) .with( configuration: context.configuration, - relative_url: "/#{entity.pluralized_name}/test/export_relations/download?relation=repository", + relative_url: "/#{entity.pluralized_name}/#{CGI.escape(entity.source_full_path)}" \ + '/export_relations/download?relation=repository', tmpdir: tmpdir, filename: 'repository.tar.gz') .and_return(download_service) diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index c91b99caba2..df123ef638f 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -98,36 +98,6 @@ RSpec.describe Gitlab::Regex, feature_category: :tooling do } end - describe '.bulk_import_destination_namespace_path_regex' do - subject { described_class.bulk_import_destination_namespace_path_regex } - - it { is_expected.not_to match('?gitlab') } - it { is_expected.not_to match("Users's something") } - it { is_expected.not_to match('/source') } - it { is_expected.not_to match('http:') } - it { is_expected.not_to match('https:') } - it { is_expected.not_to match('example.com/?stuff=true') } - it { is_expected.not_to match('example.com:5000/?stuff=true') } - it { is_expected.not_to match('http://gitlab.example/gitlab-org/manage/import/gitlab-migration-test') } - it { is_expected.not_to match('_good_for_me!') } - it { is_expected.not_to match('good_for+you') } - it { is_expected.not_to match('source/') } - it { is_expected.not_to match('.source/full./path') } - it { is_expected.not_to match('.source/.full/.path') } - it { is_expected.not_to match('_source') } - it { is_expected.not_to match('.source') } - - it { is_expected.to match('source') } - it { is_expected.to match('source/full') } - it { is_expected.to match('source/full/path') } - it { is_expected.to match('sou_rce/fu-ll/pa.th') } - it { is_expected.to match('domain_namespace') } - it { is_expected.to match('gitlab-migration-test') } - it { is_expected.to match('1-project-path') } - it { is_expected.to match('e-project-path') } - it { is_expected.to match('') } # it is possible to pass an empty string for destination_namespace in bulk_import POST request - end - describe '.bulk_import_source_full_path_regex_message' do subject { described_class.bulk_import_source_full_path_regex_message } @@ -141,44 +111,6 @@ RSpec.describe Gitlab::Regex, feature_category: :tooling do } end - describe '.bulk_import_source_full_path_regex' do - subject { described_class.bulk_import_source_full_path_regex } - - it { is_expected.not_to match("Users's something") } - it { is_expected.not_to match('/source') } - it { is_expected.not_to match('http:') } - it { is_expected.not_to match('https:') } - it { is_expected.not_to match('example.com/?stuff=true') } - it { is_expected.not_to match('example.com:5000/?stuff=true') } - it { is_expected.not_to match('http://gitlab.example/gitlab-org/manage/import/gitlab-migration-test') } - it { is_expected.not_to match('source/') } - it { is_expected.not_to match('') } - it { is_expected.not_to match('.source/full./path') } - it { is_expected.not_to match('?gitlab') } - it { is_expected.not_to match('_good_for_me!') } - it { is_expected.not_to match('group/@*%_my_other-project-----') } - it { is_expected.not_to match('_foog-for-me!') } - it { is_expected.not_to match('.source/full/path.') } - - it { is_expected.to match('good_for+you') } - it { is_expected.to match('source') } - it { is_expected.to match('.source') } - it { is_expected.to match('_source') } - it { is_expected.to match('source/full') } - it { is_expected.to match('source/full/path') } - it { is_expected.to match('domain_namespace') } - it { is_expected.to match('gitlab-migration-test') } - it { is_expected.to match('source/full/path-') } - it { is_expected.to match('.source/full/path') } - it { is_expected.to match('.source/.full/.path') } - it { is_expected.to match('source/full/.path') } - it { is_expected.to match('source/full/..path') } - it { is_expected.to match('source/full/---1path') } - it { is_expected.to match('source/full/-___path') } - it { is_expected.to match('source/full/path---') } - it { is_expected.to match('group/__my_other-project-----') } - end - describe '.group_path_regex' do subject { described_class.group_path_regex } diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/count_bulk_imports_entities_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/count_bulk_imports_entities_metric_spec.rb index 0deb586d488..f19e83935f8 100644 --- a/spec/lib/gitlab/usage/metrics/instrumentations/count_bulk_imports_entities_metric_spec.rb +++ b/spec/lib/gitlab/usage/metrics/instrumentations/count_bulk_imports_entities_metric_spec.rb @@ -5,17 +5,17 @@ require 'spec_helper' RSpec.describe Gitlab::Usage::Metrics::Instrumentations::CountBulkImportsEntitiesMetric, feature_category: :importers do let_it_be(:user) { create(:user) } let_it_be(:bulk_import_projects) do - create_list(:bulk_import_entity, 2, source_type: 'project_entity', created_at: 3.weeks.ago, status: 2) - create(:bulk_import_entity, source_type: 'project_entity', created_at: 3.weeks.ago, status: 0) + create_list(:bulk_import_entity, 2, :project_entity, created_at: 3.weeks.ago, status: 2) + create(:bulk_import_entity, :project_entity, created_at: 3.weeks.ago, status: 0) end let_it_be(:bulk_import_groups) do - create_list(:bulk_import_entity, 2, source_type: 'group_entity', created_at: 3.weeks.ago, status: 2) - create(:bulk_import_entity, source_type: 'group_entity', created_at: 3.weeks.ago, status: 0) + create_list(:bulk_import_entity, 2, :group_entity, created_at: 3.weeks.ago, status: 2) + create(:bulk_import_entity, :group_entity, created_at: 3.weeks.ago, status: 0) end let_it_be(:old_bulk_import_project) do - create(:bulk_import_entity, source_type: 'project_entity', created_at: 2.months.ago, status: 2) + create(:bulk_import_entity, :project_entity, created_at: 2.months.ago, status: 2) end context 'with no source_type' do diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_total_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_total_metric_spec.rb index bd432b614e7..bf3cd810e23 100644 --- a/spec/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_total_metric_spec.rb +++ b/spec/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_total_metric_spec.rb @@ -15,15 +15,15 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::CountImportedProjectsTo let_it_be(:old_import) { create(:project, import_type: 'gitea', creator_id: user.id, created_at: 2.months.ago) } let_it_be(:bulk_import_projects) do - create_list(:bulk_import_entity, 3, source_type: 'project_entity', created_at: 3.weeks.ago) + create_list(:bulk_import_entity, 3, :project_entity, created_at: 3.weeks.ago) end let_it_be(:bulk_import_groups) do - create_list(:bulk_import_entity, 3, source_type: 'group_entity', created_at: 3.weeks.ago) + create_list(:bulk_import_entity, 3, :group_entity, created_at: 3.weeks.ago) end let_it_be(:old_bulk_import_project) do - create(:bulk_import_entity, source_type: 'project_entity', created_at: 2.months.ago) + create(:bulk_import_entity, :project_entity, created_at: 2.months.ago) end before do diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index 7179ed7cb42..da1eb12e9b8 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -24,11 +24,20 @@ RSpec.describe BulkImports::Entity, type: :model, feature_category: :importers d it { is_expected.to define_enum_for(:source_type).with_values(%i[group_entity project_entity]) } - context 'when formatting with regexes' do + context 'when source_type is group_entity' do + subject { build(:bulk_import_entity, :group_entity) } + it { is_expected.to allow_values('source', 'source/path', 'source/full/path').for(:source_full_path) } it { is_expected.not_to allow_values('/source', 'http://source/path', 'sou rce/full/path', '').for(:source_full_path) } end + context 'when source_type is project_entity' do + subject { build(:bulk_import_entity, :project_entity) } + + it { is_expected.to allow_values('source/path', 'source/full/path').for(:source_full_path) } + it { is_expected.not_to allow_values('/source', 'source', 'http://source/path', 'sou rce/full/path', '').for(:source_full_path) } + end + context 'when associated with a group and project' do it 'is invalid' do entity = build(:bulk_import_entity, group: build(:group), project: build(:project)) diff --git a/spec/requests/api/bulk_imports_spec.rb b/spec/requests/api/bulk_imports_spec.rb index b159d4ad445..fdbfbf052d0 100644 --- a/spec/requests/api/bulk_imports_spec.rb +++ b/spec/requests/api/bulk_imports_spec.rb @@ -248,6 +248,20 @@ RSpec.describe API::BulkImports, feature_category: :importers do end end + context 'when the destination_namespace is invalid' do + it 'returns invalid error' do + params[:entities][0][:destination_namespace] = 'dest?nation-namespace' + + request + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to include('entities[0][destination_namespace] must have a relative path ' \ + 'structure with no HTTP protocol characters, or leading or ' \ + 'trailing forward slashes. Path segments must not start or end ' \ + 'with a special character, and must not contain consecutive ' \ + 'special characters.') + end + end + context 'when the destination_slug is invalid' do it 'returns invalid error when restricting special characters is disabled' do Feature.disable(:restrict_special_characters_in_namespace_path) |