1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-18 09:13:33 +00:00

[PM-28036] Sidebar for Critical apps shows incorrect data - fixed (#17363)

* PM-28036 added the download button to the code

* PM-28036 fix failing tests

* PM-28036 added additional unit tests

* PM-28036 fixed failed type testing

* PM-28036 removed unwanted await from method
This commit is contained in:
Vijay Oommen
2025-11-13 13:33:05 -06:00
committed by GitHub
parent 0af77ced45
commit df59f7820a
4 changed files with 273 additions and 3 deletions

View File

@@ -22,7 +22,7 @@
type="button" type="button"
class="tw-flex-1" class="tw-flex-1"
tabindex="0" tabindex="0"
(click)="dataService.setDrawerForOrgAtRiskMembers('criticalAppsAtRiskMembers')" (click)="dataService.setDrawerForCriticalAtRiskMembers('criticalAppsAtRiskMembers')"
> >
<dirt-card <dirt-card
#criticalAppsAtRiskMembers #criticalAppsAtRiskMembers
@@ -40,7 +40,7 @@
type="button" type="button"
class="tw-flex-1" class="tw-flex-1"
tabindex="0" tabindex="0"
(click)="dataService.setDrawerForOrgAtRiskApps('criticalAppsAtRiskApplications')" (click)="dataService.setDrawerForCriticalAtRiskApps('criticalAppsAtRiskApplications')"
> >
<dirt-card <dirt-card
#criticalAppsAtRiskApplications #criticalAppsAtRiskApplications

View File

@@ -14,6 +14,15 @@
}}</span> }}</span>
@if (drawerDetails.atRiskMemberDetails?.length > 0) { @if (drawerDetails.atRiskMemberDetails?.length > 0) {
<button
bitLink
type="button"
class="tw-my-4 tw-font-bold tw-block"
(click)="downloadAtRiskMembers()"
>
<i class="bwi bwi-download tw-mr-1"></i>
{{ "downloadCSV" | i18n }}
</button>
<ng-container> <ng-container>
<div class="tw-flex tw-justify-between tw-mt-2 tw-text-muted"> <div class="tw-flex tw-justify-between tw-mt-2 tw-text-muted">
<div bitTypography="body2" class="tw-text-sm tw-font-bold"> <div bitTypography="body2" class="tw-text-sm tw-font-bold">
@@ -77,6 +86,15 @@
}}</span> }}</span>
@if (drawerDetails.atRiskAppDetails?.length > 0) { @if (drawerDetails.atRiskAppDetails?.length > 0) {
<ng-container> <ng-container>
<button
bitLink
type="button"
class="tw-my-4 tw-font-bold tw-block"
(click)="downloadAtRiskApplications()"
>
<i class="bwi bwi-download tw-mr-1"></i>
{{ "downloadCSV" | i18n }}
</button>
<div class="tw-flex tw-justify-between tw-mt-2 tw-text-muted"> <div class="tw-flex tw-justify-between tw-mt-2 tw-text-muted">
<div bitTypography="body2" class="tw-text-sm tw-font-bold"> <div bitTypography="body2" class="tw-text-sm tw-font-bold">
{{ "application" | i18n }} {{ "application" | i18n }}

View File

@@ -3,8 +3,10 @@ import { BrowserAnimationsModule } from "@angular/platform-browser/animations";
import { mock } from "jest-mock-extended"; import { mock } from "jest-mock-extended";
import { DrawerDetails, DrawerType } from "@bitwarden/bit-common/dirt/reports/risk-insights"; import { DrawerDetails, DrawerType } from "@bitwarden/bit-common/dirt/reports/risk-insights";
import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { DIALOG_DATA } from "@bitwarden/components"; import { DIALOG_DATA } from "@bitwarden/components";
import { LogService } from "@bitwarden/logging";
import { I18nPipe } from "@bitwarden/ui-common"; import { I18nPipe } from "@bitwarden/ui-common";
import { RiskInsightsDrawerDialogComponent } from "./risk-insights-drawer-dialog.component"; import { RiskInsightsDrawerDialogComponent } from "./risk-insights-drawer-dialog.component";
@@ -48,6 +50,8 @@ describe("RiskInsightsDrawerDialogComponent", () => {
let component: RiskInsightsDrawerDialogComponent; let component: RiskInsightsDrawerDialogComponent;
let fixture: ComponentFixture<RiskInsightsDrawerDialogComponent>; let fixture: ComponentFixture<RiskInsightsDrawerDialogComponent>;
const mockI18nService = mock<I18nService>(); const mockI18nService = mock<I18nService>();
const mockFileDownloadService = mock<FileDownloadService>();
const mocklogService = mock<LogService>();
const drawerDetails: DrawerDetails = { const drawerDetails: DrawerDetails = {
open: true, open: true,
invokerId: "test-invoker", invokerId: "test-invoker",
@@ -56,6 +60,7 @@ describe("RiskInsightsDrawerDialogComponent", () => {
appAtRiskMembers: null, appAtRiskMembers: null,
atRiskAppDetails: null, atRiskAppDetails: null,
}; };
mockI18nService.t.mockImplementation((key: string) => key);
beforeEach(async () => { beforeEach(async () => {
await TestBed.configureTestingModule({ await TestBed.configureTestingModule({
@@ -64,6 +69,8 @@ describe("RiskInsightsDrawerDialogComponent", () => {
{ provide: DIALOG_DATA, useValue: drawerDetails }, { provide: DIALOG_DATA, useValue: drawerDetails },
{ provide: I18nPipe, useValue: mock<I18nPipe>() }, { provide: I18nPipe, useValue: mock<I18nPipe>() },
{ provide: I18nService, useValue: mockI18nService }, { provide: I18nService, useValue: mockI18nService },
{ provide: FileDownloadService, useValue: mockFileDownloadService },
{ provide: LogService, useValue: mocklogService },
], ],
}).compileComponents(); }).compileComponents();
@@ -93,4 +100,181 @@ describe("RiskInsightsDrawerDialogComponent", () => {
expect(component.isActiveDrawerType(DrawerType.AppAtRiskMembers)).toBeFalsy(); expect(component.isActiveDrawerType(DrawerType.AppAtRiskMembers)).toBeFalsy();
}); });
}); });
describe("downloadAtRiskMembers", () => {
beforeEach(() => {
jest.clearAllMocks();
});
it("should download CSV when drawer is open with correct type and has data", async () => {
component.drawerDetails = {
open: true,
invokerId: "test-invoker",
activeDrawerType: DrawerType.OrgAtRiskMembers,
atRiskMemberDetails: [
{ email: "user@example.com", atRiskPasswordCount: 5 },
{ email: "admin@example.com", atRiskPasswordCount: 3 },
],
appAtRiskMembers: null,
atRiskAppDetails: null,
};
mockI18nService.t.mockImplementation((key: string) => key);
await component.downloadAtRiskMembers();
expect(mockFileDownloadService.download).toHaveBeenCalledWith({
fileName: expect.stringContaining("at-risk-members"),
blobData: expect.any(String),
blobOptions: { type: "text/plain" },
});
});
it("should not download when drawer is closed", async () => {
component.drawerDetails = {
open: false,
invokerId: "test-invoker",
activeDrawerType: DrawerType.OrgAtRiskMembers,
atRiskMemberDetails: [{ email: "user@example.com", atRiskPasswordCount: 5 }],
appAtRiskMembers: null,
atRiskAppDetails: null,
};
await component.downloadAtRiskMembers();
expect(mockFileDownloadService.download).not.toHaveBeenCalled();
});
it("should not download when activeDrawerType is incorrect", async () => {
component.drawerDetails = {
open: true,
invokerId: "test-invoker",
activeDrawerType: DrawerType.OrgAtRiskApps,
atRiskMemberDetails: [{ email: "user@example.com", atRiskPasswordCount: 5 }],
appAtRiskMembers: null,
atRiskAppDetails: null,
};
await component.downloadAtRiskMembers();
expect(mockFileDownloadService.download).not.toHaveBeenCalled();
});
it("should not download when atRiskMemberDetails is null", async () => {
component.drawerDetails = {
open: true,
invokerId: "test-invoker",
activeDrawerType: DrawerType.OrgAtRiskMembers,
atRiskMemberDetails: [],
appAtRiskMembers: null,
atRiskAppDetails: null,
};
await component.downloadAtRiskMembers();
expect(mockFileDownloadService.download).not.toHaveBeenCalled();
});
it("should not download when atRiskMemberDetails is empty array", async () => {
component.drawerDetails = {
open: true,
invokerId: "test-invoker",
activeDrawerType: DrawerType.OrgAtRiskMembers,
atRiskMemberDetails: [],
appAtRiskMembers: null,
atRiskAppDetails: null,
};
await component.downloadAtRiskMembers();
expect(mockFileDownloadService.download).not.toHaveBeenCalled();
});
});
describe("downloadAtRiskApplications", () => {
beforeEach(() => {
jest.clearAllMocks();
});
it("should download CSV when drawer is open with correct type and has data", async () => {
component.drawerDetails = {
open: true,
invokerId: "test-invoker",
activeDrawerType: DrawerType.OrgAtRiskApps,
atRiskMemberDetails: [],
appAtRiskMembers: null,
atRiskAppDetails: [
{ applicationName: "App1", atRiskPasswordCount: 10 },
{ applicationName: "App2", atRiskPasswordCount: 7 },
],
};
await component.downloadAtRiskApplications();
expect(mockFileDownloadService.download).toHaveBeenCalledWith({
fileName: expect.stringContaining("at-risk-applications"),
blobData: expect.any(String),
blobOptions: { type: "text/plain" },
});
});
it("should not download when drawer is closed", async () => {
component.drawerDetails = {
open: false,
invokerId: "test-invoker",
activeDrawerType: DrawerType.OrgAtRiskApps,
atRiskMemberDetails: [],
appAtRiskMembers: null,
atRiskAppDetails: [{ applicationName: "App1", atRiskPasswordCount: 10 }],
};
await component.downloadAtRiskApplications();
expect(mockFileDownloadService.download).not.toHaveBeenCalled();
});
it("should not download when activeDrawerType is incorrect", async () => {
component.drawerDetails = {
open: true,
invokerId: "test-invoker",
activeDrawerType: DrawerType.OrgAtRiskMembers,
atRiskMemberDetails: [],
appAtRiskMembers: null,
atRiskAppDetails: [{ applicationName: "App1", atRiskPasswordCount: 10 }],
};
await component.downloadAtRiskApplications();
expect(mockFileDownloadService.download).not.toHaveBeenCalled();
});
it("should not download when atRiskAppDetails is null", async () => {
component.drawerDetails = {
open: true,
invokerId: "test-invoker",
activeDrawerType: DrawerType.OrgAtRiskApps,
atRiskMemberDetails: [],
appAtRiskMembers: null,
atRiskAppDetails: null,
};
await component.downloadAtRiskApplications();
expect(mockFileDownloadService.download).not.toHaveBeenCalled();
});
it("should not download when atRiskAppDetails is empty array", async () => {
component.drawerDetails = {
open: true,
invokerId: "test-invoker",
activeDrawerType: DrawerType.OrgAtRiskApps,
atRiskMemberDetails: [],
appAtRiskMembers: null,
atRiskAppDetails: [],
};
await component.downloadAtRiskApplications();
expect(mockFileDownloadService.download).not.toHaveBeenCalled();
});
});
}); });

