From 02993f7018199557d5b8faec56f5837947694f83 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Fri, 4 Oct 2024 08:39:29 -0500 Subject: [PATCH] [PM-10439] Vault generator components (#11350) * add password/username generator strings to web app * remove JSLibServicesModule from generator dependencies * integrate username and password generators into browser extension * integrate username and password generators into web app * move Jslib order back * remove margin from within the generator components * add alternative background to web generator dialog * remove unneeded margin from username generator * remove disable margin from div * Revert "remove unneeded margin from username generator" This reverts commit ae3067e3f2cbd1a4cd15d92b13297b128d63d5c5. * remove CSS to alter internal margins of the generator components --- .../web-generator-dialog.component.html | 2 +- .../web-generator-dialog.component.spec.ts | 56 ++-- apps/web/src/locales/en/messages.json | 36 +++ .../generator/components/src/dependencies.ts | 2 - .../cipher-form-generator.component.html | 70 +---- .../cipher-form-generator.component.spec.ts | 244 +++++------------- .../cipher-form-generator.component.ts | 167 +----------- 7 files changed, 139 insertions(+), 438 deletions(-) diff --git a/apps/web/src/app/vault/components/web-generator-dialog/web-generator-dialog.component.html b/apps/web/src/app/vault/components/web-generator-dialog/web-generator-dialog.component.html index 30259cd640..afe62cdc8a 100644 --- a/apps/web/src/app/vault/components/web-generator-dialog/web-generator-dialog.component.html +++ b/apps/web/src/app/vault/components/web-generator-dialog/web-generator-dialog.component.html @@ -1,4 +1,4 @@ - + {{ title }} diff --git a/apps/web/src/app/vault/components/web-generator-dialog/web-generator-dialog.component.spec.ts b/apps/web/src/app/vault/components/web-generator-dialog/web-generator-dialog.component.spec.ts index 653f553313..818fb74963 100644 --- a/apps/web/src/app/vault/components/web-generator-dialog/web-generator-dialog.component.spec.ts +++ b/apps/web/src/app/vault/components/web-generator-dialog/web-generator-dialog.component.spec.ts @@ -1,15 +1,11 @@ -import { DIALOG_DATA, DialogRef } from "@angular/cdk/dialog"; +import { DialogRef, DIALOG_DATA } from "@angular/cdk/dialog"; +import { Component, EventEmitter, Input, Output } from "@angular/core"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { NoopAnimationsModule } from "@angular/platform-browser/animations"; import { mock, MockProxy } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { - PasswordGenerationServiceAbstraction, - UsernameGenerationServiceAbstraction, -} from "@bitwarden/generator-legacy"; import { CipherFormGeneratorComponent } from "@bitwarden/vault"; import { @@ -18,32 +14,26 @@ import { WebVaultGeneratorDialogParams, } from "./web-generator-dialog.component"; +@Component({ + selector: "vault-cipher-form-generator", + template: "", + standalone: true, +}) +class MockCipherFormGenerator { + @Input() type: "password" | "username"; + @Output() valueGenerated = new EventEmitter(); +} + describe("WebVaultGeneratorDialogComponent", () => { let component: WebVaultGeneratorDialogComponent; let fixture: ComponentFixture; let dialogRef: MockProxy>; let mockI18nService: MockProxy; - let passwordOptionsSubject: BehaviorSubject; - let usernameOptionsSubject: BehaviorSubject; - let mockPasswordGenerationService: MockProxy; - let mockUsernameGenerationService: MockProxy; beforeEach(async () => { dialogRef = mock>(); mockI18nService = mock(); - passwordOptionsSubject = new BehaviorSubject([{ type: "password" }]); - usernameOptionsSubject = new BehaviorSubject([{ type: "username" }]); - - mockPasswordGenerationService = mock(); - mockPasswordGenerationService.getOptions$.mockReturnValue( - passwordOptionsSubject.asObservable(), - ); - - mockUsernameGenerationService = mock(); - mockUsernameGenerationService.getOptions$.mockReturnValue( - usernameOptionsSubject.asObservable(), - ); const mockDialogData: WebVaultGeneratorDialogParams = { type: "password" }; @@ -66,23 +56,13 @@ describe("WebVaultGeneratorDialogComponent", () => { provide: PlatformUtilsService, useValue: mock(), }, - { - provide: PasswordGenerationServiceAbstraction, - useValue: mockPasswordGenerationService, - }, - { - provide: UsernameGenerationServiceAbstraction, - useValue: mockUsernameGenerationService, - }, - { - provide: CipherFormGeneratorComponent, - useValue: { - passwordOptions$: passwordOptionsSubject.asObservable(), - usernameOptions$: usernameOptionsSubject.asObservable(), - }, - }, ], - }).compileComponents(); + }) + .overrideComponent(WebVaultGeneratorDialogComponent, { + remove: { imports: [CipherFormGeneratorComponent] }, + add: { imports: [MockCipherFormGenerator] }, + }) + .compileComponents(); fixture = TestBed.createComponent(WebVaultGeneratorDialogComponent); component = fixture.componentInstance; diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index bad4b54805..3b6eff4439 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -9266,6 +9266,42 @@ "editAccess": { "message": "Edit access" }, + "uppercaseDescription": { + "message": "Include uppercase characters", + "description": "Tooltip for the password generator uppercase character checkbox" + }, + "uppercaseLabel": { + "message": "A-Z", + "description": "Label for the password generator uppercase character checkbox" + }, + "lowercaseDescription": { + "message": "Include lowercase characters", + "description": "Full description for the password generator lowercase character checkbox" + }, + "lowercaseLabel": { + "message": "a-z", + "description": "Label for the password generator lowercase character checkbox" + }, + "numbersDescription": { + "message": "Include numbers", + "description": "Full description for the password generator numbers checkbox" + }, + "numbersLabel": { + "message": "0-9", + "description": "Label for the password generator numbers checkbox" + }, + "specialCharactersDescription": { + "message": "Include special characters", + "description": "Full description for the password generator special characters checkbox" + }, + "specialCharactersLabel": { + "message": "!@#$%^&*", + "description": "Label for the password generator special characters checkbox" + }, + "avoidAmbiguous": { + "message": "Avoid ambiguous characters", + "description": "Label for the avoid ambiguous characters checkbox." + }, "addAttachment": { "message": "Add attachment" }, diff --git a/libs/tools/generator/components/src/dependencies.ts b/libs/tools/generator/components/src/dependencies.ts index ba9219b237..6f2c13e057 100644 --- a/libs/tools/generator/components/src/dependencies.ts +++ b/libs/tools/generator/components/src/dependencies.ts @@ -5,7 +5,6 @@ import { ReactiveFormsModule } from "@angular/forms"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { safeProvider } from "@bitwarden/angular/platform/utils/safe-provider"; import { SafeInjectionToken } from "@bitwarden/angular/services/injection-tokens"; -import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { StateProvider } from "@bitwarden/common/platform/state"; @@ -43,7 +42,6 @@ const RANDOMIZER = new SafeInjectionToken("Randomizer"); InputModule, ItemModule, JslibModule, - JslibServicesModule, ReactiveFormsModule, SectionComponent, SectionHeaderComponent, diff --git a/libs/vault/src/cipher-form/components/cipher-generator/cipher-form-generator.component.html b/libs/vault/src/cipher-form/components/cipher-generator/cipher-form-generator.component.html index 0a375d5ae5..181ca50da8 100644 --- a/libs/vault/src/cipher-form/components/cipher-generator/cipher-form-generator.component.html +++ b/libs/vault/src/cipher-form/components/cipher-generator/cipher-form-generator.component.html @@ -1,62 +1,8 @@ - - - - - {{ "password" | i18n }} - - - {{ "passphrase" | i18n }} - - - - - - - - - - - - - - - - - - - - - - - -

{{ "options" | i18n }}

-
- - Placeholder: Replace with Generator Options Component(s) when available - -
+ + diff --git a/libs/vault/src/cipher-form/components/cipher-generator/cipher-form-generator.component.spec.ts b/libs/vault/src/cipher-form/components/cipher-generator/cipher-form-generator.component.spec.ts index 85ace2f0ac..48b1250eb1 100644 --- a/libs/vault/src/cipher-form/components/cipher-generator/cipher-form-generator.component.spec.ts +++ b/libs/vault/src/cipher-form/components/cipher-generator/cipher-form-generator.component.spec.ts @@ -1,217 +1,103 @@ -import { ComponentFixture, fakeAsync, TestBed, tick } from "@angular/core/testing"; -import { mock, MockProxy } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; +import { Component, EventEmitter, Output } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { By } from "@angular/platform-browser"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { - PasswordGenerationServiceAbstraction, - PasswordGeneratorOptions, - UsernameGenerationServiceAbstraction, - UsernameGeneratorOptions, -} from "@bitwarden/generator-legacy"; + PasswordGeneratorComponent, + UsernameGeneratorComponent, +} from "@bitwarden/generator-components"; import { CipherFormGeneratorComponent } from "@bitwarden/vault"; +@Component({ + selector: "tools-password-generator", + template: ``, + standalone: true, +}) +class MockPasswordGeneratorComponent { + @Output() onGenerated = new EventEmitter(); +} + +@Component({ + selector: "tools-username-generator", + template: ``, + standalone: true, +}) +class MockUsernameGeneratorComponent { + @Output() onGenerated = new EventEmitter(); +} + describe("CipherFormGeneratorComponent", () => { let component: CipherFormGeneratorComponent; let fixture: ComponentFixture; - let mockLegacyPasswordGenerationService: MockProxy; - let mockLegacyUsernameGenerationService: MockProxy; - let mockPlatformUtilsService: MockProxy; - - let passwordOptions$: BehaviorSubject; - let usernameOptions$: BehaviorSubject; - beforeEach(async () => { - passwordOptions$ = new BehaviorSubject([ - { - type: "password", - }, - ] as [PasswordGeneratorOptions]); - usernameOptions$ = new BehaviorSubject([ - { - type: "word", - }, - ] as [UsernameGeneratorOptions]); - - mockPlatformUtilsService = mock(); - - mockLegacyPasswordGenerationService = mock(); - mockLegacyPasswordGenerationService.getOptions$.mockReturnValue(passwordOptions$); - - mockLegacyUsernameGenerationService = mock(); - mockLegacyUsernameGenerationService.getOptions$.mockReturnValue(usernameOptions$); - await TestBed.configureTestingModule({ imports: [CipherFormGeneratorComponent], - providers: [ - { provide: I18nService, useValue: { t: (key: string) => key } }, - { - provide: PasswordGenerationServiceAbstraction, - useValue: mockLegacyPasswordGenerationService, - }, - { - provide: UsernameGenerationServiceAbstraction, - useValue: mockLegacyUsernameGenerationService, - }, - { provide: PlatformUtilsService, useValue: mockPlatformUtilsService }, - ], - }).compileComponents(); + providers: [{ provide: I18nService, useValue: { t: (key: string) => key } }], + }) + .overrideComponent(CipherFormGeneratorComponent, { + remove: { imports: [PasswordGeneratorComponent, UsernameGeneratorComponent] }, + add: { imports: [MockPasswordGeneratorComponent, MockUsernameGeneratorComponent] }, + }) + .compileComponents(); fixture = TestBed.createComponent(CipherFormGeneratorComponent); component = fixture.componentInstance; - }); - - it("should create", () => { fixture.detectChanges(); - expect(component).toBeTruthy(); }); - it("should use the appropriate text based on generator type", () => { - component.type = "password"; - component.ngOnChanges(); - expect(component["regenerateButtonTitle"]).toBe("regeneratePassword"); - - component.type = "username"; - component.ngOnChanges(); - expect(component["regenerateButtonTitle"]).toBe("regenerateUsername"); - }); - - it("should emit regenerate$ when user clicks the regenerate button", fakeAsync(() => { - const regenerateSpy = jest.spyOn(component["regenerate$"], "next"); - - fixture.nativeElement.querySelector("button[data-testid='regenerate-button']").click(); - - expect(regenerateSpy).toHaveBeenCalled(); - })); - - it("should emit valueGenerated whenever a new value is generated", fakeAsync(() => { - const valueGeneratedSpy = jest.spyOn(component.valueGenerated, "emit"); - - mockLegacyPasswordGenerationService.generatePassword.mockResolvedValue("generated-password"); - component.type = "password"; - - component.ngOnChanges(); - tick(); - - expect(valueGeneratedSpy).toHaveBeenCalledWith("generated-password"); - })); - describe("password generation", () => { + let passwordGenerator: MockPasswordGeneratorComponent; + beforeEach(() => { component.type = "password"; - }); - - it("should update the generated value when the password options change", fakeAsync(() => { - mockLegacyPasswordGenerationService.generatePassword - .mockResolvedValueOnce("first-password") - .mockResolvedValueOnce("second-password"); - - component.ngOnChanges(); - tick(); - - expect(component["generatedValue"]).toBe("first-password"); - - passwordOptions$.next([{ type: "password" }]); - tick(); - - expect(component["generatedValue"]).toBe("second-password"); - expect(mockLegacyPasswordGenerationService.generatePassword).toHaveBeenCalledTimes(2); - })); - - it("should show password type toggle when the generator type is password", () => { fixture.detectChanges(); - expect(fixture.nativeElement.querySelector("bit-toggle-group")).toBeTruthy(); + passwordGenerator = fixture.debugElement.query( + By.directive(MockPasswordGeneratorComponent), + ).componentInstance; }); - it("should update the generated value when the password type is updated", fakeAsync(async () => { - mockLegacyPasswordGenerationService.generatePassword - .mockResolvedValueOnce("first-password") - .mockResolvedValueOnce("second-password"); + it("only shows `PasswordGeneratorComponent`", () => { + expect(passwordGenerator).toBeTruthy(); + expect(fixture.debugElement.query(By.directive(MockUsernameGeneratorComponent))).toBeNull(); + }); - component.ngOnChanges(); - tick(); + it("invokes `valueGenerated` with the generated credential", () => { + jest.spyOn(component.valueGenerated, "emit"); - expect(component["generatedValue"]).toBe("first-password"); + passwordGenerator.onGenerated.emit({ credential: "new-cred-password!" }); - await component["updatePasswordType"]("passphrase"); - tick(); - - expect(component["generatedValue"]).toBe("second-password"); - expect(mockLegacyPasswordGenerationService.generatePassword).toHaveBeenCalledTimes(2); - })); - - it("should update the password history when a new password is generated", fakeAsync(() => { - mockLegacyPasswordGenerationService.generatePassword.mockResolvedValue("new-password"); - - component.ngOnChanges(); - tick(); - - expect(mockLegacyPasswordGenerationService.generatePassword).toHaveBeenCalledTimes(1); - expect(mockLegacyPasswordGenerationService.addHistory).toHaveBeenCalledWith("new-password"); - expect(component["generatedValue"]).toBe("new-password"); - })); - - it("should regenerate the password when regenerate$ emits", fakeAsync(() => { - mockLegacyPasswordGenerationService.generatePassword - .mockResolvedValueOnce("first-password") - .mockResolvedValueOnce("second-password"); - - component.ngOnChanges(); - tick(); - - expect(component["generatedValue"]).toBe("first-password"); - - component["regenerate$"].next(); - tick(); - - expect(component["generatedValue"]).toBe("second-password"); - })); + expect(component.valueGenerated.emit).toHaveBeenCalledTimes(1); + expect(component.valueGenerated.emit).toHaveBeenCalledWith("new-cred-password!"); + }); }); describe("username generation", () => { + let usernameGenerator: MockUsernameGeneratorComponent; + beforeEach(() => { component.type = "username"; - }); - - it("should update the generated value when the username options change", fakeAsync(() => { - mockLegacyUsernameGenerationService.generateUsername - .mockResolvedValueOnce("first-username") - .mockResolvedValueOnce("second-username"); - - component.ngOnChanges(); - tick(); - - expect(component["generatedValue"]).toBe("first-username"); - - usernameOptions$.next([{ type: "word" }]); - tick(); - - expect(component["generatedValue"]).toBe("second-username"); - })); - - it("should regenerate the username when regenerate$ emits", fakeAsync(() => { - mockLegacyUsernameGenerationService.generateUsername - .mockResolvedValueOnce("first-username") - .mockResolvedValueOnce("second-username"); - - component.ngOnChanges(); - tick(); - - expect(component["generatedValue"]).toBe("first-username"); - - component["regenerate$"].next(); - tick(); - - expect(component["generatedValue"]).toBe("second-username"); - })); - - it("should not show password type toggle when the generator type is username", () => { fixture.detectChanges(); - expect(fixture.nativeElement.querySelector("bit-toggle-group")).toBeNull(); + usernameGenerator = fixture.debugElement.query( + By.directive(MockUsernameGeneratorComponent), + ).componentInstance; + }); + + it("only shows `UsernameGeneratorComponent`", () => { + expect(usernameGenerator).toBeTruthy(); + expect(fixture.debugElement.query(By.directive(MockPasswordGeneratorComponent))).toBeNull(); + }); + + it("invokes `valueGenerated` with the generated credential", () => { + jest.spyOn(component.valueGenerated, "emit"); + + usernameGenerator.onGenerated.emit({ credential: "new-cred-username!" }); + + expect(component.valueGenerated.emit).toHaveBeenCalledTimes(1); + expect(component.valueGenerated.emit).toHaveBeenCalledWith("new-cred-username!"); }); }); }); diff --git a/libs/vault/src/cipher-form/components/cipher-generator/cipher-form-generator.component.ts b/libs/vault/src/cipher-form/components/cipher-generator/cipher-form-generator.component.ts index 7d93ca20d9..ee06e601ad 100644 --- a/libs/vault/src/cipher-form/components/cipher-generator/cipher-form-generator.component.ts +++ b/libs/vault/src/cipher-form/components/cipher-generator/cipher-form-generator.component.ts @@ -1,36 +1,12 @@ import { CommonModule } from "@angular/common"; -import { Component, DestroyRef, EventEmitter, Input, OnChanges, Output } from "@angular/core"; -import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; -import { - combineLatest, - map, - merge, - shareReplay, - startWith, - Subject, - Subscription, - switchMap, - take, - tap, -} from "rxjs"; +import { Component, EventEmitter, Input, Output } from "@angular/core"; -import { JslibModule } from "@bitwarden/angular/jslib.module"; -import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { SectionComponent } from "@bitwarden/components"; import { - CardComponent, - ColorPasswordModule, - IconButtonModule, - ItemModule, - SectionComponent, - SectionHeaderComponent, - ToggleGroupModule, - TypographyModule, -} from "@bitwarden/components"; -import { GeneratorType } from "@bitwarden/generator-core"; -import { - PasswordGenerationServiceAbstraction, - UsernameGenerationServiceAbstraction, -} from "@bitwarden/generator-legacy"; + PasswordGeneratorComponent, + UsernameGeneratorComponent, +} from "@bitwarden/generator-components"; +import { GeneratedCredential } from "@bitwarden/generator-core"; /** * Renders a password or username generator UI and emits the most recently generated value. @@ -40,20 +16,9 @@ import { selector: "vault-cipher-form-generator", templateUrl: "./cipher-form-generator.component.html", standalone: true, - imports: [ - CommonModule, - CardComponent, - SectionComponent, - ToggleGroupModule, - JslibModule, - ItemModule, - ColorPasswordModule, - IconButtonModule, - SectionHeaderComponent, - TypographyModule, - ], + imports: [CommonModule, SectionComponent, PasswordGeneratorComponent, UsernameGeneratorComponent], }) -export class CipherFormGeneratorComponent implements OnChanges { +export class CipherFormGeneratorComponent { /** * The type of generator form to show. */ @@ -66,118 +31,8 @@ export class CipherFormGeneratorComponent implements OnChanges { @Output() valueGenerated = new EventEmitter(); - protected get isPassword() { - return this.type === "password"; - } - - protected regenerateButtonTitle: string; - protected regenerate$ = new Subject(); - protected passwordTypeSubject$ = new Subject(); - /** - * The currently generated value displayed to the user. - * @protected - */ - protected generatedValue: string = ""; - - /** - * The current username generation options. - * @private - */ - private usernameOptions$ = this.legacyUsernameGenerationService.getOptions$(); - - /** - * The current password type selected in the UI. Starts with the saved value from the service. - * @protected - */ - protected passwordType$ = merge( - this.legacyPasswordGenerationService.getOptions$().pipe( - take(1), - map(([options]) => options.type), - ), - this.passwordTypeSubject$, - ).pipe(shareReplay({ bufferSize: 1, refCount: false })); - - /** - * The current password generation options. - * @private - */ - private passwordOptions$ = combineLatest([ - this.legacyPasswordGenerationService.getOptions$(), - this.passwordType$, - ]).pipe( - map(([[options], type]) => { - options.type = type; - return options; - }), - ); - - /** - * Tracks the regenerate$ subscription - * @private - */ - private subscription: Subscription | null; - - constructor( - private i18nService: I18nService, - private legacyPasswordGenerationService: PasswordGenerationServiceAbstraction, - private legacyUsernameGenerationService: UsernameGenerationServiceAbstraction, - private destroyRef: DestroyRef, - ) {} - - ngOnChanges() { - this.regenerateButtonTitle = this.i18nService.t( - this.isPassword ? "regeneratePassword" : "regenerateUsername", - ); - - // If we have a previous subscription, clear it - if (this.subscription) { - this.subscription.unsubscribe(); - this.subscription = null; - } - - if (this.isPassword) { - this.setupPasswordGeneration(); - } else { - this.setupUsernameGeneration(); - } - } - - private setupPasswordGeneration() { - this.subscription = this.regenerate$ - .pipe( - startWith(null), - switchMap(() => this.passwordOptions$), - switchMap((options) => this.legacyPasswordGenerationService.generatePassword(options)), - tap(async (password) => { - await this.legacyPasswordGenerationService.addHistory(password); - }), - takeUntilDestroyed(this.destroyRef), - ) - .subscribe((password) => { - this.generatedValue = password; - this.valueGenerated.emit(password); - }); - } - - private setupUsernameGeneration() { - this.subscription = this.regenerate$ - .pipe( - startWith(null), - switchMap(() => this.usernameOptions$), - switchMap((options) => this.legacyUsernameGenerationService.generateUsername(options)), - takeUntilDestroyed(this.destroyRef), - ) - .subscribe((username) => { - this.generatedValue = username; - this.valueGenerated.emit(username); - }); - } - - /** - * Switch the password generation type. - * @param value The new password generation type. - */ - protected updatePasswordType = async (value: GeneratorType) => { - this.passwordTypeSubject$.next(value); + /** Event handler for both generation components */ + onCredentialGenerated = (generatedCred: GeneratedCredential) => { + this.valueGenerated.emit(generatedCred.credential); }; }