Upstream/bitwarden-server
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2024-11-23 12:25:16 +01:00
Code Issues Projects Releases Wiki Activity

[PM-10323] Remove user verification from organization user deletion methods (#4965)

Browse Source
This commit is contained in:
Rui Tomé 2024-11-04 14:48:13 +00:00 committed by GitHub
parent 96862b974f
commit 60672bbe48
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 12 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/Api/AdminConsole/Controllers/OrganizationUsersController.cs
View File

@ -1,6 +1,5 @@
using Bit.Api.AdminConsole.Models.Request.Organizations;
using Bit.Api.AdminConsole.Models.Response.Organizations;
using Bit.Api.Auth.Models.Request.Accounts;
using Bit.Api.Models.Request.Organizations;
using Bit.Api.Models.Response;
using Bit.Api.Vault.AuthorizationHandlers.Collections;
@ -545,7 +544,7 @@ public class OrganizationUsersController : Controller
[RequireFeature(FeatureFlagKeys.AccountDeprovisioning)]
[HttpDelete("{id}/delete-account")]
[HttpPost("{id}/delete-account")]
public async Task DeleteAccount(Guid orgId, Guid id, [FromBody] SecretVerificationRequestModel model)
public async Task DeleteAccount(Guid orgId, Guid id)
{
if (!await _currentContext.ManageUsers(orgId))
{
@ -558,19 +557,13 @@ public class OrganizationUsersController : Controller
throw new UnauthorizedAccessException();
}
if (!await _userService.VerifySecretAsync(currentUser, model.Secret))
{
await Task.Delay(2000);
throw new BadRequestException(string.Empty, "User verification failed.");
}
await _deleteManagedOrganizationUserAccountCommand.DeleteUserAsync(orgId, id, currentUser.Id);
}
[RequireFeature(FeatureFlagKeys.AccountDeprovisioning)]
[HttpDelete("delete-account")]
[HttpPost("delete-account")]
public async Task<ListResponseModel<OrganizationUserBulkResponseModel>> BulkDeleteAccount(Guid orgId, [FromBody] SecureOrganizationUserBulkRequestModel model)
public async Task<ListResponseModel<OrganizationUserBulkResponseModel>> BulkDeleteAccount(Guid orgId, [FromBody] OrganizationUserBulkRequestModel model)
{
if (!await _currentContext.ManageUsers(orgId))
{
@ -583,12 +576,6 @@ public class OrganizationUsersController : Controller
throw new UnauthorizedAccessException();
}
if (!await _userService.VerifySecretAsync(currentUser, model.Secret))
{
await Task.Delay(2000);
throw new BadRequestException(string.Empty, "User verification failed.");
}
var results = await _deleteManagedOrganizationUserAccountCommand.DeleteManyUsersAsync(orgId, model.Ids, currentUser.Id);
return new ListResponseModel<OrganizationUserBulkResponseModel>(results.Select(r =>

10
src/Api/AdminConsole/Models/Request/Organizations/SecureOrganizationUserBulkRequestModel.cs
View File

@ -1,10 +0,0 @@
using System.ComponentModel.DataAnnotations;
using Bit.Api.Auth.Models.Request.Accounts;
namespace Bit.Api.AdminConsole.Models.Request.Organizations;
public class SecureOrganizationUserBulkRequestModel : SecretVerificationRequestModel
{
[Required]
public IEnumerable<Guid> Ids { get; set; }
}

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

@ -1,7 +1,6 @@
using System.Security.Claims;
using Bit.Api.AdminConsole.Controllers;
using Bit.Api.AdminConsole.Models.Request.Organizations;
using Bit.Api.Auth.Models.Request.Accounts;
using Bit.Api.Vault.AuthorizationHandlers.Collections;
using Bit.Core;
using Bit.Core.AdminConsole.Entities;
@ -273,17 +272,12 @@ public class OrganizationUsersControllerTests
[Theory]
[BitAutoData]
public async Task DeleteAccount_WhenUserCanManageUsers_Success(
Guid orgId,
Guid id,
SecretVerificationRequestModel model,
User currentUser,
SutProvider<OrganizationUsersController> sutProvider)
Guid orgId, Guid id, User currentUser, SutProvider<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(true);
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(currentUser);
sutProvider.GetDependency<IUserService>().VerifySecretAsync(currentUser, model.Secret).Returns(true);
await sutProvider.Sut.DeleteAccount(orgId, id, model);
await sutProvider.Sut.DeleteAccount(orgId, id);
await sutProvider.GetDependency<IDeleteManagedOrganizationUserAccountCommand>()
.Received(1)
@ -293,60 +287,34 @@ public class OrganizationUsersControllerTests
[Theory]
[BitAutoData]
public async Task DeleteAccount_WhenUserCannotManageUsers_ThrowsNotFoundException(
Guid orgId,
Guid id,
SecretVerificationRequestModel model,
SutProvider<OrganizationUsersController> sutProvider)
Guid orgId, Guid id, SutProvider<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(false);
await Assert.ThrowsAsync<NotFoundException>(() =>
sutProvider.Sut.DeleteAccount(orgId, id, model));
sutProvider.Sut.DeleteAccount(orgId, id));
}
[Theory]
[BitAutoData]
public async Task DeleteAccount_WhenCurrentUserNotFound_ThrowsUnauthorizedAccessException(
Guid orgId,
Guid id,
SecretVerificationRequestModel model,
SutProvider<OrganizationUsersController> sutProvider)
Guid orgId, Guid id, SutProvider<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(true);
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsForAnyArgs((User)null);
await Assert.ThrowsAsync<UnauthorizedAccessException>(() =>
sutProvider.Sut.DeleteAccount(orgId, id, model));
}
[Theory]
[BitAutoData]
public async Task DeleteAccount_WhenSecretVerificationFails_ThrowsBadRequestException(
Guid orgId,
Guid id,
SecretVerificationRequestModel model,
User currentUser,
SutProvider<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(true);
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(currentUser);
sutProvider.GetDependency<IUserService>().VerifySecretAsync(currentUser, model.Secret).Returns(false);
await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.DeleteAccount(orgId, id, model));
sutProvider.Sut.DeleteAccount(orgId, id));
}
[Theory]
[BitAutoData]
public async Task BulkDeleteAccount_WhenUserCanManageUsers_Success(
Guid orgId,
SecureOrganizationUserBulkRequestModel model,
User currentUser,
List<(Guid, string)> deleteResults,
SutProvider<OrganizationUsersController> sutProvider)
Guid orgId, OrganizationUserBulkRequestModel model, User currentUser,
List<(Guid, string)> deleteResults, SutProvider<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(true);
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(currentUser);
sutProvider.GetDependency<IUserService>().VerifySecretAsync(currentUser, model.Secret).Returns(true);
sutProvider.GetDependency<IDeleteManagedOrganizationUserAccountCommand>()
.DeleteManyUsersAsync(orgId, model.Ids, currentUser.Id)
.Returns(deleteResults);
@ -363,9 +331,7 @@ public class OrganizationUsersControllerTests
[Theory]
[BitAutoData]
public async Task BulkDeleteAccount_WhenUserCannotManageUsers_ThrowsNotFoundException(
Guid orgId,
SecureOrganizationUserBulkRequestModel model,
SutProvider<OrganizationUsersController> sutProvider)
Guid orgId, OrganizationUserBulkRequestModel model, SutProvider<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(false);
@ -376,9 +342,7 @@ public class OrganizationUsersControllerTests
[Theory]
[BitAutoData]
public async Task BulkDeleteAccount_WhenCurrentUserNotFound_ThrowsUnauthorizedAccessException(
Guid orgId,
SecureOrganizationUserBulkRequestModel model,
SutProvider<OrganizationUsersController> sutProvider)
Guid orgId, OrganizationUserBulkRequestModel model, SutProvider<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(true);
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsForAnyArgs((User)null);
@ -387,21 +351,6 @@ public class OrganizationUsersControllerTests
sutProvider.Sut.BulkDeleteAccount(orgId, model));
}
[Theory]
[BitAutoData]
public async Task BulkDeleteAccount_WhenSecretVerificationFails_ThrowsBadRequestException(
Guid orgId,
SecureOrganizationUserBulkRequestModel model,
User currentUser,
SutProvider<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(true);
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(currentUser);
sutProvider.GetDependency<IUserService>().VerifySecretAsync(currentUser, model.Secret).Returns(false);
await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.BulkDeleteAccount(orgId, model));
}
private void GetMany_Setup(OrganizationAbility organizationAbility,
ICollection<OrganizationUserUserDetails> organizationUsers,
SutProvider<OrganizationUsersController> sutProvider)