From f49fb3a8915ad8d978c5b835aa8e821d509d0897 Mon Sep 17 00:00:00 2001 From: Vincent Salucci <26154748+vincentsalucci@users.noreply.github.com> Date: Tue, 6 Aug 2024 11:14:16 -0500 Subject: [PATCH] [PM-10292] Remove Flexible Collections v1 from Core (#4579) * chore: remove fc v1 from OrganizationService, refs PM-10292 * chore: remove fc v1 from CollectionService, refs PM-10292 * chore: remove fc v1 from OrganizationCiphersQuery, refs PM-10292 * fix: update CollectionServiceTests, refs PM-10292 --- .../Implementations/OrganizationService.cs | 16 +---------- .../Implementations/CollectionService.cs | 20 +++++-------- .../Vault/Queries/OrganizationCiphersQuery.cs | 28 ++----------------- .../Services/CollectionServiceTests.cs | 3 -- 4 files changed, 10 insertions(+), 57 deletions(-) diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index d6b2823ff..81fa09211 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -437,9 +437,6 @@ public class OrganizationService : IOrganizationService ValidatePlan(plan, signup.AdditionalSeats, "Password Manager"); - var flexibleCollectionsV1Enabled = - _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1); - var organization = new Organization { // Pre-generate the org id so that we can save it with the Stripe subscription. @@ -476,10 +473,6 @@ public class OrganizationService : IOrganizationService UsePasswordManager = true, // Secrets Manager not available for purchase with Consolidated Billing. UseSecretsManager = false, - - // This is a transitional setting that defaults to ON until Flexible Collections v1 is released - // (to preserve existing behavior) and defaults to OFF after release (enabling new behavior) - AllowAdminAccessToAllCollectionItems = !flexibleCollectionsV1Enabled }; var returnValue = await SignUpAsync(organization, default, signup.OwnerKey, signup.CollectionName, false); @@ -522,9 +515,6 @@ public class OrganizationService : IOrganizationService await ValidateSignUpPoliciesAsync(signup.Owner.Id); } - var flexibleCollectionsV1IsEnabled = - _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1); - var organization = new Organization { // Pre-generate the org id so that we can save it with the Stripe subscription.. @@ -561,11 +551,7 @@ public class OrganizationService : IOrganizationService RevisionDate = DateTime.UtcNow, Status = OrganizationStatusType.Created, UsePasswordManager = true, - UseSecretsManager = signup.UseSecretsManager, - - // This is a transitional setting that defaults to ON until Flexible Collections v1 is released - // (to preserve existing behavior) and defaults to OFF after release (enabling new behavior) - AllowAdminAccessToAllCollectionItems = !flexibleCollectionsV1IsEnabled + UseSecretsManager = signup.UseSecretsManager }; if (signup.UseSecretsManager) diff --git a/src/Core/Services/Implementations/CollectionService.cs b/src/Core/Services/Implementations/CollectionService.cs index d0eed1d6d..e779ac289 100644 --- a/src/Core/Services/Implementations/CollectionService.cs +++ b/src/Core/Services/Implementations/CollectionService.cs @@ -17,7 +17,6 @@ public class CollectionService : ICollectionService private readonly ICollectionRepository _collectionRepository; private readonly IReferenceEventService _referenceEventService; private readonly ICurrentContext _currentContext; - private readonly IFeatureService _featureService; public CollectionService( IEventService eventService, @@ -25,8 +24,7 @@ public class CollectionService : ICollectionService IOrganizationUserRepository organizationUserRepository, ICollectionRepository collectionRepository, IReferenceEventService referenceEventService, - ICurrentContext currentContext, - IFeatureService featureService) + ICurrentContext currentContext) { _eventService = eventService; _organizationRepository = organizationRepository; @@ -34,7 +32,6 @@ public class CollectionService : ICollectionService _collectionRepository = collectionRepository; _referenceEventService = referenceEventService; _currentContext = currentContext; - _featureService = featureService; } public async Task SaveAsync(Collection collection, IEnumerable groups = null, @@ -56,16 +53,13 @@ public class CollectionService : ICollectionService throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true."); } - // If using Flexible Collections V1 - a collection should always have someone with Can Manage permissions - if (_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1)) + // A collection should always have someone with Can Manage permissions + var groupHasManageAccess = groupsList?.Any(g => g.Manage) ?? false; + var userHasManageAccess = usersList?.Any(u => u.Manage) ?? false; + if (!groupHasManageAccess && !userHasManageAccess && !org.AllowAdminAccessToAllCollectionItems) { - var groupHasManageAccess = groupsList?.Any(g => g.Manage) ?? false; - var userHasManageAccess = usersList?.Any(u => u.Manage) ?? false; - if (!groupHasManageAccess && !userHasManageAccess && !org.AllowAdminAccessToAllCollectionItems) - { - throw new BadRequestException( - "At least one member or group must have can manage permission."); - } + throw new BadRequestException( + "At least one member or group must have can manage permission."); } if (collection.Id == default(Guid)) diff --git a/src/Core/Vault/Queries/OrganizationCiphersQuery.cs b/src/Core/Vault/Queries/OrganizationCiphersQuery.cs index 0f01dc3be..f91e3cbbb 100644 --- a/src/Core/Vault/Queries/OrganizationCiphersQuery.cs +++ b/src/Core/Vault/Queries/OrganizationCiphersQuery.cs @@ -1,6 +1,4 @@ -using Bit.Core.Exceptions; -using Bit.Core.Repositories; -using Bit.Core.Services; +using Bit.Core.Repositories; using Bit.Core.Vault.Models.Data; using Bit.Core.Vault.Repositories; @@ -10,15 +8,11 @@ public class OrganizationCiphersQuery : IOrganizationCiphersQuery { private readonly ICipherRepository _cipherRepository; private readonly ICollectionCipherRepository _collectionCipherRepository; - private readonly IFeatureService _featureService; - private bool FlexibleCollectionsV1Enabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1); - - public OrganizationCiphersQuery(ICipherRepository cipherRepository, ICollectionCipherRepository collectionCipherRepository, IFeatureService featureService) + public OrganizationCiphersQuery(ICipherRepository cipherRepository, ICollectionCipherRepository collectionCipherRepository) { _cipherRepository = cipherRepository; _collectionCipherRepository = collectionCipherRepository; - _featureService = featureService; } /// @@ -26,12 +20,6 @@ public class OrganizationCiphersQuery : IOrganizationCiphersQuery /// public async Task> GetOrganizationCiphersForUser(Guid organizationId, Guid userId) { - if (!FlexibleCollectionsV1Enabled) - { - // Flexible collections is OFF, should not be using this query - throw new FeatureUnavailableException("Flexible collections is OFF when it should be ON."); - } - var ciphers = await _cipherRepository.GetManyByUserIdAsync(userId, withOrganizations: true); var orgCiphers = ciphers.Where(c => c.OrganizationId == organizationId).ToList(); var orgCipherIds = orgCiphers.Select(c => c.Id); @@ -50,12 +38,6 @@ public class OrganizationCiphersQuery : IOrganizationCiphersQuery /// public async Task> GetAllOrganizationCiphers(Guid organizationId) { - if (!FlexibleCollectionsV1Enabled) - { - // Flexible collections is OFF, should not be using this query - throw new FeatureUnavailableException("Flexible collections is OFF when it should be ON."); - } - var orgCiphers = await _cipherRepository.GetManyOrganizationDetailsByOrganizationIdAsync(organizationId); var collectionCiphers = await _collectionCipherRepository.GetManyByOrganizationIdAsync(organizationId); var collectionCiphersGroupDict = collectionCiphers.GroupBy(c => c.CipherId).ToDictionary(s => s.Key); @@ -68,12 +50,6 @@ public class OrganizationCiphersQuery : IOrganizationCiphersQuery /// public async Task> GetUnassignedOrganizationCiphers(Guid organizationId) { - if (!FlexibleCollectionsV1Enabled) - { - // Flexible collections is OFF, should not be using this query - throw new FeatureUnavailableException("Flexible collections is OFF when it should be ON."); - } - return await _cipherRepository.GetManyUnassignedOrganizationDetailsByOrganizationIdAsync(organizationId); } } diff --git a/test/Core.Test/Services/CollectionServiceTests.cs b/test/Core.Test/Services/CollectionServiceTests.cs index 7044c2c27..7169962cf 100644 --- a/test/Core.Test/Services/CollectionServiceTests.cs +++ b/test/Core.Test/Services/CollectionServiceTests.cs @@ -113,9 +113,6 @@ public class CollectionServiceTest { collection.Id = default; sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1, Arg.Any()) - .Returns(true); organization.AllowAdminAccessToAllCollectionItems = false; var ex = await Assert.ThrowsAsync(() => sutProvider.Sut.SaveAsync(collection, null, users));