From 6b25515d1df6402cabeb04c175c791f0316bf19f Mon Sep 17 00:00:00 2001 From: jaasen-livefront Date: Mon, 18 Nov 2024 15:23:40 -0800 Subject: [PATCH] fix getReusedPasswords --- .../risk-insights/services/ciphers.mock.ts | 7 +- .../member-cipher-details-api.service.spec.ts | 5 +- .../services/password-health.service.spec.ts | 65 ++++++--------- .../services/password-health.service.ts | 81 +++++++++---------- 4 files changed, 70 insertions(+), 88 deletions(-) diff --git a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/ciphers.mock.ts b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/ciphers.mock.ts index 7538d7c92b9..23d5d39ab96 100644 --- a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/ciphers.mock.ts +++ b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/ciphers.mock.ts @@ -91,7 +91,7 @@ export const mockCiphers: any[] = [ id: "cbea34a8-bde4-46ad-9d19-b05001228xy4", organizationId: null, folderId: null, - name: "Weak password Cipher", + name: "Strong password Cipher", notes: null, type: 1, favorite: false, @@ -115,14 +115,15 @@ export const mockCiphers: any[] = [ id: "cbea34a8-bde4-46ad-9d19-b05001227nm5", organizationId: null, folderId: null, - name: "No password Cipher", + name: "Exposed password Cipher", notes: null, type: 1, favorite: false, organizationUseTotp: false, login: { hasUris: true, - uris: [createLoginUriView("123formbuilder.com")], + password: "123", + uris: [createLoginUriView("123formbuilder.com"), createLoginUriView("www.google.com")], }, edit: true, viewPassword: true, diff --git a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/member-cipher-details-api.service.spec.ts b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/member-cipher-details-api.service.spec.ts index 6ea49551239..10408ad8758 100644 --- a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/member-cipher-details-api.service.spec.ts +++ b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/member-cipher-details-api.service.spec.ts @@ -13,7 +13,6 @@ export const mockMemberCipherDetails: any = [ "cbea34a8-bde4-46ad-9d19-b05001228ab1", "cbea34a8-bde4-46ad-9d19-b05001228ab2", "cbea34a8-bde4-46ad-9d19-b05001228xy4", - "cbea34a8-bde4-46ad-9d19-b05001227nm5", ], }, { @@ -34,7 +33,6 @@ export const mockMemberCipherDetails: any = [ cipherIds: [ "cbea34a8-bde4-46ad-9d19-b05001228cd3", "cbea34a8-bde4-46ad-9d19-b05001228xy4", - "cbea34a8-bde4-46ad-9d19-b05001227nm5", "cbea34a8-bde4-46ad-9d19-b05001227nm7", ], }, @@ -56,14 +54,13 @@ export const mockMemberCipherDetails: any = [ "cbea34a8-bde4-46ad-9d19-b05001228ab1", "cbea34a8-bde4-46ad-9d19-b05001228cd3", "cbea34a8-bde4-46ad-9d19-b05001228xy4", - "cbea34a8-bde4-46ad-9d19-b05001227nm5", ], }, { userName: "Chris Finch", email: "chris.finch@wernhamhogg.uk", usesKeyConnector: true, - cipherIds: ["cbea34a8-bde4-46ad-9d19-b05001227nm5"], + cipherIds: ["cbea34a8-bde4-46ad-9d19-b05001228ab2"], }, ]; diff --git a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/password-health.service.spec.ts b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/password-health.service.spec.ts index 6d3f8bdab04..4d02431fd2d 100644 --- a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/password-health.service.spec.ts +++ b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/password-health.service.spec.ts @@ -61,28 +61,28 @@ describe("PasswordHealthService", () => { atRiskPasswords: 1, totalPasswords: 2, atRiskMembers: 2, - totalMembers: 4, - }, - { - application: "123formbuilder.com", - atRiskPasswords: 0, - totalPasswords: 1, - atRiskMembers: 0, totalMembers: 5, }, { - application: "example.com", + application: "123formbuilder.com", atRiskPasswords: 1, + totalPasswords: 1, + atRiskMembers: 1, + totalMembers: 1, + }, + { + application: "example.com", + atRiskPasswords: 2, totalPasswords: 2, atRiskMembers: 5, totalMembers: 5, }, { application: "google.com", - atRiskPasswords: 1, - totalPasswords: 1, - atRiskMembers: 2, - totalMembers: 2, + atRiskPasswords: 2, + totalPasswords: 2, + atRiskMembers: 3, + totalMembers: 3, }, ]; @@ -91,10 +91,22 @@ describe("PasswordHealthService", () => { expect(result.details.sort(sortFn)).toEqual(expected.sort(sortFn)); expect(result.totalAtRiskMembers).toBe(5); expect(result.totalMembers).toBe(6); - expect(result.totalAtRiskApps).toBe(3); + expect(result.totalAtRiskApps).toBe(4); expect(result.totalApps).toBe(4); }); + describe("getReusedPasswords", () => { + it.only("should return an array of reused passwords", () => { + const ciphers = [ + { login: { password: "123" }, type: CipherType.Login, viewPassword: true }, + { login: { password: "123" }, type: CipherType.Login, viewPassword: true }, + { login: { password: "abc" }, type: CipherType.Login, viewPassword: true }, + ] as CipherView[]; + const result = service.getReusedPasswords(ciphers); + expect(result).toEqual(new Set(["123"])); + }); + }); + describe("isWeakPassword", () => { it("should return true for a weak password", () => { const cipher = new CipherView(); @@ -114,31 +126,4 @@ describe("PasswordHealthService", () => { expect(service.isWeakPassword(cipher)).toBe(false); }); }); - - describe("isReusedPassword", () => { - it("should return false for a new password", () => { - const cipher = new CipherView(); - cipher.type = CipherType.Login; - cipher.login = { password: "uniquePassword", username: "user" } as LoginView; - cipher.viewPassword = true; - - expect(service.isReusedPassword(cipher)).toBe(false); - }); - - it("should return true for a reused password", () => { - const cipher1 = new CipherView(); - cipher1.type = CipherType.Login; - cipher1.login = { password: "reusedPassword", username: "user" } as LoginView; - cipher1.viewPassword = true; - - const cipher2 = new CipherView(); - cipher2.type = CipherType.Login; - cipher2.login = { password: "reusedPassword", username: "user" } as LoginView; - cipher2.viewPassword = true; - - service.isReusedPassword(cipher1); // Adds 'reusedPassword' to usedPasswords - - expect(service.isReusedPassword(cipher2)).toBe(true); - }); - }); }); diff --git a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/password-health.service.ts b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/password-health.service.ts index ddfe63737ad..eef7e851bf8 100644 --- a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/password-health.service.ts +++ b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/password-health.service.ts @@ -69,10 +69,12 @@ export class PasswordHealthService { // Set to store at-risk cipher IDs const atRiskCipherIds = new Set(); + const reusedPasswords = this.getReusedPasswords(ciphers); + // Determine at-risk ciphers for (const cipher of ciphers) { + const isReused = reusedPasswords.has(cipher.login.password); const isWeak = this.isWeakPassword(cipher); - const isReused = this.isReusedPassword(cipher); const isExposed = await this.isExposedPassword(cipher); if (isWeak || isReused || isExposed) { atRiskCipherIds.add(cipher.id); @@ -154,54 +156,51 @@ export class PasswordHealthService { }; } - async isExposedPassword(cipher: CipherView) { - const { type, login, isDeleted, viewPassword } = cipher; - if ( - type !== CipherType.Login || - login.password == null || - login.password === "" || - isDeleted || - !viewPassword - ) { - return; + getReusedPasswords(ciphers: CipherView[]): Set { + const seenPasswords = new Set(); + const reusedPasswords = new Set(); + + for (const cipher of ciphers) { + if (this.isValidPassword(cipher)) { + const password = cipher.login.password; + if (seenPasswords.has(password)) { + reusedPasswords.add(password); + } else { + seenPasswords.add(password); + } + } } - const exposedCount = await this.auditService.passwordLeaked(login.password); + return reusedPasswords; + } + + isValidPassword(cipher: CipherView) { + const { type, login, isDeleted, viewPassword } = cipher; + return ( + type === CipherType.Login && + login.password != null && + login.password !== "" && + !isDeleted && + viewPassword + ); + } + + async isExposedPassword(cipher: CipherView) { + if (!this.isValidPassword(cipher)) { + return false; + } + + const exposedCount = await this.auditService.passwordLeaked(cipher.login.password); return exposedCount > 0; } - isReusedPassword(cipher: CipherView) { - const { type, login, isDeleted, viewPassword } = cipher; - if ( - type !== CipherType.Login || - login.password == null || - login.password === "" || - isDeleted || - !viewPassword - ) { - return; - } - - if (this.usedPasswords.includes(login.password)) { - return true; - } - - this.usedPasswords.push(login.password); - return false; - } - isWeakPassword(cipher: CipherView) { - const { type, login, isDeleted, viewPassword } = cipher; - if ( - type !== CipherType.Login || - login.password == null || - login.password === "" || - isDeleted || - !viewPassword - ) { - return; + if (!this.isValidPassword(cipher)) { + return false; } + const { login } = cipher; + const hasUserName = !Utils.isNullOrWhitespace(cipher.login.username); let userInput: string[] = []; if (hasUserName) {