From d091d6f8ccb6616732b506c9b80b982b2e44ce96 Mon Sep 17 00:00:00 2001 From: voommen-livefront Date: Thu, 5 Dec 2024 15:10:10 -0600 Subject: [PATCH] PM-14927 Pr comments incorporated --- .../critical-apps-api.service.spec.ts | 28 ++++-- .../services/critical-apps-api.service.ts | 95 +++++++++++++------ .../all-applications.component.ts | 11 ++- .../risk-insights.component.html | 4 +- .../risk-insights.component.ts | 18 ++-- libs/common/src/types/guid.ts | 1 + 6 files changed, 101 insertions(+), 56 deletions(-) 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 index 9492bad4674..df64a45ec21 100644 --- 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 @@ -1,12 +1,12 @@ import { randomUUID } from "crypto"; -import { TestBed } from "@angular/core/testing"; +import { fakeAsync, flush, 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 { OrganizationId, PasswordHealthReportApplicationId } from "@bitwarden/common/types/guid"; import { KeyService } from "@bitwarden/key-management"; import { @@ -75,9 +75,13 @@ describe("CriticalAppsApiService", () => { 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" }, - ]; + service.setAppsInListForOrg([ + { + id: randomUUID() as PasswordHealthReportApplicationId, + organizationId: "org1" as OrganizationId, + uri: "https://example.com", + }, + ]); // two records are selected - one already in the database const selectedUrls = ["https://example.com", "https://example.org"]; @@ -110,8 +114,8 @@ describe("CriticalAppsApiService", () => { ); }); - it("should get critical apps", async () => { - const orgId = "org1"; + it("should get critical apps", fakeAsync(() => { + const orgId = "org1" as OrganizationId; const response = [ { id: "id1", organizationId: "org1", uri: "https://example.com" }, { id: "id2", organizationId: "org1", uri: "https://example.org" }, @@ -119,9 +123,13 @@ describe("CriticalAppsApiService", () => { encryptService.decryptToUtf8.mockResolvedValue("https://example.com"); apiService.send.mockResolvedValue(response); - await service.getCriticalApps(orgId); + const spy = jest.spyOn(service, "retrieveCriticalApps"); - expect(keyService.getOrgKey).toHaveBeenCalledWith(orgId); + service.setOrganizationId(orgId as OrganizationId); + flush(); + + expect(spy).toHaveBeenCalled(); + expect(keyService.getOrgKey).toHaveBeenCalledWith(orgId.toString()); expect(encryptService.decryptToUtf8).toHaveBeenCalledTimes(2); expect(apiService.send).toHaveBeenCalledWith( "GET", @@ -130,5 +138,5 @@ describe("CriticalAppsApiService", () => { 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 index c04c0bc3e06..168e615937d 100644 --- 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 @@ -1,10 +1,11 @@ -import { Injectable } from "@angular/core"; -import { BehaviorSubject, firstValueFrom, Observable } from "rxjs"; +import { Injectable, OnDestroy } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; +import { BehaviorSubject, firstValueFrom, map, 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 { OrganizationId, PasswordHealthReportApplicationId } from "@bitwarden/common/types/guid"; import { OrgKey } from "@bitwarden/common/types/key"; import { KeyService } from "@bitwarden/key-management"; @@ -14,23 +15,34 @@ import { KeyService } from "@bitwarden/key-management"; /* Retrieves and decrypts critical apps for a given organization * Encrypts and saves data for a given organization */ -export class CriticalAppsApiService { +export class CriticalAppsApiService implements OnDestroy { + private orgId = new BehaviorSubject(null); private criticalAppsList = new BehaviorSubject([]); + private fetchOrg$ = this.orgId + .pipe(takeUntilDestroyed()) + .subscribe((orgId) => this.retrieveCriticalApps(orgId)); + + ngOnDestroy(): void { + this.fetchOrg$.unsubscribe(); + } + constructor( private apiService: ApiService, private keyService: KeyService, private encryptService: EncryptService, ) {} - // Get a list of critical apps - get criticalApps$(): Observable { - return this.criticalAppsList.asObservable(); + // Get a list of critical apps for a given organization + getAppsListForOrg(orgId: string): Observable { + return this.criticalAppsList + .asObservable() + .pipe(map((apps) => apps.filter((app) => app.organizationId === orgId))); } // Reset the critical apps list - set criticalApps(value: PasswordHealthReportApplicationsResponse[]) { - this.criticalAppsList.next(value); + setAppsInListForOrg(apps: PasswordHealthReportApplicationsResponse[]) { + this.criticalAppsList.next(apps); } // Save the selected critical apps for a given organization @@ -38,8 +50,12 @@ export class CriticalAppsApiService { const key = await this.keyService.getOrgKey(orgId); // only save records that are not already in the database - const newEntries = await this.filterNewEntries(orgId, selectedUrls); - const criticalAppsRequests = await this.encryptNewEntries(orgId, key, newEntries); + const newEntries = await this.filterNewEntries(orgId as OrganizationId, selectedUrls); + const criticalAppsRequests = await this.encryptNewEntries( + orgId as OrganizationId, + key, + newEntries, + ); // save the new entries to the database const dbResponse = await this.apiService.send( @@ -51,25 +67,35 @@ export class CriticalAppsApiService { ); // add the new entries to the criticalAppsList + const updatedList = [...this.criticalAppsList.value]; for (const responseItem of dbResponse) { const decryptedUrl = await this.encryptService.decryptToUtf8( new EncString(responseItem.uri), key, ); - if (!this.criticalAppsList.value.some((f) => f.uri === decryptedUrl)) { - this.criticalAppsList.value.push({ + if (!updatedList.some((f) => f.uri === decryptedUrl)) { + updatedList.push({ id: responseItem.id, organizationId: responseItem.organizationId, uri: decryptedUrl, } as PasswordHealthReportApplicationsResponse); } } + this.criticalAppsList.next(updatedList); } // Get the critical apps for a given organization - async getCriticalApps( - orgId: string, - ): Promise> { + setOrganizationId(orgId: OrganizationId) { + this.orgId.next(orgId); + } + + async retrieveCriticalApps( + orgId: OrganizationId | null, + ): Promise { + if (orgId === null) { + return []; + } + const response = await this.apiService.send( "GET", `/reports/password-health-report-applications/${orgId.toString()}`, @@ -78,24 +104,31 @@ export class CriticalAppsApiService { true, ); - this.criticalAppsList.next([]); const key = await this.keyService.getOrgKey(orgId); + const updatedList: PasswordHealthReportApplicationsResponse[] = []; await Promise.all( - response.map(async (r: { id: Guid; organizationId: Guid; uri: string }) => { - 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); - }), + response.map( + async (r: { + id: PasswordHealthReportApplicationId; + organizationId: OrganizationId; + uri: string; + }) => { + const decryptedUrl = await this.encryptService.decryptToUtf8(new EncString(r.uri), key); + updatedList.push({ + id: r.id, + organizationId: r.organizationId, + uri: decryptedUrl, + } as PasswordHealthReportApplicationsResponse); + }, + ), ); - return this.criticalAppsList.asObservable(); + this.criticalAppsList.next(updatedList); + return updatedList; } - private async filterNewEntries(orgId: string, selectedUrls: string[]): Promise { + private async filterNewEntries(orgId: OrganizationId, selectedUrls: string[]): Promise { return await firstValueFrom(this.criticalAppsList).then((criticalApps) => { const criticalAppsUri = criticalApps .filter((f) => f.organizationId === orgId) @@ -105,7 +138,7 @@ export class CriticalAppsApiService { } private async encryptNewEntries( - orgId: string, + orgId: OrganizationId, key: OrgKey, newEntries: string[], ): Promise { @@ -122,12 +155,12 @@ export class CriticalAppsApiService { } export interface PasswordHealthReportApplicationsRequest { - organizationId: Guid; + organizationId: OrganizationId; url: string; } export interface PasswordHealthReportApplicationsResponse { - id: Guid; - organizationId: Guid; + id: PasswordHealthReportApplicationId; + organizationId: OrganizationId; uri: string; } diff --git a/bitwarden_license/bit-web/src/app/tools/access-intelligence/all-applications.component.ts b/bitwarden_license/bit-web/src/app/tools/access-intelligence/all-applications.component.ts index bb9acd91a53..ea209f62bfa 100644 --- a/bitwarden_license/bit-web/src/app/tools/access-intelligence/all-applications.component.ts +++ b/bitwarden_license/bit-web/src/app/tools/access-intelligence/all-applications.component.ts @@ -28,6 +28,7 @@ import { SharedModule } from "@bitwarden/web-vault/app/shared"; import { PipesModule } from "@bitwarden/web-vault/app/vault/individual-vault/pipes/pipes.module"; import { applicationTableMockData } from "./application-table.mock"; +import { OrganizationId } from "@bitwarden/common/types/guid"; @Component({ standalone: true, @@ -63,12 +64,12 @@ export class AllApplicationsComponent implements OnInit { return params; // TODO: use organizationId to fetch data }), - switchMap(async (params) => { - const organizationId = (await params).get("organizationId"); - await this.criticalAppsService.getCriticalApps(organizationId); - }), + switchMap(async (params) => await params), ) - .subscribe(); + .subscribe((params) => { + const orgId = params.get("organizationId"); + this.criticalAppsService.setOrganizationId(orgId as OrganizationId); + }); this.isCritialAppsFeatureEnabled = await this.configService.getFeatureFlag( FeatureFlag.CriticalApps, diff --git a/bitwarden_license/bit-web/src/app/tools/access-intelligence/risk-insights.component.html b/bitwarden_license/bit-web/src/app/tools/access-intelligence/risk-insights.component.html index 6df47e3c46f..87b47e2f56b 100644 --- a/bitwarden_license/bit-web/src/app/tools/access-intelligence/risk-insights.component.html +++ b/bitwarden_license/bit-web/src/app/tools/access-intelligence/risk-insights.component.html @@ -22,10 +22,10 @@ - + - {{ "criticalApplicationsWithCount" | i18n: criticalApps.length }} + {{ "criticalApplicationsWithCount" | i18n: (criticalApps$ | async)?.length ?? 0 }} diff --git a/bitwarden_license/bit-web/src/app/tools/access-intelligence/risk-insights.component.ts b/bitwarden_license/bit-web/src/app/tools/access-intelligence/risk-insights.component.ts index 6e420f5e34d..4d1d7e4d501 100644 --- a/bitwarden_license/bit-web/src/app/tools/access-intelligence/risk-insights.component.ts +++ b/bitwarden_license/bit-web/src/app/tools/access-intelligence/risk-insights.component.ts @@ -5,7 +5,10 @@ import { ActivatedRoute, Router } from "@angular/router"; import { JslibModule } from "@bitwarden/angular/jslib.module"; // eslint-disable-next-line no-restricted-imports -- used for dependency injection -import { CriticalAppsApiService } from "@bitwarden/bit-common/tools/reports/risk-insights"; +import { + CriticalAppsApiService, + PasswordHealthReportApplicationsResponse, +} 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"; @@ -18,6 +21,7 @@ import { NotifiedMembersTableComponent } from "./notified-members-table.componen import { PasswordHealthMembersURIComponent } from "./password-health-members-uri.component"; import { PasswordHealthMembersComponent } from "./password-health-members.component"; import { PasswordHealthComponent } from "./password-health.component"; +import { Observable } from "rxjs"; export enum RiskInsightsTabType { AllApps = 0, @@ -46,10 +50,10 @@ export enum RiskInsightsTabType { export class RiskInsightsComponent implements OnInit { tabIndex: RiskInsightsTabType; dataLastUpdated = new Date(); - isCritialAppsFeatureEnabled = false; + isCriticalAppsFeatureEnabled = false; apps: any[] = applicationTableMockData; - criticalApps: any[] = []; + criticalApps$: Observable = new Observable(); notifiedMembers: any[] = []; async refreshData() { @@ -71,7 +75,7 @@ export class RiskInsightsComponent implements OnInit { }; async ngOnInit() { - this.isCritialAppsFeatureEnabled = await this.configService.getFeatureFlag( + this.isCriticalAppsFeatureEnabled = await this.configService.getFeatureFlag( FeatureFlag.CriticalApps, ); } @@ -85,9 +89,7 @@ export class RiskInsightsComponent implements OnInit { route.queryParams.pipe(takeUntilDestroyed()).subscribe(({ tabIndex }) => { this.tabIndex = !isNaN(tabIndex) ? tabIndex : RiskInsightsTabType.AllApps; }); - - this.criticalAppsApiService.criticalApps$.pipe(takeUntilDestroyed()).subscribe((apps) => { - this.criticalApps = apps; - }); + const orgId = this.route.snapshot.paramMap.get("organizationId"); + this.criticalApps$ = this.criticalAppsApiService.getAppsListForOrg(orgId); } } diff --git a/libs/common/src/types/guid.ts b/libs/common/src/types/guid.ts index 59dbe28f907..07ca88eb7d8 100644 --- a/libs/common/src/types/guid.ts +++ b/libs/common/src/types/guid.ts @@ -10,3 +10,4 @@ export type PolicyId = Opaque; export type CipherId = Opaque; export type SendId = Opaque; export type IndexedEntityId = Opaque; +export type PasswordHealthReportApplicationId = Opaque;