fix(php): Check if an user is a space manager

I have checked incorrectly whether an user is a space manager of another space. Signed-off-by: Baptiste Fotia <[email protected]>
arawa · Mar 26, 2024 · ee756d4 · ee756d4
1 parent 3d7840a
commit ee756d4
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 16 deletions.
diff --git a/lib/Controller/GroupController.php b/lib/Controller/GroupController.php
@@ -261,7 +261,9 @@ public function removeUser(array|string $space,
 				$this->logger->debug('User removed from group: ' . $NCGroup->getDisplayName());
 				if ($groupId === WorkspaceManagerGroup::get($space['id'])) {
 					$this->logger->debug('Removing user from a workspace manager group, removing it from the WorkspacesManagers group if needed.');
-					$this->userService->removeGEFromWM($NCUser, $space);
+					if ($this->userService->canRemoveWorkspaceManagers($NCUser, $space)) {
+						$this->userService->removeGEFromWM($NCUser);
+					}
 				}
 			}
 		} else {
@@ -272,7 +274,9 @@ public function removeUser(array|string $space,
 			if ($gid === WorkspaceManagerGroup::get($space['id'])) {
 				// Removing user from a GE- group
 				$this->logger->debug('Removing user from a workspace manager group, removing it from the WorkspacesManagers group if needed.');
-				$this->userService->removeGEFromWM($NCUser, $space);
+				if ($this->userService->canRemoveWorkspaceManagers($NCUser, $space)) {
+					$this->userService->removeGEFromWM($NCUser);
+				}
 			}
 		}
 

diff --git a/lib/Controller/WorkspaceController.php b/lib/Controller/WorkspaceController.php
@@ -137,7 +137,9 @@ public function destroy(array $workspace): JSONResponse {
 		$this->logger->debug('Removing GE users from the WorkspacesManagers group if needed.');
 		$GEGroup = $this->groupManager->get(WorkspaceManagerGroup::get($workspace['id']));
 		foreach ($GEGroup->getUsers() as $user) {
-			$this->userService->removeGEFromWM($user, $workspace);
+			if ($this->userService->canRemoveWorkspaceManagers($user, $workspace)) {
+				$this->userService->removeGEFromWM($user);
+			}
 		}
 
 		// Removes all workspaces groups
@@ -245,7 +247,9 @@ public function changeUserRole(array|string $space,
 			// Changing a user's role from admin to user
 			$GEgroup->removeUser($user);
 			$this->logger->debug('Removing a user from a GE group. Removing it from the ' . ManagersWorkspace::WORKSPACES_MANAGERS . ' group if needed.');
-			$this->userService->removeGEFromWM($user, $space);
+			if ($this->userService->canRemoveWorkspaceManagers($user, $space)) {
+				$this->userService->removeGEFromWM($user, $space);
+			}
 		} else {
 			// Changing a user's role from user to admin
 			$this->groupManager->get(WorkspaceManagerGroup::get($space['id']))->addUser($user);

diff --git a/lib/Service/UserService.php b/lib/Service/UserService.php
@@ -123,33 +123,46 @@ public function isSpaceManagerOfSpace(array $space): bool {
 		return false;
 	}
 
+
 	/**
-	 * This function removes a GE from the WorkspaceManagers group when necessary
+	 * Return `true` if the user can be removed from workspace manager group (SPACE-GE), Otherwise, `false`.
+	 *
+	 * @param IUser $user
+	 * @param array|object $space
+	 * @return boolean
 	 */
-	public function removeGEFromWM(IUser $user, array|object $space): void {
-		$found = false;
+	public function canRemoveWorkspaceManagers(IUser $user, array|object $space): bool {
+		$canRemove = true;
 		$groups = $this->groupManager->getUserGroups($user);
 
-		// Checks if the user is member of the GE- group of another workspace
 		foreach ($groups as $group) {
 			$gid = $group->getGID();
-			if (strpos($gid, WorkspaceManagerGroup::get($space['id'])) === 0 &&
+			if (strpos($gid, WorkspaceManagerGroup::getPrefix()) === 0 &&
 				$gid !== UserGroup::get($space['id'])
 			) {
-				$found = true;
+				$canRemove = false;
 				break;
 			}
 		}
 
-		// Removing the user from the WorkspacesManagers group if needed
-		if (!$found) {
-			$this->logger->debug('User is not manager of any other workspace, removing it from the ' . ManagersWorkspace::WORKSPACES_MANAGERS . ' group.');
-			$workspaceUserGroup = $this->groupManager->get(ManagersWorkspace::WORKSPACES_MANAGERS);
-			$workspaceUserGroup->removeUser($user);
-		} else {
+		if (!$canRemove) {
 			$this->logger->debug('User is still manager of other workspaces, will not remove it from the ' . ManagersWorkspace::WORKSPACES_MANAGERS . ' group.');
 		}
 
+		return $canRemove;
+	}
+
+	/**
+	 * This function removes a GE from the WorkspaceManagers group when necessary
+	 *
+	 * @param IUser $user
+	 * @return void
+	 */
+	public function removeGEFromWM(IUser $user): void {
+		$this->logger->debug('User is not manager of any other workspace, removing it from the ' . ManagersWorkspace::WORKSPACES_MANAGERS . ' group.');
+		$workspaceUserGroup = $this->groupManager->get(ManagersWorkspace::WORKSPACES_MANAGERS);
+		$workspaceUserGroup->removeUser($user);
+
 		return;
 	}
 }