1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-11-22 11:45:59 +01:00

[PM-5275] Migrate state in Fido2ClientService to State Providers (#7745)

* added state definition and key definition

* created vault settings service

* created enable passkeys migrations

* created enable passkeys migrations

* renamed the state definition

* created vault settings service

* updated enable passkey key definition

* updated references with vault settings service

* renamed files to avoid conflict

* removed set and get enable passkeys from state service

* removed comment

* fixed comments

* added readonly keyword

* removed service registartion from service module

* removed readonly keyword from abstract class

* swicted to used optional chaining

* renamed files

* added disk-local argument for web
This commit is contained in:
SmithThe4th 2024-02-06 15:15:22 -05:00 committed by GitHub
parent 78008a9e1e
commit e9865c1cec
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 218 additions and 30 deletions

View File

@ -119,6 +119,7 @@ import { InternalFolderService as InternalFolderServiceAbstraction } from "@bitw
import { SyncNotifierService as SyncNotifierServiceAbstraction } from "@bitwarden/common/vault/abstractions/sync/sync-notifier.service.abstraction";
import { SyncService as SyncServiceAbstraction } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction";
import { TotpService as TotpServiceAbstraction } from "@bitwarden/common/vault/abstractions/totp.service";
import { VaultSettingsService as VaultSettingsServiceAbstraction } from "@bitwarden/common/vault/abstractions/vault-settings/vault-settings.service";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { CipherService } from "@bitwarden/common/vault/services/cipher.service";
import { CollectionService } from "@bitwarden/common/vault/services/collection.service";
@ -130,6 +131,7 @@ import { FolderService } from "@bitwarden/common/vault/services/folder/folder.se
import { SyncNotifierService } from "@bitwarden/common/vault/services/sync/sync-notifier.service";
import { SyncService } from "@bitwarden/common/vault/services/sync/sync.service";
import { TotpService } from "@bitwarden/common/vault/services/totp.service";
import { VaultSettingsService } from "@bitwarden/common/vault/services/vault-settings/vault-settings.service";
import {
IndividualVaultExportService,
IndividualVaultExportServiceAbstraction,
@ -268,6 +270,7 @@ export default class MainBackground {
fido2Service: Fido2ServiceAbstraction;
individualVaultExportService: IndividualVaultExportServiceAbstraction;
organizationVaultExportService: OrganizationVaultExportServiceAbstraction;
vaultSettingsService: VaultSettingsServiceAbstraction;
// Passed to the popup for Safari to workaround issues with theming, downloading, etc.
backgroundWindow = window;
@ -590,6 +593,8 @@ export default class MainBackground {
this.accountService,
);
this.vaultSettingsService = new VaultSettingsService(this.stateProvider);
this.vaultTimeoutService = new VaultTimeoutService(
this.cipherService,
this.folderService,
@ -719,6 +724,7 @@ export default class MainBackground {
this.configService,
this.authService,
this.stateService,
this.vaultSettingsService,
this.logService,
);

View File

@ -1,4 +1,5 @@
import { Component, OnInit } from "@angular/core";
import { firstValueFrom } from "rxjs";
import { AbstractThemingService } from "@bitwarden/angular/platform/services/theming/theming.service.abstraction";
import { SettingsService } from "@bitwarden/common/abstractions/settings.service";
@ -7,6 +8,7 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
import { ThemeType } from "@bitwarden/common/platform/enums";
import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service";
import { VaultSettingsService } from "@bitwarden/common/vault/abstractions/vault-settings/vault-settings.service";
import { UriMatchType } from "@bitwarden/common/vault/enums";
import { enableAccountSwitching } from "../../platform/flags";
@ -47,6 +49,7 @@ export class OptionsComponent implements OnInit {
i18nService: I18nService,
private themingService: AbstractThemingService,
private settingsService: SettingsService,
private vaultSettingsService: VaultSettingsService,
) {
this.themeOptions = [
{ name: i18nService.t("default"), value: ThemeType.System },
@ -102,7 +105,7 @@ export class OptionsComponent implements OnInit {
this.enableBadgeCounter = !(await this.stateService.getDisableBadgeCounter());
this.enablePasskeys = await this.stateService.getEnablePasskeys();
this.enablePasskeys = await firstValueFrom(this.vaultSettingsService.enablePasskeys$);
this.theme = await this.stateService.getTheme();
@ -123,7 +126,7 @@ export class OptionsComponent implements OnInit {
}
async updateEnablePasskeys() {
await this.stateService.setEnablePasskeys(this.enablePasskeys);
await this.vaultSettingsService.setEnablePasskeys(this.enablePasskeys);
}
async updateContextMenuItem() {

View File

@ -172,6 +172,7 @@ import {
import { SyncNotifierService as SyncNotifierServiceAbstraction } from "@bitwarden/common/vault/abstractions/sync/sync-notifier.service.abstraction";
import { SyncService as SyncServiceAbstraction } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction";
import { TotpService as TotpServiceAbstraction } from "@bitwarden/common/vault/abstractions/totp.service";
import { VaultSettingsService as VaultSettingsServiceAbstraction } from "@bitwarden/common/vault/abstractions/vault-settings/vault-settings.service";
import { CipherService } from "@bitwarden/common/vault/services/cipher.service";
import { CollectionService } from "@bitwarden/common/vault/services/collection.service";
import { CipherFileUploadService } from "@bitwarden/common/vault/services/file-upload/cipher-file-upload.service";
@ -180,6 +181,7 @@ import { FolderService } from "@bitwarden/common/vault/services/folder/folder.se
import { SyncNotifierService } from "@bitwarden/common/vault/services/sync/sync-notifier.service";
import { SyncService } from "@bitwarden/common/vault/services/sync/sync.service";
import { TotpService } from "@bitwarden/common/vault/services/totp.service";
import { VaultSettingsService } from "@bitwarden/common/vault/services/vault-settings/vault-settings.service";
import {
VaultExportService,
VaultExportServiceAbstraction,
@ -902,6 +904,11 @@ import { ModalService } from "./modal.service";
useClass: DefaultBiometricStateService,
deps: [StateProvider],
},
{
provide: VaultSettingsServiceAbstraction,
useClass: VaultSettingsService,
deps: [StateProvider],
},
],
})
export class JslibServicesModule {}

View File

@ -252,8 +252,6 @@ export abstract class StateService<T extends Account = Account> {
value: boolean,
options?: StorageOptions,
) => Promise<void>;
getEnablePasskeys: (options?: StorageOptions) => Promise<boolean>;
setEnablePasskeys: (value: boolean, options?: StorageOptions) => Promise<void>;
getDisableContextMenuItem: (options?: StorageOptions) => Promise<boolean>;
setDisableContextMenuItem: (value: boolean, options?: StorageOptions) => Promise<void>;
/**

View File

@ -1211,24 +1211,6 @@ export class StateService<
);
}
async getEnablePasskeys(options?: StorageOptions): Promise<boolean> {
return (
(await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions())))
?.enablePasskeys ?? true
);
}
async setEnablePasskeys(value: boolean, options?: StorageOptions): Promise<void> {
const globals = await this.getGlobals(
this.reconcileOptions(options, await this.defaultOnDiskOptions()),
);
globals.enablePasskeys = value;
await this.saveGlobals(
globals,
this.reconcileOptions(options, await this.defaultOnDiskOptions()),
);
}
async getDisableContextMenuItem(options?: StorageOptions): Promise<boolean> {
return (
(await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions())))

View File

@ -39,3 +39,7 @@ export const PROVIDERS_DISK = new StateDefinition("providers", "disk");
export const FOLDER_DISK = new StateDefinition("folder", "disk", { web: "memory" });
export const SYNC_STATE = new StateDefinition("sync", "disk", { web: "memory" });
export const VAULT_SETTINGS_DISK = new StateDefinition("vaultSettings", "disk", {
web: "disk-local",
});

View File

@ -12,6 +12,7 @@ import { ProviderKeyMigrator } from "./migrations/13-move-provider-keys-to-state
import { MoveBiometricClientKeyHalfToStateProviders } from "./migrations/14-move-biometric-client-key-half-state-to-providers";
import { FolderMigrator } from "./migrations/15-move-folder-state-to-state-provider";
import { LastSyncMigrator } from "./migrations/16-move-last-sync-to-state-provider";
import { EnablePasskeysMigrator } from "./migrations/17-move-enable-passkeys-to-state-providers";
import { FixPremiumMigrator } from "./migrations/3-fix-premium";
import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked";
import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys";
@ -22,7 +23,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting
import { MinVersionMigrator } from "./migrations/min-version";
export const MIN_VERSION = 2;
export const CURRENT_VERSION = 16;
export const CURRENT_VERSION = 17;
export type MinVersion = typeof MIN_VERSION;
export async function migrate(
@ -54,7 +55,8 @@ export async function migrate(
.with(ProviderKeyMigrator, 12, 13)
.with(MoveBiometricClientKeyHalfToStateProviders, 13, 14)
.with(FolderMigrator, 14, 15)
.with(LastSyncMigrator, 15, CURRENT_VERSION)
.with(LastSyncMigrator, 15, 16)
.with(EnablePasskeysMigrator, 16, CURRENT_VERSION)
.migrate(migrationHelper);
}

View File

@ -120,7 +120,7 @@ describe("FolderMigrator", () => {
describe("rollback", () => {
beforeEach(() => {
helper = mockMigrationHelper(rollbackJSON(), 14);
helper = mockMigrationHelper(rollbackJSON(), 15);
sut = new FolderMigrator(14, 15);
});

View File

@ -0,0 +1,84 @@
import { MockProxy } from "jest-mock-extended";
import { MigrationHelper } from "../migration-helper";
import { mockMigrationHelper } from "../migration-helper.spec";
import { EnablePasskeysMigrator } from "./17-move-enable-passkeys-to-state-providers";
function exampleJSON() {
return {
global: {
enablePasskeys: true,
otherStuff: "otherStuff1",
},
authenticatedAccounts: ["user-1", "user-2"],
"user-1": {
settings: {
otherStuff: "otherStuff2",
},
otherStuff: "otherStuff3",
},
"user-2": {
settings: {
otherStuff: "otherStuff4",
},
otherStuff: "otherStuff5",
},
};
}
function rollbackJSON() {
return {
global_vaultSettings_enablePasskeys: true,
global: {
otherStuff: "otherStuff1",
},
authenticatedAccounts: ["user-1", "user-2"],
"user-1": {
settings: {
otherStuff: "otherStuff2",
},
otherStuff: "otherStuff3",
},
"user-2": {
settings: {
otherStuff: "otherStuff4",
},
otherStuff: "otherStuff5",
},
};
}
describe("EnablePasskeysMigrator", () => {
let helper: MockProxy<MigrationHelper>;
let sut: EnablePasskeysMigrator;
describe("migrate", () => {
beforeEach(() => {
helper = mockMigrationHelper(exampleJSON(), 16);
sut = new EnablePasskeysMigrator(16, 17);
});
it("should remove enablePasskeys from global", async () => {
await sut.migrate(helper);
expect(helper.set).toHaveBeenCalledWith("global", {
otherStuff: "otherStuff1",
});
});
});
describe("rollback", () => {
beforeEach(() => {
helper = mockMigrationHelper(rollbackJSON(), 17);
sut = new EnablePasskeysMigrator(16, 17);
});
it("should move enablePasskeys to global", async () => {
await sut.rollback(helper);
expect(helper.set).toHaveBeenCalledWith("global", {
enablePasskeys: true,
otherStuff: "otherStuff1",
});
});
});
});

View File

@ -0,0 +1,36 @@
import { KeyDefinitionLike, MigrationHelper } from "../migration-helper";
import { Migrator } from "../migrator";
type ExpectedGlobalType = {
enablePasskeys?: boolean;
};
const USER_ENABLE_PASSKEYS: KeyDefinitionLike = {
key: "enablePasskeys",
stateDefinition: {
name: "vaultSettings",
},
};
export class EnablePasskeysMigrator extends Migrator<16, 17> {
async migrate(helper: MigrationHelper): Promise<void> {
const global = await helper.get<ExpectedGlobalType>("global");
if (global?.enablePasskeys != null) {
await helper.setToGlobal(USER_ENABLE_PASSKEYS, global.enablePasskeys);
delete global?.enablePasskeys;
await helper.set("global", global);
}
}
async rollback(helper: MigrationHelper): Promise<void> {
let global = await helper.get<ExpectedGlobalType>("global");
const globalEnablePasskeys = await helper.getFromGlobal<boolean>(USER_ENABLE_PASSKEYS);
if (globalEnablePasskeys != null) {
global = Object.assign(global ?? {}, { enablePasskeys: globalEnablePasskeys });
await helper.set("global", global);
await helper.setToGlobal(USER_ENABLE_PASSKEYS, undefined);
}
}
}

View File

@ -0,0 +1,17 @@
import { Observable } from "rxjs";
/**
* Service for managing vault settings.
*/
export abstract class VaultSettingsService {
/**
* An observable monitoring the state of the enable passkeys setting.
* The observable updates when the setting changes.
*/
enablePasskeys$: Observable<boolean>;
/**
* Saves the enable passkeys setting to disk.
* @param value The new value for the passkeys setting.
*/
setEnablePasskeys: (value: boolean) => Promise<void>;
}

View File

@ -17,6 +17,7 @@ import {
CreateCredentialParams,
FallbackRequestedError,
} from "../../abstractions/fido2/fido2-client.service.abstraction";
import { VaultSettingsService } from "../../abstractions/vault-settings/vault-settings.service";
import { Fido2AuthenticatorService } from "./fido2-authenticator.service";
import { Fido2ClientService } from "./fido2-client.service";
@ -32,6 +33,7 @@ describe("FidoAuthenticatorService", () => {
let configService!: MockProxy<ConfigServiceAbstraction>;
let authService!: MockProxy<AuthService>;
let stateService!: MockProxy<StateService>;
let vaultSettingsService: MockProxy<VaultSettingsService>;
let client!: Fido2ClientService;
let tab!: chrome.tabs.Tab;
@ -40,10 +42,17 @@ describe("FidoAuthenticatorService", () => {
configService = mock<ConfigServiceAbstraction>();
authService = mock<AuthService>();
stateService = mock<StateService>();
vaultSettingsService = mock<VaultSettingsService>();
client = new Fido2ClientService(authenticator, configService, authService, stateService);
client = new Fido2ClientService(
authenticator,
configService,
authService,
stateService,
vaultSettingsService,
);
configService.serverConfig$ = of({ environment: { vault: VaultUrl } } as any);
stateService.getEnablePasskeys.mockResolvedValue(true);
vaultSettingsService.enablePasskeys$ = of(true);
authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Unlocked);
tab = { id: 123, windowId: 456 } as chrome.tabs.Tab;
});
@ -226,7 +235,7 @@ describe("FidoAuthenticatorService", () => {
it("should throw FallbackRequestedError if passkeys state is not enabled", async () => {
const params = createParams();
stateService.getEnablePasskeys.mockResolvedValue(false);
vaultSettingsService.enablePasskeys$ = of(false);
const result = async () => await client.createCredential(params, tab);
@ -396,7 +405,7 @@ describe("FidoAuthenticatorService", () => {
it("should throw FallbackRequestedError if passkeys state is not enabled", async () => {
const params = createParams();
stateService.getEnablePasskeys.mockResolvedValue(false);
vaultSettingsService.enablePasskeys$ = of(false);
const result = async () => await client.assertCredential(params, tab);

View File

@ -26,6 +26,7 @@ import {
UserRequestedFallbackAbortReason,
UserVerification,
} from "../../abstractions/fido2/fido2-client.service.abstraction";
import { VaultSettingsService } from "../../abstractions/vault-settings/vault-settings.service";
import { isValidRpId } from "./domain-utils";
import { Fido2Utils } from "./fido2-utils";
@ -42,11 +43,12 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
private configService: ConfigServiceAbstraction,
private authService: AuthService,
private stateService: StateService,
private vaultSettingsService: VaultSettingsService,
private logService?: LogService,
) {}
async isFido2FeatureEnabled(hostname: string, origin: string): Promise<boolean> {
const userEnabledPasskeys = await this.stateService.getEnablePasskeys();
const userEnabledPasskeys = await firstValueFrom(this.vaultSettingsService.enablePasskeys$);
const isUserLoggedIn =
(await this.authService.getAuthStatus()) !== AuthenticationStatus.LoggedOut;

View File

@ -0,0 +1,9 @@
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,29 @@
import { Observable, map } from "rxjs";
import { 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";
/**
* {@link VaultSettingsServiceAbstraction}
*/
export class VaultSettingsService implements VaultSettingsServiceAbstraction {
private enablePasskeysState: GlobalState<boolean> =
this.stateProvider.getGlobal(USER_ENABLE_PASSKEYS);
/**
* {@link VaultSettingsServiceAbstraction.enablePasskeys$}
*/
readonly enablePasskeys$: Observable<boolean> = this.enablePasskeysState.state$.pipe(
map((x) => x ?? false),
);
constructor(private stateProvider: StateProvider) {}
/**
* {@link VaultSettingsServiceAbstraction.setEnablePasskeys}
*/
async setEnablePasskeys(value: boolean): Promise<void> {
await this.enablePasskeysState.update(() => value);
}
}