From a128cf150689b627ad65ef6edb2c4065e7abf447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Thu, 24 Oct 2024 15:39:35 +0100 Subject: [PATCH] [PM-12758] Add managed status to OrganizationUserDetailsResponseModel and OrganizationUserUserDetailsResponse (#4918) * Refactor OrganizationUsersController.Get to include organization management status of organization users in details endpoint * Refactor OrganizationUsersController.Get to include organization management status of an individual user in details endpoint * Remove redundant .ToDictionary() * Simpify the property xmldoc * Name tuple variables in OrganizationUsersController.Get * Name returned tuple objects in GetDetailsByIdWithCollectionsAsync method in OrganizationUserRepository * Refactor MembersController.Get to destructure tuple returned by GetDetailsByIdWithCollectionsAsync * Add test for OrganizationUsersController.Get to assert ManagedByOrganization is set accordingly --- .../OrganizationUsersController.cs | 37 ++++++++++++---- .../OrganizationUserResponseModel.cs | 17 +++++++- .../Public/Controllers/MembersController.cs | 5 +-- .../IOrganizationUserRepository.cs | 3 +- .../OrganizationUserRepository.cs | 7 ++-- .../OrganizationUserRepository.cs | 4 +- .../OrganizationUsersControllerTests.cs | 42 +++++++++++++++++-- 7 files changed, 91 insertions(+), 24 deletions(-) diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index af0aede5a..b6d41ffec 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -53,6 +53,8 @@ public class OrganizationUsersController : Controller private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; private readonly IDeleteManagedOrganizationUserAccountCommand _deleteManagedOrganizationUserAccountCommand; + private readonly IGetOrganizationUsersManagementStatusQuery _getOrganizationUsersManagementStatusQuery; + private readonly IFeatureService _featureService; public OrganizationUsersController( IOrganizationRepository organizationRepository, @@ -73,7 +75,9 @@ public class OrganizationUsersController : Controller IOrganizationUserUserDetailsQuery organizationUserUserDetailsQuery, ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, IRemoveOrganizationUserCommand removeOrganizationUserCommand, - IDeleteManagedOrganizationUserAccountCommand deleteManagedOrganizationUserAccountCommand) + IDeleteManagedOrganizationUserAccountCommand deleteManagedOrganizationUserAccountCommand, + IGetOrganizationUsersManagementStatusQuery getOrganizationUsersManagementStatusQuery, + IFeatureService featureService) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -94,22 +98,28 @@ public class OrganizationUsersController : Controller _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; _removeOrganizationUserCommand = removeOrganizationUserCommand; _deleteManagedOrganizationUserAccountCommand = deleteManagedOrganizationUserAccountCommand; + _getOrganizationUsersManagementStatusQuery = getOrganizationUsersManagementStatusQuery; + _featureService = featureService; } [HttpGet("{id}")] - public async Task Get(string id, bool includeGroups = false) + public async Task Get(Guid id, bool includeGroups = false) { - var organizationUser = await _organizationUserRepository.GetDetailsByIdWithCollectionsAsync(new Guid(id)); - if (organizationUser == null || !await _currentContext.ManageUsers(organizationUser.Item1.OrganizationId)) + var (organizationUser, collections) = await _organizationUserRepository.GetDetailsByIdWithCollectionsAsync(id); + if (organizationUser == null || !await _currentContext.ManageUsers(organizationUser.OrganizationId)) { throw new NotFoundException(); } - var response = new OrganizationUserDetailsResponseModel(organizationUser.Item1, organizationUser.Item2); + var managedByOrganization = await GetManagedByOrganizationStatusAsync( + organizationUser.OrganizationId, + [organizationUser.Id]); + + var response = new OrganizationUserDetailsResponseModel(organizationUser, managedByOrganization[organizationUser.Id], collections); if (includeGroups) { - response.Groups = await _groupRepository.GetManyIdsByUserIdAsync(organizationUser.Item1.Id); + response.Groups = await _groupRepository.GetManyIdsByUserIdAsync(organizationUser.Id); } return response; @@ -150,11 +160,13 @@ public class OrganizationUsersController : Controller } ); var organizationUsersTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(organizationUsers); + var organizationUsersManagementStatus = await GetManagedByOrganizationStatusAsync(orgId, organizationUsers.Select(o => o.Id)); var responses = organizationUsers .Select(o => { var userTwoFactorEnabled = organizationUsersTwoFactorEnabled.FirstOrDefault(u => u.user.Id == o.Id).twoFactorIsEnabled; - var orgUser = new OrganizationUserUserDetailsResponseModel(o, userTwoFactorEnabled); + var managedByOrganization = organizationUsersManagementStatus[o.Id]; + var orgUser = new OrganizationUserUserDetailsResponseModel(o, userTwoFactorEnabled, managedByOrganization); return orgUser; }); @@ -682,4 +694,15 @@ public class OrganizationUsersController : Controller return new ListResponseModel(result.Select(r => new OrganizationUserBulkResponseModel(r.Item1.Id, r.Item2))); } + + private async Task> GetManagedByOrganizationStatusAsync(Guid orgId, IEnumerable userIds) + { + if (!_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)) + { + return userIds.ToDictionary(kvp => kvp, kvp => false); + } + + var usersOrganizationManagementStatus = await _getOrganizationUsersManagementStatusQuery.GetUsersOrganizationManagementStatusAsync(orgId, userIds); + return usersOrganizationManagementStatus; + } } diff --git a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs index 874169486..64dca73aa 100644 --- a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs +++ b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs @@ -64,20 +64,27 @@ public class OrganizationUserResponseModel : ResponseModel public class OrganizationUserDetailsResponseModel : OrganizationUserResponseModel { - public OrganizationUserDetailsResponseModel(OrganizationUser organizationUser, + public OrganizationUserDetailsResponseModel( + OrganizationUser organizationUser, + bool managedByOrganization, IEnumerable collections) : base(organizationUser, "organizationUserDetails") { + ManagedByOrganization = managedByOrganization; Collections = collections.Select(c => new SelectionReadOnlyResponseModel(c)); } public OrganizationUserDetailsResponseModel(OrganizationUserUserDetails organizationUser, + bool managedByOrganization, IEnumerable collections) : base(organizationUser, "organizationUserDetails") { + ManagedByOrganization = managedByOrganization; Collections = collections.Select(c => new SelectionReadOnlyResponseModel(c)); } + public bool ManagedByOrganization { get; set; } + public IEnumerable Collections { get; set; } [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] @@ -110,7 +117,7 @@ public class OrganizationUserUserMiniDetailsResponseModel : ResponseModel public class OrganizationUserUserDetailsResponseModel : OrganizationUserResponseModel { public OrganizationUserUserDetailsResponseModel(OrganizationUserUserDetails organizationUser, - bool twoFactorEnabled, string obj = "organizationUserUserDetails") + bool twoFactorEnabled, bool managedByOrganization, string obj = "organizationUserUserDetails") : base(organizationUser, obj) { if (organizationUser == null) @@ -127,6 +134,7 @@ public class OrganizationUserUserDetailsResponseModel : OrganizationUserResponse Groups = organizationUser.Groups; // Prevent reset password when using key connector. ResetPasswordEnrolled = ResetPasswordEnrolled && !organizationUser.UsesKeyConnector; + ManagedByOrganization = managedByOrganization; } public string Name { get; set; } @@ -134,6 +142,11 @@ public class OrganizationUserUserDetailsResponseModel : OrganizationUserResponse public string AvatarColor { get; set; } public bool TwoFactorEnabled { get; set; } public bool SsoBound { get; set; } + /// + /// Indicates if the organization manages the user. If a user is "managed" by an organization, + /// the organization has greater control over their account, and some user actions are restricted. + /// + public bool ManagedByOrganization { get; set; } public IEnumerable Collections { get; set; } public IEnumerable Groups { get; set; } } diff --git a/src/Api/AdminConsole/Public/Controllers/MembersController.cs b/src/Api/AdminConsole/Public/Controllers/MembersController.cs index 7515111d2..4e99353d4 100644 --- a/src/Api/AdminConsole/Public/Controllers/MembersController.cs +++ b/src/Api/AdminConsole/Public/Controllers/MembersController.cs @@ -71,14 +71,13 @@ public class MembersController : Controller [ProducesResponseType((int)HttpStatusCode.NotFound)] public async Task Get(Guid id) { - var userDetails = await _organizationUserRepository.GetDetailsByIdWithCollectionsAsync(id); - var orgUser = userDetails?.Item1; + var (orgUser, collections) = await _organizationUserRepository.GetDetailsByIdWithCollectionsAsync(id); if (orgUser == null || orgUser.OrganizationId != _currentContext.OrganizationId) { return new NotFoundResult(); } var response = new MemberResponseModel(orgUser, await _userService.TwoFactorIsEnabledAsync(orgUser), - userDetails.Item2); + collections); return new JsonResult(response); } diff --git a/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs b/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs index 54040e6dc..a3a68b5de 100644 --- a/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs +++ b/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs @@ -22,8 +22,7 @@ public interface IOrganizationUserRepository : IRepository GetByOrganizationAsync(Guid organizationId, Guid userId); Task>> GetByIdWithCollectionsAsync(Guid id); Task GetDetailsByIdAsync(Guid id); - Task>> - GetDetailsByIdWithCollectionsAsync(Guid id); + Task<(OrganizationUserUserDetails? OrganizationUser, ICollection Collections)> GetDetailsByIdWithCollectionsAsync(Guid id); Task> GetManyDetailsByOrganizationAsync(Guid organizationId, bool includeGroups = false, bool includeCollections = false); Task> GetManyDetailsByUserAsync(Guid userId, OrganizationUserStatusType? status = null); diff --git a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs index 6da2f581f..361b1f058 100644 --- a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs @@ -196,8 +196,7 @@ public class OrganizationUserRepository : Repository, IO return results.SingleOrDefault(); } } - public async Task>> - GetDetailsByIdWithCollectionsAsync(Guid id) + public async Task<(OrganizationUserUserDetails? OrganizationUser, ICollection Collections)> GetDetailsByIdWithCollectionsAsync(Guid id) { using (var connection = new SqlConnection(ConnectionString)) { @@ -206,9 +205,9 @@ public class OrganizationUserRepository : Repository, IO new { Id = id }, commandType: CommandType.StoredProcedure); - var user = (await results.ReadAsync()).SingleOrDefault(); + var organizationUserUserDetails = (await results.ReadAsync()).SingleOrDefault(); var collections = (await results.ReadAsync()).ToList(); - return new Tuple>(user, collections); + return (organizationUserUserDetails, collections); } } diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs index 089a0a5c5..0c9f1d0b9 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs @@ -248,7 +248,7 @@ public class OrganizationUserRepository : Repository>> GetDetailsByIdWithCollectionsAsync(Guid id) + public async Task<(OrganizationUserUserDetails? OrganizationUser, ICollection Collections)> GetDetailsByIdWithCollectionsAsync(Guid id) { var organizationUserUserDetails = await GetDetailsByIdAsync(id); using (var scope = ServiceScopeFactory.CreateScope()) @@ -265,7 +265,7 @@ public class OrganizationUserRepository : Repository>(organizationUserUserDetails, collections); + return (organizationUserUserDetails, collections); } } diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs index 492112e5a..2ff5c0cb4 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs @@ -3,6 +3,7 @@ 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; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; @@ -15,6 +16,7 @@ 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; using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; @@ -185,14 +187,46 @@ public class OrganizationUsersControllerTests await Assert.ThrowsAsync(() => sutProvider.Sut.Invite(organizationAbility.Id, model)); } + [Theory] + [BitAutoData(true)] + [BitAutoData(false)] + public async Task Get_ReturnsUser( + bool accountDeprovisioningEnabled, + OrganizationUserUserDetails organizationUser, ICollection collections, + SutProvider sutProvider) + { + organizationUser.Permissions = null; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + .Returns(accountDeprovisioningEnabled); + + sutProvider.GetDependency() + .ManageUsers(organizationUser.OrganizationId) + .Returns(true); + + sutProvider.GetDependency() + .GetDetailsByIdWithCollectionsAsync(organizationUser.Id) + .Returns((organizationUser, collections)); + + sutProvider.GetDependency() + .GetUsersOrganizationManagementStatusAsync(organizationUser.OrganizationId, Arg.Is>(ids => ids.Contains(organizationUser.Id))) + .Returns(new Dictionary { { organizationUser.Id, true } }); + + var response = await sutProvider.Sut.Get(organizationUser.Id, false); + + Assert.Equal(organizationUser.Id, response.Id); + Assert.Equal(accountDeprovisioningEnabled, response.ManagedByOrganization); + } + [Theory] [BitAutoData] - public async Task Get_ReturnsUsers( + public async Task GetMany_ReturnsUsers( ICollection organizationUsers, OrganizationAbility organizationAbility, SutProvider sutProvider) { - Get_Setup(organizationAbility, organizationUsers, sutProvider); - var response = await sutProvider.Sut.Get(organizationAbility.Id); + GetMany_Setup(organizationAbility, organizationUsers, sutProvider); + var response = await sutProvider.Sut.Get(organizationAbility.Id, false, false); Assert.True(response.Data.All(r => organizationUsers.Any(ou => ou.Id == r.Id))); } @@ -368,7 +402,7 @@ public class OrganizationUsersControllerTests await Assert.ThrowsAsync(() => sutProvider.Sut.BulkDeleteAccount(orgId, model)); } - private void Get_Setup(OrganizationAbility organizationAbility, + private void GetMany_Setup(OrganizationAbility organizationAbility, ICollection organizationUsers, SutProvider sutProvider) {