diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/index.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/index.ts index f547df31f4..e3f75ea0da 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/index.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/index.ts @@ -2,5 +2,6 @@ export * from "./member-cipher-details-api.service"; export * from "./password-health.service"; export * from "./critical-apps.service"; export * from "./critical-apps-api.service"; +export * from "./risk-insights-api.service"; export * from "./risk-insights-report.service"; export * from "./risk-insights-data.service"; 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 45f9aeed1d..6c6fbb5b92 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,10 +1,7 @@ import { mock } from "jest-mock-extended"; 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 { CipherType } from "@bitwarden/common/vault/enums"; @@ -16,6 +13,7 @@ import { MemberCipherDetailsResponse } from "../response/member-cipher-details.r import { mockCiphers } from "./ciphers.mock"; import { MemberCipherDetailsApiService } from "./member-cipher-details-api.service"; import { mockMemberCipherDetails } from "./member-cipher-details-api.service.spec"; +import { PasswordHealthService } from "./password-health.service"; import { RiskInsightsApiService } from "./risk-insights-api.service"; import { RiskInsightsEncryptionService } from "./risk-insights-encryption.service"; import { RiskInsightsReportService } from "./risk-insights-report.service"; @@ -24,10 +22,9 @@ describe("RiskInsightsReportService", () => { let service: RiskInsightsReportService; // Mock services - const pwdStrengthService = mock(); - const auditService = mock(); const cipherService = mock(); const memberCipherDetailsService = mock(); + const mockPasswordHealthService = mock(); const mockRiskInsightsApiService = mock(); const mockRiskInsightsEncryptionService = mock({ encryptRiskInsightsReport: jest.fn().mockResolvedValue("encryptedReportData"), @@ -40,26 +37,38 @@ describe("RiskInsightsReportService", () => { let mockMemberDetails: MemberCipherDetailsResponse[]; beforeEach(() => { - pwdStrengthService.getPasswordStrength.mockImplementation((password: string) => { - const score = password.length < 4 ? 1 : 4; - return { score } as ZXCVBNResult; - }); - - auditService.passwordLeaked.mockImplementation((password: string) => - Promise.resolve(password === "123" ? 100 : 0), - ); - cipherService.getAllFromApiForOrganization.mockResolvedValue(mockCiphers); memberCipherDetailsService.getMemberCipherDetails.mockResolvedValue(mockMemberCipherDetails); + // Mock PasswordHealthService methods + mockPasswordHealthService.isValidCipher.mockImplementation((cipher: any) => { + return ( + cipher.type === 1 && cipher.login?.password && !cipher.isDeleted && cipher.viewPassword + ); + }); + mockPasswordHealthService.findWeakPasswordDetails.mockImplementation((cipher: any) => { + if (cipher.login?.password === "123") { + return { score: 1, detailValue: { label: "veryWeak", badgeVariant: "danger" } }; + } + return null; + }); + mockPasswordHealthService.auditPasswordLeaks$.mockImplementation((ciphers: any[]) => { + const exposedDetails = ciphers + .filter((cipher) => cipher.login?.password === "123") + .map((cipher) => ({ + exposedXTimes: 100, + cipherId: cipher.id, + })); + return of(exposedDetails); + }); + service = new RiskInsightsReportService( - pwdStrengthService, - auditService, cipherService, memberCipherDetailsService, mockRiskInsightsApiService, mockRiskInsightsEncryptionService, + mockPasswordHealthService, ); // Reset mock ciphers before each test 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 7341beb3fe..1839e89a1a 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 @@ -3,27 +3,20 @@ import { BehaviorSubject, concatMap, - filter, first, firstValueFrom, forkJoin, from, map, - mergeMap, Observable, of, switchMap, - toArray, 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"; import { @@ -37,10 +30,7 @@ import { import { LEGACY_CipherHealthReportDetail, LEGACY_CipherHealthReportUriDetail, - ExposedPasswordDetail, LEGACY_MemberDetailsFlat, - WeakPasswordDetail, - WeakPasswordScore, LEGACY_ApplicationHealthReportDetailWithCriticalFlagAndCipher, } from "../models/password-health"; import { @@ -55,6 +45,7 @@ import { } from "../models/report-models"; import { MemberCipherDetailsApiService } from "./member-cipher-details-api.service"; +import { PasswordHealthService } from "./password-health.service"; import { RiskInsightsApiService } from "./risk-insights-api.service"; import { RiskInsightsEncryptionService } from "./risk-insights-encryption.service"; @@ -81,12 +72,11 @@ export class RiskInsightsReportService { // _ciphers$ = this._ciphersSubject.asObservable(); constructor( - private passwordStrengthService: PasswordStrengthServiceAbstraction, - private auditService: AuditService, private cipherService: CipherService, private memberCipherDetailsApiService: MemberCipherDetailsApiService, private riskInsightsApiService: RiskInsightsApiService, private riskInsightsEncryptionService: RiskInsightsEncryptionService, + private passwordHealthService: PasswordHealthService, ) {} // [FIXME] CipherData @@ -367,10 +357,12 @@ export class RiskInsightsReportService { ): Promise { const cipherHealthReports: LEGACY_CipherHealthReportDetail[] = []; const passwordUseMap = new Map(); - const exposedDetails = await this.findExposedPasswords(ciphers); + const exposedDetails = await firstValueFrom( + this.passwordHealthService.auditPasswordLeaks$(ciphers), + ); for (const cipher of ciphers) { - if (this.validateCipher(cipher)) { - const weakPassword = this.findWeakPassword(cipher); + if (this.passwordHealthService.isValidCipher(cipher)) { + const weakPassword = this.passwordHealthService.findWeakPasswordDetails(cipher); // Looping over all ciphers needs to happen first to determine reused passwords over all ciphers. // Store in the set and evaluate later if (passwordUseMap.has(cipher.login.password)) { @@ -448,104 +440,6 @@ export class RiskInsightsReportService { return appReports; } - private async findExposedPasswords(ciphers: CipherView[]): Promise { - const exposedDetails: ExposedPasswordDetail[] = []; - const promises: Promise[] = []; - - ciphers.forEach((ciph) => { - if (this.validateCipher(ciph)) { - const promise = this.auditService - .passwordLeaked(ciph.login.password) - .then((exposedCount) => { - if (exposedCount > 0) { - const detail = { - exposedXTimes: exposedCount, - cipherId: ciph.id, - } as ExposedPasswordDetail; - exposedDetails.push(detail); - } - }); - promises.push(promise); - } - }); - await Promise.all(promises); - - return exposedDetails; - } - - private findWeakPassword(cipher: CipherView): WeakPasswordDetail { - const hasUserName = this.isUserNameNotEmpty(cipher); - let userInput: string[] = []; - if (hasUserName) { - const atPosition = cipher.login.username.indexOf("@"); - if (atPosition > -1) { - userInput = userInput - .concat( - cipher.login.username - .substring(0, atPosition) - .trim() - .toLowerCase() - .split(/[^A-Za-z0-9]/), - ) - .filter((i) => i.length >= 3); - } else { - userInput = cipher.login.username - .trim() - .toLowerCase() - .split(/[^A-Za-z0-9]/) - .filter((i) => i.length >= 3); - } - } - const { score } = this.passwordStrengthService.getPasswordStrength( - cipher.login.password, - null, - userInput.length > 0 ? userInput : null, - ); - - if (score != null && score <= 2) { - const scoreValue = this.weakPasswordScore(score); - const weakPasswordDetail = { score: score, detailValue: scoreValue } as WeakPasswordDetail; - return weakPasswordDetail; - } - return null; - } - - private weakPasswordScore(score: number): WeakPasswordScore { - switch (score) { - case 4: - return { label: "strong", badgeVariant: "success" }; - case 3: - return { label: "good", badgeVariant: "primary" }; - case 2: - return { label: "weak", badgeVariant: "warning" }; - default: - return { label: "veryWeak", badgeVariant: "danger" }; - } - } - - private isUserNameNotEmpty(c: CipherView): boolean { - return !Utils.isNullOrWhitespace(c.login.username); - } - - /** - * Validates that the cipher is a login item, has a password - * is not deleted, and the user can view the password - * @param c the input cipher - */ - private validateCipher(c: CipherView): boolean { - const { type, login, isDeleted, viewPassword } = c; - if ( - type !== CipherType.Login || - login.password == null || - login.password === "" || - isDeleted || - !viewPassword - ) { - return false; - } - return true; - } - private _buildPasswordUseMap(ciphers: CipherView[]): Map { const passwordUseMap = new Map(); ciphers.forEach((cipher) => { @@ -696,11 +590,13 @@ export class RiskInsightsReportService { ciphers: CipherView[], memberDetails: MemberDetails[], ): Observable { - const validCiphers = ciphers.filter((cipher) => this.isValidCipher(cipher)); + const validCiphers = ciphers.filter((cipher) => + this.passwordHealthService.isValidCipher(cipher), + ); // Build password use map const passwordUseMap = this._buildPasswordUseMap(validCiphers); - return this.auditPasswordLeaks$(validCiphers).pipe( + return this.passwordHealthService.auditPasswordLeaks$(validCiphers).pipe( map((exposedDetails) => { return validCiphers.map((cipher) => { const exposedPassword = exposedDetails.find((x) => x.cipherId === cipher.id); @@ -710,7 +606,7 @@ export class RiskInsightsReportService { cipher: cipher, cipherMembers, healthData: { - weakPasswordDetail: this.findWeakPasswordDetails(cipher), + weakPasswordDetail: this.passwordHealthService.findWeakPasswordDetails(cipher), exposedPasswordDetail: exposedPassword, reusedPasswordCount: passwordUseMap.get(cipher.login.password) ?? 0, }, @@ -721,119 +617,4 @@ export class RiskInsightsReportService { }), ); } - - // TODO This is a temp implementation until the function is available in the password health service - /** - * Validates that the cipher is a login item, has a password - * is not deleted, and the user can view the password - * @param c the input cipher - */ - isValidCipher(c: CipherView): boolean { - const { type, login, isDeleted, viewPassword } = c; - if ( - type !== CipherType.Login || - login.password == null || - login.password === "" || - isDeleted || - !viewPassword - ) { - return false; - } - return true; - } - - // TODO This is a temp implementation until the function is available in the password health service - /** - * Extracts username parts from the cipher's username. - * This is used to help determine password strength. - * - * @param cipherUsername The username from the cipher. - * @returns An array of username parts. - */ - extractUsernameParts(cipherUsername: string) { - const atPosition = cipherUsername.indexOf("@"); - const userNameToProcess = - atPosition > -1 ? cipherUsername.substring(0, atPosition) : cipherUsername; - - return userNameToProcess - .trim() - .toLowerCase() - .split(/[^A-Za-z0-9]/) - .filter((i) => i.length >= 3); - } - - // TODO This is a temp implementation until the function is available in the password health service - /** - * Checks if the cipher has a weak password based on the password strength score. - * - * @param cipher - * @returns - */ - findWeakPasswordDetails(cipher: CipherView): WeakPasswordDetail | null { - // Validate the cipher - if (!this.isValidCipher(cipher)) { - return null; - } - - // Check the username - const userInput = this.isUserNameNotEmpty(cipher) - ? this.extractUsernameParts(cipher.login.username) - : null; - - const { score } = this.passwordStrengthService.getPasswordStrength( - cipher.login.password, - null, - userInput, - ); - - // If a score is not found or a score is less than 3, it's weak - if (score != null && score <= 2) { - return { score: score, detailValue: this.getPasswordScoreInfo(score) }; - } - return null; - } - - // TODO This is a temp implementation until the function is available in the password health service - /** - * Gets the password score information based on the score. - * - * @param score - * @returns An object containing the label and badge variant for the password score. - */ - getPasswordScoreInfo(score: number): WeakPasswordScore { - switch (score) { - case 4: - return { label: "strong", badgeVariant: "success" }; - case 3: - return { label: "good", badgeVariant: "primary" }; - case 2: - return { label: "weak", badgeVariant: "warning" }; - default: - return { label: "veryWeak", badgeVariant: "danger" }; - } - } - - // TODO This is a temp implementation until the function is available in the password health service - /** - * Finds exposed passwords in a list of ciphers. - * - * @param ciphers The list of ciphers to check. - * @returns An observable that emits an array of ExposedPasswordDetail. - */ - auditPasswordLeaks$(ciphers: CipherView[]): Observable { - return from(ciphers).pipe( - filter((cipher) => this.isValidCipher(cipher)), - mergeMap((cipher) => - this.auditService - .passwordLeaked(cipher.login.password) - .then((exposedCount) => ({ cipher, exposedCount })), - ), - filter(({ exposedCount }) => exposedCount > 0), - map(({ cipher, exposedCount }) => ({ - exposedXTimes: exposedCount, - cipherId: cipher.id, - })), - toArray(), - ); - } } 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 c39f06a57a..1d80f2154b 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 @@ -5,6 +5,8 @@ import { CriticalAppsService } from "@bitwarden/bit-common/dirt/reports/risk-ins import { CriticalAppsApiService, MemberCipherDetailsApiService, + PasswordHealthService, + RiskInsightsApiService, RiskInsightsDataService, RiskInsightsReportService, } from "@bitwarden/bit-common/dirt/reports/risk-insights/services"; @@ -29,13 +31,22 @@ import { RiskInsightsComponent } from "./risk-insights.component"; provide: MemberCipherDetailsApiService, deps: [ApiService], }, + { + provide: PasswordHealthService, + deps: [PasswordStrengthServiceAbstraction, AuditService], + }, + { + provide: RiskInsightsApiService, + deps: [ApiService], + }, { provide: RiskInsightsReportService, deps: [ - PasswordStrengthServiceAbstraction, - AuditService, CipherService, MemberCipherDetailsApiService, + RiskInsightsApiService, + RiskInsightsEncryptionService, + PasswordHealthService, ], }, safeProvider({