1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-06 00:13:28 +00:00

[PM-25609] use password health service (#16482)

* isValidCipher and findWeakPasswordDetails

* auditPasswordLeaks$

* missing deps fix

* refactor: remove unused dependencies from RiskInsightsReportService

- Remove PasswordStrengthServiceAbstraction and AuditService from constructor
- Update module dependency injection to only provide these services to PasswordHealthService
- Remove unused imports and mock services from test file
- Ensure proper separation of concerns where password health logic is centralized in PasswordHealthService
This commit is contained in:
Alex
2025-09-26 14:59:38 -04:00
committed by GitHub
parent 7baf250288
commit 8ba22f3080
4 changed files with 51 additions and 249 deletions

View File

@@ -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";

View File

@@ -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<PasswordStrengthServiceAbstraction>();
const auditService = mock<AuditService>();
const cipherService = mock<CipherService>();
const memberCipherDetailsService = mock<MemberCipherDetailsApiService>();
const mockPasswordHealthService = mock<PasswordHealthService>();
const mockRiskInsightsApiService = mock<RiskInsightsApiService>();
const mockRiskInsightsEncryptionService = mock<RiskInsightsEncryptionService>({
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

View File

@@ -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<LEGACY_CipherHealthReportDetail[]> {
const cipherHealthReports: LEGACY_CipherHealthReportDetail[] = [];
const passwordUseMap = new Map<string, number>();
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<ExposedPasswordDetail[]> {
const exposedDetails: ExposedPasswordDetail[] = [];
const promises: Promise<void>[] = [];
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<string, number> {
const passwordUseMap = new Map<string, number>();
ciphers.forEach((cipher) => {
@@ -696,11 +590,13 @@ export class RiskInsightsReportService {
ciphers: CipherView[],
memberDetails: MemberDetails[],
): Observable<CipherHealthReport[]> {
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<ExposedPasswordDetail[]> {
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(),
);
}
}

View File

@@ -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({