[AC-1784] Lining up new Manage collection permissions for users with deprecated EditAssignedCollections permission (#3406)

* [AC-1784] Setting up collections with permission 'Manage = true' if flexible collections feature flag is off and user has EditAssignedCollections

* [AC-1784] Added unit tests

* [AC-1784] Deleted duplicated variable

src/Core/Services/Implementations/CollectionService.cs

@@ -53,21 +53,36 @@ public class CollectionService : ICollectionService
         }
 
         var groupsList = groups?.ToList();
-        var usersList = users?.ToList();
+        var usersList = users?.ToList() ?? new List<CollectionAccessSelection>();
 
         // If using Flexible Collections - a collection should always have someone with Can Manage permissions
         if (_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext))
         {
             var groupHasManageAccess = groupsList?.Any(g => g.Manage) ?? false;
-            var userHasManageAccess = usersList?.Any(u => u.Manage) ?? false;
+            var userHasManageAccess = usersList.Any(u => u.Manage);
             if (!groupHasManageAccess && !userHasManageAccess)
             {
                 throw new BadRequestException(
                     "At least one member or group must have can manage permission.");
             }
         }
+        else
+        {
+            // If not using Flexible Collections
+            // all Organization users with EditAssignedCollections permission should have Manage permission for the collection
+            var organizationUsers = await _organizationUserRepository
+                .GetManyByOrganizationAsync(collection.OrganizationId, null);
+            foreach (var orgUser in organizationUsers.Where(ou => ou.GetPermissions()?.EditAssignedCollections ?? false))
+            {
+                var user = usersList.FirstOrDefault(u => u.Id == orgUser.Id);
+                if (user != null)
+                {
+                    user.Manage = true;
+                }
+            }
+        }
 
-        if (collection.Id == default(Guid))
+        if (collection.Id == default)
         {
             if (org.MaxCollections.HasValue)
             {

src/Core/Services/Implementations/OrganizationService.cs

@@ -64,6 +64,9 @@ public class OrganizationService : IOrganizationService
     private readonly IDataProtectorTokenFactory<OrgUserInviteTokenable> _orgUserInviteTokenDataFactory;
     private readonly IFeatureService _featureService;
 
+    private bool FlexibleCollectionsIsEnabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext);
+    private bool FlexibleCollectionsV1IsEnabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1, _currentContext);
+
     public OrganizationService(
         IOrganizationRepository organizationRepository,
         IOrganizationUserRepository organizationUserRepository,
@@ -437,11 +440,6 @@ public class OrganizationService : IOrganizationService
             await ValidateSignUpPoliciesAsync(signup.Owner.Id);
         }
 
-        var flexibleCollectionsIsEnabled =
-            _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext);
-        var flexibleCollectionsV1IsEnabled =
-            _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1, _currentContext);
-
         var organization = new Organization
         {
             // Pre-generate the org id so that we can save it with the Stripe subscription..
@@ -479,8 +477,8 @@ public class OrganizationService : IOrganizationService
             Status = OrganizationStatusType.Created,
             UsePasswordManager = true,
             UseSecretsManager = signup.UseSecretsManager,
-            LimitCollectionCreationDeletion = !flexibleCollectionsIsEnabled,
+            LimitCollectionCreationDeletion = !FlexibleCollectionsIsEnabled,
-            AllowAdminAccessToAllCollectionItems = !flexibleCollectionsV1IsEnabled
+            AllowAdminAccessToAllCollectionItems = !FlexibleCollectionsV1IsEnabled
         };
 
         if (signup.UseSecretsManager)
@@ -937,6 +935,10 @@ public class OrganizationService : IOrganizationService
                         orgUser.Permissions = JsonSerializer.Serialize(invite.Permissions, JsonHelpers.CamelCase);
                     }
 
+                    // If Flexible Collections is disabled and the user has EditAssignedCollections permission
+                    // grant Manage permission for all assigned collections
+                    invite.Collections = ApplyManageCollectionPermissions(orgUser, invite.Collections);
+
                     if (!orgUser.AccessAll && invite.Collections.Any())
                     {
                         limitedCollectionOrgUsers.Add((orgUser, invite.Collections));
@@ -1323,11 +1325,9 @@ public class OrganizationService : IOrganizationService
             }
         }
 
-        if (user.AccessAll)
+        // If Flexible Collections is disabled and the user has EditAssignedCollections permission
-        {
+        // grant Manage permission for all assigned collections
-            // We don't need any collections if we're flagged to have all access.
+        collections = ApplyManageCollectionPermissions(user, collections);
-            collections = new List<CollectionAccessSelection>();
-        }
         await _organizationUserRepository.ReplaceAsync(user, collections);
 
         if (groups != null)
