[PM-14964] revert passphrase minimum (bitwarden#12019)

* revert passphrase minimum * add recommendation text to browser refresh; hide hint text when value exceeds recommendation * migrate validators to generator configuration
dlfi · Nov 18, 2024 · 3521c54 · 3521c54
1 parent 1541865
commit 3521c54
Show file tree

Hide file tree

Showing 13 changed files with 292 additions and 156 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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),
   },
 });