Merge pull request #876 from code0-tech/851-prevent-role-deletion-if-no-member-has-a-different-admin-role

Prevent role deletion if no member has a different admin role

Author: Taucher2003

app/services/namespaces/roles/assign_abilities_service.rb

@@ -63,6 +63,7 @@ def check_admin_existing(t)
 
         unless role.namespace.roles.where.not(id: role.id)
                    .joins(:abilities)
+                   .joins(:member_roles)
                    .exists?(abilities: { ability: :namespace_administrator }) ||
                abilities.include?(:namespace_administrator)
           t.rollback_and_return! ServiceResponse.error(

app/services/namespaces/roles/delete_service.rb

@@ -20,6 +20,7 @@ def execute
         if !namespace_role.namespace.has_owner? &&
            !namespace_role.namespace.roles.where.not(id: namespace_role.id)
                           .joins(:abilities)
+                          .joins(:member_roles)
                           .exists?(abilities: { ability: :namespace_administrator })
           return ServiceResponse.error(message: 'Cannot delete last administrator role',
                                        error_code: :cannot_delete_last_admin_role)

spec/requests/graphql/mutation/namespace/roles/assign_abilities_mutation_spec.rb

@@ -30,6 +30,7 @@
   before do
     create(:namespace_role, namespace: namespace_role.namespace).tap do |role|
       create(:namespace_role_ability, namespace_role: role, ability: :namespace_administrator)
+      create(:namespace_member_role, role: role, member: create(:namespace_member, namespace: role.namespace))
     end
   end
 

spec/requests/graphql/mutation/namespace/roles/delete_mutation_spec.rb

@@ -38,6 +38,7 @@
   before do
     create(:namespace_role, namespace: namespace).tap do |role|
       create(:namespace_role_ability, namespace_role: role, ability: :namespace_administrator)
+      create(:namespace_member_role, role: role, member: create(:namespace_member, namespace: role.namespace))
     end
   end
 

spec/services/namespaces/roles/assign_abilities_service_spec.rb

@@ -9,12 +9,17 @@
   let(:role) { create(:namespace_role) }
   let(:abilities) { [] }
 
-  let!(:admin_role) do
+  let(:admin_role) do
     create(:namespace_role, namespace: role.namespace).tap do |role|
       create(:namespace_role_ability, namespace_role: role, ability: :namespace_administrator)
     end
   end
 
+  let!(:admin_role_member) do
+    member = create(:namespace_member, namespace: admin_role.namespace, user: create(:user))
+    create(:namespace_member_role, role: admin_role, member: member)
+  end
+
   context 'when user is nil' do
     let(:current_user) { nil }
 
@@ -74,6 +79,24 @@
           expect { service_response }.not_to create_audit_event
         end
       end
+
+      context 'when another role has the namespace_administrator ability but no members' do
+        let(:abilities) { [] }
+
+        before do
+          stub_allowed_ability(NamespacePolicy, :assign_role_abilities, user: current_user, subject: role.namespace)
+          create(:namespace_role_ability, namespace_role: role, ability: :namespace_administrator)
+          admin_role_member.delete
+        end
+
+        it { is_expected.not_to be_success }
+        it { expect(service_response.payload[:error_code]).to eq(:cannot_remove_last_admin_ability) }
+        it { expect { service_response }.not_to change { NamespaceRoleAbility.count } }
+
+        it do
+          expect { service_response }.not_to create_audit_event
+        end
+      end
     end
 
     context 'when adding an ability' do

spec/services/namespaces/roles/delete_service_spec.rb

@@ -48,18 +48,41 @@
     end
 
     it { is_expected.not_to be_success }
+    it { expect(service_response.payload[:error_code]).to eq(:cannot_delete_last_admin_role) }
     it { expect { service_response }.not_to change { NamespaceRole.count } }
 
     it do
       expect { service_response }.not_to create_audit_event
     end
   end
 
-  context 'when user is a member' do
+  context 'when another role has the namespace_administrator ability but no members' do
     let(:current_user) { create(:user) }
 
     before do
       create(:namespace_member, namespace: namespace_role.namespace, user: current_user)
+      create(:namespace_role_ability, namespace_role: namespace_role, ability: :namespace_administrator)
+      create(:namespace_member_role, member: create(:namespace_member, namespace: namespace_role.namespace),
+                                     role: namespace_role)
+      stub_allowed_ability(NamespacePolicy, :delete_namespace_role, user: current_user,
+                                                                    subject: namespace_role.namespace)
+    end
+
+    it { is_expected.not_to be_success }
+    it { expect(service_response.payload[:error_code]).to eq(:cannot_delete_last_admin_role) }
+    it { expect { service_response }.not_to change { NamespaceRole.count } }
+
+    it do
+      expect { service_response }.not_to create_audit_event
+    end
+  end
+
+  context 'when user is a member' do
+    let(:current_user) { create(:user) }
+
+    before do
+      member = create(:namespace_member, namespace: namespace_role.namespace, user: current_user)
+      create(:namespace_member_role, member: member, role: admin_role)
       stub_allowed_ability(NamespacePolicy, :delete_namespace_role, user: current_user,
                                                                     subject: namespace_role.namespace)
     end