@@ -2440,4 +2440,18 @@ public class OrganizationService : IOrganizationService
             await _collectionRepository.CreateAsync(defaultCollection);
         }
     }
+
+    private IEnumerable<CollectionAccessSelection> ApplyManageCollectionPermissions(OrganizationUser orgUser, IEnumerable<CollectionAccessSelection> collections)
+    {
+        if (!FlexibleCollectionsIsEnabled && (orgUser.GetPermissions()?.EditAssignedCollections ?? false))
+        {
+            return collections.Select(c =>
+            {
+                c.Manage = true;
+                return c;
+            }).ToList();
+        }
+
+        return collections;
+    }
 }

test/Core.Test/Services/CollectionServiceTests.cs

@@ -8,6 +8,7 @@ using Bit.Core.Repositories;
 using Bit.Core.Services;
 using Bit.Core.Test.AutoFixture;
 using Bit.Core.Test.AutoFixture.OrganizationFixtures;
+using Bit.Core.Utilities;
 using Bit.Test.Common.AutoFixture;
 using Bit.Test.Common.AutoFixture.Attributes;
 using NSubstitute;
@@ -37,6 +38,43 @@ public class CollectionServiceTest
         Assert.True(collection.RevisionDate - utcNow < TimeSpan.FromSeconds(1));
     }
 
+    [Theory, BitAutoData]
+    public async Task SaveAsync_DefaultIdWithUsers_WithOneEditAssignedCollectionsUser_WhileFCFlagDisabled_CreatesCollectionInTheRepository(
+        Collection collection, Organization organization,
+        [CollectionAccessSelectionCustomize] IEnumerable<CollectionAccessSelection> users,
+        SutProvider<CollectionService> sutProvider)
+    {
+        sutProvider.GetDependency<IFeatureService>()
+            .IsEnabled(FeatureFlagKeys.FlexibleCollections, Arg.Any<ICurrentContext>(), Arg.Any<bool>())
+            .Returns(false);
+
+        collection.Id = default;
+        collection.OrganizationId = organization.Id;
+        sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(collection.OrganizationId).Returns(organization);
+        sutProvider.GetDependency<IOrganizationUserRepository>()
+            .GetManyByOrganizationAsync(collection.OrganizationId, Arg.Any<OrganizationUserType?>())
+            .Returns(new List<OrganizationUser>
+            {
+                users.Select(x => new OrganizationUser
+                {
+                    Id = x.Id,
+                    Type = OrganizationUserType.Custom,
+                    Permissions = CoreHelpers.ClassToJsonData(new Permissions { EditAssignedCollections = true })
+                }).First()
+            });
+        var utcNow = DateTime.UtcNow;
+
+        await sutProvider.Sut.SaveAsync(collection, null, users);
+
+        await sutProvider.GetDependency<ICollectionRepository>().Received()
+            .CreateAsync(collection, Arg.Is<List<CollectionAccessSelection>>(l => l == null),
+                Arg.Is<List<CollectionAccessSelection>>(l => l.Count(i => i.Manage == true) == 1));
+        await sutProvider.GetDependency<IEventService>().Received()
+            .LogCollectionEventAsync(collection, EventType.Collection_Created);
+        Assert.True(collection.CreationDate - utcNow < TimeSpan.FromSeconds(1));
+        Assert.True(collection.RevisionDate - utcNow < TimeSpan.FromSeconds(1));
+    }
+
     [Theory, BitAutoData]
     public async Task SaveAsync_DefaultIdWithGroupsAndUsers_CreateCollectionWithGroupsAndUsersInRepository(Collection collection,
         [CollectionAccessSelectionCustomize(true)] IEnumerable<CollectionAccessSelection> groups, IEnumerable<CollectionAccessSelection> users, Organization organization, SutProvider<CollectionService> sutProvider)

test/Core.Test/Services/OrganizationServiceTests.cs

@@ -23,6 +23,7 @@ using Bit.Core.Repositories;
 using Bit.Core.Services;
 using Bit.Core.Settings;
 using Bit.Core.Test.AdminConsole.AutoFixture;
+using Bit.Core.Test.AutoFixture;
 using Bit.Core.Test.AutoFixture.OrganizationFixtures;
 using Bit.Core.Test.AutoFixture.OrganizationUserFixtures;
 using Bit.Core.Tokens;
@@ -826,6 +827,53 @@ public class OrganizationServiceTests
         });
     }
 
