From cf94438150d114b06f030f5d215bb8c4343669fe Mon Sep 17 00:00:00 2001 From: Brandon Treston Date: Wed, 20 Aug 2025 11:10:06 -0400 Subject: [PATCH] [PM-22586/PM-22587] Remove feature flagged logic (#6194) * remove feature flagged logic * remove feature flag * remove OrganizationService.ImportAsync and tests * remove unused function --- .../Controllers/OrganizationController.cs | 19 +- .../Services/IOrganizationService.cs | 4 - .../Implementations/OrganizationService.cs | 222 ------------------ src/Core/Constants.cs | 1 - ...tOrganizationUsersAndGroupsCommandTests.cs | 2 - .../Services/OrganizationServiceTests.cs | 132 ----------- 6 files changed, 3 insertions(+), 377 deletions(-) diff --git a/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs b/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs index 18afa10ac0..5531204033 100644 --- a/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs +++ b/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs @@ -4,7 +4,6 @@ using System.Net; using Bit.Api.AdminConsole.Public.Models.Request; using Bit.Api.Models.Public.Response; -using Bit.Core; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Context; using Bit.Core.Exceptions; @@ -57,25 +56,13 @@ public class OrganizationController : Controller throw new BadRequestException("You cannot import this much data at once."); } - if (_featureService.IsEnabled(FeatureFlagKeys.ImportAsyncRefactor)) - { - await _importOrganizationUsersAndGroupsCommand.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()), model.Members.Where(u => u.Deleted).Select(u => u.ExternalId), - model.OverwriteExisting.GetValueOrDefault()); - } - else - { - await _organizationService.ImportAsync( - _currentContext.OrganizationId.Value, - model.Groups.Select(g => g.ToImportedGroup(_currentContext.OrganizationId.Value)), - model.Members.Where(u => !u.Deleted).Select(u => u.ToImportedOrganizationUser()), - model.Members.Where(u => u.Deleted).Select(u => u.ExternalId), - model.OverwriteExisting.GetValueOrDefault(), - Core.Enums.EventSystemUser.PublicApi); - } + model.OverwriteExisting.GetValueOrDefault() + ); return new OkResult(); } diff --git a/src/Core/AdminConsole/Services/IOrganizationService.cs b/src/Core/AdminConsole/Services/IOrganizationService.cs index e54e6fee12..8c47ae049c 100644 --- a/src/Core/AdminConsole/Services/IOrganizationService.cs +++ b/src/Core/AdminConsole/Services/IOrganizationService.cs @@ -2,7 +2,6 @@ #nullable disable using Bit.Core.AdminConsole.Entities; -using Bit.Core.AdminConsole.Models.Business; using Bit.Core.Auth.Enums; using Bit.Core.Entities; using Bit.Core.Enums; @@ -29,9 +28,6 @@ public interface IOrganizationService IEnumerable<(OrganizationUserInvite invite, string externalId)> invites); Task>> ResendInvitesAsync(Guid organizationId, Guid? invitingUserId, IEnumerable organizationUsersId); Task UpdateUserResetPasswordEnrollmentAsync(Guid organizationId, Guid userId, string resetPasswordKey, Guid? callingUserId); - Task ImportAsync(Guid organizationId, IEnumerable groups, - IEnumerable newUsers, IEnumerable removeUserExternalIds, - bool overwriteExisting, EventSystemUser eventSystemUser); Task DeleteSsoUserAsync(Guid userId, Guid? organizationId); Task ReplaceAndUpdateCacheAsync(Organization org, EventType? orgEvent = null); Task<(bool canScale, string failureReason)> CanScaleAsync(Organization organization, int seatsToAdd); diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 575cdb0230..f418737508 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -5,7 +5,6 @@ using System.Text.Json; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Enums.Provider; -using Bit.Core.AdminConsole.Models.Business; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers; @@ -27,7 +26,6 @@ using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Business; using Bit.Core.Models.Data; -using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.OrganizationFeatures.OrganizationSubscriptions.Interface; using Bit.Core.Platform.Push; using Bit.Core.Repositories; @@ -923,214 +921,6 @@ public class OrganizationService : IOrganizationService : EventType.OrganizationUser_ResetPassword_Withdraw); } - public async Task ImportAsync(Guid organizationId, - IEnumerable groups, - IEnumerable newUsers, - IEnumerable removeUserExternalIds, - bool overwriteExisting, - EventSystemUser eventSystemUser - ) - { - var organization = await GetOrgById(organizationId); - if (organization == null) - { - throw new NotFoundException(); - } - - if (!organization.UseDirectory) - { - throw new BadRequestException("Organization cannot use directory syncing."); - } - - var newUsersSet = new HashSet(newUsers?.Select(u => u.ExternalId) ?? new List()); - var existingUsers = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(organizationId); - var existingExternalUsers = existingUsers.Where(u => !string.IsNullOrWhiteSpace(u.ExternalId)).ToList(); - var existingExternalUsersIdDict = existingExternalUsers.ToDictionary(u => u.ExternalId, u => u.Id); - - // Users - - var events = new List<(OrganizationUserUserDetails ou, EventType e, DateTime? d)>(); - - // Remove Users - if (removeUserExternalIds?.Any() ?? false) - { - var existingUsersDict = existingExternalUsers.ToDictionary(u => u.ExternalId); - var removeUsersSet = new HashSet(removeUserExternalIds) - .Except(newUsersSet) - .Where(u => existingUsersDict.TryGetValue(u, out var existingUser) && - existingUser.Type != OrganizationUserType.Owner) - .Select(u => existingUsersDict[u]); - - await _organizationUserRepository.DeleteManyAsync(removeUsersSet.Select(u => u.Id)); - events.AddRange(removeUsersSet.Select(u => ( - u, - EventType.OrganizationUser_Removed, - (DateTime?)DateTime.UtcNow - )) - ); - } - - if (overwriteExisting) - { - // Remove existing external users that are not in new user set - var usersToDelete = existingExternalUsers.Where(u => - u.Type != OrganizationUserType.Owner && - !newUsersSet.Contains(u.ExternalId) && - existingExternalUsersIdDict.ContainsKey(u.ExternalId)); - await _organizationUserRepository.DeleteManyAsync(usersToDelete.Select(u => u.Id)); - events.AddRange(usersToDelete.Select(u => ( - u, - EventType.OrganizationUser_Removed, - (DateTime?)DateTime.UtcNow - )) - ); - foreach (var deletedUser in usersToDelete) - { - existingExternalUsersIdDict.Remove(deletedUser.ExternalId); - } - } - - if (newUsers?.Any() ?? false) - { - // Marry existing users - var existingUsersEmailsDict = existingUsers - .Where(u => string.IsNullOrWhiteSpace(u.ExternalId)) - .ToDictionary(u => u.Email); - var newUsersEmailsDict = newUsers.ToDictionary(u => u.Email); - var usersToAttach = existingUsersEmailsDict.Keys.Intersect(newUsersEmailsDict.Keys).ToList(); - var usersToUpsert = new List(); - foreach (var user in usersToAttach) - { - var orgUserDetails = existingUsersEmailsDict[user]; - var orgUser = await _organizationUserRepository.GetByIdAsync(orgUserDetails.Id); - if (orgUser != null) - { - orgUser.ExternalId = newUsersEmailsDict[user].ExternalId; - usersToUpsert.Add(orgUser); - existingExternalUsersIdDict.Add(orgUser.ExternalId, orgUser.Id); - } - } - - await _organizationUserRepository.UpsertManyAsync(usersToUpsert); - - // Add new users - var existingUsersSet = new HashSet(existingExternalUsersIdDict.Keys); - var usersToAdd = newUsersSet.Except(existingUsersSet).ToList(); - - var seatsAvailable = int.MaxValue; - var enoughSeatsAvailable = true; - if (organization.Seats.HasValue) - { - var seatCounts = - await _organizationRepository.GetOccupiedSeatCountByOrganizationIdAsync(organization.Id); - seatsAvailable = organization.Seats.Value - seatCounts.Total; - enoughSeatsAvailable = seatsAvailable >= usersToAdd.Count; - } - - var hasStandaloneSecretsManager = await _paymentService.HasSecretsManagerStandalone(organization); - - var userInvites = new List<(OrganizationUserInvite, string)>(); - foreach (var user in newUsers) - { - if (!usersToAdd.Contains(user.ExternalId) || string.IsNullOrWhiteSpace(user.Email)) - { - continue; - } - - try - { - var invite = new OrganizationUserInvite - { - Emails = new List { user.Email }, - Type = OrganizationUserType.User, - Collections = new List(), - AccessSecretsManager = hasStandaloneSecretsManager - }; - userInvites.Add((invite, user.ExternalId)); - } - catch (BadRequestException) - { - // Thrown when the user is already invited to the organization - continue; - } - } - - var invitedUsers = await InviteUsersAsync(organizationId, invitingUserId: null, systemUser: eventSystemUser, - userInvites); - foreach (var invitedUser in invitedUsers) - { - existingExternalUsersIdDict.Add(invitedUser.ExternalId, invitedUser.Id); - } - } - - - // Groups - if (groups?.Any() ?? false) - { - if (!organization.UseGroups) - { - throw new BadRequestException("Organization cannot use groups."); - } - - var groupsDict = groups.ToDictionary(g => g.Group.ExternalId); - var existingGroups = await _groupRepository.GetManyByOrganizationIdAsync(organizationId); - var existingExternalGroups = existingGroups - .Where(u => !string.IsNullOrWhiteSpace(u.ExternalId)).ToList(); - var existingExternalGroupsDict = existingExternalGroups.ToDictionary(g => g.ExternalId); - - var newGroups = groups - .Where(g => !existingExternalGroupsDict.ContainsKey(g.Group.ExternalId)) - .Select(g => g.Group).ToList(); - - var savedGroups = new List(); - foreach (var group in newGroups) - { - group.CreationDate = group.RevisionDate = DateTime.UtcNow; - - savedGroups.Add(await _groupRepository.CreateAsync(group)); - await UpdateUsersAsync(group, groupsDict[group.ExternalId].ExternalUserIds, - existingExternalUsersIdDict); - } - - await _eventService.LogGroupEventsAsync( - savedGroups.Select(g => (g, EventType.Group_Created, (EventSystemUser?)eventSystemUser, - (DateTime?)DateTime.UtcNow))); - - var updateGroups = existingExternalGroups - .Where(g => groupsDict.ContainsKey(g.ExternalId)) - .ToList(); - - if (updateGroups.Any()) - { - var groupUsers = await _groupRepository.GetManyGroupUsersByOrganizationIdAsync(organizationId); - var existingGroupUsers = groupUsers - .GroupBy(gu => gu.GroupId) - .ToDictionary(g => g.Key, g => new HashSet(g.Select(gr => gr.OrganizationUserId))); - - foreach (var group in updateGroups) - { - var updatedGroup = groupsDict[group.ExternalId].Group; - if (group.Name != updatedGroup.Name) - { - group.RevisionDate = DateTime.UtcNow; - group.Name = updatedGroup.Name; - - await _groupRepository.ReplaceAsync(group); - } - - await UpdateUsersAsync(group, groupsDict[group.ExternalId].ExternalUserIds, - existingExternalUsersIdDict, - existingGroupUsers.ContainsKey(group.Id) ? existingGroupUsers[group.Id] : null); - } - - await _eventService.LogGroupEventsAsync( - updateGroups.Select(g => (g, EventType.Group_Updated, (EventSystemUser?)eventSystemUser, - (DateTime?)DateTime.UtcNow))); - } - } - - await _eventService.LogOrganizationUserEventsAsync(events.Select(e => (e.ou, e.e, eventSystemUser, e.d))); - } public async Task DeleteSsoUserAsync(Guid userId, Guid? organizationId) { @@ -1147,18 +937,6 @@ public class OrganizationService : IOrganizationService } } - private async Task UpdateUsersAsync(Group group, HashSet groupUsers, - Dictionary existingUsersIdDict, HashSet existingUsers = null) - { - var availableUsers = groupUsers.Intersect(existingUsersIdDict.Keys); - var users = new HashSet(availableUsers.Select(u => existingUsersIdDict[u])); - if (existingUsers != null && existingUsers.Count == users.Count && users.SetEquals(existingUsers)) - { - return; - } - - await _groupRepository.UpdateUsersAsync(group.Id, users); - } public async Task ReplaceAndUpdateCacheAsync(Organization org, EventType? orgEvent = null) { diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 81b7c59259..2f66df9d22 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -113,7 +113,6 @@ public static class FeatureFlagKeys public const string ScimInviteUserOptimization = "pm-16811-optimize-invite-user-flow-to-fail-fast"; public const string EventBasedOrganizationIntegrations = "event-based-organization-integrations"; public const string SeparateCustomRolePermissions = "pm-19917-separate-custom-role-permissions"; - public const string ImportAsyncRefactor = "pm-22583-refactor-import-async"; public const string CreateDefaultLocation = "pm-19467-create-default-location"; public const string DirectoryConnectorPreventUserRemoval = "pm-24592-directory-connector-prevent-user-removal"; diff --git a/test/Api.IntegrationTest/AdminConsole/Import/ImportOrganizationUsersAndGroupsCommandTests.cs b/test/Api.IntegrationTest/AdminConsole/Import/ImportOrganizationUsersAndGroupsCommandTests.cs index f04fb62c1a..2aea7ac4cd 100644 --- a/test/Api.IntegrationTest/AdminConsole/Import/ImportOrganizationUsersAndGroupsCommandTests.cs +++ b/test/Api.IntegrationTest/AdminConsole/Import/ImportOrganizationUsersAndGroupsCommandTests.cs @@ -28,8 +28,6 @@ public class ImportOrganizationUsersAndGroupsCommandTests : IClassFixture { - featureService.IsEnabled(FeatureFlagKeys.ImportAsyncRefactor) - .Returns(true); featureService.IsEnabled(FeatureFlagKeys.DirectoryConnectorPreventUserRemoval) .Returns(true); }); diff --git a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs index f619fed278..e3f26a898d 100644 --- a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs @@ -42,139 +42,7 @@ public class OrganizationServiceTests { private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory = new FakeDataProtectorTokenFactory(); - [Theory, PaidOrganizationCustomize, BitAutoData] - public async Task OrgImportCreateNewUsers(SutProvider sutProvider, Organization org, List existingUsers, List newUsers) - { - // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order to avoid resetting mocks - sutProvider.SetDependency(_orgUserInviteTokenDataFactory, "orgUserInviteTokenDataFactory"); - sutProvider.Create(); - org.UseDirectory = true; - org.Seats = 10; - newUsers.Add(new ImportedOrganizationUser - { - Email = existingUsers.First().Email, - ExternalId = existingUsers.First().ExternalId - }); - var expectedNewUsersCount = newUsers.Count - 1; - - existingUsers.First().Type = OrganizationUserType.Owner; - - sutProvider.GetDependency().GetByIdAsync(org.Id).Returns(org); - sutProvider.GetDependency() - .GetOccupiedSeatCountByOrganizationIdAsync(org.Id).Returns(new OrganizationSeatCounts - { - Sponsored = 0, - Users = 1 - }); - var organizationUserRepository = sutProvider.GetDependency(); - SetupOrgUserRepositoryCreateManyAsyncMock(organizationUserRepository); - - organizationUserRepository.GetManyDetailsByOrganizationAsync(org.Id) - .Returns(existingUsers); - organizationUserRepository.GetCountByOrganizationIdAsync(org.Id) - .Returns(existingUsers.Count); - sutProvider.GetDependency() - .HasConfirmedOwnersExceptAsync(org.Id, Arg.Any>()) - .Returns(true); - sutProvider.GetDependency().ManageUsers(org.Id).Returns(true); - - - await sutProvider.Sut.ImportAsync(org.Id, null, newUsers, null, false, EventSystemUser.PublicApi); - - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - await sutProvider.GetDependency().Received(1) - .UpsertManyAsync(Arg.Is>(users => !users.Any())); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() - .CreateAsync(default); - - // Create new users - await sutProvider.GetDependency().Received(1) - .CreateManyAsync(Arg.Is>(users => users.Count() == expectedNewUsersCount)); - - await sutProvider.GetDependency().Received(1) - .SendInvitesAsync( - Arg.Is( - info => info.Users.Length == expectedNewUsersCount && - info.Organization == org)); - - // Send events - await sutProvider.GetDependency().Received(1) - .LogOrganizationUserEventsAsync(Arg.Is>(events => - events.Count() == expectedNewUsersCount)); - } - - [Theory, PaidOrganizationCustomize, BitAutoData] - public async Task OrgImportCreateNewUsersAndMarryExistingUser(SutProvider sutProvider, Organization org, List existingUsers, - List newUsers) - { - // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order to avoid resetting mocks - sutProvider.SetDependency(_orgUserInviteTokenDataFactory, "orgUserInviteTokenDataFactory"); - sutProvider.Create(); - - org.UseDirectory = true; - org.Seats = newUsers.Count + existingUsers.Count + 1; - var reInvitedUser = existingUsers.First(); - reInvitedUser.ExternalId = null; - newUsers.Add(new ImportedOrganizationUser - { - Email = reInvitedUser.Email, - ExternalId = reInvitedUser.Email, - }); - var expectedNewUsersCount = newUsers.Count - 1; - sutProvider.GetDependency() - .GetOccupiedSeatCountByOrganizationIdAsync(org.Id).Returns(new OrganizationSeatCounts - { - Sponsored = 0, - Users = 1 - }); - sutProvider.GetDependency().GetByIdAsync(org.Id).Returns(org); - sutProvider.GetDependency().GetManyDetailsByOrganizationAsync(org.Id) - .Returns(existingUsers); - sutProvider.GetDependency().GetCountByOrganizationIdAsync(org.Id) - .Returns(existingUsers.Count); - sutProvider.GetDependency().GetByIdAsync(reInvitedUser.Id) - .Returns(new OrganizationUser { Id = reInvitedUser.Id }); - - var organizationUserRepository = sutProvider.GetDependency(); - - sutProvider.GetDependency() - .HasConfirmedOwnersExceptAsync(org.Id, Arg.Any>()) - .Returns(true); - - SetupOrgUserRepositoryCreateManyAsyncMock(organizationUserRepository); - - var currentContext = sutProvider.GetDependency(); - currentContext.ManageUsers(org.Id).Returns(true); - - await sutProvider.Sut.ImportAsync(org.Id, null, newUsers, null, false, EventSystemUser.PublicApi); - - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() - .CreateAsync(default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() - .CreateAsync(default, default); - - // Upserted existing user - await sutProvider.GetDependency().Received(1) - .UpsertManyAsync(Arg.Is>(users => users.Count() == 1)); - - // Created and invited new users - await sutProvider.GetDependency().Received(1) - .CreateManyAsync(Arg.Is>(users => users.Count() == expectedNewUsersCount)); - - await sutProvider.GetDependency().Received(1) - .SendInvitesAsync(Arg.Is(request => - request.Users.Length == expectedNewUsersCount && - request.Organization == org)); - - // Sent events - await sutProvider.GetDependency().Received(1) - .LogOrganizationUserEventsAsync(Arg.Is>(events => - events.Count(e => e.Item2 == EventType.OrganizationUser_Invited) == expectedNewUsersCount)); - } [Theory] [OrganizationInviteCustomize(InviteeUserType = OrganizationUserType.User,