From e309c3cc2f4001f89a485df20dafc029a61b27c3 Mon Sep 17 00:00:00 2001 From: Brandon Date: Wed, 11 Jun 2025 10:34:47 -0400 Subject: [PATCH] clean up --- .../Controllers/OrganizationController.cs | 8 ++-- ...ImportOrganizationUserAndGroupsCommand.cs} | 6 ++- ...mportOrganizationUsersAndGroupsCommand.cs} | 42 +++++++++---------- .../Import}/OrganizationGroupImportData.cs | 25 ++++++++++- .../Import}/OrganizationUserImportData.cs | 8 ++-- .../ImportOrganizationUserCommandTests.cs | 1 - 6 files changed, 55 insertions(+), 35 deletions(-) rename src/Core/AdminConsole/OrganizationFeatures/{OrganizationUsers/Interfaces/IImportOrganizationUserCommand.cs => Import/IImportOrganizationUserAndGroupsCommand.cs} (73%) rename src/Core/AdminConsole/OrganizationFeatures/{OrganizationUsers/ImportOrganizationUserCommand.cs => Import/ImportOrganizationUsersAndGroupsCommand.cs} (93%) rename src/Core/AdminConsole/{Models/Data/Organizations => OrganizationFeatures/Import}/OrganizationGroupImportData.cs (54%) rename src/Core/AdminConsole/{Models/Data/Organizations/OrganizationUsers => OrganizationFeatures/Import}/OrganizationUserImportData.cs (87%) diff --git a/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs b/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs index 0ce26c2708..3e9e644220 100644 --- a/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs +++ b/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs @@ -20,20 +20,20 @@ public class OrganizationController : Controller private readonly IOrganizationService _organizationService; private readonly ICurrentContext _currentContext; private readonly GlobalSettings _globalSettings; - private readonly IImportOrganizationUserCommand _importOrganizationUserCommand; + private readonly IImportOrganizationUsersAndGroupsCommand _importOrganizationUsersAndGroupsCommand; private readonly IFeatureService _featureService; public OrganizationController( IOrganizationService organizationService, ICurrentContext currentContext, GlobalSettings globalSettings, - IImportOrganizationUserCommand importOrganizationUserCommand, + IImportOrganizationUsersAndGroupsCommand importOrganizationUsersAndGroupsCommand, IFeatureService featureService) { _organizationService = organizationService; _currentContext = currentContext; _globalSettings = globalSettings; - _importOrganizationUserCommand = importOrganizationUserCommand; + _importOrganizationUsersAndGroupsCommand = importOrganizationUsersAndGroupsCommand; _featureService = featureService; } @@ -57,7 +57,7 @@ public class OrganizationController : Controller if (_featureService.IsEnabled(FeatureFlagKeys.ScimInviteUserOptimization)) { - await _importOrganizationUserCommand.ImportAsync( + await _importOrganizationUsersAndGroupsCommand.ImportAsync( _currentContext.OrganizationId.Value, model.Groups.Select(g => g.ToImportedGroup(_currentContext.OrganizationId.Value)), model.Members.Where(u => !u.Deleted).Select(u => u.ToImportedOrganizationUser()), diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IImportOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Import/IImportOrganizationUserAndGroupsCommand.cs similarity index 73% rename from src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IImportOrganizationUserCommand.cs rename to src/Core/AdminConsole/OrganizationFeatures/Import/IImportOrganizationUserAndGroupsCommand.cs index 4d9425e04f..b74da0a2e8 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IImportOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Import/IImportOrganizationUserAndGroupsCommand.cs @@ -1,9 +1,11 @@ -using Bit.Core.AdminConsole.Models.Business; +#nullable enable + +using Bit.Core.AdminConsole.Models.Business; using Bit.Core.Models.Business; namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; -public interface IImportOrganizationUserCommand +public interface IImportOrganizationUsersAndGroupsCommand { Task ImportAsync(Guid organizationId, IEnumerable groups, diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommand.cs similarity index 93% rename from src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommand.cs rename to src/Core/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommand.cs index 018e3da117..13aeb80ef0 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommand.cs @@ -1,4 +1,6 @@ -using Bit.Core.AdminConsole.Entities; +#nullable enable + +using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Models.Business; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers; @@ -16,11 +18,9 @@ using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Repositories; using Bit.Core.Services; -#nullable enable - namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; -public class ImportOrganizationUserCommand : IImportOrganizationUserCommand +public class ImportOrganizationUsersAndGroupsCommand : IImportOrganizationUsersAndGroupsCommand { private readonly IOrganizationRepository _organizationRepository; private readonly IOrganizationUserRepository _organizationUserRepository; @@ -34,7 +34,7 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand private readonly EventSystemUser _EventSystemUser = EventSystemUser.PublicApi; - public ImportOrganizationUserCommand(IOrganizationRepository organizationRepository, + public ImportOrganizationUsersAndGroupsCommand(IOrganizationRepository organizationRepository, IOrganizationUserRepository organizationUserRepository, IPaymentService paymentService, IGroupRepository groupRepository, @@ -95,7 +95,7 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand await OverwriteExisting(events, importUserData); } - await Update(importedUsers, importUserData); + await UpdateExistingUsers(importedUsers, importUserData); await AddNewUsers(organization, importedUsers, importUserData); @@ -144,7 +144,7 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand /// /// List of imported organization users. /// Data containing existing and imported users, along with mapping dictionaries. - private async Task Update(IEnumerable importedUsers, OrganizationUserImportData importUserData) + private async Task UpdateExistingUsers(IEnumerable importedUsers, OrganizationUserImportData importUserData) { if (!importedUsers.Any()) { @@ -160,27 +160,24 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand var importedUsersEmailsDict = importedUsers.ToDictionary(u => u.Email); // Determine which users to update. - var userDetailsToUpdate = existingUsersEmailsDict.Keys.Intersect(importedUsersEmailsDict.Keys).ToList(); - var userIdsToUpdate = importUserData.ExistingUsers - .Where(u => userDetailsToUpdate.Contains(u.Email)) - .Select(u => u.Id) - .ToList(); + var userEmailsToUpdate = existingUsersEmailsDict.Keys.Intersect(importedUsersEmailsDict.Keys).ToList(); + var userIdsToUpdate = userEmailsToUpdate.Select(e => existingUsersEmailsDict[e].Id).ToList(); var organizationUsers = (await _organizationUserRepository.GetManyAsync(userIdsToUpdate)).ToDictionary(u => u.Id); - foreach (var userEmail in userDetailsToUpdate) + foreach (var userEmail in userEmailsToUpdate) { // verify userEmail has an associated OrganizationUser existingUsersEmailsDict.TryGetValue(userEmail, out var existingUser); organizationUsers.TryGetValue(existingUser!.Id, out var organizationUser); - importedUsersEmailsDict.TryGetValue(userEmail, out var user); + importedUsersEmailsDict.TryGetValue(userEmail, out var importedUser); - if (organizationUser is null || user is null) + if (organizationUser is null || importedUser is null) { continue; } - organizationUser.ExternalId = user.ExternalId; + organizationUser.ExternalId = importedUser.ExternalId; updateUsers.Add(organizationUser); importUserData.ExistingExternalUsersIdDict.Add(organizationUser.ExternalId, organizationUser.Id); } @@ -302,11 +299,8 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand /// The organization into which groups are being imported. /// A collection of groups to be imported. /// Data containing information about existing and imported users. - private async Task ImportGroups(Organization organization, - IEnumerable importedGroups, - OrganizationUserImportData importUserData) + private async Task ImportGroups(Organization organization, IEnumerable importedGroups, OrganizationUserImportData importUserData) { - if (!importedGroups.Any()) { return; @@ -314,7 +308,7 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand if (!organization.UseGroups) { - throw new BadRequestException("Organization cannot use importedGroups."); + throw new BadRequestException("Organization cannot use groups."); } var existingGroups = await _groupRepository.GetManyByOrganizationIdAsync(organization.Id); @@ -332,9 +326,11 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand /// Data containing information about existing and imported users. private async Task SaveNewGroups(OrganizationGroupImportData importGroupData, OrganizationUserImportData importUserData) { + var existingExternalGroupsDict = importGroupData.ExistingExternalGroups.ToDictionary(g => g.ExternalId!); var newGroups = importGroupData.Groups - .Where(g => !importGroupData.ExistingExternalGroups.ToDictionary(g => g.ExternalId!).ContainsKey(g.Group.ExternalId!)) - .Select(g => g.Group).ToList()!; + .Where(g => !existingExternalGroupsDict.ContainsKey(g.Group.ExternalId!)) + .Select(g => g.Group) + .ToList()!; var savedGroups = new List(); foreach (var group in newGroups) diff --git a/src/Core/AdminConsole/Models/Data/Organizations/OrganizationGroupImportData.cs b/src/Core/AdminConsole/OrganizationFeatures/Import/OrganizationGroupImportData.cs similarity index 54% rename from src/Core/AdminConsole/Models/Data/Organizations/OrganizationGroupImportData.cs rename to src/Core/AdminConsole/OrganizationFeatures/Import/OrganizationGroupImportData.cs index fb7c21e00a..6f49cb82e6 100644 --- a/src/Core/AdminConsole/Models/Data/Organizations/OrganizationGroupImportData.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Import/OrganizationGroupImportData.cs @@ -1,19 +1,40 @@ -using Bit.Core.AdminConsole.Entities; +#nullable enable + +using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Models.Business; namespace Bit.Core.Models.Data.Organizations; +/// +/// Represents the data required to import organization groups, +/// including newly imported groups and existing groups within the organization. +/// public class OrganizationGroupImportData { + /// + /// The collection of groups that are being imported. + /// public readonly IEnumerable Groups; + + /// + /// Collection of groups that already exist in the organization. + /// public readonly ICollection ExistingGroups; + + /// + /// Existing groups with ExternalId set. + /// public readonly IEnumerable ExistingExternalGroups; + + /// + /// Mapping of imported groups keyed by their ExternalId. + /// public readonly IDictionary GroupsDict; public OrganizationGroupImportData(IEnumerable groups, ICollection existingGroups) { Groups = groups; - GroupsDict = groups.ToDictionary(g => g.Group.ExternalId); + GroupsDict = groups.ToDictionary(g => g.Group.ExternalId!); ExistingGroups = existingGroups; ExistingExternalGroups = existingGroups.Where(u => !string.IsNullOrWhiteSpace(u.ExternalId)).ToList(); } diff --git a/src/Core/AdminConsole/Models/Data/Organizations/OrganizationUsers/OrganizationUserImportData.cs b/src/Core/AdminConsole/OrganizationFeatures/Import/OrganizationUserImportData.cs similarity index 87% rename from src/Core/AdminConsole/Models/Data/Organizations/OrganizationUsers/OrganizationUserImportData.cs rename to src/Core/AdminConsole/OrganizationFeatures/Import/OrganizationUserImportData.cs index 117e34d637..6575afe842 100644 --- a/src/Core/AdminConsole/Models/Data/Organizations/OrganizationUsers/OrganizationUserImportData.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Import/OrganizationUserImportData.cs @@ -1,4 +1,6 @@ -using Bit.Core.Models.Business; +#nullable enable + +using Bit.Core.Models.Business; namespace Bit.Core.Models.Data.Organizations.OrganizationUsers; public class OrganizationUserImportData @@ -8,11 +10,11 @@ public class OrganizationUserImportData /// public readonly HashSet ImportedExternalIds; /// - /// Exising organization users details + /// All existing OrganizationUsers for the organization /// public readonly ICollection ExistingUsers; /// - /// List of ExternalIds belonging to existing organization Users + /// Existing OrganizationUsers with ExternalIds set. /// public readonly IEnumerable ExistingExternalUsers; /// diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommandTests.cs index 4abbd2b8af..bd407477b9 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommandTests.cs @@ -1,5 +1,4 @@ using Bit.Core.AdminConsole.Models.Business; -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers.Models; using Bit.Core.AdminConsole.Utilities.Commands;