+    [Theory]
+    [OrganizationInviteCustomize(
+        InviteeUserType = OrganizationUserType.Custom,
+        InvitorUserType = OrganizationUserType.Owner
+    ), BitAutoData]
+    public async Task InviteUser_WithEditAssignedCollectionsTrue_WhileFCFlagDisabled_SetsCollectionsManageTrue(Organization organization, (OrganizationUserInvite invite, string externalId) invite,
+        OrganizationUser invitor,
+        [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner,
+        SutProvider<OrganizationService> sutProvider)
+    {
+        invite.invite.Permissions = new Permissions { EditAssignedCollections = true };
+
+        sutProvider.GetDependency<IFeatureService>()
+            .IsEnabled(FeatureFlagKeys.FlexibleCollections, Arg.Any<ICurrentContext>(), Arg.Any<bool>())
+            .Returns(false);
+
+        var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();
+        var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
+        var currentContext = sutProvider.GetDependency<ICurrentContext>();
+
+        organizationRepository.GetByIdAsync(organization.Id).Returns(organization);
+        organizationUserRepository.GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner)
+            .Returns(new[] { owner });
+        currentContext.ManageUsers(organization.Id).Returns(true);
+        currentContext.EditAssignedCollections(organization.Id).Returns(true);
+        currentContext.GetOrganization(organization.Id)
+            .Returns(new CurrentContextOrganization
+            {
+                Permissions = new Permissions
+                {
+                    CreateNewCollections = true,
+                    DeleteAnyCollection = true
+                }
+            });
+
+        await sutProvider.Sut.InviteUsersAsync(organization.Id, invitor.UserId, new[] { invite });
+
+        await sutProvider.GetDependency<IOrganizationUserRepository>()
+            .Received(invite.invite.Emails.Count())
+            .CreateAsync(Arg.Is<OrganizationUser>(ou =>
+                    ou.OrganizationId == organization.Id &&
+                    ou.Type == invite.invite.Type &&
+                    invite.invite.Emails.Contains(ou.Email)),
+                Arg.Is<IEnumerable<CollectionAccessSelection>>(collections =>
+                    collections.All(c => c.Manage)));
+    }
+
     private void InviteUserHelper_ArrangeValidPermissions(Organization organization, OrganizationUser savingUser,
     SutProvider<OrganizationService> sutProvider)
     {
@@ -887,6 +935,42 @@ public class OrganizationServiceTests
         await sutProvider.Sut.SaveUserAsync(newUserData, savingUser.UserId, collections, groups);
     }
 
+    [Theory, BitAutoData]
+    public async Task SaveUser_WithEditAssignedCollections_WhileFCFlagDisabled_SetsCollectionsManageTrue(
+        Organization organization,
+        OrganizationUser oldUserData,
+        OrganizationUser newUserData,
+        [CollectionAccessSelectionCustomize] IEnumerable<CollectionAccessSelection> collections,
+        IEnumerable<Guid> groups,
+        [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser savingUser,
+        SutProvider<OrganizationService> sutProvider)
+    {
+        sutProvider.GetDependency<IFeatureService>()
+            .IsEnabled(FeatureFlagKeys.FlexibleCollections, Arg.Any<ICurrentContext>(), Arg.Any<bool>())
+            .Returns(false);
+
+        var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();
+        var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
+        var currentContext = sutProvider.GetDependency<ICurrentContext>();
+
+        organizationRepository.GetByIdAsync(organization.Id).Returns(organization);
+
+        newUserData.Id = oldUserData.Id;
+        newUserData.UserId = oldUserData.UserId;
+        newUserData.OrganizationId = savingUser.OrganizationId = oldUserData.OrganizationId = organization.Id;
+        newUserData.Permissions = CoreHelpers.ClassToJsonData(new Permissions { EditAssignedCollections = true });
+        organizationUserRepository.GetByIdAsync(oldUserData.Id).Returns(oldUserData);
+        organizationUserRepository.GetManyByOrganizationAsync(savingUser.OrganizationId, OrganizationUserType.Owner)
+            .Returns(new List<OrganizationUser> { savingUser });
+        currentContext.OrganizationOwner(savingUser.OrganizationId).Returns(true);
+
+        await sutProvider.Sut.SaveUserAsync(newUserData, savingUser.UserId, collections, groups);
+
+        await sutProvider.GetDependency<IOrganizationUserRepository>().Received(1).ReplaceAsync(
+            Arg.Is<OrganizationUser>(ou => ou.Id == newUserData.Id),
+            Arg.Is<IEnumerable<CollectionAccessSelection>>(i => i.All(c => c.Manage)));
+    }
+
     [Theory, BitAutoData]
     public async Task SaveUser_WithCustomType_WhenUseCustomPermissionsIsFalse_Throws(
         Organization organization,