diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index a62dac05430..38e683c5e01 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -2889,8 +2889,8 @@ "generateEmail": { "message": "Generate email" }, - "generatorBoundariesHint": { - "message": "Value must be between $MIN$ and $MAX$", + "spinboxBoundariesHint": { + "message": "Value must be between $MIN$ and $MAX$.", "description": "Explains spin box minimum and maximum values to the user", "placeholders": { "min": { @@ -2903,6 +2903,26 @@ } } }, + "passwordLengthRecommendationHint": { + "message": " Use $RECOMMENDED$ characters or more to generate a strong password.", + "description": "Appended to `spinboxBoundariesHint` to recommend a length to the user. This must include any language-specific 'sentence' separator characters (e.g. a space in english).", + "placeholders": { + "recommended": { + "content": "$1", + "example": "14" + } + } + }, + "passphraseNumWordsRecommendationHint": { + "message": " Use $RECOMMENDED$ words or more to generate a strong passphrase.", + "description": "Appended to `spinboxBoundariesHint` to recommend a number of words to the user. This must include any language-specific 'sentence' separator characters (e.g. a space in english).", + "placeholders": { + "recommended": { + "content": "$1", + "example": "6" + } + } + }, "usernameType": { "message": "Username type" }, diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index 83fa064a35e..78358bc0099 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -2444,8 +2444,8 @@ "generateEmail": { "message": "Generate email" }, - "generatorBoundariesHint": { - "message": "Value must be between $MIN$ and $MAX$", + "spinboxBoundariesHint": { + "message": "Value must be between $MIN$ and $MAX$.", "description": "Explains spin box minimum and maximum values to the user", "placeholders": { "min": { @@ -2458,6 +2458,26 @@ } } }, + "passwordLengthRecommendationHint": { + "message": " Use $RECOMMENDED$ characters or more to generate a strong password.", + "description": "Appended to `spinboxBoundariesHint` to recommend a length to the user. This must include any language-specific 'sentence' separator characters (e.g. a space in english).", + "placeholders": { + "recommended": { + "content": "$1", + "example": "14" + } + } + }, + "passphraseNumWordsRecommendationHint": { + "message": " Use $RECOMMENDED$ words or more to generate a strong passphrase.", + "description": "Appended to `spinboxBoundariesHint` to recommend a number of words to the user. This must include any language-specific 'sentence' separator characters (e.g. a space in english).", + "placeholders": { + "recommended": { + "content": "$1", + "example": "6" + } + } + }, "usernameType": { "message": "Username type" }, diff --git a/apps/web/src/app/admin-console/organizations/policies/password-generator.component.ts b/apps/web/src/app/admin-console/organizations/policies/password-generator.component.ts index e1568da0487..9876876b295 100644 --- a/apps/web/src/app/admin-console/organizations/policies/password-generator.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/password-generator.component.ts @@ -5,7 +5,7 @@ import { BehaviorSubject, map } from "rxjs"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { DefaultPassphraseBoundaries, DefaultPasswordBoundaries } from "@bitwarden/generator-core"; +import { Generators } from "@bitwarden/generator-core"; import { BasePolicy, BasePolicyComponent } from "./base-policy.component"; @@ -26,8 +26,8 @@ export class PasswordGeneratorPolicyComponent extends BasePolicyComponent { minLength: [ null, [ - Validators.min(DefaultPasswordBoundaries.length.min), - Validators.max(DefaultPasswordBoundaries.length.max), + Validators.min(Generators.password.settings.constraints.length.min), + Validators.max(Generators.password.settings.constraints.length.max), ], ], useUpper: [null], @@ -37,22 +37,22 @@ export class PasswordGeneratorPolicyComponent extends BasePolicyComponent { minNumbers: [ null, [ - Validators.min(DefaultPasswordBoundaries.minDigits.min), - Validators.max(DefaultPasswordBoundaries.minDigits.max), + Validators.min(Generators.password.settings.constraints.minNumber.min), + Validators.max(Generators.password.settings.constraints.minNumber.max), ], ], minSpecial: [ null, [ - Validators.min(DefaultPasswordBoundaries.minSpecialCharacters.min), - Validators.max(DefaultPasswordBoundaries.minSpecialCharacters.max), + Validators.min(Generators.password.settings.constraints.minSpecial.min), + Validators.max(Generators.password.settings.constraints.minSpecial.max), ], ], minNumberWords: [ null, [ - Validators.min(DefaultPassphraseBoundaries.numWords.min), - Validators.max(DefaultPassphraseBoundaries.numWords.max), + Validators.min(Generators.passphrase.settings.constraints.numWords.min), + Validators.max(Generators.passphrase.settings.constraints.numWords.max), ], ], capitalize: [null], diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index cb0456bc039..a24a9acd90d 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -6468,8 +6468,8 @@ "generateEmail": { "message": "Generate email" }, - "generatorBoundariesHint": { - "message": "Value must be between $MIN$ and $MAX$", + "spinboxBoundariesHint": { + "message": "Value must be between $MIN$ and $MAX$.", "description": "Explains spin box minimum and maximum values to the user", "placeholders": { "min": { @@ -6482,6 +6482,26 @@ } } }, + "passwordLengthRecommendationHint": { + "message": " Use $RECOMMENDED$ characters or more to generate a strong password.", + "description": "Appended to `spinboxBoundariesHint` to recommend a length to the user. This must include any language-specific 'sentence' separator characters (e.g. a space in english).", + "placeholders": { + "recommended": { + "content": "$1", + "example": "14" + } + } + }, + "passphraseNumWordsRecommendationHint": { + "message": " Use $RECOMMENDED$ words or more to generate a strong passphrase.", + "description": "Appended to `spinboxBoundariesHint` to recommend a number of words to the user. This must include any language-specific 'sentence' separator characters (e.g. a space in english).", + "placeholders": { + "recommended": { + "content": "$1", + "example": "6" + } + } + }, "usernameType": { "message": "Username type" }, diff --git a/libs/common/src/tools/types.ts b/libs/common/src/tools/types.ts index 9b746924278..83f451351c2 100644 --- a/libs/common/src/tools/types.ts +++ b/libs/common/src/tools/types.ts @@ -28,6 +28,11 @@ type NumberConstraints = { /** maximum number value. When absent, min value is unbounded. */ max?: number; + /** recommended value. This is the value bitwarden recommends + * to the user as an appropriate value. + */ + recommendation?: number; + /** requires the number be a multiple of the step value; * this field must be a positive number. +0 and Infinity are * prohibited. When absent, any number is accepted. diff --git a/libs/tools/generator/components/src/passphrase-settings.component.ts b/libs/tools/generator/components/src/passphrase-settings.component.ts index 9ab692348eb..7dba24a8ab5 100644 --- a/libs/tools/generator/components/src/passphrase-settings.component.ts +++ b/libs/tools/generator/components/src/passphrase-settings.component.ts @@ -82,9 +82,24 @@ export class PassphraseSettingsComponent implements OnInit, OnDestroy { const settings = await this.generatorService.settings(Generators.passphrase, { singleUserId$ }); // skips reactive event emissions to break a subscription cycle - settings.pipe(takeUntil(this.destroyed$)).subscribe((s) => { - this.settings.patchValue(s, { emitEvent: false }); - }); + settings.withConstraints$ + .pipe(takeUntil(this.destroyed$)) + .subscribe(({ state, constraints }) => { + this.settings.patchValue(state, { emitEvent: false }); + + let boundariesHint = this.i18nService.t( + "spinboxBoundariesHint", + constraints.numWords.min?.toString(), + constraints.numWords.max?.toString(), + ); + if (state.numWords <= (constraints.numWords.recommendation ?? 0)) { + boundariesHint += this.i18nService.t( + "passphraseNumWordsRecommendationHint", + constraints.numWords.recommendation?.toString(), + ); + } + this.numWordsBoundariesHint.next(boundariesHint); + }); // the first emission is the current value; subsequent emissions are updates settings.pipe(skip(1), takeUntil(this.destroyed$)).subscribe(this.onUpdated); @@ -99,13 +114,6 @@ export class PassphraseSettingsComponent implements OnInit, OnDestroy { this.toggleEnabled(Controls.capitalize, !constraints.capitalize?.readonly); this.toggleEnabled(Controls.includeNumber, !constraints.includeNumber?.readonly); - - const boundariesHint = this.i18nService.t( - "generatorBoundariesHint", - constraints.numWords.min?.toString(), - constraints.numWords.max?.toString(), - ); - this.numWordsBoundariesHint.next(boundariesHint); }); // now that outputs are set up, connect inputs diff --git a/libs/tools/generator/components/src/password-settings.component.ts b/libs/tools/generator/components/src/password-settings.component.ts index 7693072971d..8e52a638f62 100644 --- a/libs/tools/generator/components/src/password-settings.component.ts +++ b/libs/tools/generator/components/src/password-settings.component.ts @@ -112,20 +112,33 @@ export class PasswordSettingsComponent implements OnInit, OnDestroy { const settings = await this.generatorService.settings(Generators.password, { singleUserId$ }); // bind settings to the UI - settings + settings.withConstraints$ .pipe( - map((settings) => { + map(({ state, constraints }) => { // interface is "avoid" while storage is "include" - const s: any = { ...settings }; + const s: any = { ...state }; s.avoidAmbiguous = !s.ambiguous; delete s.ambiguous; - return s; + return [s, constraints] as const; }), takeUntil(this.destroyed$), ) - .subscribe((s) => { + .subscribe(([state, constraints]) => { + let boundariesHint = this.i18nService.t( + "spinboxBoundariesHint", + constraints.length.min?.toString(), + constraints.length.max?.toString(), + ); + if (state.length <= (constraints.length.recommendation ?? 0)) { + boundariesHint += this.i18nService.t( + "passwordLengthRecommendationHint", + constraints.length.recommendation?.toString(), + ); + } + this.lengthBoundariesHint.next(boundariesHint); + // skips reactive event emissions to break a subscription cycle - this.settings.patchValue(s, { emitEvent: false }); + this.settings.patchValue(state, { emitEvent: false }); }); // explain policy & disable policy-overridden fields @@ -148,13 +161,6 @@ export class PasswordSettingsComponent implements OnInit, OnDestroy { for (const [control, enabled] of toggles) { this.toggleEnabled(control, enabled); } - - const boundariesHint = this.i18nService.t( - "generatorBoundariesHint", - constraints.length.min?.toString(), - constraints.length.max?.toString(), - ); - this.lengthBoundariesHint.next(boundariesHint); }); // cascade selections between checkboxes and spinboxes diff --git a/libs/tools/generator/core/src/data/default-passphrase-boundaries.ts b/libs/tools/generator/core/src/data/default-passphrase-boundaries.ts index 875a15051a2..d4aca717093 100644 --- a/libs/tools/generator/core/src/data/default-passphrase-boundaries.ts +++ b/libs/tools/generator/core/src/data/default-passphrase-boundaries.ts @@ -1,6 +1,6 @@ function initializeBoundaries() { const numWords = Object.freeze({ - min: 6, + min: 3, max: 20, }); diff --git a/libs/tools/generator/core/src/data/generators.ts b/libs/tools/generator/core/src/data/generators.ts index 467a84dd3b4..5ee451bfc5b 100644 --- a/libs/tools/generator/core/src/data/generators.ts +++ b/libs/tools/generator/core/src/data/generators.ts @@ -71,6 +71,7 @@ const PASSPHRASE: CredentialGeneratorConfiguration< numWords: { min: DefaultPassphraseBoundaries.numWords.min, max: DefaultPassphraseBoundaries.numWords.max, + recommendation: DefaultPassphraseGenerationOptions.numWords, }, wordSeparator: { maxLength: 1 }, }, @@ -101,7 +102,8 @@ const PASSPHRASE: CredentialGeneratorConfiguration< }), combine: passphraseLeastPrivilege, createEvaluator: (policy) => new PassphraseGeneratorOptionsEvaluator(policy), - toConstraints: (policy) => new PassphrasePolicyConstraints(policy), + toConstraints: (policy) => + new PassphrasePolicyConstraints(policy, PASSPHRASE.settings.constraints), }, }); @@ -130,6 +132,7 @@ const PASSWORD: CredentialGeneratorConfiguration< length: { min: DefaultPasswordBoundaries.length.min, max: DefaultPasswordBoundaries.length.max, + recommendation: DefaultPasswordGenerationOptions.length, }, minNumber: { min: DefaultPasswordBoundaries.minDigits.min, @@ -177,7 +180,8 @@ const PASSWORD: CredentialGeneratorConfiguration< }), combine: passwordLeastPrivilege, createEvaluator: (policy) => new PasswordGeneratorOptionsEvaluator(policy), - toConstraints: (policy) => new DynamicPasswordPolicyConstraints(policy), + toConstraints: (policy) => + new DynamicPasswordPolicyConstraints(policy, PASSWORD.settings.constraints), }, }); diff --git a/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.spec.ts b/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.spec.ts index d05d75ffb76..c8ae02ef723 100644 --- a/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.spec.ts +++ b/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.spec.ts @@ -1,28 +1,36 @@ -import { DefaultPasswordBoundaries, DefaultPasswordGenerationOptions, Policies } from "../data"; +import { ObjectKey } from "@bitwarden/common/tools/state/object-key"; + +import { Generators } from "../data"; +import { PasswordGeneratorSettings } from "../types"; import { AtLeastOne, Zero } from "./constraints"; import { DynamicPasswordPolicyConstraints } from "./dynamic-password-policy-constraints"; +const accoutSettings = Generators.password.settings.account as ObjectKey; +const defaultOptions = accoutSettings.initial; +const disabledPolicy = Generators.password.policy.disabledValue; +const someConstraints = Generators.password.settings.constraints; + describe("DynamicPasswordPolicyConstraints", () => { describe("constructor", () => { it("uses default boundaries when the policy is disabled", () => { - const { constraints } = new DynamicPasswordPolicyConstraints(Policies.Password.disabledValue); + const { constraints } = new DynamicPasswordPolicyConstraints(disabledPolicy, someConstraints); expect(constraints.policyInEffect).toBeFalsy(); - expect(constraints.length).toEqual(DefaultPasswordBoundaries.length); + expect(constraints.length).toEqual(someConstraints.length); expect(constraints.lowercase).toBeUndefined(); expect(constraints.uppercase).toBeUndefined(); expect(constraints.number).toBeUndefined(); expect(constraints.special).toBeUndefined(); expect(constraints.minLowercase).toBeUndefined(); expect(constraints.minUppercase).toBeUndefined(); - expect(constraints.minNumber).toEqual(DefaultPasswordBoundaries.minDigits); - expect(constraints.minSpecial).toEqual(DefaultPasswordBoundaries.minSpecialCharacters); + expect(constraints.minNumber).toEqual(someConstraints.minNumber); + expect(constraints.minSpecial).toEqual(someConstraints.minSpecial); }); it("1 <= minLowercase when the policy requires lowercase", () => { - const policy = { ...Policies.Password.disabledValue, useLowercase: true }; - const { constraints } = new DynamicPasswordPolicyConstraints(policy); + const policy = { ...disabledPolicy, useLowercase: true }; + const { constraints } = new DynamicPasswordPolicyConstraints(policy, someConstraints); expect(constraints.policyInEffect).toBeTruthy(); expect(constraints.lowercase.readonly).toEqual(true); @@ -31,8 +39,8 @@ describe("DynamicPasswordPolicyConstraints", () => { }); it("1 <= minUppercase when the policy requires uppercase", () => { - const policy = { ...Policies.Password.disabledValue, useUppercase: true }; - const { constraints } = new DynamicPasswordPolicyConstraints(policy); + const policy = { ...disabledPolicy, useUppercase: true }; + const { constraints } = new DynamicPasswordPolicyConstraints(policy, someConstraints); expect(constraints.policyInEffect).toBeTruthy(); expect(constraints.uppercase.readonly).toEqual(true); @@ -41,8 +49,8 @@ describe("DynamicPasswordPolicyConstraints", () => { }); it("1 <= minNumber <= 9 when the policy requires a number", () => { - const policy = { ...Policies.Password.disabledValue, useNumbers: true }; - const { constraints } = new DynamicPasswordPolicyConstraints(policy); + const policy = { ...disabledPolicy, useNumbers: true }; + const { constraints } = new DynamicPasswordPolicyConstraints(policy, someConstraints); expect(constraints.policyInEffect).toBeTruthy(); expect(constraints.number.readonly).toEqual(true); @@ -51,8 +59,8 @@ describe("DynamicPasswordPolicyConstraints", () => { }); it("1 <= minSpecial <= 9 when the policy requires a special character", () => { - const policy = { ...Policies.Password.disabledValue, useSpecial: true }; - const { constraints } = new DynamicPasswordPolicyConstraints(policy); + const policy = { ...disabledPolicy, useSpecial: true }; + const { constraints } = new DynamicPasswordPolicyConstraints(policy, someConstraints); expect(constraints.policyInEffect).toBeTruthy(); expect(constraints.special.readonly).toEqual(true); @@ -61,8 +69,8 @@ describe("DynamicPasswordPolicyConstraints", () => { }); it("numberCount <= minNumber <= 9 when the policy requires numberCount", () => { - const policy = { ...Policies.Password.disabledValue, useNumbers: true, numberCount: 2 }; - const { constraints } = new DynamicPasswordPolicyConstraints(policy); + const policy = { ...disabledPolicy, useNumbers: true, numberCount: 2 }; + const { constraints } = new DynamicPasswordPolicyConstraints(policy, someConstraints); expect(constraints.policyInEffect).toBeTruthy(); expect(constraints.number.readonly).toEqual(true); @@ -71,8 +79,8 @@ describe("DynamicPasswordPolicyConstraints", () => { }); it("specialCount <= minSpecial <= 9 when the policy requires specialCount", () => { - const policy = { ...Policies.Password.disabledValue, useSpecial: true, specialCount: 2 }; - const { constraints } = new DynamicPasswordPolicyConstraints(policy); + const policy = { ...disabledPolicy, useSpecial: true, specialCount: 2 }; + const { constraints } = new DynamicPasswordPolicyConstraints(policy, someConstraints); expect(constraints.policyInEffect).toBeTruthy(); expect(constraints.special.readonly).toEqual(true); @@ -81,16 +89,16 @@ describe("DynamicPasswordPolicyConstraints", () => { }); it("uses the policy's minimum length when the policy defines one", () => { - const policy = { ...Policies.Password.disabledValue, minLength: 10 }; - const { constraints } = new DynamicPasswordPolicyConstraints(policy); + const policy = { ...disabledPolicy, minLength: 10 }; + const { constraints } = new DynamicPasswordPolicyConstraints(policy, someConstraints); expect(constraints.policyInEffect).toBeTruthy(); - expect(constraints.length).toEqual({ min: 10, max: 128 }); + expect(constraints.length).toEqual({ ...someConstraints.length, min: 10 }); }); it("overrides the minimum length when it is less than the sum of minimums", () => { const policy = { - ...Policies.Password.disabledValue, + ...disabledPolicy, useUppercase: true, useLowercase: true, useNumbers: true, @@ -98,24 +106,27 @@ describe("DynamicPasswordPolicyConstraints", () => { useSpecial: true, specialCount: 5, }; - const { constraints } = new DynamicPasswordPolicyConstraints(policy); + const { constraints } = new DynamicPasswordPolicyConstraints(policy, someConstraints); // lower + upper + number + special = 1 + 1 + 5 + 5 = 12 - expect(constraints.length).toEqual({ min: 12, max: 128 }); + expect(constraints.length).toEqual({ ...someConstraints.length, min: 12 }); }); }); describe("calibrate", () => { it("copies the boolean constraints into the calibration", () => { - const dynamic = new DynamicPasswordPolicyConstraints({ - ...Policies.Password.disabledValue, - useUppercase: true, - useLowercase: true, - useNumbers: true, - useSpecial: true, - }); + const dynamic = new DynamicPasswordPolicyConstraints( + { + ...disabledPolicy, + useUppercase: true, + useLowercase: true, + useNumbers: true, + useSpecial: true, + }, + someConstraints, + ); - const calibrated = dynamic.calibrate(DefaultPasswordGenerationOptions); + const calibrated = dynamic.calibrate(defaultOptions); expect(calibrated.constraints.uppercase).toEqual(dynamic.constraints.uppercase); expect(calibrated.constraints.lowercase).toEqual(dynamic.constraints.lowercase); @@ -126,12 +137,15 @@ describe("DynamicPasswordPolicyConstraints", () => { it.each([[true], [false], [undefined]])( "outputs at least 1 constraint when the state's lowercase flag is true and useLowercase is %p", (useLowercase) => { - const dynamic = new DynamicPasswordPolicyConstraints({ - ...Policies.Password.disabledValue, - useLowercase, - }); + const dynamic = new DynamicPasswordPolicyConstraints( + { + ...disabledPolicy, + useLowercase, + }, + someConstraints, + ); const state = { - ...DefaultPasswordGenerationOptions, + ...defaultOptions, lowercase: true, }; @@ -142,9 +156,9 @@ describe("DynamicPasswordPolicyConstraints", () => { ); it("outputs the `minLowercase` constraint when the state's lowercase flag is true and policy is disabled", () => { - const dynamic = new DynamicPasswordPolicyConstraints(Policies.Password.disabledValue); + const dynamic = new DynamicPasswordPolicyConstraints(disabledPolicy, someConstraints); const state = { - ...DefaultPasswordGenerationOptions, + ...defaultOptions, lowercase: true, }; @@ -154,9 +168,9 @@ describe("DynamicPasswordPolicyConstraints", () => { }); it("disables the minLowercase constraint when the state's lowercase flag is false", () => { - const dynamic = new DynamicPasswordPolicyConstraints(Policies.Password.disabledValue); + const dynamic = new DynamicPasswordPolicyConstraints(disabledPolicy, someConstraints); const state = { - ...DefaultPasswordGenerationOptions, + ...defaultOptions, lowercase: false, }; @@ -168,12 +182,15 @@ describe("DynamicPasswordPolicyConstraints", () => { it.each([[true], [false], [undefined]])( "outputs at least 1 constraint when the state's uppercase flag is true and useUppercase is %p", (useUppercase) => { - const dynamic = new DynamicPasswordPolicyConstraints({ - ...Policies.Password.disabledValue, - useUppercase, - }); + const dynamic = new DynamicPasswordPolicyConstraints( + { + ...disabledPolicy, + useUppercase, + }, + someConstraints, + ); const state = { - ...DefaultPasswordGenerationOptions, + ...defaultOptions, uppercase: true, }; @@ -184,9 +201,9 @@ describe("DynamicPasswordPolicyConstraints", () => { ); it("disables the minUppercase constraint when the state's uppercase flag is false", () => { - const dynamic = new DynamicPasswordPolicyConstraints(Policies.Password.disabledValue); + const dynamic = new DynamicPasswordPolicyConstraints(disabledPolicy, someConstraints); const state = { - ...DefaultPasswordGenerationOptions, + ...defaultOptions, uppercase: false, }; @@ -196,9 +213,9 @@ describe("DynamicPasswordPolicyConstraints", () => { }); it("outputs the minNumber constraint when the state's number flag is true", () => { - const dynamic = new DynamicPasswordPolicyConstraints(Policies.Password.disabledValue); + const dynamic = new DynamicPasswordPolicyConstraints(disabledPolicy, someConstraints); const state = { - ...DefaultPasswordGenerationOptions, + ...defaultOptions, number: true, }; @@ -208,9 +225,9 @@ describe("DynamicPasswordPolicyConstraints", () => { }); it("outputs the zero constraint when the state's number flag is false", () => { - const dynamic = new DynamicPasswordPolicyConstraints(Policies.Password.disabledValue); + const dynamic = new DynamicPasswordPolicyConstraints(disabledPolicy, someConstraints); const state = { - ...DefaultPasswordGenerationOptions, + ...defaultOptions, number: false, }; @@ -220,9 +237,9 @@ describe("DynamicPasswordPolicyConstraints", () => { }); it("outputs the minSpecial constraint when the state's special flag is true", () => { - const dynamic = new DynamicPasswordPolicyConstraints(Policies.Password.disabledValue); + const dynamic = new DynamicPasswordPolicyConstraints(disabledPolicy, someConstraints); const state = { - ...DefaultPasswordGenerationOptions, + ...defaultOptions, special: true, }; @@ -232,9 +249,9 @@ describe("DynamicPasswordPolicyConstraints", () => { }); it("outputs the zero constraint when the state's special flag is false", () => { - const dynamic = new DynamicPasswordPolicyConstraints(Policies.Password.disabledValue); + const dynamic = new DynamicPasswordPolicyConstraints(disabledPolicy, someConstraints); const state = { - ...DefaultPasswordGenerationOptions, + ...defaultOptions, special: false, }; diff --git a/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.ts b/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.ts index 7fe76061885..eff77098bd2 100644 --- a/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.ts +++ b/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.ts @@ -1,10 +1,10 @@ import { + Constraints, DynamicStateConstraints, PolicyConstraints, StateConstraints, } from "@bitwarden/common/tools/types"; -import { DefaultPasswordBoundaries } from "../data"; import { PasswordGeneratorPolicy, PasswordGeneratorSettings } from "../types"; import { atLeast, atLeastSum, maybe, readonlyTrueWhen, AtLeastOne, Zero } from "./constraints"; @@ -18,26 +18,29 @@ export class DynamicPasswordPolicyConstraints * @param policy the password policy to enforce. This cannot be * `null` or `undefined`. */ - constructor(policy: PasswordGeneratorPolicy) { + constructor( + policy: PasswordGeneratorPolicy, + readonly defaults: Constraints, + ) { const minLowercase = maybe(policy.useLowercase, AtLeastOne); const minUppercase = maybe(policy.useUppercase, AtLeastOne); const minNumber = atLeast( policy.numberCount || (policy.useNumbers && AtLeastOne.min), - DefaultPasswordBoundaries.minDigits, + defaults.minNumber, ); const minSpecial = atLeast( policy.specialCount || (policy.useSpecial && AtLeastOne.min), - DefaultPasswordBoundaries.minSpecialCharacters, + defaults.minSpecial, ); - const baseLength = atLeast(policy.minLength, DefaultPasswordBoundaries.length); + const baseLength = atLeast(policy.minLength, defaults.length); const subLengths = [minLowercase, minUppercase, minNumber, minSpecial]; const length = atLeastSum(baseLength, subLengths); this.constraints = Object.freeze({ - policyInEffect: policyInEffect(policy), + policyInEffect: policyInEffect(policy, defaults), lowercase: readonlyTrueWhen(policy.useLowercase), uppercase: readonlyTrueWhen(policy.useUppercase), number: readonlyTrueWhen(policy.useNumbers), @@ -85,15 +88,18 @@ export class DynamicPasswordPolicyConstraints } } -function policyInEffect(policy: PasswordGeneratorPolicy): boolean { +function policyInEffect( + policy: PasswordGeneratorPolicy, + defaults: Constraints, +): boolean { const policies = [ policy.useUppercase, policy.useLowercase, policy.useNumbers, policy.useSpecial, - policy.minLength > DefaultPasswordBoundaries.length.min, - policy.numberCount > DefaultPasswordBoundaries.minDigits.min, - policy.specialCount > DefaultPasswordBoundaries.minSpecialCharacters.min, + policy.minLength > defaults.length.min, + policy.numberCount > defaults.minNumber.min, + policy.specialCount > defaults.minSpecial.min, ]; return policies.includes(true); diff --git a/libs/tools/generator/core/src/policies/passphrase-policy-constraints.spec.ts b/libs/tools/generator/core/src/policies/passphrase-policy-constraints.spec.ts index 7565be7936f..d6e0a5615dc 100644 --- a/libs/tools/generator/core/src/policies/passphrase-policy-constraints.spec.ts +++ b/libs/tools/generator/core/src/policies/passphrase-policy-constraints.spec.ts @@ -1,4 +1,4 @@ -import { DefaultPassphraseBoundaries, Policies } from "../data"; +import { Generators } from "../data"; import { PassphrasePolicyConstraints } from "./passphrase-policy-constraints"; @@ -9,54 +9,66 @@ const SomeSettings = { wordSeparator: "-", }; +const disabledPolicy = Generators.passphrase.policy.disabledValue; +const someConstraints = Generators.passphrase.settings.constraints; + describe("PassphrasePolicyConstraints", () => { describe("constructor", () => { it("uses default boundaries when the policy is disabled", () => { - const { constraints } = new PassphrasePolicyConstraints(Policies.Passphrase.disabledValue); + const { constraints } = new PassphrasePolicyConstraints(disabledPolicy, someConstraints); expect(constraints.policyInEffect).toBeFalsy(); expect(constraints.capitalize).toBeUndefined(); expect(constraints.includeNumber).toBeUndefined(); - expect(constraints.numWords).toEqual(DefaultPassphraseBoundaries.numWords); + expect(constraints.numWords).toEqual(someConstraints.numWords); }); it("requires capitalization when the policy requires capitalization", () => { - const { constraints } = new PassphrasePolicyConstraints({ - ...Policies.Passphrase.disabledValue, - capitalize: true, - }); + const { constraints } = new PassphrasePolicyConstraints( + { + ...disabledPolicy, + capitalize: true, + }, + someConstraints, + ); expect(constraints.policyInEffect).toBeTruthy(); expect(constraints.capitalize).toMatchObject({ readonly: true, requiredValue: true }); }); it("requires a number when the policy requires a number", () => { - const { constraints } = new PassphrasePolicyConstraints({ - ...Policies.Passphrase.disabledValue, - includeNumber: true, - }); + const { constraints } = new PassphrasePolicyConstraints( + { + ...disabledPolicy, + includeNumber: true, + }, + someConstraints, + ); expect(constraints.policyInEffect).toBeTruthy(); expect(constraints.includeNumber).toMatchObject({ readonly: true, requiredValue: true }); }); it("minNumberWords <= numWords.min when the policy requires numberCount", () => { - const { constraints } = new PassphrasePolicyConstraints({ - ...Policies.Passphrase.disabledValue, - minNumberWords: 10, - }); + const { constraints } = new PassphrasePolicyConstraints( + { + ...disabledPolicy, + minNumberWords: 10, + }, + someConstraints, + ); expect(constraints.policyInEffect).toBeTruthy(); expect(constraints.numWords).toMatchObject({ min: 10, - max: DefaultPassphraseBoundaries.numWords.max, + max: someConstraints.numWords.max, }); }); }); describe("adjust", () => { it("allows an empty word separator", () => { - const policy = new PassphrasePolicyConstraints(Policies.Passphrase.disabledValue); + const policy = new PassphrasePolicyConstraints(disabledPolicy, someConstraints); const { wordSeparator } = policy.adjust({ ...SomeSettings, wordSeparator: "" }); @@ -64,7 +76,7 @@ describe("PassphrasePolicyConstraints", () => { }); it("takes only the first character of wordSeparator", () => { - const policy = new PassphrasePolicyConstraints(Policies.Passphrase.disabledValue); + const policy = new PassphrasePolicyConstraints(disabledPolicy, someConstraints); const { wordSeparator } = policy.adjust({ ...SomeSettings, wordSeparator: "?." }); @@ -72,26 +84,32 @@ describe("PassphrasePolicyConstraints", () => { }); it.each([ - [1, 6], - [21, 20], - ])("fits numWords (=%p) within the default bounds (6 <= %p <= 20)", (value, expected) => { - const policy = new PassphrasePolicyConstraints(Policies.Passphrase.disabledValue); + [1, someConstraints.numWords.min, 3, someConstraints.numWords.max], + [21, someConstraints.numWords.min, 20, someConstraints.numWords.max], + ])( + `fits numWords (=%p) within the default bounds (%p <= %p <= %p)`, + (value, _, expected, __) => { + const policy = new PassphrasePolicyConstraints(disabledPolicy, someConstraints); - const { numWords } = policy.adjust({ ...SomeSettings, numWords: value }); + const { numWords } = policy.adjust({ ...SomeSettings, numWords: value }); - expect(numWords).toEqual(expected); - }); + expect(numWords).toEqual(expected); + }, + ); it.each([ - [1, 6, 6], - [21, 20, 20], + [1, 6, 6, someConstraints.numWords.max], + [21, 20, 20, someConstraints.numWords.max], ])( - "fits numWords (=%p) within the policy bounds (%p <= %p <= 20)", - (value, minNumberWords, expected) => { - const policy = new PassphrasePolicyConstraints({ - ...Policies.Passphrase.disabledValue, - minNumberWords, - }); + "fits numWords (=%p) within the policy bounds (%p <= %p <= %p)", + (value, minNumberWords, expected, _) => { + const policy = new PassphrasePolicyConstraints( + { + ...disabledPolicy, + minNumberWords, + }, + someConstraints, + ); const { numWords } = policy.adjust({ ...SomeSettings, numWords: value }); @@ -100,10 +118,13 @@ describe("PassphrasePolicyConstraints", () => { ); it("sets capitalize to true when the policy requires it", () => { - const policy = new PassphrasePolicyConstraints({ - ...Policies.Passphrase.disabledValue, - capitalize: true, - }); + const policy = new PassphrasePolicyConstraints( + { + ...disabledPolicy, + capitalize: true, + }, + someConstraints, + ); const { capitalize } = policy.adjust({ ...SomeSettings, capitalize: false }); @@ -111,10 +132,13 @@ describe("PassphrasePolicyConstraints", () => { }); it("sets includeNumber to true when the policy requires it", () => { - const policy = new PassphrasePolicyConstraints({ - ...Policies.Passphrase.disabledValue, - includeNumber: true, - }); + const policy = new PassphrasePolicyConstraints( + { + ...disabledPolicy, + includeNumber: true, + }, + someConstraints, + ); const { includeNumber } = policy.adjust({ ...SomeSettings, capitalize: false }); @@ -124,7 +148,7 @@ describe("PassphrasePolicyConstraints", () => { describe("fix", () => { it("returns its input", () => { - const policy = new PassphrasePolicyConstraints(Policies.Passphrase.disabledValue); + const policy = new PassphrasePolicyConstraints(disabledPolicy, someConstraints); const result = policy.fix(SomeSettings); diff --git a/libs/tools/generator/core/src/policies/passphrase-policy-constraints.ts b/libs/tools/generator/core/src/policies/passphrase-policy-constraints.ts index 27fb76991ec..aae5918ed46 100644 --- a/libs/tools/generator/core/src/policies/passphrase-policy-constraints.ts +++ b/libs/tools/generator/core/src/policies/passphrase-policy-constraints.ts @@ -1,6 +1,6 @@ -import { PolicyConstraints, StateConstraints } from "@bitwarden/common/tools/types"; +import { Constraints, PolicyConstraints, StateConstraints } from "@bitwarden/common/tools/types"; -import { DefaultPassphraseBoundaries, DefaultPassphraseGenerationOptions } from "../data"; +import { DefaultPassphraseGenerationOptions } from "../data"; import { PassphraseGenerationOptions, PassphraseGeneratorPolicy } from "../types"; import { atLeast, enforceConstant, fitLength, fitToBounds, readonlyTrueWhen } from "./constraints"; @@ -10,13 +10,16 @@ export class PassphrasePolicyConstraints implements StateConstraints, + ) { this.constraints = { - policyInEffect: policyInEffect(policy), + policyInEffect: policyInEffect(policy, defaults), wordSeparator: { minLength: 0, maxLength: 1 }, capitalize: readonlyTrueWhen(policy.capitalize), includeNumber: readonlyTrueWhen(policy.includeNumber), - numWords: atLeast(policy.minNumberWords, DefaultPassphraseBoundaries.numWords), + numWords: atLeast(policy.minNumberWords, defaults.numWords), }; } @@ -40,11 +43,14 @@ export class PassphrasePolicyConstraints implements StateConstraints, +): boolean { const policies = [ policy.capitalize, policy.includeNumber, - policy.minNumberWords > DefaultPassphraseBoundaries.numWords.min, + policy.minNumberWords > defaults.numWords.min, ]; return policies.includes(true);