Refactor BulkAutomaticallyConfirmOrganizationUsers functionality and update related tests

JaredScar · JaredScar · commit aeed94b8ef9a · 2026-04-29T15:18:32.000-04:00
- Updated the BulkAutoConfirmOnLogin feature flag key in Constants.cs. - Enhanced BulkAutomaticallyConfirmOrganizationUsersValidator with additional comments for clarity. - Refactored the OrganizationUserRepository to use a transaction for confirming organization users and updated the logic for handling confirmed IDs. - Modified the stored procedure to remove the default value for the @type parameter. - Improved unit tests for BulkAutomaticallyConfirmOrganizationUsersCommand to streamline request building and validation handling.
diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/BulkAutomaticallyConfirmOrganizationUsersValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/BulkAutomaticallyConfirmOrganizationUsersValidator.cs
@@ -70,6 +70,10 @@ public async Task<IEnumerable<ValidationResult<AutomaticallyConfirmOrganizationU
         var autoConfirmPolicyByUserId = autoConfirmPolicyResults.ToDictionary(r => r.UserId, r => r.Requirement);
 
         // Group all org memberships by the user so we can do the multi-org check in memory.
+        // GetManyByManyUsersAsync queries by UserId only (WHERE UserId IN (...)), matching the
+        // single-user path's GetManyByUserAsync. Neither returns Invited rows with a null UserId.
+        // The || Email fallback in AutomaticUserConfirmationPolicyEnforcementValidator is only
+        // used to locate the current org-user row defensively and does not extend the data fetched here.
         var orgUsersByUserId = allOrgUsersForUsers
             .GroupBy(ou => ou.UserId!.Value)
             .ToDictionary(g => g.Key, g => g.Count());
diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs
@@ -162,8 +162,6 @@ public static class FeatureFlagKeys
     public const string PolicyRequirements = "pm-14439-policy-requirements";
     public const string ScimInviteUserOptimization = "pm-16811-optimize-invite-user-flow-to-fail-fast";
     public const string AutomaticConfirmUsers = "pm-19934-auto-confirm-organization-users";
-    // TODO: Replace with the real LaunchDarkly feature flag key in pm-XXXXX-bulk-auto-confirm-on-login format once the flag is created.
-    public const string BulkAutoConfirmOnLogin = "bulk-auto-confirm-on-login";
     public const string AdminResetTwoFactor = "pm-15489-reset-two-factor-account-recovery";
     public const string BulkAutoConfirmOnLogin = "pm-35803-browser-auto-confirm-log-in";
     public const string PublicMembersInviteRefactor = "pm-33398-refactor-members-invite-org-users-command";
diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs
@@ -1056,24 +1056,33 @@ public async Task<ICollection<Guid>> ConfirmManyOrganizationUsersAsync(
         var orgUserIds = usersToConfirmList.Select(u => u.OrganizationUserId).ToList();
         var keyByOrgUserId = usersToConfirmList.ToDictionary(u => u.OrganizationUserId, u => u.Key);
 
-        var confirmedIds = await dbContext.OrganizationUsers
+        await using var transaction = await dbContext.Database.BeginTransactionAsync();
+
+        var rowsToUpdate = await dbContext.OrganizationUsers
             .Where(ou => orgUserIds.Contains(ou.Id) && ou.Status == OrganizationUserStatusType.Accepted)
-            .Select(ou => ou.Id)
             .ToListAsync();
 
-        if (confirmedIds.Count == 0)
+        if (rowsToUpdate.Count == 0)
         {
-            return confirmedIds;
+            await transaction.RollbackAsync();
+            return [];
         }
 
-        await dbContext.OrganizationUsers
-            .Where(ou => confirmedIds.Contains(ou.Id))
-            .ExecuteUpdateAsync(s => s
-                .SetProperty(ou => ou.Status, OrganizationUserStatusType.Confirmed)
-                .SetProperty(ou => ou.Key, ou => keyByOrgUserId[ou.Id]));
+        var revisionDate = DateTime.UtcNow;
+        foreach (var ou in rowsToUpdate)
+        {
+            ou.Status = OrganizationUserStatusType.Confirmed;
+            ou.Key = keyByOrgUserId[ou.Id];
+            ou.RevisionDate = revisionDate;
+        }
+
+        await dbContext.SaveChangesAsync();
 
+        var confirmedIds = rowsToUpdate.Select(o => o.Id).ToList();
         await dbContext.UserBumpAccountRevisionDateByOrganizationUserIdsAsync(confirmedIds);
 
+        await transaction.CommitAsync();
+
         return confirmedIds;
     }
 
