From ce120277e985dd915d3ae2ecba9c9d0f379969c2 Mon Sep 17 00:00:00 2001 From: Steve Abrams Date: Fri, 29 Apr 2022 15:29:19 -0600 Subject: [PATCH 1/2] Update package namespace settings permissions Updates permissions to change namespace package settings from developer to maintainer. This includes duplicate package settings. This aligns with settings permissions throughout the rest of GitLab. Changelog: changed --- app/graphql/mutations/namespace/package_settings/update.rb | 2 +- app/graphql/types/namespace/package_settings_type.rb | 2 +- app/policies/group_policy.rb | 3 +-- app/policies/namespaces/user_namespace_policy.rb | 3 +-- app/services/namespaces/package_settings/update_service.rb | 2 +- doc/user/packages/generic_packages/index.md | 3 ++- doc/user/packages/maven_repository/index.md | 3 ++- doc/user/permissions.md | 1 + .../mutations/namespace/package_settings/update_spec.rb | 6 +++--- spec/graphql/types/namespace/package_settings_type_spec.rb | 2 +- spec/policies/namespaces/project_namespace_policy_spec.rb | 4 ++-- spec/policies/namespaces/user_namespace_policy_spec.rb | 2 +- .../mutations/namespace/package_settings/update_spec.rb | 4 ++-- .../namespaces/package_settings/update_service_spec.rb | 4 ++-- .../shared_contexts/policies/group_policy_shared_context.rb | 3 +-- 15 files changed, 22 insertions(+), 22 deletions(-) diff --git a/app/graphql/mutations/namespace/package_settings/update.rb b/app/graphql/mutations/namespace/package_settings/update.rb index 934b75193d7f09..e499e646781466 100644 --- a/app/graphql/mutations/namespace/package_settings/update.rb +++ b/app/graphql/mutations/namespace/package_settings/update.rb @@ -8,7 +8,7 @@ class Update < Mutations::BaseMutation include Mutations::ResolvesNamespace - authorize :create_package_settings + authorize :admin_package argument :namespace_path, GraphQL::Types::ID, diff --git a/app/graphql/types/namespace/package_settings_type.rb b/app/graphql/types/namespace/package_settings_type.rb index cb546bbf3ec859..7a0abe619a5aac 100644 --- a/app/graphql/types/namespace/package_settings_type.rb +++ b/app/graphql/types/namespace/package_settings_type.rb @@ -6,7 +6,7 @@ class Namespace::PackageSettingsType < BaseObject description 'Namespace-level Package Registry settings' - authorize :read_package_settings + authorize :admin_package field :generic_duplicate_exception_regex, Types::UntrustedRegexp, null: true, description: 'When generic_duplicates_allowed is false, you can publish duplicate packages with names that match this regex. Otherwise, this setting has no effect.' field :generic_duplicates_allowed, GraphQL::Types::Boolean, null: false, description: 'Indicates whether duplicate generic packages are allowed for this namespace.' diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index eaefd7fcf8f639..f4e7256773c2ba 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -139,7 +139,6 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy enable :update_metrics_dashboard_annotation enable :create_custom_emoji enable :create_package - enable :create_package_settings enable :developer_access enable :admin_crm_organization enable :admin_crm_contact @@ -157,13 +156,13 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy enable :read_metrics_dashboard_annotation enable :read_prometheus enable :read_package - enable :read_package_settings enable :read_crm_organization enable :read_crm_contact end rule { maintainer }.policy do enable :destroy_package + enable :admin_package enable :create_projects enable :admin_pipeline enable :admin_build diff --git a/app/policies/namespaces/user_namespace_policy.rb b/app/policies/namespaces/user_namespace_policy.rb index 09b0f5d608d68a..028247497e54eb 100644 --- a/app/policies/namespaces/user_namespace_policy.rb +++ b/app/policies/namespaces/user_namespace_policy.rb @@ -14,8 +14,7 @@ class UserNamespacePolicy < ::NamespacePolicy enable :read_namespace enable :read_statistics enable :create_jira_connect_subscription - enable :create_package_settings - enable :read_package_settings + enable :admin_package end rule { ~can_create_personal_project }.prevent :create_projects diff --git a/app/services/namespaces/package_settings/update_service.rb b/app/services/namespaces/package_settings/update_service.rb index cbadbe5c907f1f..c0af090045003b 100644 --- a/app/services/namespaces/package_settings/update_service.rb +++ b/app/services/namespaces/package_settings/update_service.rb @@ -32,7 +32,7 @@ def package_settings end def allowed? - Ability.allowed?(current_user, :create_package_settings, @container) + Ability.allowed?(current_user, :admin_package, @container) end def package_settings_params diff --git a/doc/user/packages/generic_packages/index.md b/doc/user/packages/generic_packages/index.md index 9dc859a37e2d55..37e1f0c3eb12fa 100644 --- a/doc/user/packages/generic_packages/index.md +++ b/doc/user/packages/generic_packages/index.md @@ -101,7 +101,8 @@ API or the UI. #### Do not allow duplicate Generic packages -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/293755) in GitLab 13.12. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/293755) in GitLab 13.12. +> - [Required permissions](https://gitlab.com/gitlab-org/gitlab/-/issues/350682) changed from developer to maintainer in GitLab 15.0. To prevent users from publishing duplicate generic packages, you can use the [GraphQl API](../../../api/graphql/reference/index.md#packagesettings) or the UI. diff --git a/doc/user/packages/maven_repository/index.md b/doc/user/packages/maven_repository/index.md index 6a515b78fc17fb..5bac7fc546363a 100644 --- a/doc/user/packages/maven_repository/index.md +++ b/doc/user/packages/maven_repository/index.md @@ -617,7 +617,8 @@ To delete these older package versions, consider using the Packages API or the U #### Do not allow duplicate Maven packages -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/296895) in GitLab 13.9. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/296895) in GitLab 13.9. +> - [Required permissions](https://gitlab.com/gitlab-org/gitlab/-/issues/350682) changed from developer to maintainer in GitLab 15.0. To prevent users from publishing duplicate Maven packages, you can use the [GraphQl API](../../../api/graphql/reference/index.md#packagesettings) or the UI. diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 134bc0c4206fcb..af126b0f811e6b 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -390,6 +390,7 @@ The following table lists group permissions available for each role: | Publish [packages](packages/index.md) | | | ✓ | ✓ | ✓ | | Pull [packages](packages/index.md) | | ✓ | ✓ | ✓ | ✓ | | Delete [packages](packages/index.md) | | | | ✓ | ✓ | +| Create/edit/delete [Maven and generic package duplicate settings](packages/generic_packages/index.md#do-not-allow-duplicate-generic-packages) | | | | ✓ | ✓ | | Pull a Container Registry image | ✓ (7) | ✓ | ✓ | ✓ | ✓ | | Remove a Container Registry image | | | ✓ | ✓ | ✓ | | View [Group DevOps Adoption](group/devops_adoption/index.md) | | ✓ | ✓ | ✓ | ✓ | diff --git a/spec/graphql/mutations/namespace/package_settings/update_spec.rb b/spec/graphql/mutations/namespace/package_settings/update_spec.rb index 978c81fadfad2e..631e02ff3dc178 100644 --- a/spec/graphql/mutations/namespace/package_settings/update_spec.rb +++ b/spec/graphql/mutations/namespace/package_settings/update_spec.rb @@ -10,7 +10,7 @@ let(:params) { { namespace_path: namespace.full_path } } - specify { expect(described_class).to require_graphql_authorizations(:create_package_settings) } + specify { expect(described_class).to require_graphql_authorizations(:admin_package) } describe '#resolve' do subject { described_class.new(object: namespace, context: { current_user: user }, field: nil).resolve(**params) } @@ -68,7 +68,7 @@ where(:user_role, :shared_examples_name) do :maintainer | 'updating the namespace package setting' - :developer | 'updating the namespace package setting' + :developer | 'denying access to namespace package setting' :reporter | 'denying access to namespace package setting' :guest | 'denying access to namespace package setting' :anonymous | 'denying access to namespace package setting' @@ -88,7 +88,7 @@ where(:user_role, :shared_examples_name) do :maintainer | 'creating the namespace package setting' - :developer | 'creating the namespace package setting' + :developer | 'denying access to namespace package setting' :reporter | 'denying access to namespace package setting' :guest | 'denying access to namespace package setting' :anonymous | 'denying access to namespace package setting' diff --git a/spec/graphql/types/namespace/package_settings_type_spec.rb b/spec/graphql/types/namespace/package_settings_type_spec.rb index b9592d230caea0..f63a0a7010f85e 100644 --- a/spec/graphql/types/namespace/package_settings_type_spec.rb +++ b/spec/graphql/types/namespace/package_settings_type_spec.rb @@ -7,7 +7,7 @@ specify { expect(described_class.description).to eq('Namespace-level Package Registry settings') } - specify { expect(described_class).to require_graphql_authorizations(:read_package_settings) } + specify { expect(described_class).to require_graphql_authorizations(:admin_package) } describe 'maven_duplicate_exception_regex field' do subject { described_class.fields['mavenDuplicateExceptionRegex'] } diff --git a/spec/policies/namespaces/project_namespace_policy_spec.rb b/spec/policies/namespaces/project_namespace_policy_spec.rb index f1022747fab8e8..9aa8fed0ef62ca 100644 --- a/spec/policies/namespaces/project_namespace_policy_spec.rb +++ b/spec/policies/namespaces/project_namespace_policy_spec.rb @@ -9,8 +9,8 @@ let(:permissions) do [:owner_access, :create_projects, :admin_namespace, :read_namespace, - :read_statistics, :transfer_projects, :create_package_settings, - :read_package_settings, :create_jira_connect_subscription] + :read_statistics, :transfer_projects, :admin_package, + :admin_package, :create_jira_connect_subscription] end subject { described_class.new(current_user, namespace) } diff --git a/spec/policies/namespaces/user_namespace_policy_spec.rb b/spec/policies/namespaces/user_namespace_policy_spec.rb index 06db2f6e243051..22c3f6a6d67ff4 100644 --- a/spec/policies/namespaces/user_namespace_policy_spec.rb +++ b/spec/policies/namespaces/user_namespace_policy_spec.rb @@ -8,7 +8,7 @@ let_it_be(:admin) { create(:admin) } let_it_be(:namespace) { create(:user_namespace, owner: owner) } - let(:owner_permissions) { [:owner_access, :create_projects, :admin_namespace, :read_namespace, :read_statistics, :transfer_projects, :create_package_settings, :read_package_settings] } + let(:owner_permissions) { [:owner_access, :create_projects, :admin_namespace, :read_namespace, :read_statistics, :transfer_projects, :admin_package] } subject { described_class.new(current_user, namespace) } diff --git a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb index d335642d3217de..194e42bf59db1f 100644 --- a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb +++ b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb @@ -109,7 +109,7 @@ where(:user_role, :shared_examples_name) do :maintainer | 'accepting the mutation request updating the package settings' - :developer | 'accepting the mutation request updating the package settings' + :developer | 'denying the mutation request' :reporter | 'denying the mutation request' :guest | 'denying the mutation request' :anonymous | 'denying the mutation request' @@ -131,7 +131,7 @@ where(:user_role, :shared_examples_name) do :maintainer | 'accepting the mutation request creating the package settings' - :developer | 'accepting the mutation request creating the package settings' + :developer | 'denying the mutation request' :reporter | 'denying the mutation request' :guest | 'denying the mutation request' :anonymous | 'denying the mutation request' diff --git a/spec/services/namespaces/package_settings/update_service_spec.rb b/spec/services/namespaces/package_settings/update_service_spec.rb index 030bc03038e278..ed385f1cd7f680 100644 --- a/spec/services/namespaces/package_settings/update_service_spec.rb +++ b/spec/services/namespaces/package_settings/update_service_spec.rb @@ -71,7 +71,7 @@ where(:user_role, :shared_examples_name) do :maintainer | 'updating the namespace package setting' - :developer | 'updating the namespace package setting' + :developer | 'denying access to namespace package setting' :reporter | 'denying access to namespace package setting' :guest | 'denying access to namespace package setting' :anonymous | 'denying access to namespace package setting' @@ -91,7 +91,7 @@ where(:user_role, :shared_examples_name) do :maintainer | 'creating the namespace package setting' - :developer | 'creating the namespace package setting' + :developer | 'denying access to namespace package setting' :reporter | 'denying access to namespace package setting' :guest | 'denying access to namespace package setting' :anonymous | 'denying access to namespace package setting' diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index e35f5ae9f99f72..483bca07ba65ab 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -33,7 +33,6 @@ read_container_image read_metrics_dashboard_annotation read_prometheus - read_package_settings read_crm_contact read_crm_organization ] @@ -46,7 +45,6 @@ update_metrics_dashboard_annotation create_custom_emoji create_package - create_package_settings read_cluster ] end @@ -54,6 +52,7 @@ let(:maintainer_permissions) do %i[ destroy_package + admin_package create_projects create_cluster update_cluster admin_cluster add_cluster ] -- GitLab From 998dbbf3299f3298e78e6622273b85d7d109aaf3 Mon Sep 17 00:00:00 2001 From: Steve Abrams Date: Mon, 9 May 2022 12:43:50 -0600 Subject: [PATCH 2/2] Remove duplicate permission --- spec/policies/namespaces/project_namespace_policy_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/policies/namespaces/project_namespace_policy_spec.rb b/spec/policies/namespaces/project_namespace_policy_spec.rb index 9aa8fed0ef62ca..5ceea9dfb9d1aa 100644 --- a/spec/policies/namespaces/project_namespace_policy_spec.rb +++ b/spec/policies/namespaces/project_namespace_policy_spec.rb @@ -10,7 +10,7 @@ let(:permissions) do [:owner_access, :create_projects, :admin_namespace, :read_namespace, :read_statistics, :transfer_projects, :admin_package, - :admin_package, :create_jira_connect_subscription] + :create_jira_connect_subscription] end subject { described_class.new(current_user, namespace) } -- GitLab