nhyatt/bitwarden
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2025-07-02 00:22:50 -05:00
Code Activity

[AC-1139] Rewrote CollectionAuthorizationHandler to be similar to other AuthHandlers; Revisited unit tests

Browse Source
This commit is contained in:
Rui Tome
2023-11-24 16:16:07 +00:00
parent 22a90dee77
commit 95e6211ab9
2 changed files with 59 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
src/Api/Vault/AuthorizationHandlers/Collections/CollectionAuthorizationHandler.cs
View File

@ -16,7 +16,6 @@ public class CollectionAuthorizationHandler : AuthorizationHandler<CollectionOpe
{ {
private readonly ICurrentContext _currentContext; private readonly ICurrentContext _currentContext;
private readonly IFeatureService _featureService; private readonly IFeatureService _featureService;
private Guid _targetOrganizationId;
private bool UseFlexibleCollections => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); private bool UseFlexibleCollections => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext);
@ -65,22 +64,21 @@ public class CollectionAuthorizationHandler : AuthorizationHandler<CollectionOpe
} }
private async Task CanReadAllAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement, private async Task CanReadAllAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement,
CurrentContextOrganization org) CurrentContextOrganization? org)
{ {
if (org != null) // Owners, Admins, and users with EditAnyCollection, DeleteAnyCollection,
{ // or AccessImportExport permission can always read a collection
// Acting user is a member of the target organization, check permissions if (org is
if (org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin || { Type: OrganizationUserType.Owner or OrganizationUserType.Admin } or
org.Permissions.EditAnyCollection || { Permissions.EditAnyCollection: true } or
org.Permissions.DeleteAnyCollection || { Permissions.DeleteAnyCollection: true } or
org.Permissions.AccessImportExport) { Permissions.AccessImportExport: true })
{ {
context.Succeed(requirement); context.Succeed(requirement);
return; return;
} }
}
// Check if acting user is a provider user for the target organization // Allow provider users to read collections if they are a provider for the target organization
if (await _currentContext.ProviderUserForOrgAsync(requirement.OrganizationId)) if (await _currentContext.ProviderUserForOrgAsync(requirement.OrganizationId))
{ {
context.Succeed(requirement); context.Succeed(requirement);
@ -88,21 +86,20 @@ public class CollectionAuthorizationHandler : AuthorizationHandler<CollectionOpe
} }
private async Task CanReadAllWithAccessAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement, private async Task CanReadAllWithAccessAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement,
CurrentContextOrganization org) CurrentContextOrganization? org)
{ {
if (org != null) // Owners, Admins, and users with EditAnyCollection or DeleteAnyCollection
{ // permission can always read a collection
// Acting user is a member of the target organization, check permissions if (org is
if (org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin || { Type: OrganizationUserType.Owner or OrganizationUserType.Admin } or
org.Permissions.EditAnyCollection || { Permissions.EditAnyCollection: true } or
org.Permissions.DeleteAnyCollection) { Permissions.DeleteAnyCollection: true })
{ {
context.Succeed(requirement); context.Succeed(requirement);
return; return;
} }
}
// Check if acting user is a provider user for the target organization // Allow provider users to read collections if they are a provider for the target organization
if (await _currentContext.ProviderUserForOrgAsync(requirement.OrganizationId)) if (await _currentContext.ProviderUserForOrgAsync(requirement.OrganizationId))
{ {
context.Succeed(requirement); context.Succeed(requirement);

59
test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs
View File

@ -101,7 +101,7 @@ public class CollectionAuthorizationHandlerTests
[Theory] [Theory]
[BitAutoData(OrganizationUserType.User)] [BitAutoData(OrganizationUserType.User)]
[BitAutoData(OrganizationUserType.Custom)] [BitAutoData(OrganizationUserType.Custom)]
public async Task CanReadAllAsync_WhenMissingAccess_Failure( public async Task CanReadAllAsync_WhenMissingPermissions_NoSuccess(
OrganizationUserType userType, OrganizationUserType userType,
SutProvider<CollectionAuthorizationHandler> sutProvider, SutProvider<CollectionAuthorizationHandler> sutProvider,
CurrentContextOrganization organization) CurrentContextOrganization organization)
@ -211,7 +211,7 @@ public class CollectionAuthorizationHandlerTests
[Theory] [Theory]
[BitAutoData(OrganizationUserType.User)] [BitAutoData(OrganizationUserType.User)]
[BitAutoData(OrganizationUserType.Custom)] [BitAutoData(OrganizationUserType.Custom)]
public async Task CanReadAllWithAccessAsync_WhenMissingAccess_Failure( public async Task CanReadAllWithAccessAsync_WhenMissingPermissions_NoSuccess(
OrganizationUserType userType, OrganizationUserType userType,
SutProvider<CollectionAuthorizationHandler> sutProvider, SutProvider<CollectionAuthorizationHandler> sutProvider,
CurrentContextOrganization organization) CurrentContextOrganization organization)
@ -240,25 +240,7 @@ public class CollectionAuthorizationHandlerTests
} }
[Theory, BitAutoData] [Theory, BitAutoData]
public async Task HandleRequirementAsync_MissingUserId_Failure( public async Task HandleRequirementAsync_WhenMissingOrgAccess_NoSuccess(
Guid organizationId,
SutProvider<CollectionAuthorizationHandler> sutProvider)
{
var context = new AuthorizationHandlerContext(
new[] { CollectionOperations.ReadAll(organizationId) },
new ClaimsPrincipal(),
null
);
// Simulate missing user id
sutProvider.GetDependency<ICurrentContext>().UserId.Returns((Guid?)null);
await sutProvider.Sut.HandleAsync(context);
Assert.False(context.HasSucceeded);
}
[Theory, BitAutoData]
public async Task HandleRequirementAsync_MissingOrg_Failure(
Guid userId, Guid userId,
Guid organizationId, Guid organizationId,
SutProvider<CollectionAuthorizationHandler> sutProvider) SutProvider<CollectionAuthorizationHandler> sutProvider)
@ -275,4 +257,39 @@ public class CollectionAuthorizationHandlerTests
await sutProvider.Sut.HandleAsync(context); await sutProvider.Sut.HandleAsync(context);
Assert.False(context.HasSucceeded); Assert.False(context.HasSucceeded);
} }
[Theory, BitAutoData]
public async Task HandleRequirementAsync_MissingUserId_Failure(
Guid organizationId,
SutProvider<CollectionAuthorizationHandler> sutProvider)
{
var context = new AuthorizationHandlerContext(
new[] { CollectionOperations.ReadAll(organizationId) },
new ClaimsPrincipal(),
null
);
// Simulate missing user id
sutProvider.GetDependency<ICurrentContext>().UserId.Returns((Guid?)null);
await sutProvider.Sut.HandleAsync(context);
Assert.True(context.HasFailed);
}
[Theory, BitAutoData]
public async Task HandleRequirementAsync_NoSpecifiedOrgId_Failure(
SutProvider<CollectionAuthorizationHandler> sutProvider)
{
var context = new AuthorizationHandlerContext(
new[] { CollectionOperations.ReadAll(default) },
new ClaimsPrincipal(),
null
);
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(new Guid());
await sutProvider.Sut.HandleAsync(context);
Assert.True(context.HasFailed);
}
} }