From 12eb77fd45c18a078454b62428f42c8be2947fca Mon Sep 17 00:00:00 2001 From: Daniel James Smith <2670567+djsmith85@users.noreply.github.com> Date: Wed, 18 Dec 2024 17:13:13 +0100 Subject: [PATCH] [PM-13455] Wire up results from RiskInsightsReportService (#12206) * Risk insights aggregation in a new service. Initial PR. * Wire up results from RiskInsightsReportService * Ignoring all non-login items and refactoring into a method * Cleaning up the documentation a little * logic for generating the report summary * application summary to list at risk applications not passwords * Adding more documentation and moving types to it's own file * Awaiting the raw data report and adding the start of the test file * Extend access-intelligence.module to provide needed services * Register access-intelligence.module with bit-web AppModule * Use provided RiskInsightsService instead of new'ing one in the component * Removing the injectable attribute from RiskInsightsReportService * Fix tests * Adding more test cases * Removing unnecessary file * Test cases update * Fixing memeber details test to have new member * Fixing password health tests * Moving to observables * removing commented code * commented code * Switching from ternary to if/else * nullable types * one more nullable type * Adding the fixme for strict types * moving the fixme * No need to access the password use map and switching to the observable * PM-13455 fixes to unit tests --------- Co-authored-by: Tom Co-authored-by: Daniel James Smith Co-authored-by: Tom <144813356+ttalty@users.noreply.github.com> Co-authored-by: voommen-livefront --- .../risk-insights-report.service.spec.ts | 56 ++++++++----------- .../services/risk-insights-report.service.ts | 5 +- .../bit-web/src/app/app.module.ts | 2 + .../access-intelligence.module.ts | 24 ++++++++ .../password-health.component.html | 10 ++-- .../password-health.component.spec.ts | 22 +------- .../password-health.component.ts | 41 +++----------- 7 files changed, 64 insertions(+), 96 deletions(-) diff --git a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/risk-insights-report.service.spec.ts b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/risk-insights-report.service.spec.ts index 7505b692a8f..705eb1231a9 100644 --- a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/risk-insights-report.service.spec.ts +++ b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/risk-insights-report.service.spec.ts @@ -1,5 +1,6 @@ -import { TestBed } from "@angular/core/testing"; +import { mock } from "jest-mock-extended"; import { firstValueFrom } from "rxjs"; +import { ZXCVBNResult } from "zxcvbn"; import { AuditService } from "@bitwarden/common/abstractions/audit.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; @@ -12,42 +13,31 @@ import { RiskInsightsReportService } from "./risk-insights-report.service"; describe("RiskInsightsReportService", () => { let service: RiskInsightsReportService; + const pwdStrengthService = mock(); + const auditService = mock(); + const cipherService = mock(); + const memberCipherDetailsService = mock(); beforeEach(() => { - TestBed.configureTestingModule({ - providers: [ - RiskInsightsReportService, - { - provide: PasswordStrengthServiceAbstraction, - useValue: { - getPasswordStrength: (password: string) => { - const score = password.length < 4 ? 1 : 4; - return { score }; - }, - }, - }, - { - provide: AuditService, - useValue: { - passwordLeaked: (password: string) => Promise.resolve(password === "123" ? 100 : 0), - }, - }, - { - provide: CipherService, - useValue: { - getAllFromApiForOrganization: jest.fn().mockResolvedValue(mockCiphers), - }, - }, - { - provide: MemberCipherDetailsApiService, - useValue: { - getMemberCipherDetails: jest.fn().mockResolvedValue(mockMemberCipherDetails), - }, - }, - ], + pwdStrengthService.getPasswordStrength.mockImplementation((password: string) => { + const score = password.length < 4 ? 1 : 4; + return { score } as ZXCVBNResult; }); - service = TestBed.inject(RiskInsightsReportService); + auditService.passwordLeaked.mockImplementation((password: string) => + Promise.resolve(password === "123" ? 100 : 0), + ); + + cipherService.getAllFromApiForOrganization.mockResolvedValue(mockCiphers); + + memberCipherDetailsService.getMemberCipherDetails.mockResolvedValue(mockMemberCipherDetails); + + service = new RiskInsightsReportService( + pwdStrengthService, + auditService, + cipherService, + memberCipherDetailsService, + ); }); it("should generate the raw data report correctly", async () => { diff --git a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/risk-insights-report.service.ts b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/risk-insights-report.service.ts index f4b30735584..45746cd2f5a 100644 --- a/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/risk-insights-report.service.ts +++ b/bitwarden_license/bit-common/src/tools/reports/risk-insights/services/risk-insights-report.service.ts @@ -1,7 +1,5 @@ -// FIXME: Update this file to be type safe and remove this and next line +// FIXME: Update this file to be type safe // @ts-strict-ignore - -import { Injectable } from "@angular/core"; import { concatMap, first, from, map, Observable, zip } from "rxjs"; import { AuditService } from "@bitwarden/common/abstractions/audit.service"; @@ -24,7 +22,6 @@ import { import { MemberCipherDetailsApiService } from "./member-cipher-details-api.service"; -@Injectable() export class RiskInsightsReportService { constructor( private passwordStrengthService: PasswordStrengthServiceAbstraction, diff --git a/bitwarden_license/bit-web/src/app/app.module.ts b/bitwarden_license/bit-web/src/app/app.module.ts index 4db1e2f5e20..fd1a3b0b84c 100644 --- a/bitwarden_license/bit-web/src/app/app.module.ts +++ b/bitwarden_license/bit-web/src/app/app.module.ts @@ -20,6 +20,7 @@ import { MaximumVaultTimeoutPolicyComponent } from "./admin-console/policies/max import { AppRoutingModule } from "./app-routing.module"; import { AppComponent } from "./app.component"; import { FreeFamiliesSponsorshipPolicyComponent } from "./billing/policies/free-families-sponsorship.component"; +import { AccessIntelligenceModule } from "./tools/access-intelligence/access-intelligence.module"; /** * This is the AppModule for the commercial version of Bitwarden. @@ -41,6 +42,7 @@ import { FreeFamiliesSponsorshipPolicyComponent } from "./billing/policies/free- AppRoutingModule, OssRoutingModule, OrganizationsModule, // Must be after OssRoutingModule for competing routes to resolve properly + AccessIntelligenceModule, RouterModule, WildcardRoutingModule, // Needs to be last to catch all non-existing routes ], diff --git a/bitwarden_license/bit-web/src/app/tools/access-intelligence/access-intelligence.module.ts b/bitwarden_license/bit-web/src/app/tools/access-intelligence/access-intelligence.module.ts index 3f177119aa8..87b75dee70c 100644 --- a/bitwarden_license/bit-web/src/app/tools/access-intelligence/access-intelligence.module.ts +++ b/bitwarden_license/bit-web/src/app/tools/access-intelligence/access-intelligence.module.ts @@ -1,9 +1,33 @@ import { NgModule } from "@angular/core"; +import { + MemberCipherDetailsApiService, + RiskInsightsReportService, +} from "@bitwarden/bit-common/tools/reports/risk-insights/services"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { AuditService } from "@bitwarden/common/abstractions/audit.service"; +import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength/password-strength.service.abstraction"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; + import { AccessIntelligenceRoutingModule } from "./access-intelligence-routing.module"; import { RiskInsightsComponent } from "./risk-insights.component"; @NgModule({ imports: [RiskInsightsComponent, AccessIntelligenceRoutingModule], + providers: [ + { + provide: MemberCipherDetailsApiService, + deps: [ApiService], + }, + { + provide: RiskInsightsReportService, + deps: [ + PasswordStrengthServiceAbstraction, + AuditService, + CipherService, + MemberCipherDetailsApiService, + ], + }, + ], }) export class AccessIntelligenceModule {} diff --git a/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.html b/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.html index 5b1fe4610d9..aeaa9f33197 100644 --- a/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.html +++ b/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.html @@ -34,10 +34,10 @@ - {{ passwordStrengthMap.get(r.id)[0] | i18n }} + {{ r.weakPasswordDetail?.detailValue.label | i18n }} @@ -46,8 +46,8 @@ - - {{ "exposedXTimes" | i18n: exposedPasswordMap.get(r.id) }} + + {{ "exposedXTimes" | i18n: r.exposedPasswordDetail?.exposedXTimes }} diff --git a/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.spec.ts b/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.spec.ts index 1f1756731f6..4329cfbde14 100644 --- a/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.spec.ts +++ b/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.spec.ts @@ -3,15 +3,8 @@ import { ActivatedRoute, convertToParamMap } from "@angular/router"; import { mock } from "jest-mock-extended"; import { of } from "rxjs"; -import { - MemberCipherDetailsApiService, - PasswordHealthService, -} from "@bitwarden/bit-common/tools/reports/risk-insights"; -import { ApiService } from "@bitwarden/common/abstractions/api.service"; -import { AuditService } from "@bitwarden/common/abstractions/audit.service"; +import { RiskInsightsReportService } from "@bitwarden/bit-common/tools/reports/risk-insights"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; -import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { TableModule } from "@bitwarden/components"; import { LooseComponentsModule } from "@bitwarden/web-vault/app/shared"; import { PipesModule } from "@bitwarden/web-vault/app/vault/individual-vault/pipes/pipes.module"; @@ -28,19 +21,8 @@ describe("PasswordHealthComponent", () => { imports: [PasswordHealthComponent, PipesModule, TableModule, LooseComponentsModule], declarations: [], providers: [ - { provide: CipherService, useValue: mock() }, + { provide: RiskInsightsReportService, useValue: mock() }, { provide: I18nService, useValue: mock() }, - { provide: AuditService, useValue: mock() }, - { provide: ApiService, useValue: mock() }, - { provide: MemberCipherDetailsApiService, useValue: mock() }, - { - provide: PasswordStrengthServiceAbstraction, - useValue: mock(), - }, - { - provide: PasswordHealthService, - useValue: mock(), - }, { provide: ActivatedRoute, useValue: { diff --git a/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.ts b/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.ts index 06f7de439cf..62d543a080d 100644 --- a/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.ts +++ b/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.ts @@ -4,21 +4,14 @@ import { CommonModule } from "@angular/common"; import { Component, DestroyRef, inject, OnInit } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { ActivatedRoute } from "@angular/router"; -import { map } from "rxjs"; +import { firstValueFrom, map } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; -import { - MemberCipherDetailsApiService, - PasswordHealthService, -} from "@bitwarden/bit-common/tools/reports/risk-insights"; -import { AuditService } from "@bitwarden/common/abstractions/audit.service"; +import { RiskInsightsReportService } from "@bitwarden/bit-common/tools/reports/risk-insights"; +import { CipherHealthReportDetail } from "@bitwarden/bit-common/tools/reports/risk-insights/models/password-health"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -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"; import { BadgeModule, - BadgeVariant, ContainerComponent, TableDataSource, TableModule, @@ -41,28 +34,19 @@ import { PipesModule } from "@bitwarden/web-vault/app/vault/individual-vault/pip HeaderModule, TableModule, ], - providers: [PasswordHealthService, MemberCipherDetailsApiService], }) export class PasswordHealthComponent implements OnInit { - passwordStrengthMap = new Map(); - passwordUseMap = new Map(); - - exposedPasswordMap = new Map(); - - dataSource = new TableDataSource(); + dataSource = new TableDataSource(); loading = true; private destroyRef = inject(DestroyRef); constructor( - protected cipherService: CipherService, - protected passwordStrengthService: PasswordStrengthServiceAbstraction, - protected auditService: AuditService, + protected riskInsightsReportService: RiskInsightsReportService, protected i18nService: I18nService, protected activatedRoute: ActivatedRoute, - protected memberCipherDetailsApiService: MemberCipherDetailsApiService, ) {} ngOnInit() { @@ -78,20 +62,9 @@ export class PasswordHealthComponent implements OnInit { } async setCiphers(organizationId: string) { - const passwordHealthService = new PasswordHealthService( - this.passwordStrengthService, - this.auditService, - this.cipherService, - this.memberCipherDetailsApiService, - organizationId, + this.dataSource.data = await firstValueFrom( + this.riskInsightsReportService.generateRawDataReport$(organizationId), ); - - await passwordHealthService.generateReport(); - - this.dataSource.data = passwordHealthService.reportCiphers; - this.exposedPasswordMap = passwordHealthService.exposedPasswordMap; - this.passwordStrengthMap = passwordHealthService.passwordStrengthMap; - this.passwordUseMap = passwordHealthService.passwordUseMap; this.loading = false; } }