nhyatt/bitwarden
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2025-06-30 15:42:48 -05:00
Code Activity

[AC-2538] Limit admin access - fix ManageUsers custom permission (#4032)

Browse Source 
* Fix issue where ManageUsers custom permission could not
  grant access to collections
* Split ModifyAccess operation to ModifyUserAccess and
  ModifyGroupAccess to reflect more granular operations
This commit is contained in:
Thomas Rittson
2024-05-01 10:06:24 +10:00
committed by GitHub
parent 3749fa6113
commit 5012d56e5a
7 changed files with 193 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs
View File

@ -120,7 +120,7 @@ public class OrganizationUsersControllerTests
sutProvider.GetDependency<IAuthorizationService>()
.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(),
Arg.Any<IEnumerable<Collection>>(),
Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyAccess)))
Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyUserAccess)))
.Returns(AuthorizationResult.Success());
sutProvider.GetDependency<IUserService>().GetProperUserId(Arg.Any<ClaimsPrincipal>()).Returns(userId);
@ -147,7 +147,7 @@ public class OrganizationUsersControllerTests
sutProvider.GetDependency<IAuthorizationService>()
.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(),
Arg.Any<IEnumerable<Collection>>(),
Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyAccess)))
Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyUserAccess)))
.Returns(AuthorizationResult.Failed());
sutProvider.GetDependency<IUserService>().GetProperUserId(Arg.Any<ClaimsPrincipal>()).Returns(userId);
@ -309,13 +309,13 @@ public class OrganizationUsersControllerTests
// Authorize the editedCollection
sutProvider.GetDependency<IAuthorizationService>()
.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(), Arg.Is<Collection>(c => c.Id == editedCollectionId),
Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyAccess)))
Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyUserAccess)))
.Returns(AuthorizationResult.Success());
// Do not authorize the readonly collections
sutProvider.GetDependency<IAuthorizationService>()
.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(), Arg.Is<Collection>(c => c.Id == readonlyCollectionId1 || c.Id == readonlyCollectionId2),
Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyAccess)))
Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyUserAccess)))
.Returns(AuthorizationResult.Failed());
await sutProvider.Sut.Put(organizationAbility.Id, organizationUser.Id, model);
@ -357,7 +357,7 @@ public class OrganizationUsersControllerTests
sutProvider.GetDependency<IAuthorizationService>()
.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(), Arg.Is<Collection>(c => collections.Contains(c)),
Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyAccess)))
Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyUserAccess)))
.Returns(AuthorizationResult.Failed());
var exception = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.Put(organizationAbility.Id, organizationUser.Id, model));
@ -466,7 +466,7 @@ public class OrganizationUsersControllerTests
sutProvider.GetDependency<IAuthorizationService>()
.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(), Arg.Is<Collection>(c => collections.Contains(c)),
Arg.Is<IEnumerable<IAuthorizationRequirement>>(r => r.Contains(BulkCollectionOperations.ModifyAccess)))
Arg.Is<IEnumerable<IAuthorizationRequirement>>(r => r.Contains(BulkCollectionOperations.ModifyUserAccess)))
.Returns(AuthorizationResult.Success());
}
}

17
test/Api.Test/Controllers/CollectionsControllerTests.cs
View File

@ -345,7 +345,9 @@ public class CollectionsControllerTests
sutProvider.GetDependency<IAuthorizationService>().AuthorizeAsync(
Arg.Any<ClaimsPrincipal>(), ExpectedCollectionAccess(),
Arg.Is<IEnumerable<IAuthorizationRequirement>>(
r => r.Contains(BulkCollectionOperations.ModifyAccess)
reqs => reqs.All(r =>
r == BulkCollectionOperations.ModifyUserAccess ||
r == BulkCollectionOperations.ModifyGroupAccess)
))
.Returns(AuthorizationResult.Success());
@ -359,8 +361,9 @@ public class CollectionsControllerTests
Arg.Any<ClaimsPrincipal>(),
ExpectedCollectionAccess(),
Arg.Is<IEnumerable<IAuthorizationRequirement>>(
r => r.Contains(BulkCollectionOperations.ModifyAccess))
);
reqs => reqs.All(r =>
r == BulkCollectionOperations.ModifyUserAccess ||
r == BulkCollectionOperations.ModifyGroupAccess)));
await sutProvider.GetDependency<IBulkAddCollectionAccessCommand>().Received()
.AddAccessAsync(
Arg.Is<ICollection<Collection>>(g => g.SequenceEqual(collections)),
@ -500,7 +503,9 @@ public class CollectionsControllerTests
sutProvider.GetDependency<IAuthorizationService>().AuthorizeAsync(
Arg.Any<ClaimsPrincipal>(), ExpectedCollectionAccess(),
Arg.Is<IEnumerable<IAuthorizationRequirement>>(
r => r.Contains(BulkCollectionOperations.ModifyAccess)
reqs => reqs.All(r =>
r == BulkCollectionOperations.ModifyUserAccess ||
r == BulkCollectionOperations.ModifyGroupAccess)
))
.Returns(AuthorizationResult.Failed());
@ -511,7 +516,9 @@ public class CollectionsControllerTests
Arg.Any<ClaimsPrincipal>(),
ExpectedCollectionAccess(),
Arg.Is<IEnumerable<IAuthorizationRequirement>>(
r => r.Contains(BulkCollectionOperations.ModifyAccess))
reqs => reqs.All(r =>
r == BulkCollectionOperations.ModifyUserAccess ||
r == BulkCollectionOperations.ModifyGroupAccess))
);
await sutProvider.GetDependency<IBulkAddCollectionAccessCommand>().DidNotReceiveWithAnyArgs()
.AddAccessAsync(default, default, default);

