From 9eeaf0a61f534dc404cfc10704b80a018f0ef391 Mon Sep 17 00:00:00 2001 From: Daniel James Smith <2670567+djsmith85@users.noreply.github.com> Date: Fri, 27 Sep 2024 01:38:18 +0200 Subject: [PATCH] [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 --- .../reports/pages/cipher-report.component.ts | 26 +++-- .../weak-passwords-report.component.html | 16 +-- .../pages/weak-passwords-report.component.ts | 97 ++++++++----------- 3 files changed, 61 insertions(+), 78 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 index 7d4309b7bc..cfefbbe4d7 100644 --- 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,7 +82,7 @@ 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) { @@ -89,22 +90,20 @@ export class CipherReportComponent implements OnDestroy { 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,9 +182,7 @@ 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) { @@ -193,7 +190,6 @@ export class CipherReportComponent implements OnDestroy { } 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 index 0ebecab9f1..e5e5245575 100644 --- 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 @@ - + - {{ "name" | i18n }} - {{ "owner" | i18n }} - + {{ "name" | i18n }} + + {{ "owner" | i18n }} + + @@ -80,7 +82,7 @@
{{ r.subTitle }} - + - - {{ passwordStrengthMap.get(r.id)[0] | i18n }} + + {{ r.reportValue.label | i18n }} 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 index f33e0626ab..7782e96e97 100644 --- 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(); disabled = true; - private passwordStrengthCache = new Map(); - 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(); + 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); - - if (score != null && score <= 2) { - this.passwordStrengthMap.set(id, this.scoreKey(score)); - this.weakPasswordCiphers.push(ciph); - } - }); - this.weakPasswordCiphers.sort((a, b) => { - return ( - this.passwordStrengthCache.get(this.getCacheKey(a)) - - this.passwordStrengthCache.get(this.getCacheKey(b)) + const result = this.passwordStrengthService.getPasswordStrength( + login.password, + null, + userInput.length > 0 ? userInput : null, ); - }); + 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.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" }; } } }