Skip to content
Snippets Groups Projects
Commit 93472e2c authored by Moaz Khalifa's avatar Moaz Khalifa Committed by David Fernandez
Browse files

Raise the permissions of the group-level APIs to owner

Update the permissions level of the GitLab API to require Owner level permissions to CRUD any Package stage-related settings.

Changelog: changed
parent 65342c49
No related branches found
No related tags found
1 merge request!118360Raise the permissions of the group-level APIs to owner
Showing
with 223 additions and 29 deletions
......@@ -8,10 +8,11 @@ class Update < Mutations::BaseMutation
include Mutations::ResolvesGroup
description 'These settings can be adjusted by the group Owner or Maintainer. However, in GitLab 16.0, we ' \
'will be limiting this to the Owner role. ' \
'[GitLab-#364441](https://gitlab.com/gitlab-org/gitlab/-/issues/364441) proposes making ' \
'this change to match the permissions level in the user interface.'
description <<~DESC
These settings can be adjusted by the group Owner or Maintainer.
[Issue 370471](https://gitlab.com/gitlab-org/gitlab/-/issues/370471) proposes limiting
this to Owners only to match the permissions level in the user interface.
DESC
authorize :admin_dependency_proxy
......
......@@ -8,6 +8,12 @@ class Update < Mutations::BaseMutation
include Mutations::ResolvesGroup
description <<~DESC
These settings can be adjusted by the group Owner or Maintainer.
[Issue 370471](https://gitlab.com/gitlab-org/gitlab/-/issues/370471) proposes limiting
this to Owners only to match the permissions level in the user interface.
DESC
authorize :admin_dependency_proxy
argument :group_path,
......
......@@ -8,6 +8,12 @@ class Update < Mutations::BaseMutation
include Mutations::ResolvesNamespace
description <<~DESC
These settings can be adjusted by the group Owner or Maintainer.
[Issue 370471](https://gitlab.com/gitlab-org/gitlab/-/issues/370471) proposes limiting
this to Owners only to match the permissions level in the user interface.
DESC
authorize :admin_package
argument :namespace_path,
......
......@@ -109,6 +109,10 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
@subject.runner_registration_enabled?
end
condition(:raise_admin_package_to_owner_enabled) do
Feature.enabled?(:raise_group_admin_package_permission_to_owner, @subject)
end
rule { can?(:read_group) & design_management_enabled }.policy do
enable :read_design_activity
end
......@@ -202,7 +206,6 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
rule { maintainer }.policy do
enable :destroy_package
enable :admin_package
enable :create_projects
enable :import_projects
enable :admin_pipeline
......@@ -304,7 +307,11 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
rule { dependency_proxy_access_allowed & dependency_proxy_available }
.enable :read_dependency_proxy
rule { maintainer & dependency_proxy_available }.policy do
rule { maintainer & dependency_proxy_available & ~raise_admin_package_to_owner_enabled }.policy do
enable :admin_dependency_proxy
end
rule { owner & dependency_proxy_available & raise_admin_package_to_owner_enabled }.policy do
enable :admin_dependency_proxy
end
......@@ -370,6 +377,9 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
# Should be matched with ProjectPolicy#read_internal_note
rule { admin | reporter }.enable :read_internal_note
rule { maintainer & ~raise_admin_package_to_owner_enabled }.enable :admin_package
rule { owner & raise_admin_package_to_owner_enabled }.enable :admin_package
def access_level(for_any_session: false)
return GroupMember::NO_ACCESS if @user.nil?
return GroupMember::NO_ACCESS unless user_is_user?
......
---
name: raise_group_admin_package_permission_to_owner
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/118360
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/409642
milestone: '16.1'
type: development
group: group::package registry
default_enabled: false
......@@ -6375,6 +6375,10 @@ Input type: `UpdateContainerExpirationPolicyInput`
 
### `Mutation.updateDependencyProxyImageTtlGroupPolicy`
 
These settings can be adjusted by the group Owner or Maintainer.
[Issue 370471](https://gitlab.com/gitlab-org/gitlab/-/issues/370471) proposes limiting
this to Owners only to match the permissions level in the user interface.
Input type: `UpdateDependencyProxyImageTtlGroupPolicyInput`
 
#### Arguments
......@@ -6396,7 +6400,9 @@ Input type: `UpdateDependencyProxyImageTtlGroupPolicyInput`
 
### `Mutation.updateDependencyProxySettings`
 
These settings can be adjusted by the group Owner or Maintainer. However, in GitLab 16.0, we will be limiting this to the Owner role. [GitLab-#364441](https://gitlab.com/gitlab-org/gitlab/-/issues/364441) proposes making this change to match the permissions level in the user interface.
These settings can be adjusted by the group Owner or Maintainer.
[Issue 370471](https://gitlab.com/gitlab-org/gitlab/-/issues/370471) proposes limiting
this to Owners only to match the permissions level in the user interface.
 
Input type: `UpdateDependencyProxySettingsInput`
 
......@@ -6557,6 +6563,10 @@ Input type: `UpdateIterationInput`
 
### `Mutation.updateNamespacePackageSettings`
 
These settings can be adjusted by the group Owner or Maintainer.
[Issue 370471](https://gitlab.com/gitlab-org/gitlab/-/issues/370471) proposes limiting
this to Owners only to match the permissions level in the user interface.
Input type: `UpdateNamespacePackageSettingsInput`
 
#### Arguments
......@@ -35,6 +35,8 @@ For a list of planned additions, view the
## Enable or turn off the Dependency Proxy for a group
> Required role [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/350682) from Developer to Maintainer in GitLab 15.0.
To enable or turn off the Dependency Proxy for a group:
1. On the top bar, select **Main menu > Groups** and find your group.
......
......@@ -26,7 +26,7 @@ image or tag from Docker Hub.
## Cleanup policies
> [Required permissions](https://gitlab.com/gitlab-org/gitlab/-/issues/350682) changed from developer to maintainer in GitLab 15.0.
> Required role [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/350682) from Developer to Maintainer in GitLab 15.0.
### Enable cleanup policies from within GitLab
......
......@@ -118,7 +118,7 @@ API or the UI.
#### Do not allow duplicate Generic packages
> - [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.
> - Required role [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/350682) 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.
......
......@@ -474,6 +474,8 @@ To delete older package versions, consider using the Packages API or the UI.
### Do not allow duplicate Maven packages
> Required role [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/350682) 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.
In the UI:
......
......@@ -899,6 +899,16 @@ def stub_group_saml_config(enabled)
context 'with ip restriction' do
let(:current_user) { maintainer }
# To be removed when raise_group_admin_package_permission_to_owner FF is removed
shared_examples 'disabling admin_package feature flag' do
before do
stub_feature_flags(raise_group_admin_package_permission_to_owner: false)
end
it { is_expected.to be_allowed(:admin_package) }
it { is_expected.to be_allowed(:admin_dependency_proxy) }
end
before do
allow(Gitlab::IpAddressState).to receive(:current).and_return('192.168.0.2')
stub_licensed_features(group_ip_restriction: true)
......@@ -911,9 +921,11 @@ def stub_group_saml_config(enabled)
it { is_expected.to be_allowed(:read_package) }
it { is_expected.to be_allowed(:create_package) }
it { is_expected.to be_allowed(:destroy_package) }
it { is_expected.to be_allowed(:admin_package) }
it { is_expected.to be_allowed(:read_dependency_proxy) }
it { is_expected.to be_allowed(:admin_dependency_proxy) }
it { is_expected.to be_disallowed(:admin_package) }
it { is_expected.to be_disallowed(:admin_dependency_proxy) }
it_behaves_like 'disabling admin_package feature flag'
end
context 'with restriction' do
......@@ -929,9 +941,11 @@ def stub_group_saml_config(enabled)
it { is_expected.to be_allowed(:read_package) }
it { is_expected.to be_allowed(:create_package) }
it { is_expected.to be_allowed(:destroy_package) }
it { is_expected.to be_allowed(:admin_package) }
it { is_expected.to be_allowed(:read_dependency_proxy) }
it { is_expected.to be_allowed(:admin_dependency_proxy) }
it { is_expected.to be_disallowed(:admin_package) }
it { is_expected.to be_disallowed(:admin_dependency_proxy) }
it_behaves_like 'disabling admin_package feature flag'
end
context 'address is outside the range' do
......
......@@ -10,6 +10,15 @@
context 'with ip restriction' do
let(:current_user) { maintainer }
# To be removed when raise_group_admin_package_permission_to_owner FF is removed
shared_examples 'disabling admin_package feature flag' do
before do
stub_feature_flags(raise_group_admin_package_permission_to_owner: false)
end
it { is_expected.to be_allowed(:admin_package) }
end
before do
allow(Gitlab::IpAddressState).to receive(:current).and_return('192.168.0.2')
stub_licensed_features(group_ip_restriction: true)
......@@ -20,7 +29,9 @@
it { is_expected.to be_allowed(:read_package) }
it { is_expected.to be_allowed(:create_package) }
it { is_expected.to be_allowed(:destroy_package) }
it { is_expected.to be_allowed(:admin_package) }
it { is_expected.to be_disallowed(:admin_package) }
it_behaves_like 'disabling admin_package feature flag'
end
context 'with restriction' do
......@@ -34,7 +45,9 @@
it { is_expected.to be_allowed(:read_package) }
it { is_expected.to be_allowed(:create_package) }
it { is_expected.to be_allowed(:destroy_package) }
it { is_expected.to be_allowed(:admin_package) }
it { is_expected.to be_disallowed(:admin_package) }
it_behaves_like 'disabling admin_package feature flag'
end
context 'with address is outside the range' do
......
......@@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe Mutations::DependencyProxy::GroupSettings::Update do
RSpec.describe Mutations::DependencyProxy::GroupSettings::Update, feature_category: :dependency_proxy do
using RSpec::Parameterized::TableSyntax
let_it_be_with_reload(:group) { create(:group) }
......@@ -36,7 +36,8 @@
end
where(:user_role, :shared_examples_name) do
:maintainer | 'updating the dependency proxy group settings'
:owner | 'updating the dependency proxy group settings'
:maintainer | 'denying access to dependency proxy group settings'
:developer | 'denying access to dependency proxy group settings'
:reporter | 'denying access to dependency proxy group settings'
:guest | 'denying access to dependency proxy group settings'
......@@ -50,6 +51,14 @@
end
it_behaves_like params[:shared_examples_name]
context 'with disabled admin_package feature flag' do
before do
stub_feature_flags(raise_group_admin_package_permission_to_owner: false)
end
it_behaves_like 'updating the dependency proxy group settings' if params[:user_role] == :maintainer
end
end
end
end
......@@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe Mutations::DependencyProxy::ImageTtlGroupPolicy::Update do
RSpec.describe Mutations::DependencyProxy::ImageTtlGroupPolicy::Update, feature_category: :dependency_proxy do
using RSpec::Parameterized::TableSyntax
let_it_be_with_reload(:group) { create(:group) }
......@@ -58,6 +58,15 @@
end
end
# To be removed when raise_group_admin_package_permission_to_owner FF is removed
shared_examples 'disabling admin_package feature flag' do |action:|
before do
stub_feature_flags(raise_group_admin_package_permission_to_owner: false)
end
it_behaves_like "#{action} the dependency proxy image ttl policy"
end
before do
stub_config(dependency_proxy: { enabled: true })
end
......@@ -71,7 +80,8 @@
end
where(:user_role, :shared_examples_name) do
:maintainer | 'updating the dependency proxy image ttl policy'
:owner | 'updating the dependency proxy image ttl policy'
:maintainer | 'denying access to dependency proxy image ttl policy'
:developer | 'denying access to dependency proxy image ttl policy'
:reporter | 'denying access to dependency proxy image ttl policy'
:guest | 'denying access to dependency proxy image ttl policy'
......@@ -84,6 +94,7 @@
end
it_behaves_like params[:shared_examples_name]
it_behaves_like 'disabling admin_package feature flag', action: :updating if params[:user_role] == :maintainer
end
end
......@@ -91,7 +102,8 @@
let_it_be(:ttl_policy) { group.dependency_proxy_image_ttl_policy }
where(:user_role, :shared_examples_name) do
:maintainer | 'creating the dependency proxy image ttl policy'
:owner | 'creating the dependency proxy image ttl policy'
:maintainer | 'denying access to dependency proxy image ttl policy'
:developer | 'denying access to dependency proxy image ttl policy'
:reporter | 'denying access to dependency proxy image ttl policy'
:guest | 'denying access to dependency proxy image ttl policy'
......@@ -104,6 +116,7 @@
end
it_behaves_like params[:shared_examples_name]
it_behaves_like 'disabling admin_package feature flag', action: :creating if params[:user_role] == :maintainer
end
end
end
......
......@@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe Mutations::Namespace::PackageSettings::Update do
RSpec.describe Mutations::Namespace::PackageSettings::Update, feature_category: :package_registry do
using RSpec::Parameterized::TableSyntax
let_it_be_with_reload(:namespace) { create(:group) }
......@@ -77,6 +77,15 @@
end
end
# To be removed when raise_group_admin_package_permission_to_owner FF is removed
RSpec.shared_examples 'disabling admin_package feature flag' do |action:|
before do
stub_feature_flags(raise_group_admin_package_permission_to_owner: false)
end
it_behaves_like "#{action} the namespace package setting"
end
context 'with existing namespace package setting' do
let_it_be(:package_settings) { create(:namespace_package_setting, namespace: namespace) }
let_it_be(:params) do
......@@ -96,7 +105,8 @@
end
where(:user_role, :shared_examples_name) do
:maintainer | 'updating the namespace package setting'
:owner | 'updating the namespace package setting'
:maintainer | 'denying access to namespace package setting'
:developer | 'denying access to namespace package setting'
:reporter | 'denying access to namespace package setting'
:guest | 'denying access to namespace package setting'
......@@ -109,6 +119,7 @@
end
it_behaves_like params[:shared_examples_name]
it_behaves_like 'disabling admin_package feature flag', action: :updating if params[:user_role] == :maintainer
end
end
......@@ -116,7 +127,8 @@
let_it_be(:package_settings) { namespace.package_settings }
where(:user_role, :shared_examples_name) do
:maintainer | 'creating the namespace package setting'
:owner | 'creating the namespace package setting'
:maintainer | 'denying access to namespace package setting'
:developer | 'denying access to namespace package setting'
:reporter | 'denying access to namespace package setting'
:guest | 'denying access to namespace package setting'
......@@ -129,6 +141,7 @@
end
it_behaves_like params[:shared_examples_name]
it_behaves_like 'disabling admin_package feature flag', action: :creating if params[:user_role] == :maintainer
end
end
end
......
......@@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe GitlabSchema.types['DependencyProxyImageTtlGroupPolicy'] do
RSpec.describe GitlabSchema.types['DependencyProxyImageTtlGroupPolicy'], feature_category: :dependency_proxy do
it { expect(described_class.graphql_name).to eq('DependencyProxyImageTtlGroupPolicy') }
it { expect(described_class.description).to eq('Group-level Dependency Proxy TTL policy settings') }
......
......@@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe GitlabSchema.types['PackageSettings'] do
RSpec.describe GitlabSchema.types['PackageSettings'], feature_category: :package_registry do
specify { expect(described_class.graphql_name).to eq('PackageSettings') }
specify { expect(described_class.description).to eq('Namespace-level Package Registry settings') }
......
......@@ -1167,6 +1167,14 @@
end
describe 'dependency proxy' do
RSpec.shared_examples 'disabling admin_package feature flag' do
before do
stub_feature_flags(raise_group_admin_package_permission_to_owner: false)
end
it { is_expected.to be_allowed(:admin_dependency_proxy) }
end
context 'feature disabled' do
let(:current_user) { owner }
......@@ -1196,8 +1204,19 @@
context 'maintainer' do
let(:current_user) { maintainer }
it { is_expected.to be_allowed(:read_dependency_proxy) }
it { is_expected.to be_disallowed(:admin_dependency_proxy) }
it_behaves_like 'disabling admin_package feature flag'
end
context 'owner' do
let(:current_user) { owner }
it { is_expected.to be_allowed(:read_dependency_proxy) }
it { is_expected.to be_allowed(:admin_dependency_proxy) }
it_behaves_like 'disabling admin_package feature flag'
end
end
end
......@@ -1760,4 +1779,38 @@
specify { is_expected.to be_allowed(:read_achievement) }
end
end
describe 'admin_package ability' do
context 'with maintainer' do
let(:current_user) { maintainer }
context 'with feature flag enabled' do
specify { is_expected.to be_disallowed(:admin_package) }
end
context 'with feature flag disabled' do
before do
stub_feature_flags(raise_group_admin_package_permission_to_owner: false)
end
specify { is_expected.to be_allowed(:admin_package) }
end
end
context 'with owner' do
let(:current_user) { owner }
context 'with feature flag enabled' do
specify { is_expected.to be_allowed(:admin_package) }
end
context 'with feature flag disabled' do
before do
stub_feature_flags(raise_group_admin_package_permission_to_owner: false)
end
specify { is_expected.to be_allowed(:admin_package) }
end
end
end
end
......@@ -46,12 +46,15 @@
context 'with different permissions' do
where(:group_visibility, :role, :access_granted) do
:private | :maintainer | true
:private | :owner | true
:private | :maintainer | false
:private | :developer | false
:private | :reporter | false
:private | :guest | false
:private | :anonymous | false
:public | :maintainer | true
:public | :owner | true
:public | :maintainer | false
:public | :developer | false
:public | :reporter | false
:public | :guest | false
......@@ -73,6 +76,20 @@
expect(dependency_proxy_group_setting_response).to be_blank
end
end
context 'with disabled admin_package feature flag' do
before do
stub_feature_flags(raise_group_admin_package_permission_to_owner: false)
end
if params[:role] == :maintainer
it 'return the proper response' do
subject
expect(dependency_proxy_group_setting_response).to eq('enabled' => true)
end
end
end
end
end
end
......
......@@ -45,12 +45,15 @@
context 'with different permissions' do
where(:group_visibility, :role, :access_granted) do
:private | :maintainer | true
:private | :owner | true
:private | :maintainer | false
:private | :developer | false
:private | :reporter | false
:private | :guest | false
:private | :anonymous | false
:public | :maintainer | true
:public | :owner | true
:public | :maintainer | false
:public | :developer | false
:public | :reporter | false
:public | :guest | false
......@@ -72,6 +75,20 @@
expect(dependency_proxy_image_ttl_policy_response).to be_blank
end
end
context 'when the feature flag is disabled' do
before do
stub_feature_flags(raise_group_admin_package_permission_to_owner: false)
end
if params[:role] == :maintainer
it 'returns the proper response' do
subject
expect(dependency_proxy_image_ttl_policy_response).to eq("createdAt" => nil, "enabled" => false, "ttl" => 90, "updatedAt" => nil)
end
end
end
end
end
end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment