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

[PM-13876] replace angular validation with html constraints validation (#11816)

* rough-in passphrase validation failure handling

* trigger valid change from settings

* fix `max` constraint enforcement

* add taps for generator validation monitoring/debugging

* HTML constraints validation rises like a phoenix

* remove min/max boundaries to fix chrome display issue

* bind settings components as view children of options components

* remove defunct `okSettings$`

* extend validationless generator to passwords

* extend validationless generator to catchall emails

* extend validationless generator to forwarder emails

* extend validationless generator to subaddress emails

* extend validationless generator to usernames

* fix observable cycle

* disable generate button when no algorithm is selected

* prevent duplicate algorithm emissions

* add constraints that assign email address defaults
This commit is contained in:
✨ Audrey ✨
2024-11-06 11:54:29 -05:00
committed by GitHub
parent a9595b4d14
commit 414bdde232
30 changed files with 552 additions and 218 deletions

View File

@@ -1,7 +1,10 @@
import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { Policy } from "@bitwarden/common/admin-console/models/domain/policy";
import { GENERATOR_DISK } from "@bitwarden/common/platform/state";
import { ApiSettings } from "@bitwarden/common/tools/integration/rpc";
import { PublicClassifier } from "@bitwarden/common/tools/public-classifier";
import { IdentityConstraint } from "@bitwarden/common/tools/state/identity-state-constraint";
import { ObjectKey } from "@bitwarden/common/tools/state/object-key";
import {
EmailRandomizer,
@@ -19,12 +22,12 @@ import {
PasswordGeneratorOptionsEvaluator,
passwordLeastPrivilege,
} from "../policies";
import { CatchallConstraints } from "../policies/catchall-constraints";
import { SubaddressConstraints } from "../policies/subaddress-constraints";
import {
CATCHALL_SETTINGS,
EFF_USERNAME_SETTINGS,
PASSPHRASE_SETTINGS,
PASSWORD_SETTINGS,
SUBADDRESS_SETTINGS,
} from "../strategies/storage";
import {
CatchallGenerationOptions,
@@ -178,79 +181,115 @@ const USERNAME = Object.freeze({
},
} satisfies CredentialGeneratorConfiguration<EffUsernameGenerationOptions, NoPolicy>);
const CATCHALL = Object.freeze({
id: "catchall",
category: "email",
nameKey: "catchallEmail",
descriptionKey: "catchallEmailDesc",
generateKey: "generateEmail",
generatedValueKey: "email",
copyKey: "copyEmail",
onlyOnRequest: false,
request: [],
engine: {
create(
dependencies: GeneratorDependencyProvider,
): CredentialGenerator<CatchallGenerationOptions> {
return new EmailRandomizer(dependencies.randomizer);
const CATCHALL: CredentialGeneratorConfiguration<CatchallGenerationOptions, NoPolicy> =
Object.freeze({
id: "catchall",
category: "email",
nameKey: "catchallEmail",
descriptionKey: "catchallEmailDesc",
generateKey: "generateEmail",
generatedValueKey: "email",
copyKey: "copyEmail",
onlyOnRequest: false,
request: [],
engine: {
create(
dependencies: GeneratorDependencyProvider,
): CredentialGenerator<CatchallGenerationOptions> {
return new EmailRandomizer(dependencies.randomizer);
},
},
},
settings: {
initial: DefaultCatchallOptions,
constraints: { catchallDomain: { minLength: 1 } },
account: CATCHALL_SETTINGS,
},
policy: {
type: PolicyType.PasswordGenerator,
disabledValue: {},
combine(_acc: NoPolicy, _policy: Policy) {
return {};
settings: {
initial: DefaultCatchallOptions,
constraints: { catchallDomain: { minLength: 1 } },
account: {
key: "catchallGeneratorSettings",
target: "object",
format: "plain",
classifier: new PublicClassifier<CatchallGenerationOptions>([
"catchallType",
"catchallDomain",
]),
state: GENERATOR_DISK,
initial: {
catchallType: "random",
catchallDomain: "",
},
options: {
deserializer: (value) => value,
clearOn: ["logout"],
},
} satisfies ObjectKey<CatchallGenerationOptions>,
},
createEvaluator(_policy: NoPolicy) {
return new DefaultPolicyEvaluator<CatchallGenerationOptions>();
policy: {
type: PolicyType.PasswordGenerator,
disabledValue: {},
combine(_acc: NoPolicy, _policy: Policy) {
return {};
},
createEvaluator(_policy: NoPolicy) {
return new DefaultPolicyEvaluator<CatchallGenerationOptions>();
},
toConstraints(_policy: NoPolicy, email: string) {
return new CatchallConstraints(email);
},
},
toConstraints(_policy: NoPolicy) {
return new IdentityConstraint<CatchallGenerationOptions>();
},
},
} satisfies CredentialGeneratorConfiguration<CatchallGenerationOptions, NoPolicy>);
});
const SUBADDRESS = Object.freeze({
id: "subaddress",
category: "email",
nameKey: "plusAddressedEmail",
descriptionKey: "plusAddressedEmailDesc",
generateKey: "generateEmail",
generatedValueKey: "email",
copyKey: "copyEmail",
onlyOnRequest: false,
request: [],
engine: {
create(
dependencies: GeneratorDependencyProvider,
): CredentialGenerator<SubaddressGenerationOptions> {
return new EmailRandomizer(dependencies.randomizer);
const SUBADDRESS: CredentialGeneratorConfiguration<SubaddressGenerationOptions, NoPolicy> =
Object.freeze({
id: "subaddress",
category: "email",
nameKey: "plusAddressedEmail",
descriptionKey: "plusAddressedEmailDesc",
generateKey: "generateEmail",
generatedValueKey: "email",
copyKey: "copyEmail",
onlyOnRequest: false,
request: [],
engine: {
create(
dependencies: GeneratorDependencyProvider,
): CredentialGenerator<SubaddressGenerationOptions> {
return new EmailRandomizer(dependencies.randomizer);
},
},
},
settings: {
initial: DefaultSubaddressOptions,
constraints: {},
account: SUBADDRESS_SETTINGS,
},
policy: {
type: PolicyType.PasswordGenerator,
disabledValue: {},
combine(_acc: NoPolicy, _policy: Policy) {
return {};
settings: {
initial: DefaultSubaddressOptions,
constraints: {},
account: {
key: "subaddressGeneratorSettings",
target: "object",
format: "plain",
classifier: new PublicClassifier<SubaddressGenerationOptions>([
"subaddressType",
"subaddressEmail",
]),
state: GENERATOR_DISK,
initial: {
subaddressType: "random",
subaddressEmail: "",
},
options: {
deserializer: (value) => value,
clearOn: ["logout"],
},
} satisfies ObjectKey<SubaddressGenerationOptions>,
},
createEvaluator(_policy: NoPolicy) {
return new DefaultPolicyEvaluator<SubaddressGenerationOptions>();
policy: {
type: PolicyType.PasswordGenerator,
disabledValue: {},
combine(_acc: NoPolicy, _policy: Policy) {
return {};
},
createEvaluator(_policy: NoPolicy) {
return new DefaultPolicyEvaluator<SubaddressGenerationOptions>();
},
toConstraints(_policy: NoPolicy, email: string) {
return new SubaddressConstraints(email);
},
},
toConstraints(_policy: NoPolicy) {
return new IdentityConstraint<SubaddressGenerationOptions>();
},
},
} satisfies CredentialGeneratorConfiguration<SubaddressGenerationOptions, NoPolicy>);
});
export function toCredentialGeneratorConfiguration<Settings extends ApiSettings = ApiSettings>(
configuration: ForwarderConfiguration<Settings>,

View File

@@ -0,0 +1,45 @@
import { Constraints, StateConstraints } from "@bitwarden/common/tools/types";
import { CatchallGenerationOptions } from "../types";
/** Parses the domain part of an email address
*/
const DOMAIN_PARSER = new RegExp("[^@]+@(?<domain>.+)");
/** A constraint that sets the catchall domain using a fixed email address */
export class CatchallConstraints implements StateConstraints<CatchallGenerationOptions> {
/** Creates a catchall constraints
* @param email - the email address containing the domain.
*/
constructor(email: string) {
if (!email) {
this.domain = "";
return;
}
const parsed = DOMAIN_PARSER.exec(email);
if (parsed && parsed.groups?.domain) {
this.domain = parsed.groups.domain;
}
}
private domain: string;
constraints: Readonly<Constraints<CatchallGenerationOptions>> = {};
adjust(state: CatchallGenerationOptions) {
const currentDomain = (state.catchallDomain ?? "").trim();
if (currentDomain !== "") {
return state;
}
const options = { ...state };
options.catchallDomain = this.domain;
return options;
}
fix(state: CatchallGenerationOptions) {
return state;
}
}

View File

@@ -0,0 +1,34 @@
import { Constraints, StateConstraints } from "@bitwarden/common/tools/types";
import { SubaddressGenerationOptions } from "../types";
/** A constraint that sets the subaddress email using a fixed email address */
export class SubaddressConstraints implements StateConstraints<SubaddressGenerationOptions> {
/** Creates a catchall constraints
* @param email - the email address containing the domain.
*/
constructor(readonly email: string) {
if (!email) {
this.email = "";
}
}
constraints: Readonly<Constraints<SubaddressGenerationOptions>> = {};
adjust(state: SubaddressGenerationOptions) {
const currentDomain = (state.subaddressEmail ?? "").trim();
if (currentDomain !== "") {
return state;
}
const options = { ...state };
options.subaddressEmail = this.email;
return options;
}
fix(state: SubaddressGenerationOptions) {
return state;
}
}

View File

@@ -23,11 +23,12 @@ export function mapPolicyToEvaluator<Policy, Evaluator>(
*/
export function mapPolicyToConstraints<Policy, Evaluator>(
configuration: PolicyConfiguration<Policy, Evaluator>,
email: string,
) {
return pipe(
reduceCollection(configuration.combine, configuration.disabledValue),
distinctIfShallowMatch(),
map(configuration.toConstraints),
map((policy) => configuration.toConstraints(policy, email)),
);
}

View File

@@ -202,6 +202,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const generated = new ObservableTracker(generator.generate$(SomeConfiguration));
@@ -223,6 +224,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const generated = new ObservableTracker(generator.generate$(SomeConfiguration));
@@ -248,6 +250,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const generated = new ObservableTracker(generator.generate$(SomeConfiguration));
@@ -276,6 +279,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const website$ = new BehaviorSubject("some website");
const generated = new ObservableTracker(generator.generate$(SomeConfiguration, { website$ }));
@@ -297,6 +301,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const website$ = new BehaviorSubject("some website");
let error = null;
@@ -322,6 +327,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const website$ = new BehaviorSubject("some website");
let completed = false;
@@ -348,6 +354,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId$ = new BehaviorSubject(AnotherUser).asObservable();
const generated = new ObservableTracker(generator.generate$(SomeConfiguration, { userId$ }));
@@ -368,6 +375,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId = new BehaviorSubject(SomeUser);
const userId$ = userId.pipe(filter((u) => !!u));
@@ -392,6 +400,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId$ = new BehaviorSubject(SomeUser);
let error = null;
@@ -417,6 +426,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId$ = new BehaviorSubject(SomeUser);
let completed = false;
@@ -443,6 +453,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const on$ = new Subject<void>();
const results: any[] = [];
@@ -485,6 +496,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const on$ = new Subject<void>();
let error: any = null;
@@ -511,6 +523,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const on$ = new Subject<void>();
let complete = false;
@@ -542,6 +555,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const result = generator.algorithms("password");
@@ -563,6 +577,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const result = generator.algorithms("username");
@@ -583,6 +598,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const result = generator.algorithms("email");
@@ -604,6 +620,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const result = generator.algorithms(["username", "email"]);
@@ -629,6 +646,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const result = await firstValueFrom(generator.algorithms$("password"));
@@ -646,6 +664,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const result = await firstValueFrom(generator.algorithms$("username"));
@@ -662,6 +681,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const result = await firstValueFrom(generator.algorithms$("email"));
@@ -679,6 +699,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const result = await firstValueFrom(generator.algorithms$(["username", "email"]));
@@ -701,6 +722,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const result = await firstValueFrom(generator.algorithms$(["password"]));
@@ -726,6 +748,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const results: any = [];
const sub = generator.algorithms$("password").subscribe((r) => results.push(r));
@@ -763,6 +786,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId$ = new BehaviorSubject(AnotherUser).asObservable();
@@ -784,6 +808,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId = new BehaviorSubject(SomeUser);
const userId$ = userId.asObservable();
@@ -814,6 +839,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId = new BehaviorSubject(SomeUser);
const userId$ = userId.asObservable();
@@ -840,6 +866,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId = new BehaviorSubject(SomeUser);
const userId$ = userId.asObservable();
@@ -866,6 +893,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId = new BehaviorSubject(SomeUser);
const userId$ = userId.asObservable();
@@ -898,6 +926,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const result = await firstValueFrom(generator.settings$(SomeConfiguration));
@@ -916,6 +945,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const result = await firstValueFrom(generator.settings$(SomeConfiguration));
@@ -936,6 +966,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const result = await firstValueFrom(generator.settings$(SomeConfiguration));
@@ -961,6 +992,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const results: any = [];
const sub = generator.settings$(SomeConfiguration).subscribe((r) => results.push(r));
@@ -986,6 +1018,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId$ = new BehaviorSubject(AnotherUser).asObservable();
@@ -1007,6 +1040,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId = new BehaviorSubject(SomeUser);
const userId$ = userId.asObservable();
@@ -1034,6 +1068,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId = new BehaviorSubject(SomeUser);
const userId$ = userId.asObservable();
@@ -1060,6 +1095,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId = new BehaviorSubject(SomeUser);
const userId$ = userId.asObservable();
@@ -1086,6 +1122,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId = new BehaviorSubject(SomeUser);
const userId$ = userId.asObservable();
@@ -1118,6 +1155,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const subject = await generator.settings(SomeConfiguration, { singleUserId$ });
@@ -1139,6 +1177,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
let completed = false;
@@ -1165,6 +1204,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId$ = new BehaviorSubject(SomeUser).asObservable();
@@ -1182,6 +1222,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId$ = new BehaviorSubject(SomeUser).asObservable();
const policy$ = new BehaviorSubject([somePolicy]);
@@ -1201,6 +1242,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId = new BehaviorSubject(SomeUser);
const userId$ = userId.asObservable();
@@ -1230,6 +1272,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId = new BehaviorSubject(SomeUser);
const userId$ = userId.asObservable();
@@ -1260,6 +1303,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId = new BehaviorSubject(SomeUser);
const userId$ = userId.asObservable();
@@ -1286,6 +1330,7 @@ describe("CredentialGeneratorService", () => {
i18nService,
encryptService,
keyService,
accountService,
);
const userId = new BehaviorSubject(SomeUser);
const userId$ = userId.asObservable();

View File

@@ -23,6 +23,7 @@ import { Simplify } from "type-fest";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { StateProvider } from "@bitwarden/common/platform/state";
@@ -98,6 +99,7 @@ export class CredentialGeneratorService {
private readonly i18nService: I18nService,
private readonly encryptService: EncryptService,
private readonly keyService: KeyService,
private readonly accountService: AccountService,
) {}
private getDependencyProvider(): GeneratorDependencyProvider {
@@ -380,17 +382,30 @@ export class CredentialGeneratorService {
configuration: Configuration<Settings, Policy>,
dependencies: Policy$Dependencies,
): Observable<GeneratorConstraints<Settings>> {
const completion$ = dependencies.userId$.pipe(ignoreElements(), endWith(true));
const email$ = dependencies.userId$.pipe(
distinctUntilChanged(),
withLatestFrom(this.accountService.accounts$),
filter((accounts) => !!accounts),
map(([userId, accounts]) => {
if (userId in accounts) {
return { userId, email: accounts[userId].email };
}
const constraints$ = dependencies.userId$.pipe(
switchMap((userId) => {
// complete policy emissions otherwise `mergeMap` holds `policies$` open indefinitely
return { userId, email: null };
}),
);
const constraints$ = email$.pipe(
switchMap(({ userId, email }) => {
// complete policy emissions otherwise `switchMap` holds `policies$` open indefinitely
const policies$ = this.policyService
.getAll$(configuration.policy.type, userId)
.pipe(takeUntil(completion$));
.pipe(
mapPolicyToConstraints(configuration.policy, email),
takeUntil(anyComplete(email$)),
);
return policies$;
}),
mapPolicyToConstraints(configuration.policy),
);
return constraints$;

View File

@@ -24,9 +24,13 @@ export type PolicyConfiguration<Policy, Settings> = {
createEvaluator: (policy: Policy) => PolicyEvaluator<Policy, Settings>;
/** Converts policy service data into actionable policy constraints.
*
* @param policy - the policy to map into policy constraints.
* @param email - the default email to extend.
*
* @remarks this version includes constraints needed for the reactive forms;
* it was introduced so that the constraints can be incrementally introduced
* as the new UI is built.
*/
toConstraints: (policy: Policy) => GeneratorConstraints<Settings>;
toConstraints: (policy: Policy, email: string) => GeneratorConstraints<Settings>;
};