mirror of
https://github.com/bitwarden/server.git
synced 2025-05-31 00:00:34 -05:00
[AC-2086] Update CanDelete to handle V1 flag logic (#3979)
* feat: Update authorization handler to handle V1 collection enhancement, refs AC-2086 * feat: update tests to account for new V1 flag/setting logic, refs AC-2086 * feat: update CanDelete with all collection enhancement combinations, refs AC-2086 * feat: add tests for new delete flows, refs AC-2086 * fix: update new conditionals with bool return value, refs AC-2086 * feat: simplify conditional in regards to LimitCollectionCreationDeletion, refs AC-2086 * feat: simplify AllowAdminAccessToAllCollectionItems conditional, refs AC-2086 * feat: add unit test making sure admins can't delete collections without can manage, refs AC-2086
This commit is contained in:
parent
45be4d5069
commit
df4d1d5552
@ -227,24 +227,29 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler<BulkC
|
|||||||
|
|
||||||
private async Task<bool> CanDeleteAsync(ICollection<Collection> resources, CurrentContextOrganization? org)
|
private async Task<bool> CanDeleteAsync(ICollection<Collection> resources, CurrentContextOrganization? org)
|
||||||
{
|
{
|
||||||
// Owners, Admins, and users with DeleteAnyCollection permission can always delete collections
|
// Users with DeleteAnyCollection permission can always delete collections
|
||||||
if (org is
|
if (org is { Permissions.DeleteAnyCollection: true })
|
||||||
{ Type: OrganizationUserType.Owner or OrganizationUserType.Admin } or
|
|
||||||
{ Permissions.DeleteAnyCollection: true })
|
|
||||||
{
|
{
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check for non-null org here: the user must be apart of the organization for this setting to take affect
|
// If AllowAdminAccessToAllCollectionItems is true, Owners and Admins can delete any collection, regardless of LimitCollectionCreationDeletion setting
|
||||||
// The limit collection management setting is disabled,
|
var organizationAbility = await GetOrganizationAbilityAsync(org);
|
||||||
// ensure acting user has manage permissions for all collections being deleted
|
var allowAdminAccessToAllCollectionItems = !_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1) ||
|
||||||
if (await GetOrganizationAbilityAsync(org) is { LimitCollectionCreationDeletion: false })
|
organizationAbility is { AllowAdminAccessToAllCollectionItems: true };
|
||||||
|
if (allowAdminAccessToAllCollectionItems && org is { Type: OrganizationUserType.Owner or OrganizationUserType.Admin })
|
||||||
{
|
{
|
||||||
var canManageCollections = await CanManageCollectionsAsync(resources, org);
|
return true;
|
||||||
if (canManageCollections)
|
}
|
||||||
{
|
|
||||||
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
|
// Allow providers to delete collections if they are a provider for the target organization
|
||||||
|
@ -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<BulkCollectionAuthorizationHandler> sutProvider,
|
|
||||||
ICollection<Collection> 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<ICurrentContext>().UserId.Returns(userId);
|
|
||||||
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
|
|
||||||
|
|
||||||
await sutProvider.Sut.HandleAsync(context);
|
|
||||||
|
|
||||||
Assert.True(context.HasSucceeded);
|
|
||||||
}
|
|
||||||
|
|
||||||
[Theory, BitAutoData, CollectionCustomization]
|
[Theory, BitAutoData, CollectionCustomization]
|
||||||
public async Task CanDeleteAsync_WithDeleteAnyCollectionPermission_Success(
|
public async Task CanDeleteAsync_WithDeleteAnyCollectionPermission_Success(
|
||||||
SutProvider<BulkCollectionAuthorizationHandler> sutProvider,
|
SutProvider<BulkCollectionAuthorizationHandler> sutProvider,
|
||||||
@ -959,8 +932,63 @@ public class BulkCollectionAuthorizationHandlerTests
|
|||||||
Assert.True(context.HasSucceeded);
|
Assert.True(context.HasSucceeded);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Theory, CollectionCustomization]
|
||||||
|
[BitAutoData(OrganizationUserType.Admin)]
|
||||||
|
[BitAutoData(OrganizationUserType.Owner)]
|
||||||
|
public async Task CanDeleteAsync_WhenAdminOrOwner_AllowAdminAccessToAllCollectionItemsTrue_Success(
|
||||||
|
OrganizationUserType userType,
|
||||||
|
Guid userId, SutProvider<BulkCollectionAuthorizationHandler> sutProvider,
|
||||||
|
ICollection<Collection> 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<ICurrentContext>().UserId.Returns(userId);
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().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<BulkCollectionAuthorizationHandler> sutProvider,
|
||||||
|
ICollection<Collection> 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<ICurrentContext>().UserId.Returns(userId);
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
|
||||||
|
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(false);
|
||||||
|
|
||||||
|
await sutProvider.Sut.HandleAsync(context);
|
||||||
|
|
||||||
|
Assert.True(context.HasSucceeded);
|
||||||
|
}
|
||||||
|
|
||||||
[Theory, BitAutoData, CollectionCustomization]
|
[Theory, BitAutoData, CollectionCustomization]
|
||||||
public async Task CanDeleteAsync_WithManageCollectionPermission_Success(
|
public async Task CanDeleteAsync_WhenUser_LimitCollectionCreationDeletionFalse_WithCanManagePermission_Success(
|
||||||
SutProvider<BulkCollectionAuthorizationHandler> sutProvider,
|
SutProvider<BulkCollectionAuthorizationHandler> sutProvider,
|
||||||
ICollection<CollectionDetails> collections,
|
ICollection<CollectionDetails> collections,
|
||||||
CurrentContextOrganization organization)
|
CurrentContextOrganization organization)
|
||||||
@ -991,6 +1019,184 @@ public class BulkCollectionAuthorizationHandlerTests
|
|||||||
Assert.True(context.HasSucceeded);
|
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<BulkCollectionAuthorizationHandler> sutProvider,
|
||||||
|
ICollection<CollectionDetails> collections,
|
||||||
|
CurrentContextOrganization organization)
|
||||||
|
{
|
||||||
|
var actingUserId = Guid.NewGuid();
|
||||||
|
|
||||||
|
organization.Type = userType;
|
||||||
|
organization.Permissions = new Permissions();
|
||||||
|
|
||||||
|
ArrangeOrganizationAbility(sutProvider, organization, false, false);
|
||||||
|
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
|
||||||
|
sutProvider.GetDependency<ICollectionRepository>().GetManyByUserIdAsync(actingUserId, Arg.Any<bool>()).Returns(collections);
|
||||||
|
sutProvider.GetDependency<IFeatureService>().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<BulkCollectionAuthorizationHandler> sutProvider,
|
||||||
|
ICollection<CollectionDetails> collections,
|
||||||
|
CurrentContextOrganization organization)
|
||||||
|
{
|
||||||
|
var actingUserId = Guid.NewGuid();
|
||||||
|
|
||||||
|
organization.Type = userType;
|
||||||
|
organization.Permissions = new Permissions();
|
||||||
|
|
||||||
|
ArrangeOrganizationAbility(sutProvider, organization, true, false);
|
||||||
|
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
|
||||||
|
sutProvider.GetDependency<ICollectionRepository>().GetManyByUserIdAsync(actingUserId, Arg.Any<bool>()).Returns(collections);
|
||||||
|
sutProvider.GetDependency<IFeatureService>().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<BulkCollectionAuthorizationHandler> sutProvider,
|
||||||
|
ICollection<CollectionDetails> collections,
|
||||||
|
CurrentContextOrganization organization)
|
||||||
|
{
|
||||||
|
var actingUserId = Guid.NewGuid();
|
||||||
|
|
||||||
|
organization.Type = userType;
|
||||||
|
organization.Permissions = new Permissions();
|
||||||
|
|
||||||
|
ArrangeOrganizationAbility(sutProvider, organization, true, false);
|
||||||
|
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
|
||||||
|
sutProvider.GetDependency<ICollectionRepository>().GetManyByUserIdAsync(actingUserId, Arg.Any<bool>()).Returns(collections);
|
||||||
|
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(Arg.Any<Guid>()).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<BulkCollectionAuthorizationHandler> sutProvider,
|
||||||
|
ICollection<CollectionDetails> collections,
|
||||||
|
CurrentContextOrganization organization)
|
||||||
|
{
|
||||||
|
var actingUserId = Guid.NewGuid();
|
||||||
|
|
||||||
|
organization.Type = OrganizationUserType.User;
|
||||||
|
organization.Permissions = new Permissions();
|
||||||
|
|
||||||
|
ArrangeOrganizationAbility(sutProvider, organization, true);
|
||||||
|
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
|
||||||
|
sutProvider.GetDependency<ICollectionRepository>().GetManyByUserIdAsync(actingUserId, Arg.Any<bool>()).Returns(collections);
|
||||||
|
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(Arg.Any<Guid>()).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<BulkCollectionAuthorizationHandler> sutProvider,
|
||||||
|
ICollection<CollectionDetails> collections,
|
||||||
|
CurrentContextOrganization organization)
|
||||||
|
{
|
||||||
|
var actingUserId = Guid.NewGuid();
|
||||||
|
|
||||||
|
organization.Type = OrganizationUserType.User;
|
||||||
|
organization.Permissions = new Permissions();
|
||||||
|
|
||||||
|
ArrangeOrganizationAbility(sutProvider, organization, true, false);
|
||||||
|
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
|
||||||
|
sutProvider.GetDependency<ICollectionRepository>().GetManyByUserIdAsync(actingUserId, Arg.Any<bool>()).Returns(collections);
|
||||||
|
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
|
||||||
|
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(Arg.Any<Guid>()).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]
|
[Theory, CollectionCustomization]
|
||||||
[BitAutoData(OrganizationUserType.User)]
|
[BitAutoData(OrganizationUserType.User)]
|
||||||
[BitAutoData(OrganizationUserType.Custom)]
|
[BitAutoData(OrganizationUserType.Custom)]
|
||||||
@ -1102,7 +1308,8 @@ public class BulkCollectionAuthorizationHandlerTests
|
|||||||
{ collections.First().OrganizationId,
|
{ collections.First().OrganizationId,
|
||||||
new OrganizationAbility
|
new OrganizationAbility
|
||||||
{
|
{
|
||||||
LimitCollectionCreationDeletion = true
|
LimitCollectionCreationDeletion = true,
|
||||||
|
AllowAdminAccessToAllCollectionItems = true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
@ -1177,12 +1384,14 @@ public class BulkCollectionAuthorizationHandlerTests
|
|||||||
|
|
||||||
private static void ArrangeOrganizationAbility(
|
private static void ArrangeOrganizationAbility(
|
||||||
SutProvider<BulkCollectionAuthorizationHandler> sutProvider,
|
SutProvider<BulkCollectionAuthorizationHandler> sutProvider,
|
||||||
CurrentContextOrganization organization, bool limitCollectionCreationDeletion)
|
CurrentContextOrganization organization, bool limitCollectionCreationDeletion,
|
||||||
|
bool allowAdminAccessToAllCollectionItems = true)
|
||||||
{
|
{
|
||||||
var organizationAbility = new OrganizationAbility();
|
var organizationAbility = new OrganizationAbility();
|
||||||
organizationAbility.Id = organization.Id;
|
organizationAbility.Id = organization.Id;
|
||||||
organizationAbility.FlexibleCollections = true;
|
organizationAbility.FlexibleCollections = true;
|
||||||
organizationAbility.LimitCollectionCreationDeletion = limitCollectionCreationDeletion;
|
organizationAbility.LimitCollectionCreationDeletion = limitCollectionCreationDeletion;
|
||||||
|
organizationAbility.AllowAdminAccessToAllCollectionItems = allowAdminAccessToAllCollectionItems;
|
||||||
|
|
||||||
sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organizationAbility.Id)
|
sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organizationAbility.Id)
|
||||||
.Returns(organizationAbility);
|
.Returns(organizationAbility);
|
||||||
|
Loading…
x
Reference in New Issue
Block a user