From bb031f6779fe19afff3b89a8b9aff947c869cd41 Mon Sep 17 00:00:00 2001 From: Joseph Yu <39754176+joseph082@users.noreply.github.com> Date: Fri, 9 Feb 2024 08:07:53 -0800 Subject: [PATCH] [PM-2311] Allow empty passphrase separator (#5473) * Change passphrase generator's default wordSeparator to the empty string '' * Create DefaultPassphraseGenerationOptions * Use DefaultPassphraseGenerationOptions.wordSeparator in passphrase generation * Add `empty` separator option to passphrase generator CLI and an example * Change DefaultPassphraseGenerationOptions numWords to 3 * Use `DefaultPassphraseGenerationOptions.numWords` in CLI passphrase gen --- apps/cli/src/program.ts | 1 + apps/cli/src/tools/generate.command.ts | 13 +++++++++++-- libs/common/src/tools/generator/passphrase/index.ts | 1 + .../passphrase/passphrase-generation-options.ts | 11 ++++++++++- .../passphrase-generator-options-evaluator.spec.ts | 10 ++++++++++ .../passphrase-generator-options-evaluator.ts | 12 +++++++++--- .../password/password-generation.service.ts | 3 --- 7 files changed, 42 insertions(+), 9 deletions(-) diff --git a/apps/cli/src/program.ts b/apps/cli/src/program.ts index a2c6ea8a8d..a79f3847da 100644 --- a/apps/cli/src/program.ts +++ b/apps/cli/src/program.ts @@ -322,6 +322,7 @@ export class Program { writeLn(" bw generate -ul"); writeLn(" bw generate -p --separator _"); writeLn(" bw generate -p --words 5 --separator space"); + writeLn(" bw generate -p --words 5 --separator empty"); writeLn("", true); }) .action(async (options) => { diff --git a/apps/cli/src/tools/generate.command.ts b/apps/cli/src/tools/generate.command.ts index 484dd4f457..4763c6992e 100644 --- a/apps/cli/src/tools/generate.command.ts +++ b/apps/cli/src/tools/generate.command.ts @@ -1,4 +1,5 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { DefaultPassphraseGenerationOptions } from "@bitwarden/common/tools/generator/passphrase"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; import { PasswordGeneratorOptions } from "@bitwarden/common/tools/generator/password/password-generator-options"; @@ -65,8 +66,14 @@ class Options { this.ambiguous = CliUtils.convertBooleanOption(passedOptions?.ambiguous); this.length = CliUtils.convertNumberOption(passedOptions?.length, 14); this.type = passedOptions?.passphrase ? "passphrase" : "password"; - this.separator = CliUtils.convertStringOption(passedOptions?.separator, "-"); - this.words = CliUtils.convertNumberOption(passedOptions?.words, 3); + this.separator = CliUtils.convertStringOption( + passedOptions?.separator, + DefaultPassphraseGenerationOptions.wordSeparator, + ); + this.words = CliUtils.convertNumberOption( + passedOptions?.words, + DefaultPassphraseGenerationOptions.numWords, + ); this.minNumber = CliUtils.convertNumberOption(passedOptions?.minNumber, 1); this.minSpecial = CliUtils.convertNumberOption(passedOptions?.minSpecial, 1); @@ -83,6 +90,8 @@ class Options { } if (this.separator === "space") { this.separator = " "; + } else if (this.separator === "empty") { + this.separator = ""; } else if (this.separator != null && this.separator.length > 1) { this.separator = this.separator[0]; } diff --git a/libs/common/src/tools/generator/passphrase/index.ts b/libs/common/src/tools/generator/passphrase/index.ts index a14ead1e0f..175f15663e 100644 --- a/libs/common/src/tools/generator/passphrase/index.ts +++ b/libs/common/src/tools/generator/passphrase/index.ts @@ -2,3 +2,4 @@ export { PassphraseGeneratorOptionsEvaluator } from "./passphrase-generator-options-evaluator"; export { PassphraseGeneratorPolicy } from "./passphrase-generator-policy"; export { PassphraseGeneratorStrategy } from "./passphrase-generator-strategy"; +export { DefaultPassphraseGenerationOptions } from "./passphrase-generation-options"; diff --git a/libs/common/src/tools/generator/passphrase/passphrase-generation-options.ts b/libs/common/src/tools/generator/passphrase/passphrase-generation-options.ts index 8d2ac78ace..8d6e8eedab 100644 --- a/libs/common/src/tools/generator/passphrase/passphrase-generation-options.ts +++ b/libs/common/src/tools/generator/passphrase/passphrase-generation-options.ts @@ -4,7 +4,7 @@ */ export type PassphraseGenerationOptions = { /** The number of words to include in the passphrase. - * This value defaults to 4. + * This value defaults to 3. */ numWords?: number; @@ -24,3 +24,12 @@ export type PassphraseGenerationOptions = { */ includeNumber?: boolean; }; + +/** The default options for passphrase generation. */ +export const DefaultPassphraseGenerationOptions: Partial = + Object.freeze({ + numWords: 3, + wordSeparator: "-", + capitalize: false, + includeNumber: false, + }); diff --git a/libs/common/src/tools/generator/passphrase/passphrase-generator-options-evaluator.spec.ts b/libs/common/src/tools/generator/passphrase/passphrase-generator-options-evaluator.spec.ts index c939fa9510..b587afbd6e 100644 --- a/libs/common/src/tools/generator/passphrase/passphrase-generator-options-evaluator.spec.ts +++ b/libs/common/src/tools/generator/passphrase/passphrase-generator-options-evaluator.spec.ts @@ -239,6 +239,16 @@ describe("Password generator options builder", () => { expect(sanitizedOptions.wordSeparator).toEqual("-"); }); + it("should leave `wordSeparator` as the empty string '' when it is the empty string", () => { + const policy = Object.assign({}, DisabledPassphraseGeneratorPolicy); + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ wordSeparator: "" }); + + const sanitizedOptions = builder.sanitize(options); + + expect(sanitizedOptions.wordSeparator).toEqual(""); + }); + it("should preserve unknown properties", () => { const policy = Object.assign({}, DisabledPassphraseGeneratorPolicy); const builder = new PassphraseGeneratorOptionsEvaluator(policy); diff --git a/libs/common/src/tools/generator/passphrase/passphrase-generator-options-evaluator.ts b/libs/common/src/tools/generator/passphrase/passphrase-generator-options-evaluator.ts index c4ce2fd226..207ffe8675 100644 --- a/libs/common/src/tools/generator/passphrase/passphrase-generator-options-evaluator.ts +++ b/libs/common/src/tools/generator/passphrase/passphrase-generator-options-evaluator.ts @@ -1,6 +1,9 @@ import { PolicyEvaluator } from "../abstractions/policy-evaluator.abstraction"; -import { PassphraseGenerationOptions } from "./passphrase-generation-options"; +import { + DefaultPassphraseGenerationOptions, + PassphraseGenerationOptions, +} from "./passphrase-generation-options"; import { PassphraseGeneratorPolicy } from "./passphrase-generator-policy"; type Boundary = { @@ -108,8 +111,11 @@ export class PassphraseGeneratorOptionsEvaluator * @returns A passphrase generation request with cascade applied. */ sanitize(options: PassphraseGenerationOptions): PassphraseGenerationOptions { - // ensure words are separated by a single character - const wordSeparator = options.wordSeparator?.[0] ?? "-"; + // ensure words are separated by a single character or the empty string + const wordSeparator = + options.wordSeparator === "" + ? "" + : options.wordSeparator?.[0] ?? DefaultPassphraseGenerationOptions.wordSeparator; return { ...options, diff --git a/libs/common/src/tools/generator/password/password-generation.service.ts b/libs/common/src/tools/generator/password/password-generation.service.ts index a66c6d7201..831ec20278 100644 --- a/libs/common/src/tools/generator/password/password-generation.service.ts +++ b/libs/common/src/tools/generator/password/password-generation.service.ts @@ -147,9 +147,6 @@ export class PasswordGenerationService implements PasswordGenerationServiceAbstr if (o.numWords == null || o.numWords <= 2) { o.numWords = DefaultOptions.numWords; } - if (o.wordSeparator == null || o.wordSeparator.length === 0 || o.wordSeparator.length > 1) { - o.wordSeparator = " "; - } if (o.capitalize == null) { o.capitalize = false; }