diff --git a/src/Core/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommand.cs index 89288eb4ba..87c6ddea6f 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommand.cs @@ -1,12 +1,7 @@ -#nullable enable - -using Bit.Core.AdminConsole.Entities; +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.Repositories; -using Bit.Core.Billing.Pricing; -using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -17,7 +12,7 @@ using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Repositories; using Bit.Core.Services; -namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; +namespace Bit.Core.AdminConsole.OrganizationFeatures.Import; public class ImportOrganizationUsersAndGroupsCommand : IImportOrganizationUsersAndGroupsCommand { @@ -26,10 +21,8 @@ public class ImportOrganizationUsersAndGroupsCommand : IImportOrganizationUsersA private readonly IPaymentService _paymentService; private readonly IGroupRepository _groupRepository; private readonly IEventService _eventService; - private readonly ICurrentContext _currentContext; private readonly IOrganizationService _organizationService; - private readonly IInviteOrganizationUsersCommand _inviteOrganizationUsersCommand; - private readonly IPricingClient _pricingClient; + private readonly IFeatureService _featureService; private readonly EventSystemUser _EventSystemUser = EventSystemUser.PublicApi; @@ -38,21 +31,16 @@ public class ImportOrganizationUsersAndGroupsCommand : IImportOrganizationUsersA IPaymentService paymentService, IGroupRepository groupRepository, IEventService eventService, - ICurrentContext currentContext, IOrganizationService organizationService, - IInviteOrganizationUsersCommand inviteOrganizationUsersCommand, - IPricingClient pricingClient - ) + IFeatureService featureService) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; _paymentService = paymentService; _groupRepository = groupRepository; _eventService = eventService; - _currentContext = currentContext; _organizationService = organizationService; - _inviteOrganizationUsersCommand = inviteOrganizationUsersCommand; - _pricingClient = pricingClient; + _featureService = featureService; } /// @@ -243,10 +231,23 @@ public class ImportOrganizationUsersAndGroupsCommand : IImportOrganizationUsersA List<(OrganizationUserUserDetails ou, EventType e, DateTime? d)> events, OrganizationUserImportData importUserData) { - var usersToDelete = importUserData.ExistingExternalUsers.Where(u => - u.Type != OrganizationUserType.Owner && - !importUserData.ImportedExternalIds.Contains(u.ExternalId) && - importUserData.ExistingExternalUsersIdDict.ContainsKey(u.ExternalId)); + var usersToDelete = importUserData.ExistingExternalUsers + .Where(u => + u.Type != OrganizationUserType.Owner && + !importUserData.ImportedExternalIds.Contains(u.ExternalId) && + importUserData.ExistingExternalUsersIdDict.ContainsKey(u.ExternalId)) + .ToList(); + + if (_featureService.IsEnabled(FeatureFlagKeys.DirectoryConnectorPreventUserRemoval) && + usersToDelete.Any(u => !u.HasMasterPassword)) + { + // Removing users without an MP will put their account in an unrecoverable state. + // We allow this during normal syncs for offboarding, but overwriteExisting risks bricking every user in + // the organization, so you don't get to do it here. + throw new BadRequestException( + "Sync failed. To proceed, disable the 'Remove and re-add users during next sync' setting and try again."); + } + await _organizationUserRepository.DeleteManyAsync(usersToDelete.Select(u => u.Id)); events.AddRange(usersToDelete.Select(u => ( u, diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 5e54434a17..be82d9fc21 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -116,6 +116,7 @@ public static class FeatureFlagKeys public const string ImportAsyncRefactor = "pm-22583-refactor-import-async"; public const string CreateDefaultLocation = "pm-19467-create-default-location"; public const string MembersGetEndpointOptimization = "pm-23113-optimize-get-members-endpoint"; + public const string DirectoryConnectorPreventUserRemoval = "pm-24592-directory-connector-prevent-user-removal"; /* Auth Team */ public const string PM9112DeviceApprovalPersistence = "pm-9112-device-approval-persistence"; diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index 998354b9f8..234d6f1a84 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -2,6 +2,7 @@ using Bit.Core.AdminConsole.OrganizationAuth.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.Groups; using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.Import; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationApiKeys; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationApiKeys.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationConnections; diff --git a/test/Api.IntegrationTest/AdminConsole/Import/ImportOrganizationUsersAndGroupsCommandTests.cs b/test/Api.IntegrationTest/AdminConsole/Import/ImportOrganizationUsersAndGroupsCommandTests.cs index 5c29b8b1b7..f04fb62c1a 100644 --- a/test/Api.IntegrationTest/AdminConsole/Import/ImportOrganizationUsersAndGroupsCommandTests.cs +++ b/test/Api.IntegrationTest/AdminConsole/Import/ImportOrganizationUsersAndGroupsCommandTests.cs @@ -26,8 +26,13 @@ public class ImportOrganizationUsersAndGroupsCommandTests : IClassFixture featureService.IsEnabled(FeatureFlagKeys.ImportAsyncRefactor) - .Returns(true)); + => + { + featureService.IsEnabled(FeatureFlagKeys.ImportAsyncRefactor) + .Returns(true); + featureService.IsEnabled(FeatureFlagKeys.DirectoryConnectorPreventUserRemoval) + .Returns(true); + }); _client = _factory.CreateClient(); _loginHelper = new LoginHelper(_factory, _client); } @@ -309,4 +314,29 @@ public class ImportOrganizationUsersAndGroupsCommandTests : IClassFixture(); @@ -78,7 +79,7 @@ public static class OrganizationTestHelpers Key = null, Type = type, Status = userStatusType, - ExternalId = null, + ExternalId = externalId, AccessSecretsManager = accessSecretsManager, Email = userEmail }; @@ -110,7 +111,7 @@ public static class OrganizationTestHelpers await factory.LoginWithNewAccount(email); // Create organizationUser - var organizationUser = await OrganizationTestHelpers.CreateUserAsync(factory, organizationId, email, userType, + var organizationUser = await CreateUserAsync(factory, organizationId, email, userType, permissions: permissions); return (email, organizationUser); @@ -168,4 +169,27 @@ public static class OrganizationTestHelpers await policyRepository.CreateAsync(policy); } + + /// + /// Creates a user account without a Master Password and adds them as a member to the specified organization. + /// + public static async Task<(User User, OrganizationUser OrganizationUser)> CreateUserWithoutMasterPasswordAsync(ApiApplicationFactory factory, string email, Guid organizationId) + { + var userRepository = factory.GetService(); + var user = await userRepository.CreateAsync(new User + { + Email = email, + Culture = "en-US", + SecurityStamp = "D7ZH62BWAZ5R5CASKULCDDIQGKDA2EJ6", + PublicKey = "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwMj7W00xS7H0NWasGn7PfEq8VfH3fa5XuZucsKxLLRAHHZk0xGRZJH2lFIznizv3GpF8vzhHhe9VpmMkrdIa5oWhwHpy+D7Z1QCQxuUXzvMKpa95GOntr89nN/mWKpk6abjgjmDcqFJ0lhDqkKnDfes+d8BBd5oEA8p41/Ykz7OfG7AiktVBpTQFW09MQh1NOvcLxVgiUUVRPwNRKrOeCekWDtOjZhASMETv3kI1ogvhHukOQ3ztDzrxvmwnLQ+cXl1EeD8gQnGDp3QLiJqxPgh2EdmANh4IzjRexoDn6BqhRGqLLIoLAbbkoiNrd6NYujrWW0N8KMMoVEXuJL2g4wIDAQAB", + PrivateKey = "2.Ytudv+Qk3ET9hN8whqpuGg==|ijsFhmjaf1aaT9uz+IPhVTzMS+2W/ldAP8LdT5VyJaFdx4HSdLcWSZvz5xWuuW94zfv1Qh+p3iQIuZOr29G4jcx47rYtz4ssiFtB7Ia552ZeF+cb7uuVg40CIe7ycuJQITk00o8gots+wFnaEvk0Vjgycnqutm0jpeBJ1joWJWqTVgSsYdUGLu7PiJywQ9NgY4+bJXqadlcviS3rhPKJXtiXYJhqJqSw+vI0Yxp96MJ0HcFJk/LG22YJPTvL5kzuDq/Wzj40kj8blQ+ag+xHD4P/KJ/MppEB3OpDw3UoJ50Ek+YB9pOqGxZtvqMEzBDsgh0yoz1O992UnhaUqtJ5e9Bxy3PA6cJsdyn9npduNOreEb8vePCidN2XC+chjJpPFpjms9muHLKgfaTIfpiJA2Tz8E9dvSyhHHTE1mY+xEA7P08BYKN3LNoSGIjdiZuouJ1V/KZvCssDfVG1tli2qpnhTIh4m3rAMhbM8WW3B7wCV8N0MpcJJSvndkVcMgRbgWcbivLeXuKdE/K98n01RvOLSJyslhLGCGEQQKw6N3HQ2iELfv84YQZi2fjDK+OqAmXDq1pNcjKX2I8dqBwl31tPC8qSZiWnfinwLdqQTvSQjOIyAHb4sSjAwgdMbCRzUTChRr09l+PAZqGWdMC5N2Bw+bA8WP0l2Wdxuv9Abxl3F7xGeAA9Rw9PU5wGKujaMRmO4V9MFjNyyCcw4D9pzKMW6OUKsHsHE7tsG7KskCzksHzrZGawAt0S41BYQA/JwePCrD3F6dM92anlC1LfA00KJb0tmFdU0yJNmJfR+S78yn8yM6wDgIs2cFB3W1fYfpfUvQm+zzPoEQihNxBxnwFsBtMAOtPy54FjSzKmxsQTrYT9E6NFb8k6ZIIm2gNeOPK9OUJgjw+4g2BXErM6ikHTzM3xcaTq/cQaePZ52emndw1qOtdV06hr2EeuLM8frfLHpsknUe8JeYeW5p9E8QdZjjSN9034usdYNamUdxzmn/Mw/ar8z1xSKS6zcaQoTQ7aYLEX3dWJndc4W64HyiaRkLjO6qLUFeOerfz5UvcxxRY89eAA0KLC2xnGkBMOhXxYzIB3lF8Zxqb4JMhoBGw1n31TDfhRDGDHHEAsZuAIcH7aC5RDVxU08Jxmw4oLmeTDZA5BFcqp2A3fusNVZUnfpmMy6DCJyFprlRl8jSlJMAvhbxVuuLFDZnjl77Z2of796Ur6DgmNwYtMPNEntZPIcZ76VPLWAL8lqiRBm20c4qiwr5rNSr5kry9bR1EfXHwFRjy5pxFQ+5+ilpRl8WPfT/iUuORd8J2wnCmghm7uxiJd9t82kX0s6benhL29dQ1etqt5soX2RnlfKan16GVWoI3xrljIQrCAY4xpdptSpglOnrpSClbN1nhGkDfFPNq2pWhQrDbznDknAJ9MxQaVnLYPhn7I849GMd7EvpSkydwQu7QXn9+H4jxn6UEntNGxcL0xkG+xippvZEe+HBvcDD40efDQW1bDbILLjPb4rNRx4d3xaQnVNaF7L33osm5LgfXAQSwHJiURdkU4zmhtPP4zn0br0OdFlR3mPcrkeNeSvs7FxiKtD6n6s+av+4bKjbLL1OyuwmTnMilL6p+m8ldte0yos/r+zOuxWeI=|euhiXWXehYbFQhlAV6LIECSIPCIRaHbNdr9OI4cTPUM=", + ApiKey = "CfGrD4MoJu3NprOBZNL8tu5ocmtnmU", + KdfIterations = 600000 + }); + + var organizationUser = await CreateUserAsync(factory, organizationId, user.Email, + OrganizationUserType.User, externalId: email); + + return (user, organizationUser); + } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommandTests.cs index da02fbcf4d..bff1af1cde 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Import/ImportOrganizationUsersAndGroupsCommandTests.cs @@ -1,9 +1,9 @@ using Bit.Core.AdminConsole.Models.Business; -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; +using Bit.Core.AdminConsole.OrganizationFeatures.Import; using Bit.Core.Auth.Models.Business.Tokenables; -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.OrganizationUsers; @@ -18,11 +18,10 @@ using NSubstitute; using Xunit; using Organization = Bit.Core.AdminConsole.Entities.Organization; -namespace Bit.Core.Test.OrganizationFeatures.OrganizationUsers; +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Import; public class ImportOrganizationUsersAndGroupsCommandTests { - private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory = new FakeDataProtectorTokenFactory(); [Theory, PaidOrganizationCustomize, BitAutoData] @@ -66,7 +65,6 @@ public class ImportOrganizationUsersAndGroupsCommandTests Users = existingUsers.Count, Sponsored = 0 }); - sutProvider.GetDependency().ManageUsers(org.Id).Returns(true); sutProvider.GetDependency().InviteUsersAsync(org.Id, Guid.Empty, EventSystemUser.PublicApi, Arg.Any>()) .Returns(orgUsers); @@ -92,6 +90,38 @@ public class ImportOrganizationUsersAndGroupsCommandTests .LogOrganizationUserEventsAsync(Arg.Any>()); } + [Theory, PaidOrganizationCustomize, BitAutoData] + public async Task OverwriteExistingUsers_WhenRemovingUserWithoutMasterPassword_Throws( + SutProvider sutProvider, + Organization org, List existingUsers) + { + SetupOrganizationConfigForImport(sutProvider, org, existingUsers, []); + + // Existing user does not have a master password + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.DirectoryConnectorPreventUserRemoval) + .Returns(true); + existingUsers.First().HasMasterPassword = false; + + sutProvider.GetDependency().GetByIdAsync(org.Id).Returns(org); + sutProvider.GetDependency().GetManyDetailsByOrganizationAsync(org.Id).Returns(existingUsers); + + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.ImportAsync(org.Id, [], [], [], true)); + + Assert.Contains("Sync failed. To proceed, disable the 'Remove and re-add users during next sync' setting and try again.", exception.Message); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .UpsertAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .UpsertManyAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .CreateAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .InviteUsersAsync(default, default, default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventsAsync(Arg.Any>()); + } + [Theory, PaidOrganizationCustomize, BitAutoData] public async Task OrgImportCreateNewUsersAndMarryExistingUser( SutProvider sutProvider,