From 880c5ffac7b3b28660a5356f458c3e422120514d Mon Sep 17 00:00:00 2001 From: Sven Date: Wed, 18 Feb 2026 13:37:49 -0600 Subject: [PATCH 1/5] Remove usage of policy requirements retrieval by org --- .../ConfirmOrganizationUserCommand.cs | 50 ++++---------- .../v1/RestoreOrganizationUserCommand.cs | 34 +++++---- .../Policies/IPolicyRequirementQuery.cs | 9 --- .../Implementations/PolicyRequirementQuery.cs | 23 ------- .../ConfirmOrganizationUserCommandTests.cs | 10 +-- .../RestoreOrganizationUserCommandTests.cs | 34 +++++++-- .../Policies/PolicyRequirementQueryTests.cs | 69 ------------------- 7 files changed, 68 insertions(+), 161 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs index 02f3346ba636..1708282905a9 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs @@ -88,7 +88,7 @@ public async Task ConfirmUserAsync(Guid organizationId, Guid o throw new BadRequestException(error); } - await CreateDefaultCollectionAsync(orgUser, defaultUserCollectionName); + await CreateManyDefaultCollectionsAsync(organizationId, [orgUser], defaultUserCollectionName); return orgUser; } @@ -103,13 +103,10 @@ public async Task>> ConfirmUsersAsync(Guid .Select(r => r.Item1) .ToList(); - if (confirmedOrganizationUsers.Count == 1) + if (confirmedOrganizationUsers.Count > 0) { - await CreateDefaultCollectionAsync(confirmedOrganizationUsers.Single(), defaultUserCollectionName); - } - else if (confirmedOrganizationUsers.Count > 1) - { - await CreateManyDefaultCollectionsAsync(organizationId, confirmedOrganizationUsers, defaultUserCollectionName); + await CreateManyDefaultCollectionsAsync(organizationId, confirmedOrganizationUsers, + defaultUserCollectionName); } return result; @@ -274,31 +271,6 @@ private async Task> GetUserDeviceIdsAsync(Guid userId) .Select(d => d.Id.ToString()); } - /// - /// 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 encrypted default user collection name. - private async Task CreateDefaultCollectionAsync(OrganizationUser organizationUser, string defaultUserCollectionName) - { - // Skip if no collection name provided (backwards compatibility) - if (string.IsNullOrWhiteSpace(defaultUserCollectionName)) - { - return; - } - - var organizationDataOwnershipPolicy = await _policyRequirementQuery.GetAsync(organizationUser.UserId!.Value); - if (!organizationDataOwnershipPolicy.RequiresDefaultCollectionOnConfirm(organizationUser.OrganizationId)) - { - return; - } - - await _collectionRepository.CreateDefaultCollectionsAsync( - organizationUser.OrganizationId, - [organizationUser.Id], - defaultUserCollectionName); - } - /// /// Creates default collections for multiple users if required by the Organization Data Ownership policy. /// @@ -314,12 +286,16 @@ private async Task CreateManyDefaultCollectionsAsync(Guid organizationId, return; } - var policyEligibleOrganizationUserIds = await _policyRequirementQuery - .GetManyByOrganizationIdAsync(organizationId); + var confirmedUsersByUserId = confirmedOrganizationUsers.ToDictionary(k => k.UserId!.Value); + + var policiesForUsers = await _policyRequirementQuery + .GetAsync(confirmedUsersByUserId.Keys); - var eligibleOrganizationUserIds = confirmedOrganizationUsers - .Where(ou => policyEligibleOrganizationUserIds.Contains(ou.Id)) - .Select(ou => ou.Id) + var eligibleOrganizationUserIds = policiesForUsers + .Where(x => x.Requirement.RequiresDefaultCollectionOnConfirm(organizationId)) + .Select(s => confirmedUsersByUserId.GetValueOrDefault(s.UserId)?.Id) + .Where(w => w != null) + .Select(s => s.Value) .ToList(); if (eligibleOrganizationUserIds.Count == 0) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs index dd9c73a21d5b..66ec1b501dc0 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs @@ -263,22 +263,30 @@ await CreateDefaultCollectionsForConfirmedUsersAsync(organizationId, defaultColl private async Task CreateDefaultCollectionsForConfirmedUsersAsync(Guid organizationId, string defaultCollectionName, ICollection restoredUsers) { - if (!string.IsNullOrWhiteSpace(defaultCollectionName)) + if (string.IsNullOrWhiteSpace(defaultCollectionName)) { - var organizationUsersDataOwnershipEnabled = (await policyRequirementQuery - .GetManyByOrganizationIdAsync(organizationId)) - .ToList(); + return; + } - var usersToCreateDefaultCollectionsFor = restoredUsers.Where(x => - organizationUsersDataOwnershipEnabled.Contains(x.Id) - && x.Status == OrganizationUserStatusType.Confirmed).ToList(); + var restoredUsersById = restoredUsers + .Where(w => w.UserId != null) + .ToDictionary(k => k.UserId.Value); - if (usersToCreateDefaultCollectionsFor.Count != 0) - { - await collectionRepository.CreateDefaultCollectionsAsync(organizationId, - usersToCreateDefaultCollectionsFor.Select(x => x.Id), - defaultCollectionName); - } + var restoredUserPolicyRequirements = await + policyRequirementQuery.GetAsync(restoredUsersById.Keys); + + var orgUserIdsToCreateDefaultCollectionsFor = restoredUserPolicyRequirements + .Where(w => w.Requirement.RequiresDefaultCollectionOnConfirm(organizationId)) + .Select(s => restoredUsersById.GetValueOrDefault(s.UserId)) + .Where(w => w?.Status == OrganizationUserStatusType.Confirmed) + .Select(s => s.Id) + .ToList(); + + if (orgUserIdsToCreateDefaultCollectionsFor.Count != 0) + { + await collectionRepository.CreateDefaultCollectionsAsync(organizationId, + orgUserIdsToCreateDefaultCollectionsFor, + defaultCollectionName); } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/IPolicyRequirementQuery.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/IPolicyRequirementQuery.cs index 2d6bd94fd12b..02d2dedfc114 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/IPolicyRequirementQuery.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/IPolicyRequirementQuery.cs @@ -26,13 +26,4 @@ public interface IPolicyRequirementQuery /// The users that you need to enforce the policy against. /// The IPolicyRequirement that corresponds to the policy you want to enforce. Task> GetAsync(IEnumerable userIds) where T : IPolicyRequirement; - - /// - /// Get all organization user IDs within an organization that are affected by a given policy type. - /// Respects role/status/provider exemptions via the policy factory's Enforce predicate. - /// - /// The organization to check. - /// The IPolicyRequirement that corresponds to the policy type to evaluate. - /// Organization user IDs for whom the policy applies within the organization. - Task> GetManyByOrganizationIdAsync(Guid organizationId) where T : IPolicyRequirement; } diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/PolicyRequirementQuery.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/PolicyRequirementQuery.cs index 809069154045..c38693fdfd9d 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/PolicyRequirementQuery.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/PolicyRequirementQuery.cs @@ -32,29 +32,6 @@ public async Task GetAsync(Guid userId) where T : IPolicyRequirement return policyRequirements; } - public async Task> GetManyByOrganizationIdAsync(Guid organizationId) - where T : IPolicyRequirement - { - var factory = factories.OfType>().SingleOrDefault(); - if (factory is null) - { - throw new NotImplementedException("No Requirement Factory found for " + typeof(T)); - } - - var organizationPolicyDetails = await GetOrganizationPolicyDetails(organizationId, factory.PolicyType); - - var eligibleOrganizationUserIds = organizationPolicyDetails - .Where(p => p.PolicyType == factory.PolicyType) - .Where(factory.Enforce) - .Select(p => p.OrganizationUserId) - .ToList(); - - return eligibleOrganizationUserIds; - } - private async Task> GetPolicyDetails(IEnumerable userIds, PolicyType policyType) => await policyRepository.GetPolicyDetailsByUserIdsAndPolicyType(userIds, policyType); - - private async Task> GetOrganizationPolicyDetails(Guid organizationId, PolicyType policyType) - => await policyRepository.GetPolicyDetailsByOrganizationIdAsync(organizationId, policyType); } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs index 6643f26eb5f4..ea0ce1b485a9 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs @@ -485,8 +485,9 @@ public async Task ConfirmUserAsync_WithOrganizationDataOwnershipPolicyApplicable PolicyType = PolicyType.OrganizationDataOwnership }; sutProvider.GetDependency() - .GetAsync(orgUser.UserId!.Value) - .Returns(new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [policyDetails])); + .GetAsync( + Arg.Is>(ids => ids.Contains(orgUser.UserId!.Value))) + .Returns([(orgUser.UserId!.Value, new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [policyDetails]))]); await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, collectionName); @@ -534,8 +535,9 @@ public async Task ConfirmUserAsync_WithOrganizationDataOwnershipPolicyNotApplica sutProvider.GetDependency().GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); sutProvider.GetDependency() - .GetAsync(orgUser.UserId!.Value) - .Returns(new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Disabled, [])); + .GetAsync( + Arg.Is>(ids => ids.Contains(orgUser.UserId!.Value))) + .Returns([(orgUser.UserId!.Value, new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Disabled, []))]); await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.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 29c996cee99f..bae0eeea3ab0 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs @@ -1396,14 +1396,19 @@ public async Task RestoreUsers_Bulk_WithDataOwnershipPolicy_CreatesCollectionsFo orgUser2.Key = null; orgUser2.OrganizationId = organization.Id; + var orgUser1PolicyRequirement = new OrganizationDataOwnershipPolicyRequirement( + OrganizationDataOwnershipState.Enabled, + [new PolicyDetails { OrganizationId = organization.Id, OrganizationUserId = orgUser1.Id }]); + organizationUserRepository .GetManyAsync(Arg.Is>(ids => ids.Contains(orgUser1.Id) && ids.Contains(orgUser2.Id))) .Returns([orgUser1, orgUser2]); // Setup bulk policy query - returns org user IDs with policy enabled sutProvider.GetDependency() - .GetManyByOrganizationIdAsync(organization.Id) - .Returns([orgUser1.Id]); + .GetAsync( + Arg.Is>(ids => ids.Contains(orgUser1.UserId.Value))) + .Returns([(orgUser1.UserId!.Value, orgUser1PolicyRequirement)]); sutProvider.GetDependency() .TwoFactorIsEnabledAsync(Arg.Any>()) @@ -1453,14 +1458,22 @@ public async Task RestoreUsers_Bulk_WithMixedPolicyStates_OnlyCreatesForEnabledP orgUser2.Email = null; orgUser2.OrganizationId = organization.Id; + var orgUser1PolicyRequirement = new OrganizationDataOwnershipPolicyRequirement( + OrganizationDataOwnershipState.Enabled, + [new PolicyDetails { OrganizationId = organization.Id, OrganizationUserId = orgUser1.Id }]); + + var orgUser2PolicyRequirement = new OrganizationDataOwnershipPolicyRequirement( + OrganizationDataOwnershipState.Disabled, []); + organizationUserRepository .GetManyAsync(Arg.Is>(ids => ids.Contains(orgUser1.Id) && ids.Contains(orgUser2.Id))) .Returns([orgUser1, orgUser2]); // Setup bulk policy query - only orgUser1 has policy enabled sutProvider.GetDependency() - .GetManyByOrganizationIdAsync(organization.Id) - .Returns([orgUser1.Id]); + .GetAsync( + Arg.Is>(ids => ids.Contains(orgUser1.UserId!.Value) && ids.Contains(orgUser2.UserId!.Value))) + .Returns([(orgUser1.UserId!.Value, orgUser1PolicyRequirement), (orgUser2.UserId!.Value, orgUser2PolicyRequirement)]); sutProvider.GetDependency() .TwoFactorIsEnabledAsync(Arg.Any>()) @@ -1510,14 +1523,23 @@ public async Task RestoreUsers_Bulk_WithNullCollectionName_DoesNotCreateAnyColle orgUser2.Email = null; orgUser2.OrganizationId = organization.Id; + var orgUser1PolicyRequirement = new OrganizationDataOwnershipPolicyRequirement( + OrganizationDataOwnershipState.Enabled, + [new PolicyDetails { OrganizationId = organization.Id, OrganizationUserId = orgUser1.Id }]); + + var orgUser2PolicyRequirement = new OrganizationDataOwnershipPolicyRequirement( + OrganizationDataOwnershipState.Enabled, + [new PolicyDetails { OrganizationId = organization.Id, OrganizationUserId = orgUser2.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]); + .GetAsync( + Arg.Is>(ids => ids.Contains(orgUser1.UserId!.Value) && ids.Contains(orgUser2.UserId!.Value))) + .Returns([(orgUser1.UserId!.Value, orgUser1PolicyRequirement), (orgUser2.UserId!.Value, orgUser2PolicyRequirement)]); sutProvider.GetDependency() .TwoFactorIsEnabledAsync(Arg.Any>()) diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirementQueryTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirementQueryTests.cs index 823de897571e..e652181a461d 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirementQueryTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirementQueryTests.cs @@ -194,73 +194,4 @@ public async Task GetAsync_WithMultipleUserIds_ReturnsEmptyRequirementForUserWit Assert.Contains(policyA, requirements[0].Requirement.Policies); Assert.Empty(requirements[1].Requirement.Policies); } - - [Theory, BitAutoData] - public async Task GetManyByOrganizationIdAsync_IgnoresOtherPolicyTypes(Guid organizationId) - { - var policyRepository = Substitute.For(); - var thisPolicy = new OrganizationPolicyDetails { PolicyType = PolicyType.SingleOrg, OrganizationUserId = Guid.NewGuid() }; - var otherPolicy = new OrganizationPolicyDetails { PolicyType = PolicyType.RequireSso, OrganizationUserId = Guid.NewGuid() }; - // Force the repository to return both policies even though that is not the expected result - policyRepository.GetPolicyDetailsByOrganizationIdAsync(organizationId, PolicyType.SingleOrg) - .Returns([thisPolicy, otherPolicy]); - - var factory = new TestPolicyRequirementFactory(_ => true); - var sut = new PolicyRequirementQuery(policyRepository, [factory]); - - var organizationUserIds = await sut.GetManyByOrganizationIdAsync(organizationId); - - await policyRepository.Received(1).GetPolicyDetailsByOrganizationIdAsync(organizationId, PolicyType.SingleOrg); - - Assert.Contains(thisPolicy.OrganizationUserId, organizationUserIds); - Assert.DoesNotContain(otherPolicy.OrganizationUserId, organizationUserIds); - } - - [Theory, BitAutoData] - public async Task GetManyByOrganizationIdAsync_CallsEnforceCallback(Guid organizationId) - { - var policyRepository = Substitute.For(); - var thisPolicy = new OrganizationPolicyDetails { PolicyType = PolicyType.SingleOrg, OrganizationUserId = Guid.NewGuid() }; - var otherPolicy = new OrganizationPolicyDetails { PolicyType = PolicyType.SingleOrg, OrganizationUserId = Guid.NewGuid() }; - policyRepository.GetPolicyDetailsByOrganizationIdAsync(organizationId, PolicyType.SingleOrg).Returns([thisPolicy, otherPolicy]); - - var callback = Substitute.For>(); - callback(Arg.Any()).Returns(x => x.Arg() == thisPolicy); - - var factory = new TestPolicyRequirementFactory(callback); - var sut = new PolicyRequirementQuery(policyRepository, [factory]); - - var organizationUserIds = await sut.GetManyByOrganizationIdAsync(organizationId); - - Assert.Contains(thisPolicy.OrganizationUserId, organizationUserIds); - Assert.DoesNotContain(otherPolicy.OrganizationUserId, organizationUserIds); - callback.Received()(Arg.Is(p => p == thisPolicy)); - callback.Received()(Arg.Is(p => p == otherPolicy)); - } - - [Theory, BitAutoData] - public async Task GetManyByOrganizationIdAsync_ThrowsIfNoFactoryRegistered(Guid organizationId) - { - var policyRepository = Substitute.For(); - var sut = new PolicyRequirementQuery(policyRepository, []); - - var exception = await Assert.ThrowsAsync(() - => sut.GetManyByOrganizationIdAsync(organizationId)); - - Assert.Contains("No Requirement Factory found", exception.Message); - } - - [Theory, BitAutoData] - public async Task GetManyByOrganizationIdAsync_HandlesNoPolicies(Guid organizationId) - { - var policyRepository = Substitute.For(); - policyRepository.GetPolicyDetailsByOrganizationIdAsync(organizationId, PolicyType.SingleOrg).Returns([]); - - var factory = new TestPolicyRequirementFactory(x => x.IsProvider); - var sut = new PolicyRequirementQuery(policyRepository, [factory]); - - var organizationUserIds = await sut.GetManyByOrganizationIdAsync(organizationId); - - Assert.Empty(organizationUserIds); - } } From a0e14e4fc7c4d6016f5ec8e745e52f2a708058a4 Mon Sep 17 00:00:00 2001 From: Sven Date: Wed, 18 Feb 2026 17:19:58 -0600 Subject: [PATCH 2/5] Simplify result model --- ...AutomaticallyConfirmOrganizationUserCommand.cs | 2 +- .../ConfirmOrganizationUserCommand.cs | 13 +++++++------ .../v1/RestoreOrganizationUserCommand.cs | 15 ++++++++------- .../OrganizationDataOwnershipPolicyRequirement.cs | 9 +++++++-- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommand.cs index 029238185702..93106ccdd056 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommand.cs @@ -102,7 +102,7 @@ await collectionRepository.CreateDefaultCollectionsAsync( private async Task ShouldCreateDefaultCollectionAsync(AutomaticallyConfirmOrganizationUserValidationRequest request) => !string.IsNullOrWhiteSpace(request.DefaultUserCollectionName) && (await policyRequirementQuery.GetAsync(request.OrganizationUser!.UserId!.Value)) - .RequiresDefaultCollectionOnConfirm(request.Organization!.Id); + .GetDefaultCollectionRequestOnConfirm(request.Organization!.Id).ShouldCreateDefaultCollection; private async Task PushSyncOrganizationKeysAsync(AutomaticallyConfirmOrganizationUserValidationRequest request) { diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs index 1708282905a9..83fb5599800c 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs @@ -286,16 +286,17 @@ private async Task CreateManyDefaultCollectionsAsync(Guid organizationId, return; } - var confirmedUsersByUserId = confirmedOrganizationUsers.ToDictionary(k => k.UserId!.Value); + var confirmedUserIds = confirmedOrganizationUsers + .Select(s => s.UserId!.Value) + .ToList(); var policiesForUsers = await _policyRequirementQuery - .GetAsync(confirmedUsersByUserId.Keys); + .GetAsync(confirmedUserIds); var eligibleOrganizationUserIds = policiesForUsers - .Where(x => x.Requirement.RequiresDefaultCollectionOnConfirm(organizationId)) - .Select(s => confirmedUsersByUserId.GetValueOrDefault(s.UserId)?.Id) - .Where(w => w != null) - .Select(s => s.Value) + .Select(x => x.Requirement.GetDefaultCollectionRequestOnConfirm(organizationId)) + .Where(w => w.ShouldCreateDefaultCollection) + .Select(s => s.OrganizationUserId) .ToList(); if (eligibleOrganizationUserIds.Count == 0) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs index 66ec1b501dc0..cc6351448988 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs @@ -268,18 +268,19 @@ private async Task CreateDefaultCollectionsForConfirmedUsersAsync(Guid organizat return; } - var restoredUsersById = restoredUsers + var restoredConfirmedUsers = restoredUsers + .Where(w => w.Status == OrganizationUserStatusType.Confirmed) .Where(w => w.UserId != null) - .ToDictionary(k => k.UserId.Value); + .Select(s => s.UserId.Value) + .ToList(); var restoredUserPolicyRequirements = await - policyRequirementQuery.GetAsync(restoredUsersById.Keys); + policyRequirementQuery.GetAsync(restoredConfirmedUsers); var orgUserIdsToCreateDefaultCollectionsFor = restoredUserPolicyRequirements - .Where(w => w.Requirement.RequiresDefaultCollectionOnConfirm(organizationId)) - .Select(s => restoredUsersById.GetValueOrDefault(s.UserId)) - .Where(w => w?.Status == OrganizationUserStatusType.Confirmed) - .Select(s => s.Id) + .Select(s => s.Requirement.GetDefaultCollectionRequestOnConfirm(organizationId)) + .Where(w => w.ShouldCreateDefaultCollection) + .Select(s => s.OrganizationUserId) .ToList(); if (orgUserIdsToCreateDefaultCollectionsFor.Count != 0) diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/OrganizationDataOwnershipPolicyRequirement.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/OrganizationDataOwnershipPolicyRequirement.cs index d30ba5c39f0e..5a2026d8f14c 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/OrganizationDataOwnershipPolicyRequirement.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/OrganizationDataOwnershipPolicyRequirement.cs @@ -68,9 +68,14 @@ public DefaultCollectionRequest GetDefaultCollectionRequestOnPolicyEnable(Guid o return noCollectionNeeded; } - public bool RequiresDefaultCollectionOnConfirm(Guid organizationId) + public DefaultCollectionRequest GetDefaultCollectionRequestOnConfirm(Guid organizationId) { - return _policyDetails.Any(p => p.OrganizationId == organizationId); + var matchingOrgUserId = + _policyDetails.FirstOrDefault(p => p.OrganizationId == organizationId)?.OrganizationUserId; + + return new DefaultCollectionRequest( + OrganizationUserId: matchingOrgUserId.GetValueOrDefault(Guid.Empty), + ShouldCreateDefaultCollection: matchingOrgUserId.HasValue); } /// From 9fcb0cdf73bf0473c682267d9b070f8c72583f3f Mon Sep 17 00:00:00 2001 From: Sven Date: Thu, 19 Feb 2026 10:43:49 -0600 Subject: [PATCH 3/5] Return early if no restored confirmed users exist to create default collections for --- .../RestoreUser/v1/RestoreOrganizationUserCommand.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs index cc6351448988..13a8de93e18e 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs @@ -274,6 +274,11 @@ private async Task CreateDefaultCollectionsForConfirmedUsersAsync(Guid organizat .Select(s => s.UserId.Value) .ToList(); + if (restoredConfirmedUsers.Count == 0) + { + return; + } + var restoredUserPolicyRequirements = await policyRequirementQuery.GetAsync(restoredConfirmedUsers); From c1b995a662a633057e62963ffb8468e538ef72fb Mon Sep 17 00:00:00 2001 From: Sven Date: Fri, 27 Feb 2026 11:09:14 -0600 Subject: [PATCH 4/5] fix merge conflict flog --- .../ConfirmOrganizationUserCommand.cs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs index 21bafb54ff37..11e721d26d11 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs @@ -108,14 +108,7 @@ public async Task>> ConfirmUsersAsync(Guid .Select(r => r.Item1) .ToList(); - if (confirmedOrganizationUsers.Count == 1) - { - await CreateDefaultCollectionAsync(confirmedOrganizationUsers.Single(), organization, defaultUserCollectionName); - } - else if (confirmedOrganizationUsers.Count > 1) - { - await CreateManyDefaultCollectionsAsync(organization, confirmedOrganizationUsers, defaultUserCollectionName); - } + await CreateManyDefaultCollectionsAsync(organization, confirmedOrganizationUsers, defaultUserCollectionName); return result; } @@ -306,7 +299,7 @@ private async Task CreateManyDefaultCollectionsAsync(Organization organization, .GetAsync(confirmedUserIds); var eligibleOrganizationUserIds = policiesForUsers - .Select(x => x.Requirement.GetDefaultCollectionRequestOnConfirm(organizationId)) + .Select(x => x.Requirement.GetDefaultCollectionRequestOnConfirm(organization.Id)) .Where(w => w.ShouldCreateDefaultCollection) .Select(s => s.OrganizationUserId) .ToList(); From 333a06ec97428987903dd1064f0c82acbe6c272f Mon Sep 17 00:00:00 2001 From: Sven Date: Fri, 27 Feb 2026 11:27:03 -0600 Subject: [PATCH 5/5] Fix test mocks --- .../ConfirmOrganizationUserCommandTests.cs | 35 ++++++++++++++----- .../RestoreOrganizationUserCommandTests.cs | 35 +++++++++++++------ 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs index 7d5a7ba963dd..3d9fe31ac74c 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs @@ -910,8 +910,10 @@ public async Task ConfirmUserAsync_UseMyItemsEnabled_CreatesDefaultCollection( PolicyType = PolicyType.OrganizationDataOwnership }; sutProvider.GetDependency() - .GetAsync(orgUser.UserId!.Value) - .Returns(new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [policyDetails])); + .GetAsync(Arg.Is>(ids => ids.Contains(orgUser.UserId!.Value))) + .Returns([ + (orgUser.UserId!.Value, new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [policyDetails])) + ]); // Act await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, collectionName); @@ -951,10 +953,6 @@ public async Task ConfirmUsersAsync_UseMyItemsDisabled_DoesNotCreateDefaultColle 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); @@ -990,9 +988,30 @@ public async Task ConfirmUsersAsync_UseMyItemsEnabled_CreatesDefaultCollections( sutProvider.GetDependency().GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser1, orgUser2 }); sutProvider.GetDependency().GetManyAsync(default).ReturnsForAnyArgs(new[] { user1, user2 }); + var policyDetails1 = new PolicyDetails + { + OrganizationId = organization.Id, + OrganizationUserId = orgUser1.Id, + IsProvider = false, + OrganizationUserStatus = orgUser1.Status, + OrganizationUserType = orgUser1.Type, + PolicyType = PolicyType.OrganizationDataOwnership + }; + var policyDetails2 = new PolicyDetails + { + OrganizationId = organization.Id, + OrganizationUserId = orgUser2.Id, + IsProvider = false, + OrganizationUserStatus = orgUser2.Status, + OrganizationUserType = orgUser2.Type, + PolicyType = PolicyType.OrganizationDataOwnership + }; sutProvider.GetDependency() - .GetManyByOrganizationIdAsync(organization.Id) - .Returns([orgUser1.Id, orgUser2.Id]); + .GetAsync(Arg.Is>(ids => ids.Contains(orgUser1.UserId!.Value) && ids.Contains(orgUser2.UserId!.Value))) + .Returns([ + (orgUser1.UserId!.Value, new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [policyDetails1])), + (orgUser2.UserId!.Value, new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [policyDetails2])) + ]); // Act await sutProvider.Sut.ConfirmUsersAsync(organization.Id, keys, confirmingUser.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 953905288b49..a8c71bfb3081 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs @@ -1666,11 +1666,6 @@ public async Task RestoreUsersAsync_UseMyItemsDisabled_DoesNotCreateCollections( .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)> @@ -1723,11 +1718,6 @@ public async Task RestoreUsersAsync_UseMyItemsEnabled_CreatesCollections( .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)> @@ -1736,6 +1726,31 @@ public async Task RestoreUsersAsync_UseMyItemsEnabled_CreatesCollections( (orgUser2.UserId!.Value, true) }); + var policyDetails1 = new PolicyDetails + { + OrganizationId = organization.Id, + OrganizationUserId = orgUser1.Id, + IsProvider = false, + OrganizationUserStatus = OrganizationUserStatusType.Confirmed, + OrganizationUserType = orgUser1.Type, + PolicyType = PolicyType.OrganizationDataOwnership + }; + var policyDetails2 = new PolicyDetails + { + OrganizationId = organization.Id, + OrganizationUserId = orgUser2.Id, + IsProvider = false, + OrganizationUserStatus = OrganizationUserStatusType.Confirmed, + OrganizationUserType = orgUser2.Type, + PolicyType = PolicyType.OrganizationDataOwnership + }; + sutProvider.GetDependency() + .GetAsync(Arg.Is>(ids => ids.Contains(orgUser1.UserId!.Value) && ids.Contains(orgUser2.UserId!.Value))) + .Returns([ + (orgUser1.UserId!.Value, new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [policyDetails1])), + (orgUser2.UserId!.Value, new OrganizationDataOwnershipPolicyRequirement(OrganizationDataOwnershipState.Enabled, [policyDetails2])) + ]); + // Act var result = await sutProvider.Sut.RestoreUsersAsync( organization.Id,