mirror of
https://github.com/bitwarden/server.git
synced 2025-06-30 15:42:48 -05:00
[AC-2172] Member modal - limit admin access (#3934)
* update OrganizationUsersController PUT and POST * enforces new collection access checks when updating members * refactor BulkCollectionAuthorizationHandler to avoid repeated db calls
This commit is contained in:
Thomas Rittson
@ -1,6 +1,8 @@
|
||||
using System.Security.Claims;
|
||||
using Bit.Api.AdminConsole.Controllers;
|
||||
using Bit.Api.AdminConsole.Models.Request.Organizations;
|
||||
using Bit.Api.Models.Request;
|
||||
using Bit.Api.Vault.AuthorizationHandlers.Collections;
|
||||
using Bit.Core;
|
||||
using Bit.Core.AdminConsole.Entities;
|
||||
using Bit.Core.AdminConsole.Enums;
|
||||
@ -10,6 +12,8 @@ using Bit.Core.AdminConsole.Repositories;
|
||||
using Bit.Core.Context;
|
||||
using Bit.Core.Entities;
|
||||
using Bit.Core.Enums;
|
||||
using Bit.Core.Exceptions;
|
||||
using Bit.Core.Models.Business;
|
||||
using Bit.Core.Models.Data;
|
||||
using Bit.Core.Models.Data.Organizations;
|
||||
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
|
||||
@ -104,23 +108,69 @@ public class OrganizationUsersControllerTests
|
||||
.UpdateUserResetPasswordEnrollmentAsync(orgId, user.Id, model.ResetPasswordKey, user.Id);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[BitAutoData]
|
||||
public async Task Invite_Success(OrganizationAbility organizationAbility, OrganizationUserInviteRequestModel model,
|
||||
Guid userId, SutProvider<OrganizationUsersController> sutProvider)
|
||||
{
|
||||
organizationAbility.FlexibleCollections = true;
|
||||
sutProvider.GetDependency<ICurrentContext>().ManageUsers(organizationAbility.Id).Returns(true);
|
||||
sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organizationAbility.Id)
|
||||
.Returns(organizationAbility);
|
||||
sutProvider.GetDependency<IAuthorizationService>()
|
||||
.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(),
|
||||
Arg.Any<IEnumerable<Collection>>(),
|
||||
Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyAccess)))
|
||||
.Returns(AuthorizationResult.Success());
|
||||
sutProvider.GetDependency<IUserService>().GetProperUserId(Arg.Any<ClaimsPrincipal>()).Returns(userId);
|
||||
|
||||
await sutProvider.Sut.Invite(organizationAbility.Id, model);
|
||||
|
||||
await sutProvider.GetDependency<IOrganizationService>().Received(1).InviteUsersAsync(organizationAbility.Id,
|
||||
userId, Arg.Is<IEnumerable<(OrganizationUserInvite, string)>>(invites =>
|
||||
invites.Count() == 1 &&
|
||||
invites.First().Item1.Emails.SequenceEqual(model.Emails) &&
|
||||
invites.First().Item1.Type == model.Type &&
|
||||
invites.First().Item1.AccessSecretsManager == model.AccessSecretsManager));
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[BitAutoData]
|
||||
public async Task Invite_NotAuthorizedToGiveAccessToCollections_Throws(OrganizationAbility organizationAbility, OrganizationUserInviteRequestModel model,
|
||||
Guid userId, SutProvider<OrganizationUsersController> sutProvider)
|
||||
{
|
||||
organizationAbility.FlexibleCollections = true;
|
||||
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
|
||||
sutProvider.GetDependency<ICurrentContext>().ManageUsers(organizationAbility.Id).Returns(true);
|
||||
sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organizationAbility.Id)
|
||||
.Returns(organizationAbility);
|
||||
sutProvider.GetDependency<IAuthorizationService>()
|
||||
.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(),
|
||||
Arg.Any<IEnumerable<Collection>>(),
|
||||
Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyAccess)))
|
||||
.Returns(AuthorizationResult.Failed());
|
||||
sutProvider.GetDependency<IUserService>().GetProperUserId(Arg.Any<ClaimsPrincipal>()).Returns(userId);
|
||||
|
||||
var exception = await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.Invite(organizationAbility.Id, model));
|
||||
Assert.Contains("You are not authorized", exception.Message);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[BitAutoData]
|
||||
public async Task Put_Success(OrganizationUserUpdateRequestModel model,
|
||||
OrganizationUser organizationUser, OrganizationAbility organizationAbility,
|
||||
SutProvider<OrganizationUsersController> sutProvider, Guid savingUserId)
|
||||
{
|
||||
var orgId = organizationAbility.Id = organizationUser.OrganizationId;
|
||||
sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(true);
|
||||
sutProvider.GetDependency<IOrganizationUserRepository>().GetByIdAsync(organizationUser.Id).Returns(organizationUser);
|
||||
sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(orgId)
|
||||
.Returns(organizationAbility);
|
||||
sutProvider.GetDependency<IUserService>().GetProperUserId(Arg.Any<ClaimsPrincipal>()).Returns(savingUserId);
|
||||
organizationAbility.FlexibleCollections = false;
|
||||
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(false);
|
||||
|
||||
Put_Setup(sutProvider, organizationAbility, organizationUser, savingUserId, model, false);
|
||||
|
||||
// Save these for later - organizationUser object will be mutated
|
||||
var orgUserId = organizationUser.Id;
|
||||
var orgUserEmail = organizationUser.Email;
|
||||
|
||||
await sutProvider.Sut.Put(orgId, organizationUser.Id, model);
|
||||
await sutProvider.Sut.Put(organizationAbility.Id, organizationUser.Id, model);
|
||||
|
||||
await sutProvider.GetDependency<IUpdateOrganizationUserCommand>().Received(1).UpdateUserAsync(Arg.Is<OrganizationUser>(ou =>
|
||||
ou.Type == model.Type &&
|
||||
@ -142,21 +192,16 @@ public class OrganizationUsersControllerTests
|
||||
{
|
||||
// Updating self
|
||||
organizationUser.UserId = savingUserId;
|
||||
organizationAbility.FlexibleCollections = true;
|
||||
organizationAbility.AllowAdminAccessToAllCollectionItems = false;
|
||||
organizationAbility.FlexibleCollections = true;
|
||||
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
|
||||
|
||||
var orgId = organizationAbility.Id = organizationUser.OrganizationId;
|
||||
sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(true);
|
||||
sutProvider.GetDependency<IOrganizationUserRepository>().GetByIdAsync(organizationUser.Id).Returns(organizationUser);
|
||||
sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(orgId)
|
||||
.Returns(organizationAbility);
|
||||
sutProvider.GetDependency<IUserService>().GetProperUserId(Arg.Any<ClaimsPrincipal>()).Returns(savingUserId);
|
||||
Put_Setup(sutProvider, organizationAbility, organizationUser, savingUserId, model, true);
|
||||
|
||||
var orgUserId = organizationUser.Id;
|
||||
var orgUserEmail = organizationUser.Email;
|
||||
|
||||
await sutProvider.Sut.Put(orgId, organizationUser.Id, model);
|
||||
await sutProvider.Sut.Put(organizationAbility.Id, organizationUser.Id, model);
|
||||
|
||||
await sutProvider.GetDependency<IUpdateOrganizationUserCommand>().Received(1).UpdateUserAsync(Arg.Is<OrganizationUser>(ou =>
|
||||
ou.Type == model.Type &&
|
||||
@ -182,17 +227,12 @@ public class OrganizationUsersControllerTests
|
||||
organizationAbility.AllowAdminAccessToAllCollectionItems = true;
|
||||
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
|
||||
|
||||
var orgId = organizationAbility.Id = organizationUser.OrganizationId;
|
||||
sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(true);
|
||||
sutProvider.GetDependency<IOrganizationUserRepository>().GetByIdAsync(organizationUser.Id).Returns(organizationUser);
|
||||
sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(orgId)
|
||||
.Returns(organizationAbility);
|
||||
sutProvider.GetDependency<IUserService>().GetProperUserId(Arg.Any<ClaimsPrincipal>()).Returns(savingUserId);
|
||||
Put_Setup(sutProvider, organizationAbility, organizationUser, savingUserId, model, true);
|
||||
|
||||
var orgUserId = organizationUser.Id;
|
||||
var orgUserEmail = organizationUser.Email;
|
||||
|
||||
await sutProvider.Sut.Put(orgId, organizationUser.Id, model);
|
||||
await sutProvider.Sut.Put(organizationAbility.Id, organizationUser.Id, model);
|
||||
|
||||
await sutProvider.GetDependency<IUpdateOrganizationUserCommand>().Received(1).UpdateUserAsync(Arg.Is<OrganizationUser>(ou =>
|
||||
ou.Type == model.Type &&
|
||||
@ -206,6 +246,124 @@ public class OrganizationUsersControllerTests
|
||||
model.Groups);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[BitAutoData]
|
||||
public async Task Put_UpdateCollections_OnlyUpdatesCollectionsTheSavingUserCanUpdate(OrganizationUserUpdateRequestModel model,
|
||||
OrganizationUser organizationUser, OrganizationAbility organizationAbility,
|
||||
SutProvider<OrganizationUsersController> sutProvider, Guid savingUserId)
|
||||
{
|
||||
organizationAbility.FlexibleCollections = true;
|
||||
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
|
||||
Put_Setup(sutProvider, organizationAbility, organizationUser, savingUserId, model, false);
|
||||
|
||||
var editedCollectionId = CoreHelpers.GenerateComb();
|
||||
var readonlyCollectionId1 = CoreHelpers.GenerateComb();
|
||||
var readonlyCollectionId2 = CoreHelpers.GenerateComb();
|
||||
|
||||
var currentCollectionAccess = new List<CollectionAccessSelection>
|
||||
{
|
||||
new()
|
||||
{
|
||||
Id = editedCollectionId,
|
||||
HidePasswords = true,
|
||||
Manage = false,
|
||||
ReadOnly = true
|
||||
},
|
||||
new()
|
||||
{
|
||||
Id = readonlyCollectionId1,
|
||||
HidePasswords = false,
|
||||
Manage = true,
|
||||
ReadOnly = false
|
||||
},
|
||||
new()
|
||||
{
|
||||
Id = readonlyCollectionId2,
|
||||
HidePasswords = false,
|
||||
Manage = false,
|
||||
ReadOnly = false
|
||||
},
|
||||
};
|
||||
|
||||
// User is upgrading editedCollectionId to manage
|
||||
model.Collections = new List<SelectionReadOnlyRequestModel>
|
||||
{
|
||||
new() { Id = editedCollectionId, HidePasswords = false, Manage = true, ReadOnly = false }
|
||||
};
|
||||
|
||||
// Save these for later - organizationUser object will be mutated
|
||||
var orgUserId = organizationUser.Id;
|
||||
var orgUserEmail = organizationUser.Email;
|
||||
|
||||
sutProvider.GetDependency<IOrganizationUserRepository>()
|
||||
.GetByIdWithCollectionsAsync(organizationUser.Id)
|
||||
.Returns(new Tuple<OrganizationUser, ICollection<CollectionAccessSelection>>(organizationUser,
|
||||
currentCollectionAccess));
|
||||
|
||||
var currentCollections = currentCollectionAccess
|
||||
.Select(cas => new Collection { Id = cas.Id }).ToList();
|
||||
sutProvider.GetDependency<ICollectionRepository>()
|
||||
.GetManyByManyIdsAsync(Arg.Any<IEnumerable<Guid>>())
|
||||
.Returns(currentCollections);
|
||||
|
||||
// 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)))
|
||||
.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)))
|
||||
.Returns(AuthorizationResult.Failed());
|
||||
|
||||
await sutProvider.Sut.Put(organizationAbility.Id, organizationUser.Id, model);
|
||||
|
||||
// Expect all collection access (modified and unmodified) to be saved
|
||||
await sutProvider.GetDependency<IUpdateOrganizationUserCommand>().Received(1).UpdateUserAsync(Arg.Is<OrganizationUser>(ou =>
|
||||
ou.Type == model.Type &&
|
||||
ou.Permissions == CoreHelpers.ClassToJsonData(model.Permissions) &&
|
||||
ou.AccessSecretsManager == model.AccessSecretsManager &&
|
||||
ou.Id == orgUserId &&
|
||||
ou.Email == orgUserEmail),
|
||||
savingUserId,
|
||||
Arg.Is<List<CollectionAccessSelection>>(cas =>
|
||||
cas.Select(c => c.Id).SequenceEqual(currentCollectionAccess.Select(c => c.Id)) &&
|
||||
cas.First(c => c.Id == editedCollectionId).Manage == true &&
|
||||
cas.First(c => c.Id == editedCollectionId).ReadOnly == false &&
|
||||
cas.First(c => c.Id == editedCollectionId).HidePasswords == false),
|
||||
model.Groups);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[BitAutoData]
|
||||
public async Task Put_UpdateCollections_ThrowsIfSavingUserCannotUpdateCollections(OrganizationUserUpdateRequestModel model,
|
||||
OrganizationUser organizationUser, OrganizationAbility organizationAbility,
|
||||
SutProvider<OrganizationUsersController> sutProvider, Guid savingUserId)
|
||||
{
|
||||
organizationAbility.FlexibleCollections = true;
|
||||
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
|
||||
Put_Setup(sutProvider, organizationAbility, organizationUser, savingUserId, model, false);
|
||||
|
||||
sutProvider.GetDependency<IOrganizationUserRepository>()
|
||||
.GetByIdWithCollectionsAsync(organizationUser.Id)
|
||||
.Returns(new Tuple<OrganizationUser, ICollection<CollectionAccessSelection>>(organizationUser,
|
||||
model.Collections.Select(cas => cas.ToSelectionReadOnly()).ToList()));
|
||||
var collections = model.Collections.Select(cas => new Collection { Id = cas.Id }).ToList();
|
||||
sutProvider.GetDependency<ICollectionRepository>()
|
||||
.GetManyByManyIdsAsync(Arg.Is<IEnumerable<Guid>>(guids => guids.SequenceEqual(collections.Select(c => c.Id))))
|
||||
.Returns(collections);
|
||||
|
||||
sutProvider.GetDependency<IAuthorizationService>()
|
||||
.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(), Arg.Is<Collection>(c => collections.Contains(c)),
|
||||
Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyAccess)))
|
||||
.Returns(AuthorizationResult.Failed());
|
||||
|
||||
var exception = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.Put(organizationAbility.Id, organizationUser.Id, model));
|
||||
Assert.Contains("You must have Can Manage permission", exception.Message);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[BitAutoData]
|
||||
public async Task Get_WithFlexibleCollections_ReturnsUsers(
|
||||
@ -283,6 +441,36 @@ public class OrganizationUsersControllerTests
|
||||
Assert.False(customUserResponse.Permissions.DeleteAssignedCollections);
|
||||
}
|
||||
|
||||
private void Put_Setup(SutProvider<OrganizationUsersController> sutProvider, OrganizationAbility organizationAbility,
|
||||
OrganizationUser organizationUser, Guid savingUserId, OrganizationUserUpdateRequestModel model, bool authorizeAll)
|
||||
{
|
||||
var orgId = organizationAbility.Id = organizationUser.OrganizationId;
|
||||
|
||||
sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(true);
|
||||
sutProvider.GetDependency<IOrganizationUserRepository>().GetByIdAsync(organizationUser.Id).Returns(organizationUser);
|
||||
sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(orgId)
|
||||
.Returns(organizationAbility);
|
||||
sutProvider.GetDependency<IUserService>().GetProperUserId(Arg.Any<ClaimsPrincipal>()).Returns(savingUserId);
|
||||
|
||||
if (authorizeAll)
|
||||
{
|
||||
// Simple case: saving user can edit all collections, all collection access is replaced
|
||||
sutProvider.GetDependency<IOrganizationUserRepository>()
|
||||
.GetByIdWithCollectionsAsync(organizationUser.Id)
|
||||
.Returns(new Tuple<OrganizationUser, ICollection<CollectionAccessSelection>>(organizationUser,
|
||||
model.Collections.Select(cas => cas.ToSelectionReadOnly()).ToList()));
|
||||
var collections = model.Collections.Select(cas => new Collection { Id = cas.Id }).ToList();
|
||||
sutProvider.GetDependency<ICollectionRepository>()
|
||||
.GetManyByManyIdsAsync(Arg.Is<IEnumerable<Guid>>(guids => guids.SequenceEqual(collections.Select(c => c.Id))))
|
||||
.Returns(collections);
|
||||
|
||||
sutProvider.GetDependency<IAuthorizationService>()
|
||||
.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(), Arg.Is<Collection>(c => collections.Contains(c)),
|
||||
Arg.Is<IEnumerable<IAuthorizationRequirement>>(r => r.Contains(BulkCollectionOperations.ModifyAccess)))
|
||||
.Returns(AuthorizationResult.Success());
|
||||
}
|
||||
}
|
||||
|
||||
private void Get_Setup(OrganizationAbility organizationAbility,
|
||||
ICollection<OrganizationUserUserDetails> organizationUsers,
|
||||
SutProvider<OrganizationUsersController> sutProvider)
|
||||
|
||||
Reference in New Issue
Block a user