diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs index 74165a5a71..0bae1c0675 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs @@ -1,5 +1,6 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Enums; @@ -22,7 +23,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,11 +273,19 @@ 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)) + if (featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements)) { - twoFactorCompliant = false; + var requirement = await policyRequirementQuery.GetAsync(userId); + twoFactorCompliant = requirement.CanBeRestored(userHasTwoFactorEnabled, orgUser.OrganizationId); + } + else + { + var invitedTwoFactorPolicies = await policyService.GetPoliciesApplicableToUserAsync(userId, + PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Revoked); + if (invitedTwoFactorPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) + { + twoFactorCompliant = false; + } } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs index d6880a3a12..fc6f6df9d0 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs @@ -1,6 +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.OrganizationUsers.RestoreUser.v1; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Enums; @@ -208,6 +210,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 +288,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, @@ -363,6 +456,63 @@ public class RestoreOrganizationUserCommandTests .PushSyncOrgKeysAsync(Arg.Any()); } + [Theory, BitAutoData] + 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; // 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.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); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); + + Assert.Contains("test@bitwarden.com is not compliant with the single organization and 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_vNext_With2FAPolicyEnabled_WithoutUser2FAConfigured_Fails( Organization organization, @@ -672,6 +822,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,