Skip to content

Commit 245f781

Browse files
r-tomer-tome
committed
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
1 parent 3debb5e commit 245f781

File tree

6 files changed

+103
-89
lines changed

6 files changed

+103
-89
lines changed

‎src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/PersonalOwnershipPolicyRequirement.cs

+7-20
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using Bit.Core.AdminConsole.Enums;
22
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
3-
using Bit.Core.Enums;
43

54
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
65

@@ -13,27 +12,15 @@ public class PersonalOwnershipPolicyRequirement : IPolicyRequirement
1312
/// Indicates whether Personal Ownership is disabled for the user. If true, members are required to save items to an organization.
1413
/// </summary>
1514
public bool DisablePersonalOwnership { get; init; }
15+
}
1616

17-
/// <summary>
18-
/// Creates a new PersonalOwnershipPolicyRequirement.
19-
/// </summary>
20-
/// <param name="policyDetails">All PolicyDetails relating to the user.</param>
21-
/// <remarks>
22-
/// This is a <see cref="RequirementFactory{T}"/> for the PersonalOwnershipPolicyRequirement.
23-
/// </remarks>
24-
public static PersonalOwnershipPolicyRequirement Create(IEnumerable<PolicyDetails> policyDetails)
25-
{
26-
var filteredPolicies = policyDetails
27-
.ExemptRoles([OrganizationUserType.Owner, OrganizationUserType.Admin])
28-
.ExemptStatus([OrganizationUserStatusType.Invited, OrganizationUserStatusType.Revoked])
29-
.ExemptProviders()
30-
.ToList();
31-
32-
var result = new PersonalOwnershipPolicyRequirement
33-
{
34-
DisablePersonalOwnership = filteredPolicies.GetPolicyType(PolicyType.PersonalOwnership).Any()
35-
};
17+
public class PersonalOwnershipPolicyRequirementFactory : BasePolicyRequirementFactory<PersonalOwnershipPolicyRequirement>
18+
{
19+
public override PolicyType PolicyType => PolicyType.PersonalOwnership;
3620

21+
public override PersonalOwnershipPolicyRequirement Create(IEnumerable<PolicyDetails> policyDetails)
22+
{
23+
var result = new PersonalOwnershipPolicyRequirement { DisablePersonalOwnership = policyDetails.Any() };
3724
return result;
3825
}
3926
}

‎src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyServiceCollectionExtensions.cs

+1
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,6 @@ private static void AddPolicyRequirements(this IServiceCollection services)
3333
{
3434
services.AddScoped<IPolicyRequirementFactory<IPolicyRequirement>, DisableSendPolicyRequirementFactory>();
3535
services.AddScoped<IPolicyRequirementFactory<IPolicyRequirement>, SendOptionsPolicyRequirementFactory>();
36+
services.AddScoped<IPolicyRequirementFactory<IPolicyRequirement>, PersonalOwnershipPolicyRequirementFactory>();
3637
}
3738
}

