From 52f6b2402029a8a262eeb3805376200f90b68064 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=9C=A8=20Audrey=20=E2=9C=A8?= Date: Mon, 22 Jul 2024 15:22:08 -0400 Subject: [PATCH] [PM-9118] fix duplicate website name (#9782) --- .../components/generator.component.ts | 9 +- ...legacy-username-generation.service.spec.ts | 40 ++++++++- .../src/legacy-username-generation.service.ts | 85 +++++++++++++++---- 3 files changed, 112 insertions(+), 22 deletions(-) diff --git a/libs/angular/src/tools/generator/components/generator.component.ts b/libs/angular/src/tools/generator/components/generator.component.ts index e17ac7071c..94bd3fc287 100644 --- a/libs/angular/src/tools/generator/components/generator.component.ts +++ b/libs/angular/src/tools/generator/components/generator.component.ts @@ -151,9 +151,6 @@ export class GeneratorComponent implements OnInit, OnDestroy { this.usernameOptions.subaddressType = this.usernameOptions.catchallType = "random"; } else { this.usernameOptions.website = this.usernameWebsite; - const websiteOption = { name: this.i18nService.t("websiteName"), value: "website-name" }; - this.subaddressOptions.push(websiteOption); - this.catchallOptions.push(websiteOption); } } @@ -201,6 +198,12 @@ export class GeneratorComponent implements OnInit, OnDestroy { takeUntil(this.destroy$), ), ); + + if (this.usernameWebsite !== null) { + const websiteOption = { name: this.i18nService.t("websiteName"), value: "website-name" }; + this.subaddressOptions.push(websiteOption); + this.catchallOptions.push(websiteOption); + } } ngOnDestroy() { diff --git a/libs/tools/generator/extensions/legacy/src/legacy-username-generation.service.spec.ts b/libs/tools/generator/extensions/legacy/src/legacy-username-generation.service.spec.ts index 1391381bd0..ebe5f9074c 100644 --- a/libs/tools/generator/extensions/legacy/src/legacy-username-generation.service.spec.ts +++ b/libs/tools/generator/extensions/legacy/src/legacy-username-generation.service.spec.ts @@ -638,6 +638,10 @@ describe("LegacyUsernameGenerationService", () => { }); describe("saveOptions", () => { + // this test is awful, but the coupling of the legacy username generator + // would cause the test file's size to bloat to ~2000 loc. Since the legacy + // generators are actively being rewritten, this heinous test seemed the lesser + // of two evils. it("saves option sets to its inner generators", async () => { const account = mockAccountServiceWith(SomeUser); const navigation = createNavigationGenerator({ type: "password" }); @@ -665,7 +669,7 @@ describe("LegacyUsernameGenerationService", () => { simpleLogin, ); - await generator.saveOptions({ + const options: UsernameGeneratorOptions = { type: "catchall", wordCapitalize: true, wordIncludeNumber: false, @@ -685,7 +689,9 @@ describe("LegacyUsernameGenerationService", () => { forwardedSimpleLoginApiKey: "simpleLoginToken", forwardedSimpleLoginBaseUrl: "https://simplelogin.api.example.com", website: null, - }); + }; + + await generator.saveOptions(options); expect(navigation.saveOptions).toHaveBeenCalledWith(SomeUser, { type: "password", @@ -699,18 +705,28 @@ describe("LegacyUsernameGenerationService", () => { website: null, }); + options.type = "word"; + await generator.saveOptions(options); + expect(effUsername.saveOptions).toHaveBeenCalledWith(SomeUser, { wordCapitalize: true, wordIncludeNumber: false, website: null, }); + options.type = "subaddress"; + await generator.saveOptions(options); + expect(subaddress.saveOptions).toHaveBeenCalledWith(SomeUser, { subaddressType: "random", subaddressEmail: "foo@example.com", website: null, }); + options.type = "forwarded"; + options.forwardedService = "anonaddy"; + await generator.saveOptions(options); + expect(addyIo.saveOptions).toHaveBeenCalledWith(SomeUser, { token: "addyIoToken", domain: "addyio.example.com", @@ -718,27 +734,47 @@ describe("LegacyUsernameGenerationService", () => { website: null, }); + options.type = "forwarded"; + options.forwardedService = "duckduckgo"; + await generator.saveOptions(options); + expect(duckDuckGo.saveOptions).toHaveBeenCalledWith(SomeUser, { token: "ddgToken", website: null, }); + options.type = "forwarded"; + options.forwardedService = "fastmail"; + await generator.saveOptions(options); + expect(fastmail.saveOptions).toHaveBeenCalledWith(SomeUser, { token: "fastmailToken", website: null, }); + options.type = "forwarded"; + options.forwardedService = "firefoxrelay"; + await generator.saveOptions(options); + expect(firefoxRelay.saveOptions).toHaveBeenCalledWith(SomeUser, { token: "firefoxToken", website: null, }); + options.type = "forwarded"; + options.forwardedService = "forwardemail"; + await generator.saveOptions(options); + expect(forwardEmail.saveOptions).toHaveBeenCalledWith(SomeUser, { token: "forwardEmailToken", domain: "example.com", website: null, }); + options.type = "forwarded"; + options.forwardedService = "simplelogin"; + await generator.saveOptions(options); + expect(simpleLogin.saveOptions).toHaveBeenCalledWith(SomeUser, { token: "simpleLoginToken", baseUrl: "https://simplelogin.api.example.com", diff --git a/libs/tools/generator/extensions/legacy/src/legacy-username-generation.service.ts b/libs/tools/generator/extensions/legacy/src/legacy-username-generation.service.ts index fa1dbf9495..ee9e5bce31 100644 --- a/libs/tools/generator/extensions/legacy/src/legacy-username-generation.service.ts +++ b/libs/tools/generator/extensions/legacy/src/legacy-username-generation.service.ts @@ -1,6 +1,7 @@ import { zip, firstValueFrom, map, concatMap, combineLatest } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { UserId } from "@bitwarden/common/types/guid"; import { ApiOptions, EmailDomainOptions, @@ -13,6 +14,8 @@ import { EffUsernameGenerationOptions, Forwarders, SubaddressGenerationOptions, + UsernameGeneratorType, + ForwarderId, } from "@bitwarden/generator-core"; import { GeneratorNavigationService, GeneratorNavigation } from "@bitwarden/generator-navigation"; @@ -178,28 +181,76 @@ export class LegacyUsernameGenerationService implements UsernameGenerationServic const stored = this.toStoredOptions(options); const activeAccount = await firstValueFrom(this.accountService.activeAccount$); + const saved = await this.saveGeneratorOptions(activeAccount.id, options.type, stored); + if (!saved) { + await this.saveForwarderOptions(activeAccount.id, options.forwardedService, stored); + } + + // run navigation options 2nd so that navigation options update doesn't race the `saved options` + // update in Firefox. + await this.saveNavigationOptions(activeAccount.id, stored); + } + + private async saveNavigationOptions(account: UserId, options: MappedOptions) { // generator settings needs to preserve whether password or passphrase is selected, // so `navigationOptions` is mutated. const navigationOptions$ = zip( - this.navigation.options$(activeAccount.id), - this.navigation.defaults$(activeAccount.id), + this.navigation.options$(account), + this.navigation.defaults$(account), ).pipe(map(([options, defaults]) => options ?? defaults)); - let navigationOptions = await firstValueFrom(navigationOptions$); - navigationOptions = Object.assign(navigationOptions, stored.generator); - await this.navigation.saveOptions(activeAccount.id, navigationOptions); - // overwrite all other settings with latest values - await Promise.all([ - this.catchall.saveOptions(activeAccount.id, stored.algorithms.catchall), - this.effUsername.saveOptions(activeAccount.id, stored.algorithms.effUsername), - this.subaddress.saveOptions(activeAccount.id, stored.algorithms.subaddress), - this.addyIo.saveOptions(activeAccount.id, stored.forwarders.addyIo), - this.duckDuckGo.saveOptions(activeAccount.id, stored.forwarders.duckDuckGo), - this.fastmail.saveOptions(activeAccount.id, stored.forwarders.fastmail), - this.firefoxRelay.saveOptions(activeAccount.id, stored.forwarders.firefoxRelay), - this.forwardEmail.saveOptions(activeAccount.id, stored.forwarders.forwardEmail), - this.simpleLogin.saveOptions(activeAccount.id, stored.forwarders.simpleLogin), - ]); + let navigationOptions = await firstValueFrom(navigationOptions$); + navigationOptions = Object.assign(navigationOptions, options.generator); + await this.navigation.saveOptions(account, navigationOptions); + } + + private async saveGeneratorOptions( + account: UserId, + type: UsernameGeneratorType, + options: MappedOptions, + ) { + switch (type) { + case "word": + await this.effUsername.saveOptions(account, options.algorithms.effUsername); + return true; + case "subaddress": + await this.subaddress.saveOptions(account, options.algorithms.subaddress); + return true; + case "catchall": + await this.catchall.saveOptions(account, options.algorithms.catchall); + return true; + default: + return false; + } + } + + private async saveForwarderOptions( + account: UserId, + forwarder: ForwarderId | "", + options: MappedOptions, + ) { + switch (forwarder) { + case "anonaddy": + await this.addyIo.saveOptions(account, options.forwarders.addyIo); + return true; + case "duckduckgo": + await this.duckDuckGo.saveOptions(account, options.forwarders.duckDuckGo); + return true; + case "fastmail": + await this.fastmail.saveOptions(account, options.forwarders.fastmail); + return true; + case "firefoxrelay": + await this.firefoxRelay.saveOptions(account, options.forwarders.firefoxRelay); + return true; + case "forwardemail": + await this.forwardEmail.saveOptions(account, options.forwarders.forwardEmail); + return true; + case "simplelogin": + await this.simpleLogin.saveOptions(account, options.forwarders.simpleLogin); + return true; + default: + return false; + } } private toStoredOptions(options: UsernameGeneratorOptions) {