From 7fe4fe16cb8270be83235617f15d1f69e894ac46 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Fri, 12 Jul 2024 06:13:10 +1000 Subject: [PATCH] [AC-1331] Remove Manager role - final (#4493) * Remove OrganizationUserType.Manager * Add EnumDataType validation to prevent invalid enum values --- .../OrganizationUserRequestModels.cs | 2 + .../Public/Models/MemberBaseModel.cs | 4 +- .../Enums/OrganizationUserType.cs | 2 +- .../UpdateOrganizationUserCommand.cs | 5 --- .../Implementations/OrganizationService.cs | 5 --- src/Core/Context/CurrentContext.cs | 17 --------- src/Core/Context/ICurrentContext.cs | 2 - src/Core/Identity/Claims.cs | 1 - src/Core/Utilities/CoreHelpers.cs | 6 --- src/Identity/IdentityServer/ApiResources.cs | 1 - .../UpdateOrganizationUserCommandTests.cs | 38 ------------------- .../Services/OrganizationServiceTests.cs | 26 +------------ .../Endpoints/IdentityServerTests.cs | 4 -- .../openid-configuration.json | 1 - 14 files changed, 6 insertions(+), 108 deletions(-) diff --git a/src/Api/AdminConsole/Models/Request/Organizations/OrganizationUserRequestModels.cs b/src/Api/AdminConsole/Models/Request/Organizations/OrganizationUserRequestModels.cs index 40aa62c9d..bbbb571f4 100644 --- a/src/Api/AdminConsole/Models/Request/Organizations/OrganizationUserRequestModels.cs +++ b/src/Api/AdminConsole/Models/Request/Organizations/OrganizationUserRequestModels.cs @@ -14,6 +14,7 @@ public class OrganizationUserInviteRequestModel [StrictEmailAddressList] public IEnumerable Emails { get; set; } [Required] + [EnumDataType(typeof(OrganizationUserType))] public OrganizationUserType? Type { get; set; } public bool AccessSecretsManager { get; set; } public Permissions Permissions { get; set; } @@ -83,6 +84,7 @@ public class OrganizationUserBulkConfirmRequestModel public class OrganizationUserUpdateRequestModel { [Required] + [EnumDataType(typeof(OrganizationUserType))] public OrganizationUserType? Type { get; set; } public bool AccessSecretsManager { get; set; } public Permissions Permissions { get; set; } diff --git a/src/Api/AdminConsole/Public/Models/MemberBaseModel.cs b/src/Api/AdminConsole/Public/Models/MemberBaseModel.cs index 58f644409..79ec0ad78 100644 --- a/src/Api/AdminConsole/Public/Models/MemberBaseModel.cs +++ b/src/Api/AdminConsole/Public/Models/MemberBaseModel.cs @@ -45,10 +45,10 @@ public abstract class MemberBaseModel } /// - /// The member's type (or role) within the organization. If your organization has is using the latest collection enhancements, - /// you will not be allowed to assign the Manager role (OrganizationUserType = 3). + /// The member's type (or role) within the organization. /// [Required] + [EnumDataType(typeof(OrganizationUserType))] public OrganizationUserType? Type { get; set; } /// /// External identifier for reference or linking this member to another system, such as a user directory. diff --git a/src/Core/AdminConsole/Enums/OrganizationUserType.cs b/src/Core/AdminConsole/Enums/OrganizationUserType.cs index 620eaeb33..be5986a65 100644 --- a/src/Core/AdminConsole/Enums/OrganizationUserType.cs +++ b/src/Core/AdminConsole/Enums/OrganizationUserType.cs @@ -5,6 +5,6 @@ public enum OrganizationUserType : byte Owner = 0, Admin = 1, User = 2, - Manager = 3, + // Manager = 3 has been intentionally permanently deleted Custom = 4, } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs index 9d25e6442..8dcac12dd 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs @@ -61,11 +61,6 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand // If the organization is using Flexible Collections, prevent use of any deprecated permissions var organization = await _organizationRepository.GetByIdAsync(user.OrganizationId); - if (organization.FlexibleCollections && user.Type == OrganizationUserType.Manager) - { - throw new BadRequestException("The Manager role has been deprecated by collection enhancements. Use the collection Can Manage permission instead."); - } - if (organization.FlexibleCollections && user.AccessAll) { throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the user to collections instead."); diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 8aa5f5bc4..5e66bf445 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -1039,11 +1039,6 @@ public class OrganizationService : IOrganizationService } // If the organization is using Flexible Collections, prevent use of any deprecated permissions - if (organization.FlexibleCollections && invites.Any(i => i.invite.Type is OrganizationUserType.Manager)) - { - throw new BadRequestException("The Manager role has been deprecated by collection enhancements. Use the collection Can Manage permission instead."); - } - if (organization.FlexibleCollections && invites.Any(i => i.invite.AccessAll)) { throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the user to collections instead."); diff --git a/src/Core/Context/CurrentContext.cs b/src/Core/Context/CurrentContext.cs index 52eea7e7f..4458b8da6 100644 --- a/src/Core/Context/CurrentContext.cs +++ b/src/Core/Context/CurrentContext.cs @@ -217,17 +217,6 @@ public class CurrentContext : ICurrentContext })); } - if (claimsDict.ContainsKey(Claims.OrganizationManager)) - { - organizations.AddRange(claimsDict[Claims.OrganizationManager].Select(c => - new CurrentContextOrganization - { - Id = new Guid(c.Value), - Type = OrganizationUserType.Manager, - AccessSecretsManager = accessSecretsManager.ContainsKey(c.Value), - })); - } - if (claimsDict.ContainsKey(Claims.OrganizationCustom)) { organizations.AddRange(claimsDict[Claims.OrganizationCustom].Select(c => @@ -274,12 +263,6 @@ public class CurrentContext : ICurrentContext return (Organizations?.Any(o => o.Id == orgId) ?? false) || await OrganizationOwner(orgId); } - public async Task OrganizationManager(Guid orgId) - { - return await OrganizationAdmin(orgId) || - (Organizations?.Any(o => o.Id == orgId && o.Type == OrganizationUserType.Manager) ?? false); - } - public async Task OrganizationAdmin(Guid orgId) { return await OrganizationOwner(orgId) || diff --git a/src/Core/Context/ICurrentContext.cs b/src/Core/Context/ICurrentContext.cs index 480d5f7d1..fcf4f6847 100644 --- a/src/Core/Context/ICurrentContext.cs +++ b/src/Core/Context/ICurrentContext.cs @@ -36,8 +36,6 @@ public interface ICurrentContext Task OrganizationUser(Guid orgId); - [Obsolete("Manager role is deprecated after Flexible Collections.")] - Task OrganizationManager(Guid orgId); Task OrganizationAdmin(Guid orgId); Task OrganizationOwner(Guid orgId); Task OrganizationCustom(Guid orgId); diff --git a/src/Core/Identity/Claims.cs b/src/Core/Identity/Claims.cs index 318f0b400..b1223a6e6 100644 --- a/src/Core/Identity/Claims.cs +++ b/src/Core/Identity/Claims.cs @@ -9,7 +9,6 @@ public static class Claims public const string OrganizationOwner = "orgowner"; public const string OrganizationAdmin = "orgadmin"; - public const string OrganizationManager = "orgmanager"; public const string OrganizationUser = "orguser"; public const string OrganizationCustom = "orgcustom"; public const string ProviderAdmin = "providerprovideradmin"; diff --git a/src/Core/Utilities/CoreHelpers.cs b/src/Core/Utilities/CoreHelpers.cs index c4f7595c2..043a36c15 100644 --- a/src/Core/Utilities/CoreHelpers.cs +++ b/src/Core/Utilities/CoreHelpers.cs @@ -700,12 +700,6 @@ public static class CoreHelpers claims.Add(new KeyValuePair(Claims.OrganizationAdmin, org.Id.ToString())); } break; - case Enums.OrganizationUserType.Manager: - foreach (var org in group) - { - claims.Add(new KeyValuePair(Claims.OrganizationManager, org.Id.ToString())); - } - break; case Enums.OrganizationUserType.User: foreach (var org in group) { diff --git a/src/Identity/IdentityServer/ApiResources.cs b/src/Identity/IdentityServer/ApiResources.cs index aa4104127..a0712aafe 100644 --- a/src/Identity/IdentityServer/ApiResources.cs +++ b/src/Identity/IdentityServer/ApiResources.cs @@ -20,7 +20,6 @@ public class ApiResources Claims.Device, Claims.OrganizationOwner, Claims.OrganizationAdmin, - Claims.OrganizationManager, Claims.OrganizationUser, Claims.OrganizationCustom, Claims.ProviderAdmin, diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs index ed0a1cdff..ce2981f35 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs @@ -77,44 +77,6 @@ public class UpdateOrganizationUserCommandTests Arg.Is>(i => i.Contains(newUserData.Id))); } - [Theory, BitAutoData] - public async Task UpdateUserAsync_WithFlexibleCollections_WhenUpgradingToManager_Throws( - Organization organization, - [OrganizationUser(type: OrganizationUserType.User)] OrganizationUser oldUserData, - [OrganizationUser(type: OrganizationUserType.Manager)] OrganizationUser newUserData, - [OrganizationUser(type: OrganizationUserType.Owner, status: OrganizationUserStatusType.Confirmed)] OrganizationUser savingUser, - ICollection collections, - IEnumerable groups, - SutProvider sutProvider) - { - organization.FlexibleCollections = true; - newUserData.Id = oldUserData.Id; - newUserData.UserId = oldUserData.UserId; - newUserData.OrganizationId = oldUserData.OrganizationId = savingUser.OrganizationId = organization.Id; - newUserData.Permissions = CoreHelpers.ClassToJsonData(new Permissions()); - - sutProvider.GetDependency() - .GetByIdAsync(organization.Id) - .Returns(organization); - - sutProvider.GetDependency() - .HasConfirmedOwnersExceptAsync(newUserData.OrganizationId, Arg.Is>(i => i.Contains(newUserData.Id))) - .Returns(true); - - sutProvider.GetDependency() - .GetByIdAsync(oldUserData.Id) - .Returns(oldUserData); - - sutProvider.GetDependency() - .GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) - .Returns(new List { savingUser }); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.UpdateUserAsync(newUserData, oldUserData.UserId, collections, groups)); - - Assert.Contains("manager role has been deprecated", exception.Message.ToLowerInvariant()); - } - [Theory, BitAutoData] public async Task UpdateUserAsync_WithFlexibleCollections_WithAccessAll_Throws( Organization organization, diff --git a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs index d1d6394d5..398b88174 100644 --- a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs @@ -732,7 +732,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) [Theory] [OrganizationCustomize(FlexibleCollections = false)] [BitAutoData(OrganizationUserType.Admin)] - [BitAutoData(OrganizationUserType.Manager)] [BitAutoData(OrganizationUserType.Owner)] [BitAutoData(OrganizationUserType.User)] public async Task InviteUsers_WithNonCustomType_WhenUseCustomPermissionsIsFalse_Passes(OrganizationUserType inviteUserType, Organization organization, OrganizationUserInvite invite, @@ -762,7 +761,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) [Theory] [OrganizationInviteCustomize( - InviteeUserType = OrganizationUserType.Manager, + InviteeUserType = OrganizationUserType.User, InvitorUserType = OrganizationUserType.Custom ), OrganizationCustomize(FlexibleCollections = false), BitAutoData] public async Task InviteUsers_CustomUserWithoutManageUsersConfiguringUser_Throws(Organization organization, OrganizationUserInvite invite, @@ -1183,28 +1182,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) }); } - [Theory, OrganizationCustomize(FlexibleCollections = true), BitAutoData] - public async Task InviteUsers_WithFlexibleCollections_WhenInvitingManager_Throws(Organization organization, - OrganizationUserInvite invite, OrganizationUser invitor, SutProvider sutProvider) - { - invite.Type = OrganizationUserType.Manager; - organization.FlexibleCollections = true; - - sutProvider.GetDependency() - .GetByIdAsync(organization.Id) - .Returns(organization); - - sutProvider.GetDependency() - .ManageUsers(organization.Id) - .Returns(true); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.InviteUsersAsync(organization.Id, invitor.UserId, systemUser: null, - new (OrganizationUserInvite, string)[] { (invite, null) })); - - Assert.Contains("manager role has been deprecated", exception.Message.ToLowerInvariant()); - } - [Theory, OrganizationCustomize(FlexibleCollections = true), BitAutoData] public async Task InviteUsers_WithFlexibleCollections_WithAccessAll_Throws(Organization organization, OrganizationUserInvite invite, OrganizationUser invitor, SutProvider sutProvider) @@ -2297,7 +2274,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) [BitAutoData(OrganizationUserType.Owner)] [BitAutoData(OrganizationUserType.Admin)] [BitAutoData(OrganizationUserType.User)] - [BitAutoData(OrganizationUserType.Manager)] public async Task ValidateOrganizationCustomPermissionsEnabledAsync_WithNotCustomType_IsValid( OrganizationUserType newType, Guid organizationId, diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs index a41c4449f..ae64b832f 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs @@ -145,7 +145,6 @@ public class IdentityServerTests : IClassFixture [BitAutoData(OrganizationUserType.Owner)] [BitAutoData(OrganizationUserType.Admin)] [BitAutoData(OrganizationUserType.User)] - [BitAutoData(OrganizationUserType.Manager)] [BitAutoData(OrganizationUserType.Custom)] public async Task TokenEndpoint_GrantTypePassword_WithAllUserTypes_WithSsoPolicyDisabled_WithEnforceSsoPolicyForAllUsersTrue_Success(OrganizationUserType organizationUserType, Guid organizationId, string deviceId, int generatedUsername) { @@ -173,7 +172,6 @@ public class IdentityServerTests : IClassFixture [BitAutoData(OrganizationUserType.Owner)] [BitAutoData(OrganizationUserType.Admin)] [BitAutoData(OrganizationUserType.User)] - [BitAutoData(OrganizationUserType.Manager)] [BitAutoData(OrganizationUserType.Custom)] public async Task TokenEndpoint_GrantTypePassword_WithAllUserTypes_WithSsoPolicyDisabled_WithEnforceSsoPolicyForAllUsersFalse_Success(OrganizationUserType organizationUserType, Guid organizationId, string deviceId, int generatedUsername) { @@ -201,7 +199,6 @@ public class IdentityServerTests : IClassFixture [BitAutoData(OrganizationUserType.Owner)] [BitAutoData(OrganizationUserType.Admin)] [BitAutoData(OrganizationUserType.User)] - [BitAutoData(OrganizationUserType.Manager)] [BitAutoData(OrganizationUserType.Custom)] public async Task TokenEndpoint_GrantTypePassword_WithAllUserTypes_WithSsoPolicyEnabled_WithEnforceSsoPolicyForAllUsersTrue_Throw(OrganizationUserType organizationUserType, Guid organizationId, string deviceId, int generatedUsername) { @@ -253,7 +250,6 @@ public class IdentityServerTests : IClassFixture [Theory] [BitAutoData(OrganizationUserType.User)] - [BitAutoData(OrganizationUserType.Manager)] [BitAutoData(OrganizationUserType.Custom)] public async Task TokenEndpoint_GrantTypePassword_WithNonOwnerOrAdmin_WithSsoPolicyEnabled_WithEnforceSsoPolicyForAllUsersFalse_Throws(OrganizationUserType organizationUserType, Guid organizationId, string deviceId, int generatedUsername) { diff --git a/test/Identity.IntegrationTest/openid-configuration.json b/test/Identity.IntegrationTest/openid-configuration.json index 8cd464d1d..23e5a67c0 100644 --- a/test/Identity.IntegrationTest/openid-configuration.json +++ b/test/Identity.IntegrationTest/openid-configuration.json @@ -26,7 +26,6 @@ "device", "orgowner", "orgadmin", - "orgmanager", "orguser", "orgcustom", "providerprovideradmin",