diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index f3426efddc..3770d867cf 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -1,4 +1,6 @@ using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; @@ -27,6 +29,8 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand private readonly IUserRepository _userRepository; private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory; + private readonly IFeatureService _featureService; + private readonly IPolicyRequirementQuery _policyRequirementQuery; public AcceptOrgUserCommand( IDataProtectionProvider dataProtectionProvider, @@ -37,9 +41,10 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand IMailService mailService, IUserRepository userRepository, ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, - IDataProtectorTokenFactory orgUserInviteTokenDataFactory) + IDataProtectorTokenFactory orgUserInviteTokenDataFactory, + IFeatureService featureService, + IPolicyRequirementQuery policyRequirementQuery) { - // TODO: remove data protector when old token validation removed _dataProtector = dataProtectionProvider.CreateProtector(OrgUserInviteTokenable.DataProtectorPurpose); _globalSettings = globalSettings; @@ -50,6 +55,8 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand _userRepository = userRepository; _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; + _featureService = featureService; + _policyRequirementQuery = policyRequirementQuery; } public async Task AcceptOrgUserByEmailTokenAsync(Guid organizationUserId, User user, string emailToken, @@ -196,15 +203,7 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand } // Enforce Two Factor Authentication Policy of organization user is trying to join - 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."); - } - } + await ValidateTwoFactorAuthenticationPolicyAsync(user, orgUser.OrganizationId); orgUser.Status = OrganizationUserStatusType.Accepted; orgUser.UserId = user.Id; @@ -224,4 +223,33 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand return orgUser; } + private async Task ValidateTwoFactorAuthenticationPolicyAsync(User user, Guid organizationId) + { + if (_featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements)) + { + if (await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user)) + { + // If the user has two-step login enabled, we skip checking the 2FA policy + return; + } + + var twoFactorPolicyRequirement = await _policyRequirementQuery.GetAsync(user.Id); + if (twoFactorPolicyRequirement.IsTwoFactorRequiredForOrganization(organizationId)) + { + throw new BadRequestException("You cannot join this organization until you enable two-step login on your user account."); + } + + return; + } + + if (!await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user)) + { + 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."); + } + } + } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs index 9bfe8f791e..806cf5a533 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs @@ -1,5 +1,7 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Enums; @@ -24,6 +26,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 +39,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 +53,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 +126,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 +150,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 ValidateTwoFactorAuthenticationPolicyAsync(user, organizationId, userTwoFactorEnabled); var hasOtherOrgs = userOrgs.Any(ou => ou.OrganizationId != organizationId); var singleOrgPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.SingleOrg); @@ -168,6 +171,33 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand } } + private async Task ValidateTwoFactorAuthenticationPolicyAsync(User user, Guid organizationId, bool userTwoFactorEnabled) + { + if (_featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements)) + { + if (userTwoFactorEnabled) + { + // If the user has two-step login enabled, we skip checking the 2FA policy + return; + } + + var twoFactorPolicyRequirement = await _policyRequirementQuery.GetAsync(user.Id); + if (twoFactorPolicyRequirement.IsTwoFactorRequiredForOrganization(organizationId)) + { + throw new BadRequestException("User does not have two-step login enabled."); + } + + return; + } + + 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."); + } + } + private async Task DeleteAndPushUserRegistrationAsync(Guid organizationId, Guid userId) { var devices = await GetUserDeviceIdsAsync(userId); diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs index 74165a5a71..fe19cd1389 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs @@ -1,5 +1,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Enums; @@ -22,7 +24,9 @@ public class RestoreOrganizationUserCommand( ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, IPolicyService policyService, IUserRepository userRepository, - IOrganizationService organizationService) : IRestoreOrganizationUserCommand + IOrganizationService organizationService, + IFeatureService featureService, + IPolicyRequirementQuery policyRequirementQuery) : IRestoreOrganizationUserCommand { public async Task RestoreUserAsync(OrganizationUser organizationUser, Guid? restoringUserId) { @@ -270,12 +274,7 @@ public class RestoreOrganizationUserCommand( // Enforce 2FA Policy of organization user is trying to join if (!userHasTwoFactorEnabled) { - var invitedTwoFactorPolicies = await policyService.GetPoliciesApplicableToUserAsync(userId, - PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Revoked); - if (invitedTwoFactorPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) - { - twoFactorCompliant = false; - } + twoFactorCompliant = !await IsTwoFactorRequiredForOrganizationAsync(userId, orgUser.OrganizationId); } var user = await userRepository.GetByIdAsync(userId); @@ -299,4 +298,17 @@ public class RestoreOrganizationUserCommand( throw new BadRequestException(user.Email + " is not compliant with the two-step login policy"); } } + + private async Task IsTwoFactorRequiredForOrganizationAsync(Guid userId, Guid organizationId) + { + if (featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements)) + { + var requirement = await policyRequirementQuery.GetAsync(userId); + return requirement.IsTwoFactorRequiredForOrganization(organizationId); + } + + var invitedTwoFactorPolicies = await policyService.GetPoliciesApplicableToUserAsync(userId, + PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Revoked); + return invitedTwoFactorPolicies.Any(p => p.OrganizationId == organizationId); + } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/RequireTwoFactorPolicyRequirement.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/RequireTwoFactorPolicyRequirement.cs new file mode 100644 index 0000000000..bbc997a83d --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/RequireTwoFactorPolicyRequirement.cs @@ -0,0 +1,52 @@ +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.Enums; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; + +/// +/// Policy requirements for the Require Two-Factor Authentication policy. +/// +public class RequireTwoFactorPolicyRequirement : IPolicyRequirement +{ + private readonly IEnumerable _policyDetails; + + public RequireTwoFactorPolicyRequirement(IEnumerable policyDetails) + { + _policyDetails = policyDetails; + } + + /// + /// Checks if two-factor authentication is required for the organization due to an active policy. + /// + /// The ID of the organization to check. + /// True if two-factor authentication is required for the organization, false otherwise. + /// + /// This should be used to check whether the member needs to have 2FA enabled before being + /// accepted, confirmed, or restored to the organization. + /// + public bool IsTwoFactorRequiredForOrganization(Guid organizationId) => + _policyDetails.Any(p => p.OrganizationId == organizationId); + + /// + /// Returns tuples of (OrganizationId, OrganizationUserId) for active memberships where two-factor authentication is required. + /// Users should be revoked from these organizations if they disable all 2FA methods. + /// + public IEnumerable<(Guid OrganizationId, Guid OrganizationUserId)> OrganizationsRequiringTwoFactor => + _policyDetails + .Where(p => p.OrganizationUserStatus is + OrganizationUserStatusType.Accepted or + OrganizationUserStatusType.Confirmed) + .Select(p => (p.OrganizationId, p.OrganizationUserId)); +} + +public class RequireTwoFactorPolicyRequirementFactory : BasePolicyRequirementFactory +{ + public override PolicyType PolicyType => PolicyType.TwoFactorAuthentication; + protected override IEnumerable ExemptStatuses => []; + + public override RequireTwoFactorPolicyRequirement Create(IEnumerable policyDetails) + { + return new RequireTwoFactorPolicyRequirement(policyDetails); + } +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyServiceCollectionExtensions.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyServiceCollectionExtensions.cs index 1be0e61af7..f98135b70d 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyServiceCollectionExtensions.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyServiceCollectionExtensions.cs @@ -36,5 +36,6 @@ public static class PolicyServiceCollectionExtensions services.AddScoped, ResetPasswordPolicyRequirementFactory>(); services.AddScoped, PersonalOwnershipPolicyRequirementFactory>(); services.AddScoped, RequireSsoPolicyRequirementFactory>(); + services.AddScoped, RequireTwoFactorPolicyRequirementFactory>(); } } diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 26ff421328..7640a82fcb 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -14,7 +14,6 @@ using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Repositories; -using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Constants; using Bit.Core.Billing.Enums; using Bit.Core.Billing.Extensions; @@ -45,7 +44,6 @@ public class OrganizationService : IOrganizationService private readonly IOrganizationRepository _organizationRepository; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly ICollectionRepository _collectionRepository; - private readonly IUserRepository _userRepository; private readonly IGroupRepository _groupRepository; private readonly IMailService _mailService; private readonly IPushNotificationService _pushNotificationService; @@ -69,7 +67,6 @@ public class OrganizationService : IOrganizationService private readonly IUpdateSecretsManagerSubscriptionCommand _updateSecretsManagerSubscriptionCommand; private readonly IProviderRepository _providerRepository; private readonly IFeatureService _featureService; - private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; private readonly IHasConfirmedOwnersExceptQuery _hasConfirmedOwnersExceptQuery; private readonly IPricingClient _pricingClient; private readonly IPolicyRequirementQuery _policyRequirementQuery; @@ -79,7 +76,6 @@ public class OrganizationService : IOrganizationService IOrganizationRepository organizationRepository, IOrganizationUserRepository organizationUserRepository, ICollectionRepository collectionRepository, - IUserRepository userRepository, IGroupRepository groupRepository, IMailService mailService, IPushNotificationService pushNotificationService, @@ -103,7 +99,6 @@ public class OrganizationService : IOrganizationService IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand, IProviderRepository providerRepository, IFeatureService featureService, - ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery, IPricingClient pricingClient, IPolicyRequirementQuery policyRequirementQuery, @@ -113,7 +108,6 @@ public class OrganizationService : IOrganizationService _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; _collectionRepository = collectionRepository; - _userRepository = userRepository; _groupRepository = groupRepository; _mailService = mailService; _pushNotificationService = pushNotificationService; @@ -137,7 +131,6 @@ public class OrganizationService : IOrganizationService _updateSecretsManagerSubscriptionCommand = updateSecretsManagerSubscriptionCommand; _providerRepository = providerRepository; _featureService = featureService; - _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; _hasConfirmedOwnersExceptQuery = hasConfirmedOwnersExceptQuery; _pricingClient = pricingClient; _policyRequirementQuery = policyRequirementQuery; @@ -1722,72 +1715,6 @@ public class OrganizationService : IOrganizationService return result; } - private async Task CheckPoliciesBeforeRestoreAsync(OrganizationUser orgUser, bool userHasTwoFactorEnabled) - { - // An invited OrganizationUser isn't linked with a user account yet, so these checks are irrelevant - // The user will be subject to the same checks when they try to accept the invite - if (GetPriorActiveOrganizationUserStatusType(orgUser) == OrganizationUserStatusType.Invited) - { - return; - } - - var userId = orgUser.UserId.Value; - - // Enforce Single Organization Policy of organization user is being restored to - var allOrgUsers = await _organizationUserRepository.GetManyByUserAsync(userId); - var hasOtherOrgs = allOrgUsers.Any(ou => ou.OrganizationId != orgUser.OrganizationId); - var singleOrgPoliciesApplyingToRevokedUsers = await _policyService.GetPoliciesApplicableToUserAsync(userId, - PolicyType.SingleOrg, OrganizationUserStatusType.Revoked); - var singleOrgPolicyApplies = singleOrgPoliciesApplyingToRevokedUsers.Any(p => p.OrganizationId == orgUser.OrganizationId); - - var singleOrgCompliant = true; - var belongsToOtherOrgCompliant = true; - var twoFactorCompliant = true; - - if (hasOtherOrgs && singleOrgPolicyApplies) - { - singleOrgCompliant = false; - } - - // Enforce Single Organization Policy of other organizations user is a member of - var anySingleOrgPolicies = await _policyService.AnyPoliciesApplicableToUserAsync(userId, - PolicyType.SingleOrg); - if (anySingleOrgPolicies) - { - belongsToOtherOrgCompliant = false; - } - - // Enforce Two Factor Authentication Policy of organization user is trying to join - if (!userHasTwoFactorEnabled) - { - var invitedTwoFactorPolicies = await _policyService.GetPoliciesApplicableToUserAsync(userId, - PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Revoked); - if (invitedTwoFactorPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) - { - twoFactorCompliant = false; - } - } - - var user = await _userRepository.GetByIdAsync(userId); - - if (!singleOrgCompliant && !twoFactorCompliant) - { - throw new BadRequestException(user.Email + " is not compliant with the single organization and two-step login polciy"); - } - else if (!singleOrgCompliant) - { - throw new BadRequestException(user.Email + " is not compliant with the single organization policy"); - } - else if (!belongsToOtherOrgCompliant) - { - throw new BadRequestException(user.Email + " belongs to an organization that doesn't allow them to join multiple organizations"); - } - else if (!twoFactorCompliant) - { - throw new BadRequestException(user.Email + " is not compliant with the two-step login policy"); - } - } - public static OrganizationUserStatusType GetPriorActiveOrganizationUserStatusType(OrganizationUser organizationUser) { // Determine status to revert back to diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 76520b4085..f0c97b8589 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -6,6 +6,8 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Enums; @@ -81,6 +83,7 @@ public class UserService : UserManager, IUserService, IDisposable private readonly IRevokeNonCompliantOrganizationUserCommand _revokeNonCompliantOrganizationUserCommand; private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; private readonly IDistributedCache _distributedCache; + private readonly IPolicyRequirementQuery _policyRequirementQuery; public UserService( IUserRepository userRepository, @@ -119,7 +122,8 @@ public class UserService : UserManager, IUserService, IDisposable IRemoveOrganizationUserCommand removeOrganizationUserCommand, IRevokeNonCompliantOrganizationUserCommand revokeNonCompliantOrganizationUserCommand, ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, - IDistributedCache distributedCache) + IDistributedCache distributedCache, + IPolicyRequirementQuery policyRequirementQuery) : base( store, optionsAccessor, @@ -164,6 +168,7 @@ public class UserService : UserManager, IUserService, IDisposable _revokeNonCompliantOrganizationUserCommand = revokeNonCompliantOrganizationUserCommand; _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; _distributedCache = distributedCache; + _policyRequirementQuery = policyRequirementQuery; } public Guid? GetProperUserId(ClaimsPrincipal principal) @@ -1394,9 +1399,40 @@ public class UserService : UserManager, IUserService, IDisposable private async Task CheckPoliciesOnTwoFactorRemovalAsync(User user) { + if (_featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements)) + { + var requirement = await _policyRequirementQuery.GetAsync(user.Id); + if (!requirement.OrganizationsRequiringTwoFactor.Any()) + { + Logger.LogInformation("No organizations requiring two factor for user {userId}.", user.Id); + return; + } + + var organizationIds = requirement.OrganizationsRequiringTwoFactor.Select(o => o.OrganizationId).ToList(); + var organizations = await _organizationRepository.GetManyByIdsAsync(organizationIds); + var organizationLookup = organizations.ToDictionary(org => org.Id); + + var revokeOrgUserTasks = requirement.OrganizationsRequiringTwoFactor + .Where(o => organizationLookup.ContainsKey(o.OrganizationId)) + .Select(async o => + { + var organization = organizationLookup[o.OrganizationId]; + await _revokeNonCompliantOrganizationUserCommand.RevokeNonCompliantOrganizationUsersAsync( + new RevokeOrganizationUsersRequest( + o.OrganizationId, + [new OrganizationUserUserDetails { Id = o.OrganizationUserId, OrganizationId = o.OrganizationId }], + new SystemUser(EventSystemUser.TwoFactorDisabled))); + await _mailService.SendOrganizationUserRevokedForTwoFactorPolicyEmailAsync(organization.DisplayName(), user.Email); + }).ToArray(); + + await Task.WhenAll(revokeOrgUserTasks); + + return; + } + var twoFactorPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication); - var removeOrgUserTasks = twoFactorPolicies.Select(async p => + var legacyRevokeOrgUserTasks = twoFactorPolicies.Select(async p => { var organization = await _organizationRepository.GetByIdAsync(p.OrganizationId); await _revokeNonCompliantOrganizationUserCommand.RevokeNonCompliantOrganizationUsersAsync( @@ -1407,7 +1443,7 @@ public class UserService : UserManager, IUserService, IDisposable await _mailService.SendOrganizationUserRevokedForTwoFactorPolicyEmailAsync(organization.DisplayName(), user.Email); }).ToArray(); - await Task.WhenAll(removeOrgUserTasks); + await Task.WhenAll(legacyRevokeOrgUserTasks); } public override async Task ConfirmEmailAsync(User user, string token) diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index baf844acae..540bac4d1c 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -1,5 +1,8 @@ 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.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; @@ -29,7 +32,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(); @@ -166,9 +168,6 @@ public class AcceptOrgUserCommandTests // Arrange SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); - // User doesn't have 2FA enabled - _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); - // Organization they are trying to join requires 2FA var twoFactorPolicy = new OrganizationUserPolicyDetails { OrganizationId = orgUser.OrganizationId }; sutProvider.GetDependency() @@ -185,6 +184,107 @@ public class AcceptOrgUserCommandTests exception.Message); } + [Theory] + [BitAutoData] + public async Task AcceptOrgUserAsync_WithPolicyRequirementsEnabled_UserWithout2FAJoining2FARequiredOrg_ThrowsBadRequest( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + + // Organization they are trying to join requires 2FA + sutProvider.GetDependency() + .GetAsync(user.Id) + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = orgUser.OrganizationId, + OrganizationUserStatus = OrganizationUserStatusType.Invited, + PolicyType = PolicyType.TwoFactorAuthentication, + } + ])); + + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); + + Assert.Equal("You cannot join this organization until you enable two-step login on your user account.", + exception.Message); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserAsync_WithPolicyRequirementsEnabled_UserWith2FAJoining2FARequiredOrg_Succeeds( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + + // User has 2FA enabled + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(user) + .Returns(true); + + // Organization they are trying to join requires 2FA + sutProvider.GetDependency() + .GetAsync(user.Id) + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = orgUser.OrganizationId, + OrganizationUserStatus = OrganizationUserStatusType.Invited, + PolicyType = PolicyType.TwoFactorAuthentication, + } + ])); + + await sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService); + + await sutProvider.GetDependency() + .Received(1) + .ReplaceAsync(Arg.Is(ou => ou.Status == OrganizationUserStatusType.Accepted)); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserAsync_WithPolicyRequirementsEnabled_UserJoiningOrgWithout2FARequirement_Succeeds( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + + // Organization they are trying to join doesn't require 2FA + sutProvider.GetDependency() + .GetAsync(user.Id) + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = Guid.NewGuid(), + OrganizationUserStatus = OrganizationUserStatusType.Invited, + PolicyType = PolicyType.TwoFactorAuthentication, + } + ])); + + await sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService); + + await sutProvider.GetDependency() + .Received(1) + .ReplaceAsync(Arg.Is(ou => ou.Status == OrganizationUserStatusType.Accepted)); + } + [Theory] [BitAutoData(OrganizationUserType.Admin)] [BitAutoData(OrganizationUserType.Owner)] @@ -647,9 +747,6 @@ public class AcceptOrgUserCommandTests .AnyPoliciesApplicableToUserAsync(user.Id, PolicyType.SingleOrg) .Returns(false); - // User doesn't have 2FA enabled - _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); - // Org does not require 2FA sutProvider.GetDependency().GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited) diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs index 06335f668d..366d8cb2d6 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs @@ -1,6 +1,9 @@ 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.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Enums; @@ -321,4 +324,122 @@ 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( + [ + 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) }); + + 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( + [ + 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) }); + + 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( + [ + 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) }); + + 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)); + } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs index d6880a3a12..fbd711307c 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs @@ -1,6 +1,9 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Enums; @@ -208,6 +211,57 @@ public class RestoreOrganizationUserCommandTests .PushSyncOrgKeysAsync(Arg.Any()); } + [Theory, BitAutoData] + public async Task RestoreUser_WithPolicyRequirementsEnabled_With2FAPolicyEnabled_WithoutUser2FAConfigured_Fails( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, + SutProvider sutProvider) + { + organizationUser.Email = null; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, false) }); + + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .GetAsync(organizationUser.UserId.Value) + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = organizationUser.OrganizationId, + OrganizationUserStatus = OrganizationUserStatusType.Revoked, + PolicyType = PolicyType.TwoFactorAuthentication + } + ])); + + var user = new User(); + user.Email = "test@bitwarden.com"; + sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); + + Assert.Contains("test@bitwarden.com is not compliant with the two-step login policy", exception.Message.ToLowerInvariant()); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); + } + [Theory, BitAutoData] public async Task RestoreUser_With2FAPolicyEnabled_WithUser2FAConfigured_Success( Organization organization, @@ -235,6 +289,46 @@ public class RestoreOrganizationUserCommandTests .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); } + [Theory, BitAutoData] + public async Task RestoreUser_WithPolicyRequirementsEnabled_With2FAPolicyEnabled_WithUser2FAConfigured_Success( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, + SutProvider sutProvider) + { + organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, true) }); + sutProvider.GetDependency() + .GetAsync(organizationUser.UserId.Value) + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = organizationUser.OrganizationId, + OrganizationUserStatus = OrganizationUserStatusType.Revoked, + PolicyType = PolicyType.TwoFactorAuthentication + } + ])); + + await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); + + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Confirmed); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); + } + [Theory, BitAutoData] public async Task RestoreUser_WithSingleOrgPolicyEnabled_Fails( Organization organization, @@ -277,45 +371,6 @@ public class RestoreOrganizationUserCommandTests .PushSyncOrgKeysAsync(Arg.Any()); } - [Theory, BitAutoData] - public async Task RestoreUser_vNext_WithOtherOrganizationSingleOrgPolicyEnabled_Fails( - Organization organization, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, - [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser secondOrganizationUser, - SutProvider sutProvider) - { - organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke - secondOrganizationUser.UserId = organizationUser.UserId; - RestoreUser_Setup(organization, owner, organizationUser, sutProvider); - - sutProvider.GetDependency() - .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) - .Returns(new List<(Guid userId, bool twoFactorIsEnabled)> { (organizationUser.UserId.Value, true) }); - - sutProvider.GetDependency() - .AnyPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) - .Returns(true); - - var user = new User { Email = "test@bitwarden.com" }; - sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); - - Assert.Contains("test@bitwarden.com belongs to an organization that doesn't allow them to join multiple organizations", exception.Message.ToLowerInvariant()); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .RestoreAsync(Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .PushSyncOrgKeysAsync(Arg.Any()); - } - [Theory, BitAutoData] public async Task RestoreUser_WithSingleOrgPolicyEnabled_And_2FA_Policy_Fails( Organization organization, @@ -364,20 +419,42 @@ public class RestoreOrganizationUserCommandTests } [Theory, BitAutoData] - public async Task RestoreUser_vNext_With2FAPolicyEnabled_WithoutUser2FAConfigured_Fails( + public async Task RestoreUser_WithPolicyRequirementsEnabled_WithSingleOrgPolicyEnabled_And_2FA_Policy_Fails( Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser secondOrganizationUser, SutProvider sutProvider) { - organizationUser.Email = null; - + organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke + secondOrganizationUser.UserId = organizationUser.UserId; RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + + sutProvider.GetDependency() + .GetManyByUserAsync(organizationUser.UserId.Value) + .Returns(new[] { organizationUser, secondOrganizationUser }); sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) - .Returns([new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } - ]); + .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) + .Returns(new[] + { + new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.SingleOrg, OrganizationUserStatus = OrganizationUserStatusType.Revoked } + }); + + sutProvider.GetDependency() + .GetAsync(organizationUser.UserId.Value) + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = organizationUser.OrganizationId, + OrganizationUserStatus = OrganizationUserStatusType.Revoked, + PolicyType = PolicyType.TwoFactorAuthentication + } + ])); var user = new User { Email = "test@bitwarden.com" }; sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); @@ -385,7 +462,7 @@ public class RestoreOrganizationUserCommandTests var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); - Assert.Contains("test@bitwarden.com is not compliant with the two-step login policy", exception.Message.ToLowerInvariant()); + Assert.Contains("test@bitwarden.com is not compliant with the single organization and two-step login policy", exception.Message.ToLowerInvariant()); await sutProvider.GetDependency() .DidNotReceiveWithAnyArgs() @@ -398,35 +475,6 @@ public class RestoreOrganizationUserCommandTests .PushSyncOrgKeysAsync(Arg.Any()); } - [Theory, BitAutoData] - public async Task RestoreUser_vNext_With2FAPolicyEnabled_WithUser2FAConfigured_Success( - Organization organization, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, - SutProvider sutProvider) - { - organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke - RestoreUser_Setup(organization, owner, organizationUser, sutProvider); - - sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) - .Returns([new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } - ]); - - sutProvider.GetDependency() - .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) - .Returns(new List<(Guid userId, bool twoFactorIsEnabled)> { (organizationUser.UserId.Value, true) }); - - await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); - - await sutProvider.GetDependency() - .Received(1) - .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Confirmed); - await sutProvider.GetDependency() - .Received(1) - .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); - } - [Theory, BitAutoData] public async Task RestoreUser_WhenUserOwningAnotherFreeOrganization_ThenRestoreUserFails( Organization organization, @@ -672,6 +720,77 @@ public class RestoreOrganizationUserCommandTests .RestoreAsync(orgUser3.Id, OrganizationUserStatusType.Invited); } + [Theory, BitAutoData] + public async Task RestoreUsers_WithPolicyRequirementsEnabled_With2FAPolicy_BlocksNonCompliantUser(Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser1, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser2, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser3, + SutProvider sutProvider) + { + // Arrange + RestoreUser_Setup(organization, owner, orgUser1, sutProvider); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + + var organizationUserRepository = sutProvider.GetDependency(); + var userRepository = sutProvider.GetDependency(); + var policyService = sutProvider.GetDependency(); + var userService = Substitute.For(); + + orgUser1.Email = orgUser2.Email = null; + orgUser3.UserId = null; + orgUser3.Key = null; + orgUser1.OrganizationId = orgUser2.OrganizationId = orgUser3.OrganizationId = organization.Id; + organizationUserRepository + .GetManyAsync(Arg.Is>(ids => ids.Contains(orgUser1.Id) && ids.Contains(orgUser2.Id) && ids.Contains(orgUser3.Id))) + .Returns(new[] { orgUser1, orgUser2, orgUser3 }); + + userRepository.GetByIdAsync(orgUser2.UserId!.Value).Returns(new User { Email = "test@example.com" }); + + // Setup 2FA policy + sutProvider.GetDependency() + .GetAsync(Arg.Any()) + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = organization.Id, + OrganizationUserStatus = OrganizationUserStatusType.Revoked, + PolicyType = PolicyType.TwoFactorAuthentication + } + ])); + + // User1 has 2FA, User2 doesn't + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(orgUser1.UserId!.Value) && ids.Contains(orgUser2.UserId!.Value))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)> + { + (orgUser1.UserId!.Value, true), + (orgUser2.UserId!.Value, false) + }); + + // Act + var result = await sutProvider.Sut.RestoreUsersAsync(organization.Id, [orgUser1.Id, orgUser2.Id, orgUser3.Id], owner.Id, userService); + + // Assert + Assert.Equal(3, result.Count); + Assert.Empty(result[0].Item2); // First user should succeed + Assert.Contains("two-step login", result[1].Item2); // Second user should fail + Assert.Empty(result[2].Item2); // Third user should succeed + await organizationUserRepository + .Received(1) + .RestoreAsync(orgUser1.Id, OrganizationUserStatusType.Confirmed); + await organizationUserRepository + .DidNotReceive() + .RestoreAsync(orgUser2.Id, Arg.Any()); + await organizationUserRepository + .Received(1) + .RestoreAsync(orgUser3.Id, OrganizationUserStatusType.Invited); + } + [Theory, BitAutoData] public async Task RestoreUsers_UserOwnsAnotherFreeOrganization_BlocksOwnerUserFromBeingRestored(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/RequireTwoFactorPolicyRequirementFactoryTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/RequireTwoFactorPolicyRequirementFactoryTests.cs new file mode 100644 index 0000000000..c20ea494ab --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/RequireTwoFactorPolicyRequirementFactoryTests.cs @@ -0,0 +1,117 @@ +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; +using Bit.Core.Enums; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; + +[SutProviderCustomize] +public class RequireTwoFactorPolicyRequirementFactoryTests +{ + [Theory] + [BitAutoData] + public void IsTwoFactorRequiredForOrganization_WithNoPolicies_ReturnsFalse( + Guid organizationId, + SutProvider sutProvider) + { + var actual = sutProvider.Sut.Create([]); + + Assert.False(actual.IsTwoFactorRequiredForOrganization(organizationId)); + } + + [Theory] + [BitAutoData] + public void IsTwoFactorRequiredForOrganization_WithOrganizationPolicy_ReturnsTrue( + Guid organizationId, + SutProvider sutProvider) + { + var actual = sutProvider.Sut.Create( + [ + new PolicyDetails + { + OrganizationId = organizationId, + PolicyType = PolicyType.TwoFactorAuthentication, + } + ]); + + Assert.True(actual.IsTwoFactorRequiredForOrganization(organizationId)); + } + + [Theory] + [BitAutoData] + public void IsTwoFactorRequiredForOrganization_WithOtherOrganizationPolicy_ReturnsFalse( + Guid organizationId, + SutProvider sutProvider) + { + var actual = sutProvider.Sut.Create( + [ + new PolicyDetails + { + OrganizationId = Guid.NewGuid(), + PolicyType = PolicyType.TwoFactorAuthentication, + }, + ]); + + Assert.False(actual.IsTwoFactorRequiredForOrganization(organizationId)); + } + + [Theory, BitAutoData] + public void OrganizationsRequiringTwoFactor_WithNoPolicies_ReturnsEmptyCollection( + SutProvider sutProvider) + { + var actual = sutProvider.Sut.Create([]); + + Assert.Empty(actual.OrganizationsRequiringTwoFactor); + } + + [Theory, BitAutoData] + public void OrganizationsRequiringTwoFactor_WithMultiplePolicies_ReturnsActiveMemberships( + Guid orgId1, Guid orgUserId1, Guid orgId2, Guid orgUserId2, + Guid orgId3, Guid orgUserId3, Guid orgId4, Guid orgUserId4, + SutProvider sutProvider) + { + var policies = new[] + { + new PolicyDetails + { + OrganizationId = orgId1, + OrganizationUserId = orgUserId1, + PolicyType = PolicyType.TwoFactorAuthentication, + OrganizationUserStatus = OrganizationUserStatusType.Accepted + }, + new PolicyDetails + { + OrganizationId = orgId2, + OrganizationUserId = orgUserId2, + PolicyType = PolicyType.TwoFactorAuthentication, + OrganizationUserStatus = OrganizationUserStatusType.Confirmed + }, + new PolicyDetails + { + OrganizationId = orgId3, + OrganizationUserId = orgUserId3, + PolicyType = PolicyType.TwoFactorAuthentication, + OrganizationUserStatus = OrganizationUserStatusType.Invited + }, + new PolicyDetails + { + OrganizationId = orgId4, + OrganizationUserId = orgUserId4, + PolicyType = PolicyType.TwoFactorAuthentication, + OrganizationUserStatus = OrganizationUserStatusType.Revoked + } + }; + + var actual = sutProvider.Sut.Create(policies); + + var result = actual.OrganizationsRequiringTwoFactor.ToList(); + Assert.Equal(2, result.Count); + Assert.Contains(result, p => p.OrganizationId == orgId1 && p.OrganizationUserId == orgUserId1); + Assert.Contains(result, p => p.OrganizationId == orgId2 && p.OrganizationUserId == orgUserId2); + Assert.DoesNotContain(result, p => p.OrganizationId == orgId3 && p.OrganizationUserId == orgUserId3); + Assert.DoesNotContain(result, p => p.OrganizationId == orgId4 && p.OrganizationUserId == orgUserId4); + } +} diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index ac7f6e4018..a5bfe35152 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -2,8 +2,11 @@ using System.Text.Json; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Enums; @@ -326,7 +329,8 @@ public class UserServiceTests sutProvider.GetDependency(), sutProvider.GetDependency(), sutProvider.GetDependency(), - sutProvider.GetDependency() + sutProvider.GetDependency(), + sutProvider.GetDependency() ); var actualIsVerified = await sut.VerifySecretAsync(user, secret); @@ -462,6 +466,78 @@ public class UserServiceTests .SendOrganizationUserRevokedForTwoFactorPolicyEmailAsync(organization2.DisplayName(), user.Email); } + [Theory, BitAutoData] + public async Task DisableTwoFactorProviderAsync_WithPolicyRequirementsEnabled_WhenOrganizationHas2FAPolicyEnabled_DisablingAllProviders_RevokesUserAndSendsEmail( + SutProvider sutProvider, User user, + Organization organization1, Guid organizationUserId1, + Organization organization2, Guid organizationUserId2) + { + user.SetTwoFactorProviders(new Dictionary + { + [TwoFactorProviderType.Email] = new() { Enabled = true } + }); + organization1.Enabled = organization2.Enabled = true; + organization1.UseSso = organization2.UseSso = true; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + sutProvider.GetDependency() + .GetAsync(user.Id) + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = organization1.Id, + OrganizationUserId = organizationUserId1, + OrganizationUserStatus = OrganizationUserStatusType.Accepted, + PolicyType = PolicyType.TwoFactorAuthentication + }, + new PolicyDetails + { + OrganizationId = organization2.Id, + OrganizationUserId = organizationUserId2, + OrganizationUserStatus = OrganizationUserStatusType.Confirmed, + PolicyType = PolicyType.TwoFactorAuthentication + } + ])); + sutProvider.GetDependency() + .GetManyByIdsAsync(Arg.Is>(ids => ids.Contains(organization1.Id) && ids.Contains(organization2.Id))) + .Returns(new[] { organization1, organization2 }); + var expectedSavedProviders = JsonHelpers.LegacySerialize(new Dictionary(), JsonHelpers.LegacyEnumKeyResolver); + + await sutProvider.Sut.DisableTwoFactorProviderAsync(user, TwoFactorProviderType.Email); + + await sutProvider.GetDependency() + .Received(1) + .ReplaceAsync(Arg.Is(u => u.Id == user.Id && u.TwoFactorProviders == expectedSavedProviders)); + await sutProvider.GetDependency() + .Received(1) + .LogUserEventAsync(user.Id, EventType.User_Disabled2fa); + + // Revoke the user from the first organization + await sutProvider.GetDependency() + .Received(1) + .RevokeNonCompliantOrganizationUsersAsync( + Arg.Is(r => r.OrganizationId == organization1.Id && + r.OrganizationUsers.First().Id == organizationUserId1 && + r.OrganizationUsers.First().OrganizationId == organization1.Id)); + await sutProvider.GetDependency() + .Received(1) + .SendOrganizationUserRevokedForTwoFactorPolicyEmailAsync(organization1.DisplayName(), user.Email); + + // Remove the user from the second organization + await sutProvider.GetDependency() + .Received(1) + .RevokeNonCompliantOrganizationUsersAsync( + Arg.Is(r => r.OrganizationId == organization2.Id && + r.OrganizationUsers.First().Id == organizationUserId2 && + r.OrganizationUsers.First().OrganizationId == organization2.Id)); + await sutProvider.GetDependency() + .Received(1) + .SendOrganizationUserRevokedForTwoFactorPolicyEmailAsync(organization2.DisplayName(), user.Email); + } + [Theory, BitAutoData] public async Task DisableTwoFactorProviderAsync_UserHasOneProviderEnabled_DoesNotRevokeUserFromOrganization( SutProvider sutProvider, User user, Organization organization) @@ -509,6 +585,53 @@ public class UserServiceTests .SendOrganizationUserRevokedForTwoFactorPolicyEmailAsync(default, default); } + [Theory, BitAutoData] + public async Task DisableTwoFactorProviderAsync_WithPolicyRequirementsEnabled_UserHasOneProviderEnabled_DoesNotRevokeUserFromOrganization( + SutProvider sutProvider, User user, Organization organization) + { + user.SetTwoFactorProviders(new Dictionary + { + [TwoFactorProviderType.Email] = new() { Enabled = true }, + [TwoFactorProviderType.Remember] = new() { Enabled = true } + }); + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + sutProvider.GetDependency() + .GetAsync(user.Id) + .Returns(new RequireTwoFactorPolicyRequirement( + [ + new PolicyDetails + { + OrganizationId = organization.Id, + OrganizationUserStatus = OrganizationUserStatusType.Accepted, + PolicyType = PolicyType.TwoFactorAuthentication + } + ])); + sutProvider.GetDependency() + .GetByIdAsync(organization.Id) + .Returns(organization); + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(user) + .Returns(true); + var expectedSavedProviders = JsonHelpers.LegacySerialize(new Dictionary + { + [TwoFactorProviderType.Remember] = new() { Enabled = true } + }, JsonHelpers.LegacyEnumKeyResolver); + + await sutProvider.Sut.DisableTwoFactorProviderAsync(user, TwoFactorProviderType.Email); + + await sutProvider.GetDependency() + .Received(1) + .ReplaceAsync(Arg.Is(u => u.Id == user.Id && u.TwoFactorProviders == expectedSavedProviders)); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RevokeNonCompliantOrganizationUsersAsync(default); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .SendOrganizationUserRevokedForTwoFactorPolicyEmailAsync(default, default); + } + [Theory, BitAutoData] public async Task ResendNewDeviceVerificationEmail_UserNull_SendTwoFactorEmailAsyncNotCalled( SutProvider sutProvider, string email, string secret) @@ -800,7 +923,8 @@ public class UserServiceTests sutProvider.GetDependency(), sutProvider.GetDependency(), sutProvider.GetDependency(), - sutProvider.GetDependency() + sutProvider.GetDependency(), + sutProvider.GetDependency() ); } }