From 2c40dc0602241c8a9b5adb4526ab4353723f5839 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Tue, 4 Jun 2024 08:47:12 +1000 Subject: [PATCH] [AC-2654] Remove old permissions code from OrganizationUsersController (#4149) --- .../OrganizationUsersController.cs | 100 ++++++------------ .../OrganizationUsersControllerTests.cs | 19 +--- 2 files changed, 38 insertions(+), 81 deletions(-) diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index 558434d0c..0b93839d2 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -93,17 +93,15 @@ public class OrganizationUsersController : Controller } var response = new OrganizationUserDetailsResponseModel(organizationUser.Item1, organizationUser.Item2); - if (await FlexibleCollectionsIsEnabledAsync(organizationUser.Item1.OrganizationId)) - { - // Downgrade Custom users with no other permissions than 'Edit/Delete Assigned Collections' to User - response.Type = GetFlexibleCollectionsUserType(response.Type, response.Permissions); - // Set 'Edit/Delete Assigned Collections' custom permissions to false - if (response.Permissions is not null) - { - response.Permissions.EditAssignedCollections = false; - response.Permissions.DeleteAssignedCollections = false; - } + // Downgrade Custom users with no other permissions than 'Edit/Delete Assigned Collections' to User + response.Type = GetFlexibleCollectionsUserType(response.Type, response.Permissions); + + // Set 'Edit/Delete Assigned Collections' custom permissions to false + if (response.Permissions is not null) + { + response.Permissions.EditAssignedCollections = false; + response.Permissions.DeleteAssignedCollections = false; } if (includeGroups) @@ -117,24 +115,35 @@ public class OrganizationUsersController : Controller [HttpGet("")] public async Task> Get(Guid orgId, bool includeGroups = false, bool includeCollections = false) { - if (await FlexibleCollectionsIsEnabledAsync(orgId)) - { - return await Get_vNext(orgId, includeGroups, includeCollections); - } - - var authorized = await _currentContext.ViewAllCollections(orgId) || - await _currentContext.ViewAssignedCollections(orgId) || - await _currentContext.ManageGroups(orgId) || - await _currentContext.ManageUsers(orgId); + var authorized = (await _authorizationService.AuthorizeAsync( + User, OrganizationUserOperations.ReadAll(orgId))).Succeeded; if (!authorized) { throw new NotFoundException(); } - var organizationUsers = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(orgId, includeGroups, includeCollections); - var responseTasks = organizationUsers.Select(async o => new OrganizationUserUserDetailsResponseModel(o, - await _userService.TwoFactorIsEnabledAsync(o))); + var organizationUsers = await _organizationUserRepository + .GetManyDetailsByOrganizationAsync(orgId, includeGroups, includeCollections); + var responseTasks = organizationUsers + .Select(async o => + { + var orgUser = new OrganizationUserUserDetailsResponseModel(o, + await _userService.TwoFactorIsEnabledAsync(o)); + + // Downgrade Custom users with no other permissions than 'Edit/Delete Assigned Collections' to User + orgUser.Type = GetFlexibleCollectionsUserType(orgUser.Type, orgUser.Permissions); + + // Set 'Edit/Delete Assigned Collections' custom permissions to false + if (orgUser.Permissions is not null) + { + orgUser.Permissions.EditAssignedCollections = false; + orgUser.Permissions.DeleteAssignedCollections = false; + } + + return orgUser; + }); var responses = await Task.WhenAll(responseTasks); + return new ListResponseModel(responses); } @@ -210,9 +219,7 @@ public class OrganizationUsersController : Controller } // Flexible Collections - check the user has permission to grant access to the collections for the new user - if (await FlexibleCollectionsIsEnabledAsync(orgId) && - _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1) && - model.Collections?.Any() == true) + if (_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1) && model.Collections?.Any() == true) { var collections = await _collectionRepository.GetManyByManyIdsAsync(model.Collections.Select(a => a.Id)); var authorized = @@ -347,7 +354,7 @@ public class OrganizationUsersController : Controller [HttpPost("{id}")] public async Task Put(Guid orgId, Guid id, [FromBody] OrganizationUserUpdateRequestModel model) { - if (await FlexibleCollectionsIsEnabledAsync(orgId) && _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1)) + if (_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1)) { // Use new Flexible Collections v1 logic await Put_vNext(orgId, id, model); @@ -625,47 +632,6 @@ public class OrganizationUsersController : Controller new OrganizationUserBulkResponseModel(r.Item1.Id, r.Item2))); } - private async Task FlexibleCollectionsIsEnabledAsync(Guid organizationId) - { - var organizationAbility = await _applicationCacheService.GetOrganizationAbilityAsync(organizationId); - return organizationAbility?.FlexibleCollections ?? false; - } - - private async Task> Get_vNext(Guid orgId, - bool includeGroups = false, bool includeCollections = false) - { - var authorized = (await _authorizationService.AuthorizeAsync( - User, OrganizationUserOperations.ReadAll(orgId))).Succeeded; - if (!authorized) - { - throw new NotFoundException(); - } - - var organizationUsers = await _organizationUserRepository - .GetManyDetailsByOrganizationAsync(orgId, includeGroups, includeCollections); - var responseTasks = organizationUsers - .Select(async o => - { - var orgUser = new OrganizationUserUserDetailsResponseModel(o, - await _userService.TwoFactorIsEnabledAsync(o)); - - // Downgrade Custom users with no other permissions than 'Edit/Delete Assigned Collections' to User - orgUser.Type = GetFlexibleCollectionsUserType(orgUser.Type, orgUser.Permissions); - - // Set 'Edit/Delete Assigned Collections' custom permissions to false - if (orgUser.Permissions is not null) - { - orgUser.Permissions.EditAssignedCollections = false; - orgUser.Permissions.DeleteAssignedCollections = false; - } - - return orgUser; - }); - var responses = await Task.WhenAll(responseTasks); - - return new ListResponseModel(responses); - } - private OrganizationUserType GetFlexibleCollectionsUserType(OrganizationUserType type, Permissions permissions) { // Downgrade Custom users with no other permissions than 'Edit/Delete Assigned Collections' to User diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs index c057c5322..5fef2885e 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs @@ -113,7 +113,6 @@ public class OrganizationUsersControllerTests public async Task Invite_Success(OrganizationAbility organizationAbility, OrganizationUserInviteRequestModel model, Guid userId, SutProvider sutProvider) { - organizationAbility.FlexibleCollections = true; sutProvider.GetDependency().ManageUsers(organizationAbility.Id).Returns(true); sutProvider.GetDependency().GetOrganizationAbilityAsync(organizationAbility.Id) .Returns(organizationAbility); @@ -139,7 +138,6 @@ public class OrganizationUsersControllerTests public async Task Invite_NotAuthorizedToGiveAccessToCollections_Throws(OrganizationAbility organizationAbility, OrganizationUserInviteRequestModel model, Guid userId, SutProvider sutProvider) { - organizationAbility.FlexibleCollections = true; sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true); sutProvider.GetDependency().ManageUsers(organizationAbility.Id).Returns(true); sutProvider.GetDependency().GetOrganizationAbilityAsync(organizationAbility.Id) @@ -161,10 +159,9 @@ public class OrganizationUsersControllerTests OrganizationUser organizationUser, OrganizationAbility organizationAbility, SutProvider sutProvider, Guid savingUserId) { - organizationAbility.FlexibleCollections = false; sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(false); - Put_Setup(sutProvider, organizationAbility, organizationUser, savingUserId, model, false); + Put_Setup(sutProvider, organizationAbility, organizationUser, savingUserId, model, true); // Save these for later - organizationUser object will be mutated var orgUserId = organizationUser.Id; @@ -193,7 +190,6 @@ public class OrganizationUsersControllerTests // Updating self organizationUser.UserId = savingUserId; organizationAbility.AllowAdminAccessToAllCollectionItems = false; - organizationAbility.FlexibleCollections = true; sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true); Put_Setup(sutProvider, organizationAbility, organizationUser, savingUserId, model, false); @@ -223,7 +219,6 @@ public class OrganizationUsersControllerTests // Updating self organizationUser.UserId = savingUserId; organizationAbility.AllowAdminAccessToAllCollectionItems = false; - organizationAbility.FlexibleCollections = true; sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true); Put_Setup(sutProvider, organizationAbility, organizationUser, savingUserId, model, true); @@ -253,7 +248,6 @@ public class OrganizationUsersControllerTests { // Updating self organizationUser.UserId = savingUserId; - organizationAbility.FlexibleCollections = true; organizationAbility.AllowAdminAccessToAllCollectionItems = true; sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true); @@ -282,7 +276,6 @@ public class OrganizationUsersControllerTests OrganizationUser organizationUser, OrganizationAbility organizationAbility, SutProvider sutProvider, Guid savingUserId) { - organizationAbility.FlexibleCollections = true; sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true); Put_Setup(sutProvider, organizationAbility, organizationUser, savingUserId, model, false); @@ -372,7 +365,6 @@ public class OrganizationUsersControllerTests OrganizationUser organizationUser, OrganizationAbility organizationAbility, SutProvider sutProvider, Guid savingUserId) { - organizationAbility.FlexibleCollections = true; sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true); Put_Setup(sutProvider, organizationAbility, organizationUser, savingUserId, model, false); @@ -396,7 +388,7 @@ public class OrganizationUsersControllerTests [Theory] [BitAutoData] - public async Task Get_WithFlexibleCollections_ReturnsUsers( + public async Task Get_ReturnsUsers( ICollection organizationUsers, OrganizationAbility organizationAbility, SutProvider sutProvider) { @@ -408,7 +400,7 @@ public class OrganizationUsersControllerTests [Theory] [BitAutoData] - public async Task Get_WithFlexibleCollections_HandlesNullPermissionsObject( + public async Task Get_HandlesNullPermissionsObject( ICollection organizationUsers, OrganizationAbility organizationAbility, SutProvider sutProvider) { @@ -421,7 +413,7 @@ public class OrganizationUsersControllerTests [Theory] [BitAutoData] - public async Task Get_WithFlexibleCollections_SetsDeprecatedCustomPermissionstoFalse( + public async Task Get_SetsDeprecatedCustomPermissionstoFalse( ICollection organizationUsers, OrganizationAbility organizationAbility, SutProvider sutProvider) { @@ -449,7 +441,7 @@ public class OrganizationUsersControllerTests [Theory] [BitAutoData] - public async Task Get_WithFlexibleCollections_DowngradesCustomUsersWithDeprecatedPermissions( + public async Task Get_DowngradesCustomUsersWithDeprecatedPermissions( ICollection organizationUsers, OrganizationAbility organizationAbility, SutProvider sutProvider) { @@ -544,7 +536,6 @@ public class OrganizationUsersControllerTests ICollection organizationUsers, SutProvider sutProvider) { - organizationAbility.FlexibleCollections = true; foreach (var orgUser in organizationUsers) { orgUser.Permissions = null;