From f04a3d638b05958957db4e5823d7fb8a511992fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Wed, 26 Mar 2025 09:40:13 +0000 Subject: [PATCH] [PM-18235] Add PersonalOwnershipPolicyRequirement (#5439) * Add PersonalOwnershipPolicyRequirement for managing personal ownership policy * Add tests for PersonalOwnershipPolicyRequirement * Register PersonalOwnershipPolicyRequirement in policy requirement factory * Update ImportCiphersCommand to check PersonalOwnershipPolicyRequirement if the PolicyRequirements flag is enabled Update unit tests * Update CipherService to support PersonalOwnershipPolicyRequirement with feature flag - Add support for checking personal ownership policy using PolicyRequirementQuery when feature flag is enabled - Update CipherService constructor to inject new dependencies - Add tests for personal vault restrictions with and without feature flag * Clean up redundant "Arrange", "Act", and "Assert" comments in test methods * Refactor PersonalOwnershipPolicyRequirementTests method names for clarity - Improve test method names to better describe their purpose and behavior - Rename methods to follow a more descriptive naming convention - No functional changes to the test logic * Remove commented code explaining policy check * Refactor PersonalOwnership Policy Requirement implementation - Add PersonalOwnershipPolicyRequirementFactory to replace static Create method - Simplify policy requirement creation logic - Update PolicyServiceCollectionExtensions to register new factory - Update ImportCiphersCommand to use correct user ID parameter - Remove redundant PersonalOwnershipPolicyRequirementTests * Remove redundant PersonalOwnershipPolicyRequirementTests * Remove unnecessary tests from PersonalOwnershipPolicyRequirementFactoryTests --- .../PersonalOwnershipPolicyRequirement.cs | 26 +++++ .../PolicyServiceCollectionExtensions.cs | 1 + .../ImportFeatures/ImportCiphersCommand.cs | 20 +++- .../Services/Implementations/CipherService.cs | 18 +++- ...lOwnershipPolicyRequirementFactoryTests.cs | 31 ++++++ .../ImportCiphersAsyncCommandTests.cs | 58 ++++++++++- .../Vault/Services/CipherServiceTests.cs | 96 +++++++++++++++++++ 7 files changed, 240 insertions(+), 10 deletions(-) create mode 100644 src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/PersonalOwnershipPolicyRequirement.cs create mode 100644 test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/PersonalOwnershipPolicyRequirementFactoryTests.cs diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/PersonalOwnershipPolicyRequirement.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/PersonalOwnershipPolicyRequirement.cs new file mode 100644 index 0000000000..6f3f017bb9 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/PersonalOwnershipPolicyRequirement.cs @@ -0,0 +1,26 @@ +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; + +/// +/// Policy requirements for the Disable Personal Ownership policy. +/// +public class PersonalOwnershipPolicyRequirement : IPolicyRequirement +{ + /// + /// Indicates whether Personal Ownership is disabled for the user. If true, members are required to save items to an organization. + /// + public bool DisablePersonalOwnership { get; init; } +} + +public class PersonalOwnershipPolicyRequirementFactory : BasePolicyRequirementFactory +{ + public override PolicyType PolicyType => PolicyType.PersonalOwnership; + + public override PersonalOwnershipPolicyRequirement Create(IEnumerable policyDetails) + { + var result = new PersonalOwnershipPolicyRequirement { DisablePersonalOwnership = policyDetails.Any() }; + return result; + } +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyServiceCollectionExtensions.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyServiceCollectionExtensions.cs index d386006ad2..d330c57291 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyServiceCollectionExtensions.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyServiceCollectionExtensions.cs @@ -34,5 +34,6 @@ public static class PolicyServiceCollectionExtensions services.AddScoped, DisableSendPolicyRequirementFactory>(); services.AddScoped, SendOptionsPolicyRequirementFactory>(); services.AddScoped, ResetPasswordPolicyRequirementFactory>(); + services.AddScoped, PersonalOwnershipPolicyRequirementFactory>(); } } diff --git a/src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs b/src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs index 59d3e5be34..3c58dca183 100644 --- a/src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs +++ b/src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs @@ -1,10 +1,13 @@ 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.Context; using Bit.Core.Entities; using Bit.Core.Exceptions; using Bit.Core.Platform.Push; using Bit.Core.Repositories; +using Bit.Core.Services; using Bit.Core.Tools.Enums; using Bit.Core.Tools.ImportFeatures.Interfaces; using Bit.Core.Tools.Models.Business; @@ -26,7 +29,8 @@ public class ImportCiphersCommand : IImportCiphersCommand private readonly ICollectionRepository _collectionRepository; private readonly IReferenceEventService _referenceEventService; private readonly ICurrentContext _currentContext; - + private readonly IPolicyRequirementQuery _policyRequirementQuery; + private readonly IFeatureService _featureService; public ImportCiphersCommand( ICipherRepository cipherRepository, @@ -37,7 +41,9 @@ public class ImportCiphersCommand : IImportCiphersCommand IPushNotificationService pushService, IPolicyService policyService, IReferenceEventService referenceEventService, - ICurrentContext currentContext) + ICurrentContext currentContext, + IPolicyRequirementQuery policyRequirementQuery, + IFeatureService featureService) { _cipherRepository = cipherRepository; _folderRepository = folderRepository; @@ -48,9 +54,10 @@ public class ImportCiphersCommand : IImportCiphersCommand _policyService = policyService; _referenceEventService = referenceEventService; _currentContext = currentContext; + _policyRequirementQuery = policyRequirementQuery; + _featureService = featureService; } - public async Task ImportIntoIndividualVaultAsync( List folders, List ciphers, @@ -58,8 +65,11 @@ public class ImportCiphersCommand : IImportCiphersCommand Guid importingUserId) { // Make sure the user can save new ciphers to their personal vault - var anyPersonalOwnershipPolicies = await _policyService.AnyPoliciesApplicableToUserAsync(importingUserId, PolicyType.PersonalOwnership); - if (anyPersonalOwnershipPolicies) + var isPersonalVaultRestricted = _featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements) + ? (await _policyRequirementQuery.GetAsync(importingUserId)).DisablePersonalOwnership + : await _policyService.AnyPoliciesApplicableToUserAsync(importingUserId, PolicyType.PersonalOwnership); + + if (isPersonalVaultRestricted) { throw new BadRequestException("You cannot import items into your personal vault because you are " + "a member of an organization which forbids it."); diff --git a/src/Core/Vault/Services/Implementations/CipherService.cs b/src/Core/Vault/Services/Implementations/CipherService.cs index a315528e59..b9daafe599 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -1,5 +1,7 @@ using System.Text.Json; 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.Context; using Bit.Core.Enums; @@ -41,6 +43,8 @@ public class CipherService : ICipherService private readonly IReferenceEventService _referenceEventService; private readonly ICurrentContext _currentContext; private readonly IGetCipherPermissionsForUserQuery _getCipherPermissionsForUserQuery; + private readonly IPolicyRequirementQuery _policyRequirementQuery; + private readonly IFeatureService _featureService; public CipherService( ICipherRepository cipherRepository, @@ -58,7 +62,9 @@ public class CipherService : ICipherService GlobalSettings globalSettings, IReferenceEventService referenceEventService, ICurrentContext currentContext, - IGetCipherPermissionsForUserQuery getCipherPermissionsForUserQuery) + IGetCipherPermissionsForUserQuery getCipherPermissionsForUserQuery, + IPolicyRequirementQuery policyRequirementQuery, + IFeatureService featureService) { _cipherRepository = cipherRepository; _folderRepository = folderRepository; @@ -76,6 +82,8 @@ public class CipherService : ICipherService _referenceEventService = referenceEventService; _currentContext = currentContext; _getCipherPermissionsForUserQuery = getCipherPermissionsForUserQuery; + _policyRequirementQuery = policyRequirementQuery; + _featureService = featureService; } public async Task SaveAsync(Cipher cipher, Guid savingUserId, DateTime? lastKnownRevisionDate, @@ -143,9 +151,11 @@ public class CipherService : ICipherService } else { - // Make sure the user can save new ciphers to their personal vault - var anyPersonalOwnershipPolicies = await _policyService.AnyPoliciesApplicableToUserAsync(savingUserId, PolicyType.PersonalOwnership); - if (anyPersonalOwnershipPolicies) + var isPersonalVaultRestricted = _featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements) + ? (await _policyRequirementQuery.GetAsync(savingUserId)).DisablePersonalOwnership + : await _policyService.AnyPoliciesApplicableToUserAsync(savingUserId, PolicyType.PersonalOwnership); + + if (isPersonalVaultRestricted) { throw new BadRequestException("Due to an Enterprise Policy, you are restricted from saving items to your personal vault."); } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/PersonalOwnershipPolicyRequirementFactoryTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/PersonalOwnershipPolicyRequirementFactoryTests.cs new file mode 100644 index 0000000000..2ce75ca61e --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/PersonalOwnershipPolicyRequirementFactoryTests.cs @@ -0,0 +1,31 @@ +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; +using Bit.Core.Test.AdminConsole.AutoFixture; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; + +[SutProviderCustomize] +public class PersonalOwnershipPolicyRequirementFactoryTests +{ + [Theory, BitAutoData] + public void DisablePersonalOwnership_WithNoPolicies_ReturnsFalse(SutProvider sutProvider) + { + var actual = sutProvider.Sut.Create([]); + + Assert.False(actual.DisablePersonalOwnership); + } + + [Theory, BitAutoData] + public void DisablePersonalOwnership_WithPersonalOwnershipPolicies_ReturnsTrue( + [PolicyDetails(PolicyType.PersonalOwnership)] PolicyDetails[] policies, + SutProvider sutProvider) + { + var actual = sutProvider.Sut.Create(policies); + + Assert.True(actual.DisablePersonalOwnership); + } +} diff --git a/test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs b/test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs index 5e7a30d814..89e6d152cc 100644 --- a/test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs +++ b/test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs @@ -1,10 +1,13 @@ 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.Entities; using Bit.Core.Exceptions; using Bit.Core.Platform.Push; using Bit.Core.Repositories; +using Bit.Core.Services; using Bit.Core.Test.AutoFixture.CipherFixtures; using Bit.Core.Tools.Enums; using Bit.Core.Tools.ImportFeatures; @@ -18,7 +21,6 @@ using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; using Xunit; - namespace Bit.Core.Test.Tools.ImportFeatures; [UserCipherCustomize] @@ -51,6 +53,34 @@ public class ImportCiphersAsyncCommandTests await sutProvider.GetDependency().Received(1).PushSyncVaultAsync(importingUserId); } + [Theory, BitAutoData] + public async Task ImportIntoIndividualVaultAsync_WithPolicyRequirementsEnabled_WithDisablePersonalOwnershipPolicyDisabled_Success( + Guid importingUserId, + List ciphers, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + + sutProvider.GetDependency() + .GetAsync(importingUserId) + .Returns(new PersonalOwnershipPolicyRequirement { DisablePersonalOwnership = false }); + + sutProvider.GetDependency() + .GetManyByUserIdAsync(importingUserId) + .Returns(new List()); + + var folders = new List { new Folder { UserId = importingUserId } }; + + var folderRelationships = new List>(); + + await sutProvider.Sut.ImportIntoIndividualVaultAsync(folders, ciphers, folderRelationships, importingUserId); + + await sutProvider.GetDependency().Received(1).CreateAsync(ciphers, Arg.Any>()); + await sutProvider.GetDependency().Received(1).PushSyncVaultAsync(importingUserId); + } + [Theory, BitAutoData] public async Task ImportIntoIndividualVaultAsync_ThrowsBadRequestException( List folders, @@ -73,6 +103,32 @@ public class ImportCiphersAsyncCommandTests Assert.Equal("You cannot import items into your personal vault because you are a member of an organization which forbids it.", exception.Message); } + [Theory, BitAutoData] + public async Task ImportIntoIndividualVaultAsync_WithPolicyRequirementsEnabled_WithDisablePersonalOwnershipPolicyEnabled_ThrowsBadRequestException( + List folders, + List ciphers, + SutProvider sutProvider) + { + var userId = Guid.NewGuid(); + folders.ForEach(f => f.UserId = userId); + ciphers.ForEach(c => c.UserId = userId); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + + sutProvider.GetDependency() + .GetAsync(userId) + .Returns(new PersonalOwnershipPolicyRequirement { DisablePersonalOwnership = true }); + + var folderRelationships = new List>(); + + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.ImportIntoIndividualVaultAsync(folders, ciphers, folderRelationships, userId)); + + Assert.Equal("You cannot import items into your personal vault because you are a member of an organization which forbids it.", exception.Message); + } + [Theory, BitAutoData] public async Task ImportIntoOrganizationalVaultAsync_Success( Organization organization, diff --git a/test/Core.Test/Vault/Services/CipherServiceTests.cs b/test/Core.Test/Vault/Services/CipherServiceTests.cs index 3ef29146c2..a7dcbddcea 100644 --- a/test/Core.Test/Vault/Services/CipherServiceTests.cs +++ b/test/Core.Test/Vault/Services/CipherServiceTests.cs @@ -1,5 +1,9 @@ using System.Text.Json; 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.Billing.Enums; using Bit.Core.Entities; using Bit.Core.Enums; @@ -107,6 +111,98 @@ public class CipherServiceTests await sutProvider.GetDependency().Received(1).ReplaceAsync(cipherDetails); } + [Theory] + [BitAutoData] + public async Task SaveDetailsAsync_PersonalVault_WithDisablePersonalOwnershipPolicyEnabled_Throws( + SutProvider sutProvider, + CipherDetails cipher, + Guid savingUserId) + { + cipher.Id = default; + cipher.UserId = savingUserId; + cipher.OrganizationId = null; + + sutProvider.GetDependency() + .AnyPoliciesApplicableToUserAsync(savingUserId, PolicyType.PersonalOwnership) + .Returns(true); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveDetailsAsync(cipher, savingUserId, null)); + Assert.Contains("restricted from saving items to your personal vault", exception.Message); + } + + [Theory] + [BitAutoData] + public async Task SaveDetailsAsync_PersonalVault_WithDisablePersonalOwnershipPolicyDisabled_Succeeds( + SutProvider sutProvider, + CipherDetails cipher, + Guid savingUserId) + { + cipher.Id = default; + cipher.UserId = savingUserId; + cipher.OrganizationId = null; + + sutProvider.GetDependency() + .AnyPoliciesApplicableToUserAsync(savingUserId, PolicyType.PersonalOwnership) + .Returns(false); + + await sutProvider.Sut.SaveDetailsAsync(cipher, savingUserId, null); + + await sutProvider.GetDependency() + .Received(1) + .CreateAsync(cipher); + } + + [Theory] + [BitAutoData] + public async Task SaveDetailsAsync_PersonalVault_WithPolicyRequirementsEnabled_WithDisablePersonalOwnershipPolicyEnabled_Throws( + SutProvider sutProvider, + CipherDetails cipher, + Guid savingUserId) + { + cipher.Id = default; + cipher.UserId = savingUserId; + cipher.OrganizationId = null; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + + sutProvider.GetDependency() + .GetAsync(savingUserId) + .Returns(new PersonalOwnershipPolicyRequirement { DisablePersonalOwnership = true }); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveDetailsAsync(cipher, savingUserId, null)); + Assert.Contains("restricted from saving items to your personal vault", exception.Message); + } + + [Theory] + [BitAutoData] + public async Task SaveDetailsAsync_PersonalVault_WithPolicyRequirementsEnabled_WithDisablePersonalOwnershipPolicyDisabled_Succeeds( + SutProvider sutProvider, + CipherDetails cipher, + Guid savingUserId) + { + cipher.Id = default; + cipher.UserId = savingUserId; + cipher.OrganizationId = null; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PolicyRequirements) + .Returns(true); + + sutProvider.GetDependency() + .GetAsync(savingUserId) + .Returns(new PersonalOwnershipPolicyRequirement { DisablePersonalOwnership = false }); + + await sutProvider.Sut.SaveDetailsAsync(cipher, savingUserId, null); + + await sutProvider.GetDependency() + .Received(1) + .CreateAsync(cipher); + } + [Theory] [BitAutoData("")] [BitAutoData("Correct Time")]