diff --git a/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByOrganizationId.sql b/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByOrganizationId.sql
@@ -1,6 +1,6 @@
 ﻿CREATE PROCEDURE [dbo].[OrganizationUser_ReadByOrganizationId]
     @OrganizationId UNIQUEIDENTIFIER,
-    @Type TINYINT = NULL
+    @Type TINYINT
 AS
 BEGIN
     SET NOCOUNT ON
diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/BulkAutomaticallyConfirmOrganizationUsersCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/BulkAutomaticallyConfirmOrganizationUsersCommandTests.cs
@@ -1,15 +1,15 @@
 ﻿using Bit.Core.AdminConsole.Entities;
 using Bit.Core.AdminConsole.Models.Data;
+using Bit.Core.AdminConsole.Models.Data.OrganizationUsers;
 using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.AutoConfirmUser;
-using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
-using Bit.Core.AdminConsole.Utilities.v2.Results;
+using Bit.Core.AdminConsole.Utilities.v2.Validation;
 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 OneOf.Types;
 using Xunit;
 
 namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.OrganizationUsers.AutoConfirmUser;
@@ -31,40 +31,23 @@ public async Task BulkAutomaticallyConfirmOrganizationUsersAsync_AllSucceed_Retu
         orgUser1.OrganizationId = organization.Id;
         orgUser2.OrganizationId = organization.Id;
 
-        var requests = new[]
-        {
-            new AutomaticallyConfirmOrganizationUserRequest
-            {
-                OrganizationUserId = orgUser1.Id,
-                OrganizationId = organization.Id,
-                Key = key1,
-                DefaultUserCollectionName = string.Empty,
-                PerformedBy = new StandardUser(performingUserId, false)
-            },
-            new AutomaticallyConfirmOrganizationUserRequest
-            {
-                OrganizationUserId = orgUser2.Id,
-                OrganizationId = organization.Id,
-                Key = key2,
-                DefaultUserCollectionName = string.Empty,
-                PerformedBy = new StandardUser(performingUserId, false)
-            }
-        };
+        var requests = BuildRequests(organization.Id, performingUserId,
+            (orgUser1.Id, key1),
+            (orgUser2.Id, key2));
+
+        SetupCommonMocks(sutProvider, organization, [orgUser1, orgUser2]);
+        SetupValidatorAllValid(sutProvider, requests, [orgUser1, orgUser2], organization);
 
-        sutProvider.GetDependency<IAutomaticallyConfirmOrganizationUserCommand>()
-            .AutomaticallyConfirmOrganizationUserAsync(Arg.Any<AutomaticallyConfirmOrganizationUserRequest>())
-            .Returns(new CommandResult(new None()));
+        sutProvider.GetDependency<IOrganizationUserRepository>()
+            .ConfirmManyOrganizationUsersAsync(Arg.Any<IEnumerable<AcceptedOrganizationUserToConfirm>>())
+            .Returns([orgUser1.Id, orgUser2.Id]);
 
         // Act
         var results = await sutProvider.Sut.BulkAutomaticallyConfirmOrganizationUsersAsync(requests);
 
         // Assert
         Assert.Equal(2, results.Count);
         Assert.All(results, r => Assert.Null(r.Error));
-
-        await sutProvider.GetDependency<IAutomaticallyConfirmOrganizationUserCommand>()
-            .Received(2)
-            .AutomaticallyConfirmOrganizationUserAsync(Arg.Any<AutomaticallyConfirmOrganizationUserRequest>());
     }
 
     [Theory, BitAutoData]
