Merge branch '235822-group-package-permissions' into 'master'

Fix group level package permissions

See merge request gitlab-org/gitlab!43007

app/policies/group_policy.rb

@@ -46,6 +46,16 @@ class GroupPolicy < BasePolicy
     group_projects_for(user: @user, group: @subject, only_owned: false).any? { |p| p.design_management_enabled? }
   end
 
+  desc "Deploy token with read_package_registry scope"
+  condition(:read_package_registry_deploy_token) do
+    @user.is_a?(DeployToken) && @user.groups.include?(@subject) && @user.read_package_registry
+  end
+
+  desc "Deploy token with write_package_registry scope"
+  condition(:write_package_registry_deploy_token) do
+    @user.is_a?(DeployToken) && @user.groups.include?(@subject) && @user.write_package_registry
+  end
+
   rule { design_management_enabled }.policy do
     enable :read_design_activity
   end
@@ -91,7 +101,6 @@ class GroupPolicy < BasePolicy
 
   rule { developer }.policy do
     enable :admin_milestone
-    enable :read_package
     enable :create_metrics_dashboard_annotation
     enable :delete_metrics_dashboard_annotation
     enable :update_metrics_dashboard_annotation
@@ -105,6 +114,7 @@ class GroupPolicy < BasePolicy
     enable :admin_issue
     enable :read_metrics_dashboard_annotation
     enable :read_prometheus
+    enable :read_package
   end
 
   rule { maintainer }.policy do
@@ -167,6 +177,16 @@ class GroupPolicy < BasePolicy
 
   rule { maintainer & can?(:create_projects) }.enable :transfer_projects
 
+  rule { read_package_registry_deploy_token }.policy do
+    enable :read_package
+    enable :read_group
+  end
+
+  rule { write_package_registry_deploy_token }.policy do
+    enable :create_package
+    enable :read_group
+  end
+
   def access_level
     return GroupMember::NO_ACCESS if @user.nil?
     return GroupMember::NO_ACCESS unless user_is_user?

changelogs/unreleased/235822-group-package-permissions.yml

+---
+title: Fix group deploy tokens permissions for package access
+merge_request: 43007
+author:
+type: fixed

spec/policies/group_policy_spec.rb

@@ -812,4 +812,72 @@ RSpec.describe GroupPolicy do
       it { is_expected.to be_disallowed(:create_jira_connect_subscription) }
     end
   end
+
+  describe 'read_package' do
+    context 'admin' do
+      let(:current_user) { admin }
+
+      it { is_expected.to be_allowed(:read_package) }
+    end
+
+    context 'with owner' do
+      let(:current_user) { owner }
+
+      it { is_expected.to be_allowed(:read_package) }
+    end
+
+    context 'with maintainer' do
+      let(:current_user) { maintainer }
+
+      it { is_expected.to be_allowed(:read_package) }
+    end
+
+    context 'with reporter' do
+      let(:current_user) { reporter }
+
+      it { is_expected.to be_allowed(:read_package) }
+    end
+
+    context 'with guest' do
+      let(:current_user) { guest }
+
+      it { is_expected.to be_disallowed(:read_package) }
+    end
+
+    context 'with non member' do
+      let(:current_user) { create(:user) }
+
+      it { is_expected.to be_disallowed(:read_package) }
+    end
+
+    context 'with anonymous' do
+      let(:current_user) { nil }
+
+      it { is_expected.to be_disallowed(:read_package) }
+    end
+  end
+
+  context 'deploy token access' do
+    let!(:group_deploy_token) do
+      create(:group_deploy_token, group: group, deploy_token: deploy_token)
+    end
+
+    subject { described_class.new(deploy_token, group) }
+
+    context 'a deploy token with read_package_registry scope' do
+      let(:deploy_token) { create(:deploy_token, :group, read_package_registry: true) }
+
+      it { is_expected.to be_allowed(:read_package) }
+      it { is_expected.to be_allowed(:read_group) }
+      it { is_expected.to be_disallowed(:create_package) }
+    end
+
+    context 'a deploy token with write_package_registry scope' do
+      let(:deploy_token) { create(:deploy_token, :group, write_package_registry: true) }
+
+      it { is_expected.to be_allowed(:create_package) }
+      it { is_expected.to be_allowed(:read_group) }
+      it { is_expected.to be_disallowed(:destroy_package) }
+    end
+  end
 end

spec/requests/api/group_packages_spec.rb

@@ -77,7 +77,7 @@ RSpec.describe API::GroupPackages do
         it_behaves_like 'returns packages', :group, :owner
         it_behaves_like 'returns packages', :group, :maintainer
         it_behaves_like 'returns packages', :group, :developer
-        it_behaves_like 'rejects packages access', :group, :reporter, :forbidden
+        it_behaves_like 'returns packages', :group, :reporter
         it_behaves_like 'rejects packages access', :group, :guest, :forbidden
 
         context 'with subgroup' do
@@ -88,7 +88,7 @@ RSpec.describe API::GroupPackages do
           it_behaves_like 'returns packages with subgroups', :group, :owner
           it_behaves_like 'returns packages with subgroups', :group, :maintainer
           it_behaves_like 'returns packages with subgroups', :group, :developer
-          it_behaves_like 'rejects packages access', :group, :reporter, :forbidden
+          it_behaves_like 'returns packages with subgroups', :group, :reporter
           it_behaves_like 'rejects packages access', :group, :guest, :forbidden
 
           context 'excluding subgroup' do
@@ -97,7 +97,7 @@ RSpec.describe API::GroupPackages do
             it_behaves_like 'returns packages', :group, :owner
             it_behaves_like 'returns packages', :group, :maintainer
             it_behaves_like 'returns packages', :group, :developer
-            it_behaves_like 'rejects packages access', :group, :reporter, :forbidden
+            it_behaves_like 'returns packages', :group, :reporter
             it_behaves_like 'rejects packages access', :group, :guest, :forbidden
           end
         end