Skip to content

Prevent removing last group owner if that owner is deactivated

Rutger Wessels requested to merge rw-check-at-least-one-owner-left into master

What does this MR do and why?

A group should not be left without an owner:

  • If an owner is blocked and it is the last owner of the group, we do not allow to delete the owner.
  • But if the owner is deactivated, it is possible to remove the owner and the result is that the group does not have an owner anymore.

This MR will fix this: a group should always be left with one owner (or parent group with owner)

Issue: #356909 (closed)

Implementation

Currently, an Owner can be removed if it is an active user or if it a blocked user. A natural solution would be to include an additional check for this: we can also remove owners that are deactivated. However, we should separate the state of the User from the possibility of removing the last owner. See this comment for reference.

So this MR will implement the change by removing the dependency on user state (active or blocked). It will instead allow deleting of an owner if these conditions are met:

  • There must be at least one remaining owner
  • An owner can also come from a group higher up in the hierarchy
  • The remaining owner must not be a project bot

Additionally, we could remove some code:

  • method Group.member_last_blocked_owner? git grep member_last_blocked_owner does not show anything. git grep BlockedOwner is also empty
  • property GroupMember.last_blocked_owner git grep last_blocked_owner does not show anything.

Screenshots or screen recordings

Master branch: I can delete deactivated user: image

This branch: Not possible:

image

How to set up and validate locally

Setup:

  • I used local development (gdk) using 'root' account
  • Add an owner to a group
  • Remove 'administrator' ("Leave Group")
  • As an administrator, deactivate the remaining owner

Master branch:

  • Remove the non-deactivated owner. It will allow it

This branch:

  • Remove the non-deactivated owner. It will show a popup

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Rutger Wessels

Merge request reports