@@ -81,35 +64,26 @@ public async Task BulkAutomaticallyConfirmOrganizationUsersAsync_OneFailsOneSucc
         orgUser1.OrganizationId = organization.Id;
         orgUser2.OrganizationId = organization.Id;
 
-        var request1 = new AutomaticallyConfirmOrganizationUserRequest
-        {
-            OrganizationUserId = orgUser1.Id,
-            OrganizationId = organization.Id,
-            Key = key1,
-            DefaultUserCollectionName = string.Empty,
-            PerformedBy = new StandardUser(performingUserId, false)
-        };
-        var request2 = new AutomaticallyConfirmOrganizationUserRequest
-        {
-            OrganizationUserId = orgUser2.Id,
-            OrganizationId = organization.Id,
-            Key = key2,
-            DefaultUserCollectionName = string.Empty,
-            PerformedBy = new StandardUser(performingUserId, false)
-        };
+        var requests = BuildRequests(organization.Id, performingUserId,
+            (orgUser1.Id, key1),
+            (orgUser2.Id, key2));
 
-        sutProvider.GetDependency<IAutomaticallyConfirmOrganizationUserCommand>()
-            .AutomaticallyConfirmOrganizationUserAsync(Arg.Is<AutomaticallyConfirmOrganizationUserRequest>(r =>
-                r.OrganizationUserId == orgUser1.Id))
-            .Returns(new CommandResult(new None()));
+        SetupCommonMocks(sutProvider, organization, [orgUser1, orgUser2]);
 
-        sutProvider.GetDependency<IAutomaticallyConfirmOrganizationUserCommand>()
-            .AutomaticallyConfirmOrganizationUserAsync(Arg.Is<AutomaticallyConfirmOrganizationUserRequest>(r =>
-                r.OrganizationUserId == orgUser2.Id))
-            .Returns(new CommandResult(new UserIsNotAccepted()));
+        // orgUser1 passes validation; orgUser2 fails
+        sutProvider.GetDependency<IBulkAutomaticallyConfirmOrganizationUsersValidator>()
+            .ValidateManyAsync(Arg.Any<IEnumerable<AutomaticallyConfirmOrganizationUserValidationRequest>>())
+            .Returns([
+                ValidationResultHelpers.Valid(BuildValidationRequest(requests[0], orgUser1, organization)),
+                ValidationResultHelpers.Invalid(BuildValidationRequest(requests[1], orgUser2, organization), new UserIsNotAccepted())
+            ]);
+
+        sutProvider.GetDependency<IOrganizationUserRepository>()
+            .ConfirmManyOrganizationUsersAsync(Arg.Any<IEnumerable<AcceptedOrganizationUserToConfirm>>())
+            .Returns([orgUser1.Id]);
 
         // Act
-        var results = await sutProvider.Sut.BulkAutomaticallyConfirmOrganizationUsersAsync([request1, request2]);
+        var results = await sutProvider.Sut.BulkAutomaticallyConfirmOrganizationUsersAsync(requests);
 
         // Assert
         Assert.Equal(2, results.Count);
@@ -131,9 +105,9 @@ public async Task BulkAutomaticallyConfirmOrganizationUsersAsync_EmptyList_Retur
         // Assert
         Assert.Empty(results);
 
-        await sutProvider.GetDependency<IAutomaticallyConfirmOrganizationUserCommand>()
+        await sutProvider.GetDependency<IBulkAutomaticallyConfirmOrganizationUsersValidator>()
             .DidNotReceive()
-            .AutomaticallyConfirmOrganizationUserAsync(Arg.Any<AutomaticallyConfirmOrganizationUserRequest>());
+            .ValidateManyAsync(Arg.Any<IEnumerable<AutomaticallyConfirmOrganizationUserValidationRequest>>());
     }
 
     [Theory, BitAutoData]
@@ -147,24 +121,78 @@ public async Task BulkAutomaticallyConfirmOrganizationUsersAsync_PreservesOrgani
         // Arrange
         orgUser.OrganizationId = organization.Id;
 
