diff --git a/apps/browser/src/popup/settings/options.component.ts b/apps/browser/src/popup/settings/options.component.ts index 813eeda144..a4260b688f 100644 --- a/apps/browser/src/popup/settings/options.component.ts +++ b/apps/browser/src/popup/settings/options.component.ts @@ -114,8 +114,10 @@ export class OptionsComponent implements OnInit { this.autofillSettingsService.enableContextMenu$, ); - this.showCardsCurrentTab = !(await this.stateService.getDontShowCardsCurrentTab()); - this.showIdentitiesCurrentTab = !(await this.stateService.getDontShowIdentitiesCurrentTab()); + this.showCardsCurrentTab = await firstValueFrom(this.vaultSettingsService.showCardsCurrentTab$); + this.showIdentitiesCurrentTab = await firstValueFrom( + this.vaultSettingsService.showIdentitiesCurrentTab$, + ); this.enableAutoTotpCopy = await firstValueFrom(this.autofillSettingsService.autoCopyTotp$); @@ -178,11 +180,11 @@ export class OptionsComponent implements OnInit { } async updateShowCardsCurrentTab() { - await this.stateService.setDontShowCardsCurrentTab(!this.showCardsCurrentTab); + await this.vaultSettingsService.setShowCardsCurrentTab(this.showCardsCurrentTab); } async updateShowIdentitiesCurrentTab() { - await this.stateService.setDontShowIdentitiesCurrentTab(!this.showIdentitiesCurrentTab); + await this.vaultSettingsService.setShowIdentitiesCurrentTab(this.showIdentitiesCurrentTab); } async saveTheme() { diff --git a/apps/browser/src/vault/popup/components/vault/current-tab.component.ts b/apps/browser/src/vault/popup/components/vault/current-tab.component.ts index d1fcb5d439..e4fdc7525e 100644 --- a/apps/browser/src/vault/popup/components/vault/current-tab.component.ts +++ b/apps/browser/src/vault/popup/components/vault/current-tab.component.ts @@ -10,10 +10,10 @@ import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/s 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 { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; +import { VaultSettingsService } from "@bitwarden/common/vault/abstractions/vault-settings/vault-settings.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; @@ -65,11 +65,11 @@ export class CurrentTabComponent implements OnInit, OnDestroy { private changeDetectorRef: ChangeDetectorRef, private syncService: SyncService, private searchService: SearchService, - private stateService: StateService, private autofillSettingsService: AutofillSettingsServiceAbstraction, private passwordRepromptService: PasswordRepromptService, private organizationService: OrganizationService, private vaultFilterService: VaultFilterService, + private vaultSettingsService: VaultSettingsService, ) {} async ngOnInit() { @@ -271,8 +271,10 @@ export class CurrentTabComponent implements OnInit, OnDestroy { }); const otherTypes: CipherType[] = []; - const dontShowCards = await this.stateService.getDontShowCardsCurrentTab(); - const dontShowIdentities = await this.stateService.getDontShowIdentitiesCurrentTab(); + const dontShowCards = !(await firstValueFrom(this.vaultSettingsService.showCardsCurrentTab$)); + const dontShowIdentities = !(await firstValueFrom( + this.vaultSettingsService.showIdentitiesCurrentTab$, + )); this.showOrganizations = this.organizationService.hasOrganizations(); if (!dontShowCards) { otherTypes.push(CipherType.Card); diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index d9b18e509b..1ec764974f 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -192,10 +192,6 @@ export abstract class StateService { setDisableFavicon: (value: boolean, options?: StorageOptions) => Promise; getDisableGa: (options?: StorageOptions) => Promise; setDisableGa: (value: boolean, options?: StorageOptions) => Promise; - getDontShowCardsCurrentTab: (options?: StorageOptions) => Promise; - setDontShowCardsCurrentTab: (value: boolean, options?: StorageOptions) => Promise; - getDontShowIdentitiesCurrentTab: (options?: StorageOptions) => Promise; - setDontShowIdentitiesCurrentTab: (value: boolean, options?: StorageOptions) => Promise; getDuckDuckGoSharedKey: (options?: StorageOptions) => Promise; setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise; getDeviceKey: (options?: StorageOptions) => Promise; diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 2c3c2eab67..0c85307032 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -198,8 +198,6 @@ export class AccountSettings { autoConfirmFingerPrints?: boolean; defaultUriMatch?: UriMatchStrategySetting; disableGa?: boolean; - dontShowCardsCurrentTab?: boolean; - dontShowIdentitiesCurrentTab?: boolean; enableAlwaysOnTop?: boolean; enableBiometric?: boolean; minimizeOnCopyToClipboard?: boolean; diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index d7d302db4c..6722cb9347 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -852,42 +852,6 @@ export class StateService< ); } - async getDontShowCardsCurrentTab(options?: StorageOptions): Promise { - return ( - (await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.settings?.dontShowCardsCurrentTab ?? false - ); - } - - async setDontShowCardsCurrentTab(value: boolean, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.settings.dontShowCardsCurrentTab = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - - async getDontShowIdentitiesCurrentTab(options?: StorageOptions): Promise { - return ( - (await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.settings?.dontShowIdentitiesCurrentTab ?? false - ); - } - - async setDontShowIdentitiesCurrentTab(value: boolean, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.settings.dontShowIdentitiesCurrentTab = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getDuckDuckGoSharedKey(options?: StorageOptions): Promise { options = this.reconcileOptions(options, await this.defaultSecureStorageOptions()); if (options?.userId == null) { diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index 6fb7c0288c..1051ee952b 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -31,6 +31,7 @@ import { PreferredLanguageMigrator } from "./migrations/32-move-preferred-langua import { AppIdMigrator } from "./migrations/33-move-app-id-to-state-providers"; import { DomainSettingsMigrator } from "./migrations/34-move-domain-settings-to-state-providers"; import { MoveThemeToStateProviderMigrator } from "./migrations/35-move-theme-to-state-providers"; +import { VaultSettingsKeyMigrator } from "./migrations/36-move-show-card-and-identity-to-state-provider"; import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked"; import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys"; import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key"; @@ -40,7 +41,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 2; -export const CURRENT_VERSION = 35; +export const CURRENT_VERSION = 36; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -78,7 +79,8 @@ export function createMigrationBuilder() { .with(PreferredLanguageMigrator, 31, 32) .with(AppIdMigrator, 32, 33) .with(DomainSettingsMigrator, 33, 34) - .with(MoveThemeToStateProviderMigrator, 34, CURRENT_VERSION); + .with(MoveThemeToStateProviderMigrator, 34, 35) + .with(VaultSettingsKeyMigrator, 35, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migrations/36-move-show-card-and-identity-to-state-provider.spec.ts b/libs/common/src/state-migrations/migrations/36-move-show-card-and-identity-to-state-provider.spec.ts new file mode 100644 index 0000000000..64a7fd8efa --- /dev/null +++ b/libs/common/src/state-migrations/migrations/36-move-show-card-and-identity-to-state-provider.spec.ts @@ -0,0 +1,142 @@ +import { MockProxy, any } from "jest-mock-extended"; + +import { MigrationHelper, StateDefinitionLike } from "../migration-helper"; +import { mockMigrationHelper } from "../migration-helper.spec"; + +import { VaultSettingsKeyMigrator } from "./36-move-show-card-and-identity-to-state-provider"; + +function exampleJSON() { + return { + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["user-1", "user-2", "user-3"], + "user-1": { + settings: { + dontShowCardsCurrentTab: true, + dontShowIdentitiesCurrentTab: true, + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }, + "user-2": { + settings: { + otherStuff: "otherStuff4", + }, + otherStuff: "otherStuff5", + }, + }; +} + +function rollbackJSON() { + return { + "user_user-1_vaultSettings_showCardsCurrentTab": true, + "user_user-1_vaultSettings_showIdentitiesCurrentTab": true, + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["user-1", "user-2", "user-3"], + "user-1": { + settings: { + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }, + "user-2": { + settings: { + otherStuff: "otherStuff4", + }, + otherStuff: "otherStuff5", + }, + }; +} + +const vaultSettingsStateDefinition: { + stateDefinition: StateDefinitionLike; +} = { + stateDefinition: { + name: "vaultSettings", + }, +}; + +describe("VaultSettingsKeyMigrator", () => { + let helper: MockProxy; + let sut: VaultSettingsKeyMigrator; + + describe("migrate", () => { + beforeEach(() => { + helper = mockMigrationHelper(exampleJSON(), 35); + sut = new VaultSettingsKeyMigrator(35, 36); + }); + + it("should remove dontShowCardsCurrentTab and dontShowIdentitiesCurrentTab from all accounts", async () => { + await sut.migrate(helper); + expect(helper.set).toHaveBeenCalledTimes(1); + expect(helper.set).toHaveBeenCalledWith("user-1", { + settings: { + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }); + }); + + it("should set showCardsCurrentTab and showIdentitiesCurrentTab values for each account", async () => { + await sut.migrate(helper); + + expect(helper.setToUser).toHaveBeenCalledTimes(2); + expect(helper.setToUser).toHaveBeenCalledWith( + "user-1", + { ...vaultSettingsStateDefinition, key: "showCardsCurrentTab" }, + true, + ); + expect(helper.setToUser).toHaveBeenCalledWith( + "user-1", + { ...vaultSettingsStateDefinition, key: "showIdentitiesCurrentTab" }, + true, + ); + }); + }); + + describe("rollback", () => { + beforeEach(() => { + helper = mockMigrationHelper(rollbackJSON(), 36); + sut = new VaultSettingsKeyMigrator(35, 36); + }); + + it("should null out new values for each account", async () => { + await sut.rollback(helper); + + expect(helper.setToUser).toHaveBeenCalledTimes(2); + expect(helper.setToUser).toHaveBeenCalledWith( + "user-1", + { ...vaultSettingsStateDefinition, key: "showCardsCurrentTab" }, + null, + ); + expect(helper.setToUser).toHaveBeenCalledWith( + "user-1", + { ...vaultSettingsStateDefinition, key: "showIdentitiesCurrentTab" }, + null, + ); + }); + + it("should add explicit value back to accounts", async () => { + await sut.rollback(helper); + + expect(helper.set).toHaveBeenCalledTimes(1); + expect(helper.set).toHaveBeenCalledWith("user-1", { + settings: { + otherStuff: "otherStuff2", + dontShowCardsCurrentTab: false, + dontShowIdentitiesCurrentTab: false, + }, + otherStuff: "otherStuff3", + }); + }); + + it("should not try to restore values to missing accounts", async () => { + await sut.rollback(helper); + + expect(helper.set).not.toHaveBeenCalledWith("user-3", any()); + }); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/36-move-show-card-and-identity-to-state-provider.ts b/libs/common/src/state-migrations/migrations/36-move-show-card-and-identity-to-state-provider.ts new file mode 100644 index 0000000000..572e074cf1 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/36-move-show-card-and-identity-to-state-provider.ts @@ -0,0 +1,105 @@ +import { MigrationHelper, StateDefinitionLike } from "../migration-helper"; +import { Migrator } from "../migrator"; + +type ExpectedAccountState = { + settings?: { + dontShowCardsCurrentTab?: boolean; + dontShowIdentitiesCurrentTab?: boolean; + }; +}; + +const vaultSettingsStateDefinition: { + stateDefinition: StateDefinitionLike; +} = { + stateDefinition: { + name: "vaultSettings", + }, +}; + +export class VaultSettingsKeyMigrator extends Migrator<35, 36> { + async migrate(helper: MigrationHelper): Promise { + const accounts = await helper.getAccounts(); + + await Promise.all([...accounts.map(({ userId, account }) => migrateAccount(userId, account))]); + + async function migrateAccount(userId: string, account: ExpectedAccountState): Promise { + let updateAccount = false; + const accountSettings = account?.settings; + + if (accountSettings?.dontShowCardsCurrentTab != null) { + await helper.setToUser( + userId, + { ...vaultSettingsStateDefinition, key: "showCardsCurrentTab" }, + accountSettings.dontShowCardsCurrentTab, + ); + delete account.settings.dontShowCardsCurrentTab; + updateAccount = true; + } + + if (accountSettings?.dontShowIdentitiesCurrentTab != null) { + await helper.setToUser( + userId, + { ...vaultSettingsStateDefinition, key: "showIdentitiesCurrentTab" }, + accountSettings.dontShowIdentitiesCurrentTab, + ); + delete account.settings.dontShowIdentitiesCurrentTab; + updateAccount = true; + } + + if (updateAccount) { + await helper.set(userId, account); + } + } + } + + async rollback(helper: MigrationHelper): Promise { + const accounts = await helper.getAccounts(); + + await Promise.all([...accounts.map(({ userId, account }) => rollbackAccount(userId, account))]); + + async function rollbackAccount(userId: string, account: ExpectedAccountState): Promise { + let updateAccount = false; + let settings = account?.settings ?? {}; + + const showCardsCurrentTab = await helper.getFromUser(userId, { + ...vaultSettingsStateDefinition, + key: "showCardsCurrentTab", + }); + + const showIdentitiesCurrentTab = await helper.getFromUser(userId, { + ...vaultSettingsStateDefinition, + key: "showIdentitiesCurrentTab", + }); + + if (showCardsCurrentTab != null) { + // invert the value to match the new naming convention + settings = { ...settings, dontShowCardsCurrentTab: !showCardsCurrentTab }; + + await helper.setToUser( + userId, + { ...vaultSettingsStateDefinition, key: "showCardsCurrentTab" }, + null, + ); + + updateAccount = true; + } + + if (showIdentitiesCurrentTab != null) { + // invert the value to match the new naming convention + settings = { ...settings, dontShowIdentitiesCurrentTab: !showIdentitiesCurrentTab }; + + await helper.setToUser( + userId, + { ...vaultSettingsStateDefinition, key: "showIdentitiesCurrentTab" }, + null, + ); + + updateAccount = true; + } + + if (updateAccount) { + await helper.set(userId, { ...account, settings }); + } + } + } +} diff --git a/libs/common/src/vault/abstractions/vault-settings/vault-settings.service.ts b/libs/common/src/vault/abstractions/vault-settings/vault-settings.service.ts index 9e935b763c..e3132d9ae1 100644 --- a/libs/common/src/vault/abstractions/vault-settings/vault-settings.service.ts +++ b/libs/common/src/vault/abstractions/vault-settings/vault-settings.service.ts @@ -8,10 +8,29 @@ export abstract class VaultSettingsService { * The observable updates when the setting changes. */ enablePasskeys$: Observable; + /** + * An observable monitoring the state of the show cards on the current tab. + */ + showCardsCurrentTab$: Observable; + /** + * An observable monitoring the state of the show identities on the current tab. + */ + showIdentitiesCurrentTab$: Observable; + /** /** * Saves the enable passkeys setting to disk. * @param value The new value for the passkeys setting. */ setEnablePasskeys: (value: boolean) => Promise; + /** + * Saves the show cards on tab page setting to disk. + * @param value The new value for the show cards on tab page setting. + */ + setShowCardsCurrentTab: (value: boolean) => Promise; + /** + * Saves the show identities on tab page setting to disk. + * @param value The new value for the show identities on tab page setting. + */ + setShowIdentitiesCurrentTab: (value: boolean) => Promise; } diff --git a/libs/common/src/vault/services/key-state/enable-passkey.state.ts b/libs/common/src/vault/services/key-state/enable-passkey.state.ts deleted file mode 100644 index dccbf8fd11..0000000000 --- a/libs/common/src/vault/services/key-state/enable-passkey.state.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { VAULT_SETTINGS_DISK, KeyDefinition } from "../../../platform/state"; - -export const USER_ENABLE_PASSKEYS = new KeyDefinition( - VAULT_SETTINGS_DISK, - "enablePasskeys", - { - deserializer: (obj) => obj, - }, -); diff --git a/libs/common/src/vault/services/key-state/vault-settings.state.ts b/libs/common/src/vault/services/key-state/vault-settings.state.ts new file mode 100644 index 0000000000..90b47912ee --- /dev/null +++ b/libs/common/src/vault/services/key-state/vault-settings.state.ts @@ -0,0 +1,23 @@ +import { VAULT_SETTINGS_DISK, KeyDefinition } from "../../../platform/state"; + +export const USER_ENABLE_PASSKEYS = new KeyDefinition( + VAULT_SETTINGS_DISK, + "enablePasskeys", + { + deserializer: (obj) => obj, + }, +); + +export const SHOW_CARDS_CURRENT_TAB = new KeyDefinition( + VAULT_SETTINGS_DISK, + "showCardsCurrentTab", + { + deserializer: (obj) => obj, + }, +); + +export const SHOW_IDENTITIES_CURRENT_TAB = new KeyDefinition( + VAULT_SETTINGS_DISK, + "showIdentitiesCurrentTab", + { deserializer: (obj) => obj }, +); diff --git a/libs/common/src/vault/services/vault-settings/vault-settings.service.ts b/libs/common/src/vault/services/vault-settings/vault-settings.service.ts index 22192c271b..39b9631821 100644 --- a/libs/common/src/vault/services/vault-settings/vault-settings.service.ts +++ b/libs/common/src/vault/services/vault-settings/vault-settings.service.ts @@ -1,8 +1,12 @@ import { Observable, map } from "rxjs"; -import { GlobalState, StateProvider } from "../../../platform/state"; +import { ActiveUserState, GlobalState, StateProvider } from "../../../platform/state"; import { VaultSettingsService as VaultSettingsServiceAbstraction } from "../../abstractions/vault-settings/vault-settings.service"; -import { USER_ENABLE_PASSKEYS } from "../key-state/enable-passkey.state"; +import { + SHOW_CARDS_CURRENT_TAB, + SHOW_IDENTITIES_CURRENT_TAB, + USER_ENABLE_PASSKEYS, +} from "../key-state/vault-settings.state"; /** * {@link VaultSettingsServiceAbstraction} @@ -10,7 +14,6 @@ import { USER_ENABLE_PASSKEYS } from "../key-state/enable-passkey.state"; export class VaultSettingsService implements VaultSettingsServiceAbstraction { private enablePasskeysState: GlobalState = this.stateProvider.getGlobal(USER_ENABLE_PASSKEYS); - /** * {@link VaultSettingsServiceAbstraction.enablePasskeys$} */ @@ -18,8 +21,40 @@ export class VaultSettingsService implements VaultSettingsServiceAbstraction { map((x) => x ?? true), ); + private showCardsCurrentTabState: ActiveUserState = + this.stateProvider.getActive(SHOW_CARDS_CURRENT_TAB); + /** + * {@link VaultSettingsServiceAbstraction.showCardsCurrentTab$} + */ + readonly showCardsCurrentTab$: Observable = this.showCardsCurrentTabState.state$.pipe( + map((x) => x ?? true), + ); + + private showIdentitiesCurrentTabState: ActiveUserState = this.stateProvider.getActive( + SHOW_IDENTITIES_CURRENT_TAB, + ); + /** + * {@link VaultSettingsServiceAbstraction.showIdentitiesCurrentTab$} + */ + readonly showIdentitiesCurrentTab$: Observable = + this.showIdentitiesCurrentTabState.state$.pipe(map((x) => x ?? true)); + constructor(private stateProvider: StateProvider) {} + /** + * {@link VaultSettingsServiceAbstraction.setShowCardsCurrentTab} + */ + async setShowCardsCurrentTab(value: boolean): Promise { + await this.showCardsCurrentTabState.update(() => value); + } + + /** + * {@link VaultSettingsServiceAbstraction.setDontShowIdentitiesCurrentTab} + */ + async setShowIdentitiesCurrentTab(value: boolean): Promise { + await this.showIdentitiesCurrentTabState.update(() => value); + } + /** * {@link VaultSettingsServiceAbstraction.setEnablePasskeys} */