107
test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs
View File

@ -548,7 +548,9 @@ public class BulkCollectionAuthorizationHandlerTests
var operationsToTest = new[]
{
BulkCollectionOperations.Update, BulkCollectionOperations.ModifyAccess
BulkCollectionOperations.Update,
BulkCollectionOperations.ModifyUserAccess,
BulkCollectionOperations.ModifyGroupAccess,
};
foreach (var op in operationsToTest)
@ -586,7 +588,9 @@ public class BulkCollectionAuthorizationHandlerTests
var operationsToTest = new[]
{
BulkCollectionOperations.Update, BulkCollectionOperations.ModifyAccess
BulkCollectionOperations.Update,
BulkCollectionOperations.ModifyUserAccess,
BulkCollectionOperations.ModifyGroupAccess,
};
foreach (var op in operationsToTest)
@ -627,7 +631,9 @@ public class BulkCollectionAuthorizationHandlerTests
var operationsToTest = new[]
{
BulkCollectionOperations.Update, BulkCollectionOperations.ModifyAccess
BulkCollectionOperations.Update,
BulkCollectionOperations.ModifyUserAccess,
BulkCollectionOperations.ModifyGroupAccess,
};
foreach (var op in operationsToTest)
@ -668,7 +674,9 @@ public class BulkCollectionAuthorizationHandlerTests
var operationsToTest = new[]
{
BulkCollectionOperations.Update, BulkCollectionOperations.ModifyAccess
BulkCollectionOperations.Update,
BulkCollectionOperations.ModifyUserAccess,
BulkCollectionOperations.ModifyGroupAccess,
};
foreach (var op in operationsToTest)
@ -708,7 +716,9 @@ public class BulkCollectionAuthorizationHandlerTests
var operationsToTest = new[]
{
BulkCollectionOperations.Update, BulkCollectionOperations.ModifyAccess
BulkCollectionOperations.Update,
BulkCollectionOperations.ModifyUserAccess,
BulkCollectionOperations.ModifyGroupAccess,
};
foreach (var op in operationsToTest)
@ -760,7 +770,9 @@ public class BulkCollectionAuthorizationHandlerTests
var operationsToTest = new[]
{
BulkCollectionOperations.Update, BulkCollectionOperations.ModifyAccess
BulkCollectionOperations.Update,
BulkCollectionOperations.ModifyUserAccess,
BulkCollectionOperations.ModifyGroupAccess,
};
foreach (var op in operationsToTest)
@ -790,7 +802,9 @@ public class BulkCollectionAuthorizationHandlerTests
{
var operationsToTest = new[]
{
BulkCollectionOperations.Update, BulkCollectionOperations.ModifyAccess
BulkCollectionOperations.Update,
BulkCollectionOperations.ModifyUserAccess,
BulkCollectionOperations.ModifyGroupAccess,
};
foreach (var op in operationsToTest)
@ -813,6 +827,82 @@ public class BulkCollectionAuthorizationHandlerTests
}
}
[Theory, BitAutoData, CollectionCustomization]
public async Task CanUpdateUsers_WithManageUsersCustomPermission_Success(
SutProvider<BulkCollectionAuthorizationHandler> sutProvider,
ICollection<Collection> collections,
CurrentContextOrganization organization)
{
var actingUserId = Guid.NewGuid();
organization.Type = OrganizationUserType.Custom;
organization.Permissions = new Permissions
{
ManageUsers = true
};
var operationsToTest = new[]
{
BulkCollectionOperations.ModifyUserAccess,
};
foreach (var op in operationsToTest)
{
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
var context = new AuthorizationHandlerContext(
new[] { op },
new ClaimsPrincipal(),
collections);
await sutProvider.Sut.HandleAsync(context);
Assert.True(context.HasSucceeded);
// Recreate the SUT to reset the mocks/dependencies between tests
sutProvider.Recreate();
}
}
[Theory, BitAutoData, CollectionCustomization]
public async Task CanUpdateGroups_WithManageGroupsCustomPermission_Success(
SutProvider<BulkCollectionAuthorizationHandler> sutProvider,
ICollection<Collection> collections,
CurrentContextOrganization organization)
{
var actingUserId = Guid.NewGuid();
organization.Type = OrganizationUserType.Custom;
organization.Permissions = new Permissions
{
ManageGroups = true
};
var operationsToTest = new[]
{
BulkCollectionOperations.ModifyGroupAccess,
};
foreach (var op in operationsToTest)
{
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
var context = new AuthorizationHandlerContext(
new[] { op },
new ClaimsPrincipal(),
collections);
await sutProvider.Sut.HandleAsync(context);
Assert.True(context.HasSucceeded);
// Recreate the SUT to reset the mocks/dependencies between tests
sutProvider.Recreate();
}
}
[Theory, CollectionCustomization]
[BitAutoData(OrganizationUserType.Admin)]
[BitAutoData(OrganizationUserType.Owner)]
@ -1023,7 +1113,8 @@ public class BulkCollectionAuthorizationHandlerTests
BulkCollectionOperations.Read,
BulkCollectionOperations.ReadAccess,
BulkCollectionOperations.Update,
BulkCollectionOperations.ModifyAccess,
BulkCollectionOperations.ModifyUserAccess,
BulkCollectionOperations.ModifyGroupAccess,
BulkCollectionOperations.Delete,
};