[PM-12303] fix password state spurious emissions (#11670)

* trace generation requests * eliminate spurious save caused by validator changes * fix emissions caused by setting bounds attrbutes --------- Co-authored-by: Daniel James Smith <[email protected]>
bitwarden · Oct 23, 2024 · 22be52d · 22be52d
1 parent 7c79487
commit 22be52d
Show file tree

Hide file tree

Showing 11 changed files with 64 additions and 91 deletions.
diff --git a/libs/tools/generator/components/src/credential-generator.component.html b/libs/tools/generator/components/src/credential-generator.component.html
@@ -20,9 +20,11 @@
       type="button"
       bitIconButton="bwi-generate"
       buttonType="main"
-      (click)="generate$.next()"
+      (click)="generate('user request')"
       [appA11yTitle]="credentialTypeGenerateLabel$ | async"
-    ></button>
+    >
+      {{ credentialTypeGenerateLabel$ | async }}
+    </button>
     <button
       type="button"
       bitIconButton="bwi-clone"
@@ -37,13 +39,13 @@
   class="tw-mt-6"
   *ngIf="(showAlgorithm$ | async)?.id === 'password'"
   [userId]="userId$ | async"
-  (onUpdated)="generate$.next()"
+  (onUpdated)="generate('password settings')"
 />
 <tools-passphrase-settings
   class="tw-mt-6"
   *ngIf="(showAlgorithm$ | async)?.id === 'passphrase'"
   [userId]="userId$ | async"
-  (onUpdated)="generate$.next()"
+  (onUpdated)="generate('passphrase settings')"
 />
 <bit-section *ngIf="(category$ | async) !== 'password'">
   <bit-section-header>
@@ -69,7 +71,7 @@ <h2 bitTypography="h6">{{ "options" | i18n }}</h2>
       <tools-catchall-settings
         *ngIf="(showAlgorithm$ | async)?.id === 'catchall'"
         [userId]="userId$ | async"
-        (onUpdated)="generate$.next()"
+        (onUpdated)="generate('catchall settings')"
       />
       <tools-forwarder-settings
         *ngIf="!!(forwarderId$ | async)"
@@ -79,12 +81,12 @@ <h2 bitTypography="h6">{{ "options" | i18n }}</h2>
       <tools-subaddress-settings
         *ngIf="(showAlgorithm$ | async)?.id === 'subaddress'"
         [userId]="userId$ | async"
-        (onUpdated)="generate$.next()"
+        (onUpdated)="generate('subaddress settings')"
       />
       <tools-username-settings
         *ngIf="(showAlgorithm$ | async)?.id === 'username'"
         [userId]="userId$ | async"
-        (onUpdated)="generate$.next()"
+        (onUpdated)="generate('username settings')"
       />
     </bit-card>
   </div>

diff --git a/libs/tools/generator/components/src/credential-generator.component.ts b/libs/tools/generator/components/src/credential-generator.component.ts
@@ -376,7 +376,7 @@ export class CredentialGeneratorComponent implements OnInit, OnDestroy {
         if (!a || a.onlyOnRequest) {
           this.value$.next("-");
         } else {
-          this.generate$.next();
+          this.generate("autogenerate");
         }
       });
     });
