From ec81ed786a8e673a89fd6dde332a8e1a336ecbbf Mon Sep 17 00:00:00 2001 From: Rui Tome Date: Tue, 20 May 2025 16:51:15 +0100 Subject: [PATCH] Enhance ConfirmOrganizationUserCommand to validate two-factor authentication policy based on feature flag; refactor validation logic and update related tests for improved policy handling. --- .../ConfirmOrganizationUserCommand.cs | 40 ++++++++++++------- .../ConfirmOrganizationUserCommandTests.cs | 31 ++++++++++++-- 2 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs index 29a405aa4f..af49baf729 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs @@ -152,7 +152,19 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand ICollection userOrgs, bool userTwoFactorEnabled) { // Enforce Two Factor Authentication Policy for this organization - await CheckTwoFactorPolicyAsync(organizationId, user, userTwoFactorEnabled); + if (_featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements)) + { + await ValidateTwoFactorAuthenticationPolicyAsync(user, organizationId, userTwoFactorEnabled); + } + else + { + var orgRequiresTwoFactor = (await _policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication)) + .Any(p => p.OrganizationId == organizationId); + if (orgRequiresTwoFactor && !userTwoFactorEnabled) + { + throw new BadRequestException("User does not have two-step login enabled."); + } + } var hasOtherOrgs = userOrgs.Any(ou => ou.OrganizationId != organizationId); var singleOrgPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.SingleOrg); @@ -170,27 +182,25 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand } } - private async Task CheckTwoFactorPolicyAsync(Guid organizationId, User user, bool userTwoFactorEnabled) + /// + /// 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. + /// Whether the user has two-step login enabled. + private async Task ValidateTwoFactorAuthenticationPolicyAsync(User user, Guid organizationId, bool userTwoFactorEnabled) { if (userTwoFactorEnabled) { + // If the user has two-step login enabled, the policy is not enforced. return; } - bool requireTwoFactor; + var requirement = await _policyRequirementQuery.GetAsync(user.Id); + var canBeConfirmed = requirement.CanBeConfirmed(userTwoFactorEnabled, organizationId); - if (_featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements)) - { - var requirement = await _policyRequirementQuery.GetAsync(user.Id); - requireTwoFactor = requirement.RequireTwoFactor; - } - else - { - requireTwoFactor = (await _policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication)) - .Any(p => p.OrganizationId == organizationId); - } - - if (requireTwoFactor) + if (!canBeConfirmed) { throw new BadRequestException("User does not have two-step login enabled."); } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs index adc7b5d99d..deb18c2dd9 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.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.OrganizationUsers; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.Services; @@ -344,7 +345,15 @@ public class ConfirmOrganizationUserCommandTests userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements).Returns(true); policyRequirementQuery.GetAsync(user.Id) - .Returns(new RequireTwoFactorPolicyRequirement { RequireTwoFactor = true }); + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = org.Id, + OrganizationUserStatus = OrganizationUserStatusType.Accepted, + PolicyType = PolicyType.TwoFactorAuthentication + } + ])); twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(user.Id))) .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (user.Id, false) }); @@ -374,7 +383,15 @@ public class ConfirmOrganizationUserCommandTests userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements).Returns(true); policyRequirementQuery.GetAsync(user.Id) - .Returns(new RequireTwoFactorPolicyRequirement { RequireTwoFactor = false }); + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = Guid.NewGuid(), + OrganizationUserStatus = OrganizationUserStatusType.Invited, + PolicyType = PolicyType.TwoFactorAuthentication, + } + ])); twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(user.Id))) .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (user.Id, false) }); @@ -406,7 +423,15 @@ public class ConfirmOrganizationUserCommandTests userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements).Returns(true); policyRequirementQuery.GetAsync(user.Id) - .Returns(new RequireTwoFactorPolicyRequirement { RequireTwoFactor = true }); + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = org.Id, + OrganizationUserStatus = OrganizationUserStatusType.Accepted, + PolicyType = PolicyType.TwoFactorAuthentication + } + ])); twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(user.Id))) .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (user.Id, true) });