Fix “Share lock” policy for deeply nested groups

2 files changed, 36 insertions, 10 deletions

diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb
index dab27bbf6da..45416bb7149 100644
--- a/app/policies/group_policy.rb
+++ b/app/policies/group_policy.rb
@@ -16,7 +16,7 @@ class GroupPolicy < BasePolicy
   condition(:nested_groups_supported, scope: :global) { Group.supports_nested_groups? }
 
   condition(:parent_share_locked) { @subject.has_parent? && @subject.parent.share_with_group_lock? }
-  condition(:parent_owner) { @subject.has_parent? && @subject.parent.has_owner?(@user) }
+  condition(:can_change_parent_share_with_group_lock) { @subject.has_parent? && can?(:change_share_with_group_lock, @subject.parent) }
 
   condition(:has_projects) do
     GroupProjectsFinder.new(group: @subject, current_user: @user).execute.any?
@@ -57,7 +57,7 @@ class GroupPolicy < BasePolicy
   rule { ~can?(:view_globally) }.prevent   :request_access
   rule { has_access }.prevent              :request_access
 
-  rule { owner & (~parent_share_locked | parent_owner) }.enable :change_share_with_group_lock
+  rule { owner & (~parent_share_locked | can_change_parent_share_with_group_lock) }.enable :change_share_with_group_lock
 
   def access_level
     return GroupMember::NO_ACCESS if @user.nil?
diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb
index dcc7b7d5b9c..fdf588f6455 100644
--- a/spec/policies/group_policy_spec.rb
+++ b/spec/policies/group_policy_spec.rb
@@ -248,19 +248,45 @@ describe GroupPolicy do
       let(:group) { create(:group, parent: parent) }
 
       context 'when the parent share_with_group_lock is enabled' do
-        let(:parent) { create(:group, share_with_group_lock: true) }
         let(:current_user) { owner }
 
-        context 'when current_user owns the parent' do
-          before do
-            parent.add_owner(owner)
-          end
+        context 'when the group has a grandparent' do
+          let(:grandparent) { create(:group, share_with_group_lock: true) }
+          let(:parent) { create(:group, share_with_group_lock: true, parent: grandparent) }
 
-          it { expect_allowed(:change_share_with_group_lock) }
+          context 'and the grandparent share_with_group_lock is enabled' do
+            context 'when current_user owns the grandparent' do
+              before do
+                grandparent.add_owner(owner)
+              end
+
+              it { expect_allowed(:change_share_with_group_lock) }
+            end
+
+            context 'when current_user owns the parent but not the grandparent' do
+              before do
+                parent.add_owner(owner)
+              end
+
+              it { expect_disallowed(:change_share_with_group_lock) }
+            end
+          end
         end
 
-        context 'when current_user owns the group but not the parent' do
-          it { expect_disallowed(:change_share_with_group_lock) }
+        context 'when the group does not have a grandparent' do
+          let(:parent) { create(:group, share_with_group_lock: true) }
+
+          context 'when current_user owns the parent' do
+            before do
+              parent.add_owner(owner)
+            end
+
+            it { expect_allowed(:change_share_with_group_lock) }
+          end
+
+          context 'when current_user owns the group but not the parent' do
+            it { expect_disallowed(:change_share_with_group_lock) }
+          end
         end
       end