diff --git a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs index 95299b098e..b037483caf 100644 --- a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs +++ b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs @@ -91,6 +91,7 @@ public class PolicyService : IPolicyService var organizationUserPolicyDetails = await _organizationUserRepository.GetByUserIdWithPolicyDetailsAsync(userId, policyType); OrganizationUserType[] excludedUserTypes; + var appliesToProviders = false; if (policyType == PolicyType.SingleOrg && _featureService.IsEnabled(FeatureFlagKeys.AutomaticConfirmUsers) @@ -98,6 +99,7 @@ public class PolicyService : IPolicyService { minStatus = OrganizationUserStatusType.Revoked; excludedUserTypes = []; + appliesToProviders = true; } else { @@ -105,12 +107,13 @@ public class PolicyService : IPolicyService } var orgAbilities = await _applicationCacheService.GetOrganizationAbilitiesAsync(); + return organizationUserPolicyDetails.Where(o => (!orgAbilities.TryGetValue(o.OrganizationId, out var orgAbility) || orgAbility.UsePolicies) && o.PolicyEnabled && !excludedUserTypes.Contains(o.OrganizationUserType) && o.OrganizationUserStatus >= minStatus && - !o.IsProvider); + (o.IsProvider && appliesToProviders || !o.IsProvider)); // the user is a provider and the policy applies to providers, or they are not a provider } private OrganizationUserType[] GetUserTypesExcludedFromPolicy(PolicyType policyType) diff --git a/test/Core.Test/AdminConsole/AutoFixture/OrganizationUserPolicyDetailsFixtures.cs b/test/Core.Test/AdminConsole/AutoFixture/OrganizationUserPolicyDetailsFixtures.cs index 634b234e70..53511de550 100644 --- a/test/Core.Test/AdminConsole/AutoFixture/OrganizationUserPolicyDetailsFixtures.cs +++ b/test/Core.Test/AdminConsole/AutoFixture/OrganizationUserPolicyDetailsFixtures.cs @@ -2,6 +2,7 @@ using AutoFixture; using AutoFixture.Xunit2; using Bit.Core.AdminConsole.Enums; +using Bit.Core.Enums; using Bit.Core.Models.Data.Organizations.OrganizationUsers; namespace Bit.Core.Test.AdminConsole.AutoFixture; @@ -9,10 +10,16 @@ namespace Bit.Core.Test.AdminConsole.AutoFixture; internal class OrganizationUserPolicyDetailsCustomization : ICustomization { public PolicyType Type { get; set; } + public OrganizationUserStatusType Status { get; set; } + public OrganizationUserType UserType { get; set; } + public bool IsProvider { get; set; } - public OrganizationUserPolicyDetailsCustomization(PolicyType type) + public OrganizationUserPolicyDetailsCustomization(PolicyType type, OrganizationUserStatusType status, OrganizationUserType userType, bool isProvider) { Type = type; + Status = status; + UserType = userType; + IsProvider = isProvider; } public void Customize(IFixture fixture) @@ -20,6 +27,9 @@ internal class OrganizationUserPolicyDetailsCustomization : ICustomization fixture.Customize(composer => composer .With(o => o.OrganizationId, Guid.NewGuid()) .With(o => o.PolicyType, Type) + .With(o => o.OrganizationUserStatus, Status) + .With(o => o.OrganizationUserType, UserType) + .With(o => o.IsProvider, IsProvider) .With(o => o.PolicyEnabled, true)); } } @@ -27,14 +37,25 @@ internal class OrganizationUserPolicyDetailsCustomization : ICustomization public class OrganizationUserPolicyDetailsAttribute : CustomizeAttribute { private readonly PolicyType _type; + private readonly OrganizationUserStatusType _status; + private readonly OrganizationUserType _userType; + private readonly bool _isProvider; - public OrganizationUserPolicyDetailsAttribute(PolicyType type) + public OrganizationUserPolicyDetailsAttribute(PolicyType type) : this(type, OrganizationUserStatusType.Accepted, OrganizationUserType.User, false) { _type = type; } + public OrganizationUserPolicyDetailsAttribute(PolicyType type, OrganizationUserStatusType status, OrganizationUserType userType, bool isProvider) + { + _type = type; + _status = status; + _userType = userType; + _isProvider = isProvider; + } + public override ICustomization GetCustomization(ParameterInfo parameter) { - return new OrganizationUserPolicyDetailsCustomization(_type); + return new OrganizationUserPolicyDetailsCustomization(_type, _status, _userType, _isProvider); } } diff --git a/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs b/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs index c2d713317d..be88d7e812 100644 --- a/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs @@ -11,6 +11,7 @@ using Bit.Core.Models.Data.Organizations; using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Test.AdminConsole.AutoFixture; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; @@ -127,60 +128,68 @@ public class PolicyServiceTests [Theory, BitAutoData] public async Task GetPoliciesApplicableToUserAsync_WithAutoConfirmEnabled_WithSingleOrgPolicy_IncludesRevokedUsers( Guid userId, + [OrganizationUserPolicyDetails(PolicyType.SingleOrg, + OrganizationUserStatusType.Revoked, + OrganizationUserType.Admin, + false)] OrganizationUserPolicyDetails singleOrgPolicyDetails, + [OrganizationUserPolicyDetails(PolicyType.AutomaticUserConfirmation)] OrganizationUserPolicyDetails autoConfirmPolicyDetails, SutProvider sutProvider) { - // Arrange - Setup SingleOrg policy with Revoked user + // Arrange + singleOrgPolicyDetails.OrganizationUserStatus = OrganizationUserStatusType.Revoked; + singleOrgPolicyDetails.OrganizationUserType = OrganizationUserType.Owner; + sutProvider.GetDependency() .GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.SingleOrg) - .Returns(new List - { - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.SingleOrg, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Revoked, IsProvider = false }, - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.SingleOrg, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false } - }); + .Returns([singleOrgPolicyDetails]); - // Enable AutomaticConfirmUsers feature flag sutProvider.GetDependency() .IsEnabled(FeatureFlagKeys.AutomaticConfirmUsers) .Returns(true); - // Mock repository call - user has AutomaticUserConfirmation policy details - var autoConfirmPolicies = new List - { - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.AutomaticUserConfirmation, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Revoked, IsProvider = false } - }; sutProvider.GetDependency() .GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.AutomaticUserConfirmation) - .Returns(autoConfirmPolicies); + .Returns([autoConfirmPolicyDetails]); sutProvider.GetDependency() .GetOrganizationAbilitiesAsync() - .Returns(Task.FromResult>( - new Dictionary())); + .Returns(new Dictionary() + { + { + singleOrgPolicyDetails.OrganizationId, + new OrganizationAbility + { + Id = singleOrgPolicyDetails.OrganizationId, + UsePolicies = true + } + } + }); // Act var result = await sutProvider.Sut .GetPoliciesApplicableToUserAsync(userId, PolicyType.SingleOrg); // Assert - Should include Revoked user because auto-confirm is enabled - Assert.Equal(2, result.Count()); - Assert.Contains(result, p => p.OrganizationUserStatus == OrganizationUserStatusType.Revoked); + Assert.Single(result); + Assert.Contains(result, p => p.OrganizationUserStatus == singleOrgPolicyDetails.OrganizationUserStatus); Assert.Contains(result, p => p.OrganizationUserType == OrganizationUserType.Owner); } [Theory, BitAutoData] public async Task GetPoliciesApplicableToUserAsync_WithAutoConfirmEnabled_WithSingleOrgPolicy_IncludesOwnerAndAdmin( Guid userId, + Guid organizationId, + [OrganizationUserPolicyDetails(PolicyType.SingleOrg, OrganizationUserStatusType.Confirmed, OrganizationUserType.Admin, false)] OrganizationUserPolicyDetails admin, + [OrganizationUserPolicyDetails(PolicyType.SingleOrg, OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner, false)] OrganizationUserPolicyDetails owner, + [OrganizationUserPolicyDetails(PolicyType.SingleOrg, OrganizationUserStatusType.Confirmed, OrganizationUserType.User, false)] OrganizationUserPolicyDetails user, SutProvider sutProvider) { + owner.OrganizationId = admin.OrganizationId = user.OrganizationId = organizationId; + // Arrange - Setup SingleOrg policy with Owner and Admin users (normally excluded from SingleOrg) sutProvider.GetDependency() .GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.SingleOrg) - .Returns(new List - { - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.SingleOrg, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false }, - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.SingleOrg, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Admin, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false }, - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.SingleOrg, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false } - }); + .Returns([admin, owner, user]); // Enable AutomaticConfirmUsers feature flag sutProvider.GetDependency() @@ -190,7 +199,7 @@ public class PolicyServiceTests // Mock repository call - user has AutomaticUserConfirmation policy details var autoConfirmPolicies = new List { - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.AutomaticUserConfirmation, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false } + new() { OrganizationId = organizationId, PolicyType = PolicyType.AutomaticUserConfirmation, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false } }; sutProvider.GetDependency() .GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.AutomaticUserConfirmation) @@ -198,15 +207,17 @@ public class PolicyServiceTests sutProvider.GetDependency() .GetOrganizationAbilitiesAsync() - .Returns(Task.FromResult>( - new Dictionary())); + .Returns(new Dictionary + { + { organizationId, new OrganizationAbility { Id = organizationId, UsePolicies = true } } + }); // Act var result = await sutProvider.Sut .GetPoliciesApplicableToUserAsync(userId, PolicyType.SingleOrg); // Assert - Should include Owner and Admin because excludedUserTypes is empty when auto-confirm is enabled - Assert.Equal(3, result.Count()); + Assert.Equal(3, result.Count); Assert.Contains(result, p => p.OrganizationUserType == OrganizationUserType.Owner); Assert.Contains(result, p => p.OrganizationUserType == OrganizationUserType.Admin); Assert.Contains(result, p => p.OrganizationUserType == OrganizationUserType.User); @@ -215,98 +226,100 @@ public class PolicyServiceTests [Theory, BitAutoData] public async Task GetPoliciesApplicableToUserAsync_WithAutoConfirmDisabled_WithSingleOrgPolicy_ExcludesRevokedUsers( Guid userId, + Guid organizationId, + [OrganizationUserPolicyDetails(PolicyType.SingleOrg, OrganizationUserStatusType.Revoked, OrganizationUserType.User, false)] OrganizationUserPolicyDetails revoked, + [OrganizationUserPolicyDetails(PolicyType.SingleOrg, OrganizationUserStatusType.Confirmed, OrganizationUserType.User, false)] OrganizationUserPolicyDetails confirmed, SutProvider sutProvider) { - // Arrange - Setup SingleOrg policy with Revoked and Confirmed users + revoked.OrganizationId = confirmed.OrganizationId = organizationId; + + // Arrange sutProvider.GetDependency() .GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.SingleOrg) - .Returns(new List - { - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.SingleOrg, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Revoked, IsProvider = false }, - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.SingleOrg, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false } - }); + .Returns([revoked, confirmed]); - // Disable AutomaticConfirmUsers feature flag sutProvider.GetDependency() .IsEnabled(FeatureFlagKeys.AutomaticConfirmUsers) .Returns(false); sutProvider.GetDependency() .GetOrganizationAbilitiesAsync() - .Returns(Task.FromResult>( - new Dictionary())); + .Returns(new Dictionary + { + { organizationId, new OrganizationAbility { Id = organizationId, UsePolicies = true } } + }); // Act var result = await sutProvider.Sut .GetPoliciesApplicableToUserAsync(userId, PolicyType.SingleOrg); - // Assert - Should NOT include Revoked user because feature flag is disabled + // Assert Assert.Single(result); Assert.DoesNotContain(result, p => p.OrganizationUserStatus == OrganizationUserStatusType.Revoked); - Assert.All(result, p => Assert.True(p.OrganizationUserStatus >= OrganizationUserStatusType.Accepted)); + Assert.DoesNotContain(result, p => p.OrganizationUserStatus == OrganizationUserStatusType.Invited); + Assert.Contains(result, p => p.OrganizationUserStatus == confirmed.OrganizationUserStatus); } [Theory, BitAutoData] public async Task GetPoliciesApplicableToUserAsync_WithAutoConfirmEnabled_NoAutoConfirmPolicy_ExcludesOwnerAndAdmin( Guid userId, + Guid organizationId, + [OrganizationUserPolicyDetails(PolicyType.SingleOrg, OrganizationUserStatusType.Revoked, OrganizationUserType.Admin, false)] OrganizationUserPolicyDetails admin, + [OrganizationUserPolicyDetails(PolicyType.SingleOrg, OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner, false)] OrganizationUserPolicyDetails owner, + [OrganizationUserPolicyDetails(PolicyType.SingleOrg, OrganizationUserStatusType.Confirmed, OrganizationUserType.User, false)] OrganizationUserPolicyDetails user, SutProvider sutProvider) { - // Arrange - Setup SingleOrg policy with Owner, Admin, and User + // Arrange + user.OrganizationId = admin.OrganizationId = owner.OrganizationId = organizationId; + sutProvider.GetDependency() .GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.SingleOrg) - .Returns(new List - { - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.SingleOrg, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false }, - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.SingleOrg, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Admin, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false }, - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.SingleOrg, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false } - }); + .Returns([admin, owner, user]); - // Enable AutomaticConfirmUsers feature flag sutProvider.GetDependency() .IsEnabled(FeatureFlagKeys.AutomaticConfirmUsers) .Returns(true); - // Mock repository call - user has NO AutomaticUserConfirmation policy details sutProvider.GetDependency() .GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.AutomaticUserConfirmation) - .Returns(new List()); + .Returns([]); sutProvider.GetDependency() .GetOrganizationAbilitiesAsync() - .Returns(Task.FromResult>( - new Dictionary())); + .Returns(new Dictionary + { + { organizationId, new OrganizationAbility { Id = organizationId, UsePolicies = true } } + }); // Act var result = await sutProvider.Sut .GetPoliciesApplicableToUserAsync(userId, PolicyType.SingleOrg); - // Assert - Should NOT include Owner/Admin because user doesn't have auto-confirm policy + // Assert Assert.Single(result); Assert.DoesNotContain(result, p => p.OrganizationUserType == OrganizationUserType.Owner); Assert.DoesNotContain(result, p => p.OrganizationUserType == OrganizationUserType.Admin); - Assert.All(result, p => Assert.Equal(OrganizationUserType.User, p.OrganizationUserType)); + Assert.All(result, p => Assert.Equal(user.OrganizationUserType, p.OrganizationUserType)); } [Theory, BitAutoData] public async Task GetPoliciesApplicableToUserAsync_WithNonSingleOrgPolicy_IgnoresAutoConfirmSettings( Guid userId, + Guid organizationId, + [OrganizationUserPolicyDetails(PolicyType.DisableSend)] OrganizationUserPolicyDetails disableSendPolicy, SutProvider sutProvider) { - // Arrange - Setup DisableSend policy (not SingleOrg) + // Arrange + disableSendPolicy.OrganizationId = organizationId; + sutProvider.GetDependency() .GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.DisableSend) - .Returns(new List - { - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.DisableSend, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Revoked, IsProvider = false }, - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.DisableSend, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false } - }); + .Returns([disableSendPolicy]); - // Enable AutomaticConfirmUsers feature flag sutProvider.GetDependency() .IsEnabled(FeatureFlagKeys.AutomaticConfirmUsers) .Returns(true); - // User has AutomaticUserConfirmation policy (but we're querying DisableSend, not SingleOrg) var autoConfirmPolicies = new List { new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.AutomaticUserConfirmation, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false } @@ -317,17 +330,19 @@ public class PolicyServiceTests sutProvider.GetDependency() .GetOrganizationAbilitiesAsync() - .Returns(Task.FromResult>( - new Dictionary())); + .Returns(new Dictionary + { + { organizationId, new OrganizationAbility { Id = organizationId, UsePolicies = true } } + }); // Act var result = await sutProvider.Sut .GetPoliciesApplicableToUserAsync(userId, PolicyType.DisableSend); - // Assert - Should NOT include Revoked user because auto-confirm only applies to SingleOrg policy + // Assert Assert.Single(result); Assert.DoesNotContain(result, p => p.OrganizationUserStatus == OrganizationUserStatusType.Revoked); - Assert.All(result, p => Assert.Equal(OrganizationUserStatusType.Confirmed, p.OrganizationUserStatus)); + Assert.All(result, p => Assert.Equal(disableSendPolicy.OrganizationUserStatus, p.OrganizationUserStatus)); } [Theory, BitAutoData]