1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-15 07:43:35 +00:00

[PM-2367] [BEEEP]: Extract password strength from password-generation-service (#5502)

* Extract passwordStrength from passwordGenerationService

Extract passwordStrength from password-generation.service.ts
Create new password-strength.service.ts
Create new password-strength.service.abstraction.ts
Register new password-strength service
Fix usages in libs

* Fix usage in web

* Fix usage in desktop

* Fix usage in CLI

* Fix usage in browser

Move password-generation-factory to tools

* Fix tests

* Change dependency in jslib-services.module
This commit is contained in:
Daniel James Smith
2023-06-13 23:22:25 +02:00
committed by GitHub
parent 22caae116c
commit 72a5ba455c
28 changed files with 224 additions and 111 deletions

View File

@@ -11,7 +11,10 @@ import { StateService } from "../../platform/abstractions/state.service";
import { Utils } from "../../platform/misc/utils";
import { Account, AccountProfile, AccountTokens } from "../../platform/models/domain/account";
import { EncString } from "../../platform/models/domain/enc-string";
import { PasswordGenerationService } from "../../tools/generator/password";
import {
PasswordStrengthService,
PasswordStrengthServiceAbstraction,
} from "../../tools/password-strength";
import { AuthService } from "../abstractions/auth.service";
import { TokenService } from "../abstractions/token.service";
import { TwoFactorService } from "../abstractions/two-factor.service";
@@ -85,7 +88,7 @@ describe("LogInStrategy", () => {
let twoFactorService: MockProxy<TwoFactorService>;
let authService: MockProxy<AuthService>;
let policyService: MockProxy<PolicyService>;
let passwordGenerationService: MockProxy<PasswordGenerationService>;
let passwordStrengthService: MockProxy<PasswordStrengthServiceAbstraction>;
let passwordLogInStrategy: PasswordLogInStrategy;
let credentials: PasswordLogInCredentials;
@@ -102,7 +105,7 @@ describe("LogInStrategy", () => {
twoFactorService = mock<TwoFactorService>();
authService = mock<AuthService>();
policyService = mock<PolicyService>();
passwordGenerationService = mock<PasswordGenerationService>();
passwordStrengthService = mock<PasswordStrengthService>();
appIdService.getAppId.mockResolvedValue(deviceId);
tokenService.decodeToken.calledWith(accessToken).mockResolvedValue(decodedToken);
@@ -118,7 +121,7 @@ describe("LogInStrategy", () => {
logService,
stateService,
twoFactorService,
passwordGenerationService,
passwordStrengthService,
policyService,
authService
);

View File

@@ -11,7 +11,10 @@ import { PlatformUtilsService } from "../../platform/abstractions/platform-utils
import { StateService } from "../../platform/abstractions/state.service";
import { Utils } from "../../platform/misc/utils";
import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypto-key";
import { PasswordGenerationService } from "../../tools/generator/password";
import {
PasswordStrengthService,
PasswordStrengthServiceAbstraction,
} from "../../tools/password-strength";
import { AuthService } from "../abstractions/auth.service";
import { TokenService } from "../abstractions/token.service";
import { TwoFactorService } from "../abstractions/two-factor.service";
@@ -51,7 +54,7 @@ describe("PasswordLogInStrategy", () => {
let twoFactorService: MockProxy<TwoFactorService>;
let authService: MockProxy<AuthService>;
let policyService: MockProxy<PolicyService>;
let passwordGenerationService: MockProxy<PasswordGenerationService>;
let passwordStrengthService: MockProxy<PasswordStrengthServiceAbstraction>;
let passwordLogInStrategy: PasswordLogInStrategy;
let credentials: PasswordLogInCredentials;
@@ -68,7 +71,7 @@ describe("PasswordLogInStrategy", () => {
twoFactorService = mock<TwoFactorService>();
authService = mock<AuthService>();
policyService = mock<PolicyService>();
passwordGenerationService = mock<PasswordGenerationService>();
passwordStrengthService = mock<PasswordStrengthService>();
appIdService.getAppId.mockResolvedValue(deviceId);
tokenService.decodeToken.mockResolvedValue({});
@@ -94,7 +97,7 @@ describe("PasswordLogInStrategy", () => {
logService,
stateService,
twoFactorService,
passwordGenerationService,
passwordStrengthService,
policyService,
authService
);
@@ -141,7 +144,7 @@ describe("PasswordLogInStrategy", () => {
});
it("does not force the user to update their master password when it meets requirements", async () => {
passwordGenerationService.passwordStrength.mockReturnValue({ score: 5 } as any);
passwordStrengthService.getPasswordStrength.mockReturnValue({ score: 5 } as any);
policyService.evaluateMasterPassword.mockReturnValue(true);
const result = await passwordLogInStrategy.logIn(credentials);
@@ -151,7 +154,7 @@ describe("PasswordLogInStrategy", () => {
});
it("forces the user to update their master password on successful login when it does not meet master password policy requirements", async () => {
passwordGenerationService.passwordStrength.mockReturnValue({ score: 0 } as any);
passwordStrengthService.getPasswordStrength.mockReturnValue({ score: 0 } as any);
policyService.evaluateMasterPassword.mockReturnValue(false);
const result = await passwordLogInStrategy.logIn(credentials);
@@ -164,7 +167,7 @@ describe("PasswordLogInStrategy", () => {
});
it("forces the user to update their master password on successful 2FA login when it does not meet master password policy requirements", async () => {
passwordGenerationService.passwordStrength.mockReturnValue({ score: 0 } as any);
passwordStrengthService.getPasswordStrength.mockReturnValue({ score: 0 } as any);
policyService.evaluateMasterPassword.mockReturnValue(false);
const token2FAResponse = new IdentityTwoFactorResponse({

View File

@@ -9,7 +9,7 @@ import { MessagingService } from "../../platform/abstractions/messaging.service"
import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service";
import { StateService } from "../../platform/abstractions/state.service";
import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypto-key";
import { PasswordGenerationServiceAbstraction } from "../../tools/generator/password";
import { PasswordStrengthServiceAbstraction } from "../../tools/password-strength";
import { AuthService } from "../abstractions/auth.service";
import { TokenService } from "../abstractions/token.service";
import { TwoFactorService } from "../abstractions/two-factor.service";
@@ -54,7 +54,7 @@ export class PasswordLogInStrategy extends LogInStrategy {
logService: LogService,
protected stateService: StateService,
twoFactorService: TwoFactorService,
private passwordGenerationService: PasswordGenerationServiceAbstraction,
private passwordStrengthService: PasswordStrengthServiceAbstraction,
private policyService: PolicyService,
private authService: AuthService
) {
@@ -158,7 +158,7 @@ export class PasswordLogInStrategy extends LogInStrategy {
{ masterPassword, email }: PasswordLogInCredentials,
options: MasterPasswordPolicyOptions
): boolean {
const passwordStrength = this.passwordGenerationService.passwordStrength(
const passwordStrength = this.passwordStrengthService.getPasswordStrength(
masterPassword,
email
)?.score;

View File

@@ -17,7 +17,7 @@ import { PlatformUtilsService } from "../../platform/abstractions/platform-utils
import { StateService } from "../../platform/abstractions/state.service";
import { Utils } from "../../platform/misc/utils";
import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypto-key";
import { PasswordGenerationServiceAbstraction } from "../../tools/generator/password";
import { PasswordStrengthServiceAbstraction } from "../../tools/password-strength";
import { AuthService as AuthServiceAbstraction } from "../abstractions/auth.service";
import { KeyConnectorService } from "../abstractions/key-connector.service";
import { TokenService } from "../abstractions/token.service";
@@ -102,7 +102,7 @@ export class AuthService implements AuthServiceAbstraction {
protected twoFactorService: TwoFactorService,
protected i18nService: I18nService,
protected encryptService: EncryptService,
protected passwordGenerationService: PasswordGenerationServiceAbstraction,
protected passwordStrengthService: PasswordStrengthServiceAbstraction,
protected policyService: PolicyService
) {}
@@ -133,7 +133,7 @@ export class AuthService implements AuthServiceAbstraction {
this.logService,
this.stateService,
this.twoFactorService,
this.passwordGenerationService,
this.passwordStrengthService,
this.policyService,
this
);

View File

@@ -1,5 +1,3 @@
import * as zxcvbn from "zxcvbn";
import { PasswordGeneratorPolicyOptions } from "../../../admin-console/models/domain/password-generator-policy-options";
import { GeneratedPasswordHistory } from "./generated-password-history";
@@ -17,11 +15,6 @@ export abstract class PasswordGenerationServiceAbstraction {
getHistory: () => Promise<GeneratedPasswordHistory[]>;
addHistory: (password: string) => Promise<void>;
clear: (userId?: string) => Promise<void>;
passwordStrength: (
password: string,
email?: string,
userInputs?: string[]
) => zxcvbn.ZXCVBNResult;
normalizeOptions: (
options: PasswordGeneratorOptions,
enforcedPolicyOptions: PasswordGeneratorPolicyOptions

View File

@@ -1,5 +1,3 @@
import * as zxcvbn from "zxcvbn";
import { PolicyService } from "../../../admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyType } from "../../../admin-console/enums";
import { PasswordGeneratorPolicyOptions } from "../../../admin-console/models/domain/password-generator-policy-options";
@@ -387,33 +385,6 @@ export class PasswordGenerationService implements PasswordGenerationServiceAbstr
await this.stateService.setDecryptedPasswordGenerationHistory(null, { userId: userId });
}
/**
* Calculates a password strength score using zxcvbn.
* @param password The password to calculate the strength of.
* @param emailInput An unparsed email address to use as user input.
* @param userInputs An array of additional user inputs to use when calculating the strength.
*/
passwordStrength(
password: string,
emailInput: string = null,
userInputs: string[] = null
): zxcvbn.ZXCVBNResult {
if (password == null || password.length === 0) {
return null;
}
const globalUserInputs = [
"bitwarden",
"bit",
"warden",
...(userInputs ?? []),
...this.emailToUserInputs(emailInput),
];
// Use a hash set to get rid of any duplicate user inputs
const finalUserInputs = Array.from(new Set(globalUserInputs));
const result = zxcvbn(password, finalUserInputs);
return result;
}
normalizeOptions(
options: PasswordGeneratorOptions,
enforcedPolicyOptions: PasswordGeneratorPolicyOptions
@@ -476,27 +447,6 @@ export class PasswordGenerationService implements PasswordGenerationServiceAbstr
this.sanitizePasswordLength(options, false);
}
/**
* Convert an email address into a list of user inputs for zxcvbn by
* taking the local part of the email address and splitting it into words.
* @param email
* @private
*/
private emailToUserInputs(email: string): string[] {
if (email == null || email.length === 0) {
return [];
}
const atPosition = email.indexOf("@");
if (atPosition < 0) {
return [];
}
return email
.substring(0, atPosition)
.trim()
.toLowerCase()
.split(/[^A-Za-z0-9]/);
}
private capitalize(str: string) {
return str.charAt(0).toUpperCase() + str.slice(1);
}

View File

@@ -0,0 +1,2 @@
export { PasswordStrengthServiceAbstraction } from "./password-strength.service.abstraction";
export { PasswordStrengthService } from "./password-strength.service";

View File

@@ -0,0 +1,5 @@
import { ZXCVBNResult } from "zxcvbn";
export abstract class PasswordStrengthServiceAbstraction {
getPasswordStrength: (password: string, email?: string, userInputs?: string[]) => ZXCVBNResult;
}

View File

@@ -0,0 +1,53 @@
import * as zxcvbn from "zxcvbn";
import { PasswordStrengthServiceAbstraction } from "./password-strength.service.abstraction";
export class PasswordStrengthService implements PasswordStrengthServiceAbstraction {
/**
* Calculates a password strength score using zxcvbn.
* @param password The password to calculate the strength of.
* @param emailInput An unparsed email address to use as user input.
* @param userInputs An array of additional user inputs to use when calculating the strength.
*/
getPasswordStrength(
password: string,
emailInput: string = null,
userInputs: string[] = null
): zxcvbn.ZXCVBNResult {
if (password == null || password.length === 0) {
return null;
}
const globalUserInputs = [
"bitwarden",
"bit",
"warden",
...(userInputs ?? []),
...this.emailToUserInputs(emailInput),
];
// Use a hash set to get rid of any duplicate user inputs
const finalUserInputs = Array.from(new Set(globalUserInputs));
const result = zxcvbn(password, finalUserInputs);
return result;
}
/**
* Convert an email address into a list of user inputs for zxcvbn by
* taking the local part of the email address and splitting it into words.
* @param email
* @private
*/
private emailToUserInputs(email: string): string[] {
if (email == null || email.length === 0) {
return [];
}
const atPosition = email.indexOf("@");
if (atPosition < 0) {
return [];
}
return email
.substring(0, atPosition)
.trim()
.toLowerCase()
.split(/[^A-Za-z0-9]/);
}
}