[AC-1784] Revert changes made on assigning Manage permission to collections (#3501)

This reverts commit fe702c6535243b4fac88d2021c4abe771f72da94.

src/Core/AdminConsole/Services/Implementations/OrganizationService.cs

@@ -64,9 +64,6 @@ 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,
@@ -440,6 +437,11 @@ 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..
@@ -477,8 +479,8 @@ public class OrganizationService : IOrganizationService
             Status = OrganizationStatusType.Created,
             UsePasswordManager = true,
             UseSecretsManager = signup.UseSecretsManager,
-            LimitCollectionCreationDeletion = !FlexibleCollectionsIsEnabled,
+            LimitCollectionCreationDeletion = !flexibleCollectionsIsEnabled,
-            AllowAdminAccessToAllCollectionItems = !FlexibleCollectionsV1IsEnabled
+            AllowAdminAccessToAllCollectionItems = !flexibleCollectionsV1IsEnabled
         };
 
         if (signup.UseSecretsManager)
@@ -935,10 +937,6 @@ 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));
@@ -1325,9 +1323,11 @@ public class OrganizationService : IOrganizationService
             }
         }
 
-        // If Flexible Collections is disabled and the user has EditAssignedCollections permission
+        if (user.AccessAll)
-        // grant Manage permission for all assigned collections
+        {
-        collections = ApplyManageCollectionPermissions(user, collections);
+            // We don't need any collections if we're flagged to have all access.
+            collections = new List<CollectionAccessSelection>();
+        }
         await _organizationUserRepository.ReplaceAsync(user, collections);
 
         if (groups != null)
@@ -2440,18 +2440,4 @@ 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;
-    }
 }

src/Core/Services/Implementations/CollectionService.cs

@@ -53,36 +53,21 @@ public class CollectionService : ICollectionService
         }
 
         var groupsList = groups?.ToList();
-        var usersList = users?.ToList() ?? new List<CollectionAccessSelection>();
+        var usersList = users?.ToList();
 
         // 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);
+            var userHasManageAccess = usersList?.Any(u => u.Manage) ?? false;
             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)
+        if (collection.Id == default(Guid))
         {
             if (org.MaxCollections.HasValue)
             {

test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs

@@ -23,7 +23,6 @@ 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;
@@ -827,53 +826,6 @@ 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)
     {
@@ -935,42 +887,6 @@ 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,

test/Core.Test/Services/CollectionServiceTests.cs

@@ -8,7 +8,6 @@ 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;
@@ -38,43 +37,6 @@ 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)