View File

@@ -1,7 +1,12 @@
import { Component, ChangeDetectionStrategy, Inject } from "@angular/core"; import { Component, ChangeDetectionStrategy, Inject } from "@angular/core";
import { DrawerDetails, DrawerType } from "@bitwarden/bit-common/dirt/reports/risk-insights"; import { DrawerDetails, DrawerType } from "@bitwarden/bit-common/dirt/reports/risk-insights";
import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { DIALOG_DATA } from "@bitwarden/components"; import { DIALOG_DATA } from "@bitwarden/components";
import { LogService } from "@bitwarden/logging";
import { ExportHelper } from "@bitwarden/vault-export-core";
import { exportToCSV } from "@bitwarden/web-vault/app/dirt/reports/report-utils";
import { SharedModule } from "@bitwarden/web-vault/app/shared"; import { SharedModule } from "@bitwarden/web-vault/app/shared";
@Component({ @Component({
@@ -10,7 +15,12 @@ import { SharedModule } from "@bitwarden/web-vault/app/shared";
changeDetection: ChangeDetectionStrategy.OnPush, changeDetection: ChangeDetectionStrategy.OnPush,
}) })
export class RiskInsightsDrawerDialogComponent { export class RiskInsightsDrawerDialogComponent {
constructor(@Inject(DIALOG_DATA) public drawerDetails: DrawerDetails) {} constructor(
@Inject(DIALOG_DATA) public drawerDetails: DrawerDetails,
private fileDownloadService: FileDownloadService,
private i18nService: I18nService,
private logService: LogService,
) {}
// Get a list of drawer types // Get a list of drawer types
get drawerTypes(): typeof DrawerType { get drawerTypes(): typeof DrawerType {
@@ -20,4 +30,62 @@ export class RiskInsightsDrawerDialogComponent {
isActiveDrawerType(type: DrawerType): boolean { isActiveDrawerType(type: DrawerType): boolean {
return this.drawerDetails.activeDrawerType === type; return this.drawerDetails.activeDrawerType === type;
} }
/**
* downloads at risk members as CSV
*/
downloadAtRiskMembers() {
try {
// Validate drawer is open and showing the correct drawer type
if (
!this.drawerDetails.open ||
this.drawerDetails.activeDrawerType !== DrawerType.OrgAtRiskMembers ||
!this.drawerDetails.atRiskMemberDetails ||
this.drawerDetails.atRiskMemberDetails.length === 0
) {
return;
}
this.fileDownloadService.download({
fileName: ExportHelper.getFileName("at-risk-members"),
blobData: exportToCSV(this.drawerDetails.atRiskMemberDetails, {
email: this.i18nService.t("email"),
atRiskPasswordCount: this.i18nService.t("atRiskPasswords"),
}),
blobOptions: { type: "text/plain" },
});
} catch (error) {
// Log error for debugging
this.logService.error("Failed to download at-risk members", error);
}
}
/**
* downloads at risk applications as CSV
*/
downloadAtRiskApplications() {
try {
// Validate drawer is open and showing the correct drawer type
if (
!this.drawerDetails.open ||
this.drawerDetails.activeDrawerType !== DrawerType.OrgAtRiskApps ||
!this.drawerDetails.atRiskAppDetails ||
this.drawerDetails.atRiskAppDetails.length === 0
) {
return;
}
this.fileDownloadService.download({
fileName: ExportHelper.getFileName("at-risk-applications"),
blobData: exportToCSV(this.drawerDetails.atRiskAppDetails, {
applicationName: this.i18nService.t("application"),
atRiskPasswordCount: this.i18nService.t("atRiskPasswords"),
}),
blobOptions: { type: "text/plain" },
});
} catch (error) {
// Log error for debugging
this.logService.error("Failed to download at-risk applications", error);
}
}
} }