diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 2c48a8563e..e451b55c9e 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1463,7 +1463,14 @@ export default class MainBackground { }); if (needStorageReseed) { - await this.reseedStorage(); + await this.reseedStorage( + await firstValueFrom( + this.configService.userCachedFeatureFlag$( + FeatureFlag.StorageReseedRefactor, + userBeingLoggedOut, + ), + ), + ); } if (BrowserApi.isManifestVersion(3)) { @@ -1518,7 +1525,7 @@ export default class MainBackground { await SafariApp.sendMessageToApp("showPopover", null, true); } - async reseedStorage() { + async reseedStorage(doFillBuffer: boolean) { if ( !this.platformUtilsService.isChrome() && !this.platformUtilsService.isVivaldi() && @@ -1527,7 +1534,11 @@ export default class MainBackground { return; } - await this.storageService.reseed(); + if (doFillBuffer) { + await this.storageService.fillBuffer(); + } else { + await this.storageService.reseed(); + } } async clearClipboard(clipboardValue: string, clearMs: number) { diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index 44e395659b..424449f0b6 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -1,4 +1,4 @@ -import { firstValueFrom, map, mergeMap } from "rxjs"; +import { firstValueFrom, map, mergeMap, of, switchMap } from "rxjs"; import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; @@ -272,9 +272,25 @@ export default class RuntimeBackground { await this.main.refreshBadge(); await this.main.refreshMenu(); break; - case "bgReseedStorage": - await this.main.reseedStorage(); + case "bgReseedStorage": { + const doFillBuffer = await firstValueFrom( + this.accountService.activeAccount$.pipe( + switchMap((account) => { + if (account == null) { + return of(false); + } + + return this.configService.userCachedFeatureFlag$( + FeatureFlag.StorageReseedRefactor, + account.id, + ); + }), + ), + ); + + await this.main.reseedStorage(doFillBuffer); break; + } case "authResult": { const env = await firstValueFrom(this.environmentService.environment$); const vaultUrl = env.getWebVaultUrl(); diff --git a/apps/browser/src/platform/services/browser-local-storage.service.ts b/apps/browser/src/platform/services/browser-local-storage.service.ts index 0ba200055b..15cf26d1fb 100644 --- a/apps/browser/src/platform/services/browser-local-storage.service.ts +++ b/apps/browser/src/platform/services/browser-local-storage.service.ts @@ -32,6 +32,46 @@ export default class BrowserLocalStorageService extends AbstractChromeStorageSer } } + async fillBuffer() { + // Write 4MB of data in chrome.storage.local, log files will hold 4MB of data (by default) + // before forcing a compaction. To force a compaction and have it remove previously saved data, + // we want to fill it's buffer so that anything newly marked for deletion is gone. + // https://github.com/google/leveldb/blob/main/doc/impl.md#log-files + // It's important that if Google uses a different buffer length that we match that, as far as I can tell + // Google uses the default value in Chromium: + // https://github.com/chromium/chromium/blob/148774efa6b3a047369af6179a4248566b39d68f/components/value_store/lazy_leveldb.cc#L65-L66 + const fakeData = "0".repeat(1024 * 1024); // 1MB of data + await new Promise((resolve, reject) => { + this.chromeStorageApi.set( + { + fake_data_1: fakeData, + fake_data_2: fakeData, + fake_data_3: fakeData, + fake_data_4: fakeData, + }, + () => { + if (chrome.runtime.lastError) { + return reject(chrome.runtime.lastError); + } + + resolve(); + }, + ); + }); + await new Promise((resolve, reject) => { + this.chromeStorageApi.remove( + ["fake_data_1", "fake_data_2", "fake_data_3", "fake_data_4"], + () => { + if (chrome.runtime.lastError) { + return reject(chrome.runtime.lastError); + } + + resolve(); + }, + ); + }); + } + override async get(key: string): Promise { await this.awaitReseed(); return super.get(key); diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 9ec88f019e..a2423dc1a9 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -34,6 +34,7 @@ export enum FeatureFlag { AccountDeprovisioning = "pm-10308-account-deprovisioning", NotificationBarAddLoginImprovements = "notification-bar-add-login-improvements", AC2476_DeprecateStripeSourcesAPI = "AC-2476-deprecate-stripe-sources-api", + StorageReseedRefactor = "storage-reseed-refactor", CipherKeyEncryption = "cipher-key-encryption", } @@ -76,6 +77,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.EnableUpgradePasswordManagerSub]: FALSE, [FeatureFlag.GenerateIdentityFillScriptRefactor]: FALSE, [FeatureFlag.DelayFido2PageScriptInitWithinMv2]: FALSE, + [FeatureFlag.StorageReseedRefactor]: FALSE, [FeatureFlag.AccountDeprovisioning]: FALSE, [FeatureFlag.NotificationBarAddLoginImprovements]: FALSE, [FeatureFlag.AC2476_DeprecateStripeSourcesAPI]: FALSE, diff --git a/libs/common/src/platform/abstractions/config/config.service.ts b/libs/common/src/platform/abstractions/config/config.service.ts index 6985430acc..9b16cee385 100644 --- a/libs/common/src/platform/abstractions/config/config.service.ts +++ b/libs/common/src/platform/abstractions/config/config.service.ts @@ -2,6 +2,7 @@ import { Observable } from "rxjs"; import { SemVer } from "semver"; import { FeatureFlag, FeatureFlagValueType } from "../../../enums/feature-flag.enum"; +import { UserId } from "../../../types/guid"; import { Region } from "../environment.service"; import { ServerConfig } from "./server-config"; @@ -17,6 +18,18 @@ export abstract class ConfigService { * @returns An observable that emits the value of the feature flag, updates as the server config changes */ getFeatureFlag$: (key: Flag) => Observable>; + + /** + * Retrieves the cached feature flag value for a give user. This will NOT call to the server to get + * the most up to date feature flag. + * @param key The feature flag key to get the value for. + * @param userId The user id of the user to get the feature flag value for. + */ + abstract userCachedFeatureFlag$( + key: Flag, + userId: UserId, + ): Observable>; + /** * Retrieves the value of a feature flag for the currently active user * @param key The feature flag to retrieve diff --git a/libs/common/src/platform/models/response/server-config.response.ts b/libs/common/src/platform/models/response/server-config.response.ts index f611acf6f4..a546d2d3de 100644 --- a/libs/common/src/platform/models/response/server-config.response.ts +++ b/libs/common/src/platform/models/response/server-config.response.ts @@ -1,3 +1,4 @@ +import { AllowedFeatureFlagTypes } from "../../../enums/feature-flag.enum"; import { BaseResponse } from "../../../models/response/base.response"; import { Region } from "../../abstractions/environment.service"; @@ -6,7 +7,7 @@ export class ServerConfigResponse extends BaseResponse { gitHash: string; server: ThirdPartyServerConfigResponse; environment: EnvironmentServerConfigResponse; - featureStates: { [key: string]: string } = {}; + featureStates: { [key: string]: AllowedFeatureFlagTypes } = {}; constructor(response: any) { super(response); diff --git a/libs/common/src/platform/services/config/config.service.spec.ts b/libs/common/src/platform/services/config/config.service.spec.ts index efe75f0882..369338f945 100644 --- a/libs/common/src/platform/services/config/config.service.spec.ts +++ b/libs/common/src/platform/services/config/config.service.spec.ts @@ -16,6 +16,7 @@ import { import { subscribeTo } from "../../../../spec/observable-tracker"; import { AuthService } from "../../../auth/abstractions/auth.service"; import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; +import { FeatureFlag } from "../../../enums/feature-flag.enum"; import { UserId } from "../../../types/guid"; import { ConfigApiServiceAbstraction } from "../../abstractions/config/config-api.service.abstraction"; import { ServerConfig } from "../../abstractions/config/server-config"; @@ -277,6 +278,48 @@ describe("ConfigService", () => { }); }); + describe("userCachedFeatureFlag$", () => { + it("maps saved user config to a feature flag", async () => { + const updateFeature = (value: boolean) => { + return new ServerConfig( + new ServerConfigData({ + featureStates: { + "test-feature": value, + }, + }), + ); + }; + + const configService = new DefaultConfigService( + configApiService, + environmentService, + logService, + stateProvider, + authService, + ); + + userState.nextState(null); + + const promise = firstValueFrom( + configService + .userCachedFeatureFlag$("test-feature" as FeatureFlag, userId) + .pipe(bufferCount(3)), + ); + + userState.nextState(updateFeature(true)); + userState.nextState(updateFeature(false)); + + const values = await promise; + + // We wouldn't normally expect this to be undefined, the logic + // should normally return the feature flags default value but since + // we are faking a feature flag key, undefined is expected + expect(values[0]).toBe(undefined); + expect(values[1]).toBe(true); + expect(values[2]).toBe(false); + }); + }); + describe("slow configuration", () => { const environmentSubject = new BehaviorSubject(null); diff --git a/libs/common/src/platform/services/config/default-config.service.ts b/libs/common/src/platform/services/config/default-config.service.ts index 74dd5055d4..e0603ed509 100644 --- a/libs/common/src/platform/services/config/default-config.service.ts +++ b/libs/common/src/platform/services/config/default-config.service.ts @@ -115,16 +115,27 @@ export class DefaultConfigService implements ConfigService { getFeatureFlag$(key: Flag) { return this.serverConfig$.pipe( - map((serverConfig) => { - if (serverConfig?.featureStates == null || serverConfig.featureStates[key] == null) { - return DefaultFeatureFlagValue[key]; - } - - return serverConfig.featureStates[key] as FeatureFlagValueType; - }), + map((serverConfig) => this.getFeatureFlagValue(serverConfig, key)), ); } + private getFeatureFlagValue( + serverConfig: ServerConfig | null, + flag: Flag, + ) { + if (serverConfig?.featureStates == null || serverConfig.featureStates[flag] == null) { + return DefaultFeatureFlagValue[flag]; + } + + return serverConfig.featureStates[flag] as FeatureFlagValueType; + } + + userCachedFeatureFlag$(key: Flag, userId: UserId) { + return this.stateProvider + .getUser(userId, USER_SERVER_CONFIG) + .state$.pipe(map((config) => this.getFeatureFlagValue(config, key))); + } + async getFeatureFlag(key: Flag) { return await firstValueFrom(this.getFeatureFlag$(key)); }