@@ -472,7 +472,15 @@ export class CredentialGeneratorComponent implements OnInit, OnDestroy {
   protected readonly userId$ = new BehaviorSubject<UserId>(null);
 
   /** Emits when a new credential is requested */
-  protected readonly generate$ = new Subject<void>();
+  private readonly generate$ = new Subject<string>();
+
+  /** Request a new value from the generator
+   * @param requestor a label used to trace generation request
+   *  origin in the debugger.
+   */
+  protected generate(requestor: string) {
+    this.generate$.next(requestor);
+  }
 
   private toOptions(algorithms: AlgorithmInfo[]) {
     const options: Option<string>[] = algorithms.map((algorithm) => ({

diff --git a/libs/tools/generator/components/src/forwarder-settings.component.ts b/libs/tools/generator/components/src/forwarder-settings.component.ts
@@ -143,6 +143,8 @@ export class ForwarderSettingsComponent implements OnInit, OnChanges, OnDestroy
             control.clearValidators();
           }
         }
+
+        this.settings.updateValueAndValidity({ emitEvent: false });
       });
 
     // the first emission is the current value; subsequent emissions are updates

diff --git a/libs/tools/generator/components/src/passphrase-settings.component.html b/libs/tools/generator/components/src/passphrase-settings.component.html
@@ -7,14 +7,7 @@ <h6 bitTypography="h6">{{ "options" | i18n }}</h6>
       <bit-card>
         <bit-form-field disableMargin>
           <bit-label>{{ "numWords" | i18n }}</bit-label>
-          <input
-            bitInput
-            formControlName="numWords"
-            id="num-words"
-            type="number"
-            [min]="minNumWords"
-            [max]="maxNumWords"
-          />
+          <input bitInput formControlName="numWords" id="num-words" type="number" />
         </bit-form-field>
       </bit-card>
     </div>

diff --git a/libs/tools/generator/components/src/passphrase-settings.component.ts b/libs/tools/generator/components/src/passphrase-settings.component.ts
@@ -91,9 +91,8 @@ export class PassphraseSettingsComponent implements OnInit, OnDestroy {
           .get(Controls.wordSeparator)
           .setValidators(toValidators(Controls.wordSeparator, Generators.passphrase, constraints));
 
-        // forward word boundaries to the template (can't do it through the rx form)
-        this.minNumWords = constraints.numWords.min;
-        this.maxNumWords = constraints.numWords.max;
+        this.settings.updateValueAndValidity({ emitEvent: false });
+
         this.policyInEffect = constraints.policyInEffect;
 
         this.toggleEnabled(Controls.capitalize, !constraints.capitalize?.readonly);
@@ -104,12 +103,6 @@ export class PassphraseSettingsComponent implements OnInit, OnDestroy {
     this.settings.valueChanges.pipe(takeUntil(this.destroyed$)).subscribe(settings);
   }
 
-  /** attribute binding for numWords[min] */
-  protected minNumWords: number;
-
-  /** attribute binding for numWords[max] */
-  protected maxNumWords: number;
-
   /** display binding for enterprise policy notice */
   protected policyInEffect: boolean;
 

diff --git a/libs/tools/generator/components/src/password-generator.component.html b/libs/tools/generator/components/src/password-generator.component.html
@@ -18,9 +18,11 @@
       type="button"
       bitIconButton="bwi-generate"
       buttonType="main"
-      (click)="generate$.next()"
+      (click)="generate('user request')"
       [appA11yTitle]="credentialTypeGenerateLabel$ | async"
-    ></button>
+    >
+      {{ credentialTypeGenerateLabel$ | async }}
+    </button>
     <button
       type="button"
       bitIconButton="bwi-clone"
@@ -36,12 +38,12 @@
   *ngIf="(algorithm$ | async)?.id === 'password'"
   [userId]="this.userId$ | async"
   [disableMargin]="disableMargin"
-  (onUpdated)="generate$.next()"
+  (onUpdated)="generate('password settings')"
 />
 <tools-passphrase-settings
   class="tw-mt-6"
   *ngIf="(algorithm$ | async)?.id === 'passphrase'"
   [userId]="this.userId$ | async"
-  (onUpdated)="generate$.next()"
+  (onUpdated)="generate('passphrase settings')"
   [disableMargin]="disableMargin"
 />
diff --git a/libs/tools/generator/components/src/password-generator.component.ts b/libs/tools/generator/components/src/password-generator.component.ts
@@ -59,7 +59,15 @@ export class PasswordGeneratorComponent implements OnInit, OnDestroy {
   protected readonly userId$ = new BehaviorSubject<UserId>(null);
 
   /** Emits when a new credential is requested */
-  protected readonly generate$ = new Subject<void>();
+  private readonly generate$ = new Subject<string>();
+
+  /** Request a new value from the generator
+   * @param requestor a label used to trace generation request
+   *  origin in the debugger.
+   */
+  protected generate(requestor: string) {
+    this.generate$.next(requestor);
+  }
 
   /** Tracks changes to the selected credential type
    * @param type the new credential type
@@ -154,7 +162,7 @@ export class PasswordGeneratorComponent implements OnInit, OnDestroy {
         filter((a) => !a.onlyOnRequest),
         takeUntil(this.destroyed),
       )
-      .subscribe(() => this.generate$.next());
+      .subscribe(() => this.generate("autogenerate"));
   }
 
   private typeToGenerator$(type: CredentialAlgorithm) {

diff --git a/libs/tools/generator/components/src/password-settings.component.html b/libs/tools/generator/components/src/password-settings.component.html
@@ -7,13 +7,7 @@ <h2 bitTypography="h6">{{ "options" | i18n }}</h2>
       <bit-card>
         <bit-form-field disableMargin>
           <bit-label>{{ "length" | i18n }}</bit-label>
-          <input
-            bitInput
-            formControlName="length"
-            type="number"
-            [min]="minLength"
-            [max]="maxLength"
-          />
+          <input bitInput formControlName="length" type="number" />
         </bit-form-field>
       </bit-card>
     </div>
@@ -57,23 +51,11 @@ <h2 bitTypography="h6">{{ "options" | i18n }}</h2>
         <div class="tw-flex">
           <bit-form-field class="tw-w-full tw-basis-1/2 tw-mr-4">
             <bit-label>{{ "minNumbers" | i18n }}</bit-label>
-            <input
-              bitInput
-              type="number"
-              [min]="minMinNumber"
-              [max]="maxMinNumber"
-              formControlName="minNumber"
-            />
+            <input bitInput type="number" formControlName="minNumber" />
           </bit-form-field>
           <bit-form-field class="tw-w-full tw-basis-1/2">
             <bit-label>{{ "minSpecial" | i18n }}</bit-label>
-            <input
-              bitInput
-              type="number"
-              [min]="minMinSpecial"
-              [max]="maxMinSpecial"
-              formControlName="minSpecial"
-            />
+            <input bitInput type="number" formControlName="minSpecial" />
           </bit-form-field>
         </div>
         <bit-form-control [disableMargin]="!policyInEffect">

diff --git a/libs/tools/generator/components/src/password-settings.component.ts b/libs/tools/generator/components/src/password-settings.component.ts
@@ -1,7 +1,7 @@
 import { coerceBooleanProperty } from "@angular/cdk/coercion";
 import { OnInit, Input, Output, EventEmitter, Component, OnDestroy } from "@angular/core";
 import { FormBuilder } from "@angular/forms";
-import { BehaviorSubject, takeUntil, Subject, map, filter, tap, debounceTime, skip } from "rxjs";
+import { BehaviorSubject, takeUntil, Subject, map, filter, tap, skip } from "rxjs";
 
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { UserId } from "@bitwarden/common/types/guid";
@@ -132,14 +132,6 @@ export class PasswordSettingsComponent implements OnInit, OnDestroy {
           toValidators(Controls.minSpecial, Generators.password, constraints),
         );
 
-        // forward word boundaries to the template (can't do it through the rx form)
-        this.minLength = constraints.length.min;
-        this.maxLength = constraints.length.max;
-        this.minMinNumber = constraints.minNumber.min;
-        this.maxMinNumber = constraints.minNumber.max;
-        this.minMinSpecial = constraints.minSpecial.min;
-        this.maxMinSpecial = constraints.minSpecial.max;
-
         this.policyInEffect = constraints.policyInEffect;
 
         const toggles = [
@@ -201,9 +193,6 @@ export class PasswordSettingsComponent implements OnInit, OnDestroy {
     // now that outputs are set up, connect inputs
     this.settings.valueChanges
       .pipe(
-        // debounce ensures rapid edits to a field, such as partial edits to a
-        // spinbox or rapid button clicks don't emit spurious generator updates
-        debounceTime(this.waitMs),
         map((settings) => {
           // interface is "avoid" while storage is "include"
           const s: any = { ...settings };
@@ -216,24 +205,6 @@ export class PasswordSettingsComponent implements OnInit, OnDestroy {
       .subscribe(settings);
   }
 
-  /** attribute binding for length[min] */
-  protected minLength: number;
-
-  /** attribute binding for length[max] */
-  protected maxLength: number;
-
-  /** attribute binding for minNumber[min] */
-  protected minMinNumber: number;
-
-  /** attribute binding for minNumber[max] */
-  protected maxMinNumber: number;
-
-  /** attribute binding for minSpecial[min] */
-  protected minMinSpecial: number;
-
-  /** attribute binding for minSpecial[max] */
-  protected maxMinSpecial: number;
-
   /** display binding for enterprise policy notice */
   protected policyInEffect: boolean;
 

diff --git a/libs/tools/generator/components/src/username-generator.component.html b/libs/tools/generator/components/src/username-generator.component.html
@@ -7,17 +7,21 @@
       type="button"
       bitIconButton="bwi-generate"
       buttonType="main"
-      (click)="generate$.next()"
+      (click)="generate('user request')"
       [appA11yTitle]="credentialTypeGenerateLabel$ | async"
-    ></button>
+    >
+      {{ credentialTypeGenerateLabel$ | async }}
+    </button>
     <button
       type="button"
       bitIconButton="bwi-clone"
       buttonType="main"
       showToast
       [appA11yTitle]="credentialTypeCopyLabel$ | async"
       [appCopyClick]="value$ | async"
-    ></button>
+    >
+      {{ credentialTypeCopyLabel$ | async }}
+    </button>
   </div>
 </bit-card>
 <bit-section [disableMargin]="disableMargin">
@@ -44,7 +48,7 @@ <h2 bitTypography="h6">{{ "options" | i18n }}</h2>
       <tools-catchall-settings
         *ngIf="(algorithm$ | async)?.id === 'catchall'"
         [userId]="this.userId$ | async"
-        (onUpdated)="generate$.next()"
+        (onUpdated)="generate('catchall settings')"
       />
       <tools-forwarder-settings
         *ngIf="!!(forwarderId$ | async)"
@@ -54,12 +58,12 @@ <h2 bitTypography="h6">{{ "options" | i18n }}</h2>
       <tools-subaddress-settings
         *ngIf="(algorithm$ | async)?.id === 'subaddress'"
         [userId]="this.userId$ | async"
-        (onUpdated)="generate$.next()"
+        (onUpdated)="generate('subaddress settings')"
       />
       <tools-username-settings
         *ngIf="(algorithm$ | async)?.id === 'username'"
         [userId]="this.userId$ | async"
-        (onUpdated)="generate$.next()"
+        (onUpdated)="generate('username settings')"
       />
     </bit-card>
   </div>

diff --git a/libs/tools/generator/components/src/username-generator.component.ts b/libs/tools/generator/components/src/username-generator.component.ts
@@ -313,7 +313,7 @@ export class UsernameGeneratorComponent implements OnInit, OnDestroy {
         if (!a || a.onlyOnRequest) {
           this.value$.next("-");
         } else {
-          this.generate$.next();
+          this.generate("autogenerate");
         }
       });
     });
@@ -391,7 +391,15 @@ export class UsernameGeneratorComponent implements OnInit, OnDestroy {
   protected readonly userId$ = new BehaviorSubject<UserId>(null);
 
   /** Emits when a new credential is requested */
-  protected readonly generate$ = new Subject<void>();
+  private readonly generate$ = new Subject<string>();
+
+  /** Request a new value from the generator
+   * @param requestor a label used to trace generation request
+   *  origin in the debugger.
+   */
+  protected generate(requestor: string) {
+    this.generate$.next(requestor);
+  }
 
   private toOptions(algorithms: AlgorithmInfo[]) {
     const options: Option<string>[] = algorithms.map((algorithm) => ({
-Original file line number
+Diff line change
@@ Expand Up @@
                 control.clearValidators();
               }
             }
+            this.settings.updateValueAndValidity({ emitEvent: false });
           });
         // the first emission is the current value; subsequent emissions are updates
@@ Expand Down @@