From cbe7ae68cc031c322957eb706b4718b858517fc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=9C=A8=20Audrey=20=E2=9C=A8?= Date: Fri, 9 Aug 2024 08:54:00 -0400 Subject: [PATCH] [PM-10107] evaluate the override password type policy (#10277) --- .../components/generator.component.ts | 20 ++++++-- .../password-generator-policy-options.ts | 4 +- .../core/src/data/generator-types.ts | 5 ++ libs/tools/generator/core/src/data/index.ts | 1 + .../core/src/types/generator-type.ts | 7 ++- ...legacy-password-generation.service.spec.ts | 8 ++-- .../src/legacy-password-generation.service.ts | 2 +- ...fault-generator-navigation.service.spec.ts | 2 +- .../generator-navigation-evaluator.spec.ts | 46 +++++++++---------- .../src/generator-navigation-evaluator.ts | 17 ++++--- .../src/generator-navigation-policy.spec.ts | 16 +++---- .../src/generator-navigation-policy.ts | 12 +++-- 12 files changed, 84 insertions(+), 56 deletions(-) create mode 100644 libs/tools/generator/core/src/data/generator-types.ts diff --git a/libs/angular/src/tools/generator/components/generator.component.ts b/libs/angular/src/tools/generator/components/generator.component.ts index 94bd3fc287..b68e861a06 100644 --- a/libs/angular/src/tools/generator/components/generator.component.ts +++ b/libs/angular/src/tools/generator/components/generator.component.ts @@ -34,7 +34,6 @@ export class GeneratorComponent implements OnInit, OnDestroy { usernameGeneratingPromise: Promise; typeOptions: any[]; - passTypeOptions: any[]; usernameTypeOptions: any[]; subaddressOptions: any[]; catchallOptions: any[]; @@ -48,6 +47,11 @@ export class GeneratorComponent implements OnInit, OnDestroy { enforcedPasswordPolicyOptions: PasswordGeneratorPolicyOptions; usernameWebsite: string = null; + get passTypeOptions() { + return this._passTypeOptions.filter((o) => !o.disabled); + } + private _passTypeOptions: { name: string; value: GeneratorType; disabled: boolean }[]; + private destroy$ = new Subject(); private isInitialized$ = new BehaviorSubject(false); @@ -79,9 +83,9 @@ export class GeneratorComponent implements OnInit, OnDestroy { { name: i18nService.t("password"), value: "password" }, { name: i18nService.t("username"), value: "username" }, ]; - this.passTypeOptions = [ - { name: i18nService.t("password"), value: "password" }, - { name: i18nService.t("passphrase"), value: "passphrase" }, + this._passTypeOptions = [ + { name: i18nService.t("password"), value: "password", disabled: false }, + { name: i18nService.t("passphrase"), value: "passphrase", disabled: false }, ]; this.usernameTypeOptions = [ { @@ -138,6 +142,14 @@ export class GeneratorComponent implements OnInit, OnDestroy { this.passwordOptions.type = this.passwordOptions.type === "passphrase" ? "passphrase" : "password"; + const overrideType = this.enforcedPasswordPolicyOptions.overridePasswordType ?? ""; + const isDisabled = overrideType.length + ? (value: string, policyValue: string) => value !== policyValue + : (_value: string, _policyValue: string) => false; + for (const option of this._passTypeOptions) { + option.disabled = isDisabled(option.value, overrideType); + } + if (this.usernameOptions.type == null) { this.usernameOptions.type = "word"; } diff --git a/libs/common/src/admin-console/models/domain/password-generator-policy-options.ts b/libs/common/src/admin-console/models/domain/password-generator-policy-options.ts index c52962a0a1..4ce27173ca 100644 --- a/libs/common/src/admin-console/models/domain/password-generator-policy-options.ts +++ b/libs/common/src/admin-console/models/domain/password-generator-policy-options.ts @@ -5,7 +5,7 @@ import Domain from "../../../platform/models/domain/domain-base"; */ export class PasswordGeneratorPolicyOptions extends Domain { /** The default kind of credential to generate */ - defaultType: "password" | "passphrase" | "" = ""; + overridePasswordType: "password" | "passphrase" | "" = ""; /** The minimum length of generated passwords. * When this is less than or equal to zero, it is ignored. @@ -70,7 +70,7 @@ export class PasswordGeneratorPolicyOptions extends Domain { */ inEffect() { return ( - this.defaultType || + this.overridePasswordType || this.minLength > 0 || this.numberCount > 0 || this.specialCount > 0 || diff --git a/libs/tools/generator/core/src/data/generator-types.ts b/libs/tools/generator/core/src/data/generator-types.ts new file mode 100644 index 0000000000..c68131f604 --- /dev/null +++ b/libs/tools/generator/core/src/data/generator-types.ts @@ -0,0 +1,5 @@ +/** Types of passwords that may be configured by the password generator */ +export const PasswordTypes = Object.freeze(["password", "passphrase"] as const); + +/** Types of generators that may be configured by the password generator */ +export const GeneratorTypes = Object.freeze([...PasswordTypes, "username"] as const); diff --git a/libs/tools/generator/core/src/data/index.ts b/libs/tools/generator/core/src/data/index.ts index d87e7f7c52..b4aa4fe3b9 100644 --- a/libs/tools/generator/core/src/data/index.ts +++ b/libs/tools/generator/core/src/data/index.ts @@ -17,3 +17,4 @@ export * from "./forwarders"; export * from "./integrations"; export * from "./policies"; export * from "./username-digits"; +export * from "./generator-types"; diff --git a/libs/tools/generator/core/src/types/generator-type.ts b/libs/tools/generator/core/src/types/generator-type.ts index f17eeb9c92..717b0e994e 100644 --- a/libs/tools/generator/core/src/types/generator-type.ts +++ b/libs/tools/generator/core/src/types/generator-type.ts @@ -1,2 +1,7 @@ +import { GeneratorTypes, PasswordTypes } from "../data/generator-types"; + /** The kind of credential being generated. */ -export type GeneratorType = "password" | "passphrase" | "username"; +export type GeneratorType = (typeof GeneratorTypes)[number]; + +/** The kinds of passwords that can be generated. */ +export type PasswordType = (typeof PasswordTypes)[number]; diff --git a/libs/tools/generator/extensions/legacy/src/legacy-password-generation.service.spec.ts b/libs/tools/generator/extensions/legacy/src/legacy-password-generation.service.spec.ts index 57d618c04a..4c1dee7a5d 100644 --- a/libs/tools/generator/extensions/legacy/src/legacy-password-generation.service.spec.ts +++ b/libs/tools/generator/extensions/legacy/src/legacy-password-generation.service.spec.ts @@ -270,7 +270,7 @@ describe("LegacyPasswordGenerationService", () => { const navigation = createNavigationGenerator( {}, { - defaultType: "password", + overridePasswordType: "password", }, ); const generator = new LegacyPasswordGenerationService( @@ -284,7 +284,7 @@ describe("LegacyPasswordGenerationService", () => { const [, policy] = await generator.getOptions(); expect(policy).toEqual({ - defaultType: "password", + overridePasswordType: "password", minLength: 20, numberCount: 10, specialCount: 11, @@ -402,7 +402,7 @@ describe("LegacyPasswordGenerationService", () => { const navigation = createNavigationGenerator( {}, { - defaultType: "password", + overridePasswordType: "password", }, ); const generator = new LegacyPasswordGenerationService( @@ -416,7 +416,7 @@ describe("LegacyPasswordGenerationService", () => { const [, policy] = await generator.enforcePasswordGeneratorPoliciesOnOptions({}); expect(policy).toEqual({ - defaultType: "password", + overridePasswordType: "password", minLength: 20, numberCount: 10, specialCount: 11, diff --git a/libs/tools/generator/extensions/legacy/src/legacy-password-generation.service.ts b/libs/tools/generator/extensions/legacy/src/legacy-password-generation.service.ts index e9bc3bbf2b..e62e4eb9a1 100644 --- a/libs/tools/generator/extensions/legacy/src/legacy-password-generation.service.ts +++ b/libs/tools/generator/extensions/legacy/src/legacy-password-generation.service.ts @@ -248,7 +248,7 @@ export class LegacyPasswordGenerationService implements PasswordGenerationServic ...options, ...navigationEvaluator.sanitize(navigationApplied), }; - if (options.type === "password") { + if (navigationSanitized.type === "password") { const applied = passwordEvaluator.applyPolicy(navigationSanitized); const sanitized = passwordEvaluator.sanitize(applied); return [sanitized, policy]; diff --git a/libs/tools/generator/extensions/navigation/src/default-generator-navigation.service.spec.ts b/libs/tools/generator/extensions/navigation/src/default-generator-navigation.service.spec.ts index d1aa5a98a8..1e558ab352 100644 --- a/libs/tools/generator/extensions/navigation/src/default-generator-navigation.service.spec.ts +++ b/libs/tools/generator/extensions/navigation/src/default-generator-navigation.service.spec.ts @@ -69,7 +69,7 @@ describe("DefaultGeneratorNavigationService", () => { organizationId: "" as any, enabled: true, type: PolicyType.PasswordGenerator, - data: { defaultType: "password" }, + data: { overridePasswordType: "password" }, }), ]); }, diff --git a/libs/tools/generator/extensions/navigation/src/generator-navigation-evaluator.spec.ts b/libs/tools/generator/extensions/navigation/src/generator-navigation-evaluator.spec.ts index 6fa8f2ef8f..218121a3a7 100644 --- a/libs/tools/generator/extensions/navigation/src/generator-navigation-evaluator.spec.ts +++ b/libs/tools/generator/extensions/navigation/src/generator-navigation-evaluator.spec.ts @@ -4,18 +4,18 @@ import { GeneratorNavigationEvaluator } from "./generator-navigation-evaluator"; describe("GeneratorNavigationEvaluator", () => { describe("policyInEffect", () => { it.each([["passphrase"], ["password"]] as const)( - "returns true if the policy has a defaultType (= %p)", - (defaultType) => { - const evaluator = new GeneratorNavigationEvaluator({ defaultType }); + "returns true if the policy has a overridePasswordType (= %p)", + (overridePasswordType) => { + const evaluator = new GeneratorNavigationEvaluator({ overridePasswordType }); expect(evaluator.policyInEffect).toEqual(true); }, ); it.each([[undefined], [null], ["" as any]])( - "returns false if the policy has a falsy defaultType (= %p)", - (defaultType) => { - const evaluator = new GeneratorNavigationEvaluator({ defaultType }); + "returns false if the policy has a falsy overridePasswordType (= %p)", + (overridePasswordType) => { + const evaluator = new GeneratorNavigationEvaluator({ overridePasswordType }); expect(evaluator.policyInEffect).toEqual(false); }, @@ -23,7 +23,7 @@ describe("GeneratorNavigationEvaluator", () => { }); describe("applyPolicy", () => { - it("returns the input options", () => { + it("returns the input options when a policy is not in effect", () => { const evaluator = new GeneratorNavigationEvaluator(null); const options = { type: "password" as const }; @@ -31,19 +31,27 @@ describe("GeneratorNavigationEvaluator", () => { expect(result).toEqual(options); }); + + it.each([["passphrase"], ["password"]] as const)( + "defaults options to the policy's default type (= %p) when a policy is in effect", + (overridePasswordType) => { + const evaluator = new GeneratorNavigationEvaluator({ overridePasswordType }); + + const result = evaluator.applyPolicy({}); + + expect(result).toEqual({ type: overridePasswordType }); + }, + ); }); describe("sanitize", () => { - it.each([["passphrase"], ["password"]] as const)( - "defaults options to the policy's default type (= %p) when a policy is in effect", - (defaultType) => { - const evaluator = new GeneratorNavigationEvaluator({ defaultType }); + it("retains the options type when it is set", () => { + const evaluator = new GeneratorNavigationEvaluator({ overridePasswordType: "passphrase" }); - const result = evaluator.sanitize({}); + const result = evaluator.sanitize({ type: "password" }); - expect(result).toEqual({ type: defaultType }); - }, - ); + expect(result).toEqual({ type: "password" }); + }); it("defaults options to the default generator navigation type when a policy is not in effect", () => { const evaluator = new GeneratorNavigationEvaluator(null); @@ -52,13 +60,5 @@ describe("GeneratorNavigationEvaluator", () => { expect(result.type).toEqual(DefaultGeneratorNavigation.type); }); - - it("retains the options type when it is set", () => { - const evaluator = new GeneratorNavigationEvaluator({ defaultType: "passphrase" }); - - const result = evaluator.sanitize({ type: "password" }); - - expect(result).toEqual({ type: "password" }); - }); }); }); diff --git a/libs/tools/generator/extensions/navigation/src/generator-navigation-evaluator.ts b/libs/tools/generator/extensions/navigation/src/generator-navigation-evaluator.ts index 772342a73a..75871e056c 100644 --- a/libs/tools/generator/extensions/navigation/src/generator-navigation-evaluator.ts +++ b/libs/tools/generator/extensions/navigation/src/generator-navigation-evaluator.ts @@ -1,4 +1,4 @@ -import { PolicyEvaluator } from "@bitwarden/generator-core"; +import { PasswordTypes, PolicyEvaluator } from "@bitwarden/generator-core"; import { DefaultGeneratorNavigation } from "./default-generator-navigation"; import { GeneratorNavigation } from "./generator-navigation"; @@ -17,7 +17,7 @@ export class GeneratorNavigationEvaluator /** {@link PolicyEvaluator.policyInEffect} */ get policyInEffect(): boolean { - return this.policy?.defaultType ? true : false; + return PasswordTypes.includes(this.policy?.overridePasswordType); } /** Apply policy to the input options. @@ -25,7 +25,13 @@ export class GeneratorNavigationEvaluator * @returns A new password generation request with policy applied. */ applyPolicy(options: GeneratorNavigation): GeneratorNavigation { - return options; + const result = { ...options }; + + if (this.policyInEffect) { + result.type = this.policy.overridePasswordType ?? result.type; + } + + return result; } /** Ensures internal options consistency. @@ -33,12 +39,9 @@ export class GeneratorNavigationEvaluator * @returns A passphrase generation request with cascade applied. */ sanitize(options: GeneratorNavigation): GeneratorNavigation { - const defaultType = this.policyInEffect - ? this.policy.defaultType - : DefaultGeneratorNavigation.type; return { ...options, - type: options.type ?? defaultType, + type: options.type ?? DefaultGeneratorNavigation.type, }; } } diff --git a/libs/tools/generator/extensions/navigation/src/generator-navigation-policy.spec.ts b/libs/tools/generator/extensions/navigation/src/generator-navigation-policy.spec.ts index 3c90f8a7e8..e4f0b08a3d 100644 --- a/libs/tools/generator/extensions/navigation/src/generator-navigation-policy.spec.ts +++ b/libs/tools/generator/extensions/navigation/src/generator-navigation-policy.spec.ts @@ -38,26 +38,26 @@ describe("leastPrivilege", () => { }); it("should take the %p from the policy", () => { - const policy = createPolicy({ defaultType: "passphrase" }); + const policy = createPolicy({ overridePasswordType: "passphrase" }); const result = preferPassword({ ...DisabledGeneratorNavigationPolicy }, policy); - expect(result).toEqual({ defaultType: "passphrase" }); + expect(result).toEqual({ overridePasswordType: "passphrase" }); }); it("should override passphrase with password", () => { - const policy = createPolicy({ defaultType: "password" }); + const policy = createPolicy({ overridePasswordType: "password" }); - const result = preferPassword({ defaultType: "passphrase" }, policy); + const result = preferPassword({ overridePasswordType: "passphrase" }, policy); - expect(result).toEqual({ defaultType: "password" }); + expect(result).toEqual({ overridePasswordType: "password" }); }); it("should not override password", () => { - const policy = createPolicy({ defaultType: "passphrase" }); + const policy = createPolicy({ overridePasswordType: "passphrase" }); - const result = preferPassword({ defaultType: "password" }, policy); + const result = preferPassword({ overridePasswordType: "password" }, policy); - expect(result).toEqual({ defaultType: "password" }); + expect(result).toEqual({ overridePasswordType: "password" }); }); }); diff --git a/libs/tools/generator/extensions/navigation/src/generator-navigation-policy.ts b/libs/tools/generator/extensions/navigation/src/generator-navigation-policy.ts index f52344d1fd..7dcf96c20e 100644 --- a/libs/tools/generator/extensions/navigation/src/generator-navigation-policy.ts +++ b/libs/tools/generator/extensions/navigation/src/generator-navigation-policy.ts @@ -2,14 +2,14 @@ import { PolicyType } from "@bitwarden/common/admin-console/enums"; // FIXME: use index.ts imports once policy abstractions and models // implement ADR-0002 import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; -import { GeneratorType } from "@bitwarden/generator-core"; +import { PasswordType } from "@bitwarden/generator-core"; /** Policy settings affecting password generator navigation */ export type GeneratorNavigationPolicy = { /** The type of generator that should be shown by default when opening * the password generator. */ - defaultType?: GeneratorType; + overridePasswordType?: PasswordType; }; /** Reduces a policy into an accumulator by preferring the password generator @@ -27,13 +27,15 @@ export function preferPassword( return acc; } - const isOverridable = acc.defaultType !== "password" && policy.data.defaultType; - const result = isOverridable ? { ...acc, defaultType: policy.data.defaultType } : acc; + const isOverridable = acc.overridePasswordType !== "password" && policy.data.overridePasswordType; + const result = isOverridable + ? { ...acc, overridePasswordType: policy.data.overridePasswordType } + : acc; return result; } /** The default options for password generation policy. */ export const DisabledGeneratorNavigationPolicy: GeneratorNavigationPolicy = Object.freeze({ - defaultType: undefined, + overridePasswordType: null, });