-        var request = new AutomaticallyConfirmOrganizationUserRequest
-        {
-            OrganizationUserId = orgUser.Id,
-            OrganizationId = organization.Id,
-            Key = key,
-            DefaultUserCollectionName = string.Empty,
-            PerformedBy = new StandardUser(performingUserId, false)
-        };
+        var requests = BuildRequests(organization.Id, performingUserId, (orgUser.Id, key));
 
-        sutProvider.GetDependency<IAutomaticallyConfirmOrganizationUserCommand>()
-            .AutomaticallyConfirmOrganizationUserAsync(Arg.Any<AutomaticallyConfirmOrganizationUserRequest>())
-            .Returns(new CommandResult(new None()));
+        SetupCommonMocks(sutProvider, organization, [orgUser]);
+        SetupValidatorAllValid(sutProvider, requests, [orgUser], organization);
+
+        sutProvider.GetDependency<IOrganizationUserRepository>()
+            .ConfirmManyOrganizationUsersAsync(Arg.Any<IEnumerable<AcceptedOrganizationUserToConfirm>>())
+            .Returns([orgUser.Id]);
 
         // Act
-        var results = await sutProvider.Sut.BulkAutomaticallyConfirmOrganizationUsersAsync([request]);
+        var results = await sutProvider.Sut.BulkAutomaticallyConfirmOrganizationUsersAsync(requests);
 
         // Assert
         Assert.Single(results);
         Assert.Equal(orgUser.Id, results[0].OrganizationUserId);
     }
+
+    private static List<AutomaticallyConfirmOrganizationUserRequest> BuildRequests(
+        Guid organizationId,
+        Guid performingUserId,
+        params (Guid OrgUserId, string Key)[] userKeys) =>
+        userKeys.Select(u => new AutomaticallyConfirmOrganizationUserRequest
+        {
+            OrganizationUserId = u.OrgUserId,
+            OrganizationId = organizationId,
+            Key = u.Key,
+            DefaultUserCollectionName = string.Empty,
+            PerformedBy = new StandardUser(performingUserId, false)
+        }).ToList();
+
+    private static AutomaticallyConfirmOrganizationUserValidationRequest BuildValidationRequest(
+        AutomaticallyConfirmOrganizationUserRequest request,
+        OrganizationUser orgUser,
+        Organization organization) =>
+        new()
+        {
+            OrganizationUserId = request.OrganizationUserId,
+            OrganizationId = request.OrganizationId,
+            Key = request.Key,
+            DefaultUserCollectionName = request.DefaultUserCollectionName,
+            PerformedBy = request.PerformedBy,
+            OrganizationUser = orgUser,
+            Organization = organization
+        };
+
+    private static void SetupCommonMocks(
+        SutProvider<BulkAutomaticallyConfirmOrganizationUsersCommand> sutProvider,
+        Organization organization,
+        ICollection<OrganizationUser> orgUsers)
+    {
+        sutProvider.GetDependency<IOrganizationRepository>()
+            .GetByIdAsync(organization.Id)
+            .Returns(organization);
+
+        sutProvider.GetDependency<IOrganizationUserRepository>()
+            .GetManyAsync(Arg.Any<IEnumerable<Guid>>())
+            .Returns(orgUsers);
+    }
+
+    private static void SetupValidatorAllValid(
+        SutProvider<BulkAutomaticallyConfirmOrganizationUsersCommand> sutProvider,
+        IEnumerable<AutomaticallyConfirmOrganizationUserRequest> requests,
+        ICollection<OrganizationUser> orgUsers,
+        Organization organization)
+    {
+        var orgUserById = orgUsers.ToDictionary(ou => ou.Id);
+        var validationResults = requests
+            .Select(r => ValidationResultHelpers.Valid(BuildValidationRequest(r, orgUserById[r.OrganizationUserId], organization)))
+            .ToList<ValidationResult<AutomaticallyConfirmOrganizationUserValidationRequest>>();
+
+        sutProvider.GetDependency<IBulkAutomaticallyConfirmOrganizationUsersValidator>()
+            .ValidateManyAsync(Arg.Any<IEnumerable<AutomaticallyConfirmOrganizationUserValidationRequest>>())
+            .Returns(validationResults);
+    }
 }
