From 31d5b639e948635863b9b305db0806fb54f35e82 Mon Sep 17 00:00:00 2001 From: Leslie Tilton <23057410+Banrion@users.noreply.github.com> Date: Thu, 11 Sep 2025 13:17:13 -0500 Subject: [PATCH] [PM-20578] [PM-20579] Merge existing feature branch into main (#16364) * PM-20578 Added api to fetch and save data (#15334) * [PM-20579] Update risk-insights report service to use api service with encryption (#15357) * Fix type error * Fix paths for changed key generation service * Finalize the api services * Fixing test case for summary date range * Fixing report service tests. Encryption will be modified in the future * Fixing encryption service tests * fixing linting issues --------- Co-authored-by: Vijay Oommen Co-authored-by: Tom --- .../risk-insights/models/password-health.ts | 40 +++- .../risk-insights-api.service.spec.ts | 181 +++++++++++--- .../services/risk-insights-api.service.ts | 54 ++++- .../services/risk-insights-data.service.ts | 6 +- .../risk-insights-encryption.service.spec.ts | 220 ++++++++++++++++++ .../risk-insights-encryption.service.ts | 105 +++++++++ .../risk-insights-report.service.spec.ts | 124 +++++++++- .../services/risk-insights-report.service.ts | 112 ++++++++- .../access-intelligence.module.ts | 7 + .../all-applications.component.ts | 4 +- .../critical-applications.component.ts | 7 +- .../risk-insights.component.ts | 8 +- libs/common/src/types/guid.ts | 1 + 13 files changed, 806 insertions(+), 63 deletions(-) create mode 100644 bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-encryption.service.spec.ts create mode 100644 bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-encryption.service.ts diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/password-health.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/password-health.ts index 6be24d60b89..07037f7da6d 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/password-health.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/password-health.ts @@ -1,11 +1,11 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore - import { Opaque } from "type-fest"; import { OrganizationId } from "@bitwarden/common/types/guid"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { BadgeVariant } from "@bitwarden/components"; +import { EncString } from "@bitwarden/sdk-internal"; /** * All applications report summary. The total members, @@ -132,6 +132,16 @@ export type AppAtRiskMembersDialogParams = { applicationName: string; }; +/* + * After data is encrypted, it is returned with the + * encryption key used to encrypt the data. + */ +export interface EncryptedDataWithKey { + organizationId: OrganizationId; + encryptedData: EncString; + encryptionKey: EncString; +} + /** * Request to drop a password health report application * Model is expected by the API endpoint @@ -174,4 +184,32 @@ export enum DrawerType { OrgAtRiskApps = 3, } +export interface RiskInsightsReport { + organizationId: OrganizationId; + date: string; + reportData: string; + reportKey: string; +} + +export interface ReportInsightsReportData { + data: ApplicationHealthReportDetail[]; + summary: ApplicationHealthReportSummary; +} + +export interface SaveRiskInsightsReportRequest { + data: RiskInsightsReport; +} + +export interface SaveRiskInsightsReportResponse { + id: string; +} + +export interface GetRiskInsightsReportResponse { + id: string; + organizationId: OrganizationId; + date: string; + reportData: EncString; + reportKey: EncString; +} + export type PasswordHealthReportApplicationId = Opaque; diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-api.service.spec.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-api.service.spec.ts index ef9fc768944..19172d2e7fb 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-api.service.spec.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-api.service.spec.ts @@ -1,8 +1,10 @@ import { mock } from "jest-mock-extended"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { makeEncString } from "@bitwarden/common/spec"; +import { OrganizationId, OrganizationReportId } from "@bitwarden/common/types/guid"; -import { EncryptedDataModel } from "../models/password-health"; +import { EncryptedDataModel, SaveRiskInsightsReportRequest } from "../models/password-health"; import { RiskInsightsApiService } from "./risk-insights-api.service"; @@ -10,6 +12,30 @@ describe("RiskInsightsApiService", () => { let service: RiskInsightsApiService; const mockApiService = mock(); + const orgId = "org1" as OrganizationId; + + const getRiskInsightsReportResponse = { + organizationId: orgId, + date: new Date().toISOString(), + reportData: "test", + reportKey: "test-key", + }; + + const reportData = makeEncString("test").encryptedString?.toString() ?? ""; + const reportKey = makeEncString("test-key").encryptedString?.toString() ?? ""; + + const saveRiskInsightsReportRequest: SaveRiskInsightsReportRequest = { + data: { + organizationId: orgId, + date: new Date().toISOString(), + reportData: reportData, + reportKey: reportKey, + }, + }; + const saveRiskInsightsReportResponse = { + ...saveRiskInsightsReportRequest.data, + }; + beforeEach(() => { service = new RiskInsightsApiService(mockApiService); }); @@ -18,66 +44,157 @@ describe("RiskInsightsApiService", () => { expect(service).toBeTruthy(); }); - describe("getRiskInsightsSummary", () => { - it("should call apiService.send with correct parameters and return an Observable", (done) => { - const orgId = "org123"; - const minDate = new Date("2024-01-01"); - const maxDate = new Date("2024-01-31"); - const mockResponse: EncryptedDataModel[] = [{ encryptedData: "abc" } as EncryptedDataModel]; + it("should call apiService.send with correct parameters and return the response for getRiskInsightsReport ", (done) => { + mockApiService.send.mockReturnValue(Promise.resolve(getRiskInsightsReportResponse)); - mockApiService.send.mockResolvedValueOnce(mockResponse); + service.getRiskInsightsReport(orgId).subscribe((result) => { + expect(result).toEqual(getRiskInsightsReportResponse); + expect(mockApiService.send).toHaveBeenCalledWith( + "GET", + `/reports/organizations/${orgId.toString()}/latest`, + null, + true, + true, + ); + done(); + }); + }); - service.getRiskInsightsSummary(orgId, minDate, maxDate).subscribe((result) => { + it("should return null if apiService.send rejects with 404 error for getRiskInsightsReport", (done) => { + const error = { statusCode: 404 }; + mockApiService.send.mockReturnValue(Promise.reject(error)); + + service.getRiskInsightsReport(orgId).subscribe((result) => { + expect(result).toBeNull(); + done(); + }); + }); + + it("should throw error if apiService.send rejects with non-404 error for getRiskInsightsReport", (done) => { + const error = { statusCode: 500, message: "Server error" }; + mockApiService.send.mockReturnValue(Promise.reject(error)); + + service.getRiskInsightsReport(orgId).subscribe({ + next: () => { + // Should not reach here + fail("Expected error to be thrown"); + }, + error: () => { expect(mockApiService.send).toHaveBeenCalledWith( "GET", - `organization-report-summary/org123?from=2024-01-01&to=2024-01-31`, + `/reports/organizations/${orgId.toString()}/latest`, null, true, true, ); - expect(result).toEqual(mockResponse); done(); - }); + }, + complete: () => { + done(); + }, }); }); - describe("saveRiskInsightsSummary", () => { - it("should call apiService.send with correct parameters and return an Observable", (done) => { - const data: EncryptedDataModel = { encryptedData: "xyz" } as EncryptedDataModel; + it("should call apiService.send with correct parameters for saveRiskInsightsReport", (done) => { + mockApiService.send.mockReturnValue(Promise.resolve(saveRiskInsightsReportResponse)); - mockApiService.send.mockResolvedValueOnce(undefined); + service.saveRiskInsightsReport(saveRiskInsightsReportRequest, orgId).subscribe((result) => { + expect(result).toEqual(saveRiskInsightsReportResponse); + expect(mockApiService.send).toHaveBeenCalledWith( + "POST", + `/reports/organizations/${orgId.toString()}`, + saveRiskInsightsReportRequest.data, + true, + true, + ); + done(); + }); + }); - service.saveRiskInsightsSummary(data).subscribe((result) => { + it("should propagate errors from apiService.send for saveRiskInsightsReport - 1", (done) => { + const error = { statusCode: 500, message: "Internal Server Error" }; + mockApiService.send.mockReturnValue(Promise.reject(error)); + + service.saveRiskInsightsReport(saveRiskInsightsReportRequest, orgId).subscribe({ + next: () => { + fail("Expected error to be thrown"); + }, + error: () => { expect(mockApiService.send).toHaveBeenCalledWith( "POST", - "organization-report-summary", - data, + `/reports/organizations/${orgId.toString()}`, + saveRiskInsightsReportRequest.data, true, true, ); - expect(result).toBeUndefined(); done(); - }); + }, + complete: () => { + done(); + }, }); }); - describe("updateRiskInsightsSummary", () => { - it("should call apiService.send with correct parameters and return an Observable", (done) => { - const data: EncryptedDataModel = { encryptedData: "xyz" } as EncryptedDataModel; + it("should propagate network errors from apiService.send for saveRiskInsightsReport - 2", (done) => { + const error = new Error("Network error"); + mockApiService.send.mockReturnValue(Promise.reject(error)); - mockApiService.send.mockResolvedValueOnce(undefined); - - service.updateRiskInsightsSummary(data).subscribe((result) => { + service.saveRiskInsightsReport(saveRiskInsightsReportRequest, orgId).subscribe({ + next: () => { + fail("Expected error to be thrown"); + }, + error: () => { expect(mockApiService.send).toHaveBeenCalledWith( - "PUT", - "organization-report-summary", - data, + "POST", + `/reports/organizations/${orgId.toString()}`, + saveRiskInsightsReportRequest.data, true, true, ); - expect(result).toBeUndefined(); done(); - }); + }, + complete: () => { + done(); + }, + }); + }); + + it("should call apiService.send with correct parameters and return an Observable", (done) => { + const minDate = new Date("2024-01-01"); + const maxDate = new Date("2024-01-31"); + const mockResponse: EncryptedDataModel[] = [{ encryptedData: "abc" } as EncryptedDataModel]; + + mockApiService.send.mockResolvedValueOnce(mockResponse); + + service.getRiskInsightsSummary(orgId, minDate, maxDate).subscribe((result) => { + expect(mockApiService.send).toHaveBeenCalledWith( + "GET", + `/reports/organizations/${orgId.toString()}/data/summary?startDate=${minDate.toISOString().split("T")[0]}&endDate=${maxDate.toISOString().split("T")[0]}`, + null, + true, + true, + ); + expect(result).toEqual(mockResponse); + done(); + }); + }); + + it("should call apiService.send with correct parameters and return an Observable", (done) => { + const data: EncryptedDataModel = { encryptedData: "xyz" } as EncryptedDataModel; + const reportId = "report123" as OrganizationReportId; + + mockApiService.send.mockResolvedValueOnce(undefined); + + service.updateRiskInsightsSummary(data, orgId, reportId).subscribe((result) => { + expect(mockApiService.send).toHaveBeenCalledWith( + "PATCH", + `/reports/organizations/${orgId.toString()}/data/summary/${reportId.toString()}`, + data, + true, + true, + ); + expect(result).toBeUndefined(); + done(); }); }); }); diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-api.service.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-api.service.ts index 0d69947b826..6beef8d1401 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-api.service.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-api.service.ts @@ -1,12 +1,46 @@ import { from, Observable } from "rxjs"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { OrganizationId, OrganizationReportId } from "@bitwarden/common/types/guid"; -import { EncryptedDataModel } from "../models/password-health"; +import { + EncryptedDataModel, + GetRiskInsightsReportResponse, + SaveRiskInsightsReportRequest, + SaveRiskInsightsReportResponse, +} from "../models/password-health"; export class RiskInsightsApiService { constructor(private apiService: ApiService) {} + getRiskInsightsReport(orgId: OrganizationId): Observable { + const dbResponse = this.apiService + .send("GET", `/reports/organizations/${orgId.toString()}/latest`, null, true, true) + .catch((error: any): any => { + if (error.statusCode === 404) { + return null; // Handle 404 by returning null or an appropriate default value + } + throw error; // Re-throw other errors + }); + + return from(dbResponse as Promise); + } + + saveRiskInsightsReport( + request: SaveRiskInsightsReportRequest, + organizationId: OrganizationId, + ): Observable { + const dbResponse = this.apiService.send( + "POST", + `/reports/organizations/${organizationId.toString()}`, + request.data, + true, + true, + ); + + return from(dbResponse as Promise); + } + getRiskInsightsSummary( orgId: string, minDate: Date, @@ -16,7 +50,7 @@ export class RiskInsightsApiService { const maxDateStr = maxDate.toISOString().split("T")[0]; const dbResponse = this.apiService.send( "GET", - `organization-report-summary/${orgId.toString()}?from=${minDateStr}&to=${maxDateStr}`, + `/reports/organizations/${orgId.toString()}/data/summary?startDate=${minDateStr}&endDate=${maxDateStr}`, null, true, true, @@ -25,10 +59,14 @@ export class RiskInsightsApiService { return from(dbResponse as Promise); } - saveRiskInsightsSummary(data: EncryptedDataModel): Observable { + updateRiskInsightsSummary( + data: EncryptedDataModel, + organizationId: OrganizationId, + reportId: OrganizationReportId, + ): Observable { const dbResponse = this.apiService.send( - "POST", - "organization-report-summary", + "PATCH", + `/reports/organizations/${organizationId.toString()}/data/summary/${reportId.toString()}`, data, true, true, @@ -36,10 +74,4 @@ export class RiskInsightsApiService { return from(dbResponse as Promise); } - - updateRiskInsightsSummary(data: EncryptedDataModel): Observable { - const dbResponse = this.apiService.send("PUT", "organization-report-summary", data, true, true); - - return from(dbResponse as Promise); - } } diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-data.service.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-data.service.ts index 386c6fd6865..5e6dbcd54b5 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-data.service.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-data.service.ts @@ -1,6 +1,8 @@ import { BehaviorSubject } from "rxjs"; import { finalize } from "rxjs/operators"; +import { OrganizationId } from "@bitwarden/common/types/guid"; + import { AppAtRiskMembersDialogParams, ApplicationHealthReportDetail, @@ -40,7 +42,7 @@ export class RiskInsightsDataService { * Fetches the applications report and updates the applicationsSubject. * @param organizationId The ID of the organization. */ - fetchApplicationsReport(organizationId: string, isRefresh?: boolean): void { + fetchApplicationsReport(organizationId: OrganizationId, isRefresh?: boolean): void { if (isRefresh) { this.isRefreshingSubject.next(true); } else { @@ -66,7 +68,7 @@ export class RiskInsightsDataService { }); } - refreshApplicationsReport(organizationId: string): void { + refreshApplicationsReport(organizationId: OrganizationId): void { this.fetchApplicationsReport(organizationId, true); } diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-encryption.service.spec.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-encryption.service.spec.ts new file mode 100644 index 00000000000..dae56327c29 --- /dev/null +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-encryption.service.spec.ts @@ -0,0 +1,220 @@ +import { mock } from "jest-mock-extended"; +import { BehaviorSubject } from "rxjs"; + +import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; +import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; +import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; +import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { makeSymmetricCryptoKey } from "@bitwarden/common/spec"; +import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; +import { OrgKey } from "@bitwarden/common/types/key"; +import { KeyService } from "@bitwarden/key-management"; + +import { RiskInsightsEncryptionService } from "./risk-insights-encryption.service"; + +describe("RiskInsightsEncryptionService", () => { + let service: RiskInsightsEncryptionService; + const mockKeyService = mock(); + const mockEncryptService = mock(); + const mockKeyGenerationService = mock(); + + const ENCRYPTED_TEXT = "This data has been encrypted"; + const ENCRYPTED_KEY = "Re-encrypted Cipher Key"; + const orgId = "org-123" as OrganizationId; + const userId = "user-123" as UserId; + const orgKey = makeSymmetricCryptoKey(); + const contentEncryptionKey = new SymmetricCryptoKey(new Uint8Array(64)); + const testData = { foo: "bar" }; + const OrgRecords: Record = { + [orgId]: orgKey, + ["testOrg" as OrganizationId]: makeSymmetricCryptoKey(), + }; + const orgKey$ = new BehaviorSubject(OrgRecords); + + beforeEach(() => { + service = new RiskInsightsEncryptionService( + mockKeyService, + mockEncryptService, + mockKeyGenerationService, + ); + + jest.clearAllMocks(); + + // Always use the same contentEncryptionKey for both encrypt and decrypt tests + mockKeyGenerationService.createKey.mockResolvedValue(contentEncryptionKey); + mockEncryptService.wrapSymmetricKey.mockResolvedValue(new EncString(ENCRYPTED_KEY)); + mockEncryptService.encryptString.mockResolvedValue(new EncString(ENCRYPTED_TEXT)); + mockEncryptService.unwrapSymmetricKey.mockResolvedValue(contentEncryptionKey); + mockEncryptService.decryptString.mockResolvedValue(JSON.stringify(testData)); + mockKeyService.orgKeys$.mockReturnValue(orgKey$); + }); + + describe("encryptRiskInsightsReport", () => { + it("should encrypt data and return encrypted packet", async () => { + // arrange: setup our mocks + mockKeyService.orgKeys$.mockReturnValue(orgKey$); + + // Act: call the method under test + const result = await service.encryptRiskInsightsReport(orgId, userId, testData); + + // Assert: ensure that the methods were called with the expected parameters + expect(mockKeyService.orgKeys$).toHaveBeenCalledWith(userId); + expect(mockKeyGenerationService.createKey).toHaveBeenCalledWith(512); + expect(mockEncryptService.encryptString).toHaveBeenCalledWith( + JSON.stringify(testData), + contentEncryptionKey, + ); + expect(mockEncryptService.wrapSymmetricKey).toHaveBeenCalledWith( + contentEncryptionKey, + orgKey, + ); + expect(result).toEqual({ + organizationId: orgId, + encryptedData: ENCRYPTED_TEXT, + encryptionKey: ENCRYPTED_KEY, + }); + }); + + it("should throw an error when encrypted text is null or empty", async () => { + // arrange: setup our mocks + mockKeyService.orgKeys$.mockReturnValue(orgKey$); + mockEncryptService.encryptString.mockResolvedValue(new EncString("")); + mockEncryptService.wrapSymmetricKey.mockResolvedValue(new EncString(ENCRYPTED_KEY)); + + // Act & Assert: call the method under test and expect rejection + await expect(service.encryptRiskInsightsReport(orgId, userId, testData)).rejects.toThrow( + "Encryption failed, encrypted strings are null", + ); + }); + + it("should throw an error when encrypted key is null or empty", async () => { + // arrange: setup our mocks + mockKeyService.orgKeys$.mockReturnValue(orgKey$); + mockEncryptService.encryptString.mockResolvedValue(new EncString(ENCRYPTED_TEXT)); + mockEncryptService.wrapSymmetricKey.mockResolvedValue(new EncString("")); + + // Act & Assert: call the method under test and expect rejection + await expect(service.encryptRiskInsightsReport(orgId, userId, testData)).rejects.toThrow( + "Encryption failed, encrypted strings are null", + ); + }); + + it("should throw if org key is not found", async () => { + // when we cannot get an organization key, we should throw an error + mockKeyService.orgKeys$.mockReturnValue(new BehaviorSubject({})); + + await expect(service.encryptRiskInsightsReport(orgId, userId, testData)).rejects.toThrow( + "Organization key not found", + ); + }); + }); + + describe("decryptRiskInsightsReport", () => { + it("should decrypt data and return original object", async () => { + // Arrange: setup our mocks + mockKeyService.orgKeys$.mockReturnValue(orgKey$); + mockEncryptService.unwrapSymmetricKey.mockResolvedValue(contentEncryptionKey); + mockEncryptService.decryptString.mockResolvedValue(JSON.stringify(testData)); + + // act: call the decrypt method - with any params + // actual decryption does not happen here, + // we just want to ensure the method calls are correct + const result = await service.decryptRiskInsightsReport( + orgId, + userId, + new EncString("encrypted-data"), + new EncString("wrapped-key"), + (data) => data as typeof testData, + ); + + expect(mockKeyService.orgKeys$).toHaveBeenCalledWith(userId); + expect(mockEncryptService.unwrapSymmetricKey).toHaveBeenCalledWith( + new EncString("wrapped-key"), + orgKey, + ); + expect(mockEncryptService.decryptString).toHaveBeenCalledWith( + new EncString("encrypted-data"), + contentEncryptionKey, + ); + expect(result).toEqual(testData); + }); + + it("should invoke data type validation method during decryption", async () => { + // Arrange: setup our mocks + mockKeyService.orgKeys$.mockReturnValue(orgKey$); + mockEncryptService.unwrapSymmetricKey.mockResolvedValue(contentEncryptionKey); + mockEncryptService.decryptString.mockResolvedValue(JSON.stringify(testData)); + const mockParseFn = jest.fn((data) => data as typeof testData); + + // act: call the decrypt method - with any params + // actual decryption does not happen here, + // we just want to ensure the method calls are correct + const result = await service.decryptRiskInsightsReport( + orgId, + userId, + new EncString("encrypted-data"), + new EncString("wrapped-key"), + mockParseFn, + ); + + expect(mockParseFn).toHaveBeenCalledWith(JSON.parse(JSON.stringify(testData))); + expect(result).toEqual(testData); + }); + + it("should return null if org key is not found", async () => { + mockKeyService.orgKeys$.mockReturnValue(new BehaviorSubject({})); + + const result = await service.decryptRiskInsightsReport( + orgId, + userId, + new EncString("encrypted-data"), + new EncString("wrapped-key"), + (data) => data as typeof testData, + ); + + expect(result).toBeNull(); + }); + + it("should return null if decrypt throws", async () => { + mockKeyService.orgKeys$.mockReturnValue(orgKey$); + mockEncryptService.unwrapSymmetricKey.mockRejectedValue(new Error("fail")); + + const result = await service.decryptRiskInsightsReport( + orgId, + userId, + new EncString("encrypted-data"), + new EncString("wrapped-key"), + (data) => data as typeof testData, + ); + expect(result).toBeNull(); + }); + + it("should return null if decrypt throws", async () => { + mockKeyService.orgKeys$.mockReturnValue(orgKey$); + mockEncryptService.unwrapSymmetricKey.mockRejectedValue(new Error("fail")); + + const result = await service.decryptRiskInsightsReport( + orgId, + userId, + new EncString("encrypted-data"), + new EncString("wrapped-key"), + (data) => data as typeof testData, + ); + expect(result).toBeNull(); + }); + + it("should return null if decrypt throws", async () => { + mockKeyService.orgKeys$.mockReturnValue(orgKey$); + mockEncryptService.unwrapSymmetricKey.mockRejectedValue(new Error("fail")); + + const result = await service.decryptRiskInsightsReport( + orgId, + userId, + new EncString("encrypted-data"), + new EncString("wrapped-key"), + (data) => data as typeof testData, + ); + expect(result).toBeNull(); + }); + }); +}); diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-encryption.service.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-encryption.service.ts new file mode 100644 index 00000000000..f3c3a68b470 --- /dev/null +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-encryption.service.ts @@ -0,0 +1,105 @@ +import { firstValueFrom, map } from "rxjs"; +import { Jsonify } from "type-fest"; + +import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; +import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; +import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; +import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; +import { KeyService } from "@bitwarden/key-management"; + +import { EncryptedDataWithKey } from "../models/password-health"; + +export class RiskInsightsEncryptionService { + constructor( + private keyService: KeyService, + private encryptService: EncryptService, + private keyGeneratorService: KeyGenerationService, + ) {} + + async encryptRiskInsightsReport( + organizationId: OrganizationId, + userId: UserId, + data: T, + ): Promise { + const orgKey = await firstValueFrom( + this.keyService + .orgKeys$(userId) + .pipe( + map((organizationKeysById) => + organizationKeysById ? organizationKeysById[organizationId] : null, + ), + ), + ); + + if (!orgKey) { + throw new Error("Organization key not found"); + } + + const contentEncryptionKey = await this.keyGeneratorService.createKey(512); + + const dataEncrypted = await this.encryptService.encryptString( + JSON.stringify(data), + contentEncryptionKey, + ); + + const wrappedEncryptionKey = await this.encryptService.wrapSymmetricKey( + contentEncryptionKey, + orgKey, + ); + + if (!dataEncrypted.encryptedString || !wrappedEncryptionKey.encryptedString) { + throw new Error("Encryption failed, encrypted strings are null"); + } + + const encryptedData = dataEncrypted.encryptedString; + const encryptionKey = wrappedEncryptionKey.encryptedString; + + const encryptedDataPacket = { + organizationId: organizationId, + encryptedData: encryptedData, + encryptionKey: encryptionKey, + }; + + return encryptedDataPacket; + } + + async decryptRiskInsightsReport( + organizationId: OrganizationId, + userId: UserId, + encryptedData: EncString, + wrappedKey: EncString, + parser: (data: Jsonify) => T, + ): Promise { + try { + const orgKey = await firstValueFrom( + this.keyService + .orgKeys$(userId) + .pipe( + map((organizationKeysById) => + organizationKeysById ? organizationKeysById[organizationId] : null, + ), + ), + ); + + if (!orgKey) { + throw new Error("Organization key not found"); + } + + const unwrappedEncryptionKey = await this.encryptService.unwrapSymmetricKey( + wrappedKey, + orgKey, + ); + + const dataUnencrypted = await this.encryptService.decryptString( + encryptedData, + unwrappedEncryptionKey, + ); + + const dataUnencryptedJson = parser(JSON.parse(dataUnencrypted)); + + return dataUnencryptedJson as T; + } catch { + return null; + } + } +} diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.spec.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.spec.ts index 3aa624f1e59..b066bc7c16c 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.spec.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.spec.ts @@ -1,14 +1,20 @@ import { mock } from "jest-mock-extended"; -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, of } from "rxjs"; import { ZXCVBNResult } from "zxcvbn"; import { AuditService } from "@bitwarden/common/abstractions/audit.service"; +import { EncryptedString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; +import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { GetRiskInsightsReportResponse } from "../models/password-health"; + import { mockCiphers } from "./ciphers.mock"; import { MemberCipherDetailsApiService } from "./member-cipher-details-api.service"; import { mockMemberCipherDetails } from "./member-cipher-details-api.service.spec"; +import { RiskInsightsApiService } from "./risk-insights-api.service"; +import { RiskInsightsEncryptionService } from "./risk-insights-encryption.service"; import { RiskInsightsReportService } from "./risk-insights-report.service"; describe("RiskInsightsReportService", () => { @@ -17,6 +23,12 @@ describe("RiskInsightsReportService", () => { const auditService = mock(); const cipherService = mock(); const memberCipherDetailsService = mock(); + const mockRiskInsightsApiService = mock(); + const mockRiskInsightsEncryptionService = mock({ + encryptRiskInsightsReport: jest.fn().mockResolvedValue("encryptedReportData"), + decryptRiskInsightsReport: jest.fn().mockResolvedValue("decryptedReportData"), + }); + const orgId = "orgId" as OrganizationId; beforeEach(() => { pwdStrengthService.getPasswordStrength.mockImplementation((password: string) => { @@ -37,11 +49,13 @@ describe("RiskInsightsReportService", () => { auditService, cipherService, memberCipherDetailsService, + mockRiskInsightsApiService, + mockRiskInsightsEncryptionService, ); }); it("should generate the raw data report correctly", async () => { - const result = await firstValueFrom(service.generateRawDataReport$("orgId")); + const result = await firstValueFrom(service.generateRawDataReport$(orgId)); expect(result).toHaveLength(6); @@ -67,7 +81,7 @@ describe("RiskInsightsReportService", () => { }); it("should generate the raw data + uri report correctly", async () => { - const result = await firstValueFrom(service.generateRawDataUriReport$("orgId")); + const result = await firstValueFrom(service.generateRawDataUriReport$(orgId)); expect(result).toHaveLength(11); @@ -90,7 +104,7 @@ describe("RiskInsightsReportService", () => { }); it("should generate applications health report data correctly", async () => { - const result = await firstValueFrom(service.generateApplicationsReport$("orgId")); + const result = await firstValueFrom(service.generateApplicationsReport$(orgId)); expect(result).toHaveLength(8); @@ -131,7 +145,7 @@ describe("RiskInsightsReportService", () => { }); it("should generate applications summary data correctly", async () => { - const reportResult = await firstValueFrom(service.generateApplicationsReport$("orgId")); + const reportResult = await firstValueFrom(service.generateApplicationsReport$(orgId)); const reportSummary = service.generateApplicationsSummary(reportResult); expect(reportSummary.totalMemberCount).toEqual(7); @@ -139,4 +153,104 @@ describe("RiskInsightsReportService", () => { expect(reportSummary.totalApplicationCount).toEqual(8); expect(reportSummary.totalAtRiskApplicationCount).toEqual(7); }); + + describe("saveRiskInsightsReport", () => { + it("should not update subjects if save response does not have id", async () => { + const organizationId = "orgId" as OrganizationId; + const userId = "userId" as UserId; + const report = [{ applicationName: "app1" }] as any; + + const encryptedReport = { + organizationId: organizationId as OrganizationId, + encryptedData: "encryptedData" as EncryptedString, + encryptionKey: "encryptionKey" as EncryptedString, + }; + + mockRiskInsightsEncryptionService.encryptRiskInsightsReport.mockResolvedValue( + encryptedReport, + ); + + const saveResponse = { id: "" }; // Simulating no ID in response + mockRiskInsightsApiService.saveRiskInsightsReport.mockReturnValue(of(saveResponse)); + + const reportSubjectSpy = jest.spyOn((service as any).riskInsightsReportSubject, "next"); + const summarySubjectSpy = jest.spyOn((service as any).riskInsightsSummarySubject, "next"); + + await service.saveRiskInsightsReport(organizationId, userId, report); + + expect(reportSubjectSpy).not.toHaveBeenCalled(); + expect(summarySubjectSpy).not.toHaveBeenCalled(); + }); + }); + + describe("getRiskInsightsReport", () => { + beforeEach(() => { + // Reset the mocks before each test + jest.clearAllMocks(); + }); + + it("should call riskInsightsApiService.getRiskInsightsReport with the correct organizationId", () => { + // we need to ensure that the api is invoked with the specified organizationId + // here it doesn't matter what the Api returns + const apiResponse = { + id: "reportId", + date: new Date().toISOString(), + organizationId: "orgId", + reportData: "encryptedReportData", + reportKey: "encryptionKey", + } as GetRiskInsightsReportResponse; + + const organizationId = "orgId" as OrganizationId; + const userId = "userId" as UserId; + mockRiskInsightsApiService.getRiskInsightsReport.mockReturnValue(of(apiResponse)); + service.getRiskInsightsReport(organizationId, userId); + expect(mockRiskInsightsApiService.getRiskInsightsReport).toHaveBeenCalledWith(organizationId); + expect(mockRiskInsightsEncryptionService.decryptRiskInsightsReport).toHaveBeenCalledWith( + organizationId, + userId, + expect.anything(), // encryptedData + expect.anything(), // wrappedKey + expect.any(Function), // parser + ); + }); + + it("should decrypt report and update subjects if response is present", async () => { + // Arrange: setup a mock response from the API + // and ensure the decryption service is called with the correct parameters + const organizationId = "orgId" as OrganizationId; + const userId = "userId" as UserId; + + const mockResponse = { + id: "reportId", + date: new Date().toISOString(), + organizationId: organizationId as OrganizationId, + reportData: "encryptedReportData", + reportKey: "encryptionKey", + } as GetRiskInsightsReportResponse; + + const decryptedReport = { + data: [{ foo: "bar" }], + }; + mockRiskInsightsApiService.getRiskInsightsReport.mockReturnValue(of(mockResponse)); + mockRiskInsightsEncryptionService.decryptRiskInsightsReport.mockResolvedValue( + decryptedReport, + ); + + const reportSubjectSpy = jest.spyOn((service as any).riskInsightsReportSubject, "next"); + + service.getRiskInsightsReport(organizationId, userId); + + // Wait for all microtasks to complete + await Promise.resolve(); + + expect(mockRiskInsightsEncryptionService.decryptRiskInsightsReport).toHaveBeenCalledWith( + organizationId, + userId, + expect.anything(), + expect.anything(), + expect.any(Function), + ); + expect(reportSubjectSpy).toHaveBeenCalledWith(decryptedReport.data); + }); + }); }); diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.ts index 182e8aa6882..23471327fe0 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.ts @@ -1,10 +1,23 @@ // FIXME: Update this file to be type safe // @ts-strict-ignore -import { concatMap, first, from, map, Observable, zip } from "rxjs"; +import { + BehaviorSubject, + concatMap, + first, + firstValueFrom, + from, + map, + Observable, + of, + switchMap, + zip, +} from "rxjs"; import { AuditService } from "@bitwarden/common/abstractions/audit.service"; +import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; +import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; @@ -21,9 +34,12 @@ import { WeakPasswordDetail, WeakPasswordScore, ApplicationHealthReportDetailWithCriticalFlagAndCipher, + ReportInsightsReportData, } from "../models/password-health"; import { MemberCipherDetailsApiService } from "./member-cipher-details-api.service"; +import { RiskInsightsApiService } from "./risk-insights-api.service"; +import { RiskInsightsEncryptionService } from "./risk-insights-encryption.service"; export class RiskInsightsReportService { constructor( @@ -31,8 +47,21 @@ export class RiskInsightsReportService { private auditService: AuditService, private cipherService: CipherService, private memberCipherDetailsApiService: MemberCipherDetailsApiService, + private riskInsightsApiService: RiskInsightsApiService, + private riskInsightsEncryptionService: RiskInsightsEncryptionService, ) {} + private riskInsightsReportSubject = new BehaviorSubject([]); + riskInsightsReport$ = this.riskInsightsReportSubject.asObservable(); + + private riskInsightsSummarySubject = new BehaviorSubject({ + totalMemberCount: 0, + totalAtRiskMemberCount: 0, + totalApplicationCount: 0, + totalAtRiskApplicationCount: 0, + }); + riskInsightsSummary$ = this.riskInsightsSummarySubject.asObservable(); + /** * Report data from raw cipher health data. * Can be used in the Raw Data diagnostic tab (just exclude the members in the view) @@ -40,7 +69,7 @@ export class RiskInsightsReportService { * @param organizationId * @returns Cipher health report data with members and trimmed uris */ - generateRawDataReport$(organizationId: string): Observable { + generateRawDataReport$(organizationId: OrganizationId): Observable { const allCiphers$ = from(this.cipherService.getAllFromApiForOrganization(organizationId)); const memberCiphers$ = from( this.memberCipherDetailsApiService.getMemberCipherDetails(organizationId), @@ -68,7 +97,9 @@ export class RiskInsightsReportService { * @param organizationId Id of the organization * @returns Cipher health report data flattened to the uris */ - generateRawDataUriReport$(organizationId: string): Observable { + generateRawDataUriReport$( + organizationId: OrganizationId, + ): Observable { const cipherHealthDetails$ = this.generateRawDataReport$(organizationId); const results$ = cipherHealthDetails$.pipe( map((healthDetails) => this.getCipherUriDetails(healthDetails)), @@ -84,7 +115,9 @@ export class RiskInsightsReportService { * @param organizationId Id of the organization * @returns The all applications health report data */ - generateApplicationsReport$(organizationId: string): Observable { + generateApplicationsReport$( + organizationId: OrganizationId, + ): Observable { const cipherHealthUriReport$ = this.generateRawDataUriReport$(organizationId); const results$ = cipherHealthUriReport$.pipe( map((uriDetails) => this.getApplicationHealthReport(uriDetails)), @@ -167,7 +200,7 @@ export class RiskInsightsReportService { async identifyCiphers( data: ApplicationHealthReportDetail[], - organizationId: string, + organizationId: OrganizationId, ): Promise { const cipherViews = await this.cipherService.getAllFromApiForOrganization(organizationId); @@ -181,6 +214,75 @@ export class RiskInsightsReportService { return dataWithCiphers; } + getRiskInsightsReport(organizationId: OrganizationId, userId: UserId): void { + this.riskInsightsApiService + .getRiskInsightsReport(organizationId) + .pipe( + switchMap((response) => { + if (!response) { + // Return an empty report and summary if response is falsy + return of({ + data: [], + summary: { + totalMemberCount: 0, + totalAtRiskMemberCount: 0, + totalApplicationCount: 0, + totalAtRiskApplicationCount: 0, + }, + }); + } + return from( + this.riskInsightsEncryptionService.decryptRiskInsightsReport( + organizationId, + userId, + new EncString(response.reportData), + new EncString(response.reportKey), + (data) => data as ReportInsightsReportData, + ), + ); + }), + ) + .subscribe({ + next: (decryptRiskInsightsReport) => { + this.riskInsightsReportSubject.next(decryptRiskInsightsReport.data); + this.riskInsightsSummarySubject.next(decryptRiskInsightsReport.summary); + }, + }); + } + + async saveRiskInsightsReport( + organizationId: OrganizationId, + userId: UserId, + report: ApplicationHealthReportDetail[], + ): Promise { + const riskReport = { + data: report, + }; + + const encryptedReport = await this.riskInsightsEncryptionService.encryptRiskInsightsReport( + organizationId, + userId, + riskReport, + ); + + const saveRequest = { + data: { + organizationId: organizationId, + date: new Date().toISOString(), + reportData: encryptedReport.encryptedData, + reportKey: encryptedReport.encryptionKey, + }, + }; + + const response = await firstValueFrom( + this.riskInsightsApiService.saveRiskInsightsReport(saveRequest, organizationId), + ); + + if (response && response.id) { + this.riskInsightsReportSubject.next(report); + } + } + /** * Associates the members with the ciphers they have access to. Calculates the password health. * Finds the trimmed uris. diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/access-intelligence.module.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/access-intelligence.module.ts index 25baf2e0fed..0fe1737bde3 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/access-intelligence.module.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/access-intelligence.module.ts @@ -8,8 +8,10 @@ import { RiskInsightsDataService, RiskInsightsReportService, } from "@bitwarden/bit-common/dirt/reports/risk-insights/services"; +import { RiskInsightsEncryptionService } from "@bitwarden/bit-common/dirt/reports/risk-insights/services/risk-insights-encryption.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AuditService } from "@bitwarden/common/abstractions/audit.service"; +import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength/password-strength.service.abstraction"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; @@ -38,6 +40,11 @@ import { RiskInsightsComponent } from "./risk-insights.component"; provide: RiskInsightsDataService, deps: [RiskInsightsReportService], }, + { + provide: RiskInsightsEncryptionService, + useClass: RiskInsightsEncryptionService, + deps: [KeyService, EncryptService, KeyGenerationService], + }, safeProvider({ provide: CriticalAppsService, useClass: CriticalAppsService, diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications.component.ts index 75712c51a3d..e8a69ddec25 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications.component.ts @@ -16,6 +16,7 @@ import { ApplicationHealthReportDetailWithCriticalFlagAndCipher, ApplicationHealthReportSummary, } from "@bitwarden/bit-common/dirt/reports/risk-insights/models/password-health"; +import { RiskInsightsEncryptionService } from "@bitwarden/bit-common/dirt/reports/risk-insights/services/risk-insights-encryption.service"; import { getOrganizationById, OrganizationService, @@ -108,7 +109,7 @@ export class AllApplicationsComponent implements OnInit { if (data && organization) { const dataWithCiphers = await this.reportService.identifyCiphers( data, - organization.id, + organization.id as OrganizationId, ); return { @@ -145,6 +146,7 @@ export class AllApplicationsComponent implements OnInit { protected reportService: RiskInsightsReportService, private accountService: AccountService, protected criticalAppsService: CriticalAppsService, + protected riskInsightsEncryptionService: RiskInsightsEncryptionService, ) { this.searchControl.valueChanges .pipe(debounceTime(200), takeUntilDestroyed()) diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/critical-applications.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/critical-applications.component.ts index a94cc6d1441..2d28d38c29c 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/critical-applications.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/critical-applications.component.ts @@ -56,15 +56,18 @@ export class CriticalApplicationsComponent implements OnInit { protected searchControl = new FormControl("", { nonNullable: true }); private destroyRef = inject(DestroyRef); protected loading = false; - protected organizationId: string; + protected organizationId: OrganizationId; protected applicationSummary = {} as ApplicationHealthReportSummary; noItemsIcon = Security; enableRequestPasswordChange = false; async ngOnInit() { - this.organizationId = this.activatedRoute.snapshot.paramMap.get("organizationId") ?? ""; + this.organizationId = this.activatedRoute.snapshot.paramMap.get( + "organizationId", + ) as OrganizationId; const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); this.criticalAppsService.setOrganizationId(this.organizationId as OrganizationId, userId); + // this.criticalAppsService.setOrganizationId(this.organizationId as OrganizationId); combineLatest([ this.dataService.applications$, this.criticalAppsService.getAppsListForOrg(this.organizationId as OrganizationId), diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.ts index e2fb4469ec4..0b32d311e41 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.ts @@ -67,7 +67,7 @@ export class RiskInsightsComponent implements OnInit { criticalAppsCount: number = 0; notifiedMembersCount: number = 0; - private organizationId: string | null = null; + private organizationId: OrganizationId = "" as OrganizationId; private destroyRef = inject(DestroyRef); isLoading$: Observable = new Observable(); isRefreshing$: Observable = new Observable(); @@ -94,10 +94,10 @@ export class RiskInsightsComponent implements OnInit { .pipe( takeUntilDestroyed(this.destroyRef), map((params) => params.get("organizationId")), - switchMap((orgId: string | null) => { + switchMap((orgId) => { if (orgId) { - this.organizationId = orgId; - this.dataService.fetchApplicationsReport(orgId); + this.organizationId = orgId as OrganizationId; + this.dataService.fetchApplicationsReport(this.organizationId); this.isLoading$ = this.dataService.isLoading$; this.isRefreshing$ = this.dataService.isRefreshing$; this.dataLastUpdated$ = this.dataService.dataLastUpdated$; diff --git a/libs/common/src/types/guid.ts b/libs/common/src/types/guid.ts index 5a6aaf2ce51..94aa44d64ba 100644 --- a/libs/common/src/types/guid.ts +++ b/libs/common/src/types/guid.ts @@ -20,6 +20,7 @@ export type OrganizationIntegrationConfigurationId = Opaque< string, "OrganizationIntegrationConfigurationId" >; +export type OrganizationReportId = Opaque; /** * A string representation of an empty guid.