Browse Source

[PM-10338] Update the Organization 'Leave' endpoint to log EventType.OrganizationUser_Left (#4908)

* Implement UserLeaveAsync in IRemoveOrganizationUserCommand and refactor OrganizationsController to use it

* Edit summary message for IRemoveOrganizationUserCommand.UserLeaveAsync

* Refactor RemoveOrganizationUserCommand.RemoveUsersAsync to log in bulk

---------

Co-authored-by: Matt Bishop <mbishop@bitwarden.com>
pull/5134/head
Rui Tomé 1 year ago committed by GitHub
parent
commit
127f1fd34d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 2
      src/Api/AdminConsole/Controllers/OrganizationsController.cs
  2. 7
      src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRemoveOrganizationUserCommand.cs
  3. 12
      src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs
  4. 4
      test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs
  5. 136
      test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommandTests.cs

2
src/Api/AdminConsole/Controllers/OrganizationsController.cs

@ -259,7 +259,7 @@ public class OrganizationsController : Controller @@ -259,7 +259,7 @@ public class OrganizationsController : Controller
throw new BadRequestException("Managed user account cannot leave managing organization. Contact your organization administrator for additional details.");
}
await _removeOrganizationUserCommand.RemoveUserAsync(id, user.Id);
await _removeOrganizationUserCommand.UserLeaveAsync(id, user.Id);
}
[HttpDelete("{id}")]

7
src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRemoveOrganizationUserCommand.cs

@ -50,4 +50,11 @@ public interface IRemoveOrganizationUserCommand @@ -50,4 +50,11 @@ public interface IRemoveOrganizationUserCommand
/// </returns>
Task<IEnumerable<(Guid OrganizationUserId, string ErrorMessage)>> RemoveUsersAsync(
Guid organizationId, IEnumerable<Guid> organizationUserIds, EventSystemUser eventSystemUser);
/// <summary>
/// Removes a user from an organization when they have left voluntarily. This should only be called by the same user who is being removed.
/// </summary>
/// <param name="organizationId">Organization to leave.</param>
/// <param name="userId">User to leave.</param>
Task UserLeaveAsync(Guid organizationId, Guid userId);
}

12
src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs

@ -114,6 +114,16 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand @@ -114,6 +114,16 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand
return result.Select(r => (r.OrganizationUser.Id, r.ErrorMessage));
}
public async Task UserLeaveAsync(Guid organizationId, Guid userId)
{
var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(organizationId, userId);
ValidateRemoveUser(organizationId, organizationUser);
await RepositoryRemoveUserAsync(organizationUser, deletingUserId: null, eventSystemUser: null);
await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Left);
}
private void ValidateRemoveUser(Guid organizationId, OrganizationUser orgUser)
{
if (orgUser == null || orgUser.OrganizationId != organizationId)
@ -234,7 +244,7 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand @@ -234,7 +244,7 @@ public class RemoveOrganizationUserCommand : IRemoveOrganizationUserCommand
await _organizationUserRepository.DeleteManyAsync(organizationUsersToRemove.Select(ou => ou.Id));
foreach (var orgUser in organizationUsersToRemove.Where(ou => ou.UserId.HasValue))
{
await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId.Value);
await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId!.Value);
}
}

4
test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs

@ -130,7 +130,7 @@ public class OrganizationsControllerTests : IDisposable @@ -130,7 +130,7 @@ public class OrganizationsControllerTests : IDisposable
Assert.Contains("Your organization's Single Sign-On settings prevent you from leaving.",
exception.Message);
await _removeOrganizationUserCommand.DidNotReceiveWithAnyArgs().RemoveUserAsync(default, default);
await _removeOrganizationUserCommand.DidNotReceiveWithAnyArgs().UserLeaveAsync(default, default);
}
[Theory, AutoData]
@ -193,7 +193,7 @@ public class OrganizationsControllerTests : IDisposable @@ -193,7 +193,7 @@ public class OrganizationsControllerTests : IDisposable
await _sut.Leave(orgId);
await _removeOrganizationUserCommand.Received(1).RemoveUserAsync(orgId, user.Id);
await _removeOrganizationUserCommand.Received(1).UserLeaveAsync(orgId, user.Id);
}
[Theory, AutoData]

