From f5d03068cbebac62e2f0a23e2fea9ba957a90308 Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Mon, 1 Dec 2025 10:24:38 -0600 Subject: [PATCH] Cleaned up accept org user command tests --- .../Services/Implementations/PolicyService.cs | 10 +- .../AcceptOrgUserCommandTests.cs | 147 +++--------------- 2 files changed, 29 insertions(+), 128 deletions(-) diff --git a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs index 0bf92e16df..95299b098e 100644 --- a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs +++ b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs @@ -92,12 +92,12 @@ public class PolicyService : IPolicyService OrganizationUserType[] excludedUserTypes; - if (policyType == PolicyType.SingleOrg // looking for single org - && _featureService.IsEnabled(FeatureFlagKeys.AutomaticConfirmUsers) // if autoconfirm is enabled - && (await _organizationUserRepository.GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.AutomaticUserConfirmation)).Any()) // any auto confirm details associated with user id + if (policyType == PolicyType.SingleOrg + && _featureService.IsEnabled(FeatureFlagKeys.AutomaticConfirmUsers) + && (await _organizationUserRepository.GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.AutomaticUserConfirmation)).Any()) { - minStatus = OrganizationUserStatusType.Revoked; // all statuses count - excludedUserTypes = []; // no excluded types + minStatus = OrganizationUserStatusType.Revoked; + excludedUserTypes = []; } else { diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index 41320265a0..359c48108c 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -680,100 +680,26 @@ public class AcceptOrgUserCommandTests [Theory] [BitAutoData] - public async Task AcceptOrgUserAsync_WithAutoConfirmEnabledOnOrgAndUserBelongsToAnotherOrg_ThrowsBadRequest( - SutProvider sutProvider, - User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails, - OrganizationUser otherOrgUser) - { - // Arrange - SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); - - // User belongs to another organization - otherOrgUser.UserId = user.Id; - otherOrgUser.OrganizationId = Guid.NewGuid(); // Different org - var allOrgUsers = new List { otherOrgUser }; - sutProvider.GetDependency() - .GetManyByUserAsync(user.Id) - .Returns(Task.FromResult>(allOrgUsers)); - - // Mock auto-confirm enforcement query to return error - sutProvider.GetDependency() - .IsCompliantAsync(Arg.Is(r => - r.OrganizationUser == orgUser && - r.User == user)) - .Returns(Invalid( - new AutomaticUserConfirmationPolicyEnforcementRequest(orgUser, allOrgUsers, user), - new OrganizationEnforcesSingleOrgPolicy())); - - // Act & Assert - var exception = await Assert.ThrowsAsync(() => - sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); - - Assert.Equal(new OrganizationEnforcesSingleOrgPolicy().Message, exception.Message); - } - - [Theory] - [BitAutoData] - public async Task AcceptOrgUserAsync_WithAutoConfirmEnabledOnOtherOrgAndUserBelongsToAnotherOrg_ThrowsBadRequest( - SutProvider sutProvider, - User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails, - OrganizationUser otherOrgUser) - { - // Arrange - SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); - - // User belongs to another organization - otherOrgUser.UserId = user.Id; - otherOrgUser.OrganizationId = Guid.NewGuid(); // Different org - var allOrgUsers = new List { otherOrgUser }; - sutProvider.GetDependency() - .GetManyByUserAsync(user.Id) - .Returns(Task.FromResult>(allOrgUsers)); - - // Mock auto-confirm enforcement query to return error for other org having auto-confirm - sutProvider.GetDependency() - .IsCompliantAsync(Arg.Is(r => - r.OrganizationUser == orgUser && - r.User == user)) - .Returns(Invalid( - new AutomaticUserConfirmationPolicyEnforcementRequest(orgUser, allOrgUsers, user), - new OtherOrganizationEnforcesSingleOrgPolicy())); - - // Act & Assert - var exception = await Assert.ThrowsAsync(() => - sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); - - Assert.Equal(new OtherOrganizationEnforcesSingleOrgPolicy().Message, exception.Message); - } - - [Theory] - [BitAutoData] - public async Task AcceptOrgUserAsync_WithAutoConfirmEnabledAndUserIsProvider_ThrowsBadRequest( + public async Task AcceptOrgUserAsync_WithAutoConfirmIsNotEnabled_DoesNotCheckCompliance( SutProvider sutProvider, User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { // Arrange SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); - // Mock auto-confirm enforcement query to return provider error - sutProvider.GetDependency() - .IsCompliantAsync(Arg.Is(r => - r.OrganizationUser == orgUser && - r.User == user)) - .Returns(Invalid( - new AutomaticUserConfirmationPolicyEnforcementRequest(orgUser, [], user), - new ProviderUsersCannotJoin())); + // Act + var resultOrgUser = await sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService); - // Act & Assert - var exception = await Assert.ThrowsAsync(() => - sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); + // Assert + AssertValidAcceptedOrgUser(resultOrgUser, orgUser, user); - Assert.Equal(new ProviderUsersCannotJoin().Message, exception.Message); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .IsCompliantAsync(Arg.Any()); } [Theory] [BitAutoData] - public async Task AcceptOrgUserAsync_WithAutoConfirmNotApplicable_AcceptsUser( + public async Task AcceptOrgUserAsync_WithUserThatIsCompliantWithAutoConfirm_AcceptsUser( SutProvider sutProvider, User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { @@ -797,7 +723,7 @@ public class AcceptOrgUserCommandTests [Theory] [BitAutoData] - public async Task AcceptOrgUserAsync_WithAutoConfirmValidationBeforeSingleOrgPolicy_ChecksAutoConfirmFirst( + public async Task AcceptOrgUserAsync_WithAutoConfirmIsEnabledAndFailsCompliance_ThrowsBadRequestException( SutProvider sutProvider, User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails, OrganizationUser otherOrgUser) @@ -805,36 +731,22 @@ public class AcceptOrgUserCommandTests // Arrange SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); - // Setup conditions that would fail BOTH auto-confirm AND single org policy checks - otherOrgUser.UserId = user.Id; - otherOrgUser.OrganizationId = Guid.NewGuid(); - var allOrgUsers = new List { otherOrgUser }; - sutProvider.GetDependency() - .GetManyByUserAsync(user.Id) - .Returns(Task.FromResult>(allOrgUsers)); + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AutomaticConfirmUsers) + .Returns(true); - // Make organization they are trying to join have the single org policy (would fail single org check) - var singleOrgPolicy = new OrganizationUserPolicyDetails { OrganizationId = orgUser.OrganizationId }; - sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(user.Id, PolicyType.SingleOrg, OrganizationUserStatusType.Invited) - .Returns(Task.FromResult>( - new List { singleOrgPolicy })); - - // Mock auto-confirm to fail FIRST (should be checked before single org policy) sutProvider.GetDependency() .IsCompliantAsync(Arg.Any()) .Returns(Invalid( - new AutomaticUserConfirmationPolicyEnforcementRequest(orgUser, allOrgUsers, user), + new AutomaticUserConfirmationPolicyEnforcementRequest(orgUser, [otherOrgUser], user), new OrganizationEnforcesSingleOrgPolicy())); // Act & Assert var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); - // Should get auto-confirm error (OrganizationEnforcesSingleOrgPolicy) NOT the regular single org policy error + // Should get auto-confirm error Assert.Equal(new OrganizationEnforcesSingleOrgPolicy().Message, exception.Message); - Assert.NotEqual("You may not join this organization until you leave or remove all other organizations.", - exception.Message); } // Private helpers ------------------------------------------------------------------------------------------------- @@ -880,7 +792,7 @@ public class AcceptOrgUserCommandTests /// - Provides mock data for an admin to validate email functionality. /// - Returns the corresponding organization for the given org ID. /// - private void SetupCommonAcceptOrgUserMocks(SutProvider sutProvider, User user, + private static void SetupCommonAcceptOrgUserMocks(SutProvider sutProvider, User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { @@ -893,18 +805,12 @@ public class AcceptOrgUserCommandTests // User is not part of any other orgs sutProvider.GetDependency() .GetManyByUserAsync(user.Id) - .Returns( - Task.FromResult>(new List()) - ); + .Returns([]); // Org they are trying to join does not have single org policy sutProvider.GetDependency() .GetPoliciesApplicableToUserAsync(user.Id, PolicyType.SingleOrg, OrganizationUserStatusType.Invited) - .Returns( - Task.FromResult>( - new List() - ) - ); + .Returns([]); // User is not part of any organization that applies the single org policy sutProvider.GetDependency() @@ -914,29 +820,24 @@ public class AcceptOrgUserCommandTests // Org does not require 2FA sutProvider.GetDependency().GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited) - .Returns(Task.FromResult>( - new List())); + .Returns([]); // Provide at least 1 admin to test email functionality sutProvider.GetDependency() .GetManyByMinimumRoleAsync(orgUser.OrganizationId, OrganizationUserType.Admin) - .Returns(Task.FromResult>( - new List() { adminUserDetails } - )); + .Returns([adminUserDetails]); // Return org sutProvider.GetDependency() .GetByIdAsync(org.Id) - .Returns(Task.FromResult(org)); + .Returns(org); // Auto-confirm enforcement query returns valid by default (no restrictions) + var request = new AutomaticUserConfirmationPolicyEnforcementRequest(orgUser, [], user); + sutProvider.GetDependency() - .IsCompliantAsync(Arg.Any()) - .Returns(callInfo => - { - var request = callInfo.Arg(); - return Valid(request); - }); + .IsCompliantAsync(request) + .Returns(Valid(request)); }