bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2026-02-20 11:24:07 +00:00
Code Issues Releases Wiki Activity

[PM-31803] Fix Password Manager reports not displaying items with limited collection access (#18956)

Browse Source 
When "Owners and admins can manage all collections and items" is OFF, Password Manager
reports incorrectly filter out items from collections where the user has "Can view",
"Can view except passwords", or "Can edit except passwords" access.

The root cause is that all five PM report components filter ciphers using
`(!this.organization && !edit) || !viewPassword`. Since PM reports run without an
organization context (this.organization is undefined), this condition excludes any
item where edit=false or viewPassword=false. These permission checks are unnecessary
for PM reports because:

1. Personal vault items always have edit=true and viewPassword=true, so the checks
   never applied to them.
2. Organization items should appear in reports regardless of permission level — the
   user has collection access, and edit restrictions should only affect the item
   dialog, not report visibility.
3. Admin Console reports (which work correctly) skip this filtering because
   this.organization is always set, making the condition always false.

This also explains why "Can edit except passwords" items only appeared in the
Unsecured Websites report — it was the only report that didn't check !viewPassword.

Removed the edit/viewPassword filter conditions from all five PM report components:
- exposed-passwords-report
- weak-passwords-report
- reused-passwords-report
- inactive-two-factor-report
- unsecured-websites-report
This commit is contained in:
Alex
2026-02-18 09:00:36 -07:00
committed by GitHub
parent dda862a8c6
commit 1ef8f257b0
10 changed files with 41 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.spec.ts
View File

@@ -122,19 +122,16 @@ describe("ExposedPasswordsReportComponent", () => {
expect(component).toBeTruthy();
});
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";
it("should get ciphers with exposed passwords regardless of edit access", async () => {
jest.spyOn(auditService, "passwordLeaked").mockReturnValue(Promise.resolve<any>(1234));
jest.spyOn(component as any, "getAllCiphers").mockReturnValue(Promise.resolve<any>(cipherData));
await component.setCiphers();
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);
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);
});
it("should call fullSync method of syncService", () => {

6
apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.ts
View File

@@ -64,14 +64,12 @@ export class ExposedPasswordsReportComponent extends CipherReportComponent imple
this.filterStatus = [0];
allCiphers.forEach((ciph) => {
const { type, login, isDeleted, edit, viewPassword } = ciph;
const { type, login, isDeleted } = ciph;
if (
type !== CipherType.Login ||
login.password == null ||
login.password === "" ||
isDeleted ||
(!this.organization && !edit) ||
!viewPassword
isDeleted
) {
return;
}

23
apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.spec.ts
View File

@@ -95,9 +95,7 @@ describe("InactiveTwoFactorReportComponent", () => {
expect(component).toBeTruthy();
});
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";
it("should get ciphers with domains in the 2fa directory regardless of edit access", async () => {
component.services.set(
"101domain.com",
"https://help.101domain.com/account-management/account-security/enabling-disabling-two-factor-verification",
@@ -110,11 +108,10 @@ describe("InactiveTwoFactorReportComponent", () => {
jest.spyOn(component as any, "getAllCiphers").mockReturnValue(Promise.resolve<any>(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", () => {
@@ -197,7 +194,7 @@ describe("InactiveTwoFactorReportComponent", () => {
expect(doc).toBe("");
});
it("should return false if cipher does not have edit access and no organization", () => {
it("should return true for cipher without edit access", () => {
component.organization = null;
const cipher = createCipherView({
edit: false,
@@ -206,11 +203,11 @@ describe("InactiveTwoFactorReportComponent", () => {
},
});
const [doc, isInactive] = (component as any).isInactive2faCipher(cipher);
expect(isInactive).toBe(false);
expect(doc).toBe("");
expect(isInactive).toBe(true);
expect(doc).toBe("https://example.com/2fa-doc");
});
it("should return false if cipher does not have viewPassword", () => {
it("should return true for cipher without viewPassword", () => {
const cipher = createCipherView({
viewPassword: false,
login: {
@@ -218,8 +215,8 @@ describe("InactiveTwoFactorReportComponent", () => {
},
});
const [doc, isInactive] = (component as any).isInactive2faCipher(cipher);
expect(isInactive).toBe(false);
expect(doc).toBe("");
expect(isInactive).toBe(true);
expect(doc).toBe("https://example.com/2fa-doc");
});
it("should check all uris and return true if any matches domain or host", () => {

6
apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.ts
View File

@@ -92,14 +92,12 @@ export class InactiveTwoFactorReportComponent extends CipherReportComponent impl
let docFor2fa: string = "";
let isInactive2faCipher: boolean = false;
const { type, login, isDeleted, edit, viewPassword } = cipher;
const { type, login, isDeleted } = cipher;
if (
type !== CipherType.Login ||
(login.totp != null && login.totp !== "") ||
!login.hasUris ||
isDeleted ||
(!this.organization && !edit) ||
!viewPassword
isDeleted
) {
return [docFor2fa, isInactive2faCipher];
}

14
apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.spec.ts
View File

@@ -109,17 +109,15 @@ describe("ReusedPasswordsReportComponent", () => {
expect(component).toBeTruthy();
});
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";
it("should get ciphers with reused passwords regardless of edit access", async () => {
jest.spyOn(component as any, "getAllCiphers").mockReturnValue(Promise.resolve<any>(cipherData));
await component.setCiphers();
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);
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);
});
it("should call fullSync method of syncService", () => {

6
apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.ts
View File

@@ -71,14 +71,12 @@ export class ReusedPasswordsReportComponent extends CipherReportComponent implem
this.filterStatus = [0];
ciphers.forEach((ciph) => {
const { type, login, isDeleted, edit, viewPassword } = ciph;
const { type, login, isDeleted } = ciph;
if (
type !== CipherType.Login ||
login.password == null ||
login.password === "" ||
isDeleted ||
(!this.organization && !edit) ||
!viewPassword
isDeleted
) {
return;
}

11
apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.spec.ts
View File

@@ -118,17 +118,14 @@ describe("UnsecuredWebsitesReportComponent", () => {
expect(component).toBeTruthy();
});
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";
it("should get unsecured ciphers regardless of edit access", async () => {
jest.spyOn(component as any, "getAllCiphers").mockReturnValue(Promise.resolve<any>(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", () => {

7
apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.ts
View File

@@ -71,12 +71,7 @@ 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 ||
(!this.organization && !cipher.edit)
) {
if (cipher.type !== CipherType.Login || !cipher.login.hasUris || cipher.isDeleted) {
return false;
}

15
apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.spec.ts
View File

@@ -114,10 +114,7 @@ describe("WeakPasswordsReportComponent", () => {
expect(component).toBeTruthy();
});
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";
it("should get ciphers with weak passwords regardless of edit access", async () => {
jest.spyOn(passwordStrengthService, "getPasswordStrength").mockReturnValue({
password: "123",
score: 0,
@@ -125,11 +122,11 @@ describe("WeakPasswordsReportComponent", () => {
jest.spyOn(component as any, "getAllCiphers").mockReturnValue(Promise.resolve<any>(cipherData));
await component.setCiphers();
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);
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);
});
it("should call fullSync method of syncService", () => {

11
apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.ts
View File

@@ -103,15 +103,8 @@ export class WeakPasswordsReportComponent extends CipherReportComponent implemen
}
protected determineWeakPasswordScore(ciph: CipherView): ReportResult | null {
const { type, login, isDeleted, edit, viewPassword } = ciph;
if (
type !== CipherType.Login ||
login.password == null ||
login.password === "" ||
isDeleted ||
(!this.organization && !edit) ||
!viewPassword
) {
const { type, login, isDeleted } = ciph;
if (type !== CipherType.Login || login.password == null || login.password === "" || isDeleted) {
return;
}