Fix ability when Share lock is off

2 files changed, 62 insertions, 45 deletions

diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb
index e1420de57b8..70622ba553c 100644
--- a/app/policies/group_policy.rb
+++ b/app/policies/group_policy.rb
@@ -15,6 +15,7 @@ class GroupPolicy < BasePolicy
 
   condition(:nested_groups_supported, scope: :global) { Group.supports_nested_groups? }
 
+  condition(:share_locked, scope: :subject) { @subject.share_with_group_lock? }
   condition(:parent_share_locked, scope: :subject) { @subject.parent&.share_with_group_lock? }
   condition(:can_change_parent_share_with_group_lock) { @subject.has_parent? && can?(:change_share_with_group_lock, @subject.parent) }
 
@@ -57,7 +58,7 @@ class GroupPolicy < BasePolicy
   rule { ~can?(:view_globally) }.prevent   :request_access
   rule { has_access }.prevent              :request_access
 
-  rule { owner & (~parent_share_locked | can_change_parent_share_with_group_lock) }.enable :change_share_with_group_lock
+  rule { owner & (~share_locked | ~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 fdf588f6455..0c4044dc7ab 100644
--- a/spec/policies/group_policy_spec.rb
+++ b/spec/policies/group_policy_spec.rb
@@ -244,76 +244,92 @@ describe GroupPolicy do
   end
 
   describe 'change_share_with_group_lock' do
-    context 'when the group has a parent', :nested_groups do
-      let(:group) { create(:group, parent: parent) }
+    context 'when the current_user owns the group' do
+      let(:current_user) { owner }
 
-      context 'when the parent share_with_group_lock is enabled' do
-        let(:current_user) { owner }
+      context 'when the group share_with_group_lock is enabled' do
+        let(:group) { create(:group, share_with_group_lock: true, parent: parent) }
 
-        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) }
+        context 'when the parent group share_with_group_lock is enabled' do
+          context 'when the group has a grandparent' do
+            let(:parent) { create(:group, share_with_group_lock: true, parent: grandparent) }
 
-          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)
+            context 'when the grandparent share_with_group_lock is enabled' do
+              let(:grandparent) { create(:group, share_with_group_lock: true) }
+
+              context 'when the current_user owns the parent' do
+                before do
+                  parent.add_owner(current_user)
+                end
+
+                context 'when the current_user owns the grandparent' do
+                  before do
+                    grandparent.add_owner(current_user)
+                  end
+
+                  it { expect_allowed(:change_share_with_group_lock) }
+                end
+
+                context 'when the current_user does not own the grandparent' do
+                  it { expect_disallowed(:change_share_with_group_lock) }
+                end
               end
 
-              it { expect_allowed(:change_share_with_group_lock) }
+              context 'when the current_user does not own the parent' do
+                it { expect_disallowed(:change_share_with_group_lock) }
+              end
             end
 
-            context 'when current_user owns the parent but not the grandparent' do
-              before do
-                parent.add_owner(owner)
+            context 'when the grandparent share_with_group_lock is disabled' do
+              let(:grandparent) { create(:group) }
+
+              context 'when the current_user owns the parent' do
+                before do
+                  parent.add_owner(current_user)
+                end
+
+                it { expect_allowed(:change_share_with_group_lock) }
               end
 
-              it { expect_disallowed(:change_share_with_group_lock) }
+              context 'when the current_user does not own the parent' do
+                it { expect_disallowed(:change_share_with_group_lock) }
+              end
             end
           end
-        end
 
-        context 'when the group does not have a grandparent' do
-          let(:parent) { create(:group, share_with_group_lock: true) }
+          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
+            context 'when the current_user owns the parent' do
+              before do
+                parent.add_owner(current_user)
+              end
 
-            it { expect_allowed(:change_share_with_group_lock) }
-          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) }
+            context 'when the current_user does not own the parent' do
+              it { expect_disallowed(:change_share_with_group_lock) }
+            end
           end
         end
-      end
 
-      context 'when the parent share_with_group_lock is disabled' do
-        let(:parent) { create(:group) }
-        let(:current_user) { owner }
-
-        context 'when current_user owns the parent' do
-          before do
-            parent.add_owner(owner)
-          end
+        context 'when the parent group share_with_group_lock is disabled' do
+          let(:parent) { create(:group) }
 
           it { expect_allowed(:change_share_with_group_lock) }
         end
+      end
 
-        context 'when current_user owns the group but not the parent' do
-          it { expect_allowed(:change_share_with_group_lock) }
-        end
+      context 'when the group share_with_group_lock is disabled' do
+        it { expect_allowed(:change_share_with_group_lock) }
       end
     end
 
-    context 'when the group does not have a parent' do
-      context 'when current_user owns the group' do
-        let(:current_user) { owner }
+    context 'when the current_user does not own the group' do
+      let(:current_user) { create(:user) }
 
-        it { expect_allowed(:change_share_with_group_lock) }
-      end
+      it { expect_disallowed(:change_share_with_group_lock) }
     end
   end
 end