From 4dce7a52cfaa13aa10bf7182a7b366d01b488b5a Mon Sep 17 00:00:00 2001 From: voommen-livefront Date: Mon, 18 Nov 2024 10:07:31 -0600 Subject: [PATCH] PM-14927 Refactor critical app service and render counts --- .../all-applications.component.ts | 58 +------- .../risk-insights/risk-insights.component.ts | 10 +- .../critical-apps-api.service.spec.ts | 134 ++++++++++++++++++ .../services/critical-apps-api.service.ts | 106 ++++++++++++++ .../services/critical-apps.service.spec.ts | 68 --------- .../services/critical-apps.service.ts | 60 -------- .../reports/risk-insights/services/index.ts | 2 +- 7 files changed, 255 insertions(+), 183 deletions(-) create mode 100644 bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps-api.service.spec.ts create mode 100644 bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps-api.service.ts delete mode 100644 bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps.service.spec.ts delete mode 100644 bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps.service.ts diff --git a/apps/web/src/app/tools/risk-insights/all-applications.component.ts b/apps/web/src/app/tools/risk-insights/all-applications.component.ts index cebf479e834..02b63edb57c 100644 --- a/apps/web/src/app/tools/risk-insights/all-applications.component.ts +++ b/apps/web/src/app/tools/risk-insights/all-applications.component.ts @@ -5,19 +5,13 @@ import { ActivatedRoute } from "@angular/router"; import { debounceTime, firstValueFrom, map, switchMap } from "rxjs"; // eslint-disable-next-line no-restricted-imports -import { - CriticalAppsService, - PasswordHealthReportApplicationsRequest, - PasswordHealthReportApplicationsResponse, -} from "@bitwarden/bit-common/tools/reports/risk-insights"; +import { CriticalAppsApiService } from "@bitwarden/bit-common/tools/reports/risk-insights"; import { AuditService } from "@bitwarden/common/abstractions/audit.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; -import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; @@ -28,7 +22,6 @@ import { TableDataSource, ToastService, } from "@bitwarden/components"; -import { KeyService } from "@bitwarden/key-management"; import { CardComponent } from "@bitwarden/tools-card"; import { HeaderModule } from "../../layouts/header/header.module"; @@ -53,7 +46,6 @@ export class AllApplicationsComponent implements OnInit { noItemsIcon = Icons.Security; protected markingAsCritical = false; isCritialAppsFeatureEnabled = false; - private flaggedCriticalApps: PasswordHealthReportApplicationsResponse[] = []; // MOCK DATA protected mockData = applicationTableMockData; @@ -74,17 +66,7 @@ export class AllApplicationsComponent implements OnInit { }), switchMap(async (params) => { const organizationId = (await params).get("organizationId"); - const result = await this.criticalAppsService.getCriticalApps(organizationId); - const key = await this.keyService.getOrgKey(this.organization.id); - const flaggedCriticalAppsPromise = result.map(async (r) => { - const decryptedUrl = await this.encryptService.decryptToUtf8(new EncString(r.uri), key); - return { - id: r.id, - organizationId: r.organizationId, - uri: decryptedUrl, - } as PasswordHealthReportApplicationsResponse; - }); - this.flaggedCriticalApps = await Promise.all(flaggedCriticalAppsPromise); + await this.criticalAppsService.getCriticalApps(organizationId); }), ) .subscribe(); @@ -103,9 +85,7 @@ export class AllApplicationsComponent implements OnInit { protected toastService: ToastService, protected organizationService: OrganizationService, protected configService: ConfigService, - protected criticalAppsService: CriticalAppsService, - private keyService: KeyService, - private encryptService: EncryptService, + protected criticalAppsService: CriticalAppsApiService, ) { this.dataSource.data = applicationTableMockData; this.searchControl.valueChanges @@ -124,38 +104,10 @@ export class AllApplicationsComponent implements OnInit { markAppsAsCritical = async () => { this.markingAsCritical = true; - const key = await this.keyService.getOrgKey(this.organization.id); - - // only save records that are not already in the database - const newEntries = Array.from(this.selectedUrls).filter((url) => { - return !this.flaggedCriticalApps.some((r) => r.uri === url); - }); - - const criticalAppsPromises = newEntries.map(async (url) => { - const encryptedUrlName = await this.encryptService.encrypt(url, key); - return { - organizationId: this.organization.id, - url: encryptedUrlName.encryptedString.toString(), - } as PasswordHealthReportApplicationsRequest; - }); - - const criticalApps = await Promise.all(criticalAppsPromises); await this.criticalAppsService - .setCriticalApps(criticalApps) - .then((result) => { - // append to flaggedCriticalApps - result - .filter((r) => !this.flaggedCriticalApps.some((f) => f.uri === r.uri)) - .forEach(async (r) => { - const decryptedUrl = await this.encryptService.decryptToUtf8(new EncString(r.uri), key); - this.flaggedCriticalApps.push({ - id: r.id, - organizationId: r.organizationId, - uri: decryptedUrl, - } as PasswordHealthReportApplicationsResponse); - }); - + .setCriticalApps(this.organization.id, Array.from(this.selectedUrls)) + .then(() => { this.toastService.showToast({ variant: "success", title: null, diff --git a/apps/web/src/app/tools/risk-insights/risk-insights.component.ts b/apps/web/src/app/tools/risk-insights/risk-insights.component.ts index 1c6a36b4454..a84d1682aee 100644 --- a/apps/web/src/app/tools/risk-insights/risk-insights.component.ts +++ b/apps/web/src/app/tools/risk-insights/risk-insights.component.ts @@ -4,6 +4,8 @@ import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { ActivatedRoute, Router } from "@angular/router"; import { JslibModule } from "@bitwarden/angular/jslib.module"; +// eslint-disable-next-line no-restricted-imports +import { CriticalAppsApiService } from "@bitwarden/bit-common/tools/reports/risk-insights"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { AsyncActionsModule, ButtonModule, TabsModule } from "@bitwarden/components"; @@ -11,6 +13,7 @@ import { AsyncActionsModule, ButtonModule, TabsModule } from "@bitwarden/compone import { HeaderModule } from "../../layouts/header/header.module"; import { AllApplicationsComponent } from "./all-applications.component"; +import { applicationTableMockData } from "./application-table.mock"; import { CriticalApplicationsComponent } from "./critical-applications.component"; import { NotifiedMembersTableComponent } from "./notified-members-table.component"; import { PasswordHealthMembersURIComponent } from "./password-health-members-uri.component"; @@ -46,7 +49,7 @@ export class RiskInsightsComponent implements OnInit { dataLastUpdated = new Date(); isCritialAppsFeatureEnabled = false; - apps: any[] = []; + apps: any[] = applicationTableMockData; criticalApps: any[] = []; notifiedMembers: any[] = []; @@ -78,9 +81,14 @@ export class RiskInsightsComponent implements OnInit { protected route: ActivatedRoute, private router: Router, private configService: ConfigService, + private criticalAppsApiService: CriticalAppsApiService, ) { route.queryParams.pipe(takeUntilDestroyed()).subscribe(({ tabIndex }) => { this.tabIndex = !isNaN(tabIndex) ? tabIndex : RiskInsightsTabType.AllApps; }); + + this.criticalAppsApiService.criticalApps$.pipe(takeUntilDestroyed()).subscribe((apps) => { + this.criticalApps = apps; + }); } } diff --git a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps-api.service.spec.ts b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps-api.service.spec.ts new file mode 100644 index 00000000000..9492bad4674 --- /dev/null +++ b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps-api.service.spec.ts @@ -0,0 +1,134 @@ +import { randomUUID } from "crypto"; + +import { TestBed } from "@angular/core/testing"; +import { mock } from "jest-mock-extended"; + +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; +import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; +import { Guid } from "@bitwarden/common/types/guid"; +import { KeyService } from "@bitwarden/key-management"; + +import { + CriticalAppsApiService, + PasswordHealthReportApplicationsRequest, + PasswordHealthReportApplicationsResponse, +} from "./critical-apps-api.service"; + +describe("CriticalAppsApiService", () => { + let service: CriticalAppsApiService; + const apiService = mock(); + const keyService = mock(); + const encryptService = mock(); + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [ + CriticalAppsApiService, + { provide: ApiService, useValue: apiService }, + { provide: KeyService, useValue: keyService }, + { provide: EncryptService, useValue: encryptService }, + ], + }); + service = TestBed.inject(CriticalAppsApiService); + + // reset mocks + jest.resetAllMocks(); + }); + + it("should be created", () => { + expect(service).toBeTruthy(); + }); + + it("should set critical apps", async () => { + // arrange + const criticalApps = ["https://example.com", "https://example.org"]; + + const request = [ + { organizationId: "org1", url: "encryptedUrlName" }, + { organizationId: "org1", url: "encryptedUrlName" }, + ] as PasswordHealthReportApplicationsRequest[]; + + const response = [ + { id: "id1", organizationId: "org1", uri: "https://example.com" }, + { id: "id2", organizationId: "org1", uri: "https://example.org" }, + ] as PasswordHealthReportApplicationsResponse[]; + + encryptService.encrypt.mockResolvedValue(new EncString("encryptedUrlName")); + apiService.send.mockResolvedValue(response); + + // act + await service.setCriticalApps("org1", criticalApps); + + // expectations + expect(keyService.getOrgKey).toHaveBeenCalledWith("org1"); + expect(encryptService.encrypt).toHaveBeenCalledTimes(2); + expect(apiService.send).toHaveBeenCalledWith( + "POST", + "/reports/password-health-report-applications/", + request, + true, + true, + ); + }); + + it("should exclude records that already exist", async () => { + // arrange + // one record already exists + service.criticalApps = [ + { id: randomUUID() as Guid, organizationId: "org1" as Guid, uri: "https://example.com" }, + ]; + + // two records are selected - one already in the database + const selectedUrls = ["https://example.com", "https://example.org"]; + + // expect only one record to be sent to the server + const request = [ + { organizationId: "org1", url: "encryptedUrlName" }, + ] as PasswordHealthReportApplicationsRequest[]; + + // mocked response + const response = [ + { id: "id1", organizationId: "org1", uri: "test" }, + ] as PasswordHealthReportApplicationsResponse[]; + + encryptService.encrypt.mockResolvedValue(new EncString("encryptedUrlName")); + apiService.send.mockResolvedValue(response); + + // act + await service.setCriticalApps("org1", selectedUrls); + + // expectations + expect(keyService.getOrgKey).toHaveBeenCalledWith("org1"); + expect(encryptService.encrypt).toHaveBeenCalledTimes(1); + expect(apiService.send).toHaveBeenCalledWith( + "POST", + "/reports/password-health-report-applications/", + request, + true, + true, + ); + }); + + it("should get critical apps", async () => { + const orgId = "org1"; + const response = [ + { id: "id1", organizationId: "org1", uri: "https://example.com" }, + { id: "id2", organizationId: "org1", uri: "https://example.org" }, + ] as PasswordHealthReportApplicationsResponse[]; + + encryptService.decryptToUtf8.mockResolvedValue("https://example.com"); + apiService.send.mockResolvedValue(response); + await service.getCriticalApps(orgId); + + expect(keyService.getOrgKey).toHaveBeenCalledWith(orgId); + expect(encryptService.decryptToUtf8).toHaveBeenCalledTimes(2); + expect(apiService.send).toHaveBeenCalledWith( + "GET", + `/reports/password-health-report-applications/${orgId}`, + null, + true, + true, + ); + }); +}); diff --git a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps-api.service.ts b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps-api.service.ts new file mode 100644 index 00000000000..f1502e8c204 --- /dev/null +++ b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps-api.service.ts @@ -0,0 +1,106 @@ +import { Injectable } from "@angular/core"; +import { BehaviorSubject, Observable } from "rxjs"; + +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; +import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; +import { Guid } from "@bitwarden/common/types/guid"; +import { KeyService } from "@bitwarden/key-management"; + +@Injectable({ + providedIn: "root", +}) +export class CriticalAppsApiService { + private criticalAppsList = new BehaviorSubject([]); + + constructor( + private apiService: ApiService, + private keyService: KeyService, + private encryptService: EncryptService, + ) {} + + get criticalApps$(): Observable { + return this.criticalAppsList.asObservable(); + } + + set criticalApps(value: PasswordHealthReportApplicationsResponse[]) { + this.criticalAppsList.next(value); + } + + async setCriticalApps(orgId: string, selectedUrls: string[]) { + const key = await this.keyService.getOrgKey(orgId); + + // only save records that are not already in the database + const newEntries = Array.from(selectedUrls).filter((url) => { + return !this.criticalAppsList.value.some((r) => r.uri === url); + }); + + const criticalAppsPromises = newEntries.map(async (url) => { + const encryptedUrlName = await this.encryptService.encrypt(url, key); + return { + organizationId: orgId, + url: encryptedUrlName.encryptedString.toString(), + } as PasswordHealthReportApplicationsRequest; + }); + + const criticalAppsRequests = await Promise.all(criticalAppsPromises); + + await this.apiService + .send( + "POST", + "/reports/password-health-report-applications/", + criticalAppsRequests, + true, + true, + ) + .then((result: PasswordHealthReportApplicationsResponse[]) => { + result.forEach(async (r) => { + const decryptedUrl = await this.encryptService.decryptToUtf8(new EncString(r.uri), key); + if (!this.criticalAppsList.value.some((f) => f.uri === decryptedUrl)) { + this.criticalAppsList.value.push({ + id: r.id, + organizationId: r.organizationId, + uri: decryptedUrl, + } as PasswordHealthReportApplicationsResponse); + } + }); + }); + } + + async getCriticalApps(orgId: string): Promise { + const response = await this.apiService.send( + "GET", + `/reports/password-health-report-applications/${orgId}`, + null, + true, + true, + ); + + this.criticalAppsList.next([]); + const key = await this.keyService.getOrgKey(orgId); + + await Promise.all( + response.map(async (r: { id: any; organizationId: any; uri: any }) => { + const decryptedUrl = await this.encryptService.decryptToUtf8(new EncString(r.uri), key); + this.criticalAppsList.value.push({ + id: r.id, + organizationId: r.organizationId, + uri: decryptedUrl, + } as PasswordHealthReportApplicationsResponse); + }), + ); + + return this.criticalAppsList.value; + } +} + +export interface PasswordHealthReportApplicationsRequest { + organizationId: Guid; + url: string; +} + +export interface PasswordHealthReportApplicationsResponse { + id: Guid; + organizationId: Guid; + uri: string; +} diff --git a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps.service.spec.ts b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps.service.spec.ts deleted file mode 100644 index 2e9a9c33e7f..00000000000 --- a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps.service.spec.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { TestBed } from "@angular/core/testing"; -import { mock } from "jest-mock-extended"; - -import { ApiService } from "@bitwarden/common/abstractions/api.service"; - -import { - CriticalAppsService, - PasswordHealthReportApplicationsRequest, - PasswordHealthReportApplicationsResponse, -} from "./critical-apps.service"; - -describe("CriticaAppsService", () => { - let service: CriticalAppsService; - const apiService = mock(); - - beforeEach(() => { - TestBed.configureTestingModule({ - providers: [CriticalAppsService, { provide: ApiService, useValue: apiService }], - }); - service = TestBed.inject(CriticalAppsService); - }); - - it("should be created", () => { - expect(service).toBeTruthy(); - }); - - it("should set critical apps", async () => { - const criticalApps = [ - { organizationId: "org1", url: "https://example.com" }, - { organizationId: "org2", url: "https://example.org" }, - ] as PasswordHealthReportApplicationsRequest[]; - - const response = [ - { id: "id1", organizationId: "org1", uri: "https://example.com" }, - { id: "id2", organizationId: "org2", uri: "https://example.org" }, - ] as PasswordHealthReportApplicationsResponse[]; - - apiService.send.mockResolvedValue(response); - await service.setCriticalApps(criticalApps); - - expect(apiService.send).toHaveBeenCalledWith( - "POST", - "/reports/password-health-report-applications/", - criticalApps, - true, - true, - ); - }); - - it("should get critical apps", async () => { - const orgId = "org1"; - const response = [ - { id: "id1", organizationId: "org1", uri: "https://example.com" }, - { id: "id2", organizationId: "org2", uri: "https://example.org" }, - ] as PasswordHealthReportApplicationsResponse[]; - - apiService.send.mockResolvedValue(response); - await service.getCriticalApps(orgId); - - expect(apiService.send).toHaveBeenCalledWith( - "GET", - `/reports/password-health-report-applications/${orgId}`, - null, - true, - true, - ); - }); -}); diff --git a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps.service.ts b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps.service.ts deleted file mode 100644 index f1fb8dd3a09..00000000000 --- a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/critical-apps.service.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { Injectable } from "@angular/core"; - -import { ApiService } from "@bitwarden/common/abstractions/api.service"; -import { Guid } from "@bitwarden/common/types/guid"; - -@Injectable({ - providedIn: "root", -}) -export class CriticalAppsService { - constructor(private apiService: ApiService) {} - - async setCriticalApps( - criticalApps: PasswordHealthReportApplicationsRequest[], - ): Promise { - const response = await this.apiService.send( - "POST", - "/reports/password-health-report-applications/", - criticalApps, - true, - true, - ); - - return response.map((r: { id: any; organizationId: any; uri: any }) => { - return { - id: r.id, - organizationId: r.organizationId, - uri: r.uri, - }; - }); - } - - async getCriticalApps(orgId: string): Promise { - const response = await this.apiService.send( - "GET", - `/reports/password-health-report-applications/${orgId}`, - null, - true, - true, - ); - - return response.map((r: { id: any; organizationId: any; uri: any }) => { - return { - id: r.id, - organizationId: r.organizationId, - uri: r.uri, - }; - }); - } -} - -export interface PasswordHealthReportApplicationsRequest { - organizationId: Guid; - url: string; -} - -export interface PasswordHealthReportApplicationsResponse { - id: Guid; - organizationId: Guid; - uri: string; -} diff --git a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/index.ts b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/index.ts index aa34a8b6f43..3d22835a2c9 100644 --- a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/index.ts +++ b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/index.ts @@ -1,3 +1,3 @@ export * from "./member-cipher-details-api.service"; export * from "./password-health.service"; -export * from "./critical-apps.service"; +export * from "./critical-apps-api.service";