From f0b93912498eb590b1f1b7f5a078607761337e58 Mon Sep 17 00:00:00 2001
From: Thomas Rittson <31796059+eliykat@users.noreply.github.com>
Date: Thu, 2 May 2024 08:32:50 +1000
Subject: [PATCH] Prevent user from adding themselves to collection (#4037)

---
 .../OrganizationUsersController.cs            | 27 +++++++++++------
 .../OrganizationUsersControllerTests.cs       | 30 +++++++++++++++++++
 2 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs
index 4691518da4..78127cbfd2 100644
--- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs
+++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs
@@ -364,26 +364,35 @@ public class OrganizationUsersController : Controller
             throw new NotFoundException();
         }
 
-        var organizationUser = await _organizationUserRepository.GetByIdAsync(id);
+        var (organizationUser, currentAccess) = await _organizationUserRepository.GetByIdWithCollectionsAsync(id);
         if (organizationUser == null || organizationUser.OrganizationId != orgId)
         {
             throw new NotFoundException();
         }
 
-        // If admins are not allowed access to all collections, you cannot add yourself to a group
-        // In this case we just don't update groups
         var userId = _userService.GetProperUserId(User).Value;
-        var organizationAbility = await _applicationCacheService.GetOrganizationAbilityAsync(orgId);
         var editingSelf = userId == organizationUser.UserId;
 
-        var groups = editingSelf && !organizationAbility.AllowAdminAccessToAllCollectionItems
+        // If admins are not allowed access to all collections, you cannot add yourself to a group.
+        // In this case we just don't update groups.
+        var organizationAbility = await _applicationCacheService.GetOrganizationAbilityAsync(orgId);
+        var groupsToSave = editingSelf && !organizationAbility.AllowAdminAccessToAllCollectionItems
             ? null
             : model.Groups;
 
+        // If admins are not allowed access to all collections, you cannot add yourself to collections.
+        // This is not caught by the requirement below that you can ModifyUserAccess and must be checked separately
+        var currentAccessIds = currentAccess.Select(c => c.Id).ToHashSet();
+        if (editingSelf &&
+            !organizationAbility.AllowAdminAccessToAllCollectionItems &&
+            model.Collections.Any(c => !currentAccessIds.Contains(c.Id)))
+        {
+            throw new BadRequestException("You cannot add yourself to a collection.");
+        }
+
         // The client only sends collections that the saving user has permissions to edit.
-        // On the server side, we need to (1) confirm this and (2) concat these with the collections that the user
-        // can't edit before saving to the database.
-        var (_, currentAccess) = await _organizationUserRepository.GetByIdWithCollectionsAsync(id);
+        // On the server side, we need to (1) make sure the user has permissions for these collections, and
+        // (2) concat these with the collections that the user can't edit before saving to the database.
         var currentCollections = await _collectionRepository
             .GetManyByManyIdsAsync(currentAccess.Select(cas => cas.Id));
 
@@ -411,7 +420,7 @@ public class OrganizationUsersController : Controller
             .ToList();
 
         await _updateOrganizationUserCommand.UpdateUserAsync(model.ToOrganizationUser(organizationUser), userId,
-            collectionsToSave, groups);
+            collectionsToSave, groupsToSave);
     }
 
     [HttpPut("{userId}/reset-password-enrollment")]
diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs
index e4831669d6..d6f8f845eb 100644
--- a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs
+++ b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs
@@ -184,6 +184,36 @@ public class OrganizationUsersControllerTests
             model.Groups);
     }
 
+    [Theory]
+    [BitAutoData]
+    public async Task Put_UpdateSelf_WithoutAllowAdminAccessToAllCollectionItems_CannotAddSelfToCollections(OrganizationUserUpdateRequestModel model,
+        OrganizationUser organizationUser, OrganizationAbility organizationAbility,
+        SutProvider<OrganizationUsersController> sutProvider, Guid savingUserId)
+    {
+        // Updating self
+        organizationUser.UserId = savingUserId;
+        organizationAbility.AllowAdminAccessToAllCollectionItems = false;
+        organizationAbility.FlexibleCollections = true;
+        sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
+
+        Put_Setup(sutProvider, organizationAbility, organizationUser, savingUserId, model, false);
+
+        // User is not currently assigned to any collections, which means they're adding themselves
+        sutProvider.GetDependency<IOrganizationUserRepository>()
+            .GetByIdWithCollectionsAsync(organizationUser.Id)
+            .Returns(new Tuple<OrganizationUser, ICollection<CollectionAccessSelection>>(organizationUser,
+                new List<CollectionAccessSelection>()));
+        sutProvider.GetDependency<ICollectionRepository>()
+            .GetManyByManyIdsAsync(Arg.Any<IEnumerable<Guid>>())
+            .Returns(new List<Collection>());
+
+        var orgUserId = organizationUser.Id;
+        var orgUserEmail = organizationUser.Email;
+
+        var exception = await Assert.ThrowsAsync<BadRequestException>(async () => await sutProvider.Sut.Put(organizationAbility.Id, organizationUser.Id, model));
+        Assert.Contains("You cannot add yourself to a collection.", exception.Message);
+    }
+
     [Theory]
     [BitAutoData]
     public async Task Put_UpdateSelf_WithoutAllowAdminAccessToAllCollectionItems_DoesNotUpdateGroups(OrganizationUserUpdateRequestModel model,