diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index f3c4244269d408a22c061709fa961e0a3404be0b..02e9abb2ced40a868d8422365ffb23aee0d34fe0 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -99,6 +99,7 @@ def allowed_group_params :two_factor_grace_period, :enabled_git_access_protocol, :project_creation_level, + :project_import_level, :subgroup_creation_level, admin_note_attributes: [ :note diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 8963b84e6d8f01b0f53c6e00dfed2d07cf3c0aca..1cafa8857523486191cdbf28af90999a42323c1d 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -292,6 +292,7 @@ def group_params_attributes :two_factor_grace_period, :enabled_git_access_protocol, :project_creation_level, + :project_import_level, :subgroup_creation_level, :default_branch_protection, :default_branch_name, diff --git a/app/finders/groups/user_groups_finder.rb b/app/finders/groups/user_groups_finder.rb index b58c1323b1fcaeade69e3e475cc2622c41494a75..86ba7f0c1c1a4daf6d40897214ac14b23b22877f 100644 --- a/app/finders/groups/user_groups_finder.rb +++ b/app/finders/groups/user_groups_finder.rb @@ -39,6 +39,8 @@ def by_permission_scope target_user.manageable_groups(include_groups_with_developer_maintainer_access: true) elsif permission_scope_transfer_projects? Groups::AcceptingProjectTransfersFinder.new(target_user).execute # rubocop: disable CodeReuse/Finder + elsif permission_scope_import_projects? + target_user.import_groups else target_user.groups end @@ -51,5 +53,9 @@ def permission_scope_create_projects? def permission_scope_transfer_projects? params[:permission_scope] == :transfer_projects end + + def permission_scope_import_projects? + params[:permission_scope] == :import_projects + end end end diff --git a/app/graphql/types/group_type.rb b/app/graphql/types/group_type.rb index 45357de55026f68903a0351081df9214a9da6efb..34ae292fc39b2797a45d522458f645f7c3fb52bf 100644 --- a/app/graphql/types/group_type.rb +++ b/app/graphql/types/group_type.rb @@ -35,6 +35,12 @@ class GroupType < NamespaceType method: :project_creation_level_str, description: 'Permission level required to create projects in the group.' + field :project_import_level, + type: GraphQL::Types::String, + null: true, + method: :project_import_level_str, + description: 'Permission level required to import projects to the group.' + field :subgroup_creation_level, type: GraphQL::Types::String, null: true, diff --git a/app/graphql/types/permission_types/group.rb b/app/graphql/types/permission_types/group.rb index 6a1031e253223c3bca47e8239f2e1593d8b0047b..969f53b1cc96a3692e58d1d8be926951bbe0230c 100644 --- a/app/graphql/types/permission_types/group.rb +++ b/app/graphql/types/permission_types/group.rb @@ -5,7 +5,7 @@ module PermissionTypes class Group < BasePermissionType graphql_name 'GroupPermissions' - abilities :read_group, :create_projects + abilities :read_group, :create_projects, :import_projects end end end diff --git a/app/graphql/types/permission_types/group_enum.rb b/app/graphql/types/permission_types/group_enum.rb index f636d43790fb4f3baa05b1762414eb24ba4f958c..e3e57bb9d07133941795db300cfcf14e254065d3 100644 --- a/app/graphql/types/permission_types/group_enum.rb +++ b/app/graphql/types/permission_types/group_enum.rb @@ -7,6 +7,9 @@ class GroupEnum < BaseEnum description 'User permission on groups' value 'CREATE_PROJECTS', value: :create_projects, description: 'Groups where the user can create projects.' + value 'IMPORT_PROJECTS', + value: :import_projects, + description: 'Groups where the user can import projects.' value 'TRANSFER_PROJECTS', value: :transfer_projects, description: 'Groups where the user can transfer projects to.' diff --git a/app/models/concerns/group_api_compatibility.rb b/app/models/concerns/group_api_compatibility.rb index f02aa2035e57f42926351ff5d0bd350f2ba8b83b..24db9f40a8e69d57ff954e0e6f3d456fd00b9b68 100644 --- a/app/models/concerns/group_api_compatibility.rb +++ b/app/models/concerns/group_api_compatibility.rb @@ -12,6 +12,10 @@ def project_creation_level_str=(value) write_attribute(:project_creation_level, ::Gitlab::Access.project_creation_string_options.fetch(value)) end + def project_import_level_str + ::Gitlab::Access.project_import_string_options.key(project_import_level) + end + def subgroup_creation_level_str ::Gitlab::Access.subgroup_creation_string_options.key(subgroup_creation_level) end diff --git a/app/models/member.rb b/app/models/member.rb index f23705816b6c57e401b38579ffabcd1c06e0b6a5..929a0d784dc0ef62a8b40c94bd9f4212d5677d6e 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -14,6 +14,7 @@ class Member < ApplicationRecord include UpdateHighestRole include RestrictedSignup include Gitlab::Experiment::Dsl + include AsCte AVATAR_SIZE = 40 ACCESS_REQUEST_APPROVERS_TO_BE_NOTIFIED_LIMIT = 10 @@ -184,6 +185,30 @@ class Member < ApplicationRecord unscoped.from(distinct_members, :members) end + # TODO This scope will ignore any previous `select` projection settings. + scope :as_inherited_levels, -> { + members_cte = all.as_cte(:members) + + skope = unscoped + .with(members_cte.to_arel) + .from(members_cte.table) + .joins('RIGHT OUTER JOIN "namespaces" ON namespaces.traversal_ids::bigint[] @> ARRAY[member_namespace_id]') + .where.not(access_level: nil) + + user_id_col = Member.arel_table['user_id'] + mn_id_col = 'namespaces.id AS member_namespace_id' + access_level_col = 'MAX(access_level) AS access_level' + + skope = skope.reselect(user_id_col, mn_id_col, access_level_col) + + skope = skope.group( + Member.arel_table['user_id'], + Namespace.arel_table['id'] + ) + + skope + } + scope :order_name_asc, -> do build_keyset_order_on_joined_column( scope: left_join_users, diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 42f362876bb95014817d8756af20f6b6fbec34a7..410257e4a827a8594cf97c7c06b8216b3fa57b65 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -134,6 +134,7 @@ class Namespace < ApplicationRecord :pypi_package_requests_forwarding, :npm_package_requests_forwarding, to: :package_settings + delegate :project_import_level, :project_import_level=, to: :namespace_settings, allow_nil: true after_save :reload_namespace_details diff --git a/app/models/namespace_setting.rb b/app/models/namespace_setting.rb index 6a87fba57acfdd27568eec4cfb991fd988fc887c..7e0d560fb0d83a70043ba1c81486236a0a910fff 100644 --- a/app/models/namespace_setting.rb +++ b/app/models/namespace_setting.rb @@ -42,6 +42,7 @@ class NamespaceSetting < ApplicationRecord enabled_git_access_protocol subgroup_runner_token_expiration_interval project_runner_token_expiration_interval + project_import_level ].freeze self.primary_key = :namespace_id diff --git a/app/models/user.rb b/app/models/user.rb index 338cebad6a1956d8297d77d808f099e042cbe4a9..a92db5d04d37273efe6f2cf3d6c5c26ffaa13797 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1112,6 +1112,26 @@ def membership_groups groups.self_and_descendants end + # Returns all groups where the user has project_import_level permission, + # i.e. where member.access_level is >= group.project_import_level. + # Since membership is inherited automatically on descendant sub-groups but not + # stored in the database, the user's membership access_level is determined + # through a Member scope :as_inherited_levels, which returns a collection of + # Member records that include `access_level`, `user_id`, and `member_namespace_id` + # attributes. + def import_groups + collection = [] + Member.as_inherited_levels.where(user_id: self.id).each do |m| # rubocop: disable Rails/FindEach + namespace = Namespace.find(m.member_namespace_id) + member_access_level = m.access_level + project_import_level = namespace.project_import_level + unless project_import_level.nil? || project_import_level == 0 || member_access_level < project_import_level + collection << namespace + end + end + collection + end + # Returns a relation of groups the user has access to, including their parent # and child groups (recursively). def all_expanded_groups diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index a2dfce6748fd018e70f5913ade0e2ce5bb53c3b4..e7460ebeac2a414cd21f1ce6de533a2c14ea366e 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -24,6 +24,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy condition(:can_change_parent_share_with_group_lock) { can?(:change_share_with_group_lock, @subject.parent) } condition(:migration_bot, scope: :user) { @user.migration_bot? } condition(:can_read_group_member) { can_read_group_member? } + condition(:can_import_projects) { can_import_projects? } desc "User is a project bot" condition(:project_bot) { user.project_bot? && access_level >= GroupMember::GUEST } @@ -232,6 +233,10 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy rule { owner }.enable :create_subgroup rule { maintainer & maintainer_can_create_group }.enable :create_subgroup + rule { can_import_projects }.policy do + enable :import_projects + end + rule { public_group | logged_in_viewable }.enable :view_globally rule { default }.enable(:request_access) @@ -339,6 +344,11 @@ def can_read_group_member? !(@subject.private? && access_level == GroupMember::NO_ACCESS) end + def can_import_projects? + import_level = @subject.project_import_level + access_level >= import_level && import_level != 0 + end + def resource_access_token_creation_allowed? resource_access_token_feature_available? && group.root_ancestor.namespace_settings.resource_access_token_creation_allowed? end diff --git a/app/services/groups/base_service.rb b/app/services/groups/base_service.rb index 9705f3a560d9228dc0e24ac37cb46f2ed4a544f4..435adb6c952b8ac833a556f64fdb5661a86499f9 100644 --- a/app/services/groups/base_service.rb +++ b/app/services/groups/base_service.rb @@ -13,6 +13,7 @@ def initialize(group, user, params = {}) private def handle_namespace_settings + handle_project_import_level settings_params = params.slice(*::NamespaceSetting.allowed_namespace_settings_params) return if settings_params.empty? @@ -27,5 +28,13 @@ def handle_namespace_settings def remove_unallowed_params # overridden in EE end + + def handle_project_import_level + param = params.delete(:project_import_level_str) + return unless param && Feature.enabled?(:project_import_level_configuration) + + import_level = ::Gitlab::Access.project_import_string_options.fetch(param) + params[:project_import_level] = import_level + end end end diff --git a/config/feature_flags/development/project_import_level_configuration.yml b/config/feature_flags/development/project_import_level_configuration.yml new file mode 100644 index 0000000000000000000000000000000000000000..c594cbf2a59afec7249b1755a1516f29fe9b8148 --- /dev/null +++ b/config/feature_flags/development/project_import_level_configuration.yml @@ -0,0 +1,8 @@ +--- +name: project_import_level_configuration +introduced_by_url: 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/96666' +rollout_issue_url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/371643' +milestone: '15.4' +type: development +group: group::import +default_enabled: false diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 67dea28cbd58366a68f49a6a275d8577c5fd612c..ca5d3ba159d768e278ca0f76b55c1723bbec8097 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -12690,6 +12690,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | `parent` | [`Group`](#group) | Parent group. | | `path` | [`String!`](#string) | Path of the namespace. | | `projectCreationLevel` | [`String`](#string) | Permission level required to create projects in the group. | +| `projectImportLevel` | [`String`](#string) | Permission level required to import projects to the group. | | `recentIssueBoards` | [`BoardConnection`](#boardconnection) | List of recently visited boards of the group. Maximum size is 4. (see [Connections](#connections)) | | `repositorySizeExcessProjectCount` | [`Int!`](#int) | Number of projects in the root namespace where the repository size exceeds the limit. | | `requestAccessEnabled` | [`Boolean`](#boolean) | Indicates if users can request access to namespace. | @@ -13444,6 +13445,7 @@ Returns [`UserMergeRequestInteraction`](#usermergerequestinteraction). | Name | Type | Description | | ---- | ---- | ----------- | | `createProjects` | [`Boolean!`](#boolean) | Indicates the user can perform `create_projects` on this resource. | +| `importProjects` | [`Boolean!`](#boolean) | Indicates the user can perform `import_projects` on this resource. | | `readGroup` | [`Boolean!`](#boolean) | Indicates the user can perform `read_group` on this resource. | ### `GroupReleaseStats` @@ -20512,6 +20514,7 @@ User permission on groups. | Value | Description | | ----- | ----------- | | `CREATE_PROJECTS` | Groups where the user can create projects. | +| `IMPORT_PROJECTS` | Groups where the user can import projects. | | `TRANSFER_PROJECTS` | Groups where the user can transfer projects to. | ### `HealthStatus` diff --git a/doc/api/groups.md b/doc/api/groups.md index c1effc78a4d1b3b7cf56efac3f92bbd1778fcd0c..cc7f2618f46b5b6518413e364fe36351fee00e58 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -49,6 +49,7 @@ GET /groups "require_two_factor_authentication": false, "two_factor_grace_period": 48, "project_creation_level": "developer", + "project_import_level": "owner", "auto_devops_enabled": null, "subgroup_creation_level": "owner", "emails_disabled": null, @@ -86,6 +87,7 @@ GET /groups?statistics=true "require_two_factor_authentication": false, "two_factor_grace_period": 48, "project_creation_level": "developer", + "project_import_level": "owner", "auto_devops_enabled": null, "subgroup_creation_level": "owner", "emails_disabled": null, @@ -166,6 +168,7 @@ GET /groups/:id/subgroups "require_two_factor_authentication": false, "two_factor_grace_period": 48, "project_creation_level": "developer", + "project_import_level": "owner", "auto_devops_enabled": null, "subgroup_creation_level": "owner", "emails_disabled": null, @@ -224,6 +227,7 @@ GET /groups/:id/descendant_groups "require_two_factor_authentication": false, "two_factor_grace_period": 48, "project_creation_level": "developer", + "project_import_level": "owner", "auto_devops_enabled": null, "subgroup_creation_level": "owner", "emails_disabled": null, @@ -249,6 +253,7 @@ GET /groups/:id/descendant_groups "require_two_factor_authentication": false, "two_factor_grace_period": 48, "project_creation_level": "developer", + "project_import_level": "owner", "auto_devops_enabled": null, "subgroup_creation_level": "owner", "emails_disabled": null, @@ -822,7 +827,8 @@ Parameters: | `lfs_enabled` | boolean | no | Enable/disable Large File Storage (LFS) for the projects in this group. | | `mentions_disabled` | boolean | no | Disable the capability of a group from getting mentioned. | | `parent_id` | integer | no | The parent group ID for creating nested group. | -| `project_creation_level` | string | no | Determine if developers can create projects in the group. Can be `noone` (No one), `maintainer` (users with the Maintainer role), or `developer` (users with the Developer or Maintainer role). | +| `project_creation_level` | string | no | Determine if developers can create projects in the group. Can be `noone` (No one), `maintainer` (users with the Maintainer role), or `developer` (users with the Developer, or Maintainer role). | +| `project_import_level` | string | no | Determine if developers can import projects to the group. Can be `noone` (No one), `owner` (users with Owner role), `maintainer` (users with the Maintainer role), or `developer` (users with the Owner, Developer or Maintainer role). | | `request_access_enabled` | boolean | no | Allow users to request member access. | | `require_two_factor_authentication` | boolean | no | Require all users in this group to setup Two-factor authentication. | | `share_with_group_lock` | boolean | no | Prevent sharing a project with another group within this group. | @@ -977,7 +983,8 @@ PUT /groups/:id | `lfs_enabled` | boolean | no | Enable/disable Large File Storage (LFS) for the projects in this group. | | `mentions_disabled` | boolean | no | Disable the capability of a group from getting mentioned. | | `prevent_sharing_groups_outside_hierarchy` | boolean | no | See [Prevent group sharing outside the group hierarchy](../user/group/access_and_permissions.md#prevent-group-sharing-outside-the-group-hierarchy). This attribute is only available on top-level groups. [Introduced in GitLab 14.1](https://gitlab.com/gitlab-org/gitlab/-/issues/333721) | -| `project_creation_level` | string | no | Determine if developers can create projects in the group. Can be `noone` (No one), `maintainer` (users with the Maintainer role), or `developer` (users with the Developer or Maintainer role). | +| `project_creation_level` | string | no | Determine if developers can create projects in the group. Can be `noone` (No one), `maintainer` (users with the Maintainer role), or `developer` (users with the Developer, or Maintainer role). | +| `project_import_level` | string | no | Determine if developers can import projects to the group. Can be `noone` (No one), `owner` (users with Owner role), `maintainer` (users with the Maintainer role), or `developer` (users with the Owner, Developer, or Maintainer role). | | `request_access_enabled` | boolean | no | Allow users to request member access. | | `require_two_factor_authentication` | boolean | no | Require all users in this group to setup Two-factor authentication. | | `shared_runners_setting` | string | no | See [Options for `shared_runners_setting`](#options-for-shared_runners_setting). Enable or disable shared runners for a group's subgroups and projects. | diff --git a/ee/spec/requests/api/graphql/current_user/groups_query_spec.rb b/ee/spec/requests/api/graphql/current_user/groups_query_spec.rb index 9c821a6cf4db4ef9bf8ca422e859d7a6226408e0..4734d4bd758b9eec257296ceb3a4aa0fe569f697 100644 --- a/ee/spec/requests/api/graphql/current_user/groups_query_spec.rb +++ b/ee/spec/requests/api/graphql/current_user/groups_query_spec.rb @@ -77,4 +77,15 @@ end end end + + context 'when permission_scope is IMPORT_PROJECTS' do + let(:group_arguments) { { permission_scope: :IMPORT_PROJECTS } } + + before do + stub_feature_flags(project_import_level_configuration: true) + post_graphql(query, current_user: current_user) + end + + it_behaves_like 'a working graphql query' + end end diff --git a/lib/api/entities/group.rb b/lib/api/entities/group.rb index 246fb81989069451cf11dda083579f2ec358bc14..d2d88fb1da74a757bcbf2414b4c80ef72609e77b 100644 --- a/lib/api/entities/group.rb +++ b/lib/api/entities/group.rb @@ -8,6 +8,7 @@ class Group < BasicGroupDetails expose :require_two_factor_authentication expose :two_factor_grace_period expose :project_creation_level_str, as: :project_creation_level + expose :project_import_level_str, as: :project_import_level expose :auto_devops_enabled expose :subgroup_creation_level_str, as: :subgroup_creation_level expose :emails_disabled diff --git a/lib/api/groups.rb b/lib/api/groups.rb index ca99e30fbf78ea2bc86f492292964f5de4a1fd64..9d6a8e7aa5fd77bd064078f368e98739fc2d1e88 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -201,10 +201,9 @@ def check_subscription!(group) use :with_custom_attributes end get feature_category: :subgroups do - groups = find_groups(declared_params(include_missing: false), params[:id]) + groups = find_groups(declared_params(include_missing: false), params[:id]).preload(:namespace_settings) # rubocop: disable CodeReuse/ActiveRecord present_groups_with_pagination_strategies params, groups end - desc 'Create a group. Available only for users who can create groups.' do success Entities::Group end diff --git a/lib/api/helpers/groups_helpers.rb b/lib/api/helpers/groups_helpers.rb index e9af50b80beea7ff7d5f228c69994e018689a8a9..ce5a1013aac653984490f1693fead84ca15e264e 100644 --- a/lib/api/helpers/groups_helpers.rb +++ b/lib/api/helpers/groups_helpers.rb @@ -24,6 +24,7 @@ module GroupsHelpers optional :request_access_enabled, type: Boolean, desc: 'Allow users to request member access' optional :default_branch_protection, type: Integer, values: ::Gitlab::Access.protection_values, desc: 'Determine if developers can push to default branch' optional :shared_runners_setting, type: String, values: ::Namespace::SHARED_RUNNERS_SETTINGS, desc: 'Enable/disable shared runners for the group and its subgroups and projects' + optional :project_import_level, type: String, values: ::Gitlab::Access.project_import_string_values, desc: 'Determine if developers can import projects to the group', as: :project_import_level_str end params :optional_params_ee do diff --git a/lib/bulk_imports/groups/graphql/get_group_query.rb b/lib/bulk_imports/groups/graphql/get_group_query.rb index 0e73a7fb4b9a5c449408c7e5862501285d6187a1..fcd7d8630daa5454ccfe844c26502255c548477d 100644 --- a/lib/bulk_imports/groups/graphql/get_group_query.rb +++ b/lib/bulk_imports/groups/graphql/get_group_query.rb @@ -23,6 +23,7 @@ def to_s lfs_enabled: lfsEnabled mentions_disabled: mentionsDisabled project_creation_level: projectCreationLevel + project_import_level: projectImportLevel request_access_enabled: requestAccessEnabled require_two_factor_authentication: requireTwoFactorAuthentication share_with_group_lock: shareWithGroupLock diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb index fa025a2658fb7308546fcc2081f071524411f0e2..c0422c9c6deb77826f6e0b4d149c63ef2d1bf4a6 100644 --- a/lib/gitlab/access.rb +++ b/lib/gitlab/access.rb @@ -138,6 +138,19 @@ def project_creation_level_name(name) project_creation_options.key(name) end + def project_import_string_options + { + 'no one' => NO_ACCESS, + 'owner' => OWNER, + 'maintainer' => MAINTAINER, + 'developer' => DEVELOPER + } + end + + def project_import_string_values + project_import_string_options.keys + end + def subgroup_creation_options { s_('SubgroupCreationlevel|Owners') => OWNER_SUBGROUP_ACCESS, diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 37cb0a1f2894f69f67070a1a609de05fe03abb71..5ecdbd35bfaedcd4458f4b9b1f97bcc87ee69e95 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -44,4 +44,70 @@ end.to change { Namespace::AdminNote.count }.by(1) end end + + describe 'PUT #members_update' do + let_it_be(:group_user) { create(:user) } + + it 'adds user to members', :aggregate_failures, :snowplow do + put :members_update, params: { + id: group, + user_id: group_user.id, + access_level: Gitlab::Access::GUEST + } + + expect(controller).to set_flash.to 'Users were successfully added.' + expect(response).to redirect_to(admin_group_path(group)) + expect(group.users).to include group_user + expect_snowplow_event( + category: 'Members::CreateService', + action: 'create_member', + label: 'admin-group-page', + property: 'existing_user', + user: admin + ) + end + + it 'can add unlimited members', :aggregate_failures do + put :members_update, params: { + id: group, + user_id: 1.upto(1000).to_a.join(','), + access_level: Gitlab::Access::GUEST + } + + expect(controller).to set_flash.to 'Users were successfully added.' + expect(response).to redirect_to(admin_group_path(group)) + end + + it 'adds no user to members', :aggregate_failures do + put :members_update, params: { + id: group, + user_id: '', + access_level: Gitlab::Access::GUEST + } + + expect(controller).to set_flash.to 'No users specified.' + expect(response).to redirect_to(admin_group_path(group)) + expect(group.users).not_to include group_user + end + + it 'updates the project_creation_level successfully' do + expect do + post :update, params: { id: group.to_param, group: { project_creation_level: ::Gitlab::Access::NO_ONE_PROJECT_ACCESS } } + end.to change { group.reload.project_creation_level }.to(::Gitlab::Access::NO_ONE_PROJECT_ACCESS) + end + + it 'updates the project_import_level successfully' do + expect do + post :update, params: { id: group.to_param, group: { project_import_level: ::Gitlab::Access::DEVELOPER } } + end.to change { group.reload.project_import_level }.to(::Gitlab::Access::DEVELOPER) + end + + it 'updates the subgroup_creation_level successfully' do + expect do + post :update, + params: { id: group.to_param, + group: { subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS } } + end.to change { group.reload.subgroup_creation_level }.to(::Gitlab::Access::OWNER_SUBGROUP_ACCESS) + end + end end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index c4e4eeec953060f0aff0ef6590275c56552e2932..ebb715f3b2a1625966f6368ce1ebfc180741447a 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -595,6 +595,13 @@ expect(group.reload.project_creation_level).to eq(::Gitlab::Access::MAINTAINER_PROJECT_ACCESS) end + it 'updates the project_import_level successfully' do + post :update, params: { id: group.to_param, group: { project_import_level: ::Gitlab::Access::DEVELOPER } } + + expect(response).to have_gitlab_http_status(:found) + expect(group.reload.project_import_level).to eq(::Gitlab::Access::DEVELOPER) + end + context 'updating default_branch_protection' do subject do put :update, params: { id: group.to_param, group: { default_branch_protection: ::Gitlab::Access::PROTECTION_DEV_CAN_MERGE } } diff --git a/spec/graphql/types/group_type_spec.rb b/spec/graphql/types/group_type_spec.rb index 0b65778ce90541fd386aa4865fdece18a3729f6d..9357df4ccf83a7dacb9149dadb4041c6b0eeca0f 100644 --- a/spec/graphql/types/group_type_spec.rb +++ b/spec/graphql/types/group_type_spec.rb @@ -26,7 +26,7 @@ dependency_proxy_image_prefix dependency_proxy_image_ttl_policy shared_runners_setting timelogs organization_state_counts organizations contact_state_counts contacts work_item_types - recent_issue_boards ci_variables + recent_issue_boards ci_variables project_import_level ] expect(described_class).to include_graphql_fields(*expected_fields) diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 68c2d1d3995b52e97cd174372fa059eb2fc8704f..f66d71b664e60d7fbc43a74e601b16682676044d 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -2368,6 +2368,21 @@ def define_cache_expectations(cache_key) end end + describe 'project_import_level' do + it 'defaults to owners' do + expect(group.project_import_level) + .to eq(Gitlab::Access::OWNER) + end + + it 'updates access level on namespace_settings table' do + group.project_import_level = Gitlab::Access::MAINTAINER + group.save! + + expect(NamespaceSetting.find_by(namespace_id: group.id).project_import_level) + .to eq(Gitlab::Access::MAINTAINER) + end + end + describe 'subgroup_creation_level' do it 'defaults to maintainers' do expect(group.subgroup_creation_level) diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 414a201aa34528e2d673f4f5ec41e322b19a1d35..98954a81ce877b0ff448c78505e4a7b3e84341ba 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -658,6 +658,160 @@ end end + shared_context 'with inherited levels' do + let!(:user) { create(:user) } + let!(:root) { create :group } + let!(:subgroup1) { create :group, parent: root } + let!(:subgroup2) { create :group, parent: root } + let!(:subsubgroup) { create :group, parent: subgroup1 } + end + + describe '.as_inherited_levels' do + include_context 'with inherited levels' + + subject { Member.where(user: user).as_inherited_levels } + + context 'when member of hierarchy' do + before do + root.add_developer user + end + + it 'assigns access level to inherited namespaces' do + expect(subject).to include( + an_object_having_attributes( + 'user_id' => user.id, + 'member_namespace_id' => root.id, + 'access_level' => Gitlab::Access::DEVELOPER + ), + an_object_having_attributes( + 'user_id' => user.id, + 'member_namespace_id' => subgroup1.id, + 'access_level' => Gitlab::Access::DEVELOPER + ), + an_object_having_attributes( + 'user_id' => user.id, + 'member_namespace_id' => subgroup2.id, + 'access_level' => Gitlab::Access::DEVELOPER + ), + an_object_having_attributes( + 'user_id' => user.id, + 'member_namespace_id' => subsubgroup.id, + 'access_level' => Gitlab::Access::DEVELOPER + ) + ) + end + end + + context 'with nested memberships in hierarchy' do + before do + root.add_developer user + subgroup1.add_maintainer user + end + + it 'assigns access level to nested inherited namespaces' do + expect(subject).to include( + an_object_having_attributes( + 'user_id' => user.id, + 'member_namespace_id' => root.id, + 'access_level' => Gitlab::Access::DEVELOPER + ), + an_object_having_attributes( + 'user_id' => user.id, + 'member_namespace_id' => subgroup1.id, + 'access_level' => Gitlab::Access::MAINTAINER + ), + an_object_having_attributes( + 'user_id' => user.id, + 'member_namespace_id' => subgroup2.id, + 'access_level' => Gitlab::Access::DEVELOPER + ), + an_object_having_attributes( + 'user_id' => user.id, + 'member_namespace_id' => subsubgroup.id, + 'access_level' => Gitlab::Access::MAINTAINER + ) + ) + end + end + end + + describe '.with_inherited' do + include_context 'with inherited levels' + + subject { Member.where(user: user).with_inherited } + + context 'when member of hierarchy' do + before do + root.add_developer user + end + + it 'assigns access level to inherited namespaces' do + expect(subject).to include( + an_object_having_attributes( + 'id' => an_instance_of(Integer), + 'user_id' => user.id, + 'member_namespace_id' => root.id, + 'access_level' => Gitlab::Access::DEVELOPER + ), + an_object_having_attributes( + 'id' => nil, + 'user_id' => user.id, + 'member_namespace_id' => subgroup1.id, + 'access_level' => Gitlab::Access::DEVELOPER + ), + an_object_having_attributes( + 'id' => nil, + 'user_id' => user.id, + 'member_namespace_id' => subgroup2.id, + 'access_level' => Gitlab::Access::DEVELOPER + ), + an_object_having_attributes( + 'id' => nil, + 'user_id' => user.id, + 'member_namespace_id' => subsubgroup.id, + 'access_level' => Gitlab::Access::DEVELOPER + ) + ) + end + end + + context 'with nested memberships in hierarchy' do + before do + root.add_developer user + subgroup1.add_maintainer user + end + + it 'assigns access level to nested inherited namespaces' do + expect(subject).to include( + an_object_having_attributes( + 'id' => an_instance_of(Integer), + 'user_id' => user.id, + 'member_namespace_id' => root.id, + 'access_level' => Gitlab::Access::DEVELOPER + ), + an_object_having_attributes( + 'id' => an_instance_of(Integer), + 'user_id' => user.id, + 'member_namespace_id' => subgroup1.id, + 'access_level' => Gitlab::Access::MAINTAINER + ), + an_object_having_attributes( + 'id' => nil, + 'user_id' => user.id, + 'member_namespace_id' => subgroup2.id, + 'access_level' => Gitlab::Access::DEVELOPER + ), + an_object_having_attributes( + 'id' => nil, + 'user_id' => user.id, + 'member_namespace_id' => subsubgroup.id, + 'access_level' => Gitlab::Access::MAINTAINER + ) + ) + end + end + end + describe '.with_invited_user_state' do subject(:with_invited_user_state) { described_class.with_invited_user_state } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a163441590f5035f4b014c4dd5d635610edd61ad..33ca3a5b9bc7bd4acb6cc8eaa4a31329a9f6bf87 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -4154,6 +4154,37 @@ def login_method(login) specify { is_expected.to contain_exactly(parent_group, child_group) } end + describe '#import_groups' do + context 'where project import permissions are not met' do + let!(:user) { create(:user) } + + let!(:parent_group) { create(:group) } + let!(:child_group) { create(:group, parent: parent_group) } + let!(:other_group) { create(:group) } + + it 'inherits access level from parent does not contain any import groups' do + parent_group.add_maintainer(user) + + expect(user.import_groups).to be_empty + end + end + + context 'where project import permissions are met' do + let!(:user) { create(:user) } + let!(:parent_group) { create(:group) } + let!(:child_group) { create(:group, parent: parent_group) } + let!(:other_group) { create(:group) } + + subject { user.import_groups } + + it 'inherits access level from parent and contains correct import groups' do + parent_group.add_owner(user) + + expect(user.import_groups).to contain_exactly(parent_group, child_group) + end + end + end + describe '#authorizations_for_projects' do let!(:user) { create(:user) } diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 5f8e582b1aaf7ee4557cfdeabf44778be3a756f4..c81a01c2226f43fe809f8c41f29e6a6640d2b766 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -718,6 +718,154 @@ end end + context 'import_project' do + context 'when group has project import level set to owner' do + before_all do + group.update!(project_import_level: ::Gitlab::Access::OWNER) + end + + context 'guest' do + let(:current_user) { guest } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:import_projects) } + end + end + + context 'when group has project import level set to maintainer' do + before_all do + group.update!(project_import_level: ::Gitlab::Access::MAINTAINER) + end + + context 'guest' do + let(:current_user) { guest } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:import_project) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_allowed(:import_projects) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:import_projects) } + end + end + + context 'when group has project import level set to developer' do + before_all do + group.update!(project_import_level: ::Gitlab::Access::DEVELOPER) + end + + context 'guest' do + let(:current_user) { guest } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_allowed(:import_projects) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_allowed(:import_projects) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:import_projects) } + end + end + end + + context 'when group has project import level set to no one' do + before_all do + group.update!(project_import_level: ::Gitlab::Access::NO_ACCESS) + end + + context 'guest' do + let(:current_user) { guest } + + it do + is_expected.to be_disallowed(:import_projects) + end + end + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_disallowed(:import_projects) } + end + end + it_behaves_like 'clusterable policies' do let(:clusterable) { create(:group, :crm_enabled) } let(:cluster) do diff --git a/spec/requests/api/graphql/current_user/groups_query_spec.rb b/spec/requests/api/graphql/current_user/groups_query_spec.rb index 6e36beb2afc7540d84286baaddeb65bd23a7d55a..74d16885beab183376bc575b8358fb91e24c565d 100644 --- a/spec/requests/api/graphql/current_user/groups_query_spec.rb +++ b/spec/requests/api/graphql/current_user/groups_query_spec.rb @@ -10,6 +10,7 @@ let_it_be(:guest_group) { create(:group, name: 'public guest', path: 'public-guest') } let_it_be(:private_maintainer_group) { create(:group, :private, name: 'b private maintainer', path: 'b-private-maintainer', parent: root_group) } let_it_be(:public_developer_group) { create(:group, project_creation_level: nil, name: 'c public developer', path: 'c-public-developer') } + let_it_be(:public_developer_group_with_import_permissions) { create(:group, project_creation_level: nil, name: 'c public developer with import', path: 'c-public-developer-import') } let_it_be(:public_maintainer_group) { create(:group, name: 'a public maintainer', path: 'a-public-maintainer', parent: root_group) } let_it_be(:public_owner_group) { create(:group, name: 'a public owner', path: 'a-public-owner') } @@ -30,8 +31,12 @@ guest_group.add_guest(user) private_maintainer_group.add_maintainer(user) public_developer_group.add_developer(user) + public_developer_group_with_import_permissions.add_developer(user) public_maintainer_group.add_maintainer(user) public_owner_group.add_owner(user) + + public_developer_group_with_import_permissions.project_import_level = ::Gitlab::Access::DEVELOPER + public_developer_group_with_import_permissions.namespace_settings.save! end subject { graphql_data.dig('currentUser', 'groups', 'nodes') } @@ -58,6 +63,7 @@ public_owner_group, private_maintainer_group, public_developer_group, + public_developer_group_with_import_permissions, guest_group ) ) @@ -72,7 +78,8 @@ public_maintainer_group, public_owner_group, private_maintainer_group, - public_developer_group + public_developer_group, + public_developer_group_with_import_permissions ) ) end @@ -91,6 +98,35 @@ end end + context 'when permission_scope is IMPORT_PROJECTS' do + let(:group_arguments) { { permission_scope: :IMPORT_PROJECTS } } + + before do + stub_feature_flags(project_import_level_configuration: true) + end + + specify do + is_expected.to match( + expected_group_hash( + public_owner_group, + public_developer_group_with_import_permissions + ) + ) + end + + context 'when search is provided' do + let(:group_arguments) { { permission_scope: :IMPORT_PROJECTS, search: 'owner' } } + + specify do + is_expected.to match( + expected_group_hash( + public_owner_group + ) + ) + end + end + end + context 'when permission_scope is TRANSFER_PROJECTS' do let(:group_arguments) { { permission_scope: :TRANSFER_PROJECTS } } diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 02d29601cebde60836ac46158e3937d78ba2a77d..6a6018c62390d41b4e85e1fcc304071d2f604605 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -103,7 +103,7 @@ expect do get api("/groups", admin) - end.not_to exceed_query_limit(control) + end.not_to exceed_query_limit(control.count) end context 'when statistics are requested' do @@ -662,6 +662,7 @@ def response_project_ids(json_response, key) expect(json_response['shared_projects']).to be_an Array expect(json_response['shared_projects'].length).to eq(1) expect(json_response['shared_projects'][0]['id']).to eq(project.id) + expect(json_response['project_import_level']).to eq('owner') end it "returns one of user1's groups without projects when with_projects option is set to false" do @@ -905,6 +906,10 @@ def make_upload_request end context 'when authenticated as the group owner' do + before do + stub_feature_flags(project_import_level_configuration: true) + end + it 'updates the group' do workhorse_form_with_file( api("/groups/#{group1.id}", user1), @@ -914,6 +919,7 @@ def make_upload_request name: new_group_name, request_access_enabled: true, project_creation_level: "noone", + project_import_level: "maintainer", subgroup_creation_level: "maintainer", default_branch_protection: ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS, prevent_sharing_groups_outside_hierarchy: true, @@ -943,6 +949,7 @@ def make_upload_request expect(json_response['default_branch_protection']).to eq(::Gitlab::Access::MAINTAINER_PROJECT_ACCESS) expect(json_response['avatar_url']).to end_with('dk.png') expect(json_response['prevent_sharing_groups_outside_hierarchy']).to eq(true) + expect(json_response['project_import_level']).to eq("maintainer") end it 'removes the group avatar' do diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb index 5c87b9ac8bb1bd8fa7c0a975ab58b4d7731b3c32..ab1bf19c01b1a82ec21f45b39ca8e315af2772b4 100644 --- a/spec/services/groups/update_service_spec.rb +++ b/spec/services/groups/update_service_spec.rb @@ -53,6 +53,34 @@ end end + context 'updating project_import_level setting' do + let!(:group) { create(:group) } + + context 'with project_import_level_configuration enabled' do + before do + stub_feature_flags(project_import_level_configuration: true) + end + + it 'updates the project_import_level' do + expect(update_group(group, user, project_import_level_str: 'developer')).to be true + expect(group.errors).to be_empty + expect(group.project_import_level).to eq(30) + end + end + + context 'with project_import_level_configuration disabled' do + before do + stub_feature_flags(project_import_level_configuration: false) + end + + it 'ignores the project_import_level_str param' do + expect { update_group(group, user, project_import_level_str: 'developer') } + .to not_change { group.project_import_level } + expect(group.project_import_level).to eq(50) + end + end + end + context "project visibility_level validation" do context "public group with public projects" do let!(:service) { described_class.new(public_group, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL) } diff --git a/spec/support/helpers/group_api_helpers.rb b/spec/support/helpers/group_api_helpers.rb index 56c4cc121a7c5449a35ed2cbd5bae65373152305..ec3cc72011ed83c46218f9f2a0ad65443642ba9d 100644 --- a/spec/support/helpers/group_api_helpers.rb +++ b/spec/support/helpers/group_api_helpers.rb @@ -4,8 +4,11 @@ module GroupAPIHelpers extend self def attributes_for_group_api(params = {}) - # project_creation_level and subgroup_creation_level are Integers in the model - # but are strings in the API - attributes_for(:group, params).except(:project_creation_level, :subgroup_creation_level) + # project_creation_level, project_import_level, and subgroup_creation_level + # are Integers in the model but are strings in the API + attributes_for(:group, params).except( + :project_creation_level, + :project_import_level, + :subgroup_creation_level) end end