1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-09 05:00:10 +00:00

Fixed test case after merge. Cleaned up per comments on review

This commit is contained in:
Leslie Tilton
2025-10-20 10:16:53 -05:00
parent 38b72da441
commit efa0d992d9
6 changed files with 129 additions and 36 deletions

View File

@@ -64,6 +64,10 @@ export type OrganizationReportSummary = {
export type OrganizationReportApplication = {
applicationName: string;
isCritical: boolean;
/**
* Captures when a report has been reviewed by a user and
* can be filtered on to check for new applications
* */
reviewedDate: Date | null;
};

View File

@@ -148,7 +148,6 @@ export class CriticalAppsService {
return;
}
// TODO Uncomment when done testing that the migration is working
await this.criticalAppsApiService.dropCriticalApp({
organizationId: app.organizationId,
passwordHealthReportApplicationIds: [app.id],

View File

@@ -165,8 +165,92 @@ export class RiskInsightsOrchestratorService {
this._initializeOrganizationTriggerSubject.next(organizationId);
}
removeCriticalApplication$(criticalApplication: string): Observable<ReportState> {
this.logService.info(
"[RiskInsightsOrchestratorService] Removing critical applications from report",
);
return this.rawReportData$.pipe(
take(1),
filter((data) => !data.loading && data.data != null),
withLatestFrom(
this.organizationDetails$.pipe(filter((org) => !!org && !!org.organizationId)),
this._userId$.pipe(filter((userId) => !!userId)),
),
map(([reportState, organizationDetails, userId]) => {
// Create a set for quick lookup of the new critical apps
const existingApplicationData = reportState?.data?.applicationData || [];
const updatedApplicationData = this._removeCriticalApplication(
existingApplicationData,
criticalApplication,
);
const updatedState = {
...reportState,
data: {
...reportState.data,
applicationData: updatedApplicationData,
},
} as ReportState;
this.logService.debug(
"[RiskInsightsOrchestratorService] Updated applications data",
updatedState,
);
return { reportState, organizationDetails, updatedState, userId };
}),
switchMap(({ reportState, organizationDetails, updatedState, userId }) => {
return from(
this.riskInsightsEncryptionService.encryptRiskInsightsReport(
{
organizationId: organizationDetails!.organizationId,
userId: userId!,
},
{
reportData: reportState?.data?.reportData ?? [],
summaryData: reportState?.data?.summaryData ?? createNewSummaryData(),
applicationData: updatedState?.data?.applicationData ?? [],
},
),
).pipe(
map((encryptedData) => ({
reportState,
organizationDetails,
updatedState,
encryptedData,
})),
);
}),
switchMap(({ reportState, organizationDetails, updatedState, encryptedData }) => {
this.logService.debug(
`[RiskInsightsOrchestratorService] Saving applicationData with toggled critical flag for report with id: ${reportState?.data?.id} and org id: ${organizationDetails?.organizationId}`,
);
if (!reportState?.data?.id || !organizationDetails?.organizationId) {
return of({ ...reportState });
}
return this.reportApiService
.updateRiskInsightsApplicationData$(
reportState.data.id,
organizationDetails.organizationId,
{
data: {
applicationData: encryptedData.encryptedApplicationData.toSdk(),
},
},
)
.pipe(
map(() => updatedState),
tap((finalState) => this._rawReportDataSubject.next(finalState)),
catchError((error: unknown) => {
this.logService.error("Failed to save updated applicationData", error);
return of({ ...reportState, error: "Failed to remove a critical application" });
}),
);
}),
);
}
saveCriticalApplications$(criticalApplications: string[]): Observable<ReportState> {
this.logService.debug(
this.logService.info(
"[RiskInsightsOrchestratorService] Saving critical applications to report",
);
return this.rawReportData$.pipe(
@@ -223,7 +307,7 @@ export class RiskInsightsOrchestratorService {
}),
switchMap(({ reportState, organizationDetails, updatedState, encryptedData }) => {
this.logService.debug(
`[RiskInsightsOrchestratorService] Saving updated applicationData with report id: ${reportState?.data?.id} and org id: ${organizationDetails?.organizationId}`,
`[RiskInsightsOrchestratorService] Saving critical applications on applicationData with report id: ${reportState?.data?.id} and org id: ${organizationDetails?.organizationId}`,
);
if (!reportState?.data?.id || !organizationDetails?.organizationId) {
return of({ ...reportState });
@@ -243,7 +327,7 @@ export class RiskInsightsOrchestratorService {
tap((finalState) => this._rawReportDataSubject.next(finalState)),
catchError((error: unknown) => {
this.logService.error("Failed to save updated applicationData", error);
return of({ ...reportState, error: "Failed to save application data" });
return of({ ...reportState, error: "Failed to save critical applications" });
}),
);
}),
@@ -406,6 +490,20 @@ export class RiskInsightsOrchestratorService {
return updatedApps;
}
// Toggles the isCritical flag on applications via criticalApplicationName
private _removeCriticalApplication(
applicationData: OrganizationReportApplication[],
criticalApplication: string,
): OrganizationReportApplication[] {
const updatedApplicationData = applicationData.map((application) => {
if (application.applicationName == criticalApplication) {
return { ...application, isCritical: false } as OrganizationReportApplication;
}
return application;
});
return updatedApplicationData;
}
private _runMigrationAndCleanup$(criticalApps: PasswordHealthReportApplicationsResponse[]) {
return of(criticalApps).pipe(
withLatestFrom(this.organizationDetails$),

View File

@@ -184,24 +184,24 @@ export class RiskInsightsDataService {
// ------------------------------ Critical application methods --------------
saveCriticalApplications(selectedUrls: string[]) {
// Saving critical applications to the report
this.orchestrator.saveCriticalApplications$(selectedUrls);
return this.orchestrator.saveCriticalApplications$(selectedUrls);
// Legacy support: also save to the CriticalAppsService for backward compatibility
return this.organizationDetails$.pipe(
exhaustMap((organizationDetails) => {
if (!organizationDetails?.organizationId) {
return EMPTY;
}
return this.criticalAppsService.setCriticalApps(
organizationDetails?.organizationId,
selectedUrls,
);
}),
catchError((error: unknown) => {
this.errorSubject.next("Failed to save critical applications");
return throwError(() => error);
}),
);
// Legacy saving CriticalAppsService for backward compatibility
// return this.organizationDetails$.pipe(
// exhaustMap((organizationDetails) => {
// if (!organizationDetails?.organizationId) {
// return EMPTY;
// }
// return this.criticalAppsService.setCriticalApps(
// organizationDetails?.organizationId,
// selectedUrls,
// );
// }),
// catchError((error: unknown) => {
// this.errorSubject.next("Failed to save critical applications");
// return throwError(() => error);
// }),
// );
}
removeCriticalApplication(hostname: string) {

View File

@@ -10,14 +10,8 @@ import {
SecurityTasksApiService,
TaskMetrics,
} from "@bitwarden/bit-common/dirt/reports/risk-insights";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { OrganizationId } from "@bitwarden/common/types/guid";
import {
ButtonModule,
ProgressModule,
ToastService,
TypographyModule,
} from "@bitwarden/components";
import { ButtonModule, ProgressModule, TypographyModule } from "@bitwarden/components";
import { DefaultAdminTaskService } from "../../../../vault/services/default-admin-task.service";
import { RenderMode } from "../../models/activity.models";
@@ -45,8 +39,6 @@ export class PasswordChangeMetricComponent implements OnInit {
private activatedRoute: ActivatedRoute,
private securityTasksApiService: SecurityTasksApiService,
private allActivitiesService: AllActivitiesService,
protected toastService: ToastService,
protected i18nService: I18nService,
protected accessIntelligenceSecurityTasksService: AccessIntelligenceSecurityTasksService,
) {}

View File

@@ -3,7 +3,7 @@ import { mock } from "jest-mock-extended";
import {
AllActivitiesService,
LEGACY_ApplicationHealthReportDetailWithCriticalFlagAndCipher,
ApplicationHealthReportDetailEnriched,
} from "@bitwarden/bit-common/dirt/reports/risk-insights";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { OrganizationId } from "@bitwarden/common/types/guid";
@@ -42,7 +42,7 @@ describe("AccessIntelligenceSecurityTasksService", () => {
{
atRiskPasswordCount: 1,
atRiskCipherIds: ["cid1"],
} as LEGACY_ApplicationHealthReportDetailWithCriticalFlagAndCipher,
} as ApplicationHealthReportDetailEnriched,
];
const spy = jest.spyOn(service, "requestPasswordChange").mockResolvedValue(2);
await service.assignTasks(organizationId, apps);
@@ -58,11 +58,11 @@ describe("AccessIntelligenceSecurityTasksService", () => {
{
atRiskPasswordCount: 2,
atRiskCipherIds: ["cid1", "cid2"],
} as LEGACY_ApplicationHealthReportDetailWithCriticalFlagAndCipher,
} as ApplicationHealthReportDetailEnriched,
{
atRiskPasswordCount: 1,
atRiskCipherIds: ["cid2"],
} as LEGACY_ApplicationHealthReportDetailWithCriticalFlagAndCipher,
} as ApplicationHealthReportDetailEnriched,
];
defaultAdminTaskServiceSpy.bulkCreateTasks.mockResolvedValue(undefined);
i18nServiceSpy.t.mockImplementation((key) => key);
@@ -87,7 +87,7 @@ describe("AccessIntelligenceSecurityTasksService", () => {
{
atRiskPasswordCount: 1,
atRiskCipherIds: ["cid3"],
} as LEGACY_ApplicationHealthReportDetailWithCriticalFlagAndCipher,
} as ApplicationHealthReportDetailEnriched,
];
defaultAdminTaskServiceSpy.bulkCreateTasks.mockRejectedValue(new Error("fail"));
i18nServiceSpy.t.mockImplementation((key) => key);
@@ -108,7 +108,7 @@ describe("AccessIntelligenceSecurityTasksService", () => {
{
atRiskPasswordCount: 0,
atRiskCipherIds: ["cid4"],
} as LEGACY_ApplicationHealthReportDetailWithCriticalFlagAndCipher,
} as ApplicationHealthReportDetailEnriched,
];
const result = await service.requestPasswordChange(organizationId, apps);