[PM-10319] - Revoke Non Complaint Users for 2FA and Single Org Policy Enablement (#5037)

- Revoking users when enabling single org and 2fa policies. - Updated emails sent when users are revoked via 2FA or Single Organization policy enablement Co-authored-by: Matt Bishop <mbishop@bitwarden.com> Co-authored-by: Rui Tomé <108268980+r-tome@users.noreply.github.com>
2025-06-30 15:42:48 -05:00 · 2024-11-26 16:37:12 -06:00
parent 8f703a29ac
commit 1b75e35c31
36 changed files with 1074 additions and 73 deletions
--- a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidatorTests.cs
+++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidatorTests.cs
@ -1,6 +1,7 @@
 using Bit.Core.AdminConsole.Entities;
 using Bit.Core.AdminConsole.Enums;
 using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
+using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests;
 using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
 using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyValidators;
 using Bit.Core.Auth.Entities;
@ -10,6 +11,7 @@ using Bit.Core.Auth.Repositories;
 using Bit.Core.Context;
 using Bit.Core.Entities;
 using Bit.Core.Enums;
+using Bit.Core.Models.Commands;
 using Bit.Core.Models.Data.Organizations.OrganizationUsers;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
@ -61,6 +63,79 @@ public class SingleOrgPolicyValidatorTests
        Assert.True(string.IsNullOrEmpty(result));
    }

+    [Theory, BitAutoData]
+    public async Task OnSaveSideEffectsAsync_RevokesNonCompliantUsers(
+        [PolicyUpdate(PolicyType.SingleOrg)] PolicyUpdate policyUpdate,
+        [Policy(PolicyType.SingleOrg, false)] Policy policy,
+        Guid savingUserId,
+        Guid nonCompliantUserId,
+        Organization organization, SutProvider<SingleOrgPolicyValidator> sutProvider)
+    {
+        policy.OrganizationId = organization.Id = policyUpdate.OrganizationId;
+
+        var compliantUser1 = new OrganizationUserUserDetails
+        {
+            OrganizationId = organization.Id,
+            Type = OrganizationUserType.User,
+            Status = OrganizationUserStatusType.Confirmed,
+            UserId = new Guid(),
+            Email = "user1@example.com"
+        };
+
+        var compliantUser2 = new OrganizationUserUserDetails
+        {
+            OrganizationId = organization.Id,
+            Type = OrganizationUserType.User,
+            Status = OrganizationUserStatusType.Confirmed,
+            UserId = new Guid(),
+            Email = "user2@example.com"
+        };
+
+        var nonCompliantUser = new OrganizationUserUserDetails
+        {
+            OrganizationId = organization.Id,
+            Type = OrganizationUserType.User,
+            Status = OrganizationUserStatusType.Confirmed,
+            UserId = nonCompliantUserId,
+            Email = "user3@example.com"
+        };
+
+        sutProvider.GetDependency<IOrganizationUserRepository>()
+            .GetManyDetailsByOrganizationAsync(policyUpdate.OrganizationId)
+            .Returns([compliantUser1, compliantUser2, nonCompliantUser]);
+
+        var otherOrganizationUser = new OrganizationUser
+        {
+            OrganizationId = new Guid(),
+            UserId = nonCompliantUserId,
+            Status = OrganizationUserStatusType.Confirmed
+        };
+
+        sutProvider.GetDependency<IOrganizationUserRepository>()
+            .GetManyByManyUsersAsync(Arg.Is<IEnumerable<Guid>>(ids => ids.Contains(nonCompliantUserId)))
+            .Returns([otherOrganizationUser]);
+
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns(savingUserId);
+        sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(policyUpdate.OrganizationId).Returns(organization);
+
+        sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
+            .Returns(true);
+
+        sutProvider.GetDependency<IRevokeNonCompliantOrganizationUserCommand>()
+            .RevokeNonCompliantOrganizationUsersAsync(Arg.Any<RevokeOrganizationUsersRequest>())
+            .Returns(new CommandResult());
+
+        await sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy);
+
+        await sutProvider.GetDependency<IRevokeNonCompliantOrganizationUserCommand>()
+            .Received(1)
+            .RevokeNonCompliantOrganizationUsersAsync(Arg.Any<RevokeOrganizationUsersRequest>());
+        await sutProvider.GetDependency<IMailService>()
+            .Received(1)
+            .SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(organization.DisplayName(),
+                "user3@example.com");
+    }
+
    [Theory, BitAutoData]
    public async Task OnSaveSideEffectsAsync_RemovesNonCompliantUsers(
        [PolicyUpdate(PolicyType.SingleOrg)] PolicyUpdate policyUpdate,
@ -116,6 +191,13 @@ public class SingleOrgPolicyValidatorTests
        sutProvider.GetDependency<ICurrentContext>().UserId.Returns(savingUserId);
        sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(policyUpdate.OrganizationId).Returns(organization);

+        sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
+            .Returns(false);
+
+        sutProvider.GetDependency<IRevokeNonCompliantOrganizationUserCommand>()
+            .RevokeNonCompliantOrganizationUsersAsync(Arg.Any<RevokeOrganizationUsersRequest>())
+            .Returns(new CommandResult());
+
        await sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy);

        await sutProvider.GetDependency<IRemoveOrganizationUserCommand>()
