1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-21 18:53:29 +00:00

[PM-6523] generator service tuning (#8155)

* rename policy$ to evaluator$
* replace `ActiveUserState` with `SingleUserState`
* implement `SingleUserState<T>` on `SecretState`
This commit is contained in:
✨ Audrey ✨
2024-03-04 13:43:38 -05:00
committed by GitHub
parent bf6fd39f15
commit d87a8f9271
18 changed files with 394 additions and 211 deletions

View File

@@ -9,18 +9,24 @@ import { PolicyType } from "../../../admin-console/enums";
// FIXME: use index.ts imports once policy abstractions and models
// implement ADR-0002
import { Policy } from "../../../admin-console/models/domain/policy";
import { StateProvider } from "../../../platform/state";
import { UserId } from "../../../types/guid";
import { PASSWORD_SETTINGS } from "../key-definitions";
import { DisabledPasswordGeneratorPolicy } from "./password-generator-policy";
import {
PasswordGenerationServiceAbstraction,
PasswordGeneratorOptionsEvaluator,
PasswordGeneratorStrategy,
} from ".";
const SomeUser = "some user" as UserId;
describe("Password generation strategy", () => {
describe("evaluator()", () => {
it("should throw if the policy type is incorrect", () => {
const strategy = new PasswordGeneratorStrategy(null);
const strategy = new PasswordGeneratorStrategy(null, null);
const policy = mock<Policy>({
type: PolicyType.DisableSend,
});
@@ -29,7 +35,7 @@ describe("Password generation strategy", () => {
});
it("should map to the policy evaluator", () => {
const strategy = new PasswordGeneratorStrategy(null);
const strategy = new PasswordGeneratorStrategy(null, null);
const policy = mock<Policy>({
type: PolicyType.PasswordGenerator,
data: {
@@ -56,21 +62,32 @@ describe("Password generation strategy", () => {
specialCount: 1,
});
});
it("should map `null` to a default policy evaluator", () => {
const strategy = new PasswordGeneratorStrategy(null, null);
const evaluator = strategy.evaluator(null);
expect(evaluator).toBeInstanceOf(PasswordGeneratorOptionsEvaluator);
expect(evaluator.policy).toMatchObject(DisabledPasswordGeneratorPolicy);
});
});
describe("disk", () => {
describe("durableState", () => {
it("should use password settings key", () => {
const provider = mock<StateProvider>();
const legacy = mock<PasswordGenerationServiceAbstraction>();
const strategy = new PasswordGeneratorStrategy(legacy);
const strategy = new PasswordGeneratorStrategy(legacy, provider);
expect(strategy.disk).toBe(PASSWORD_SETTINGS);
strategy.durableState(SomeUser);
expect(provider.getUser).toHaveBeenCalledWith(SomeUser, PASSWORD_SETTINGS);
});
});
describe("cache_ms", () => {
it("should be a positive non-zero number", () => {
const legacy = mock<PasswordGenerationServiceAbstraction>();
const strategy = new PasswordGeneratorStrategy(legacy);
const strategy = new PasswordGeneratorStrategy(legacy, null);
expect(strategy.cache_ms).toBeGreaterThan(0);
});
@@ -79,7 +96,7 @@ describe("Password generation strategy", () => {
describe("policy", () => {
it("should use password generator policy", () => {
const legacy = mock<PasswordGenerationServiceAbstraction>();
const strategy = new PasswordGeneratorStrategy(legacy);
const strategy = new PasswordGeneratorStrategy(legacy, null);
expect(strategy.policy).toBe(PolicyType.PasswordGenerator);
});
@@ -88,7 +105,7 @@ describe("Password generation strategy", () => {
describe("generate()", () => {
it("should call the legacy service with the given options", async () => {
const legacy = mock<PasswordGenerationServiceAbstraction>();
const strategy = new PasswordGeneratorStrategy(legacy);
const strategy = new PasswordGeneratorStrategy(legacy, null);
const options = {
type: "password",
minLength: 1,
@@ -107,7 +124,7 @@ describe("Password generation strategy", () => {
it("should set the generation type to password", async () => {
const legacy = mock<PasswordGenerationServiceAbstraction>();
const strategy = new PasswordGeneratorStrategy(legacy);
const strategy = new PasswordGeneratorStrategy(legacy, null);
await strategy.generate({ type: "foo" } as any);

View File

@@ -3,12 +3,17 @@ import { PolicyType } from "../../../admin-console/enums";
// FIXME: use index.ts imports once policy abstractions and models
// implement ADR-0002
import { Policy } from "../../../admin-console/models/domain/policy";
import { StateProvider } from "../../../platform/state";
import { UserId } from "../../../types/guid";
import { PASSWORD_SETTINGS } from "../key-definitions";
import { PasswordGenerationOptions } from "./password-generation-options";
import { PasswordGenerationServiceAbstraction } from "./password-generation.service.abstraction";
import { PasswordGeneratorOptionsEvaluator } from "./password-generator-options-evaluator";
import { PasswordGeneratorPolicy } from "./password-generator-policy";
import {
DisabledPasswordGeneratorPolicy,
PasswordGeneratorPolicy,
} from "./password-generator-policy";
const ONE_MINUTE = 60 * 1000;
@@ -19,11 +24,14 @@ export class PasswordGeneratorStrategy
/** instantiates the password generator strategy.
* @param legacy generates the password
*/
constructor(private legacy: PasswordGenerationServiceAbstraction) {}
constructor(
private legacy: PasswordGenerationServiceAbstraction,
private stateProvider: StateProvider,
) {}
/** {@link GeneratorStrategy.disk} */
get disk() {
return PASSWORD_SETTINGS;
/** {@link GeneratorStrategy.durableState} */
durableState(id: UserId) {
return this.stateProvider.getUser(id, PASSWORD_SETTINGS);
}
/** {@link GeneratorStrategy.policy} */
@@ -37,6 +45,10 @@ export class PasswordGeneratorStrategy
/** {@link GeneratorStrategy.evaluator} */
evaluator(policy: Policy): PasswordGeneratorOptionsEvaluator {
if (!policy) {
return new PasswordGeneratorOptionsEvaluator(DisabledPasswordGeneratorPolicy);
}
if (policy.type !== this.policy) {
const details = `Expected: ${this.policy}. Received: ${policy.type}`;
throw Error("Mismatched policy type. " + details);