From 02cbdd64a4d4833b80304cff059fd29638ca677b Mon Sep 17 00:00:00 2001 From: Rui Tome Date: Tue, 20 May 2025 16:50:33 +0100 Subject: [PATCH] Refactor AcceptOrgUserCommand to enforce two-factor authentication policy based on feature flag; update validation logic and tests accordingly. --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 48 ++++++++++----- .../AcceptOrgUserCommandTests.cs | 58 ++++++++++++++----- 2 files changed, 74 insertions(+), 32 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index d2048d012f..ed19c86c21 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -202,9 +202,21 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand } // Enforce Two Factor Authentication Policy of organization user is trying to join - if (!await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user)) + if (_featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements)) { - await ValidateTwoFactorAuthenticationPolicy(user, orgUser.OrganizationId); + await ValidateTwoFactorAuthenticationPolicyAsync(user, orgUser.OrganizationId); + } + else + { + if (!await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user)) + { + var invitedTwoFactorPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, + PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited); + if (invitedTwoFactorPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) + { + throw new BadRequestException("You cannot join this organization until you enable two-step login on your user account."); + } + } } orgUser.Status = OrganizationUserStatusType.Accepted; @@ -225,24 +237,28 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand return orgUser; } - private async Task ValidateTwoFactorAuthenticationPolicy(User user, Guid organizationId) + /// + /// Validates the two-factor authentication policy for the organization user. + /// If the user has two-step login enabled, the policy is not enforced. + /// + /// The user to validate the policy for. + /// The ID of the organization to validate the policy for. + /// Thrown if the user does not have two-step login enabled. + private async Task ValidateTwoFactorAuthenticationPolicyAsync(User user, Guid organizationId) { - if (_featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements)) + var userTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user); + if (userTwoFactorEnabled) { - var requireTwoFactorPolicyRequirement = await _policyRequirementQuery.GetAsync(user.Id); - if (requireTwoFactorPolicyRequirement.RequireTwoFactor) - { - throw new BadRequestException("You cannot join this organization until you enable two-step login on your user account."); - } + // If the user has two-step login enabled, the policy is not enforced. + return; } - else + + var requirement = await _policyRequirementQuery.GetAsync(user.Id); + var canAcceptInvitation = requirement.CanAcceptInvitation(userTwoFactorEnabled, organizationId); + + if (!canAcceptInvitation) { - var invitedTwoFactorPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, - PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited); - if (invitedTwoFactorPolicies.Any(p => p.OrganizationId == organizationId)) - { - throw new BadRequestException("You cannot join this organization until you enable two-step login on your user account."); - } + throw new BadRequestException("You cannot join this organization until you enable two-step login on your user account."); } } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index 93926137da..8d8d42d48a 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -1,5 +1,6 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Models.Business.Tokenables; @@ -30,7 +31,6 @@ namespace Bit.Core.Test.OrganizationFeatures.OrganizationUsers; public class AcceptOrgUserCommandTests { private readonly IUserService _userService = Substitute.For(); - private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery = Substitute.For(); private readonly IOrgUserInviteTokenableFactory _orgUserInviteTokenableFactory = Substitute.For(); private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory = new FakeDataProtectorTokenFactory(); @@ -168,7 +168,7 @@ public class AcceptOrgUserCommandTests SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); // User doesn't have 2FA enabled - _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); + sutProvider.GetDependency().TwoFactorIsEnabledAsync(user).Returns(false); // Organization they are trying to join requires 2FA var twoFactorPolicy = new OrganizationUserPolicyDetails { OrganizationId = orgUser.OrganizationId }; @@ -192,7 +192,6 @@ public class AcceptOrgUserCommandTests SutProvider sutProvider, User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { - // Arrange SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); sutProvider.GetDependency() @@ -200,14 +199,23 @@ public class AcceptOrgUserCommandTests .Returns(true); // User doesn't have 2FA enabled - _userService.TwoFactorIsEnabledAsync(user).Returns(false); + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(user) + .Returns(false); // Organization they are trying to join requires 2FA sutProvider.GetDependency() .GetAsync(user.Id) - .Returns(new RequireTwoFactorPolicyRequirement { RequireTwoFactor = true }); + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = orgUser.OrganizationId, + OrganizationUserStatus = OrganizationUserStatusType.Invited, + PolicyType = PolicyType.TwoFactorAuthentication, + } + ])); - // Act & Assert var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); @@ -228,12 +236,20 @@ public class AcceptOrgUserCommandTests .Returns(true); // User has 2FA enabled - _userService.TwoFactorIsEnabledAsync(user).Returns(true); + sutProvider.GetDependency().TwoFactorIsEnabledAsync(Arg.Any()).Returns(true); // Organization they are trying to join requires 2FA sutProvider.GetDependency() .GetAsync(user.Id) - .Returns(new RequireTwoFactorPolicyRequirement { RequireTwoFactor = true }); + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = orgUser.OrganizationId, + OrganizationUserStatus = OrganizationUserStatusType.Invited, + PolicyType = PolicyType.TwoFactorAuthentication, + } + ])); await sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService); @@ -253,26 +269,34 @@ public class AcceptOrgUserCommandTests bool userTwoFactorEnabled, SutProvider sutProvider, User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { - // Arrange SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); sutProvider.GetDependency() .IsEnabled(FeatureFlagKeys.PolicyRequirements) .Returns(true); - // User doesn't have 2FA enabled - _userService.TwoFactorIsEnabledAsync(user).Returns(userTwoFactorEnabled); + // User 2FA status + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(user) + .Returns(userTwoFactorEnabled); // Organization they are trying to join doesn't require 2FA sutProvider.GetDependency() .GetAsync(user.Id) - .Returns(new RequireTwoFactorPolicyRequirement { RequireTwoFactor = false }); + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = Guid.NewGuid(), + OrganizationUserStatus = OrganizationUserStatusType.Invited, + PolicyType = PolicyType.TwoFactorAuthentication, + } + ])); - // Act await sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService); - // Assert - await sutProvider.GetDependency().Received(1) + await sutProvider.GetDependency() + .Received(1) .ReplaceAsync(Arg.Is(ou => ou.Status == OrganizationUserStatusType.Accepted)); } @@ -739,7 +763,9 @@ public class AcceptOrgUserCommandTests .Returns(false); // User doesn't have 2FA enabled - _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(user) + .Returns(false); // Org does not require 2FA sutProvider.GetDependency().GetPoliciesApplicableToUserAsync(user.Id,