diff --git a/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs b/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs index 3e9e644220..ad3b6b60d4 100644 --- a/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs +++ b/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs @@ -4,7 +4,6 @@ using Bit.Api.Models.Public.Response; using Bit.Core; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Context; -using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Services; using Bit.Core.Settings; @@ -55,26 +54,25 @@ public class OrganizationController : Controller throw new BadRequestException("You cannot import this much data at once."); } - if (_featureService.IsEnabled(FeatureFlagKeys.ScimInviteUserOptimization)) + string r = ""; + + r = 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()); + + + if (_featureService.IsEnabled(FeatureFlagKeys.ImportAsyncRefactor)) { - 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()); + r = "success"; } 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(), - EventSystemUser.PublicApi - ); + r = "fail"; } - return new OkResult(); + + return new OkObjectResult(r); } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/Import/IImportOrganizationUserAndGroupsCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Import/IImportOrganizationUserAndGroupsCommand.cs index b74da0a2e8..20debd081c 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Import/IImportOrganizationUserAndGroupsCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Import/IImportOrganizationUserAndGroupsCommand.cs @@ -7,7 +7,7 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interface public interface IImportOrganizationUsersAndGroupsCommand { - Task ImportAsync(Guid organizationId, + Task ImportAsync(Guid organizationId, IEnumerable groups, IEnumerable newUsers, IEnumerable removeUserExternalIds, diff --git a/src/Core/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommand.cs index 7e44815bbe..0dcfbd6518 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommand.cs @@ -4,15 +4,14 @@ 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; -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers.Models; using Bit.Core.AdminConsole.Repositories; -using Bit.Core.AdminConsole.Utilities.Commands; using Bit.Core.Billing.Pricing; using Bit.Core.Context; 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.Repositories; @@ -67,7 +66,7 @@ public class ImportOrganizationUsersAndGroupsCommand : IImportOrganizationUsersA /// who are not included in the current import. /// Thrown if the organization does not exist. /// Thrown if the organization is not configured to use directory syncing. - public async Task ImportAsync(Guid organizationId, + public async Task ImportAsync(Guid organizationId, IEnumerable importedGroups, IEnumerable importedUsers, IEnumerable removeUserExternalIds, @@ -102,6 +101,8 @@ public class ImportOrganizationUsersAndGroupsCommand : IImportOrganizationUsersA await ImportGroups(organization, importedGroups, importUserData); await _eventService.LogOrganizationUserEventsAsync(events.Select(e => (e.ou, e.e, _EventSystemUser, e.d))); + + return "success"; } /// @@ -196,73 +197,52 @@ public class ImportOrganizationUsersAndGroupsCommand : IImportOrganizationUsersA IEnumerable importedUsers, OrganizationUserImportData importUserData) { - var userInvites = new List(); + // Determine which users are already in the organization + var existingUsersSet = new HashSet(importUserData.ExistingExternalUsersIdDict.Keys).ToList(); + var usersToAdd = importUserData.ImportedExternalIds.Except(existingUsersSet).ToList(); + + var seatsAvailable = int.MaxValue; + var enoughSeatsAvailable = true; + + if (organization.Seats.HasValue) + { + seatsAvailable = organization.Seats.Value - await _organizationUserRepository.GetOccupiedSeatCountByOrganizationIdAsync(organization.Id); + enoughSeatsAvailable = seatsAvailable >= usersToAdd.Count(); + } + + + var userInvites = new List<(OrganizationUserInvite, string)>(); + var hasStandaloneSecretsManager = await _paymentService.HasSecretsManagerStandalone(organization); foreach (var user in importedUsers) { - userInvites.Add(new OrganizationUserInviteCommandModel(user.Email, user.ExternalId)); + 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 commandResult = await InviteUsersAsync(userInvites, organization); - - switch (commandResult) + var invitedUsers = await _organizationService.InviteUsersAsync(organization.Id, Guid.Empty, _EventSystemUser, userInvites); + foreach (var invitedUser in invitedUsers) { - case Success result: - foreach (var u in result.Value.InvitedUsers) - { - importUserData.ExistingExternalUsersIdDict.Add(u.ExternalId!, u.Id); - } - break; - case Failure failure: - throw new BadRequestException(failure.Error.Message); - default: - throw new InvalidOperationException($"Unhandled commandResult type: {commandResult.GetType().Name}"); - } - } - - /// - /// Sends the user invites through the InviteOrganizationUserCommand - /// The list of organization user invites command models to be used for inviting users - /// The organization to which users are being invited - /// - private async Task> InviteUsersAsync(List invites, Organization organization) - { - var plan = await _pricingClient.GetPlanOrThrow(organization.PlanType); - var inviteOrganization = new InviteOrganization(organization, plan); - var request = new InviteOrganizationUsersRequest(invites.ToArray(), inviteOrganization, Guid.Empty, DateTimeOffset.UtcNow); - - return await _inviteOrganizationUsersCommand.InviteImportedOrganizationUsersAsync(request); - } - - /// - /// Creates an InviteOrganizationUsersRequest for the provided invites and sends the request via the InviteOrganizationUsersCommand. - /// - private async Task vNextInviteUsersAsync(List userInvites, - Organization organization, - OrganizationUserImportData importUserData) - { - var plan = await _pricingClient.GetPlanOrThrow(organization.PlanType); - var inviteOrganization = new InviteOrganization(organization, plan); - var request = new InviteOrganizationUsersRequest(userInvites.ToArray(), inviteOrganization, Guid.Empty, DateTimeOffset.UtcNow); - var commandResult = await _inviteOrganizationUsersCommand.InviteImportedOrganizationUsersAsync(request); - - switch (commandResult) - { - case Success result: - foreach (var u in result.Value.InvitedUsers) - { - if (u.ExternalId is null) - { - continue; - } - - importUserData.ExistingExternalUsersIdDict.Add(u.ExternalId, u.Id); - } - break; - case Failure failure: - throw new BadRequestException(failure.Error.Message); - default: - throw new InvalidOperationException($"Unhandled commandResult type: {commandResult.GetType().Name}"); + importUserData.ExistingExternalUsersIdDict.TryAdd(invitedUser.ExternalId!, invitedUser.Id); } } diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index e6a822452a..8bc57ce5e6 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -112,6 +112,7 @@ public static class FeatureFlagKeys public const string EventBasedOrganizationIntegrations = "event-based-organization-integrations"; public const string OptimizeNestedTraverseTypescript = "pm-21695-optimize-nested-traverse-typescript"; public const string SeparateCustomRolePermissions = "pm-19917-separate-custom-role-permissions"; + public const string ImportAsyncRefactor = "pm-22583-refactor-import-async"; /* Auth Team */ public const string PM9112DeviceApprovalPersistence = "pm-9112-device-approval-persistence"; diff --git a/test/Api.IntegrationTest/AdminConsole/Import/ImportOrganizationUsersAndGroupsCommandTests.cs b/test/Api.IntegrationTest/AdminConsole/Import/ImportOrganizationUsersAndGroupsCommandTests.cs new file mode 100644 index 0000000000..167df3ff9b --- /dev/null +++ b/test/Api.IntegrationTest/AdminConsole/Import/ImportOrganizationUsersAndGroupsCommandTests.cs @@ -0,0 +1,72 @@ +using Bit.Api.AdminConsole.Public.Models.Request; +using Bit.Api.IntegrationTest.Factories; +using Bit.Api.IntegrationTest.Helpers; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Billing.Enums; +using Bit.Core.Enums; +using Xunit; + +public class ImportOrganizationUsersAndGroupsCommandTests : IClassFixture, IAsyncLifetime +{ + private readonly HttpClient _client; + private readonly ApiApplicationFactory _factory; + private readonly LoginHelper _loginHelper; + private Organization _organization = null!; + private string _ownerEmail = null!; + + public ImportOrganizationUsersAndGroupsCommandTests(ApiApplicationFactory factory) + { + _factory = factory; + _factory.UpdateConfiguration("globalSettings:launchDarkly:flagValues:pm-22583-refactor-import-async", + "true"); + _client = _factory.CreateClient(); + _loginHelper = new LoginHelper(_factory, _client); + } + + public async Task InitializeAsync() + { + // Create the owner account + _ownerEmail = $"integration-test{Guid.NewGuid()}@bitwarden.com"; + await _factory.LoginWithNewAccount(_ownerEmail); + + // Create the organization + (_organization, _) = await OrganizationTestHelpers.SignUpAsync(_factory, plan: PlanType.EnterpriseAnnually2023, + ownerEmail: _ownerEmail, passwordManagerSeats: 10, paymentMethod: PaymentMethodType.Card); + + // Authorize with the organization api key + await _loginHelper.LoginWithOrganizationApiKeyAsync(_organization.Id); + } + + public Task DisposeAsync() + { + _client.Dispose(); + return Task.CompletedTask; + } + + [Fact] + public async Task Import_Existing_User_Success() + { + var (email, orgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory, _organization.Id, + OrganizationUserType.User); + + var request = new OrganizationImportRequestModel(); + request.LargeImport = false; + request.OverwriteExisting = false; + request.Groups = []; + request.Members = [ + new OrganizationImportRequestModel.OrganizationImportMemberRequestModel + { + Email = email, + ExternalId = Guid.NewGuid().ToString(), + Deleted = false + } + ]; + + var response = await _client.PostAsync($"/public/organization/import", JsonContent.Create(request)); + var result = await response.Content.ReadAsStringAsync(); + + Assert.Equal("success", result); + //Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + } +} diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommandTests.cs index 75afd5b971..b26be9738d 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommandTests.cs @@ -1,13 +1,11 @@ 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; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Models.Business; +using Bit.Core.Models.Data; using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Repositories; using Bit.Core.Services; @@ -32,36 +30,46 @@ public class ImportOrganizationUsersAndGroupsCommandTests SutProvider sutProvider, Organization org, List existingUsers, - List newUsers, + List importedUsers, List newGroups) { - SetupOrganizationConfigForImport(sutProvider, org, existingUsers, newUsers); + SetupOrganizationConfigForImport(sutProvider, org, existingUsers, importedUsers); - newUsers.Add(new ImportedOrganizationUser + var orgUsers = new List(); + + // fix mocked email format, mock OrganizationUsers. + foreach (var u in importedUsers) + { + u.Email += "@bitwardentest.com"; + orgUsers.Add(new OrganizationUser { Email = u.Email, ExternalId = u.ExternalId }); + } + + importedUsers.Add(new ImportedOrganizationUser { Email = existingUsers.First().Email, ExternalId = existingUsers.First().ExternalId }); - foreach (var u in newUsers) - { - u.Email += "@bitwardentest.com"; - } existingUsers.First().Type = OrganizationUserType.Owner; sutProvider.GetDependency().GetByIdAsync(org.Id).Returns(org); - sutProvider.GetDependency().HasSecretsManagerStandalone(org).Returns(false); + + var organizationUserRepository = sutProvider.GetDependency(); + SetupOrgUserRepositoryCreateManyAsyncMock(organizationUserRepository); + + sutProvider.GetDependency().HasSecretsManagerStandalone(org).Returns(true); sutProvider.GetDependency().GetManyDetailsByOrganizationAsync(org.Id).Returns(existingUsers); sutProvider.GetDependency().GetCountByOrganizationIdAsync(org.Id).Returns(existingUsers.Count); sutProvider.GetDependency().ManageUsers(org.Id).Returns(true); - sutProvider.GetDependency().InviteImportedOrganizationUsersAsync(Arg.Any()) - .Returns(new Success(new InviteOrganizationUsersResponse(org.Id))); + sutProvider.GetDependency().InviteUsersAsync(org.Id, Guid.Empty, EventSystemUser.PublicApi, + Arg.Any>()) + .Returns(orgUsers); - await sutProvider.Sut.ImportAsync(org.Id, newGroups, newUsers, new List(), false); + await sutProvider.Sut.ImportAsync(org.Id, newGroups, importedUsers, new List(), false); + + var expectedNewUsersCount = importedUsers.Count - 1; - await sutProvider.GetDependency().Received(1) - .InviteImportedOrganizationUsersAsync(Arg.Any()); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .UpsertAsync(default); await sutProvider.GetDependency().Received(1) @@ -69,6 +77,11 @@ public class ImportOrganizationUsersAndGroupsCommandTests await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .CreateAsync(default); + // Send Invites + await sutProvider.GetDependency().Received(1). + InviteUsersAsync(org.Id, Guid.Empty, EventSystemUser.PublicApi, + Arg.Is>(invites => invites.Count() == expectedNewUsersCount)); + // Send events await sutProvider.GetDependency().Received(1) .LogOrganizationUserEventsAsync(Arg.Any>()); @@ -79,36 +92,55 @@ public class ImportOrganizationUsersAndGroupsCommandTests SutProvider sutProvider, Organization org, List existingUsers, - List newUsers, + List importedUsers, List newGroups) { - SetupOrganizationConfigForImport(sutProvider, org, existingUsers, newUsers); + SetupOrganizationConfigForImport(sutProvider, org, existingUsers, importedUsers); + var orgUsers = new List(); var reInvitedUser = existingUsers.First(); - reInvitedUser.ExternalId = null; + // Existing user has no external ID. This will make the SUT call UpsertManyAsync + reInvitedUser.ExternalId = ""; + + // Mock an existing org user for this "existing" user + var reInvitedOrgUser = new OrganizationUser { Email = reInvitedUser.Email, Id = reInvitedUser.Id }; + + // fix email formatting, mock orgUsers to be returned foreach (var u in existingUsers) { u.Email += "@bitwardentest.com"; + orgUsers.Add(new OrganizationUser { Email = u.Email, ExternalId = u.ExternalId }); } - foreach (var u in newUsers) + foreach (var u in importedUsers) { u.Email += "@bitwardentest.com"; + orgUsers.Add(new OrganizationUser { Email = u.Email, ExternalId = u.ExternalId }); } - newUsers.Add(new ImportedOrganizationUser + // add the existing user to be re-imported + importedUsers.Add(new ImportedOrganizationUser { Email = reInvitedUser.Email, ExternalId = reInvitedUser.Email, }); + var expectedNewUsersCount = importedUsers.Count - 1; + sutProvider.GetDependency().GetByIdAsync(org.Id).Returns(org); + + var organizationUserRepository = sutProvider.GetDependency(); + SetupOrgUserRepositoryCreateManyAsyncMock(organizationUserRepository); + + sutProvider.GetDependency().GetManyAsync(Arg.Any>()) + .Returns(new List([reInvitedOrgUser])); sutProvider.GetDependency().GetManyDetailsByOrganizationAsync(org.Id).Returns(existingUsers); sutProvider.GetDependency().GetCountByOrganizationIdAsync(org.Id).Returns(existingUsers.Count); - sutProvider.GetDependency().GetManyAsync(Arg.Any>()).Returns(new List { new OrganizationUser { Id = reInvitedUser.Id } }); - sutProvider.GetDependency().InviteImportedOrganizationUsersAsync(Arg.Any()) - .Returns(new Success(new InviteOrganizationUsersResponse(org.Id))); - await sutProvider.Sut.ImportAsync(org.Id, newGroups, newUsers, new List(), false); + sutProvider.GetDependency().InviteUsersAsync(org.Id, Guid.Empty, EventSystemUser.PublicApi, + Arg.Any>()) + .Returns(orgUsers); + + await sutProvider.Sut.ImportAsync(org.Id, newGroups, importedUsers, new List(), false); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .UpsertAsync(default); @@ -119,28 +151,55 @@ public class ImportOrganizationUsersAndGroupsCommandTests // Upserted existing user await sutProvider.GetDependency().Received(1) - .UpsertManyAsync(Arg.Is>(users => users.Count() == 1)); + .UpsertManyAsync(Arg.Is>(users => users.Count() == 1 && users.First() == reInvitedOrgUser)); - await sutProvider.GetDependency().Received(1) - .InviteImportedOrganizationUsersAsync(Arg.Any()); + // Send Invites + await sutProvider.GetDependency().Received(1). + InviteUsersAsync(org.Id, Guid.Empty, EventSystemUser.PublicApi, + Arg.Is>(invites => invites.Count() == expectedNewUsersCount)); // Send events await sutProvider.GetDependency().Received(1) .LogOrganizationUserEventsAsync(Arg.Any>()); - } private void SetupOrganizationConfigForImport( SutProvider sutProvider, Organization org, List existingUsers, - List newUsers) + List importedUsers) { // 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; + org.Seats = importedUsers.Count + existingUsers.Count + 1; + } + + // Must set real guids in order for dictionary of guids to not throw aggregate exceptions + private void SetupOrgUserRepositoryCreateManyAsyncMock(IOrganizationUserRepository organizationUserRepository) + { + organizationUserRepository.CreateManyAsync(Arg.Any>()).Returns( + info => + { + var orgUsers = info.Arg>(); + foreach (var orgUser in orgUsers) + { + orgUser.Id = Guid.NewGuid(); + } + + return Task.FromResult>(orgUsers.Select(u => u.Id).ToList()); + } + ); + + organizationUserRepository.CreateAsync(Arg.Any(), Arg.Any>()).Returns( + info => + { + var orgUser = info.Arg(); + orgUser.Id = Guid.NewGuid(); + return Task.FromResult(orgUser.Id); + } + ); } }