diff --git a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs index 497c8b9a1..0638d16d6 100644 --- a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs +++ b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs @@ -227,24 +227,29 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler CanDeleteAsync(ICollection resources, CurrentContextOrganization? org) { - // Owners, Admins, and users with DeleteAnyCollection permission can always delete collections - if (org is - { Type: OrganizationUserType.Owner or OrganizationUserType.Admin } or - { Permissions.DeleteAnyCollection: true }) + // Users with DeleteAnyCollection permission can always delete collections + if (org is { Permissions.DeleteAnyCollection: true }) { return true; } - // Check for non-null org here: the user must be apart of the organization for this setting to take affect - // The limit collection management setting is disabled, - // ensure acting user has manage permissions for all collections being deleted - if (await GetOrganizationAbilityAsync(org) is { LimitCollectionCreationDeletion: false }) + // If AllowAdminAccessToAllCollectionItems is true, Owners and Admins can delete any collection, regardless of LimitCollectionCreationDeletion setting + var organizationAbility = await GetOrganizationAbilityAsync(org); + var allowAdminAccessToAllCollectionItems = !_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1) || + organizationAbility is { AllowAdminAccessToAllCollectionItems: true }; + if (allowAdminAccessToAllCollectionItems && org is { Type: OrganizationUserType.Owner or OrganizationUserType.Admin }) { - var canManageCollections = await CanManageCollectionsAsync(resources, org); - if (canManageCollections) - { - return true; - } + return true; + } + + // If LimitCollectionCreationDeletion is false, AllowAdminAccessToAllCollectionItems setting is irrelevant. + // Ensure acting user has manage permissions for all collections being deleted + // If LimitCollectionCreationDeletion is true, only Owners and Admins can delete collections they manage + var canDeleteManagedCollections = organizationAbility is { LimitCollectionCreationDeletion: false } || + org is { Type: OrganizationUserType.Owner or OrganizationUserType.Admin }; + if (canDeleteManagedCollections && await CanManageCollectionsAsync(resources, org)) + { + return true; } // Allow providers to delete collections if they are a provider for the target organization diff --git a/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs b/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs index dd8bacaa9..527896f93 100644 --- a/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs +++ b/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs @@ -903,33 +903,6 @@ public class BulkCollectionAuthorizationHandlerTests } } - [Theory, CollectionCustomization] - [BitAutoData(OrganizationUserType.Admin)] - [BitAutoData(OrganizationUserType.Owner)] - public async Task CanDeleteAsync_WhenAdminOrOwner_Success( - OrganizationUserType userType, - Guid userId, SutProvider sutProvider, - ICollection collections, - CurrentContextOrganization organization) - { - organization.Type = userType; - organization.Permissions = new Permissions(); - - ArrangeOrganizationAbility(sutProvider, organization, true); - - var context = new AuthorizationHandlerContext( - new[] { BulkCollectionOperations.Delete }, - new ClaimsPrincipal(), - collections); - - sutProvider.GetDependency().UserId.Returns(userId); - sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - - await sutProvider.Sut.HandleAsync(context); - - Assert.True(context.HasSucceeded); - } - [Theory, BitAutoData, CollectionCustomization] public async Task CanDeleteAsync_WithDeleteAnyCollectionPermission_Success( SutProvider sutProvider, @@ -959,8 +932,63 @@ public class BulkCollectionAuthorizationHandlerTests Assert.True(context.HasSucceeded); } + [Theory, CollectionCustomization] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + public async Task CanDeleteAsync_WhenAdminOrOwner_AllowAdminAccessToAllCollectionItemsTrue_Success( + OrganizationUserType userType, + Guid userId, SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + organization.Type = userType; + organization.Permissions = new Permissions(); + + ArrangeOrganizationAbility(sutProvider, organization, true); + + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Delete }, + new ClaimsPrincipal(), + collections); + + sutProvider.GetDependency().UserId.Returns(userId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + [Theory, CollectionCustomization] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + public async Task CanDeleteAsync_WhenAdminOrOwner_V1FlagDisabled_Success( + OrganizationUserType userType, + Guid userId, SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + organization.Type = userType; + organization.Permissions = new Permissions(); + + ArrangeOrganizationAbility(sutProvider, organization, true, false); + + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Delete }, + new ClaimsPrincipal(), + collections); + + sutProvider.GetDependency().UserId.Returns(userId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(false); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + [Theory, BitAutoData, CollectionCustomization] - public async Task CanDeleteAsync_WithManageCollectionPermission_Success( + public async Task CanDeleteAsync_WhenUser_LimitCollectionCreationDeletionFalse_WithCanManagePermission_Success( SutProvider sutProvider, ICollection collections, CurrentContextOrganization organization) @@ -991,6 +1019,184 @@ public class BulkCollectionAuthorizationHandlerTests Assert.True(context.HasSucceeded); } + [Theory, CollectionCustomization] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + [BitAutoData(OrganizationUserType.User)] + public async Task CanDeleteAsync_LimitCollectionCreationDeletionFalse_AllowAdminAccessToAllCollectionItemsFalse_WithCanManagePermission_Success( + OrganizationUserType userType, + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = userType; + organization.Permissions = new Permissions(); + + ArrangeOrganizationAbility(sutProvider, organization, false, false); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId, Arg.Any()).Returns(collections); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).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); + + Assert.True(context.HasSucceeded); + } + + [Theory, CollectionCustomization] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + public async Task CanDeleteAsync_WhenAdminOrOwner_LimitCollectionCreationDeletionTrue_AllowAdminAccessToAllCollectionItemsFalse_WithCanManagePermission_Success( + OrganizationUserType userType, + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = userType; + organization.Permissions = new Permissions(); + + ArrangeOrganizationAbility(sutProvider, organization, true, false); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId, Arg.Any()).Returns(collections); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).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); + + Assert.True(context.HasSucceeded); + } + + [Theory, CollectionCustomization] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + public async Task CanDeleteAsync_WhenAdminOrOwner_LimitCollectionCreationDeletionTrue_AllowAdminAccessToAllCollectionItemsFalse_WithoutCanManagePermission_Failure( + OrganizationUserType userType, + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = userType; + organization.Permissions = new Permissions(); + + ArrangeOrganizationAbility(sutProvider, organization, true, false); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId, Arg.Any()).Returns(collections); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + + foreach (var c in collections) + { + c.Manage = false; + } + + var context = new AuthorizationHandlerContext( + new[] { BulkCollectionOperations.Delete }, + new ClaimsPrincipal(), + collections); + + await sutProvider.Sut.HandleAsync(context); + + Assert.False(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task CanDeleteAsync_WhenUser_LimitCollectionCreationDeletionTrue_AllowAdminAccessToAllCollectionItemsTrue_Failure( + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = OrganizationUserType.User; + organization.Permissions = new Permissions(); + + ArrangeOrganizationAbility(sutProvider, organization, true); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId, Arg.Any()).Returns(collections); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).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); + + Assert.False(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task CanDeleteAsync_WhenUser_LimitCollectionCreationDeletionTrue_AllowAdminAccessToAllCollectionItemsFalse_Failure( + SutProvider sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = OrganizationUserType.User; + organization.Permissions = new Permissions(); + + ArrangeOrganizationAbility(sutProvider, organization, true, false); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId, Arg.Any()).Returns(collections); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).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); + + Assert.False(context.HasSucceeded); + } + [Theory, CollectionCustomization] [BitAutoData(OrganizationUserType.User)] [BitAutoData(OrganizationUserType.Custom)] @@ -1102,7 +1308,8 @@ public class BulkCollectionAuthorizationHandlerTests { collections.First().OrganizationId, new OrganizationAbility { - LimitCollectionCreationDeletion = true + LimitCollectionCreationDeletion = true, + AllowAdminAccessToAllCollectionItems = true } } }; @@ -1177,12 +1384,14 @@ public class BulkCollectionAuthorizationHandlerTests private static void ArrangeOrganizationAbility( SutProvider sutProvider, - CurrentContextOrganization organization, bool limitCollectionCreationDeletion) + CurrentContextOrganization organization, bool limitCollectionCreationDeletion, + bool allowAdminAccessToAllCollectionItems = true) { var organizationAbility = new OrganizationAbility(); organizationAbility.Id = organization.Id; organizationAbility.FlexibleCollections = true; organizationAbility.LimitCollectionCreationDeletion = limitCollectionCreationDeletion; + organizationAbility.AllowAdminAccessToAllCollectionItems = allowAdminAccessToAllCollectionItems; sutProvider.GetDependency().GetOrganizationAbilityAsync(organizationAbility.Id) .Returns(organizationAbility);