From 9e465a53c2a0832cc9abd47a62648149f557cc56 Mon Sep 17 00:00:00 2001 From: Leslie Tilton <23057410+Banrion@users.noreply.github.com> Date: Mon, 20 Oct 2025 13:35:03 -0500 Subject: [PATCH] Fixed decryption and encryption issue when not using existing content key --- .../helpers/risk-insights-data-mappers.ts | 17 ----- .../risk-insights/models/report-models.ts | 2 + ...risk-insights-orchestrator.service.spec.ts | 13 +++- .../risk-insights-orchestrator.service.ts | 44 ++++++------- .../risk-insights-report.service.spec.ts | 3 +- .../domain/risk-insights-report.service.ts | 65 +++++++++++-------- 6 files changed, 74 insertions(+), 70 deletions(-) diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/helpers/risk-insights-data-mappers.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/helpers/risk-insights-data-mappers.ts index 1ce21da4444..624b695e6be 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/helpers/risk-insights-data-mappers.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/helpers/risk-insights-data-mappers.ts @@ -1,5 +1,4 @@ import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { OrganizationReportId } from "@bitwarden/common/types/guid"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { @@ -11,7 +10,6 @@ import { ApplicationHealthReportDetail, MemberDetails, OrganizationReportSummary, - RiskInsightsData, } from "../models/report-models"; export function flattenMemberDetails( @@ -60,21 +58,6 @@ export function getUniqueMembers(orgMembers: MemberDetails[]): MemberDetails[] { }); } -/** - * Create a new Risk Insights Report - * - * @returns An empty report - */ -export function createNewReportData(): RiskInsightsData { - return { - id: "" as OrganizationReportId, - creationDate: new Date(), - reportData: [], - summaryData: createNewSummaryData(), - applicationData: [], - }; -} - /** * Create a new Risk Insights Report Summary * diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/report-models.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/report-models.ts index b45abd01710..93955c7dbfb 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/report-models.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/report-models.ts @@ -1,5 +1,6 @@ import { Opaque } from "type-fest"; +import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { OrganizationReportId } from "@bitwarden/common/types/guid"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { BadgeVariant } from "@bitwarden/components"; @@ -101,6 +102,7 @@ export type ReportResult = CipherView & { export interface RiskInsightsData { id: OrganizationReportId; creationDate: Date; + contentEncryptionKey: EncString; reportData: ApplicationHealthReportDetail[]; summaryData: OrganizationReportSummary; applicationData: OrganizationReportApplication[]; diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-orchestrator.service.spec.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-orchestrator.service.spec.ts index f7d7da52a35..7606e3af7f3 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-orchestrator.service.spec.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-orchestrator.service.spec.ts @@ -3,6 +3,8 @@ import { of, throwError } from "rxjs"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; +import { makeEncString } from "@bitwarden/common/spec"; import { OrganizationId, OrganizationReportId, UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { LogService } from "@bitwarden/logging"; @@ -32,6 +34,7 @@ describe("RiskInsightsOrchestratorService", () => { const mockOrgName = "Test Org"; const mockUserId = "user-101" as UserId; const mockReportId = "report-1" as OrganizationReportId; + const mockKey: EncString = makeEncString("wrappedKey"); const reportState: RiskInsightsData = { id: mockReportId, @@ -39,6 +42,7 @@ describe("RiskInsightsOrchestratorService", () => { summaryData: createNewSummaryData(), applicationData: [], creationDate: new Date(), + contentEncryptionKey: mockKey, }; const mockCiphers = [{ id: "cipher-1" }] as any; @@ -65,9 +69,12 @@ describe("RiskInsightsOrchestratorService", () => { getApplicationsSummary: jest.fn().mockReturnValue(mockSummaryData), getOrganizationApplications: jest.fn().mockReturnValue(mockApplicationData), getRiskInsightsReport$: jest.fn().mockReturnValue(of(reportState)), - saveRiskInsightsReport$: jest - .fn() - .mockReturnValue(of({ id: mockReportId } as SaveRiskInsightsReportResponse)), + saveRiskInsightsReport$: jest.fn().mockReturnValue( + of({ + response: { id: mockReportId } as SaveRiskInsightsReportResponse, + contentEncryptionKey: mockKey, + }), + ), }); // Arrange mocks for new flow mockMemberCipherDetailsApiService.getMemberCipherDetails.mockResolvedValue( diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-orchestrator.service.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-orchestrator.service.ts index ae00e6a44a0..759808d28dd 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-orchestrator.service.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-orchestrator.service.ts @@ -32,7 +32,7 @@ import { } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; -import { OrganizationId, OrganizationReportId, UserId } from "@bitwarden/common/types/guid"; +import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { LogService } from "@bitwarden/logging"; @@ -210,6 +210,7 @@ export class RiskInsightsOrchestratorService { summaryData: reportState?.data?.summaryData ?? createNewSummaryData(), applicationData: updatedState?.data?.applicationData ?? [], }, + reportState.data.contentEncryptionKey, ), ).pipe( map((encryptedData) => ({ @@ -295,6 +296,7 @@ export class RiskInsightsOrchestratorService { summaryData: reportState?.data?.summaryData ?? createNewSummaryData(), applicationData: updatedState?.data?.applicationData ?? [], }, + reportState.data.contentEncryptionKey, ), ).pipe( map((encryptedData) => ({ @@ -337,19 +339,13 @@ export class RiskInsightsOrchestratorService { private _fetchReport$(organizationId: OrganizationId, userId: UserId): Observable { return this.reportService.getRiskInsightsReport$(organizationId, userId).pipe( tap(() => this.logService.debug("[RiskInsightsOrchestratorService] Fetching report")), - map( - ({ id, reportData, summaryData, applicationData, creationDate }): ReportState => ({ + map((result): ReportState => { + return { loading: false, error: null, - data: { - id, - reportData, - summaryData, - applicationData, - creationDate, - }, - }), - ), + data: result ?? null, + }; + }), tap((fetchedReport) => this.logService.debug("[RiskInsightsOrchestratorService] _fetchReport$", fetchedReport), ), @@ -384,35 +380,39 @@ export class RiskInsightsOrchestratorService { previousReport?.data?.applicationData ?? [], ), })), - switchMap(({ report, summary, applications }) => + switchMap(({ report, summary, applications }) => { // Save the report after enrichment - this.reportService + return this.reportService .saveRiskInsightsReport$(report, summary, applications, { organizationId, userId, }) .pipe( - map(() => ({ + map((result) => ({ report, summary, applications, + id: result.response.id, + contentEncryptionKey: result.contentEncryptionKey, })), - ), - ), + ); + }), // Update the running state - map( - ({ report, summary, applications }): ReportState => ({ + map((mappedResult): ReportState => { + const { id, report, summary, applications, contentEncryptionKey } = mappedResult; + return { loading: false, error: null, data: { - id: "" as OrganizationReportId, + id, reportData: report, summaryData: summary, applicationData: applications, creationDate: new Date(), + contentEncryptionKey, }, - }), - ), + }; + }), catchError(() => { return of({ loading: false, error: "Failed to generate or save report", data: null }); }), diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.spec.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.spec.ts index f0e88f4e434..3211b44322a 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.spec.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.spec.ts @@ -143,8 +143,6 @@ describe("RiskInsightsReportService", () => { }, }); }); - - it("should encrypt and save report, then update subjects", async () => {}); }); describe("getRiskInsightsReport$", () => { @@ -236,6 +234,7 @@ describe("RiskInsightsReportService", () => { ...mockDecryptedData, id: mockResponse.id, creationDate: mockResponse.creationDate, + contentEncryptionKey: mockEncryptedKey, }); }); }); diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.ts index 00591b5efe4..470442a811b 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.ts @@ -1,9 +1,10 @@ -import { catchError, EMPTY, from, map, Observable, of, switchMap, throwError } from "rxjs"; +import { catchError, EMPTY, from, map, Observable, switchMap, throwError } from "rxjs"; +import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { OrganizationId, OrganizationReportId, UserId } from "@bitwarden/common/types/guid"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; -import { createNewReportData, getUniqueMembers } from "../../helpers/risk-insights-data-mappers"; +import { getUniqueMembers } from "../../helpers/risk-insights-data-mappers"; import { isSaveRiskInsightsReportResponse, SaveRiskInsightsReportResponse, @@ -134,7 +135,7 @@ export class RiskInsightsReportService { switchMap((response) => { if (!response) { // Return an empty report and summary if response is falsy - return of(createNewReportData()); + return EMPTY; } if (!response.contentEncryptionKey || response.contentEncryptionKey.data == "") { return throwError(() => new Error("Report key not found")); @@ -163,13 +164,17 @@ export class RiskInsightsReportService { response.contentEncryptionKey, ), ).pipe( - map((decryptedData) => ({ - id: response.id as OrganizationReportId, - reportData: decryptedData.reportData, - summaryData: decryptedData.summaryData, - applicationData: decryptedData.applicationData, - creationDate: response.creationDate, - })), + map((decryptedData) => { + const newReport: RiskInsightsData = { + id: response.id as OrganizationReportId, + reportData: decryptedData.reportData, + summaryData: decryptedData.summaryData, + applicationData: decryptedData.applicationData, + creationDate: response.creationDate, + contentEncryptionKey: response.contentEncryptionKey, + }; + return newReport; + }), catchError((error: unknown) => { return throwError(() => error); }), @@ -196,7 +201,7 @@ export class RiskInsightsReportService { organizationId: OrganizationId; userId: UserId; }, - ): Observable { + ): Observable<{ response: SaveRiskInsightsReportResponse; contentEncryptionKey: EncString }> { return from( this.riskInsightsEncryptionService.encryptRiskInsightsReport( { @@ -217,30 +222,38 @@ export class RiskInsightsReportService { encryptedApplicationData, contentEncryptionKey, }) => ({ - data: { - organizationId: encryptionParameters.organizationId, - creationDate: new Date().toISOString(), - reportData: encryptedReportData.toSdk(), - summaryData: encryptedSummaryData.toSdk(), - applicationData: encryptedApplicationData.toSdk(), - contentEncryptionKey: contentEncryptionKey.toSdk(), + requestPayload: { + data: { + organizationId: encryptionParameters.organizationId, + creationDate: new Date().toISOString(), + reportData: encryptedReportData.toSdk(), + summaryData: encryptedSummaryData.toSdk(), + applicationData: encryptedApplicationData.toSdk(), + contentEncryptionKey: contentEncryptionKey.toSdk(), + }, }, + // Keep the original EncString alongside the SDK payload so downstream can return the EncString type. + contentEncryptionKey, }), ), - switchMap((encryptedReport) => - this.riskInsightsApiService.saveRiskInsightsReport$( - encryptedReport, - encryptionParameters.organizationId, - ), + switchMap(({ requestPayload, contentEncryptionKey }) => + this.riskInsightsApiService + .saveRiskInsightsReport$(requestPayload, encryptionParameters.organizationId) + .pipe( + map((response) => ({ + response, + contentEncryptionKey, + })), + ), ), catchError((error: unknown) => { return EMPTY; }), - map((response) => { - if (!isSaveRiskInsightsReportResponse(response)) { + map((result) => { + if (!isSaveRiskInsightsReportResponse(result.response)) { throw new Error("Invalid response from API"); } - return response; + return result; }), ); }