Remove Storage Reseed FF (#11156)

This commit is contained in:
Justin Baur 2024-09-20 11:46:00 -04:00 committed by GitHub
parent 0516ca00dc
commit 972339be83
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 6 additions and 356 deletions

View File

@ -1484,14 +1484,7 @@ export default class MainBackground {
});
if (needStorageReseed) {
await this.reseedStorage(
await firstValueFrom(
this.configService.userCachedFeatureFlag$(
FeatureFlag.StorageReseedRefactor,
userBeingLoggedOut,
),
),
);
await this.reseedStorage();
}
if (BrowserApi.isManifestVersion(3)) {
@ -1546,7 +1539,7 @@ export default class MainBackground {
await SafariApp.sendMessageToApp("showPopover", null, true);
}
async reseedStorage(doFillBuffer: boolean) {
async reseedStorage() {
if (
!this.platformUtilsService.isChrome() &&
!this.platformUtilsService.isVivaldi() &&
@ -1555,11 +1548,7 @@ export default class MainBackground {
return;
}
if (doFillBuffer) {
await this.storageService.fillBuffer();
} else {
await this.storageService.reseed();
}
await this.storageService.fillBuffer();
}
async clearClipboard(clipboardValue: string, clearMs: number) {

View File

@ -1,4 +1,4 @@
import { firstValueFrom, map, mergeMap, of, switchMap } from "rxjs";
import { firstValueFrom, map, mergeMap } from "rxjs";
import { LockService } from "@bitwarden/auth/common";
import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service";
@ -281,22 +281,7 @@ export default class RuntimeBackground {
await this.main.refreshMenu();
break;
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);
await this.main.reseedStorage();
break;
}
case "authResult": {

View File

@ -1,192 +0,0 @@
import { objToStore } from "./abstractions/abstract-chrome-storage-api.service";
import BrowserLocalStorageService, {
RESEED_IN_PROGRESS_KEY,
} from "./browser-local-storage.service";
const apiGetLike =
(store: Record<any, any>) => (key: string, callback: (items: { [key: string]: any }) => void) => {
if (key == null) {
callback(store);
} else {
callback({ [key]: store[key] });
}
};
describe("BrowserLocalStorageService", () => {
let service: BrowserLocalStorageService;
let store: Record<any, any>;
let changeListener: (changes: { [key: string]: chrome.storage.StorageChange }) => void;
let saveMock: jest.Mock;
let getMock: jest.Mock;
let clearMock: jest.Mock;
let removeMock: jest.Mock;
beforeEach(() => {
store = {};
// Record change listener
chrome.storage.local.onChanged.addListener = jest.fn((listener) => {
changeListener = listener;
});
service = new BrowserLocalStorageService();
// setup mocks
getMock = chrome.storage.local.get as jest.Mock;
getMock.mockImplementation(apiGetLike(store));
saveMock = chrome.storage.local.set as jest.Mock;
saveMock.mockImplementation((update, callback) => {
Object.entries(update).forEach(([key, value]) => {
store[key] = value;
});
callback();
});
clearMock = chrome.storage.local.clear as jest.Mock;
clearMock.mockImplementation((callback) => {
store = {};
callback?.();
});
removeMock = chrome.storage.local.remove as jest.Mock;
removeMock.mockImplementation((keys, callback) => {
if (Array.isArray(keys)) {
keys.forEach((key) => {
delete store[key];
});
} else {
delete store[keys];
}
callback();
});
});
afterEach(() => {
chrome.runtime.lastError = undefined;
jest.resetAllMocks();
});
describe("reseed", () => {
it.each([
{
key1: objToStore("value1"),
key2: objToStore("value2"),
key3: null,
},
{},
])("saves all data in storage %s", async (testStore) => {
for (const key of Object.keys(testStore) as Array<keyof typeof testStore>) {
store[key] = testStore[key];
}
await service.reseed();
expect(saveMock).toHaveBeenLastCalledWith(
{ ...testStore, [RESEED_IN_PROGRESS_KEY]: objToStore(true) },
expect.any(Function),
);
});
it.each([
{
key1: objToStore("value1"),
key2: objToStore("value2"),
key3: null,
},
{},
])("results in the same store %s", async (testStore) => {
for (const key of Object.keys(testStore) as Array<keyof typeof testStore>) {
store[key] = testStore[key];
}
await service.reseed();
expect(store).toEqual(testStore);
});
it("converts non-serialized values to serialized", async () => {
store.key1 = "value1";
store.key2 = "value2";
const expectedStore = {
key1: objToStore("value1"),
key2: objToStore("value2"),
reseedInProgress: objToStore(true),
};
await service.reseed();
expect(saveMock).toHaveBeenLastCalledWith(expectedStore, expect.any(Function));
});
it("clears data", async () => {
await service.reseed();
expect(clearMock).toHaveBeenCalledTimes(1);
});
it("throws if get has chrome.runtime.lastError", async () => {
getMock.mockImplementation((key, callback) => {
chrome.runtime.lastError = new Error("Get Test Error");
callback();
});
await expect(async () => await service.reseed()).rejects.toThrow("Get Test Error");
});
it("throws if save has chrome.runtime.lastError", async () => {
saveMock.mockImplementation((obj, callback) => {
chrome.runtime.lastError = new Error("Save Test Error");
callback();
});
await expect(async () => await service.reseed()).rejects.toThrow("Save Test Error");
});
});
describe.each(["get", "has", "save", "remove"] as const)("%s", (method) => {
let interval: string | number | NodeJS.Timeout;
afterEach(() => {
if (interval) {
clearInterval(interval);
}
});
function startReseed() {
store[RESEED_IN_PROGRESS_KEY] = objToStore(true);
}
function endReseed() {
delete store[RESEED_IN_PROGRESS_KEY];
changeListener({ reseedInProgress: { oldValue: true } });
}
it("waits for reseed prior to operation", async () => {
startReseed();
const promise = service[method]("key", "value"); // note "value" is only used in save, but ignored in other methods
await expect(promise).not.toBeFulfilled(10);
endReseed();
await expect(promise).toBeResolved();
});
it("does not wait if reseed is not in progress", async () => {
const promise = service[method]("key", "value");
await expect(promise).toBeResolved(1);
});
it("awaits prior reseed operations before starting a new one", async () => {
startReseed();
const promise = service.reseed();
await expect(promise).not.toBeFulfilled(10);
endReseed();
await expect(promise).toBeResolved();
});
});
});

View File

@ -1,35 +1,8 @@
import { defer, filter, firstValueFrom, map, merge, throwError, timeout } from "rxjs";
import AbstractChromeStorageService, {
SerializedValue,
objToStore,
} from "./abstractions/abstract-chrome-storage-api.service";
export const RESEED_IN_PROGRESS_KEY = "reseedInProgress";
import AbstractChromeStorageService from "./abstractions/abstract-chrome-storage-api.service";
export default class BrowserLocalStorageService extends AbstractChromeStorageService {
constructor() {
super(chrome.storage.local);
this.chromeStorageApi.remove(RESEED_IN_PROGRESS_KEY, () => {
return;
});
}
/**
* Reads, clears, and re-saves all data in local storage. This is a hack to remove previously stored sensitive data from
* local storage logs.
*
* @see https://github.com/bitwarden/clients/issues/485
*/
async reseed(): Promise<void> {
try {
await this.save(RESEED_IN_PROGRESS_KEY, true);
const data = await this.getAll();
await this.clear();
await this.saveAll(data);
} finally {
await super.remove(RESEED_IN_PROGRESS_KEY);
}
}
async fillBuffer() {
@ -71,107 +44,4 @@ export default class BrowserLocalStorageService extends AbstractChromeStorageSer
);
});
}
override async get<T>(key: string): Promise<T> {
await this.awaitReseed();
return super.get(key);
}
override async has(key: string): Promise<boolean> {
await this.awaitReseed();
return super.has(key);
}
override async save(key: string, obj: any): Promise<void> {
await this.awaitReseed();
return super.save(key, obj);
}
override async remove(key: string): Promise<void> {
await this.awaitReseed();
return super.remove(key);
}
private async awaitReseed(): Promise<void> {
const notReseeding = async () => {
return !(await super.get(RESEED_IN_PROGRESS_KEY));
};
const finishedReseeding = this.updates$.pipe(
filter(({ key, updateType }) => key === RESEED_IN_PROGRESS_KEY && updateType === "remove"),
map(() => true),
);
await firstValueFrom(
merge(defer(notReseeding), finishedReseeding).pipe(
filter((v) => v),
timeout({
// We eventually need to give up and throw an error
first: 5_000,
with: () =>
throwError(
() => new Error("Reseeding local storage did not complete in a timely manner."),
),
}),
),
);
}
/**
* Clears local storage
*/
private async clear() {
return new Promise<void>((resolve, reject) => {
this.chromeStorageApi.clear(() => {
if (chrome.runtime.lastError) {
return reject(chrome.runtime.lastError);
}
resolve();
});
});
}
/**
* Retrieves all objects stored in local storage.
*
* @remarks This method processes values prior to resolving, do not use `chrome.storage.local` directly
* @returns Promise resolving to keyed object of all stored data
*/
private async getAll(): Promise<Record<string, unknown>> {
return new Promise((resolve, reject) => {
this.chromeStorageApi.get(null, (allStorage) => {
if (chrome.runtime.lastError) {
return reject(chrome.runtime.lastError);
}
const resolved = Object.entries(allStorage).reduce(
(agg, [key, value]) => {
agg[key] = this.processGetObject(value);
return agg;
},
{} as Record<string, unknown>,
);
resolve(resolved);
});
});
}
private async saveAll(data: Record<string, unknown>): Promise<void> {
return new Promise<void>((resolve, reject) => {
const keyedData = Object.entries(data).reduce(
(agg, [key, value]) => {
agg[key] = objToStore(value);
return agg;
},
{} as Record<string, SerializedValue>,
);
this.chromeStorageApi.set(keyedData, () => {
if (chrome.runtime.lastError) {
return reject(chrome.runtime.lastError);
}
resolve();
});
});
}
}

View File

@ -34,7 +34,6 @@ 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",
}
@ -77,7 +76,6 @@ export const DefaultFeatureFlagValue = {
[FeatureFlag.GenerateIdentityFillScriptRefactor]: FALSE,
[FeatureFlag.EnableNewCardCombinedExpiryAutofill]: FALSE,
[FeatureFlag.DelayFido2PageScriptInitWithinMv2]: FALSE,
[FeatureFlag.StorageReseedRefactor]: FALSE,
[FeatureFlag.AccountDeprovisioning]: FALSE,
[FeatureFlag.NotificationBarAddLoginImprovements]: FALSE,
[FeatureFlag.AC2476_DeprecateStripeSourcesAPI]: FALSE,