@ -126,4 +208,73 @@ public class SingleOrgPolicyValidatorTests
            .SendOrganizationUserRemovedForPolicySingleOrgEmailAsync(organization.DisplayName(),
                "user3@example.com");
    }
+
+    [Theory, BitAutoData]
+    public async Task OnSaveSideEffectsAsync_WhenAccountDeprovisioningIsEnabled_ThenUsersAreRevoked(
+        [PolicyUpdate(PolicyType.SingleOrg)] PolicyUpdate policyUpdate,
+        [Policy(PolicyType.SingleOrg, false)] Policy policy,
+        Guid savingUserId,
+        Guid nonCompliantUserId,
+        Organization organization, SutProvider<SingleOrgPolicyValidator> sutProvider)
+    {
+        policy.OrganizationId = organization.Id = policyUpdate.OrganizationId;
+
+        var compliantUser1 = new OrganizationUserUserDetails
+        {
+            OrganizationId = organization.Id,
+            Type = OrganizationUserType.User,
+            Status = OrganizationUserStatusType.Confirmed,
+            UserId = new Guid(),
+            Email = "user1@example.com"
+        };
+
+        var compliantUser2 = new OrganizationUserUserDetails
+        {
+            OrganizationId = organization.Id,
+            Type = OrganizationUserType.User,
+            Status = OrganizationUserStatusType.Confirmed,
+            UserId = new Guid(),
+            Email = "user2@example.com"
+        };
+
+        var nonCompliantUser = new OrganizationUserUserDetails
+        {
+            OrganizationId = organization.Id,
+            Type = OrganizationUserType.User,
+            Status = OrganizationUserStatusType.Confirmed,
+            UserId = nonCompliantUserId,
+            Email = "user3@example.com"
+        };
+
+        sutProvider.GetDependency<IOrganizationUserRepository>()
+            .GetManyDetailsByOrganizationAsync(policyUpdate.OrganizationId)
+            .Returns([compliantUser1, compliantUser2, nonCompliantUser]);
+
+        var otherOrganizationUser = new OrganizationUser
+        {
+            OrganizationId = new Guid(),
+            UserId = nonCompliantUserId,
+            Status = OrganizationUserStatusType.Confirmed
+        };
+
+        sutProvider.GetDependency<IOrganizationUserRepository>()
+            .GetManyByManyUsersAsync(Arg.Is<IEnumerable<Guid>>(ids => ids.Contains(nonCompliantUserId)))
+            .Returns([otherOrganizationUser]);
+
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns(savingUserId);
+        sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(policyUpdate.OrganizationId)
+            .Returns(organization);
+
+        sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.AccountDeprovisioning).Returns(true);
+
+        sutProvider.GetDependency<IRevokeNonCompliantOrganizationUserCommand>()
+            .RevokeNonCompliantOrganizationUsersAsync(Arg.Any<RevokeOrganizationUsersRequest>())
+            .Returns(new CommandResult());
+
+        await sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy);
+
+        await sutProvider.GetDependency<IRevokeNonCompliantOrganizationUserCommand>()
+            .Received()
+            .RevokeNonCompliantOrganizationUsersAsync(Arg.Any<RevokeOrganizationUsersRequest>());
+    }
 }
--- a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/TwoFactorAuthenticationPolicyValidatorTests.cs
+++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/TwoFactorAuthenticationPolicyValidatorTests.cs
@ -1,12 +1,14 @@
 using Bit.Core.AdminConsole.Entities;
 using Bit.Core.AdminConsole.Enums;
 using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
+using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests;
 using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
 using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyValidators;
 using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces;
 using Bit.Core.Context;
 using Bit.Core.Enums;
 using Bit.Core.Exceptions;
+using Bit.Core.Models.Commands;
 using Bit.Core.Models.Data.Organizations.OrganizationUsers;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
