mirror of
https://github.com/bitwarden/server.git
synced 2025-07-01 08:02:49 -05:00
[AC-1139] Addressing PR suggestions
This commit is contained in:
@ -116,10 +116,7 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler<Colle
|
||||
await _currentContext.ProviderUserForOrgAsync(org.Id))
|
||||
{
|
||||
context.Succeed(requirement);
|
||||
return;
|
||||
}
|
||||
|
||||
context.Fail();
|
||||
}
|
||||
|
||||
private async Task CanReadAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement,
|
||||
@ -132,15 +129,11 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler<Colle
|
||||
return;
|
||||
}
|
||||
|
||||
var canManageCollections = await CanManageCollectionsAsync(targetCollections, org, requireManagePermission: false);
|
||||
var canManageCollections = await HasCollectionAccessAsync(targetCollections, org, requireManagePermission: false);
|
||||
if (canManageCollections)
|
||||
{
|
||||
context.Succeed(requirement);
|
||||
}
|
||||
else
|
||||
{
|
||||
context.Fail();
|
||||
}
|
||||
}
|
||||
|
||||
private async Task CanDeleteAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement,
|
||||
@ -163,15 +156,11 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler<Colle
|
||||
return;
|
||||
}
|
||||
|
||||
var canManageCollections = await CanManageCollectionsAsync(targetCollections, org, requireManagePermission: true);
|
||||
var canManageCollections = await HasCollectionAccessAsync(targetCollections, org, requireManagePermission: true);
|
||||
if (canManageCollections)
|
||||
{
|
||||
context.Succeed(requirement);
|
||||
}
|
||||
else
|
||||
{
|
||||
context.Fail();
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@ -190,18 +179,14 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler<Colle
|
||||
return;
|
||||
}
|
||||
|
||||
var canManageCollections = await CanManageCollectionsAsync(targetCollections, org, requireManagePermission: true);
|
||||
var canManageCollections = await HasCollectionAccessAsync(targetCollections, org, requireManagePermission: true);
|
||||
if (canManageCollections)
|
||||
{
|
||||
context.Succeed(requirement);
|
||||
}
|
||||
else
|
||||
{
|
||||
context.Fail();
|
||||
}
|
||||
}
|
||||
|
||||
private async Task<bool> CanManageCollectionsAsync(
|
||||
private async Task<bool> HasCollectionAccessAsync(
|
||||
ICollection<Collection> targetCollections,
|
||||
CurrentContextOrganization org,
|
||||
bool requireManagePermission)
|
||||
|
@ -35,20 +35,12 @@ public class CollectionAuthorizationHandler : AuthorizationHandler<CollectionOpe
|
||||
throw new FeatureUnavailableException("Flexible collections is OFF when it should be ON.");
|
||||
}
|
||||
|
||||
if (!_currentContext.UserId.HasValue)
|
||||
if (!_currentContext.UserId.HasValue || requirement.OrganizationId == default)
|
||||
{
|
||||
context.Fail();
|
||||
return;
|
||||
}
|
||||
|
||||
var targetOrganizationId = requirement.OrganizationId;
|
||||
if (targetOrganizationId == default)
|
||||
{
|
||||
context.Fail();
|
||||
return;
|
||||
}
|
||||
|
||||
var org = _currentContext.GetOrganization(targetOrganizationId);
|
||||
var org = _currentContext.GetOrganization(requirement.OrganizationId);
|
||||
|
||||
switch (requirement)
|
||||
{
|
||||
@ -79,18 +71,12 @@ public class CollectionAuthorizationHandler : AuthorizationHandler<CollectionOpe
|
||||
return;
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
|
||||
// Check if acting user is a provider user for the target organization
|
||||
if (await _currentContext.ProviderUserForOrgAsync(requirement.OrganizationId))
|
||||
{
|
||||
context.Succeed(requirement);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Acting user is neither a member of the target organization or a provider user, fail
|
||||
context.Fail();
|
||||
}
|
||||
|
||||
private async Task CanReadAllWithAccessAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement,
|
||||
@ -109,17 +95,12 @@ public class CollectionAuthorizationHandler : AuthorizationHandler<CollectionOpe
|
||||
return;
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
|
||||
// Check if acting user is a provider user for the target organization
|
||||
if (await _currentContext.ProviderUserForOrgAsync(requirement.OrganizationId))
|
||||
{
|
||||
context.Succeed(requirement);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Acting user is neither a member of the target organization or a provider user, fail
|
||||
context.Fail();
|
||||
}
|
||||
}
|
||||
|
@ -216,7 +216,7 @@ public class BulkCollectionAuthorizationHandlerTests
|
||||
sutProvider.GetDependency<ICollectionRepository>().GetManyByUserIdAsync(actingUserId).Returns(collectionDetails);
|
||||
|
||||
await sutProvider.Sut.HandleAsync(context);
|
||||
Assert.True(context.HasFailed);
|
||||
Assert.False(context.HasSucceeded);
|
||||
sutProvider.GetDependency<ICurrentContext>().ReceivedWithAnyArgs().GetOrganization(default);
|
||||
await sutProvider.GetDependency<ICollectionRepository>().ReceivedWithAnyArgs()
|
||||
.GetManyByUserIdAsync(default);
|
||||
|
@ -55,7 +55,7 @@ public class CollectionAuthorizationHandlerTests
|
||||
|
||||
await sutProvider.Sut.HandleAsync(context);
|
||||
|
||||
Assert.True(expectedSuccess ? context.HasSucceeded : context.HasFailed);
|
||||
Assert.Equal(expectedSuccess, context.HasSucceeded);
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
@ -95,7 +95,7 @@ public class CollectionAuthorizationHandlerTests
|
||||
sutProvider.GetDependency<ICurrentContext>().UserId.Returns((Guid?)null);
|
||||
|
||||
await sutProvider.Sut.HandleAsync(context);
|
||||
Assert.True(context.HasFailed);
|
||||
Assert.False(context.HasSucceeded);
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
@ -114,6 +114,6 @@ public class CollectionAuthorizationHandlerTests
|
||||
sutProvider.GetDependency<ICurrentContext>().GetOrganization(Arg.Any<Guid>()).Returns((CurrentContextOrganization)null);
|
||||
|
||||
await sutProvider.Sut.HandleAsync(context);
|
||||
Assert.True(context.HasFailed);
|
||||
Assert.False(context.HasSucceeded);
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user