From bc14512a02482514366612d9038915e765eec8e0 Mon Sep 17 00:00:00 2001 From: Alex <55413326+AlexRubik@users.noreply.github.com> Date: Tue, 24 Feb 2026 09:45:28 -0800 Subject: [PATCH] Revert "[PM-31803] Fix Password Manager reports not displaying items with limited collection access (#18956)" (#19153) This reverts commit 1ef8f257b0120fbef5f22e1828e2facea9a9913e. --- ...exposed-passwords-report.component.spec.ts | 15 +++++++----- .../exposed-passwords-report.component.ts | 6 +++-- ...active-two-factor-report.component.spec.ts | 23 +++++++++++-------- .../inactive-two-factor-report.component.ts | 6 +++-- .../reused-passwords-report.component.spec.ts | 14 ++++++----- .../reused-passwords-report.component.ts | 6 +++-- ...nsecured-websites-report.component.spec.ts | 11 +++++---- .../unsecured-websites-report.component.ts | 7 +++++- .../weak-passwords-report.component.spec.ts | 15 +++++++----- .../pages/weak-passwords-report.component.ts | 11 +++++++-- 10 files changed, 73 insertions(+), 41 deletions(-) diff --git a/apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.spec.ts b/apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.spec.ts index 81e4a78b491..e056ec44af5 100644 --- a/apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.spec.ts +++ b/apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.spec.ts @@ -122,16 +122,19 @@ describe("ExposedPasswordsReportComponent", () => { expect(component).toBeTruthy(); }); - it("should get ciphers with exposed passwords regardless of edit access", async () => { + it('should get only ciphers with exposed passwords that the user has "Can Edit" access to', async () => { + const expectedIdOne: any = "cbea34a8-bde4-46ad-9d19-b05001228ab2"; + const expectedIdTwo = "cbea34a8-bde4-46ad-9d19-b05001228cd3"; + jest.spyOn(auditService, "passwordLeaked").mockReturnValue(Promise.resolve(1234)); jest.spyOn(component as any, "getAllCiphers").mockReturnValue(Promise.resolve(cipherData)); await component.setCiphers(); - const cipherIds = component.ciphers.map((c) => c.id); - expect(cipherIds).toContain("cbea34a8-bde4-46ad-9d19-b05001228ab1"); - expect(cipherIds).toContain("cbea34a8-bde4-46ad-9d19-b05001228ab2"); - expect(cipherIds).toContain("cbea34a8-bde4-46ad-9d19-b05001228cd3"); - expect(component.ciphers.length).toEqual(3); + expect(component.ciphers.length).toEqual(2); + expect(component.ciphers[0].id).toEqual(expectedIdOne); + expect(component.ciphers[0].edit).toEqual(true); + expect(component.ciphers[1].id).toEqual(expectedIdTwo); + expect(component.ciphers[1].edit).toEqual(true); }); it("should call fullSync method of syncService", () => { diff --git a/apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.ts b/apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.ts index e39ef811d66..51bdde3eda8 100644 --- a/apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.ts +++ b/apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.ts @@ -64,12 +64,14 @@ export class ExposedPasswordsReportComponent extends CipherReportComponent imple this.filterStatus = [0]; allCiphers.forEach((ciph) => { - const { type, login, isDeleted } = ciph; + const { type, login, isDeleted, edit, viewPassword } = ciph; if ( type !== CipherType.Login || login.password == null || login.password === "" || - isDeleted + isDeleted || + (!this.organization && !edit) || + !viewPassword ) { return; } diff --git a/apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.spec.ts b/apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.spec.ts index 07a772755f5..12453ea3b88 100644 --- a/apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.spec.ts +++ b/apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.spec.ts @@ -95,7 +95,9 @@ describe("InactiveTwoFactorReportComponent", () => { expect(component).toBeTruthy(); }); - it("should get ciphers with domains in the 2fa directory regardless of edit access", async () => { + it('should get only ciphers with domains in the 2fa directory that they have "Can Edit" access to', async () => { + const expectedIdOne: any = "cbea34a8-bde4-46ad-9d19-b05001228xy4"; + const expectedIdTwo: any = "cbea34a8-bde4-46ad-9d19-b05001227nm5"; component.services.set( "101domain.com", "https://help.101domain.com/account-management/account-security/enabling-disabling-two-factor-verification", @@ -108,10 +110,11 @@ describe("InactiveTwoFactorReportComponent", () => { jest.spyOn(component as any, "getAllCiphers").mockReturnValue(Promise.resolve(cipherData)); await component.setCiphers(); - const cipherIds = component.ciphers.map((c) => c.id); - expect(cipherIds).toContain("cbea34a8-bde4-46ad-9d19-b05001228xy4"); - expect(cipherIds).toContain("cbea34a8-bde4-46ad-9d19-b05001227nm5"); expect(component.ciphers.length).toEqual(2); + expect(component.ciphers[0].id).toEqual(expectedIdOne); + expect(component.ciphers[0].edit).toEqual(true); + expect(component.ciphers[1].id).toEqual(expectedIdTwo); + expect(component.ciphers[1].edit).toEqual(true); }); it("should call fullSync method of syncService", () => { @@ -194,7 +197,7 @@ describe("InactiveTwoFactorReportComponent", () => { expect(doc).toBe(""); }); - it("should return true for cipher without edit access", () => { + it("should return false if cipher does not have edit access and no organization", () => { component.organization = null; const cipher = createCipherView({ edit: false, @@ -203,11 +206,11 @@ describe("InactiveTwoFactorReportComponent", () => { }, }); const [doc, isInactive] = (component as any).isInactive2faCipher(cipher); - expect(isInactive).toBe(true); - expect(doc).toBe("https://example.com/2fa-doc"); + expect(isInactive).toBe(false); + expect(doc).toBe(""); }); - it("should return true for cipher without viewPassword", () => { + it("should return false if cipher does not have viewPassword", () => { const cipher = createCipherView({ viewPassword: false, login: { @@ -215,8 +218,8 @@ describe("InactiveTwoFactorReportComponent", () => { }, }); const [doc, isInactive] = (component as any).isInactive2faCipher(cipher); - expect(isInactive).toBe(true); - expect(doc).toBe("https://example.com/2fa-doc"); + expect(isInactive).toBe(false); + expect(doc).toBe(""); }); it("should check all uris and return true if any matches domain or host", () => { diff --git a/apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.ts b/apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.ts index cd892130518..9d7de688f3e 100644 --- a/apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.ts +++ b/apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.ts @@ -92,12 +92,14 @@ export class InactiveTwoFactorReportComponent extends CipherReportComponent impl let docFor2fa: string = ""; let isInactive2faCipher: boolean = false; - const { type, login, isDeleted } = cipher; + const { type, login, isDeleted, edit, viewPassword } = cipher; if ( type !== CipherType.Login || (login.totp != null && login.totp !== "") || !login.hasUris || - isDeleted + isDeleted || + (!this.organization && !edit) || + !viewPassword ) { return [docFor2fa, isInactive2faCipher]; } diff --git a/apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.spec.ts b/apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.spec.ts index 8f08d06e27b..1b7006d0c68 100644 --- a/apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.spec.ts +++ b/apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.spec.ts @@ -109,15 +109,17 @@ describe("ReusedPasswordsReportComponent", () => { expect(component).toBeTruthy(); }); - it("should get ciphers with reused passwords regardless of edit access", async () => { + it('should get ciphers with reused passwords that the user has "Can Edit" access to', async () => { + const expectedIdOne: any = "cbea34a8-bde4-46ad-9d19-b05001228ab2"; + const expectedIdTwo = "cbea34a8-bde4-46ad-9d19-b05001228cd3"; jest.spyOn(component as any, "getAllCiphers").mockReturnValue(Promise.resolve(cipherData)); await component.setCiphers(); - const cipherIds = component.ciphers.map((c) => c.id); - expect(cipherIds).toContain("cbea34a8-bde4-46ad-9d19-b05001228ab1"); - expect(cipherIds).toContain("cbea34a8-bde4-46ad-9d19-b05001228ab2"); - expect(cipherIds).toContain("cbea34a8-bde4-46ad-9d19-b05001228cd3"); - expect(component.ciphers.length).toEqual(3); + expect(component.ciphers.length).toEqual(2); + expect(component.ciphers[0].id).toEqual(expectedIdOne); + expect(component.ciphers[0].edit).toEqual(true); + expect(component.ciphers[1].id).toEqual(expectedIdTwo); + expect(component.ciphers[1].edit).toEqual(true); }); it("should call fullSync method of syncService", () => { diff --git a/apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.ts b/apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.ts index 7d24e61f276..0a81b19d4ff 100644 --- a/apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.ts +++ b/apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.ts @@ -71,12 +71,14 @@ export class ReusedPasswordsReportComponent extends CipherReportComponent implem this.filterStatus = [0]; ciphers.forEach((ciph) => { - const { type, login, isDeleted } = ciph; + const { type, login, isDeleted, edit, viewPassword } = ciph; if ( type !== CipherType.Login || login.password == null || login.password === "" || - isDeleted + isDeleted || + (!this.organization && !edit) || + !viewPassword ) { return; } diff --git a/apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.spec.ts b/apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.spec.ts index f116faf114f..2107e0c8df7 100644 --- a/apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.spec.ts +++ b/apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.spec.ts @@ -118,14 +118,17 @@ describe("UnsecuredWebsitesReportComponent", () => { expect(component).toBeTruthy(); }); - it("should get unsecured ciphers regardless of edit access", async () => { + it('should get only unsecured ciphers that the user has "Can Edit" access to', async () => { + const expectedIdOne: any = "cbea34a8-bde4-46ad-9d19-b05001228ab2"; + const expectedIdTwo = "cbea34a8-bde4-46ad-9d19-b05001228cd3"; jest.spyOn(component as any, "getAllCiphers").mockReturnValue(Promise.resolve(cipherData)); await component.setCiphers(); - const cipherIds = component.ciphers.map((c) => c.id); - expect(cipherIds).toContain("cbea34a8-bde4-46ad-9d19-b05001228ab2"); - expect(cipherIds).toContain("cbea34a8-bde4-46ad-9d19-b05001228cd3"); expect(component.ciphers.length).toEqual(2); + expect(component.ciphers[0].id).toEqual(expectedIdOne); + expect(component.ciphers[0].edit).toEqual(true); + expect(component.ciphers[1].id).toEqual(expectedIdTwo); + expect(component.ciphers[1].edit).toEqual(true); }); it("should call fullSync method of syncService", () => { diff --git a/apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.ts b/apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.ts index 8399395d273..4a2c0677574 100644 --- a/apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.ts +++ b/apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.ts @@ -71,7 +71,12 @@ export class UnsecuredWebsitesReportComponent extends CipherReportComponent impl * @param cipher Current cipher with unsecured uri */ private cipherContainsUnsecured(cipher: CipherView): boolean { - if (cipher.type !== CipherType.Login || !cipher.login.hasUris || cipher.isDeleted) { + if ( + cipher.type !== CipherType.Login || + !cipher.login.hasUris || + cipher.isDeleted || + (!this.organization && !cipher.edit) + ) { return false; } diff --git a/apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.spec.ts b/apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.spec.ts index f9aca0aa378..a63723dc688 100644 --- a/apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.spec.ts +++ b/apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.spec.ts @@ -114,7 +114,10 @@ describe("WeakPasswordsReportComponent", () => { expect(component).toBeTruthy(); }); - it("should get ciphers with weak passwords regardless of edit access", async () => { + it('should get only ciphers with weak passwords that the user has "Can Edit" access to', async () => { + const expectedIdOne: any = "cbea34a8-bde4-46ad-9d19-b05001228ab2"; + const expectedIdTwo = "cbea34a8-bde4-46ad-9d19-b05001228cd3"; + jest.spyOn(passwordStrengthService, "getPasswordStrength").mockReturnValue({ password: "123", score: 0, @@ -122,11 +125,11 @@ describe("WeakPasswordsReportComponent", () => { jest.spyOn(component as any, "getAllCiphers").mockReturnValue(Promise.resolve(cipherData)); await component.setCiphers(); - const cipherIds = component.ciphers.map((c) => c.id); - expect(cipherIds).toContain("cbea34a8-bde4-46ad-9d19-b05001228ab1"); - expect(cipherIds).toContain("cbea34a8-bde4-46ad-9d19-b05001228ab2"); - expect(cipherIds).toContain("cbea34a8-bde4-46ad-9d19-b05001228cd3"); - expect(component.ciphers.length).toEqual(3); + expect(component.ciphers.length).toEqual(2); + expect(component.ciphers[0].id).toEqual(expectedIdOne); + expect(component.ciphers[0].edit).toEqual(true); + expect(component.ciphers[1].id).toEqual(expectedIdTwo); + expect(component.ciphers[1].edit).toEqual(true); }); it("should call fullSync method of syncService", () => { diff --git a/apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.ts b/apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.ts index 6cde01f2d92..bb5400346fd 100644 --- a/apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.ts +++ b/apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.ts @@ -103,8 +103,15 @@ export class WeakPasswordsReportComponent extends CipherReportComponent implemen } protected determineWeakPasswordScore(ciph: CipherView): ReportResult | null { - const { type, login, isDeleted } = ciph; - if (type !== CipherType.Login || login.password == null || login.password === "" || isDeleted) { + const { type, login, isDeleted, edit, viewPassword } = ciph; + if ( + type !== CipherType.Login || + login.password == null || + login.password === "" || + isDeleted || + (!this.organization && !edit) || + !viewPassword + ) { return; }