[PM-12066] Add sorting to weak password report (#11027)

* Simplify the filter(toggle group) to filter by organizationId instead of a orgFilterStatus property which is not present on the CipherView * Add sorting to weak password report table - Create new type to represent a row within the report - Add types and remove usage of any - Include the score/badge within the data passed to the datasource/table instead of looking it up via the `passwordStrengthMap` - Remove unneeded passwordStrengthCache - Enable sorting via bitSortable - Set default sort to order by weakness * Show headers and sort also within AC version of weak-password report, but hide the Owner column * Clarify that we are filtering by OrgId * Use a typed object for the reportValue instead of an array --------- Co-authored-by: Daniel James Smith <[email protected]>
bitwarden · Sep 26, 2024 · 9eeaf0a · 9eeaf0a
1 parent eb7eb61
commit 9eeaf0a
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 77 deletions.
diff --git a/apps/web/src/app/tools/reports/pages/cipher-report.component.ts b/apps/web/src/app/tools/reports/pages/cipher-report.component.ts
@@ -5,6 +5,7 @@ import { ModalService } from "@bitwarden/angular/services/modal.service";
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { OrganizationId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction";
 import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type";
@@ -81,30 +82,28 @@ export class CipherReportComponent implements OnDestroy {
     if (filterId === 0) {
       cipherCount = this.allCiphers.length;
     } else if (filterId === 1) {
-      cipherCount = this.allCiphers.filter((c: any) => c.orgFilterStatus === null).length;
+      cipherCount = this.allCiphers.filter((c) => c.organizationId === null).length;
     } else {
       this.organizations.filter((org: Organization) => {
         if (org.id === filterId) {
           orgFilterStatus = org.id;
           return org;
         }
       });
-      cipherCount = this.allCiphers.filter(
-        (c: any) => c.orgFilterStatus === orgFilterStatus,
-      ).length;
+      cipherCount = this.allCiphers.filter((c) => c.organizationId === orgFilterStatus).length;
     }
     return cipherCount;
   }
 
   async filterOrgToggle(status: any) {
-    this.currentFilterStatus = status;
-    if (status === 0) {
-      this.dataSource.filter = null;
-    } else if (status === 1) {
-      this.dataSource.filter = (c: any) => c.orgFilterStatus == null;
-    } else {
-      this.dataSource.filter = (c: any) => c.orgFilterStatus === status;
+    let filter = null;
+    if (typeof status === "number" && status === 1) {
+      filter = (c: CipherView) => c.organizationId == null;
+    } else if (typeof status === "string") {
+      const orgId = status as OrganizationId;
+      filter = (c: CipherView) => c.organizationId === orgId;
     }
+    this.dataSource.filter = filter;
   }
 
   async load() {
@@ -183,17 +182,14 @@ export class CipherReportComponent implements OnDestroy {
   protected filterCiphersByOrg(ciphersList: CipherView[]) {
     this.allCiphers = [...ciphersList];
 
-    this.ciphers = ciphersList.map((ciph: any) => {
-      ciph.orgFilterStatus = ciph.organizationId;
-
+    this.ciphers = ciphersList.map((ciph) => {
       if (this.filterStatus.indexOf(ciph.organizationId) === -1 && ciph.organizationId != null) {
         this.filterStatus.push(ciph.organizationId);
       } else if (this.filterStatus.indexOf(1) === -1 && ciph.organizationId == null) {
         this.filterStatus.splice(1, 0, 1);
       }
       return ciph;
     });
-
     this.dataSource.data = this.ciphers;
 
     if (this.filterStatus.length > 2) {

diff --git a/apps/web/src/app/tools/reports/pages/weak-passwords-report.component.html b/apps/web/src/app/tools/reports/pages/weak-passwords-report.component.html
@@ -32,12 +32,14 @@
         </ng-container>
       </bit-toggle-group>
       <bit-table [dataSource]="dataSource">
-        <ng-container header *ngIf="!isAdminConsoleActive">
+        <ng-container header>
           <tr bitRow>
             <th bitCell></th>
-            <th bitCell>{{ "name" | i18n }}</th>
-            <th bitCell>{{ "owner" | i18n }}</th>
-            <th bitCell></th>
+            <th bitCell bitSortable="name">{{ "name" | i18n }}</th>
+            <th bitCell bitSortable="organizationId" *ngIf="!isAdminConsoleActive">
+              {{ "owner" | i18n }}
+            </th>
+            <th bitCell class="tw-text-right" bitSortable="reportValue" default></th>
           </tr>
         </ng-container>
         <ng-template body let-rows$>
@@ -80,7 +82,7 @@
               <br />
               <small>{{ r.subTitle }}</small>
             </td>
-            <td bitCell>
+            <td bitCell *ngIf="!isAdminConsoleActive">
               <app-org-badge
                 *ngIf="!organization"
                 [disabled]="disabled"
@@ -91,8 +93,8 @@
               </app-org-badge>
             </td>
             <td bitCell class="tw-text-right">
-              <span bitBadge [variant]="passwordStrengthMap.get(r.id)[1]">
-                {{ passwordStrengthMap.get(r.id)[0] | i18n }}
+              <span bitBadge [variant]="r.reportValue.badgeVariant">
+                {{ r.reportValue.label | i18n }}
               </span>
             </td>
           </tr>

diff --git a/apps/web/src/app/tools/reports/pages/weak-passwords-report.component.ts b/apps/web/src/app/tools/reports/pages/weak-passwords-report.component.ts
@@ -14,16 +14,17 @@ import { PasswordRepromptService } from "@bitwarden/vault";
 
 import { CipherReportComponent } from "./cipher-report.component";
 
+type ReportScore = { label: string; badgeVariant: BadgeVariant };
+type ReportResult = CipherView & { reportValue: ReportScore };
+
 @Component({
   selector: "app-weak-passwords-report",
   templateUrl: "weak-passwords-report.component.html",
 })
 export class WeakPasswordsReportComponent extends CipherReportComponent implements OnInit {
-  passwordStrengthMap = new Map<string, [string, BadgeVariant]>();
   disabled = true;
 
-  private passwordStrengthCache = new Map<string, number>();
-  weakPasswordCiphers: CipherView[] = [];
+  weakPasswordCiphers: ReportResult[] = [];
 
   constructor(
     protected cipherService: CipherService,
@@ -49,16 +50,15 @@ export class WeakPasswordsReportComponent extends CipherReportComponent implemen
   }
 
   async setCiphers() {
-    const allCiphers: any = await this.getAllCiphers();
-    this.passwordStrengthCache = new Map<string, number>();
+    const allCiphers = await this.getAllCiphers();
     this.weakPasswordCiphers = [];
     this.filterStatus = [0];
     this.findWeakPasswords(allCiphers);
   }
 
-  protected findWeakPasswords(ciphers: any[]): void {
+  protected findWeakPasswords(ciphers: CipherView[]): void {
     ciphers.forEach((ciph) => {
-      const { type, login, isDeleted, edit, viewPassword, id } = ciph;
+      const { type, login, isDeleted, edit, viewPassword } = ciph;
       if (
         type !== CipherType.Login ||
         login.password == null ||
@@ -71,50 +71,39 @@ export class WeakPasswordsReportComponent extends CipherReportComponent implemen
       }
 
       const hasUserName = this.isUserNameNotEmpty(ciph);
-      const cacheKey = this.getCacheKey(ciph);
-      if (!this.passwordStrengthCache.has(cacheKey)) {
-        let userInput: string[] = [];
-        if (hasUserName) {
-          const atPosition = login.username.indexOf("@");
-          if (atPosition > -1) {
-            userInput = userInput
-              .concat(
-                login.username
-                  .substr(0, atPosition)
-                  .trim()
-                  .toLowerCase()
-                  .split(/[^A-Za-z0-9]/),
-              )
-              .filter((i) => i.length >= 3);
-          } else {
-            userInput = login.username
-              .trim()
-              .toLowerCase()
-              .split(/[^A-Za-z0-9]/)
-              .filter((i: any) => i.length >= 3);
-          }
+      let userInput: string[] = [];
+      if (hasUserName) {
+        const atPosition = login.username.indexOf("@");
+        if (atPosition > -1) {
+          userInput = userInput
+            .concat(
+              login.username
+                .substr(0, atPosition)
+                .trim()
+                .toLowerCase()
+                .split(/[^A-Za-z0-9]/),
+            )
+            .filter((i) => i.length >= 3);
+        } else {
+          userInput = login.username
+            .trim()
+            .toLowerCase()
+            .split(/[^A-Za-z0-9]/)
+            .filter((i) => i.length >= 3);
         }
-        const result = this.passwordStrengthService.getPasswordStrength(
-          login.password,
-          null,
-          userInput.length > 0 ? userInput : null,
-        );
-        this.passwordStrengthCache.set(cacheKey, result.score);
       }
-      const score = this.passwordStrengthCache.get(cacheKey);
+      const result = this.passwordStrengthService.getPasswordStrength(
+        login.password,
+        null,
+        userInput.length > 0 ? userInput : null,
+      );
 
-      if (score != null && score <= 2) {
-        this.passwordStrengthMap.set(id, this.scoreKey(score));
-        this.weakPasswordCiphers.push(ciph);
+      if (result.score != null && result.score <= 2) {
+        const scoreValue = this.scoreKey(result.score);
+        const row = { ...ciph, reportValue: scoreValue } as ReportResult;
+        this.weakPasswordCiphers.push(row);
       }
     });
-    this.weakPasswordCiphers.sort((a, b) => {
-      return (
-        this.passwordStrengthCache.get(this.getCacheKey(a)) -
-        this.passwordStrengthCache.get(this.getCacheKey(b))
-      );
-    });
-
     this.filterCiphersByOrg(this.weakPasswordCiphers);
   }
 
@@ -127,20 +116,16 @@ export class WeakPasswordsReportComponent extends CipherReportComponent implemen
     return !Utils.isNullOrWhitespace(c.login.username);
   }
 
-  private getCacheKey(c: CipherView): string {
-    return c.login.password + "_____" + (this.isUserNameNotEmpty(c) ? c.login.username : "");
-  }
-
-  private scoreKey(score: number): [string, BadgeVariant] {
+  private scoreKey(score: number): ReportScore {
     switch (score) {
       case 4:
-        return ["strong", "success"];
+        return { label: "strong", badgeVariant: "success" };
       case 3:
-        return ["good", "primary"];
+        return { label: "good", badgeVariant: "primary" };
       case 2:
-        return ["weak", "warning"];
+        return { label: "weak", badgeVariant: "warning" };
       default:
-        return ["veryWeak", "danger"];
+        return { label: "veryWeak", badgeVariant: "danger" };
     }
   }
 }