From 415805679d243e58226bb04cbc3493565e8566a7 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Fri, 27 Feb 2026 11:28:55 +1000 Subject: [PATCH] PM-31847 Wire up UseMyItems organization ability (#7039) Skip My Items creation if organization.UseMyItems is disabled --- ...maticallyConfirmOrganizationUserCommand.cs | 1 + .../ConfirmOrganizationUserCommand.cs | 42 ++-- .../v1/RestoreOrganizationUserCommand.cs | 14 +- ...rganizationDataOwnershipPolicyValidator.cs | 15 ++ ...allyConfirmOrganizationUserCommandTests.cs | 217 ++++++++++++++++++ .../ConfirmOrganizationUserCommandTests.cs | 160 +++++++++++++ .../RestoreOrganizationUserCommandTests.cs | 190 +++++++++++++++ ...zationDataOwnershipPolicyValidatorTests.cs | 114 ++++++++- 8 files changed, 734 insertions(+), 19 deletions(-) create mode 100644 test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommandTests.cs diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommand.cs index 0292381857..37d57b5ad9 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommand.cs @@ -101,6 +101,7 @@ public class AutomaticallyConfirmOrganizationUserCommand(IOrganizationUserReposi /// The result is a boolean value indicating whether a default collection should be created. private async Task ShouldCreateDefaultCollectionAsync(AutomaticallyConfirmOrganizationUserValidationRequest request) => !string.IsNullOrWhiteSpace(request.DefaultUserCollectionName) + && request.Organization!.UseMyItems && (await policyRequirementQuery.GetAsync(request.OrganizationUser!.UserId!.Value)) .RequiresDefaultCollectionOnConfirm(request.Organization!.Id); diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs index 02f3346ba6..3c7e51229d 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs @@ -72,10 +72,13 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand public async Task ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key, Guid confirmingUserId, string defaultUserCollectionName = null) { + var organization = await _organizationRepository.GetByIdAsync(organizationId); + var result = await SaveChangesToDatabaseAsync( organizationId, new Dictionary() { { organizationUserId, key } }, - confirmingUserId); + confirmingUserId, + organization); if (!result.Any()) { @@ -88,7 +91,7 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand throw new BadRequestException(error); } - await CreateDefaultCollectionAsync(orgUser, defaultUserCollectionName); + await CreateDefaultCollectionAsync(orgUser, organization, defaultUserCollectionName); return orgUser; } @@ -96,7 +99,9 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand public async Task>> ConfirmUsersAsync(Guid organizationId, Dictionary keys, Guid confirmingUserId, string defaultUserCollectionName = null) { - var result = await SaveChangesToDatabaseAsync(organizationId, keys, confirmingUserId); + var organization = await _organizationRepository.GetByIdAsync(organizationId); + + var result = await SaveChangesToDatabaseAsync(organizationId, keys, confirmingUserId, organization); var confirmedOrganizationUsers = result .Where(r => string.IsNullOrEmpty(r.Item2)) @@ -105,18 +110,18 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand if (confirmedOrganizationUsers.Count == 1) { - await CreateDefaultCollectionAsync(confirmedOrganizationUsers.Single(), defaultUserCollectionName); + await CreateDefaultCollectionAsync(confirmedOrganizationUsers.Single(), organization, defaultUserCollectionName); } else if (confirmedOrganizationUsers.Count > 1) { - await CreateManyDefaultCollectionsAsync(organizationId, confirmedOrganizationUsers, defaultUserCollectionName); + await CreateManyDefaultCollectionsAsync(organization, confirmedOrganizationUsers, defaultUserCollectionName); } return result; } private async Task>> SaveChangesToDatabaseAsync(Guid organizationId, Dictionary keys, - Guid confirmingUserId) + Guid confirmingUserId, Organization organization) { var selectedOrganizationUsers = await _organizationUserRepository.GetManyAsync(keys.Keys); var validSelectedOrganizationUsers = selectedOrganizationUsers @@ -129,8 +134,6 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand } var validSelectedUserIds = validSelectedOrganizationUsers.Select(u => u.UserId.Value).ToList(); - - var organization = await _organizationRepository.GetByIdAsync(organizationId); var allUsersOrgs = await _organizationUserRepository.GetManyByManyUsersAsync(validSelectedUserIds); var users = await _userRepository.GetManyAsync(validSelectedUserIds); @@ -278,8 +281,9 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand /// Creates a default collection for a single user if required by the Organization Data Ownership policy. /// /// The organization user who has just been confirmed. + /// The organization. /// The encrypted default user collection name. - private async Task CreateDefaultCollectionAsync(OrganizationUser organizationUser, string defaultUserCollectionName) + private async Task CreateDefaultCollectionAsync(OrganizationUser organizationUser, Organization organization, string defaultUserCollectionName) { // Skip if no collection name provided (backwards compatibility) if (string.IsNullOrWhiteSpace(defaultUserCollectionName)) @@ -287,6 +291,12 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand return; } + // Skip if organization has disabled My Items + if (!organization.UseMyItems) + { + return; + } + var organizationDataOwnershipPolicy = await _policyRequirementQuery.GetAsync(organizationUser.UserId!.Value); if (!organizationDataOwnershipPolicy.RequiresDefaultCollectionOnConfirm(organizationUser.OrganizationId)) { @@ -302,10 +312,10 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand /// /// Creates default collections for multiple users if required by the Organization Data Ownership policy. /// - /// The organization ID. + /// The organization. /// The confirmed organization users. /// The encrypted default user collection name. - private async Task CreateManyDefaultCollectionsAsync(Guid organizationId, + private async Task CreateManyDefaultCollectionsAsync(Organization organization, IEnumerable confirmedOrganizationUsers, string defaultUserCollectionName) { // Skip if no collection name provided (backwards compatibility) @@ -314,8 +324,14 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand return; } + // Skip if organization has disabled My Items + if (!organization.UseMyItems) + { + return; + } + var policyEligibleOrganizationUserIds = await _policyRequirementQuery - .GetManyByOrganizationIdAsync(organizationId); + .GetManyByOrganizationIdAsync(organization.Id); var eligibleOrganizationUserIds = confirmedOrganizationUsers .Where(ou => policyEligibleOrganizationUserIds.Contains(ou.Id)) @@ -327,7 +343,7 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand return; } - await _collectionRepository.CreateDefaultCollectionsAsync(organizationId, eligibleOrganizationUserIds, defaultUserCollectionName); + await _collectionRepository.CreateDefaultCollectionsAsync(organization.Id, eligibleOrganizationUserIds, defaultUserCollectionName); } /// diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs index dd9c73a21d..34ea269495 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs @@ -106,6 +106,7 @@ public class RestoreOrganizationUserCommand( await organizationUserRepository.RestoreAsync(organizationUser.Id, status); if (organizationUser.UserId.HasValue + && organization.UseMyItems && (await policyRequirementQuery.GetAsync(organizationUser.UserId.Value)).State == OrganizationDataOwnershipState.Enabled && status == OrganizationUserStatusType.Confirmed && featureService.IsEnabled(FeatureFlagKeys.DefaultUserCollectionRestore) @@ -253,20 +254,25 @@ public class RestoreOrganizationUserCommand( if (featureService.IsEnabled(FeatureFlagKeys.DefaultUserCollectionRestore)) { - await CreateDefaultCollectionsForConfirmedUsersAsync(organizationId, defaultCollectionName, + await CreateDefaultCollectionsForConfirmedUsersAsync(organization, defaultCollectionName, result.Where(r => r.Item2 == "").Select(x => x.Item1).ToList()); } return result; } - private async Task CreateDefaultCollectionsForConfirmedUsersAsync(Guid organizationId, string defaultCollectionName, + private async Task CreateDefaultCollectionsForConfirmedUsersAsync(Organization organization, string defaultCollectionName, ICollection restoredUsers) { + if (!organization.UseMyItems) + { + return; + } + if (!string.IsNullOrWhiteSpace(defaultCollectionName)) { var organizationUsersDataOwnershipEnabled = (await policyRequirementQuery - .GetManyByOrganizationIdAsync(organizationId)) + .GetManyByOrganizationIdAsync(organization.Id)) .ToList(); var usersToCreateDefaultCollectionsFor = restoredUsers.Where(x => @@ -275,7 +281,7 @@ public class RestoreOrganizationUserCommand( if (usersToCreateDefaultCollectionsFor.Count != 0) { - await collectionRepository.CreateDefaultCollectionsAsync(organizationId, + await collectionRepository.CreateDefaultCollectionsAsync(organization.Id, usersToCreateDefaultCollectionsFor.Select(x => x.Id), defaultCollectionName); } diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/OrganizationDataOwnershipPolicyValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/OrganizationDataOwnershipPolicyValidator.cs index 104a5751ff..6e92d53d4a 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/OrganizationDataOwnershipPolicyValidator.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/OrganizationDataOwnershipPolicyValidator.cs @@ -12,6 +12,7 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyValidators; public class OrganizationDataOwnershipPolicyValidator( IPolicyRepository policyRepository, ICollectionRepository collectionRepository, + IOrganizationRepository organizationRepository, IEnumerable> factories) : OrganizationPolicyValidator(policyRepository, factories), IPostSavePolicySideEffect, IOnPolicyPostUpdateEvent { @@ -52,6 +53,20 @@ public class OrganizationDataOwnershipPolicyValidator( private async Task UpsertDefaultCollectionsForUsersAsync(PolicyUpdate policyUpdate, string defaultCollectionName) { + // FIXME: we should use the organizationAbility cache here, but it is currently flaky + // and it's not obvious how to handle a cache failure. + // https://bitwarden.atlassian.net/browse/PM-32699 + var organization = await organizationRepository.GetByIdAsync(policyUpdate.OrganizationId); + if (organization == null) + { + throw new InvalidOperationException($"Organization with ID {policyUpdate.OrganizationId} not found."); + } + + if (!organization.UseMyItems) + { + return; + } + var requirements = await GetUserPolicyRequirementsByOrganizationIdAsync(policyUpdate.OrganizationId, policyUpdate.Type); var userOrgIds = requirements diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommandTests.cs new file mode 100644 index 0000000000..b60ae31cb2 --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommandTests.cs @@ -0,0 +1,217 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.Models.Data.OrganizationUsers; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.AutoConfirmUser; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Bit.Core.Test.AutoFixture.OrganizationUserFixtures; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; +using static Bit.Core.AdminConsole.Utilities.v2.Validation.ValidationResultHelpers; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.OrganizationUsers.AutoConfirmUser; + +[SutProviderCustomize] +public class AutomaticallyConfirmOrganizationUserCommandTests +{ + [Theory, BitAutoData] + public async Task AutomaticallyConfirmOrganizationUserAsync_UseMyItemsDisabled_DoesNotCreateCollection( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, + string key, + string collectionName, + SutProvider sutProvider) + { + // Arrange + organization.UseMyItems = false; + orgUser.OrganizationId = organization.Id; + + SetupRepositoryMocks(sutProvider, organization, orgUser); + + // Mock positive validation result + var validationRequest = new AutomaticallyConfirmOrganizationUserValidationRequest + { + OrganizationUserId = orgUser.Id, + OrganizationId = organization.Id, + Key = key, + DefaultUserCollectionName = collectionName, + PerformedBy = null, + OrganizationUser = orgUser, + Organization = organization + }; + sutProvider.GetDependency() + .ValidateAsync(Arg.Any()) + .Returns(Valid(validationRequest)); + + // Mock enabled policy requirement + var policyDetails = new PolicyDetails + { + OrganizationId = organization.Id, + OrganizationUserId = orgUser.Id, + IsProvider = false, + OrganizationUserStatus = orgUser.Status, + OrganizationUserType = orgUser.Type, + PolicyType = PolicyType.OrganizationDataOwnership + }; + sutProvider.GetDependency() + .GetAsync(orgUser.UserId!.Value) + .Returns(new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [policyDetails])); + + var request = new AutomaticallyConfirmOrganizationUserRequest + { + OrganizationUserId = orgUser.Id, + OrganizationId = organization.Id, + Key = key, + DefaultUserCollectionName = collectionName, + PerformedBy = null + }; + + // Act + await sutProvider.Sut.AutomaticallyConfirmOrganizationUserAsync(request); + + // Assert - Collection repository should NOT be called + await sutProvider.GetDependency() + .DidNotReceive() + .CreateDefaultCollectionsAsync(Arg.Any(), Arg.Any>(), Arg.Any()); + } + + [Theory, BitAutoData] + public async Task AutomaticallyConfirmOrganizationUserAsync_UseMyItemsEnabled_CreatesCollection( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, + string key, + string collectionName, + SutProvider sutProvider) + { + // Arrange + organization.UseMyItems = true; + orgUser.OrganizationId = organization.Id; + + SetupRepositoryMocks(sutProvider, organization, orgUser); + + // Mock positive validation result + var validationRequest = new AutomaticallyConfirmOrganizationUserValidationRequest + { + OrganizationUserId = orgUser.Id, + OrganizationId = organization.Id, + Key = key, + DefaultUserCollectionName = collectionName, + PerformedBy = null, + OrganizationUser = orgUser, + Organization = organization + }; + sutProvider.GetDependency() + .ValidateAsync(Arg.Any()) + .Returns(Valid(validationRequest)); + + // Mock enabled policy requirement + var policyDetails = new PolicyDetails + { + OrganizationId = organization.Id, + OrganizationUserId = orgUser.Id, + IsProvider = false, + OrganizationUserStatus = orgUser.Status, + OrganizationUserType = orgUser.Type, + PolicyType = PolicyType.OrganizationDataOwnership + }; + sutProvider.GetDependency() + .GetAsync(orgUser.UserId!.Value) + .Returns(new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [policyDetails])); + + var request = new AutomaticallyConfirmOrganizationUserRequest + { + OrganizationUserId = orgUser.Id, + OrganizationId = organization.Id, + Key = key, + DefaultUserCollectionName = collectionName, + PerformedBy = null + }; + + // Act + await sutProvider.Sut.AutomaticallyConfirmOrganizationUserAsync(request); + + // Assert - Collection repository should be called + await sutProvider.GetDependency() + .Received(1) + .CreateDefaultCollectionsAsync( + organization.Id, + Arg.Is>(ids => ids.Single() == orgUser.Id), + collectionName); + } + + [Theory, BitAutoData] + public async Task AutomaticallyConfirmOrganizationUserAsync_UseMyItemsEnabled_PolicyDisabled_DoesNotCreateCollection( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, + string key, + string collectionName, + SutProvider sutProvider) + { + // Arrange + organization.UseMyItems = true; + orgUser.OrganizationId = organization.Id; + + SetupRepositoryMocks(sutProvider, organization, orgUser); + + // Mock positive validation result + var validationRequest = new AutomaticallyConfirmOrganizationUserValidationRequest + { + OrganizationUserId = orgUser.Id, + OrganizationId = organization.Id, + Key = key, + DefaultUserCollectionName = collectionName, + PerformedBy = null, + OrganizationUser = orgUser, + Organization = organization + }; + sutProvider.GetDependency() + .ValidateAsync(Arg.Any()) + .Returns(Valid(validationRequest)); + + // Mock disabled policy requirement + sutProvider.GetDependency() + .GetAsync(orgUser.UserId!.Value) + .Returns(new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Disabled, [])); + + var request = new AutomaticallyConfirmOrganizationUserRequest + { + OrganizationUserId = orgUser.Id, + OrganizationId = organization.Id, + Key = key, + DefaultUserCollectionName = collectionName, + PerformedBy = null + }; + + // Act + await sutProvider.Sut.AutomaticallyConfirmOrganizationUserAsync(request); + + // Assert - Collection repository should NOT be called when policy is disabled + await sutProvider.GetDependency() + .DidNotReceive() + .CreateDefaultCollectionsAsync(Arg.Any(), Arg.Any>(), Arg.Any()); + } + + private static void SetupRepositoryMocks( + SutProvider sutProvider, + Organization organization, + OrganizationUser organizationUser) + { + sutProvider.GetDependency() + .GetByIdAsync(organizationUser.Id) + .Returns(organizationUser); + + sutProvider.GetDependency() + .GetByIdAsync(organization.Id) + .Returns(organization); + + sutProvider.GetDependency() + .ConfirmOrganizationUserAsync(Arg.Any()) + .Returns(true); + } +} diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs index 6643f26eb5..a544dd1729 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs @@ -843,4 +843,164 @@ public class ConfirmOrganizationUserCommandTests .DidNotReceive() .SendConfirmationAsync(Arg.Any(), Arg.Any(), Arg.Any()); } + + [Theory, BitAutoData] + public async Task ConfirmUserAsync_UseMyItemsDisabled_DoesNotCreateDefaultCollection( + Organization organization, OrganizationUser confirmingUser, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, User user, + string key, string collectionName, SutProvider sutProvider) + { + // Arrange + organization.PlanType = PlanType.EnterpriseAnnually; + organization.UseMyItems = false; + orgUser.OrganizationId = confirmingUser.OrganizationId = organization.Id; + orgUser.UserId = user.Id; + + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser }); + sutProvider.GetDependency().GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); + + var policyDetails = new PolicyDetails + { + OrganizationId = organization.Id, + OrganizationUserId = orgUser.Id, + IsProvider = false, + OrganizationUserStatus = orgUser.Status, + OrganizationUserType = orgUser.Type, + PolicyType = PolicyType.OrganizationDataOwnership + }; + sutProvider.GetDependency() + .GetAsync(orgUser.UserId!.Value) + .Returns(new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [policyDetails])); + + // Act + await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, collectionName); + + // Assert - Collection repository should NOT be called + await sutProvider.GetDependency() + .DidNotReceive() + .CreateDefaultCollectionsAsync(Arg.Any(), Arg.Any>(), Arg.Any()); + } + + [Theory, BitAutoData] + public async Task ConfirmUserAsync_UseMyItemsEnabled_CreatesDefaultCollection( + Organization organization, OrganizationUser confirmingUser, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, User user, + string key, string collectionName, SutProvider sutProvider) + { + // Arrange + organization.PlanType = PlanType.EnterpriseAnnually; + organization.UseMyItems = true; + orgUser.OrganizationId = confirmingUser.OrganizationId = organization.Id; + orgUser.UserId = user.Id; + + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser }); + sutProvider.GetDependency().GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); + + var policyDetails = new PolicyDetails + { + OrganizationId = organization.Id, + OrganizationUserId = orgUser.Id, + IsProvider = false, + OrganizationUserStatus = orgUser.Status, + OrganizationUserType = orgUser.Type, + PolicyType = PolicyType.OrganizationDataOwnership + }; + sutProvider.GetDependency() + .GetAsync(orgUser.UserId!.Value) + .Returns(new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [policyDetails])); + + // Act + await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, collectionName); + + // Assert - Collection repository should be called + await sutProvider.GetDependency() + .Received(1) + .CreateDefaultCollectionsAsync( + organization.Id, + Arg.Is>(ids => ids.Single() == orgUser.Id), + collectionName); + } + + [Theory, BitAutoData] + public async Task ConfirmUsersAsync_UseMyItemsDisabled_DoesNotCreateDefaultCollections( + Organization organization, OrganizationUser confirmingUser, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser1, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser2, + User user1, User user2, string key1, string key2, string collectionName, + SutProvider sutProvider) + { + // Arrange + organization.PlanType = PlanType.EnterpriseAnnually; + organization.UseMyItems = false; + orgUser1.OrganizationId = confirmingUser.OrganizationId = organization.Id; + orgUser2.OrganizationId = organization.Id; + orgUser1.UserId = user1.Id; + orgUser2.UserId = user2.Id; + + var keys = new Dictionary + { + { orgUser1.Id, key1 }, + { orgUser2.Id, key2 } + }; + + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser1, orgUser2 }); + sutProvider.GetDependency().GetManyAsync(default).ReturnsForAnyArgs(new[] { user1, user2 }); + + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(organization.Id) + .Returns([orgUser1.Id, orgUser2.Id]); + + // Act + await sutProvider.Sut.ConfirmUsersAsync(organization.Id, keys, confirmingUser.Id, collectionName); + + // Assert - Collection repository should NOT be called + await sutProvider.GetDependency() + .DidNotReceive() + .CreateDefaultCollectionsAsync(Arg.Any(), Arg.Any>(), Arg.Any()); + } + + [Theory, BitAutoData] + public async Task ConfirmUsersAsync_UseMyItemsEnabled_CreatesDefaultCollections( + Organization organization, OrganizationUser confirmingUser, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser1, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser2, + User user1, User user2, string key1, string key2, string collectionName, + SutProvider sutProvider) + { + // Arrange + organization.PlanType = PlanType.EnterpriseAnnually; + organization.UseMyItems = true; + orgUser1.OrganizationId = confirmingUser.OrganizationId = organization.Id; + orgUser2.OrganizationId = organization.Id; + orgUser1.UserId = user1.Id; + orgUser2.UserId = user2.Id; + + var keys = new Dictionary + { + { orgUser1.Id, key1 }, + { orgUser2.Id, key2 } + }; + + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser1, orgUser2 }); + sutProvider.GetDependency().GetManyAsync(default).ReturnsForAnyArgs(new[] { user1, user2 }); + + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(organization.Id) + .Returns([orgUser1.Id, orgUser2.Id]); + + // Act + await sutProvider.Sut.ConfirmUsersAsync(organization.Id, keys, confirmingUser.Id, collectionName); + + // Assert - Collection repository should be called with correct parameters + await sutProvider.GetDependency() + .Received(1) + .CreateDefaultCollectionsAsync( + organization.Id, + Arg.Is>(ids => ids.Count() == 2 && ids.Contains(orgUser1.Id) && ids.Contains(orgUser2.Id)), + collectionName); + } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs index 29c996cee9..7c3d2c5803 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs @@ -1542,4 +1542,194 @@ public class RestoreOrganizationUserCommandTests } #endregion + + #region UseMyItems Tests + + [Theory, BitAutoData] + public async Task RestoreUserAsync_UseMyItemsDisabled_DoesNotCreateCollection( + Organization organization, + [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(status: OrganizationUserStatusType.Revoked)] OrganizationUser orgUser, + string collectionName, + SutProvider sutProvider) + { + // Arrange + RestoreUser_Setup(organization, owner, orgUser, sutProvider); + organization.UseMyItems = false; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.DefaultUserCollectionRestore) + .Returns(true); + + // User will restore to Confirmed + orgUser.Email = null; + orgUser.OrganizationId = organization.Id; + + sutProvider.GetDependency() + .GetAsync(orgUser.UserId!.Value) + .Returns(new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [])); + + // Act + await sutProvider.Sut.RestoreUserAsync(orgUser, owner.Id, collectionName); + + // Assert - No collection should be created + await sutProvider.GetDependency() + .DidNotReceive() + .CreateDefaultCollectionsAsync(Arg.Any(), Arg.Any>(), Arg.Any()); + } + + [Theory, BitAutoData] + public async Task RestoreUserAsync_UseMyItemsEnabled_CreatesCollection( + Organization organization, + [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(status: OrganizationUserStatusType.Revoked)] OrganizationUser orgUser, + string collectionName, + SutProvider sutProvider) + { + // Arrange + RestoreUser_Setup(organization, owner, orgUser, sutProvider); + organization.UseMyItems = true; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.DefaultUserCollectionRestore) + .Returns(true); + + // User will restore to Confirmed + orgUser.Email = null; + orgUser.OrganizationId = organization.Id; + + sutProvider.GetDependency() + .GetAsync(orgUser.UserId!.Value) + .Returns(new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [])); + + // Act + await sutProvider.Sut.RestoreUserAsync(orgUser, owner.Id, collectionName); + + // Assert - Collection should be created + await sutProvider.GetDependency() + .Received(1) + .CreateDefaultCollectionsAsync( + organization.Id, + Arg.Is>(ids => ids.Single() == orgUser.Id), + collectionName); + } + + [Theory, BitAutoData] + public async Task RestoreUsersAsync_UseMyItemsDisabled_DoesNotCreateCollections( + Organization organization, + [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(status: OrganizationUserStatusType.Revoked)] OrganizationUser orgUser1, + [OrganizationUser(status: OrganizationUserStatusType.Revoked)] OrganizationUser orgUser2, + string collectionName, + SutProvider sutProvider) + { + // Arrange + RestoreUser_Setup(organization, owner, orgUser1, sutProvider); + organization.UseMyItems = false; + + var organizationUserRepository = sutProvider.GetDependency(); + var userService = Substitute.For(); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.DefaultUserCollectionRestore) + .Returns(true); + + // Both users will restore to Confirmed + orgUser1.Email = null; + orgUser1.OrganizationId = organization.Id; + orgUser2.Email = null; + orgUser2.OrganizationId = organization.Id; + + organizationUserRepository + .GetManyAsync(Arg.Is>(ids => ids.Contains(orgUser1.Id) && ids.Contains(orgUser2.Id))) + .Returns([orgUser1, orgUser2]); + + // Setup bulk policy query - both users have policy enabled + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(organization.Id) + .Returns([orgUser1.Id, orgUser2.Id]); + + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Any>()) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)> + { + (orgUser1.UserId!.Value, true), + (orgUser2.UserId!.Value, true) + }); + + // Act + var result = await sutProvider.Sut.RestoreUsersAsync( + organization.Id, + [orgUser1.Id, orgUser2.Id], + owner.Id, + userService, + collectionName); + + // Assert - No collections should be created + await sutProvider.GetDependency() + .DidNotReceive() + .CreateDefaultCollectionsAsync(Arg.Any(), Arg.Any>(), Arg.Any()); + } + + [Theory, BitAutoData] + public async Task RestoreUsersAsync_UseMyItemsEnabled_CreatesCollections( + Organization organization, + [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(status: OrganizationUserStatusType.Revoked)] OrganizationUser orgUser1, + [OrganizationUser(status: OrganizationUserStatusType.Revoked)] OrganizationUser orgUser2, + string collectionName, + SutProvider sutProvider) + { + // Arrange + RestoreUser_Setup(organization, owner, orgUser1, sutProvider); + organization.UseMyItems = true; + + var organizationUserRepository = sutProvider.GetDependency(); + var userService = Substitute.For(); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.DefaultUserCollectionRestore) + .Returns(true); + + // Both users will restore to Confirmed + orgUser1.Email = null; + orgUser1.OrganizationId = organization.Id; + orgUser2.Email = null; + orgUser2.OrganizationId = organization.Id; + + organizationUserRepository + .GetManyAsync(Arg.Is>(ids => ids.Contains(orgUser1.Id) && ids.Contains(orgUser2.Id))) + .Returns([orgUser1, orgUser2]); + + // Setup bulk policy query - both users have policy enabled + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(organization.Id) + .Returns([orgUser1.Id, orgUser2.Id]); + + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Any>()) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)> + { + (orgUser1.UserId!.Value, true), + (orgUser2.UserId!.Value, true) + }); + + // Act + var result = await sutProvider.Sut.RestoreUsersAsync( + organization.Id, + [orgUser1.Id, orgUser2.Id], + owner.Id, + userService, + collectionName); + + // Assert - Collections should be created for both confirmed users + await sutProvider.GetDependency() + .Received(1) + .CreateDefaultCollectionsAsync( + organization.Id, + Arg.Is>(ids => ids.Count() == 2 && ids.Contains(orgUser1.Id) && ids.Contains(orgUser2.Id)), + collectionName); + } + + #endregion } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/OrganizationDataOwnershipPolicyValidatorTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/OrganizationDataOwnershipPolicyValidatorTests.cs index dd2f1d76e8..95c0e20542 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/OrganizationDataOwnershipPolicyValidatorTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/OrganizationDataOwnershipPolicyValidatorTests.cs @@ -24,11 +24,17 @@ public class OrganizationDataOwnershipPolicyValidatorTests [PolicyUpdate(PolicyType.OrganizationDataOwnership, true)] PolicyUpdate policyUpdate, [Policy(PolicyType.OrganizationDataOwnership, true)] Policy postUpdatedPolicy, [Policy(PolicyType.OrganizationDataOwnership, true)] Policy previousPolicyState, + Organization organization, SutProvider sutProvider) { // Arrange postUpdatedPolicy.OrganizationId = policyUpdate.OrganizationId; previousPolicyState.OrganizationId = policyUpdate.OrganizationId; + organization.Id = policyUpdate.OrganizationId; + + sutProvider.GetDependency() + .GetByIdAsync(policyUpdate.OrganizationId) + .Returns(organization); var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); @@ -46,11 +52,17 @@ public class OrganizationDataOwnershipPolicyValidatorTests [PolicyUpdate(PolicyType.OrganizationDataOwnership, false)] PolicyUpdate policyUpdate, [Policy(PolicyType.OrganizationDataOwnership, false)] Policy postUpdatedPolicy, [Policy(PolicyType.OrganizationDataOwnership)] Policy previousPolicyState, + Organization organization, SutProvider sutProvider) { // Arrange previousPolicyState.OrganizationId = policyUpdate.OrganizationId; postUpdatedPolicy.OrganizationId = policyUpdate.OrganizationId; + organization.Id = policyUpdate.OrganizationId; + + sutProvider.GetDependency() + .GetByIdAsync(policyUpdate.OrganizationId) + .Returns(organization); var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); @@ -195,12 +207,18 @@ public class OrganizationDataOwnershipPolicyValidatorTests [PolicyUpdate(PolicyType.OrganizationDataOwnership)] PolicyUpdate policyUpdate, [Policy(PolicyType.OrganizationDataOwnership, true)] Policy postUpdatedPolicy, [Policy(PolicyType.OrganizationDataOwnership, false)] Policy previousPolicyState, + Organization organization, SutProvider sutProvider) { // Arrange postUpdatedPolicy.OrganizationId = policyUpdate.OrganizationId; previousPolicyState.OrganizationId = policyUpdate.OrganizationId; policyUpdate.Enabled = true; + organization.Id = policyUpdate.OrganizationId; + + sutProvider.GetDependency() + .GetByIdAsync(policyUpdate.OrganizationId) + .Returns(organization); var policyRequest = new SavePolicyModel(policyUpdate, metadata); @@ -226,9 +244,18 @@ public class OrganizationDataOwnershipPolicyValidatorTests private static OrganizationDataOwnershipPolicyValidator ArrangeSut( OrganizationDataOwnershipPolicyRequirementFactory factory, IPolicyRepository policyRepository, - ICollectionRepository collectionRepository) + ICollectionRepository collectionRepository, + bool useMyItems = true) { - var sut = new OrganizationDataOwnershipPolicyValidator(policyRepository, collectionRepository, [factory]); + var organizationRepository = Substitute.For(); + // Default to UseMyItems = true for existing tests + organizationRepository.GetByIdAsync(Arg.Any()) + .Returns(callInfo => new Organization + { + Id = callInfo.Arg(), + UseMyItems = useMyItems + }); + var sut = new OrganizationDataOwnershipPolicyValidator(policyRepository, collectionRepository, organizationRepository, [factory]); return sut; } @@ -237,11 +264,17 @@ public class OrganizationDataOwnershipPolicyValidatorTests [PolicyUpdate(PolicyType.OrganizationDataOwnership, true)] PolicyUpdate policyUpdate, [Policy(PolicyType.OrganizationDataOwnership, true)] Policy postUpdatedPolicy, [Policy(PolicyType.OrganizationDataOwnership, true)] Policy previousPolicyState, + Organization organization, SutProvider sutProvider) { // Arrange postUpdatedPolicy.OrganizationId = policyUpdate.OrganizationId; previousPolicyState.OrganizationId = policyUpdate.OrganizationId; + organization.Id = policyUpdate.OrganizationId; + + sutProvider.GetDependency() + .GetByIdAsync(policyUpdate.OrganizationId) + .Returns(organization); var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); @@ -259,11 +292,17 @@ public class OrganizationDataOwnershipPolicyValidatorTests [PolicyUpdate(PolicyType.OrganizationDataOwnership, false)] PolicyUpdate policyUpdate, [Policy(PolicyType.OrganizationDataOwnership, false)] Policy postUpdatedPolicy, [Policy(PolicyType.OrganizationDataOwnership)] Policy previousPolicyState, + Organization organization, SutProvider sutProvider) { // Arrange previousPolicyState.OrganizationId = policyUpdate.OrganizationId; postUpdatedPolicy.OrganizationId = policyUpdate.OrganizationId; + organization.Id = policyUpdate.OrganizationId; + + sutProvider.GetDependency() + .GetByIdAsync(policyUpdate.OrganizationId) + .Returns(organization); var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); @@ -352,12 +391,18 @@ public class OrganizationDataOwnershipPolicyValidatorTests [PolicyUpdate(PolicyType.OrganizationDataOwnership)] PolicyUpdate policyUpdate, [Policy(PolicyType.OrganizationDataOwnership, true)] Policy postUpdatedPolicy, [Policy(PolicyType.OrganizationDataOwnership, false)] Policy previousPolicyState, + Organization organization, SutProvider sutProvider) { // Arrange postUpdatedPolicy.OrganizationId = policyUpdate.OrganizationId; previousPolicyState.OrganizationId = policyUpdate.OrganizationId; policyUpdate.Enabled = true; + organization.Id = policyUpdate.OrganizationId; + + sutProvider.GetDependency() + .GetByIdAsync(policyUpdate.OrganizationId) + .Returns(organization); var policyRequest = new SavePolicyModel(policyUpdate, metadata); @@ -369,4 +414,69 @@ public class OrganizationDataOwnershipPolicyValidatorTests .DidNotReceiveWithAnyArgs() .CreateDefaultCollectionsBulkAsync(default, default, default); } + + [Theory] + [BitMemberAutoData(nameof(ShouldUpsertDefaultCollectionsTestCases))] + public async Task ExecuteSideEffectsAsync_OrganizationNotFound_ThrowsInvalidOperationException( + Policy postUpdatedPolicy, + Policy? previousPolicyState, + [PolicyUpdate(PolicyType.OrganizationDataOwnership)] PolicyUpdate policyUpdate, + [OrganizationPolicyDetails(PolicyType.OrganizationDataOwnership)] IEnumerable orgPolicyDetails, + OrganizationDataOwnershipPolicyRequirementFactory factory) + { + // Arrange + var orgPolicyDetailsList = orgPolicyDetails.ToList(); + foreach (var policyDetail in orgPolicyDetailsList) + { + policyDetail.OrganizationId = policyUpdate.OrganizationId; + } + + var policyRepository = ArrangePolicyRepository(orgPolicyDetailsList); + var collectionRepository = Substitute.For(); + var organizationRepository = Substitute.For(); + + // Return null to simulate organization not found + organizationRepository.GetByIdAsync(Arg.Any()).Returns((Organization?)null); + + var sut = new OrganizationDataOwnershipPolicyValidator(policyRepository, collectionRepository, organizationRepository, [factory]); + var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); + + // Act & Assert + await Assert.ThrowsAsync(() => + sut.ExecuteSideEffectsAsync(policyRequest, postUpdatedPolicy, previousPolicyState)); + } + + [Theory] + [BitMemberAutoData(nameof(ShouldUpsertDefaultCollectionsTestCases))] + public async Task ExecuteSideEffectsAsync_UseMyItemsDisabled_DoesNotCreateCollections( + Policy postUpdatedPolicy, + Policy? previousPolicyState, + [PolicyUpdate(PolicyType.OrganizationDataOwnership)] PolicyUpdate policyUpdate, + [OrganizationPolicyDetails(PolicyType.OrganizationDataOwnership)] IEnumerable orgPolicyDetails, + OrganizationDataOwnershipPolicyRequirementFactory factory) + { + // Arrange + var orgPolicyDetailsList = orgPolicyDetails.ToList(); + foreach (var policyDetail in orgPolicyDetailsList) + { + policyDetail.OrganizationId = policyUpdate.OrganizationId; + } + + var policyRepository = ArrangePolicyRepository(orgPolicyDetailsList); + var collectionRepository = Substitute.For(); + + var sut = ArrangeSut(factory, policyRepository, collectionRepository, useMyItems: false); + var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); + + // Act + await sut.ExecuteSideEffectsAsync(policyRequest, postUpdatedPolicy, previousPolicyState); + + // Assert - Should NOT create collections when UseMyItems is disabled + await collectionRepository + .DidNotReceive() + .CreateDefaultCollectionsBulkAsync( + Arg.Any(), + Arg.Any>(), + Arg.Any()); + } }