From da0bd39b0b50535fa47542c601056b44d7ac854e Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 16 Jun 2025 13:12:55 -0700 Subject: [PATCH] feat(policy-service): Policy Service Update [PM-22723] - Changes to policy service for joint usage between other prs. --- .../policy/policy.service.abstraction.ts | 10 ++ .../policy/default-policy.service.spec.ts | 120 ++++++++++++++++++ .../services/policy/default-policy.service.ts | 109 ++++++++-------- 3 files changed, 184 insertions(+), 55 deletions(-) diff --git a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts index bf02872ed7c..4c1f7420a09 100644 --- a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts @@ -50,6 +50,16 @@ export abstract class PolicyService { policies?: Policy[], ) => Observable; + /** + * Combines all Master Password policies that are passed in and returns + * back the strongest combination of all the policies in the form of a + * MasterPasswordPolicyOptions. + * @param policies + */ + abstract combinePoliciesIntoMasterPasswordPolicyOptions( + policies: Policy[], + ): MasterPasswordPolicyOptions | undefined; + /** * Evaluates whether a proposed Master Password complies with all Master Password policies that apply to the user. */ diff --git a/libs/common/src/admin-console/services/policy/default-policy.service.spec.ts b/libs/common/src/admin-console/services/policy/default-policy.service.spec.ts index 7787bdbc943..7dbf22cdf03 100644 --- a/libs/common/src/admin-console/services/policy/default-policy.service.spec.ts +++ b/libs/common/src/admin-console/services/policy/default-policy.service.spec.ts @@ -536,6 +536,126 @@ describe("PolicyService", () => { }); }); + describe("combinePoliciesIntoMasterPasswordPolicyOptions", () => { + let policyService: DefaultPolicyService; + let stateProvider: FakeStateProvider; + let organizationService: MockProxy; + + beforeEach(() => { + stateProvider = new FakeStateProvider(mockAccountServiceWith(userId)); + organizationService = mock(); + policyService = new DefaultPolicyService(stateProvider, organizationService); + }); + + it("returns undefined when there are no policies", () => { + const result = policyService.combinePoliciesIntoMasterPasswordPolicyOptions([]); + expect(result).toBeUndefined(); + }); + + it("returns options for a single policy", () => { + const masterPasswordPolicyRequirements = { + minComplexity: 3, + minLength: 10, + requireUpper: true, + }; + const policies = [ + new Policy( + policyData( + "1", + "org1", + PolicyType.MasterPassword, + true, + masterPasswordPolicyRequirements, + ), + ), + ]; + + const result = policyService.combinePoliciesIntoMasterPasswordPolicyOptions(policies); + + expect(result).toEqual({ + minComplexity: 3, + minLength: 10, + requireUpper: true, + requireLower: false, + requireNumbers: false, + requireSpecial: false, + enforceOnLogin: false, + }); + }); + + it("merges options from multiple policies", () => { + const masterPasswordPolicyRequirements1 = { + minComplexity: 3, + minLength: 10, + requireUpper: true, + }; + const masterPasswordPolicyRequirements2 = { minComplexity: 5, requireNumbers: true }; + const policies = [ + new Policy( + policyData( + "1", + "org1", + PolicyType.MasterPassword, + true, + masterPasswordPolicyRequirements1, + ), + ), + new Policy( + policyData( + "2", + "org2", + PolicyType.MasterPassword, + true, + masterPasswordPolicyRequirements2, + ), + ), + ]; + + const result = policyService.combinePoliciesIntoMasterPasswordPolicyOptions(policies); + + expect(result).toEqual({ + minComplexity: 5, + minLength: 10, + requireUpper: true, + requireLower: false, + requireNumbers: true, + requireSpecial: false, + enforceOnLogin: false, + }); + }); + + it("ignores disabled policies", () => { + const masterPasswordPolicyRequirements = { + minComplexity: 3, + minLength: 10, + requireUpper: true, + }; + const policies = [ + new Policy( + policyData( + "1", + "org1", + PolicyType.MasterPassword, + false, + masterPasswordPolicyRequirements, + ), + ), + ]; + + const result = policyService.combinePoliciesIntoMasterPasswordPolicyOptions(policies); + + expect(result).toBeUndefined(); + }); + + it("ignores policies with no data", () => { + const policies = [new Policy(policyData("1", "org1", PolicyType.MasterPassword, true))]; + + const result = policyService.combinePoliciesIntoMasterPasswordPolicyOptions(policies); + + expect(result).toBeUndefined(); + }); + }); + function policyData( id: string, organizationId: string, diff --git a/libs/common/src/admin-console/services/policy/default-policy.service.ts b/libs/common/src/admin-console/services/policy/default-policy.service.ts index 2f079eb2ad1..7f9418c2628 100644 --- a/libs/common/src/admin-console/services/policy/default-policy.service.ts +++ b/libs/common/src/admin-console/services/policy/default-policy.service.ts @@ -88,64 +88,35 @@ export class DefaultPolicyService implements PolicyService { ): Observable { const policies$ = policies ? of(policies) : this.policies$(userId); return policies$.pipe( - map((obsPolicies) => { - let enforcedOptions: MasterPasswordPolicyOptions | undefined = undefined; - const filteredPolicies = - obsPolicies.filter((p) => p.type === PolicyType.MasterPassword) ?? []; - - if (filteredPolicies.length === 0) { - return; - } - - filteredPolicies.forEach((currentPolicy) => { - if (!currentPolicy.enabled || !currentPolicy.data) { - return; - } - - if (!enforcedOptions) { - enforcedOptions = new MasterPasswordPolicyOptions(); - } - - if ( - currentPolicy.data.minComplexity != null && - currentPolicy.data.minComplexity > enforcedOptions.minComplexity - ) { - enforcedOptions.minComplexity = currentPolicy.data.minComplexity; - } - - if ( - currentPolicy.data.minLength != null && - currentPolicy.data.minLength > enforcedOptions.minLength - ) { - enforcedOptions.minLength = currentPolicy.data.minLength; - } - - if (currentPolicy.data.requireUpper) { - enforcedOptions.requireUpper = true; - } - - if (currentPolicy.data.requireLower) { - enforcedOptions.requireLower = true; - } - - if (currentPolicy.data.requireNumbers) { - enforcedOptions.requireNumbers = true; - } - - if (currentPolicy.data.requireSpecial) { - enforcedOptions.requireSpecial = true; - } - - if (currentPolicy.data.enforceOnLogin) { - enforcedOptions.enforceOnLogin = true; - } - }); - - return enforcedOptions; - }), + map((obsPolicies) => this.combinePoliciesIntoMasterPasswordPolicyOptions(obsPolicies)), ); } + combinePoliciesIntoMasterPasswordPolicyOptions( + policies: Policy[], + ): MasterPasswordPolicyOptions | undefined { + let enforcedOptions: MasterPasswordPolicyOptions | undefined = undefined; + const filteredPolicies = policies.filter((p) => p.type === PolicyType.MasterPassword) ?? []; + + if (filteredPolicies.length === 0) { + return; + } + + filteredPolicies.forEach((currentPolicy) => { + if (!currentPolicy.enabled || !currentPolicy.data) { + return undefined; + } + + if (!enforcedOptions) { + enforcedOptions = new MasterPasswordPolicyOptions(); + } + + this.mergeMasterPasswordPolicyOptions(enforcedOptions, currentPolicy.data); + }); + + return enforcedOptions; + } + evaluateMasterPassword( passwordStrength: number, newPassword: string, @@ -245,4 +216,32 @@ export class DefaultPolicyService implements PolicyService { return organization.canManagePolicies; } } + + private mergeMasterPasswordPolicyOptions( + target: MasterPasswordPolicyOptions | undefined, + source: MasterPasswordPolicyOptions | undefined, + ) { + if (!target) { + target = new MasterPasswordPolicyOptions(); + } + + // Take the max of the complexity or the required length of the password. + // For boolean properties, take the target's value if the source is undefined, + // otherwise take true in other scenarios. + if (source) { + target.minComplexity = Math.max( + target.minComplexity, + source.minComplexity ?? target.minComplexity, + ); + target.minLength = Math.max(target.minLength, source.minLength ?? target.minLength); + target.requireUpper = target.requireUpper || (source.requireUpper ?? target.requireUpper); + target.requireLower = target.requireLower || (source.requireLower ?? target.requireLower); + target.requireNumbers = + target.requireNumbers || (source.requireNumbers ?? target.requireNumbers); + target.requireSpecial = + target.requireSpecial || (source.requireSpecial ?? target.requireSpecial); + target.enforceOnLogin = + target.enforceOnLogin || (source.enforceOnLogin ?? target.enforceOnLogin); + } + } }