Refactor two-factor authentication policy checks in AcceptOrgUserCommand and ConfirmOrganizationUserCommand to streamline validation logic and improve clarity. Update RequireTwoFactorPolicyRequirement to provide a method for checking if two-factor authentication is required for an organization. Adjust related unit tests accordingly.

2025-06-03 09:40:31 -05:00 · 2025-05-23 12:34:08 +01:00 · 2025-05-23 12:34:08 +01:00 · 7bcf1c1281
commit 7bcf1c1281
parent ee660b25b7
6 changed files with 32 additions and 231 deletions
--- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs
+++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs
@ -246,17 +246,10 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand
    /// <exception cref="BadRequestException">Thrown if the user does not have two-step login enabled.</exception>
    private async Task ValidateTwoFactorAuthenticationPolicyAsync(User user, Guid organizationId)
    {
-        var userTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user);
-        if (userTwoFactorEnabled)
-        {
-            // If the user has two-step login enabled, we skip checking the 2FA policies
-            return;
-        }
+        var twoFactorPolicyRequirement = await _policyRequirementQuery.GetAsync<RequireTwoFactorPolicyRequirement>(user.Id);
+        var twoFactorRequiredForOrganization = twoFactorPolicyRequirement.IsTwoFactorRequiredForOrganization(organizationId);

-        var requirement = await _policyRequirementQuery.GetAsync<RequireTwoFactorPolicyRequirement>(user.Id);
-        var canAcceptInvitation = requirement.CanAcceptInvitation(userTwoFactorEnabled, organizationId);
-
-        if (!canAcceptInvitation)
+        if (twoFactorRequiredForOrganization && !await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user))
        {
            throw new BadRequestException("You cannot join this organization until you enable two-step login on your user account.");
        }
--- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs
+++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs
@ -194,14 +194,14 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand
    {
        if (userTwoFactorEnabled)
        {
-            // If the user has two-step login enabled, we skip checking the 2FA policies
+            // If the user has two-step login enabled, we skip checking the 2FA policy
            return;
        }

-        var requirement = await _policyRequirementQuery.GetAsync<RequireTwoFactorPolicyRequirement>(user.Id);
-        var canBeConfirmed = requirement.CanBeConfirmed(userTwoFactorEnabled, organizationId);
+        var twoFactorPolicyRequirement = await _policyRequirementQuery.GetAsync<RequireTwoFactorPolicyRequirement>(user.Id);
+        var twoFactorRequired = twoFactorPolicyRequirement.IsTwoFactorRequiredForOrganization(organizationId);

-        if (!canBeConfirmed)
+        if (twoFactorRequired)
        {
            throw new BadRequestException("User does not have two-step login enabled.");
        }
--- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs
+++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs
@ -276,7 +276,7 @@ public class RestoreOrganizationUserCommand(
            if (featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements))
            {
                var requirement = await policyRequirementQuery.GetAsync<RequireTwoFactorPolicyRequirement>(userId);
-                twoFactorCompliant = requirement.CanBeRestored(userHasTwoFactorEnabled, orgUser.OrganizationId);
+                twoFactorCompliant = !requirement.IsTwoFactorRequiredForOrganization(orgUser.OrganizationId);
            }
            else
            {
--- a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/RequireTwoFactorPolicyRequirement.cs
+++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/RequireTwoFactorPolicyRequirement.cs
@ -16,53 +16,20 @@ public class RequireTwoFactorPolicyRequirement : IPolicyRequirement
    }

    /// <summary>
-    /// Determines if the user can accept an invitation to an organization.
+    /// Checks if two-factor authentication is required for the organization due to an active policy.
    /// </summary>
-    /// <param name="twoFactorEnabled">Whether the user has two-step login enabled.</param>
-    /// <param name="organizationId">The ID of the organization.</param>
-    /// <returns>True if the user can accept the invitation, false otherwise.</returns>
-    public bool CanAcceptInvitation(bool twoFactorEnabled, Guid organizationId) =>
-        twoFactorEnabled ||
-        !_policyDetails.Any(p => p.OrganizationId == organizationId &&
-            (p.OrganizationUserStatus is
-                OrganizationUserStatusType.Invited or
-                OrganizationUserStatusType.Accepted or
-                OrganizationUserStatusType.Confirmed));
-
+    /// <param name="organizationId">The ID of the organization to check.</param>
+    /// <returns>True if two-factor authentication is required for the organization, false otherwise.</returns>
+    /// <remarks>
+    /// This does not check the user's membership status.
+    /// </remarks>
+    public bool IsTwoFactorRequiredForOrganization(Guid organizationId) =>
+        _policyDetails.Any(p => p.OrganizationId == organizationId);

    /// <summary>
-    /// Determines if the user can be confirmed in an organization.
+    /// Gets the active two-factor authentication policies for active memberships.
    /// </summary>
-    /// <param name="twoFactorEnabled">Whether the user has two-step login enabled.</param>
-    /// <param name="organizationId">The ID of the organization.</param>
-    /// <returns>True if the user can be confirmed, false otherwise.</returns>
-    public bool CanBeConfirmed(bool twoFactorEnabled, Guid organizationId) =>
-        twoFactorEnabled ||
-        !_policyDetails.Any(p => p.OrganizationId == organizationId &&
-            (p.OrganizationUserStatus is
-                OrganizationUserStatusType.Accepted or
-                OrganizationUserStatusType.Confirmed));
-
-
-    /// <summary>
-    /// Determines if the user can be restored in an organization.
-    /// </summary>
-    /// <param name="twoFactorEnabled">Whether the user has two-step login enabled.</param>
-    /// <param name="organizationId">The ID of the organization.</param>
-    /// <returns>True if the user can be restored, false otherwise.</returns>
-    public bool CanBeRestored(bool twoFactorEnabled, Guid organizationId) =>
-        twoFactorEnabled ||
-        !_policyDetails.Any(p => p.OrganizationId == organizationId &&
-            (p.OrganizationUserStatus is
-                OrganizationUserStatusType.Revoked or
-                OrganizationUserStatusType.Invited or
-                OrganizationUserStatusType.Accepted or
-                OrganizationUserStatusType.Confirmed));
-
-    /// <summary>
-    /// Gets the two-factor policies for active memberships.
-    /// </summary>
-    /// <returns>The two-factor policies for active memberships.</returns>
+    /// <returns>The active two-factor authentication policies for active memberships.</returns>
    public IEnumerable<PolicyDetails> TwoFactorPoliciesForActiveMemberships =>
        _policyDetails.Where(p => p.OrganizationUserStatus is
                OrganizationUserStatusType.Accepted or
--- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs
+++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs
@ -253,10 +253,6 @@ public class AcceptOrgUserCommandTests

        await sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService);

-        await sutProvider.GetDependency<IPolicyRequirementQuery>()
-            .DidNotReceiveWithAnyArgs()
-            .GetAsync<RequireTwoFactorPolicyRequirement>(default);
-
        await sutProvider.GetDependency<IOrganizationUserRepository>()
            .Received(1)
            .ReplaceAsync(Arg.Is<OrganizationUser>(ou => ou.Status == OrganizationUserStatusType.Accepted));
--- a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/RequireTwoFactorPolicyRequirementFactoryTests.cs
+++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/RequireTwoFactorPolicyRequirementFactoryTests.cs
@ -11,24 +11,20 @@ namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Policies.PolicyRequire
 public class RequireTwoFactorPolicyRequirementFactoryTests
 {
    [Theory]
-    [BitAutoData(true)]
-    [BitAutoData(false)]
-    public void CanAcceptInvitation_WithNoPolicies_ReturnsTrue(
-        bool twoFactorEnabled, Guid organizationId,
+    [BitAutoData]
+    public void IsTwoFactorRequiredForOrganization_WithNoPolicies_ReturnsFalse(
+        Guid organizationId,
        SutProvider<RequireTwoFactorPolicyRequirementFactory> sutProvider)
    {
        var actual = sutProvider.Sut.Create([]);

-        Assert.True(actual.CanAcceptInvitation(twoFactorEnabled, organizationId));
+        Assert.False(actual.IsTwoFactorRequiredForOrganization(organizationId));
    }

    [Theory]
-    [BitAutoData(OrganizationUserStatusType.Revoked)]
-    [BitAutoData(OrganizationUserStatusType.Invited)]
-    [BitAutoData(OrganizationUserStatusType.Accepted)]
-    [BitAutoData(OrganizationUserStatusType.Confirmed)]
-    public void CanAcceptInvitation_WithTwoFactorEnabled_ReturnsTrue(
-        OrganizationUserStatusType userStatus, Guid organizationId,
+    [BitAutoData]
+    public void IsTwoFactorRequiredForOrganization_WithOrganizationPolicy_ReturnsTrue(
+        Guid organizationId,
        SutProvider<RequireTwoFactorPolicyRequirementFactory> sutProvider)
    {
        var actual = sutProvider.Sut.Create(
@ -37,179 +33,28 @@ public class RequireTwoFactorPolicyRequirementFactoryTests
            {
                OrganizationId = organizationId,
                PolicyType = PolicyType.TwoFactorAuthentication,
-                OrganizationUserStatus = userStatus
            }
        ]);

-        Assert.True(actual.CanAcceptInvitation(true, organizationId));
+        Assert.True(actual.IsTwoFactorRequiredForOrganization(organizationId));
    }

    [Theory]
-    [BitAutoData(OrganizationUserStatusType.Revoked)]
-    public void CanAcceptInvitation_WithExemptStatus_ReturnsTrue(
-        OrganizationUserStatusType userStatus, Guid organizationId,
+    [BitAutoData]
+    public void IsTwoFactorRequiredForOrganization_WithOtherOrganizationPolicy_ReturnsFalse(
+        Guid organizationId,
        SutProvider<RequireTwoFactorPolicyRequirementFactory> sutProvider)
    {
        var actual = sutProvider.Sut.Create(
        [
            new PolicyDetails
            {
-                OrganizationId = organizationId,
+                OrganizationId = Guid.NewGuid(),
                PolicyType = PolicyType.TwoFactorAuthentication,
-                OrganizationUserStatus = userStatus
-            }
+            },
        ]);

-        Assert.True(actual.CanAcceptInvitation(false, organizationId));
-    }
-
-    [Theory]
-    [BitAutoData(OrganizationUserStatusType.Invited)]
-    [BitAutoData(OrganizationUserStatusType.Accepted)]
-    [BitAutoData(OrganizationUserStatusType.Confirmed)]
-    public void CanAcceptInvitation_WithNonExemptStatus_ReturnsFalse(
-        OrganizationUserStatusType userStatus, Guid organizationId,
-        SutProvider<RequireTwoFactorPolicyRequirementFactory> sutProvider)
-    {
-        var actual = sutProvider.Sut.Create(
-        [
-            new PolicyDetails
-            {
-                OrganizationId = organizationId,
-                PolicyType = PolicyType.TwoFactorAuthentication,
-                OrganizationUserStatus = userStatus
-            }
-        ]);
-
-        Assert.False(actual.CanAcceptInvitation(false, organizationId));
-    }
-
-    [Theory]
-    [BitAutoData(true)]
-    [BitAutoData(false)]
-    public void CanBeConfirmed_WithNoPolicies_ReturnsTrue(
-        bool twoFactorEnabled, Guid organizationId,
-        SutProvider<RequireTwoFactorPolicyRequirementFactory> sutProvider)
-    {
-        var actual = sutProvider.Sut.Create([]);
-
-        Assert.True(actual.CanBeConfirmed(twoFactorEnabled, organizationId));
-    }
-
-    [Theory]
-    [BitAutoData(OrganizationUserStatusType.Accepted)]
-    [BitAutoData(OrganizationUserStatusType.Confirmed)]
-    public void CanBeConfirmed_WithTwoFactorEnabled_ReturnsTrue(
-        OrganizationUserStatusType userStatus, Guid organizationId,
-        SutProvider<RequireTwoFactorPolicyRequirementFactory> sutProvider)
-    {
-        var actual = sutProvider.Sut.Create(
-        [
-            new PolicyDetails
-            {
-                OrganizationId = organizationId,
-                PolicyType = PolicyType.TwoFactorAuthentication,
-                OrganizationUserStatus = userStatus
-            }
-        ]);
-
-        Assert.True(actual.CanBeConfirmed(true, organizationId));
-    }
-
-    [Theory]
-    [BitAutoData(OrganizationUserStatusType.Revoked)]
-    [BitAutoData(OrganizationUserStatusType.Invited)]
-    public void CanBeConfirmed_WithExemptStatus_ReturnsTrue(
-        OrganizationUserStatusType userStatus, Guid organizationId,
-        SutProvider<RequireTwoFactorPolicyRequirementFactory> sutProvider)
-    {
-        var actual = sutProvider.Sut.Create(
-        [
-            new PolicyDetails
-            {
-                OrganizationId = organizationId,
-                PolicyType = PolicyType.TwoFactorAuthentication,
-                OrganizationUserStatus = userStatus
-            }
-        ]);
-
-        Assert.True(actual.CanBeConfirmed(false, organizationId));
-    }
-
-    [Theory]
-    [BitAutoData(OrganizationUserStatusType.Accepted)]
-    [BitAutoData(OrganizationUserStatusType.Confirmed)]
-    public void CanBeConfirmed_WithNonExemptStatus_ReturnsFalse(
-        OrganizationUserStatusType userStatus, Guid organizationId,
-        SutProvider<RequireTwoFactorPolicyRequirementFactory> sutProvider)
-    {
-        var actual = sutProvider.Sut.Create(
-        [
-            new PolicyDetails
-            {
-                OrganizationId = organizationId,
-                PolicyType = PolicyType.TwoFactorAuthentication,
-                OrganizationUserStatus = userStatus
-            }
-        ]);
-
-        Assert.False(actual.CanBeConfirmed(false, organizationId));
-    }
-
-    [Theory]
-    [BitAutoData(true)]
-    [BitAutoData(false)]
-    public void CanBeRestored_WithNoPolicies_ReturnsTrue(
-        bool twoFactorEnabled, Guid organizationId,
-        SutProvider<RequireTwoFactorPolicyRequirementFactory> sutProvider)
-    {
-        var actual = sutProvider.Sut.Create([]);
-
-        Assert.True(actual.CanBeRestored(twoFactorEnabled, organizationId));
-    }
-
-    [Theory]
-    [BitAutoData(OrganizationUserStatusType.Revoked)]
-    [BitAutoData(OrganizationUserStatusType.Invited)]
-    [BitAutoData(OrganizationUserStatusType.Accepted)]
-    [BitAutoData(OrganizationUserStatusType.Confirmed)]
-    public void CanBeRestored_WithTwoFactorEnabled_ReturnsTrue(
-        OrganizationUserStatusType userStatus, Guid organizationId,
-        SutProvider<RequireTwoFactorPolicyRequirementFactory> sutProvider)
-    {
-        var actual = sutProvider.Sut.Create(
-        [
-            new PolicyDetails
-            {
-                OrganizationId = organizationId,
-                PolicyType = PolicyType.TwoFactorAuthentication,
-                OrganizationUserStatus = userStatus
-            }
-        ]);
-
-        Assert.True(actual.CanBeRestored(true, organizationId));
-    }
-
-    [Theory]
-    [BitAutoData(OrganizationUserStatusType.Revoked)]
-    [BitAutoData(OrganizationUserStatusType.Invited)]
-    [BitAutoData(OrganizationUserStatusType.Accepted)]
-    [BitAutoData(OrganizationUserStatusType.Confirmed)]
-    public void CanBeRestored_WithAnyStatus_ReturnsFalse(
-        OrganizationUserStatusType userStatus, Guid organizationId,
-        SutProvider<RequireTwoFactorPolicyRequirementFactory> sutProvider)
-    {
-        var actual = sutProvider.Sut.Create(
-        [
-            new PolicyDetails
-            {
-                OrganizationId = organizationId,
-                PolicyType = PolicyType.TwoFactorAuthentication,
-                OrganizationUserStatus = userStatus
-            }
-        ]);
-
-        Assert.False(actual.CanBeRestored(false, organizationId));
+        Assert.False(actual.IsTwoFactorRequiredForOrganization(organizationId));
    }

    [Theory, BitAutoData]