From 91409a45f0adbaa8a5aee2d81060cd6369656c44 Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Fri, 18 Oct 2024 11:00:01 -0400 Subject: [PATCH] Split `Organization.LimitCollectionCreationDeletion` into two separate business rules (#4730) * Add feature flag * Promoted the new Entiy Framework properties * Deprecate the old property * Update references * Fix mispelling * Re-add contextual comment regarding dropped license properties * Add back deleted assertion for deprecated property * Add back removed fixture property assignment * Improve feature toggling scenerios for self hosted org creation/update * Unblock `PutCollectionManagement` for self host * Simplify logic of a couple of conditionals * Feature toggle route unblocking * Adjust logic collection creation/deletion authorization handler * Create tests * Fix bug caught by tests * Fix bugs caught during manual testing * Remove remark about license --- .../Organizations/_ViewInformation.cshtml | 19 +- .../Controllers/OrganizationsController.cs | 11 +- .../OrganizationResponseModel.cs | 6 + .../ProfileOrganizationResponseModel.cs | 6 + ...rofileProviderOrganizationResponseModel.cs | 3 + ...nCollectionManagementUpdateRequestModel.cs | 20 +- .../BulkCollectionAuthorizationHandler.cs | 55 +- .../AdminConsole/Entities/Organization.cs | 26 +- .../Data/Organizations/OrganizationAbility.cs | 6 + .../OrganizationUserOrganizationDetails.cs | 3 + .../SelfHostedOrganizationDetails.cs | 3 + .../ProviderUserOrganizationDetails.cs | 2 + .../Implementations/OrganizationService.cs | 10 +- src/Core/Constants.cs | 1 + .../Models/Business/OrganizationLicense.cs | 19 +- .../UpdateOrganizationLicenseCommand.cs | 8 +- .../AdminConsole/Models/Organization.cs | 7 - .../Repositories/OrganizationRepository.cs | 3 + ...izationUserOrganizationDetailsViewQuery.cs | 3 + ...roviderUserOrganizationDetailsViewQuery.cs | 3 + ...BulkCollectionAuthorizationHandlerTests.cs | 553 +++++++++++++++++- .../OrganizationUserRepositoryTests.cs | 3 + 22 files changed, 701 insertions(+), 69 deletions(-) diff --git a/src/Admin/AdminConsole/Views/Organizations/_ViewInformation.cshtml b/src/Admin/AdminConsole/Views/Organizations/_ViewInformation.cshtml index db2e2c601..f3853e16a 100644 --- a/src/Admin/AdminConsole/Views/Organizations/_ViewInformation.cshtml +++ b/src/Admin/AdminConsole/Views/Organizations/_ViewInformation.cshtml @@ -1,4 +1,6 @@ -@model OrganizationViewModel +@inject Bit.Core.Services.IFeatureService FeatureService +@model OrganizationViewModel +
Id
@Model.Organization.Id
@@ -53,8 +55,19 @@
Administrators manage all collections
@(Model.Organization.AllowAdminAccessToAllCollectionItems ? "On" : "Off")
-
Limit collection creation to administrators
-
@(Model.Organization.LimitCollectionCreationDeletion ? "On" : "Off")
+ @if (!FeatureService.IsEnabled(Bit.Core.FeatureFlagKeys.LimitCollectionCreationDeletionSplit)) + { +
Limit collection creation to administrators
+
@(Model.Organization.LimitCollectionCreationDeletion ? "On" : "Off")
+ } + else + { +
Limit collection creation to administrators
+
@(Model.Organization.LimitCollectionCreation ? "On" : "Off")
+ +
Limit collection deletion to administrators
+
@(Model.Organization.LimitCollectionDeletion ? "On" : "Off")
+ }

Secrets Manager

diff --git a/src/Api/AdminConsole/Controllers/OrganizationsController.cs b/src/Api/AdminConsole/Controllers/OrganizationsController.cs index 4c5a4028c..0b3811618 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationsController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationsController.cs @@ -520,9 +520,16 @@ public class OrganizationsController : Controller } [HttpPut("{id}/collection-management")] - [SelfHosted(NotSelfHostedOnly = true)] public async Task PutCollectionManagement(Guid id, [FromBody] OrganizationCollectionManagementUpdateRequestModel model) { + if ( + _globalSettings.SelfHosted && + !_featureService.IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit) + ) + { + throw new BadRequestException("Only allowed when not self hosted."); + } + var organization = await _organizationRepository.GetByIdAsync(id); if (organization == null) { @@ -534,7 +541,7 @@ public class OrganizationsController : Controller throw new NotFoundException(); } - await _organizationService.UpdateAsync(model.ToOrganization(organization), eventType: EventType.Organization_CollectionManagement_Updated); + await _organizationService.UpdateAsync(model.ToOrganization(organization, _featureService), eventType: EventType.Organization_CollectionManagement_Updated); return new OrganizationResponseModel(organization); } } diff --git a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationResponseModel.cs b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationResponseModel.cs index 08b4e4b06..7808b564a 100644 --- a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationResponseModel.cs +++ b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationResponseModel.cs @@ -55,6 +55,9 @@ public class OrganizationResponseModel : ResponseModel SmServiceAccounts = organization.SmServiceAccounts; MaxAutoscaleSmSeats = organization.MaxAutoscaleSmSeats; MaxAutoscaleSmServiceAccounts = organization.MaxAutoscaleSmServiceAccounts; + LimitCollectionCreation = organization.LimitCollectionCreation; + LimitCollectionDeletion = organization.LimitCollectionDeletion; + // Deprecated: https://bitwarden.atlassian.net/browse/PM-10863 LimitCollectionCreationDeletion = organization.LimitCollectionCreationDeletion; AllowAdminAccessToAllCollectionItems = organization.AllowAdminAccessToAllCollectionItems; } @@ -98,6 +101,9 @@ public class OrganizationResponseModel : ResponseModel public int? SmServiceAccounts { get; set; } public int? MaxAutoscaleSmSeats { get; set; } public int? MaxAutoscaleSmServiceAccounts { get; set; } + public bool LimitCollectionCreation { get; set; } + public bool LimitCollectionDeletion { get; set; } + // Deperectated: https://bitwarden.atlassian.net/browse/PM-10863 public bool LimitCollectionCreationDeletion { get; set; } public bool AllowAdminAccessToAllCollectionItems { get; set; } } diff --git a/src/Api/AdminConsole/Models/Response/ProfileOrganizationResponseModel.cs b/src/Api/AdminConsole/Models/Response/ProfileOrganizationResponseModel.cs index a573bfb8d..1fcaba5f9 100644 --- a/src/Api/AdminConsole/Models/Response/ProfileOrganizationResponseModel.cs +++ b/src/Api/AdminConsole/Models/Response/ProfileOrganizationResponseModel.cs @@ -65,6 +65,9 @@ public class ProfileOrganizationResponseModel : ResponseModel FamilySponsorshipToDelete = organization.FamilySponsorshipToDelete; FamilySponsorshipValidUntil = organization.FamilySponsorshipValidUntil; AccessSecretsManager = organization.AccessSecretsManager; + LimitCollectionCreation = organization.LimitCollectionCreation; + LimitCollectionDeletion = organization.LimitCollectionDeletion; + // Deprecated: https://bitwarden.atlassian.net/browse/PM-10863 LimitCollectionCreationDeletion = organization.LimitCollectionCreationDeletion; AllowAdminAccessToAllCollectionItems = organization.AllowAdminAccessToAllCollectionItems; UserIsManagedByOrganization = organizationIdsManagingUser.Contains(organization.OrganizationId); @@ -124,6 +127,9 @@ public class ProfileOrganizationResponseModel : ResponseModel public DateTime? FamilySponsorshipValidUntil { get; set; } public bool? FamilySponsorshipToDelete { get; set; } public bool AccessSecretsManager { get; set; } + public bool LimitCollectionCreation { get; set; } + public bool LimitCollectionDeletion { get; set; } + // Deprecated: https://bitwarden.atlassian.net/browse/PM-10863 public bool LimitCollectionCreationDeletion { get; set; } public bool AllowAdminAccessToAllCollectionItems { get; set; } /// diff --git a/src/Api/AdminConsole/Models/Response/ProfileProviderOrganizationResponseModel.cs b/src/Api/AdminConsole/Models/Response/ProfileProviderOrganizationResponseModel.cs index 46819f886..92498834d 100644 --- a/src/Api/AdminConsole/Models/Response/ProfileProviderOrganizationResponseModel.cs +++ b/src/Api/AdminConsole/Models/Response/ProfileProviderOrganizationResponseModel.cs @@ -44,6 +44,9 @@ public class ProfileProviderOrganizationResponseModel : ProfileOrganizationRespo ProviderId = organization.ProviderId; ProviderName = organization.ProviderName; ProductTierType = StaticStore.GetPlan(organization.PlanType).ProductTier; + LimitCollectionCreation = organization.LimitCollectionCreation; + LimitCollectionDeletion = organization.LimitCollectionDeletion; + // https://bitwarden.atlassian.net/browse/PM-10863 LimitCollectionCreationDeletion = organization.LimitCollectionCreationDeletion; AllowAdminAccessToAllCollectionItems = organization.AllowAdminAccessToAllCollectionItems; } diff --git a/src/Api/Models/Request/Organizations/OrganizationCollectionManagementUpdateRequestModel.cs b/src/Api/Models/Request/Organizations/OrganizationCollectionManagementUpdateRequestModel.cs index 68e87b522..a5a6f1f74 100644 --- a/src/Api/Models/Request/Organizations/OrganizationCollectionManagementUpdateRequestModel.cs +++ b/src/Api/Models/Request/Organizations/OrganizationCollectionManagementUpdateRequestModel.cs @@ -1,15 +1,29 @@ -using Bit.Core.AdminConsole.Entities; +using Bit.Core; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Services; namespace Bit.Api.Models.Request.Organizations; public class OrganizationCollectionManagementUpdateRequestModel { + public bool LimitCollectionCreation { get; set; } + public bool LimitCollectionDeletion { get; set; } + // Deprecated: https://bitwarden.atlassian.net/browse/PM-10863 public bool LimitCreateDeleteOwnerAdmin { get; set; } public bool AllowAdminAccessToAllCollectionItems { get; set; } - public virtual Organization ToOrganization(Organization existingOrganization) + public virtual Organization ToOrganization(Organization existingOrganization, IFeatureService featureService) { - existingOrganization.LimitCollectionCreationDeletion = LimitCreateDeleteOwnerAdmin; + if (featureService.IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit)) + { + existingOrganization.LimitCollectionCreation = LimitCollectionCreation; + existingOrganization.LimitCollectionDeletion = LimitCollectionDeletion; + } + else + { + existingOrganization.LimitCollectionCreationDeletion = LimitCreateDeleteOwnerAdmin || LimitCollectionCreation || LimitCollectionDeletion; + } + existingOrganization.AllowAdminAccessToAllCollectionItems = AllowAdminAccessToAllCollectionItems; return existingOrganization; } diff --git a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs index 5d11b39ea..c26d5b595 100644 --- a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs +++ b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs @@ -1,5 +1,6 @@ #nullable enable using System.Diagnostics; +using Bit.Core; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -101,7 +102,7 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler, IStorableSubscriber, IRevisable, /// If set to false, any organization member can create a collection, and any member can delete a collection that /// they have Can Manage permissions for. /// - public bool LimitCollectionCreationDeletion { get; set; } + public bool LimitCollectionCreation { get; set; } + public bool LimitCollectionDeletion { get; set; } + // Deprecated by https://bitwarden.atlassian.net/browse/PM-10863. This + // was replaced with `LimitCollectionCreation` and + // `LimitCollectionDeletion`. + public bool LimitCollectionCreationDeletion + { + get => LimitCollectionCreation || LimitCollectionDeletion; + set + { + LimitCollectionCreation = value; + LimitCollectionDeletion = value; + } + } /// /// If set to true, admins, owners, and some custom users can read/write all collections and items in the Admin Console. @@ -265,7 +279,7 @@ public class Organization : ITableObject, IStorableSubscriber, IRevisable, return providers[provider]; } - public void UpdateFromLicense(OrganizationLicense license) + public void UpdateFromLicense(OrganizationLicense license, IFeatureService featureService) { // The following properties are intentionally excluded from being updated: // - Id - self-hosted org will have its own unique Guid @@ -300,7 +314,11 @@ public class Organization : ITableObject, IStorableSubscriber, IRevisable, UseSecretsManager = license.UseSecretsManager; SmSeats = license.SmSeats; SmServiceAccounts = license.SmServiceAccounts; - LimitCollectionCreationDeletion = license.LimitCollectionCreationDeletion; - AllowAdminAccessToAllCollectionItems = license.AllowAdminAccessToAllCollectionItems; + + if (!featureService.IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit)) + { + LimitCollectionCreationDeletion = license.LimitCollectionCreationDeletion; + AllowAdminAccessToAllCollectionItems = license.AllowAdminAccessToAllCollectionItems; + } } } diff --git a/src/Core/AdminConsole/Models/Data/Organizations/OrganizationAbility.cs b/src/Core/AdminConsole/Models/Data/Organizations/OrganizationAbility.cs index 07db80d43..a91b96083 100644 --- a/src/Core/AdminConsole/Models/Data/Organizations/OrganizationAbility.cs +++ b/src/Core/AdminConsole/Models/Data/Organizations/OrganizationAbility.cs @@ -21,6 +21,9 @@ public class OrganizationAbility UseResetPassword = organization.UseResetPassword; UseCustomPermissions = organization.UseCustomPermissions; UsePolicies = organization.UsePolicies; + LimitCollectionCreation = organization.LimitCollectionCreation; + LimitCollectionDeletion = organization.LimitCollectionDeletion; + // Deprecated: https://bitwarden.atlassian.net/browse/PM-10863 LimitCollectionCreationDeletion = organization.LimitCollectionCreationDeletion; AllowAdminAccessToAllCollectionItems = organization.AllowAdminAccessToAllCollectionItems; } @@ -37,6 +40,9 @@ public class OrganizationAbility public bool UseResetPassword { get; set; } public bool UseCustomPermissions { get; set; } public bool UsePolicies { get; set; } + public bool LimitCollectionCreation { get; set; } + public bool LimitCollectionDeletion { get; set; } + // Deprecated: https://bitwarden.atlassian.net/browse/PM-10863 public bool LimitCollectionCreationDeletion { get; set; } public bool AllowAdminAccessToAllCollectionItems { get; set; } } diff --git a/src/Core/AdminConsole/Models/Data/Organizations/OrganizationUsers/OrganizationUserOrganizationDetails.cs b/src/Core/AdminConsole/Models/Data/Organizations/OrganizationUsers/OrganizationUserOrganizationDetails.cs index cdd73cba7..435369e77 100644 --- a/src/Core/AdminConsole/Models/Data/Organizations/OrganizationUsers/OrganizationUserOrganizationDetails.cs +++ b/src/Core/AdminConsole/Models/Data/Organizations/OrganizationUsers/OrganizationUserOrganizationDetails.cs @@ -54,6 +54,9 @@ public class OrganizationUserOrganizationDetails public bool UsePasswordManager { get; set; } public int? SmSeats { get; set; } public int? SmServiceAccounts { get; set; } + public bool LimitCollectionCreation { get; set; } + public bool LimitCollectionDeletion { get; set; } + // Deprecated: https://bitwarden.atlassian.net/browse/PM-10863 public bool LimitCollectionCreationDeletion { get; set; } public bool AllowAdminAccessToAllCollectionItems { get; set; } } diff --git a/src/Core/AdminConsole/Models/Data/Organizations/SelfHostedOrganizationDetails.cs b/src/Core/AdminConsole/Models/Data/Organizations/SelfHostedOrganizationDetails.cs index d21ba9183..1fa547d98 100644 --- a/src/Core/AdminConsole/Models/Data/Organizations/SelfHostedOrganizationDetails.cs +++ b/src/Core/AdminConsole/Models/Data/Organizations/SelfHostedOrganizationDetails.cs @@ -144,6 +144,9 @@ public class SelfHostedOrganizationDetails : Organization RevisionDate = RevisionDate, MaxAutoscaleSeats = MaxAutoscaleSeats, OwnersNotifiedOfAutoscaling = OwnersNotifiedOfAutoscaling, + LimitCollectionCreation = LimitCollectionCreation, + LimitCollectionDeletion = LimitCollectionDeletion, + // Deprecated: https://bitwarden.atlassian.net/browse/PM-10863 LimitCollectionCreationDeletion = LimitCollectionCreationDeletion, AllowAdminAccessToAllCollectionItems = AllowAdminAccessToAllCollectionItems, Status = Status diff --git a/src/Core/AdminConsole/Models/Data/Provider/ProviderUserOrganizationDetails.cs b/src/Core/AdminConsole/Models/Data/Provider/ProviderUserOrganizationDetails.cs index d3d831f51..a2ac62253 100644 --- a/src/Core/AdminConsole/Models/Data/Provider/ProviderUserOrganizationDetails.cs +++ b/src/Core/AdminConsole/Models/Data/Provider/ProviderUserOrganizationDetails.cs @@ -40,6 +40,8 @@ public class ProviderUserOrganizationDetails [JsonConverter(typeof(HtmlEncodingStringConverter))] public string ProviderName { get; set; } public PlanType PlanType { get; set; } + public bool LimitCollectionCreation { get; set; } + public bool LimitCollectionDeletion { get; set; } public bool LimitCollectionCreationDeletion { get; set; } public bool AllowAdminAccessToAllCollectionItems { get; set; } } diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index f453a22b4..50a2ed84e 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -708,10 +708,16 @@ public class OrganizationService : IOrganizationService UseSecretsManager = license.UseSecretsManager, SmSeats = license.SmSeats, SmServiceAccounts = license.SmServiceAccounts, - LimitCollectionCreationDeletion = license.LimitCollectionCreationDeletion, - AllowAdminAccessToAllCollectionItems = license.AllowAdminAccessToAllCollectionItems, }; + // These fields are being removed from consideration when processing + // licenses. + if (!_featureService.IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit)) + { + organization.LimitCollectionCreationDeletion = license.LimitCollectionCreationDeletion; + organization.AllowAdminAccessToAllCollectionItems = license.AllowAdminAccessToAllCollectionItems; + } + var result = await SignUpAsync(organization, owner.Id, ownerKey, collectionName, false); var dir = $"{_globalSettings.LicenseDirectory}/organization"; diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 1fa73fcb3..6b4cf7e97 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -146,6 +146,7 @@ public static class FeatureFlagKeys public const string RemoveServerVersionHeader = "remove-server-version-header"; public const string AccessIntelligence = "pm-13227-access-intelligence"; public const string VerifiedSsoDomainEndpoint = "pm-12337-refactor-sso-details-endpoint"; + public const string LimitCollectionCreationDeletionSplit = "pm-10863-limit-collection-creation-deletion-split"; public static List GetAllKeys() { diff --git a/src/Core/Models/Business/OrganizationLicense.cs b/src/Core/Models/Business/OrganizationLicense.cs index ebc1a083f..ea5127364 100644 --- a/src/Core/Models/Business/OrganizationLicense.cs +++ b/src/Core/Models/Business/OrganizationLicense.cs @@ -53,8 +53,11 @@ public class OrganizationLicense : ILicense UseSecretsManager = org.UseSecretsManager; SmSeats = org.SmSeats; SmServiceAccounts = org.SmServiceAccounts; + + // Deprecated. Left for backwards compatibility with old license versions. LimitCollectionCreationDeletion = org.LimitCollectionCreationDeletion; AllowAdminAccessToAllCollectionItems = org.AllowAdminAccessToAllCollectionItems; + // if (subscriptionInfo?.Subscription == null) { @@ -138,8 +141,12 @@ public class OrganizationLicense : ILicense public bool UseSecretsManager { get; set; } public int? SmSeats { get; set; } public int? SmServiceAccounts { get; set; } + + // Deprecated. Left for backwards compatibility with old license versions. public bool LimitCollectionCreationDeletion { get; set; } = true; public bool AllowAdminAccessToAllCollectionItems { get; set; } = true; + // + public bool Trial { get; set; } public LicenseType? LicenseType { get; set; } public string Hash { get; set; } @@ -150,7 +157,8 @@ public class OrganizationLicense : ILicense /// Represents the current version of the license format. Should be updated whenever new fields are added. /// /// Intentionally set one version behind to allow self hosted users some time to update before - /// getting out of date license errors + /// getting out of date license errors + /// public const int CurrentLicenseFileVersion = 14; private bool ValidLicenseVersion { @@ -368,10 +376,11 @@ public class OrganizationLicense : ILicense } /* - * Version 14 added LimitCollectionCreationDeletion and Version 15 added AllowAdminAccessToAllCollectionItems, - * however these are just user settings and it is not worth failing validation if they mismatch. - * They are intentionally excluded. - */ + * Version 14 added LimitCollectionCreationDeletion and Version + * 15 added AllowAdminAccessToAllCollectionItems, however they + * are no longer used and are intentionally excluded from + * validation. + */ return valid; } diff --git a/src/Core/OrganizationFeatures/OrganizationLicenses/UpdateOrganizationLicenseCommand.cs b/src/Core/OrganizationFeatures/OrganizationLicenses/UpdateOrganizationLicenseCommand.cs index 62c46460a..1f8c6604b 100644 --- a/src/Core/OrganizationFeatures/OrganizationLicenses/UpdateOrganizationLicenseCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationLicenses/UpdateOrganizationLicenseCommand.cs @@ -17,15 +17,18 @@ public class UpdateOrganizationLicenseCommand : IUpdateOrganizationLicenseComman private readonly ILicensingService _licensingService; private readonly IGlobalSettings _globalSettings; private readonly IOrganizationService _organizationService; + private readonly IFeatureService _featureService; public UpdateOrganizationLicenseCommand( ILicensingService licensingService, IGlobalSettings globalSettings, - IOrganizationService organizationService) + IOrganizationService organizationService, + IFeatureService featureService) { _licensingService = licensingService; _globalSettings = globalSettings; _organizationService = organizationService; + _featureService = featureService; } public async Task UpdateLicenseAsync(SelfHostedOrganizationDetails selfHostedOrganization, @@ -59,7 +62,8 @@ public class UpdateOrganizationLicenseCommand : IUpdateOrganizationLicenseComman private async Task UpdateOrganizationAsync(SelfHostedOrganizationDetails selfHostedOrganizationDetails, OrganizationLicense license) { var organization = selfHostedOrganizationDetails.ToOrganization(); - organization.UpdateFromLicense(license); + + organization.UpdateFromLicense(license, _featureService); await _organizationService.ReplaceAndUpdateCacheAsync(organization); } diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Models/Organization.cs b/src/Infrastructure.EntityFramework/AdminConsole/Models/Organization.cs index 288b5c6a9..d7f83d829 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Models/Organization.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Models/Organization.cs @@ -9,10 +9,6 @@ namespace Bit.Infrastructure.EntityFramework.AdminConsole.Models; public class Organization : Core.AdminConsole.Entities.Organization { - // Shadow properties - to be introduced by https://bitwarden.atlassian.net/browse/PM-10863 - public bool LimitCollectionCreation { get => LimitCollectionCreationDeletion; set => LimitCollectionCreationDeletion = value; } - public bool LimitCollectionDeletion { get => LimitCollectionCreationDeletion; set => LimitCollectionCreationDeletion = value; } - public virtual ICollection Ciphers { get; set; } public virtual ICollection OrganizationUsers { get; set; } public virtual ICollection Groups { get; set; } @@ -42,9 +38,6 @@ public class OrganizationMapperProfile : Profile .ForMember(org => org.ApiKeys, opt => opt.Ignore()) .ForMember(org => org.Connections, opt => opt.Ignore()) .ForMember(org => org.Domains, opt => opt.Ignore()) - // Shadow properties - to be introduced by https://bitwarden.atlassian.net/browse/PM-10863 - .ForMember(org => org.LimitCollectionCreation, opt => opt.Ignore()) - .ForMember(org => org.LimitCollectionDeletion, opt => opt.Ignore()) .ReverseMap(); CreateProjection() diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs index bb9090e0a..b3ee25488 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs @@ -99,6 +99,9 @@ public class OrganizationRepository : Repository().DidNotReceiveWithAnyArgs().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.True(context.HasSucceeded); } [Theory, BitAutoData, CollectionCustomization] - public async Task CanCreateAsync_WhenUser_WithLimitCollectionCreationDeletionFalse_Success( + public async Task CanCreateAsync_WhenUser_WithLimitCollectionCreationFalse_WithLimitCollectionCreationDeletionSplitFeatureDisabled_Success( SutProvider sutProvider, ICollection collections, CurrentContextOrganization organization) @@ -57,7 +62,7 @@ public class BulkCollectionAuthorizationHandlerTests organization.Type = OrganizationUserType.User; - ArrangeOrganizationAbility(sutProvider, organization, false); + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureDisabled(sutProvider, organization, false, false); var context = new AuthorizationHandlerContext( new[] { BulkCollectionOperations.Create }, @@ -66,16 +71,49 @@ public class BulkCollectionAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit) + .Returns(false); await sutProvider.Sut.HandleAsync(context); + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); + Assert.True(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task CanCreateAsync_WhenUser_WithLimitCollectionCreationFalse_WithLimitCollectionCreationDeletionSplitFeatureEnabled_Success( + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = OrganizationUserType.User; + + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureEnabled(sutProvider, organization, false, false); + + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Create }, + new ClaimsPrincipal(), + collections); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit) + .Returns(true); + + await sutProvider.Sut.HandleAsync(context); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.True(context.HasSucceeded); } [Theory, CollectionCustomization] [BitAutoData(OrganizationUserType.User)] [BitAutoData(OrganizationUserType.Custom)] - public async Task CanCreateAsync_WhenMissingPermissions_NoSuccess( + public async Task CanCreateAsync_WhenMissingPermissions_WithLimitCollectionCreationDeletionSplitFeatureDisabled_NoSuccess( OrganizationUserType userType, SutProvider sutProvider, ICollection collections, @@ -92,7 +130,7 @@ public class BulkCollectionAuthorizationHandlerTests ManageUsers = false }; - ArrangeOrganizationAbility(sutProvider, organization, true); + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureDisabled(sutProvider, organization, true, true); var context = new AuthorizationHandlerContext( new[] { BulkCollectionOperations.Create }, @@ -102,21 +140,61 @@ public class BulkCollectionAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(false); await sutProvider.Sut.HandleAsync(context); + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); + Assert.False(context.HasSucceeded); + } + + [Theory, CollectionCustomization] + [BitAutoData(OrganizationUserType.User)] + [BitAutoData(OrganizationUserType.Custom)] + public async Task CanCreateAsync_WhenMissingPermissions_WithLimitCollectionCreationDeletionSplitFeatureEnabled_NoSuccess( + OrganizationUserType userType, + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = userType; + organization.Permissions = new Permissions + { + EditAnyCollection = false, + DeleteAnyCollection = false, + ManageGroups = false, + ManageUsers = false + }; + + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureEnabled(sutProvider, organization, true, true); + + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Create }, + new ClaimsPrincipal(), + collections); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(true); + + await sutProvider.Sut.HandleAsync(context); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.False(context.HasSucceeded); } [Theory, BitAutoData, CollectionCustomization] - public async Task CanCreateAsync_WhenMissingOrgAccess_NoSuccess( + public async Task CanCreateAsync_WhenMissingOrgAccess_WithLimitCollectionCreationDeletionSplitDisabled_NoSuccess( Guid userId, CurrentContextOrganization organization, List collections, SutProvider sutProvider) { collections.ForEach(c => c.OrganizationId = organization.Id); - ArrangeOrganizationAbility(sutProvider, organization, true); + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureDisabled(sutProvider, organization, true, true); var context = new AuthorizationHandlerContext( new[] { BulkCollectionOperations.Create }, @@ -127,8 +205,38 @@ public class BulkCollectionAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(false); await sutProvider.Sut.HandleAsync(context); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); + Assert.False(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task CanCreateAsync_WhenMissingOrgAccess_WithLimitCollectionCreationDeletionSplitEnabled_NoSuccess( + Guid userId, + CurrentContextOrganization organization, + List collections, + SutProvider sutProvider) + { + collections.ForEach(c => c.OrganizationId = organization.Id); + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureEnabled(sutProvider, organization, true, true); + + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Create }, + new ClaimsPrincipal(), + collections + ); + + sutProvider.GetDependency().UserId.Returns(userId); + sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(true); + + await sutProvider.Sut.HandleAsync(context); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.False(context.HasSucceeded); } @@ -904,7 +1012,10 @@ public class BulkCollectionAuthorizationHandlerTests DeleteAnyCollection = true }; - ArrangeOrganizationAbility(sutProvider, organization, true); + // `LimitCollectonCreationDeletionSplit` feature flag state isn't + // relevant for this test. The flag is never checked for in this + // test. This is asserted below. + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureEnabled(sutProvider, organization, true, true); var context = new AuthorizationHandlerContext( new[] { BulkCollectionOperations.Delete }, @@ -916,6 +1027,7 @@ public class BulkCollectionAuthorizationHandlerTests await sutProvider.Sut.HandleAsync(context); + sutProvider.GetDependency().DidNotReceiveWithAnyArgs().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.True(context.HasSucceeded); } @@ -931,7 +1043,10 @@ public class BulkCollectionAuthorizationHandlerTests organization.Type = userType; organization.Permissions = new Permissions(); - ArrangeOrganizationAbility(sutProvider, organization, true); + // `LimitCollectonCreationDeletionSplit` feature flag state isn't + // relevant for this test. The flag is never checked for in this + // test. This is asserted below. + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureEnabled(sutProvider, organization, true, true); var context = new AuthorizationHandlerContext( new[] { BulkCollectionOperations.Delete }, @@ -943,11 +1058,12 @@ public class BulkCollectionAuthorizationHandlerTests await sutProvider.Sut.HandleAsync(context); + sutProvider.GetDependency().DidNotReceiveWithAnyArgs().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.True(context.HasSucceeded); } [Theory, BitAutoData, CollectionCustomization] - public async Task CanDeleteAsync_WhenUser_LimitCollectionCreationDeletionFalse_WithCanManagePermission_Success( + public async Task CanDeleteAsync_WhenUser_LimitCollectionDeletionFalse_WithCanManagePermission_WithLimitCollectionCreationDeletionSplitFeatureDisabled_Success( SutProvider sutProvider, ICollection collections, CurrentContextOrganization organization) @@ -957,11 +1073,12 @@ public class BulkCollectionAuthorizationHandlerTests organization.Type = OrganizationUserType.User; organization.Permissions = new Permissions(); - ArrangeOrganizationAbility(sutProvider, organization, false); + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureDisabled(sutProvider, organization, false, false); sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collections); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(false); foreach (var c in collections) { @@ -975,6 +1092,41 @@ public class BulkCollectionAuthorizationHandlerTests await sutProvider.Sut.HandleAsync(context); + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); + Assert.True(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task CanDeleteAsync_WhenUser_LimitCollectionDeletionFalse_WithCanManagePermission_WithLimitCollectionCreationDeletionSplitFeatureEnabled_Success( + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = OrganizationUserType.User; + organization.Permissions = new Permissions(); + + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureEnabled(sutProvider, organization, false, false); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collections); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(true); + + foreach (var c in collections) + { + c.Manage = true; + } + + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Delete }, + new ClaimsPrincipal(), + collections); + + await sutProvider.Sut.HandleAsync(context); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.True(context.HasSucceeded); } @@ -982,7 +1134,7 @@ public class BulkCollectionAuthorizationHandlerTests [BitAutoData(OrganizationUserType.Admin)] [BitAutoData(OrganizationUserType.Owner)] [BitAutoData(OrganizationUserType.User)] - public async Task CanDeleteAsync_LimitCollectionCreationDeletionFalse_AllowAdminAccessToAllCollectionItemsFalse_WithCanManagePermission_Success( + public async Task CanDeleteAsync_LimitCollectionDeletionFalse_AllowAdminAccessToAllCollectionItemsFalse_WithCanManagePermission_WithLimitCollectionCreationDeletionSplitFeatureDisabled_Success( OrganizationUserType userType, SutProvider sutProvider, ICollection collections, @@ -993,11 +1145,12 @@ public class BulkCollectionAuthorizationHandlerTests organization.Type = userType; organization.Permissions = new Permissions(); - ArrangeOrganizationAbility(sutProvider, organization, false, false); + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureDisabled(sutProvider, organization, false, false, false); sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collections); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(false); foreach (var c in collections) { @@ -1011,13 +1164,15 @@ public class BulkCollectionAuthorizationHandlerTests await sutProvider.Sut.HandleAsync(context); + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.True(context.HasSucceeded); } [Theory, CollectionCustomization] [BitAutoData(OrganizationUserType.Admin)] [BitAutoData(OrganizationUserType.Owner)] - public async Task CanDeleteAsync_WhenAdminOrOwner_LimitCollectionCreationDeletionTrue_AllowAdminAccessToAllCollectionItemsFalse_WithCanManagePermission_Success( + [BitAutoData(OrganizationUserType.User)] + public async Task CanDeleteAsync_LimitCollectionDeletionFalse_AllowAdminAccessToAllCollectionItemsFalse_WithCanManagePermission_WithLimitCollectionCreationDeletionSplitFeatureEnabled_Success( OrganizationUserType userType, SutProvider sutProvider, ICollection collections, @@ -1028,11 +1183,12 @@ public class BulkCollectionAuthorizationHandlerTests organization.Type = userType; organization.Permissions = new Permissions(); - ArrangeOrganizationAbility(sutProvider, organization, true, false); + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureEnabled(sutProvider, organization, false, false, false); sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collections); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(true); foreach (var c in collections) { @@ -1046,13 +1202,14 @@ public class BulkCollectionAuthorizationHandlerTests await sutProvider.Sut.HandleAsync(context); + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.True(context.HasSucceeded); } [Theory, CollectionCustomization] [BitAutoData(OrganizationUserType.Admin)] [BitAutoData(OrganizationUserType.Owner)] - public async Task CanDeleteAsync_WhenAdminOrOwner_LimitCollectionCreationDeletionTrue_AllowAdminAccessToAllCollectionItemsFalse_WithoutCanManagePermission_Failure( + public async Task CanDeleteAsync_WhenAdminOrOwner_LimitCollectionDeletionTrue_AllowAdminAccessToAllCollectionItemsFalse_WithCanManagePermission_WithLimitCollectionCreationDeletionSplitFeatureDisabled_Success( OrganizationUserType userType, SutProvider sutProvider, ICollection collections, @@ -1063,12 +1220,87 @@ public class BulkCollectionAuthorizationHandlerTests organization.Type = userType; organization.Permissions = new Permissions(); - ArrangeOrganizationAbility(sutProvider, organization, true, false); + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureDisabled(sutProvider, organization, true, true, false); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collections); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(false); + + foreach (var c in collections) + { + c.Manage = true; + } + + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Delete }, + new ClaimsPrincipal(), + collections); + + await sutProvider.Sut.HandleAsync(context); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); + Assert.True(context.HasSucceeded); + } + + [Theory, CollectionCustomization] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + public async Task CanDeleteAsync_WhenAdminOrOwner_LimitCollectionDeletionTrue_AllowAdminAccessToAllCollectionItemsFalse_WithCanManagePermission_WithLimitCollectionCreationDeletionSplitFeatureEnabled_Success( + OrganizationUserType userType, + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = userType; + organization.Permissions = new Permissions(); + + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureEnabled(sutProvider, organization, true, true, false); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collections); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(true); + + foreach (var c in collections) + { + c.Manage = true; + } + + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Delete }, + new ClaimsPrincipal(), + collections); + + await sutProvider.Sut.HandleAsync(context); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); + Assert.True(context.HasSucceeded); + } + + [Theory, CollectionCustomization] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + public async Task CanDeleteAsync_WhenAdminOrOwner_LimitCollectionDeletionTrue_AllowAdminAccessToAllCollectionItemsFalse_WithoutCanManagePermission_WithLimitCollectionCreationDeletionSplitFeatureDisabled_Failure( + OrganizationUserType userType, + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = userType; + organization.Permissions = new Permissions(); + + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureDisabled(sutProvider, organization, true, true, false); sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collections); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(false); foreach (var c in collections) { @@ -1082,11 +1314,50 @@ public class BulkCollectionAuthorizationHandlerTests await sutProvider.Sut.HandleAsync(context); + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); + Assert.False(context.HasSucceeded); + } + + [Theory, CollectionCustomization] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + public async Task CanDeleteAsync_WhenAdminOrOwner_LimitCollectionDeletionTrue_AllowAdminAccessToAllCollectionItemsFalse_WithoutCanManagePermission_WithLimitCollectionCreationDeletionSplitFeatureEnabled_Failure( + OrganizationUserType userType, + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = userType; + organization.Permissions = new Permissions(); + + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureEnabled(sutProvider, organization, true, true, false); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collections); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(true); + + foreach (var c in collections) + { + c.Manage = false; + } + + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Delete }, + new ClaimsPrincipal(), + collections); + + await sutProvider.Sut.HandleAsync(context); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.False(context.HasSucceeded); } [Theory, BitAutoData, CollectionCustomization] - public async Task CanDeleteAsync_WhenUser_LimitCollectionCreationDeletionTrue_AllowAdminAccessToAllCollectionItemsTrue_Failure( + public async Task CanDeleteAsync_WhenUser_LimitCollectionDeletionTrue_AllowAdminAccessToAllCollectionItemsTrue_WithLimitCollectionCreationDeletionSplitFeatureDisabled_Failure( SutProvider sutProvider, ICollection collections, CurrentContextOrganization organization) @@ -1096,12 +1367,13 @@ public class BulkCollectionAuthorizationHandlerTests organization.Type = OrganizationUserType.User; organization.Permissions = new Permissions(); - ArrangeOrganizationAbility(sutProvider, organization, true); + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureDisabled(sutProvider, organization, true, true); sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collections); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(false); foreach (var c in collections) { @@ -1115,11 +1387,12 @@ public class BulkCollectionAuthorizationHandlerTests await sutProvider.Sut.HandleAsync(context); + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.False(context.HasSucceeded); } [Theory, BitAutoData, CollectionCustomization] - public async Task CanDeleteAsync_WhenUser_LimitCollectionCreationDeletionTrue_AllowAdminAccessToAllCollectionItemsFalse_Failure( + public async Task CanDeleteAsync_WhenUser_LimitCollectionDeletionTrue_AllowAdminAccessToAllCollectionItemsTrue_WithLimitCollectionCreationDeletionSplitFeatureEnabled_Failure( SutProvider sutProvider, ICollection collections, CurrentContextOrganization organization) @@ -1129,12 +1402,13 @@ public class BulkCollectionAuthorizationHandlerTests organization.Type = OrganizationUserType.User; organization.Permissions = new Permissions(); - ArrangeOrganizationAbility(sutProvider, organization, true, false); + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureEnabled(sutProvider, organization, true, true); sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collections); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(true); foreach (var c in collections) { @@ -1148,13 +1422,88 @@ public class BulkCollectionAuthorizationHandlerTests await sutProvider.Sut.HandleAsync(context); + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); + Assert.False(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task CanDeleteAsync_WhenUser_LimitCollectionDeletionTrue_AllowAdminAccessToAllCollectionItemsFalse_WithLimitCollectionCreationDeletionSplitFeatureDisabled_Failure( + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = OrganizationUserType.User; + organization.Permissions = new Permissions(); + + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureDisabled(sutProvider, organization, true, true, false); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collections); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit) + .Returns(false); + + foreach (var c in collections) + { + c.Manage = true; + } + + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Delete }, + new ClaimsPrincipal(), + collections); + + await sutProvider.Sut.HandleAsync(context); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); + Assert.False(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task CanDeleteAsync_WhenUser_LimitCollectionDeletionTrue_AllowAdminAccessToAllCollectionItemsFalse_WithLimitCollectionCreationDeletionSplitFeatureEnabled_Failure( + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = OrganizationUserType.User; + organization.Permissions = new Permissions(); + + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureEnabled(sutProvider, organization, true, true, false); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collections); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit) + .Returns(true); + + foreach (var c in collections) + { + c.Manage = true; + } + + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Delete }, + new ClaimsPrincipal(), + collections); + + await sutProvider.Sut.HandleAsync(context); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.False(context.HasSucceeded); } [Theory, CollectionCustomization] [BitAutoData(OrganizationUserType.User)] [BitAutoData(OrganizationUserType.Custom)] - public async Task CanDeleteAsync_WhenMissingPermissions_NoSuccess( + public async Task CanDeleteAsync_WhenMissingPermissions_WithLimitCollectionCreationDeletionSplitFeatureDisabled_NoSuccess( OrganizationUserType userType, SutProvider sutProvider, ICollection collections, @@ -1171,7 +1520,7 @@ public class BulkCollectionAuthorizationHandlerTests ManageUsers = false }; - ArrangeOrganizationAbility(sutProvider, organization, true); + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureDisabled(sutProvider, organization, true, true); var context = new AuthorizationHandlerContext( new[] { BulkCollectionOperations.Delete }, @@ -1181,14 +1530,54 @@ public class BulkCollectionAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(false); await sutProvider.Sut.HandleAsync(context); + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); + Assert.False(context.HasSucceeded); + } + + [Theory, CollectionCustomization] + [BitAutoData(OrganizationUserType.User)] + [BitAutoData(OrganizationUserType.Custom)] + public async Task CanDeleteAsync_WhenMissingPermissions_WithLimitCollectionCreationDeletionSplitFeatureEnabled_NoSuccess( + OrganizationUserType userType, + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = userType; + organization.Permissions = new Permissions + { + EditAnyCollection = false, + DeleteAnyCollection = false, + ManageGroups = false, + ManageUsers = false + }; + + ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureEnabled(sutProvider, organization, true, true); + + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Delete }, + new ClaimsPrincipal(), + collections); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(true); + + await sutProvider.Sut.HandleAsync(context); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.False(context.HasSucceeded); } [Theory, BitAutoData, CollectionCustomization] - public async Task CanDeleteAsync_WhenMissingOrgAccess_NoSuccess( + public async Task CanDeleteAsync_WhenMissingOrgAccess_WithLimitCollectionCreationDeletionSplitFeatureDisabled_NoSuccess( Guid userId, ICollection collections, SutProvider sutProvider) @@ -1202,8 +1591,34 @@ public class BulkCollectionAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(false); await sutProvider.Sut.HandleAsync(context); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); + Assert.False(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task CanDeleteAsync_WhenMissingOrgAccess_WithLimitCollectionCreationDeletionSplitFeatureEnabled_NoSuccess( + Guid userId, + ICollection collections, + SutProvider sutProvider) + { + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Delete }, + new ClaimsPrincipal(), + collections + ); + + sutProvider.GetDependency().UserId.Returns(userId); + sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(true); + + await sutProvider.Sut.HandleAsync(context); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); Assert.False(context.HasSucceeded); } @@ -1224,6 +1639,7 @@ public class BulkCollectionAuthorizationHandlerTests await sutProvider.Sut.HandleAsync(context); Assert.True(context.HasFailed); sutProvider.GetDependency().DidNotReceiveWithAnyArgs(); + sutProvider.GetDependency().DidNotReceiveWithAnyArgs().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); } [Theory, BitAutoData, CollectionCustomization] @@ -1247,10 +1663,11 @@ public class BulkCollectionAuthorizationHandlerTests var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.HandleAsync(context)); Assert.Equal("Requested collections must belong to the same organization.", exception.Message); sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetOrganization(default); + sutProvider.GetDependency().DidNotReceiveWithAnyArgs().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit); } [Theory, BitAutoData, CollectionCustomization] - public async Task HandleRequirementAsync_Provider_Success( + public async Task HandleRequirementAsync_Provider_WithLimitCollectionCreationDeletionSplitFeatureDisabled_Success( SutProvider sutProvider, ICollection collections) { @@ -1286,6 +1703,63 @@ public class BulkCollectionAuthorizationHandlerTests sutProvider.GetDependency().GetOrganizationAbilitiesAsync() .Returns(organizationAbilities); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(true); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(false); + + var context = new AuthorizationHandlerContext( + new[] { op }, + new ClaimsPrincipal(), + collections + ); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + await sutProvider.GetDependency().Received().ProviderUserForOrgAsync(orgId); + + // Recreate the SUT to reset the mocks/dependencies between tests + sutProvider.Recreate(); + } + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task HandleRequirementAsync_Provider_WithLimitCollectionCreationDeletionSplitFeatureEnabled_Success( + SutProvider sutProvider, + ICollection collections) + { + var actingUserId = Guid.NewGuid(); + var orgId = collections.First().OrganizationId; + + var organizationAbilities = new Dictionary + { + { collections.First().OrganizationId, + new OrganizationAbility + { + LimitCollectionCreation = true, + LimitCollectionDeletion = true, + AllowAdminAccessToAllCollectionItems = true + } + } + }; + + var operationsToTest = new[] + { + BulkCollectionOperations.Create, + BulkCollectionOperations.Read, + BulkCollectionOperations.ReadAccess, + BulkCollectionOperations.Update, + BulkCollectionOperations.ModifyUserAccess, + BulkCollectionOperations.ModifyGroupAccess, + BulkCollectionOperations.Delete, + }; + + foreach (var op in operationsToTest) + { + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(orgId).Returns((CurrentContextOrganization)null); + sutProvider.GetDependency().GetOrganizationAbilitiesAsync() + .Returns(organizationAbilities); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(true); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.LimitCollectionCreationDeletionSplit).Returns(true); var context = new AuthorizationHandlerContext( new[] { op }, @@ -1336,14 +1810,37 @@ public class BulkCollectionAuthorizationHandlerTests await sutProvider.GetDependency().Received(1).GetManyByUserIdAsync(Arg.Any()); } - private static void ArrangeOrganizationAbility( + private static void ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureDisabled( SutProvider sutProvider, - CurrentContextOrganization organization, bool limitCollectionCreationDeletion, + CurrentContextOrganization organization, + bool limitCollectionCreation, + bool limitCollectionDeletion, bool allowAdminAccessToAllCollectionItems = true) { var organizationAbility = new OrganizationAbility(); organizationAbility.Id = organization.Id; - organizationAbility.LimitCollectionCreationDeletion = limitCollectionCreationDeletion; + + organizationAbility.LimitCollectionCreationDeletion = limitCollectionCreation || limitCollectionDeletion; + + organizationAbility.AllowAdminAccessToAllCollectionItems = allowAdminAccessToAllCollectionItems; + + sutProvider.GetDependency().GetOrganizationAbilityAsync(organizationAbility.Id) + .Returns(organizationAbility); + } + + private static void ArrangeOrganizationAbility_WithLimitCollectionCreationDeletionSplitFeatureEnabled( + SutProvider sutProvider, + CurrentContextOrganization organization, + bool limitCollectionCreation, + bool limitCollectionDeletion, + bool allowAdminAccessToAllCollectionItems = true) + { + var organizationAbility = new OrganizationAbility(); + organizationAbility.Id = organization.Id; + + organizationAbility.LimitCollectionCreation = limitCollectionCreation; + organizationAbility.LimitCollectionDeletion = limitCollectionDeletion; + organizationAbility.AllowAdminAccessToAllCollectionItems = allowAdminAccessToAllCollectionItems; sutProvider.GetDependency().GetOrganizationAbilityAsync(organizationAbility.Id) diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepositoryTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepositoryTests.cs index 3b102c788..dba511074 100644 --- a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepositoryTests.cs @@ -253,6 +253,9 @@ public class OrganizationUserRepositoryTests Assert.Equal(orgUser1.Permissions, result.Permissions); Assert.Equal(organization.SmSeats, result.SmSeats); Assert.Equal(organization.SmServiceAccounts, result.SmServiceAccounts); + Assert.Equal(organization.LimitCollectionCreation, result.LimitCollectionCreation); + Assert.Equal(organization.LimitCollectionDeletion, result.LimitCollectionDeletion); + // Deprecated: https://bitwarden.atlassian.net/browse/PM-10863 Assert.Equal(organization.LimitCollectionCreationDeletion, result.LimitCollectionCreationDeletion); Assert.Equal(organization.AllowAdminAccessToAllCollectionItems, result.AllowAdminAccessToAllCollectionItems); }