[PM-5433] Migrate Showcards and Showidentities on current tab to state provider (#8252)

* added showCards and Identities to vault settings and then added migration file

* added migration file and removed fields from domain

* fixed merge conflicts
This commit is contained in:
SmithThe4th 2024-03-14 11:37:57 -04:00 committed by GitHub
parent d28634b068
commit ebf51ebaaf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 343 additions and 64 deletions

View File

@ -114,8 +114,10 @@ export class OptionsComponent implements OnInit {
this.autofillSettingsService.enableContextMenu$, this.autofillSettingsService.enableContextMenu$,
); );
this.showCardsCurrentTab = !(await this.stateService.getDontShowCardsCurrentTab()); this.showCardsCurrentTab = await firstValueFrom(this.vaultSettingsService.showCardsCurrentTab$);
this.showIdentitiesCurrentTab = !(await this.stateService.getDontShowIdentitiesCurrentTab()); this.showIdentitiesCurrentTab = await firstValueFrom(
this.vaultSettingsService.showIdentitiesCurrentTab$,
);
this.enableAutoTotpCopy = await firstValueFrom(this.autofillSettingsService.autoCopyTotp$); this.enableAutoTotpCopy = await firstValueFrom(this.autofillSettingsService.autoCopyTotp$);
@ -178,11 +180,11 @@ export class OptionsComponent implements OnInit {
} }
async updateShowCardsCurrentTab() { async updateShowCardsCurrentTab() {
await this.stateService.setDontShowCardsCurrentTab(!this.showCardsCurrentTab); await this.vaultSettingsService.setShowCardsCurrentTab(this.showCardsCurrentTab);
} }
async updateShowIdentitiesCurrentTab() { async updateShowIdentitiesCurrentTab() {
await this.stateService.setDontShowIdentitiesCurrentTab(!this.showIdentitiesCurrentTab); await this.vaultSettingsService.setShowIdentitiesCurrentTab(this.showIdentitiesCurrentTab);
} }
async saveTheme() { async saveTheme() {

View File

@ -10,10 +10,10 @@ import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/s
import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.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 { Utils } from "@bitwarden/common/platform/misc/utils";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; 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 { CipherType } from "@bitwarden/common/vault/enums";
import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type"; import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
@ -65,11 +65,11 @@ export class CurrentTabComponent implements OnInit, OnDestroy {
private changeDetectorRef: ChangeDetectorRef, private changeDetectorRef: ChangeDetectorRef,
private syncService: SyncService, private syncService: SyncService,
private searchService: SearchService, private searchService: SearchService,
private stateService: StateService,
private autofillSettingsService: AutofillSettingsServiceAbstraction, private autofillSettingsService: AutofillSettingsServiceAbstraction,
private passwordRepromptService: PasswordRepromptService, private passwordRepromptService: PasswordRepromptService,
private organizationService: OrganizationService, private organizationService: OrganizationService,
private vaultFilterService: VaultFilterService, private vaultFilterService: VaultFilterService,
private vaultSettingsService: VaultSettingsService,
) {} ) {}
async ngOnInit() { async ngOnInit() {
@ -271,8 +271,10 @@ export class CurrentTabComponent implements OnInit, OnDestroy {
}); });
const otherTypes: CipherType[] = []; const otherTypes: CipherType[] = [];
const dontShowCards = await this.stateService.getDontShowCardsCurrentTab(); const dontShowCards = !(await firstValueFrom(this.vaultSettingsService.showCardsCurrentTab$));
const dontShowIdentities = await this.stateService.getDontShowIdentitiesCurrentTab(); const dontShowIdentities = !(await firstValueFrom(
this.vaultSettingsService.showIdentitiesCurrentTab$,
));
this.showOrganizations = this.organizationService.hasOrganizations(); this.showOrganizations = this.organizationService.hasOrganizations();
if (!dontShowCards) { if (!dontShowCards) {
otherTypes.push(CipherType.Card); otherTypes.push(CipherType.Card);

View File

@ -192,10 +192,6 @@ export abstract class StateService<T extends Account = Account> {
setDisableFavicon: (value: boolean, options?: StorageOptions) => Promise<void>; setDisableFavicon: (value: boolean, options?: StorageOptions) => Promise<void>;
getDisableGa: (options?: StorageOptions) => Promise<boolean>; getDisableGa: (options?: StorageOptions) => Promise<boolean>;
setDisableGa: (value: boolean, options?: StorageOptions) => Promise<void>; setDisableGa: (value: boolean, options?: StorageOptions) => Promise<void>;
getDontShowCardsCurrentTab: (options?: StorageOptions) => Promise<boolean>;
setDontShowCardsCurrentTab: (value: boolean, options?: StorageOptions) => Promise<void>;
getDontShowIdentitiesCurrentTab: (options?: StorageOptions) => Promise<boolean>;
setDontShowIdentitiesCurrentTab: (value: boolean, options?: StorageOptions) => Promise<void>;
getDuckDuckGoSharedKey: (options?: StorageOptions) => Promise<string>; getDuckDuckGoSharedKey: (options?: StorageOptions) => Promise<string>;
setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise<void>; setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise<void>;
getDeviceKey: (options?: StorageOptions) => Promise<DeviceKey | null>; getDeviceKey: (options?: StorageOptions) => Promise<DeviceKey | null>;

View File

@ -198,8 +198,6 @@ export class AccountSettings {
autoConfirmFingerPrints?: boolean; autoConfirmFingerPrints?: boolean;
defaultUriMatch?: UriMatchStrategySetting; defaultUriMatch?: UriMatchStrategySetting;
disableGa?: boolean; disableGa?: boolean;
dontShowCardsCurrentTab?: boolean;
dontShowIdentitiesCurrentTab?: boolean;
enableAlwaysOnTop?: boolean; enableAlwaysOnTop?: boolean;
enableBiometric?: boolean; enableBiometric?: boolean;
minimizeOnCopyToClipboard?: boolean; minimizeOnCopyToClipboard?: boolean;

View File

@ -852,42 +852,6 @@ export class StateService<
); );
} }
async getDontShowCardsCurrentTab(options?: StorageOptions): Promise<boolean> {
return (
(await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions())))
?.settings?.dontShowCardsCurrentTab ?? false
);
}
async setDontShowCardsCurrentTab(value: boolean, options?: StorageOptions): Promise<void> {
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<boolean> {
return (
(await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions())))
?.settings?.dontShowIdentitiesCurrentTab ?? false
);
}
async setDontShowIdentitiesCurrentTab(value: boolean, options?: StorageOptions): Promise<void> {
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<string> { async getDuckDuckGoSharedKey(options?: StorageOptions): Promise<string> {
options = this.reconcileOptions(options, await this.defaultSecureStorageOptions()); options = this.reconcileOptions(options, await this.defaultSecureStorageOptions());
if (options?.userId == null) { if (options?.userId == null) {

View File

@ -31,6 +31,7 @@ import { PreferredLanguageMigrator } from "./migrations/32-move-preferred-langua
import { AppIdMigrator } from "./migrations/33-move-app-id-to-state-providers"; import { AppIdMigrator } from "./migrations/33-move-app-id-to-state-providers";
import { DomainSettingsMigrator } from "./migrations/34-move-domain-settings-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 { 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 { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked";
import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys"; import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys";
import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key"; 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"; import { MinVersionMigrator } from "./migrations/min-version";
export const MIN_VERSION = 2; export const MIN_VERSION = 2;
export const CURRENT_VERSION = 35; export const CURRENT_VERSION = 36;
export type MinVersion = typeof MIN_VERSION; export type MinVersion = typeof MIN_VERSION;
export function createMigrationBuilder() { export function createMigrationBuilder() {
@ -78,7 +79,8 @@ export function createMigrationBuilder() {
.with(PreferredLanguageMigrator, 31, 32) .with(PreferredLanguageMigrator, 31, 32)
.with(AppIdMigrator, 32, 33) .with(AppIdMigrator, 32, 33)
.with(DomainSettingsMigrator, 33, 34) .with(DomainSettingsMigrator, 33, 34)
.with(MoveThemeToStateProviderMigrator, 34, CURRENT_VERSION); .with(MoveThemeToStateProviderMigrator, 34, 35)
.with(VaultSettingsKeyMigrator, 35, CURRENT_VERSION);
} }
export async function currentVersion( export async function currentVersion(

View File

@ -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<MigrationHelper>;
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());
});
});
});

View File

@ -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<void> {
const accounts = await helper.getAccounts<ExpectedAccountState>();
await Promise.all([...accounts.map(({ userId, account }) => migrateAccount(userId, account))]);
async function migrateAccount(userId: string, account: ExpectedAccountState): Promise<void> {
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<void> {
const accounts = await helper.getAccounts<ExpectedAccountState>();
await Promise.all([...accounts.map(({ userId, account }) => rollbackAccount(userId, account))]);
async function rollbackAccount(userId: string, account: ExpectedAccountState): Promise<void> {
let updateAccount = false;
let settings = account?.settings ?? {};
const showCardsCurrentTab = await helper.getFromUser<boolean>(userId, {
...vaultSettingsStateDefinition,
key: "showCardsCurrentTab",
});
const showIdentitiesCurrentTab = await helper.getFromUser<boolean>(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 });
}
}
}
}

