From 06a5888c7b7aa2870923c090eded811511d0300c Mon Sep 17 00:00:00 2001 From: Rui Tome Date: Tue, 20 May 2025 11:19:46 +0100 Subject: [PATCH] Update ConfirmOrganizationUserCommand to use RequireTwoFactorPolicyRequirement to check for 2FA requirement --- .../ConfirmOrganizationUserCommand.cs | 48 ++++++++-- .../ConfirmOrganizationUserCommandTests.cs | 95 +++++++++++++++++++ 2 files changed, 133 insertions(+), 10 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs index 9bfe8f791e..29a405aa4f 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs @@ -1,5 +1,6 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Enums; @@ -24,6 +25,8 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand private readonly IPushRegistrationService _pushRegistrationService; private readonly IPolicyService _policyService; private readonly IDeviceRepository _deviceRepository; + private readonly IPolicyRequirementQuery _policyRequirementQuery; + private readonly IFeatureService _featureService; public ConfirmOrganizationUserCommand( IOrganizationRepository organizationRepository, @@ -35,7 +38,9 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand IPushNotificationService pushNotificationService, IPushRegistrationService pushRegistrationService, IPolicyService policyService, - IDeviceRepository deviceRepository) + IDeviceRepository deviceRepository, + IPolicyRequirementQuery policyRequirementQuery, + IFeatureService featureService) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -47,6 +52,8 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand _pushRegistrationService = pushRegistrationService; _policyService = policyService; _deviceRepository = deviceRepository; + _policyRequirementQuery = policyRequirementQuery; + _featureService = featureService; } public async Task ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key, @@ -118,8 +125,8 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand } } - var twoFactorEnabled = usersTwoFactorEnabled.FirstOrDefault(tuple => tuple.userId == user.Id).twoFactorIsEnabled; - await CheckPoliciesAsync(organizationId, user, orgUsers, twoFactorEnabled); + var userTwoFactorEnabled = usersTwoFactorEnabled.FirstOrDefault(tuple => tuple.userId == user.Id).twoFactorIsEnabled; + await CheckPoliciesAsync(organizationId, user, orgUsers, userTwoFactorEnabled); orgUser.Status = OrganizationUserStatusType.Confirmed; orgUser.Key = keys[orgUser.Id]; orgUser.Email = null; @@ -142,15 +149,10 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand } private async Task CheckPoliciesAsync(Guid organizationId, User user, - ICollection userOrgs, bool twoFactorEnabled) + ICollection userOrgs, bool userTwoFactorEnabled) { // Enforce Two Factor Authentication Policy for this organization - var orgRequiresTwoFactor = (await _policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication)) - .Any(p => p.OrganizationId == organizationId); - if (orgRequiresTwoFactor && !twoFactorEnabled) - { - throw new BadRequestException("User does not have two-step login enabled."); - } + await CheckTwoFactorPolicyAsync(organizationId, user, userTwoFactorEnabled); var hasOtherOrgs = userOrgs.Any(ou => ou.OrganizationId != organizationId); var singleOrgPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.SingleOrg); @@ -168,6 +170,32 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand } } + private async Task CheckTwoFactorPolicyAsync(Guid organizationId, User user, bool userTwoFactorEnabled) + { + if (userTwoFactorEnabled) + { + return; + } + + bool requireTwoFactor; + + 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) + { + throw new BadRequestException("User does not have two-step login enabled."); + } + } + private async Task DeleteAndPushUserRegistrationAsync(Guid organizationId, Guid userId) { var devices = await GetUserDeviceIdsAsync(userId); diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs index 06335f668d..adc7b5d99d 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs @@ -1,6 +1,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Enums; @@ -321,4 +322,98 @@ public class ConfirmOrganizationUserCommandTests Assert.Contains("User does not have two-step login enabled.", result[1].Item2); Assert.Contains("Cannot confirm this member to the organization until they leave or remove all other organizations.", result[2].Item2); } + + [Theory, BitAutoData] + public async Task ConfirmUserAsync_WithPolicyRequirementsEnabled_AndTwoFactorRequired_ThrowsBadRequestException( + Organization org, OrganizationUser confirmingUser, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, User user, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + var organizationRepository = sutProvider.GetDependency(); + var userRepository = sutProvider.GetDependency(); + var featureService = sutProvider.GetDependency(); + var policyRequirementQuery = sutProvider.GetDependency(); + var twoFactorIsEnabledQuery = sutProvider.GetDependency(); + + org.PlanType = PlanType.EnterpriseAnnually; + orgUser.OrganizationId = confirmingUser.OrganizationId = org.Id; + orgUser.UserId = user.Id; + organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser }); + organizationRepository.GetByIdAsync(org.Id).Returns(org); + userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); + featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements).Returns(true); + policyRequirementQuery.GetAsync(user.Id) + .Returns(new RequireTwoFactorPolicyRequirement { RequireTwoFactor = true }); + twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(user.Id))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (user.Id, false) }); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, "key", confirmingUser.Id)); + Assert.Contains("User does not have two-step login enabled.", exception.Message); + } + + [Theory, BitAutoData] + public async Task ConfirmUserAsync_WithPolicyRequirementsEnabled_AndTwoFactorNotRequired_Succeeds( + Organization org, OrganizationUser confirmingUser, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, User user, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + var organizationRepository = sutProvider.GetDependency(); + var userRepository = sutProvider.GetDependency(); + var featureService = sutProvider.GetDependency(); + var policyRequirementQuery = sutProvider.GetDependency(); + var twoFactorIsEnabledQuery = sutProvider.GetDependency(); + + org.PlanType = PlanType.EnterpriseAnnually; + orgUser.OrganizationId = confirmingUser.OrganizationId = org.Id; + orgUser.UserId = user.Id; + organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser }); + organizationRepository.GetByIdAsync(org.Id).Returns(org); + userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); + featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements).Returns(true); + policyRequirementQuery.GetAsync(user.Id) + .Returns(new RequireTwoFactorPolicyRequirement { RequireTwoFactor = false }); + twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(user.Id))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (user.Id, false) }); + + await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, "key", confirmingUser.Id); + + await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Confirmed); + await sutProvider.GetDependency().Received(1).SendOrganizationConfirmedEmailAsync(org.DisplayName(), user.Email, orgUser.AccessSecretsManager); + await organizationUserRepository.Received(1).ReplaceManyAsync(Arg.Is>(users => users.Contains(orgUser) && users.Count == 1)); + } + + [Theory, BitAutoData] + public async Task ConfirmUserAsync_WithPolicyRequirementsEnabled_AndTwoFactorEnabled_Succeeds( + Organization org, OrganizationUser confirmingUser, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, User user, + SutProvider sutProvider) + { + var organizationUserRepository = sutProvider.GetDependency(); + var organizationRepository = sutProvider.GetDependency(); + var userRepository = sutProvider.GetDependency(); + var featureService = sutProvider.GetDependency(); + var policyRequirementQuery = sutProvider.GetDependency(); + var twoFactorIsEnabledQuery = sutProvider.GetDependency(); + + org.PlanType = PlanType.EnterpriseAnnually; + orgUser.OrganizationId = confirmingUser.OrganizationId = org.Id; + orgUser.UserId = user.Id; + organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser }); + organizationRepository.GetByIdAsync(org.Id).Returns(org); + userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); + featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements).Returns(true); + policyRequirementQuery.GetAsync(user.Id) + .Returns(new RequireTwoFactorPolicyRequirement { RequireTwoFactor = true }); + twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(user.Id))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (user.Id, true) }); + + await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, "key", confirmingUser.Id); + + await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Confirmed); + await sutProvider.GetDependency().Received(1).SendOrganizationConfirmedEmailAsync(org.DisplayName(), user.Email, orgUser.AccessSecretsManager); + await organizationUserRepository.Received(1).ReplaceManyAsync(Arg.Is>(users => users.Contains(orgUser) && users.Count == 1)); + } }