From a2a15d42d5c2501d70fed66e32f5b0210447d405 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=9C=A8=20Audrey=20=E2=9C=A8?= Date: Wed, 23 Oct 2024 14:58:49 -0400 Subject: [PATCH 1/5] add test ids (#11674) --- .../src/credential-generator.component.html | 14 ++++++++++++-- .../src/username-generator.component.html | 14 ++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/libs/tools/generator/components/src/credential-generator.component.html b/libs/tools/generator/components/src/credential-generator.component.html index 06ea1f767b7..737e32fa1f9 100644 --- a/libs/tools/generator/components/src/credential-generator.component.html +++ b/libs/tools/generator/components/src/credential-generator.component.html @@ -56,7 +56,12 @@
{{ "type" | i18n }} - + + {{ credentialTypeHint$ | async }} @@ -65,7 +70,12 @@ {{ "service" | i18n }} - + + {{ "type" | i18n }} - + + {{ credentialTypeHint$ | async }} @@ -42,7 +47,12 @@
{{ "service" | i18n }} - + +
Date: Wed, 23 Oct 2024 15:38:26 -0400 Subject: [PATCH 2/5] [PM-13723] track history in generator components (#11673) * add history support to generator components * increase generator history length --- .../src/credential-generator.component.ts | 11 ++++++- .../src/password-generator.component.ts | 31 ++++++++++++++++++- .../src/username-generator.component.ts | 11 ++++++- .../history/src/generated-credential.ts | 4 +-- .../src/generator-history.abstraction.ts | 4 +-- .../src/local-generator-history.service.ts | 12 +++++-- 6 files changed, 63 insertions(+), 10 deletions(-) diff --git a/libs/tools/generator/components/src/credential-generator.component.ts b/libs/tools/generator/components/src/credential-generator.component.ts index a37de986499..e800ce4bd39 100644 --- a/libs/tools/generator/components/src/credential-generator.component.ts +++ b/libs/tools/generator/components/src/credential-generator.component.ts @@ -37,6 +37,7 @@ import { isUsernameAlgorithm, toCredentialGeneratorConfiguration, } from "@bitwarden/generator-core"; +import { GeneratorHistoryService } from "@bitwarden/generator-history"; // constants used to identify navigation selections that are not // generator algorithms @@ -51,6 +52,7 @@ const NONE_SELECTED = "none"; export class CredentialGeneratorComponent implements OnInit, OnDestroy { constructor( private generatorService: CredentialGeneratorService, + private generatorHistoryService: GeneratorHistoryService, private toastService: ToastService, private logService: LogService, private i18nService: I18nService, @@ -182,9 +184,16 @@ export class CredentialGeneratorComponent implements OnInit, OnDestroy { // continue with origin stream return generator; }), + withLatestFrom(this.userId$), takeUntil(this.destroyed), ) - .subscribe((generated) => { + .subscribe(([generated, userId]) => { + this.generatorHistoryService + .track(userId, generated.credential, generated.category, generated.generationDate) + .catch((e: unknown) => { + this.logService.error(e); + }); + // update subjects within the angular zone so that the // template bindings refresh immediately this.zone.run(() => { diff --git a/libs/tools/generator/components/src/password-generator.component.ts b/libs/tools/generator/components/src/password-generator.component.ts index 96af1b05c13..60c3f629538 100644 --- a/libs/tools/generator/components/src/password-generator.component.ts +++ b/libs/tools/generator/components/src/password-generator.component.ts @@ -2,6 +2,7 @@ import { coerceBooleanProperty } from "@angular/cdk/coercion"; import { Component, EventEmitter, Input, NgZone, OnDestroy, OnInit, Output } from "@angular/core"; import { BehaviorSubject, + catchError, distinctUntilChanged, filter, map, @@ -14,7 +15,9 @@ import { import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { UserId } from "@bitwarden/common/types/guid"; +import { ToastService } from "@bitwarden/components"; import { Option } from "@bitwarden/components/src/select/option"; import { CredentialGeneratorService, @@ -25,6 +28,7 @@ import { isPasswordAlgorithm, AlgorithmInfo, } from "@bitwarden/generator-core"; +import { GeneratorHistoryService } from "@bitwarden/generator-history"; /** Options group for passwords */ @Component({ @@ -34,6 +38,9 @@ import { export class PasswordGeneratorComponent implements OnInit, OnDestroy { constructor( private generatorService: CredentialGeneratorService, + private generatorHistoryService: GeneratorHistoryService, + private toastService: ToastService, + private logService: LogService, private i18nService: I18nService, private accountService: AccountService, private zone: NgZone, @@ -109,10 +116,32 @@ export class PasswordGeneratorComponent implements OnInit, OnDestroy { // wire up the generator this.algorithm$ .pipe( + filter((algorithm) => !!algorithm), switchMap((algorithm) => this.typeToGenerator$(algorithm.id)), + catchError((error: unknown, generator) => { + if (typeof error === "string") { + this.toastService.showToast({ + message: error, + variant: "error", + title: "", + }); + } else { + this.logService.error(error); + } + + // continue with origin stream + return generator; + }), + withLatestFrom(this.userId$), takeUntil(this.destroyed), ) - .subscribe((generated) => { + .subscribe(([generated, userId]) => { + this.generatorHistoryService + .track(userId, generated.credential, generated.category, generated.generationDate) + .catch((e: unknown) => { + this.logService.error(e); + }); + // update subjects within the angular zone so that the // template bindings refresh immediately this.zone.run(() => { diff --git a/libs/tools/generator/components/src/username-generator.component.ts b/libs/tools/generator/components/src/username-generator.component.ts index 838177d030d..7ba4b254e98 100644 --- a/libs/tools/generator/components/src/username-generator.component.ts +++ b/libs/tools/generator/components/src/username-generator.component.ts @@ -36,6 +36,7 @@ import { isUsernameAlgorithm, toCredentialGeneratorConfiguration, } from "@bitwarden/generator-core"; +import { GeneratorHistoryService } from "@bitwarden/generator-history"; // constants used to identify navigation selections that are not // generator algorithms @@ -57,6 +58,7 @@ export class UsernameGeneratorComponent implements OnInit, OnDestroy { */ constructor( private generatorService: CredentialGeneratorService, + private generatorHistoryService: GeneratorHistoryService, private toastService: ToastService, private logService: LogService, private i18nService: I18nService, @@ -153,9 +155,16 @@ export class UsernameGeneratorComponent implements OnInit, OnDestroy { // continue with origin stream return generator; }), + withLatestFrom(this.userId$), takeUntil(this.destroyed), ) - .subscribe((generated) => { + .subscribe(([generated, userId]) => { + this.generatorHistoryService + .track(userId, generated.credential, generated.category, generated.generationDate) + .catch((e: unknown) => { + this.logService.error(e); + }); + // update subjects within the angular zone so that the // template bindings refresh immediately this.zone.run(() => { diff --git a/libs/tools/generator/extensions/history/src/generated-credential.ts b/libs/tools/generator/extensions/history/src/generated-credential.ts index 59a9623bf7e..32efb752258 100644 --- a/libs/tools/generator/extensions/history/src/generated-credential.ts +++ b/libs/tools/generator/extensions/history/src/generated-credential.ts @@ -1,6 +1,6 @@ import { Jsonify } from "type-fest"; -import { GeneratorCategory } from "./options"; +import { CredentialAlgorithm } from "@bitwarden/generator-core"; /** A credential generation result */ export class GeneratedCredential { @@ -14,7 +14,7 @@ export class GeneratedCredential { */ constructor( readonly credential: string, - readonly category: GeneratorCategory, + readonly category: CredentialAlgorithm, generationDate: Date | number, ) { if (typeof generationDate === "number") { diff --git a/libs/tools/generator/extensions/history/src/generator-history.abstraction.ts b/libs/tools/generator/extensions/history/src/generator-history.abstraction.ts index 78144c3043d..06d8741b2e0 100644 --- a/libs/tools/generator/extensions/history/src/generator-history.abstraction.ts +++ b/libs/tools/generator/extensions/history/src/generator-history.abstraction.ts @@ -1,9 +1,9 @@ import { Observable } from "rxjs"; import { UserId } from "@bitwarden/common/types/guid"; +import { CredentialAlgorithm } from "@bitwarden/generator-core"; import { GeneratedCredential } from "./generated-credential"; -import { GeneratorCategory } from "./options"; /** Tracks the history of password generations. * Each user gets their own store. @@ -27,7 +27,7 @@ export abstract class GeneratorHistoryService { track: ( userId: UserId, credential: string, - category: GeneratorCategory, + category: CredentialAlgorithm, date?: Date, ) => Promise; diff --git a/libs/tools/generator/extensions/history/src/local-generator-history.service.ts b/libs/tools/generator/extensions/history/src/local-generator-history.service.ts index 2416c84b63d..99497f7ad50 100644 --- a/libs/tools/generator/extensions/history/src/local-generator-history.service.ts +++ b/libs/tools/generator/extensions/history/src/local-generator-history.service.ts @@ -8,12 +8,13 @@ import { PaddedDataPacker } from "@bitwarden/common/tools/state/padded-data-pack import { SecretState } from "@bitwarden/common/tools/state/secret-state"; import { UserKeyEncryptor } from "@bitwarden/common/tools/state/user-key-encryptor"; import { UserId } from "@bitwarden/common/types/guid"; +import { CredentialAlgorithm } from "@bitwarden/generator-core"; import { GeneratedCredential } from "./generated-credential"; import { GeneratorHistoryService } from "./generator-history.abstraction"; import { GENERATOR_HISTORY, GENERATOR_HISTORY_BUFFER } from "./key-definitions"; import { LegacyPasswordHistoryDecryptor } from "./legacy-password-history-decryptor"; -import { GeneratorCategory, HistoryServiceOptions } from "./options"; +import { HistoryServiceOptions } from "./options"; const OPTIONS_FRAME_SIZE = 2048; @@ -25,7 +26,7 @@ export class LocalGeneratorHistoryService extends GeneratorHistoryService { private readonly encryptService: EncryptService, private readonly keyService: CryptoService, private readonly stateProvider: StateProvider, - private readonly options: HistoryServiceOptions = { maxTotal: 100 }, + private readonly options: HistoryServiceOptions = { maxTotal: 200 }, ) { super(); } @@ -33,7 +34,12 @@ export class LocalGeneratorHistoryService extends GeneratorHistoryService { private _credentialStates = new Map>(); /** {@link GeneratorHistoryService.track} */ - track = async (userId: UserId, credential: string, category: GeneratorCategory, date?: Date) => { + track = async ( + userId: UserId, + credential: string, + category: CredentialAlgorithm, + date?: Date, + ) => { const state = this.getCredentialState(userId); let result: GeneratedCredential = null; From 9b471e663336af838f65eb2486dfbcbf922cb940 Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Thu, 24 Oct 2024 08:22:43 -0500 Subject: [PATCH 3/5] [PM-13715] Launching a website from the extension does not trigger an update to reference the correct autofill value (#11587) * [PM-13715] Launching page from cipher does not set correct autofill action * [PM-13715] Fix autofill not triggering for correct cipher after page has been launched from browser extension --- .../popup/components/vault/view.component.html | 2 +- libs/angular/src/vault/components/view.component.ts | 6 ++---- libs/common/src/vault/services/cipher.service.ts | 13 +++++-------- .../autofill-options-view.component.ts | 10 ++++++++-- .../src/cipher-view/cipher-view.component.html | 6 +++++- 5 files changed, 21 insertions(+), 16 deletions(-) diff --git a/apps/browser/src/vault/popup/components/vault/view.component.html b/apps/browser/src/vault/popup/components/vault/view.component.html index ff76c68464d..73415c9070a 100644 --- a/apps/browser/src/vault/popup/components/vault/view.component.html +++ b/apps/browser/src/vault/popup/components/vault/view.component.html @@ -472,7 +472,7 @@ attr.aria-label="{{ 'launch' | i18n }} {{ u.uri }}" appA11yTitle="{{ 'launch' | i18n }}" *ngIf="u.canLaunch" - (click)="launch(u)" + (click)="launch(u, cipher.id)" > diff --git a/libs/angular/src/vault/components/view.component.ts b/libs/angular/src/vault/components/view.component.ts index 4c96c10dac3..2ff34ebafa5 100644 --- a/libs/angular/src/vault/components/view.component.ts +++ b/libs/angular/src/vault/components/view.component.ts @@ -348,15 +348,13 @@ export class ViewComponent implements OnDestroy, OnInit { } } - launch(uri: Launchable, cipherId?: string) { + async launch(uri: Launchable, cipherId?: string) { if (!uri.canLaunch) { return; } if (cipherId) { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.cipherService.updateLastLaunchedDate(cipherId); + await this.cipherService.updateLastLaunchedDate(cipherId); } this.platformUtilsService.launchUri(uri.launchUri); diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index a7377a93eec..207a5da3cbf 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -650,14 +650,11 @@ export class CipherService implements CipherServiceAbstraction { ciphersLocalData = {}; } - const cipherId = id as CipherId; - if (ciphersLocalData[cipherId]) { - ciphersLocalData[cipherId].lastLaunched = new Date().getTime(); - } else { - ciphersLocalData[cipherId] = { - lastUsedDate: new Date().getTime(), - }; - } + const currentTime = new Date().getTime(); + ciphersLocalData[id as CipherId] = { + lastLaunched: currentTime, + lastUsedDate: currentTime, + }; await this.localDataState.update(() => ciphersLocalData); diff --git a/libs/vault/src/cipher-view/autofill-options/autofill-options-view.component.ts b/libs/vault/src/cipher-view/autofill-options/autofill-options-view.component.ts index b7708b5aa98..2c3739dba41 100644 --- a/libs/vault/src/cipher-view/autofill-options/autofill-options-view.component.ts +++ b/libs/vault/src/cipher-view/autofill-options/autofill-options-view.component.ts @@ -3,6 +3,7 @@ import { Component, Input } from "@angular/core"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { LoginUriView } from "@bitwarden/common/vault/models/view/login-uri.view"; import { CardComponent, @@ -30,10 +31,15 @@ import { }) export class AutofillOptionsViewComponent { @Input() loginUris: LoginUriView[]; + @Input() cipherId: string; - constructor(private platformUtilsService: PlatformUtilsService) {} + constructor( + private platformUtilsService: PlatformUtilsService, + private cipherService: CipherService, + ) {} - openWebsite(selectedUri: string) { + async openWebsite(selectedUri: string) { + await this.cipherService.updateLastLaunchedDate(this.cipherId); this.platformUtilsService.launchUri(selectedUri); } } diff --git a/libs/vault/src/cipher-view/cipher-view.component.html b/libs/vault/src/cipher-view/cipher-view.component.html index b693c448158..2dd98092cbf 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.html +++ b/libs/vault/src/cipher-view/cipher-view.component.html @@ -25,7 +25,11 @@ - + From b3b311e164bf677d3b589cb0a72fe6968044610b Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 24 Oct 2024 15:43:49 +0200 Subject: [PATCH 4/5] Add logging for decryption failures (#11683) * Add logging to decryption routines * Fix case of uknown encryption type * Remove enum to string mapping --- .../platform/enums/encryption-type.enum.ts | 8 +++ .../encrypt.service.implementation.ts | 59 +++++++++++++++---- 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/libs/common/src/platform/enums/encryption-type.enum.ts b/libs/common/src/platform/enums/encryption-type.enum.ts index b4ecd780499..a0ffe679279 100644 --- a/libs/common/src/platform/enums/encryption-type.enum.ts +++ b/libs/common/src/platform/enums/encryption-type.enum.ts @@ -8,6 +8,14 @@ export enum EncryptionType { Rsa2048_OaepSha1_HmacSha256_B64 = 6, } +export function encryptionTypeToString(encryptionType: EncryptionType): string { + if (encryptionType in EncryptionType) { + return EncryptionType[encryptionType]; + } else { + return "Unknown encryption type " + encryptionType; + } +} + /** The expected number of parts to a serialized EncString of the given encryption type. * For example, an EncString of type AesCbc256_B64 will have 2 parts, and an EncString of type * AesCbc128_HmacSha256_B64 will have 3 parts. diff --git a/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts b/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts index 681972e7e4b..8c42c724b24 100644 --- a/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts +++ b/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts @@ -2,7 +2,7 @@ import { Utils } from "../../../platform/misc/utils"; import { CryptoFunctionService } from "../../abstractions/crypto-function.service"; import { EncryptService } from "../../abstractions/encrypt.service"; import { LogService } from "../../abstractions/log.service"; -import { EncryptionType } from "../../enums"; +import { EncryptionType, encryptionTypeToString as encryptionTypeName } from "../../enums"; import { Decryptable } from "../../interfaces/decryptable.interface"; import { Encrypted } from "../../interfaces/encrypted"; import { InitializerMetadata } from "../../interfaces/initializer-metadata.interface"; @@ -70,13 +70,24 @@ export class EncryptServiceImplementation implements EncryptService { key = this.resolveLegacyKey(key, encString); + // DO NOT REMOVE OR MOVE. This prevents downgrade to mac-less CBC, which would compromise integrity and confidentiality. if (key.macKey != null && encString?.mac == null) { - this.logService.error("MAC required but not provided."); + this.logService.error( + "[Encrypt service] Key has mac key but payload is missing mac bytes. Key type " + + encryptionTypeName(key.encType) + + " Payload type " + + encryptionTypeName(encString.encryptionType), + ); return null; } if (key.encType !== encString.encryptionType) { - this.logService.error("Key encryption type does not match payload encryption type."); + this.logService.error( + "[Encrypt service] Key encryption type does not match payload encryption type. Key type " + + encryptionTypeName(key.encType) + + " Payload type " + + encryptionTypeName(encString.encryptionType), + ); return null; } @@ -94,7 +105,12 @@ export class EncryptServiceImplementation implements EncryptService { ); const macsEqual = await this.cryptoFunctionService.compareFast(fastParams.mac, computedMac); if (!macsEqual) { - this.logMacFailed("MAC comparison failed. Key or payload has changed."); + this.logMacFailed( + "[Encrypt service] MAC comparison failed. Key or payload has changed. Key type " + + encryptionTypeName(key.encType) + + " Payload type " + + encryptionTypeName(encString.encryptionType), + ); return null; } } @@ -113,13 +129,24 @@ export class EncryptServiceImplementation implements EncryptService { key = this.resolveLegacyKey(key, encThing); + // DO NOT REMOVE OR MOVE. This prevents downgrade to mac-less CBC, which would compromise integrity and confidentiality. if (key.macKey != null && encThing.macBytes == null) { - this.logService.error("MAC required but not provided."); + this.logService.error( + "[Encrypt service] Key has mac key but payload is missing mac bytes. Key type " + + encryptionTypeName(key.encType) + + " Payload type " + + encryptionTypeName(encThing.encryptionType), + ); return null; } if (key.encType !== encThing.encryptionType) { - this.logService.error("Key encryption type does not match payload encryption type."); + this.logService.error( + "[Encrypt service] Key encryption type does not match payload encryption type. Key type " + + encryptionTypeName(key.encType) + + " Payload type " + + encryptionTypeName(encThing.encryptionType), + ); return null; } @@ -129,13 +156,25 @@ export class EncryptServiceImplementation implements EncryptService { macData.set(new Uint8Array(encThing.dataBytes), encThing.ivBytes.byteLength); const computedMac = await this.cryptoFunctionService.hmac(macData, key.macKey, "sha256"); if (computedMac === null) { - this.logMacFailed("Failed to compute MAC."); + this.logMacFailed( + "[Encrypt service] Failed to compute MAC." + + " Key type " + + encryptionTypeName(key.encType) + + " Payload type " + + encryptionTypeName(encThing.encryptionType), + ); return null; } const macsMatch = await this.cryptoFunctionService.compare(encThing.macBytes, computedMac); if (!macsMatch) { - this.logMacFailed("MAC comparison failed. Key or payload has changed."); + this.logMacFailed( + "[Encrypt service] MAC comparison failed. Key or payload has changed." + + " Key type " + + encryptionTypeName(key.encType) + + " Payload type " + + encryptionTypeName(encThing.encryptionType), + ); return null; } } @@ -164,7 +203,7 @@ export class EncryptServiceImplementation implements EncryptService { async rsaDecrypt(data: EncString, privateKey: Uint8Array): Promise { if (data == null) { - throw new Error("No data provided for decryption."); + throw new Error("[Encrypt service] rsaDecrypt: No data provided for decryption."); } let algorithm: "sha1" | "sha256"; @@ -182,7 +221,7 @@ export class EncryptServiceImplementation implements EncryptService { } if (privateKey == null) { - throw new Error("No private key provided for decryption."); + throw new Error("[Encrypt service] rsaDecrypt: No private key provided for decryption."); } return this.cryptoFunctionService.rsaDecrypt(data.dataBytes, privateKey, algorithm); From 15c301d39f94b71b08a6310c88b036f51609d4d9 Mon Sep 17 00:00:00 2001 From: Jonathan Prusik Date: Thu, 24 Oct 2024 10:15:24 -0400 Subject: [PATCH 5/5] Do not redirect after saving changes to excluded domains (#11676) --- .../settings/excluded-domains.component.html | 10 ++- .../settings/excluded-domains.component.ts | 81 ++++++++++++------- 2 files changed, 61 insertions(+), 30 deletions(-) diff --git a/apps/browser/src/autofill/popup/settings/excluded-domains.component.html b/apps/browser/src/autofill/popup/settings/excluded-domains.component.html index 1dec438fdd2..e3b6bf5f802 100644 --- a/apps/browser/src/autofill/popup/settings/excluded-domains.component.html +++ b/apps/browser/src/autofill/popup/settings/excluded-domains.component.html @@ -11,7 +11,7 @@ accountSwitcherEnabled ? ("excludedDomainsDescAlt" | i18n) : ("excludedDomainsDesc" | i18n) }}

- +

{{ "domainsTitle" | i18n }}

{{ excludedDomainsState?.length || 0 }} @@ -57,7 +57,13 @@ - diff --git a/apps/browser/src/autofill/popup/settings/excluded-domains.component.ts b/apps/browser/src/autofill/popup/settings/excluded-domains.component.ts index 381ba903423..f622312ce04 100644 --- a/apps/browser/src/autofill/popup/settings/excluded-domains.component.ts +++ b/apps/browser/src/autofill/popup/settings/excluded-domains.component.ts @@ -1,13 +1,19 @@ import { CommonModule } from "@angular/common"; -import { QueryList, Component, ElementRef, OnDestroy, OnInit, ViewChildren } from "@angular/core"; +import { + QueryList, + Component, + ElementRef, + OnDestroy, + AfterViewInit, + ViewChildren, +} from "@angular/core"; import { FormsModule } from "@angular/forms"; -import { Router, RouterModule } from "@angular/router"; -import { firstValueFrom, Subject, takeUntil } from "rxjs"; +import { RouterModule } from "@angular/router"; +import { Subject, takeUntil } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service"; import { NeverDomains } from "@bitwarden/common/models/domain/domain-service"; -import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -29,8 +35,6 @@ import { PopupFooterComponent } from "../../../platform/popup/layout/popup-foote import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component"; import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.component"; -const BroadcasterSubscriptionId = "excludedDomainsState"; - @Component({ selector: "app-excluded-domains", templateUrl: "excluded-domains.component.html", @@ -55,11 +59,12 @@ const BroadcasterSubscriptionId = "excludedDomainsState"; TypographyModule, ], }) -export class ExcludedDomainsComponent implements OnInit, OnDestroy { +export class ExcludedDomainsComponent implements AfterViewInit, OnDestroy { @ViewChildren("uriInput") uriInputElements: QueryList>; accountSwitcherEnabled = false; dataIsPristine = true; + isLoading = false; excludedDomainsState: string[] = []; storedExcludedDomains: string[] = []; // How many fields should be non-editable before editable fields @@ -70,16 +75,27 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { constructor( private domainSettingsService: DomainSettingsService, private i18nService: I18nService, - private router: Router, - private broadcasterService: BroadcasterService, private platformUtilsService: PlatformUtilsService, ) { this.accountSwitcherEnabled = enableAccountSwitching(); } - async ngOnInit() { - const neverDomains = await firstValueFrom(this.domainSettingsService.neverDomains$); + async ngAfterViewInit() { + this.domainSettingsService.neverDomains$ + .pipe(takeUntil(this.destroy$)) + .subscribe((neverDomains: NeverDomains) => this.handleStateUpdate(neverDomains)); + this.uriInputElements.changes.pipe(takeUntil(this.destroy$)).subscribe(({ last }) => { + this.focusNewUriInput(last); + }); + } + + ngOnDestroy() { + this.destroy$.next(); + this.destroy$.complete(); + } + + handleStateUpdate(neverDomains: NeverDomains) { if (neverDomains) { this.storedExcludedDomains = Object.keys(neverDomains); } @@ -89,15 +105,8 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { // Do not allow the first x (pre-existing) fields to be edited this.fieldsEditThreshold = this.storedExcludedDomains.length; - this.uriInputElements.changes.pipe(takeUntil(this.destroy$)).subscribe(({ last }) => { - this.focusNewUriInput(last); - }); - } - - ngOnDestroy() { - this.broadcasterService.unsubscribe(BroadcasterSubscriptionId); - this.destroy$.next(); - this.destroy$.complete(); + this.dataIsPristine = true; + this.isLoading = false; } focusNewUriInput(elementRef: ElementRef) { @@ -116,7 +125,7 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { async removeDomain(i: number) { this.excludedDomainsState.splice(i, 1); - // if a pre-existing field was dropped, lower the edit threshold + // If a pre-existing field was dropped, lower the edit threshold if (i < this.fieldsEditThreshold) { this.fieldsEditThreshold--; } @@ -132,11 +141,11 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { async saveChanges() { if (this.dataIsPristine) { - await this.router.navigate(["/notifications"]); - return; } + this.isLoading = true; + const newExcludedDomainsSaveState: NeverDomains = {}; const uniqueExcludedDomains = new Set(this.excludedDomainsState); @@ -151,6 +160,8 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { this.i18nService.t("excludedDomainsInvalidDomain", uri), ); + // Don't reset via `handleStateUpdate` to allow existing input value correction + this.isLoading = false; return; } @@ -159,7 +170,23 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { } try { - await this.domainSettingsService.setNeverDomains(newExcludedDomainsSaveState); + const existingState = new Set(this.storedExcludedDomains); + const newState = new Set(Object.keys(newExcludedDomainsSaveState)); + const stateIsUnchanged = + existingState.size === newState.size && + new Set([...existingState, ...newState]).size === existingState.size; + + // The subscriber updates don't trigger if `setNeverDomains` sets an equivalent state + if (stateIsUnchanged) { + // Reset UI state directly + const constructedNeverDomainsState = this.storedExcludedDomains.reduce( + (neverDomains, uri) => ({ ...neverDomains, [uri]: null }), + {}, + ); + this.handleStateUpdate(constructedNeverDomainsState); + } else { + await this.domainSettingsService.setNeverDomains(newExcludedDomainsSaveState); + } this.platformUtilsService.showToast( "success", @@ -169,11 +196,9 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { } catch { this.platformUtilsService.showToast("error", null, this.i18nService.t("unexpectedError")); - // Do not navigate on error - return; + // Don't reset via `handleStateUpdate` to preserve input values + this.isLoading = false; } - - await this.router.navigate(["/notifications"]); } trackByFunction(index: number, _: string) {