View File

@ -8,10 +8,29 @@ export abstract class VaultSettingsService {
* The observable updates when the setting changes. * The observable updates when the setting changes.
*/ */
enablePasskeys$: Observable<boolean>; enablePasskeys$: Observable<boolean>;
/**
* An observable monitoring the state of the show cards on the current tab.
*/
showCardsCurrentTab$: Observable<boolean>;
/**
* An observable monitoring the state of the show identities on the current tab.
*/
showIdentitiesCurrentTab$: Observable<boolean>;
/**
/** /**
* Saves the enable passkeys setting to disk. * Saves the enable passkeys setting to disk.
* @param value The new value for the passkeys setting. * @param value The new value for the passkeys setting.
*/ */
setEnablePasskeys: (value: boolean) => Promise<void>; setEnablePasskeys: (value: boolean) => Promise<void>;
/**
* 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<void>;
/**
* 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<void>;
} }

View File

@ -1,9 +0,0 @@
import { VAULT_SETTINGS_DISK, KeyDefinition } from "../../../platform/state";
export const USER_ENABLE_PASSKEYS = new KeyDefinition<boolean>(
VAULT_SETTINGS_DISK,
"enablePasskeys",
{
deserializer: (obj) => obj,
},
);

View File

@ -0,0 +1,23 @@
import { VAULT_SETTINGS_DISK, KeyDefinition } from "../../../platform/state";
export const USER_ENABLE_PASSKEYS = new KeyDefinition<boolean>(
VAULT_SETTINGS_DISK,
"enablePasskeys",
{
deserializer: (obj) => obj,
},
);
export const SHOW_CARDS_CURRENT_TAB = new KeyDefinition<boolean>(
VAULT_SETTINGS_DISK,
"showCardsCurrentTab",
{
deserializer: (obj) => obj,
},
);
export const SHOW_IDENTITIES_CURRENT_TAB = new KeyDefinition<boolean>(
VAULT_SETTINGS_DISK,
"showIdentitiesCurrentTab",
{ deserializer: (obj) => obj },
);

View File

@ -1,8 +1,12 @@
import { Observable, map } from "rxjs"; 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 { 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} * {@link VaultSettingsServiceAbstraction}
@ -10,7 +14,6 @@ import { USER_ENABLE_PASSKEYS } from "../key-state/enable-passkey.state";
export class VaultSettingsService implements VaultSettingsServiceAbstraction { export class VaultSettingsService implements VaultSettingsServiceAbstraction {
private enablePasskeysState: GlobalState<boolean> = private enablePasskeysState: GlobalState<boolean> =
this.stateProvider.getGlobal(USER_ENABLE_PASSKEYS); this.stateProvider.getGlobal(USER_ENABLE_PASSKEYS);
/** /**
* {@link VaultSettingsServiceAbstraction.enablePasskeys$} * {@link VaultSettingsServiceAbstraction.enablePasskeys$}
*/ */
@ -18,8 +21,40 @@ export class VaultSettingsService implements VaultSettingsServiceAbstraction {
map((x) => x ?? true), map((x) => x ?? true),
); );
private showCardsCurrentTabState: ActiveUserState<boolean> =
this.stateProvider.getActive(SHOW_CARDS_CURRENT_TAB);
/**
* {@link VaultSettingsServiceAbstraction.showCardsCurrentTab$}
*/
readonly showCardsCurrentTab$: Observable<boolean> = this.showCardsCurrentTabState.state$.pipe(
map((x) => x ?? true),
);
private showIdentitiesCurrentTabState: ActiveUserState<boolean> = this.stateProvider.getActive(
SHOW_IDENTITIES_CURRENT_TAB,
);
/**
* {@link VaultSettingsServiceAbstraction.showIdentitiesCurrentTab$}
*/
readonly showIdentitiesCurrentTab$: Observable<boolean> =
this.showIdentitiesCurrentTabState.state$.pipe(map((x) => x ?? true));
constructor(private stateProvider: StateProvider) {} constructor(private stateProvider: StateProvider) {}
/**
* {@link VaultSettingsServiceAbstraction.setShowCardsCurrentTab}
*/
async setShowCardsCurrentTab(value: boolean): Promise<void> {
await this.showCardsCurrentTabState.update(() => value);
}
/**
* {@link VaultSettingsServiceAbstraction.setDontShowIdentitiesCurrentTab}
*/
async setShowIdentitiesCurrentTab(value: boolean): Promise<void> {
await this.showIdentitiesCurrentTabState.update(() => value);
}
/** /**
* {@link VaultSettingsServiceAbstraction.setEnablePasskeys} * {@link VaultSettingsServiceAbstraction.setEnablePasskeys}
*/ */