@ -176,6 +178,10 @@ public class TwoFactorAuthenticationPolicyValidatorTests
            HasMasterPassword = false
        };

+        sutProvider.GetDependency<IFeatureService>()
+            .IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
+            .Returns(false);
+
        sutProvider.GetDependency<IOrganizationUserRepository>()
            .GetManyDetailsByOrganizationAsync(policy.OrganizationId)
            .Returns(new List<OrganizationUserUserDetails>
@ -201,9 +207,151 @@ public class TwoFactorAuthenticationPolicyValidatorTests
        var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
            () => sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy));

-        Assert.Contains("Policy could not be enabled. Non-compliant members will lose access to their accounts. Identify members without two-step login from the policies column in the members page.", badRequestException.Message, StringComparison.OrdinalIgnoreCase);
+        Assert.Equal(TwoFactorAuthenticationPolicyValidator.NonCompliantMembersWillLoseAccessMessage, badRequestException.Message);

        await sutProvider.GetDependency<IRemoveOrganizationUserCommand>().DidNotReceiveWithAnyArgs()
            .RemoveUserAsync(organizationId: default, organizationUserId: default, deletingUserId: default);
    }
+
+    [Theory, BitAutoData]
+    public async Task OnSaveSideEffectsAsync_GivenUpdateTo2faPolicy_WhenAccountProvisioningIsDisabled_ThenRevokeUserCommandShouldNotBeCalled(
+            Organization organization,
+            [PolicyUpdate(PolicyType.TwoFactorAuthentication)]
+            PolicyUpdate policyUpdate,
+            [Policy(PolicyType.TwoFactorAuthentication, false)]
+            Policy policy,
+            SutProvider<TwoFactorAuthenticationPolicyValidator> sutProvider)
+    {
+        policy.OrganizationId = organization.Id = policyUpdate.OrganizationId;
+        sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
+
+        sutProvider.GetDependency<IFeatureService>()
+            .IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
+            .Returns(false);
+
+        var orgUserDetailUserAcceptedWithout2Fa = new OrganizationUserUserDetails
+        {
+            Id = Guid.NewGuid(),
+            Status = OrganizationUserStatusType.Accepted,
+            Type = OrganizationUserType.User,
+            Email = "user3@test.com",
+            Name = "TEST",
+            UserId = Guid.NewGuid(),
+            HasMasterPassword = true
+        };
+
+        sutProvider.GetDependency<IOrganizationUserRepository>()
+            .GetManyDetailsByOrganizationAsync(policyUpdate.OrganizationId)
+            .Returns(new List<OrganizationUserUserDetails>
+            {
+                orgUserDetailUserAcceptedWithout2Fa
+            });
+
+
+        sutProvider.GetDependency<ITwoFactorIsEnabledQuery>()
+            .TwoFactorIsEnabledAsync(Arg.Any<IEnumerable<OrganizationUserUserDetails>>())
+            .Returns(new List<(OrganizationUserUserDetails user, bool hasTwoFactor)>()
+            {
+                (orgUserDetailUserAcceptedWithout2Fa, false),
+            });
+
+        await sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy);
+
+        await sutProvider.GetDependency<IRevokeNonCompliantOrganizationUserCommand>()
+            .DidNotReceive()
+            .RevokeNonCompliantOrganizationUsersAsync(Arg.Any<RevokeOrganizationUsersRequest>());
+    }
+
+    [Theory, BitAutoData]
+    public async Task OnSaveSideEffectsAsync_GivenUpdateTo2faPolicy_WhenAccountProvisioningIsEnabledAndUserDoesNotHaveMasterPassword_ThenNonCompliantMembersErrorMessageWillReturn(
+        Organization organization,
+        [PolicyUpdate(PolicyType.TwoFactorAuthentication)] PolicyUpdate policyUpdate,
+        [Policy(PolicyType.TwoFactorAuthentication, false)] Policy policy,
+        SutProvider<TwoFactorAuthenticationPolicyValidator> sutProvider)
+    {
+        policy.OrganizationId = organization.Id = policyUpdate.OrganizationId;
+        sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
+
+        sutProvider.GetDependency<IFeatureService>()
+            .IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
+            .Returns(true);
+
+        var orgUserDetailUserWithout2Fa = new OrganizationUserUserDetails
+        {
+            Id = Guid.NewGuid(),
+            Status = OrganizationUserStatusType.Confirmed,
+            Type = OrganizationUserType.User,
+            Email = "user3@test.com",
+            Name = "TEST",
+            UserId = Guid.NewGuid(),
+            HasMasterPassword = false
+        };
+
+        sutProvider.GetDependency<IOrganizationUserRepository>()
+            .GetManyDetailsByOrganizationAsync(policyUpdate.OrganizationId)
+            .Returns([orgUserDetailUserWithout2Fa]);
+
+        sutProvider.GetDependency<ITwoFactorIsEnabledQuery>()
+            .TwoFactorIsEnabledAsync(Arg.Any<IEnumerable<OrganizationUserUserDetails>>())
+            .Returns(new List<(OrganizationUserUserDetails user, bool hasTwoFactor)>()
+            {
+                (orgUserDetailUserWithout2Fa, false),
+            });
+
+        var exception = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy));
+
+        Assert.Equal(TwoFactorAuthenticationPolicyValidator.NonCompliantMembersWillLoseAccessMessage, exception.Message);
+    }
+
+    [Theory, BitAutoData]
+    public async Task OnSaveSideEffectsAsync_WhenAccountProvisioningIsEnabledAndUserHasMasterPassword_ThenUserWillBeRevoked(
+        Organization organization,
+        [PolicyUpdate(PolicyType.TwoFactorAuthentication)] PolicyUpdate policyUpdate,
+        [Policy(PolicyType.TwoFactorAuthentication, false)] Policy policy,
+        SutProvider<TwoFactorAuthenticationPolicyValidator> sutProvider)
+    {
+        policy.OrganizationId = organization.Id = policyUpdate.OrganizationId;
+        sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
+
+        sutProvider.GetDependency<IFeatureService>()
+            .IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
+            .Returns(true);
+
+        var orgUserDetailUserWithout2Fa = new OrganizationUserUserDetails
+        {
+            Id = Guid.NewGuid(),
+            Status = OrganizationUserStatusType.Confirmed,
+            Type = OrganizationUserType.User,
+            Email = "user3@test.com",
+            Name = "TEST",
+            UserId = Guid.NewGuid(),
+            HasMasterPassword = true
+        };
+
+        sutProvider.GetDependency<IOrganizationUserRepository>()
+            .GetManyDetailsByOrganizationAsync(policyUpdate.OrganizationId)
+            .Returns([orgUserDetailUserWithout2Fa]);
+
+        sutProvider.GetDependency<ITwoFactorIsEnabledQuery>()
+            .TwoFactorIsEnabledAsync(Arg.Any<IEnumerable<OrganizationUserUserDetails>>())
+            .Returns(new List<(OrganizationUserUserDetails user, bool hasTwoFactor)>()
+            {
+                (orgUserDetailUserWithout2Fa, true),
+            });
+
+        sutProvider.GetDependency<IRevokeNonCompliantOrganizationUserCommand>()
+            .RevokeNonCompliantOrganizationUsersAsync(Arg.Any<RevokeOrganizationUsersRequest>())
+            .Returns(new CommandResult());
+
+        await sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy);
+
+        await sutProvider.GetDependency<IRevokeNonCompliantOrganizationUserCommand>()
+            .Received(1)
+            .RevokeNonCompliantOrganizationUsersAsync(Arg.Any<RevokeOrganizationUsersRequest>());
+
+        await sutProvider.GetDependency<IMailService>()
+            .Received(1)
+            .SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(organization.DisplayName(),
+                "user3@test.com");
+    }
 }
--- a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/SavePolicyCommandTests.cs
+++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/SavePolicyCommandTests.cs
@ -100,7 +100,7 @@ public class SavePolicyCommandTests
    }

    [Theory, BitAutoData]
-    public async Task SaveAsync_OrganizationDoesNotExist_ThrowsBadRequest(PolicyUpdate policyUpdate)
+    public async Task SaveAsync_OrganizationDoesNotExist_ThrowsBadRequest([PolicyUpdate(PolicyType.ActivateAutofill)] PolicyUpdate policyUpdate)
    {
        var sutProvider = SutProviderFactory();
        sutProvider.GetDependency<IApplicationCacheService>()
@ -115,7 +115,7 @@ public class SavePolicyCommandTests
    }

    [Theory, BitAutoData]
-    public async Task SaveAsync_OrganizationCannotUsePolicies_ThrowsBadRequest(PolicyUpdate policyUpdate)
+    public async Task SaveAsync_OrganizationCannotUsePolicies_ThrowsBadRequest([PolicyUpdate(PolicyType.ActivateAutofill)] PolicyUpdate policyUpdate)
    {
        var sutProvider = SutProviderFactory();
        sutProvider.GetDependency<IApplicationCacheService>()