136
test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommandTests.cs

@ -202,6 +202,14 @@ public class RemoveOrganizationUserCommandTests @@ -202,6 +202,14 @@ public class RemoveOrganizationUserCommandTests
.HasConfirmedOwnersExceptAsync(
organizationUser.OrganizationId,
Arg.Is<IEnumerable<Guid>>(i => i.Contains(organizationUser.Id)), true);
await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.DeleteAsync(default);
await sutProvider.GetDependency<IEventService>()
.DidNotReceiveWithAnyArgs()
.LogOrganizationUserEventAsync((OrganizationUser)default, default);
}
[Theory, BitAutoData]
@ -346,9 +354,7 @@ public class RemoveOrganizationUserCommandTests @@ -346,9 +354,7 @@ public class RemoveOrganizationUserCommandTests
[OrganizationUser(type: OrganizationUserType.User)] OrganizationUser organizationUser,
SutProvider<RemoveOrganizationUserCommand> sutProvider)
{
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
organizationUserRepository
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value)
.Returns(organizationUser);
@ -361,7 +367,13 @@ public class RemoveOrganizationUserCommandTests @@ -361,7 +367,13 @@ public class RemoveOrganizationUserCommandTests
await sutProvider.Sut.RemoveUserAsync(organizationUser.OrganizationId, organizationUser.UserId.Value);
await sutProvider.GetDependency<IEventService>().Received(1).LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed);
await sutProvider.GetDependency<IOrganizationUserRepository>()
.Received(1)
.DeleteAsync(organizationUser);
await sutProvider.GetDependency<IEventService>()
.Received(1)
.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed);
}
[Theory, BitAutoData]
@ -370,6 +382,14 @@ public class RemoveOrganizationUserCommandTests @@ -370,6 +382,14 @@ public class RemoveOrganizationUserCommandTests
{
// Act & Assert
await Assert.ThrowsAsync<NotFoundException>(async () => await sutProvider.Sut.RemoveUserAsync(organizationId, userId));
await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.DeleteAsync(default);
await sutProvider.GetDependency<IEventService>()
.DidNotReceiveWithAnyArgs()
.LogOrganizationUserEventAsync((OrganizationUser)default, default);
}
[Theory, BitAutoData]
@ -413,6 +433,14 @@ public class RemoveOrganizationUserCommandTests @@ -413,6 +433,14 @@ public class RemoveOrganizationUserCommandTests
organizationUser.OrganizationId,
Arg.Is<IEnumerable<Guid>>(i => i.Contains(organizationUser.Id)),
Arg.Any<bool>());
await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.DeleteAsync(default);
await sutProvider.GetDependency<IEventService>()
.DidNotReceiveWithAnyArgs()
.LogOrganizationUserEventAsync((OrganizationUser)default, default);
}
[Theory, BitAutoData]
@ -424,6 +452,7 @@ public class RemoveOrganizationUserCommandTests @@ -424,6 +452,7 @@ public class RemoveOrganizationUserCommandTests
var sutProvider = SutProviderFactory();
var eventDate = sutProvider.GetDependency<FakeTimeProvider>().GetUtcNow().UtcDateTime;
orgUser1.OrganizationId = orgUser2.OrganizationId = deletingUser.OrganizationId;
var organizationUsers = new[] { orgUser1, orgUser2 };
var organizationUserIds = organizationUsers.Select(u => u.Id);
@ -774,6 +803,105 @@ public class RemoveOrganizationUserCommandTests @@ -774,6 +803,105 @@ public class RemoveOrganizationUserCommandTests
Assert.Contains(RemoveOrganizationUserCommand.RemoveLastConfirmedOwnerErrorMessage, exception.Message);
}
[Theory, BitAutoData]
public async Task UserLeave_Success(
[OrganizationUser(type: OrganizationUserType.User)] OrganizationUser organizationUser,
SutProvider<RemoveOrganizationUserCommand> sutProvider)
{
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value)
.Returns(organizationUser);
sutProvider.GetDependency<IHasConfirmedOwnersExceptQuery>()
.HasConfirmedOwnersExceptAsync(
organizationUser.OrganizationId,
Arg.Is<IEnumerable<Guid>>(i => i.Contains(organizationUser.Id)),
Arg.Any<bool>())
.Returns(true);
await sutProvider.Sut.UserLeaveAsync(organizationUser.OrganizationId, organizationUser.UserId.Value);
await sutProvider.GetDependency<IOrganizationUserRepository>()
.Received(1)
.DeleteAsync(organizationUser);
await sutProvider.GetDependency<IEventService>()
.Received(1)
.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Left);
}
[Theory, BitAutoData]
public async Task UserLeave_NotFound_ThrowsException(SutProvider<RemoveOrganizationUserCommand> sutProvider,
Guid organizationId, Guid userId)
{
await Assert.ThrowsAsync<NotFoundException>(async () => await sutProvider.Sut.UserLeaveAsync(organizationId, userId));
await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.DeleteAsync(default);
await sutProvider.GetDependency<IEventService>()
.DidNotReceiveWithAnyArgs()
.LogOrganizationUserEventAsync((OrganizationUser)default, default);
}
[Theory, BitAutoData]
public async Task UserLeave_InvalidUser_ThrowsException(OrganizationUser organizationUser,
SutProvider<RemoveOrganizationUserCommand> sutProvider)
{
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value)
.Returns(organizationUser);
var exception = await Assert.ThrowsAsync<NotFoundException>(
() => sutProvider.Sut.UserLeaveAsync(Guid.NewGuid(), organizationUser.UserId.Value));
Assert.Contains(RemoveOrganizationUserCommand.UserNotFoundErrorMessage, exception.Message);
await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.DeleteAsync(default);
await sutProvider.GetDependency<IEventService>()
.DidNotReceiveWithAnyArgs()
.LogOrganizationUserEventAsync((OrganizationUser)default, default);
}
[Theory, BitAutoData]
public async Task UserLeave_RemovingLastOwner_ThrowsException(
[OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser organizationUser,
SutProvider<RemoveOrganizationUserCommand> sutProvider)
{
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value)
.Returns(organizationUser);
sutProvider.GetDependency<IHasConfirmedOwnersExceptQuery>()
.HasConfirmedOwnersExceptAsync(
organizationUser.OrganizationId,
Arg.Is<IEnumerable<Guid>>(i => i.Contains(organizationUser.Id)),
Arg.Any<bool>())
.Returns(false);
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.UserLeaveAsync(organizationUser.OrganizationId, organizationUser.UserId.Value));
Assert.Contains(RemoveOrganizationUserCommand.RemoveLastConfirmedOwnerErrorMessage, exception.Message);
_ = sutProvider.GetDependency<IHasConfirmedOwnersExceptQuery>()
.Received(1)
.HasConfirmedOwnersExceptAsync(
organizationUser.OrganizationId,
Arg.Is<IEnumerable<Guid>>(i => i.Contains(organizationUser.Id)),
Arg.Any<bool>());
await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.DeleteAsync(default);
await sutProvider.GetDependency<IEventService>()
.DidNotReceiveWithAnyArgs()
.LogOrganizationUserEventAsync((OrganizationUser)default, default);
}
/// <summary>
/// Returns a new SutProvider with a FakeTimeProvider registered in the Sut.
/// </summary>

Loading…
Cancel
Save