‎src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ public async Task ImportIntoIndividualVaultAsync(
6666
{
6767
// Make sure the user can save new ciphers to their personal vault
6868
var isPersonalVaultRestricted = _featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements)
69-
? (await _policyRequirementQuery.GetAsync<PersonalOwnershipPolicyRequirement>(userId.Value)).DisablePersonalOwnership
70-
: await _policyService.AnyPoliciesApplicableToUserAsync(userId.Value, PolicyType.PersonalOwnership);
69+
? (await _policyRequirementQuery.GetAsync<PersonalOwnershipPolicyRequirement>(importingUserId)).DisablePersonalOwnership
70+
: await _policyService.AnyPoliciesApplicableToUserAsync(importingUserId, PolicyType.PersonalOwnership);
7171

7272
if (isPersonalVaultRestricted)
7373
{

‎test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/PersonalOwnershipPolicyRequirementFactoryTests.cs

+90
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
using Bit.Core.AdminConsole.Enums;
2+
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
3+
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
4+
using Bit.Core.Enums;
5+
using Bit.Core.Test.AdminConsole.AutoFixture;
6+
using Bit.Test.Common.AutoFixture;
7+
using Bit.Test.Common.AutoFixture.Attributes;
8+
using Xunit;
9+
10+
namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
11+
12+
[SutProviderCustomize]
13+
public class PersonalOwnershipPolicyRequirementFactoryTests
14+
{
15+
[Theory, BitAutoData]
16+
public void DisablePersonalOwnership_WithNoPolicies_ReturnsFalse(SutProvider<PersonalOwnershipPolicyRequirementFactory> sutProvider)
17+
{
18+
var actual = sutProvider.Sut.Create([]);
19+
20+
Assert.False(actual.DisablePersonalOwnership);
21+
}
22+
23+
[Theory, BitAutoData]
24+
public void DisablePersonalOwnership_WithNonPersonalOwnershipPolicies_ReturnsFalse(
25+
[PolicyDetails(PolicyType.RequireSso)] PolicyDetails otherPolicy1,
26+
[PolicyDetails(PolicyType.SendOptions)] PolicyDetails otherPolicy2,
27+
SutProvider<PersonalOwnershipPolicyRequirementFactory> sutProvider)
28+
{
29+
var actual = sutProvider.Sut.Create([otherPolicy1, otherPolicy2]);
30+
31+
Assert.False(actual.DisablePersonalOwnership);
32+
}
33+
34+
[Theory, BitAutoData]
35+
public void DisablePersonalOwnership_WithPersonalOwnershipPolicies_ReturnsTrue(
36+
[PolicyDetails(PolicyType.PersonalOwnership)] PolicyDetails[] policies,
37+
SutProvider<PersonalOwnershipPolicyRequirementFactory> sutProvider
38+
)
39+
{
40+
var actual = sutProvider.Sut.Create(policies);
41+
42+
Assert.True(actual.DisablePersonalOwnership);
43+
}
44+
45+
[Theory, BitAutoData]
46+
public void DisablePersonalOwnership_WithProviderUserParameter_ReturnsFalse(
47+
[PolicyDetails(PolicyType.PersonalOwnership, isProvider: true)] PolicyDetails policyDetails,
48+
SutProvider<PersonalOwnershipPolicyRequirementFactory> sutProvider)
49+
{
50+
var actual = sutProvider.Sut.Create([policyDetails]);
51+
52+
Assert.False(actual.DisablePersonalOwnership);
53+
}
54+
55+
[Theory]
56+
[BitAutoData(OrganizationUserType.Owner, false)]
57+
[BitAutoData(OrganizationUserType.Admin, false)]
58+
[BitAutoData(OrganizationUserType.User, true)]
59+
[BitAutoData(OrganizationUserType.Custom, true)]
60+
public void DisablePersonalOwnership_RespectsExemptRoles(
61+
OrganizationUserType userType,
62+
bool shouldBeEnforced,
63+
[PolicyDetails(PolicyType.PersonalOwnership)] PolicyDetails policyDetails,
64+
SutProvider<PersonalOwnershipPolicyRequirementFactory> sutProvider)
65+
{
66+
policyDetails.OrganizationUserType = userType;
67+
68+
var actual = sutProvider.Sut.Create([policyDetails]);
69+
70+
Assert.Equal(shouldBeEnforced, actual.DisablePersonalOwnership);
71+
}
72+
73+
[Theory]
74+
[BitAutoData(OrganizationUserStatusType.Confirmed, true)]
75+
[BitAutoData(OrganizationUserStatusType.Accepted, true)]
76+
[BitAutoData(OrganizationUserStatusType.Invited, false)]
77+
[BitAutoData(OrganizationUserStatusType.Revoked, false)]
78+
public void DisablePersonalOwnership_RespectsExemptStatuses(
79+
OrganizationUserStatusType userStatus,
80+
bool shouldBeEnforced,
81+
[PolicyDetails(PolicyType.PersonalOwnership)] PolicyDetails policyDetails,
82+
SutProvider<PersonalOwnershipPolicyRequirementFactory> sutProvider)
83+
{
84+
policyDetails.OrganizationUserStatus = userStatus;
85+
86+
var actual = sutProvider.Sut.Create([policyDetails]);
87+
88+
Assert.Equal(shouldBeEnforced, actual.DisablePersonalOwnership);
89+
}
90+
}

‎test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/PersonalOwnershipPolicyRequirementTests.cs

+1-65
Original file line numberDiff line numberDiff line change
@@ -1,65 +1 @@
1-
using AutoFixture.Xunit2;
2-
using Bit.Core.AdminConsole.Enums;
3-
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
4-
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
5-
using Bit.Core.Enums;
6-
using Bit.Core.Test.AdminConsole.AutoFixture;
7-
using Xunit;
8-
9-
namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
10-
11-
public class PersonalOwnershipPolicyRequirementTests
12-
{
13-
[Theory, AutoData]
14-
public void Create_WithNonPersonalOwnershipPolicies_ReturnsDisablePersonalOwnershipFalse(
15-
[PolicyDetails(PolicyType.RequireSso)] PolicyDetails otherPolicy1,
16-
[PolicyDetails(PolicyType.SendOptions)] PolicyDetails otherPolicy2)
17-
{
18-
var actual = PersonalOwnershipPolicyRequirement.Create([otherPolicy1, otherPolicy2]);
19-
20-
Assert.False(actual.DisablePersonalOwnership);
21-
}
22-
23-
[Theory]
24-
[InlineAutoData(OrganizationUserType.Owner, false)]
25-
[InlineAutoData(OrganizationUserType.Admin, false)]
26-
[InlineAutoData(OrganizationUserType.User, true)]
27-
[InlineAutoData(OrganizationUserType.Custom, true)]
28-
public void Create_WithDifferentOrganizationUserTypes_ReturnsExpectedEnforcement(
29-
OrganizationUserType userType,
30-
bool shouldBeEnforced,
31-
[PolicyDetails(PolicyType.PersonalOwnership)] PolicyDetails policyDetails)
32-
{
33-
policyDetails.OrganizationUserType = userType;
34-
35-
var actual = PersonalOwnershipPolicyRequirement.Create([policyDetails]);
36-
37-
Assert.Equal(shouldBeEnforced, actual.DisablePersonalOwnership);
38-
}
39-
40-
[Theory, AutoData]
41-
public void Create_WithProviderPolicyDetails_ReturnsDisablePersonalOwnershipFalse(
42-
[PolicyDetails(PolicyType.PersonalOwnership, isProvider: true)] PolicyDetails policyDetails)
43-
{
44-
var actual = PersonalOwnershipPolicyRequirement.Create([policyDetails]);
45-
46-
Assert.False(actual.DisablePersonalOwnership);
47-
}
48-
49-
[Theory]
50-
[InlineAutoData(OrganizationUserStatusType.Confirmed, true)]
51-
[InlineAutoData(OrganizationUserStatusType.Accepted, true)]
52-
[InlineAutoData(OrganizationUserStatusType.Invited, false)]
53-
[InlineAutoData(OrganizationUserStatusType.Revoked, false)]
54-
public void Create_WithDifferentOrganizationUserStatuses_ReturnsExpectedEnforcement(
55-
OrganizationUserStatusType userStatus,
56-
bool shouldBeEnforced,
57-
[PolicyDetails(PolicyType.PersonalOwnership)] PolicyDetails policyDetails)
58-
{
59-
policyDetails.OrganizationUserStatus = userStatus;
60-
61-
var actual = PersonalOwnershipPolicyRequirement.Create([policyDetails]);
62-
63-
Assert.Equal(shouldBeEnforced, actual.DisablePersonalOwnership);
64-
}
65-
}
1+


‎test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public async Task ImportIntoIndividualVaultAsync_WithPolicyRequirementsEnabled_W
7575

7676
var folderRelationships = new List<KeyValuePair<int, int>>();
7777

78-
await sutProvider.Sut.ImportIntoIndividualVaultAsync(folders, ciphers, folderRelationships);
78+
await sutProvider.Sut.ImportIntoIndividualVaultAsync(folders, ciphers, folderRelationships, importingUserId);
7979

8080
await sutProvider.GetDependency<ICipherRepository>().Received(1).CreateAsync(ciphers, Arg.Any<List<Folder>>());
8181
await sutProvider.GetDependency<IPushNotificationService>().Received(1).PushSyncVaultAsync(importingUserId);
@@ -124,7 +124,7 @@ public async Task ImportIntoIndividualVaultAsync_WithPolicyRequirementsEnabled_W
124124
var folderRelationships = new List<KeyValuePair<int, int>>();
125125

126126
var exception = await Assert.ThrowsAsync<BadRequestException>(() =>
127-
sutProvider.Sut.ImportIntoIndividualVaultAsync(folders, ciphers, folderRelationships));
127+
sutProvider.Sut.ImportIntoIndividualVaultAsync(folders, ciphers, folderRelationships, userId));
128128

129129
Assert.Equal("You cannot import items into your personal vault because you are a member of an organization which forbids it.", exception.Message);
130130
}

0 commit comments

Comments
 (0)