diff --git a/apps/browser/config/development.json b/apps/browser/config/development.json index 1b628c173c..aba10eb25b 100644 --- a/apps/browser/config/development.json +++ b/apps/browser/config/development.json @@ -1,6 +1,5 @@ { "devFlags": { - "storeSessionDecrypted": false, "managedEnvironment": { "base": "https://localhost:8080" } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 0a9ad44962..fa1add0602 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -98,7 +98,9 @@ import { StateFactory } from "@bitwarden/common/platform/factories/state-factory import { Message, MessageListener, MessageSender } from "@bitwarden/common/platform/messaging"; // eslint-disable-next-line no-restricted-imports -- Used for dependency creation import { SubjectMessageSender } from "@bitwarden/common/platform/messaging/internal"; +import { Lazy } from "@bitwarden/common/platform/misc/lazy"; import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state"; +import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { AppIdService } from "@bitwarden/common/platform/services/app-id.service"; import { ConfigApiService } from "@bitwarden/common/platform/services/config/config-api.service"; import { DefaultConfigService } from "@bitwarden/common/platform/services/config/default-config.service"; @@ -404,32 +406,51 @@ export default class MainBackground { self, ); - const mv3MemoryStorageCreator = (partitionName: string) => { - if (this.popupOnlyContext) { - return new ForegroundMemoryStorageService(partitionName); + // Creates a session key for mv3 storage of large memory items + const sessionKey = new Lazy(async () => { + // Key already in session storage + const sessionStorage = new BrowserMemoryStorageService(); + const existingKey = await sessionStorage.get("session-key"); + if (existingKey) { + if (sessionStorage.valuesRequireDeserialization) { + return SymmetricCryptoKey.fromJSON(existingKey); + } + return existingKey; + } + + // New key + const { derivedKey } = await this.keyGenerationService.createKeyWithPurpose( + 128, + "ephemeral", + "bitwarden-ephemeral", + ); + await sessionStorage.save("session-key", derivedKey); + return derivedKey; + }); + + const mv3MemoryStorageCreator = () => { + if (this.popupOnlyContext) { + return new ForegroundMemoryStorageService(); } - // TODO: Consider using multithreaded encrypt service in popup only context return new LocalBackedSessionStorageService( - this.logService, + sessionKey, + this.storageService, new EncryptServiceImplementation(this.cryptoFunctionService, this.logService, false), - this.keyGenerationService, - new BrowserLocalStorageService(), - new BrowserMemoryStorageService(), this.platformUtilsService, - partitionName, + this.logService, ); }; this.secureStorageService = this.storageService; // secure storage is not supported in browsers, so we use local storage and warn users when it is used - this.memoryStorageService = BrowserApi.isManifestVersion(3) - ? mv3MemoryStorageCreator("stateService") - : new MemoryStorageService(); this.memoryStorageForStateProviders = BrowserApi.isManifestVersion(3) ? new BrowserMemoryStorageService() // mv3 stores to storage.session : new BackgroundMemoryStorageService(); // mv2 stores to memory + this.memoryStorageService = BrowserApi.isManifestVersion(3) + ? this.memoryStorageForStateProviders // manifest v3 can reuse the same storage. They are split for v2 due to lacking a good sync mechanism, which isn't true for v3 + : new MemoryStorageService(); this.largeObjectMemoryStorageForStateProviders = BrowserApi.isManifestVersion(3) - ? mv3MemoryStorageCreator("stateProviders") // mv3 stores to local-backed session storage + ? mv3MemoryStorageCreator() // mv3 stores to local-backed session storage : this.memoryStorageForStateProviders; // mv2 stores to the same location const storageServiceProvider = new BrowserStorageServiceProvider( diff --git a/apps/browser/src/platform/background/service-factories/storage-service.factory.ts b/apps/browser/src/platform/background/service-factories/storage-service.factory.ts index 83e8a780a6..e63e39944d 100644 --- a/apps/browser/src/platform/background/service-factories/storage-service.factory.ts +++ b/apps/browser/src/platform/background/service-factories/storage-service.factory.ts @@ -3,6 +3,8 @@ import { AbstractStorageService, ObservableStorageService, } from "@bitwarden/common/platform/abstractions/storage.service"; +import { Lazy } from "@bitwarden/common/platform/misc/lazy"; +import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { MemoryStorageService } from "@bitwarden/common/platform/services/memory-storage.service"; import { BrowserApi } from "../../browser/browser-api"; @@ -17,10 +19,10 @@ import { KeyGenerationServiceInitOptions, keyGenerationServiceFactory, } from "./key-generation-service.factory"; -import { logServiceFactory, LogServiceInitOptions } from "./log-service.factory"; +import { LogServiceInitOptions, logServiceFactory } from "./log-service.factory"; import { - platformUtilsServiceFactory, PlatformUtilsServiceInitOptions, + platformUtilsServiceFactory, } from "./platform-utils-service.factory"; export type DiskStorageServiceInitOptions = FactoryOptions; @@ -70,13 +72,23 @@ export function memoryStorageServiceFactory( return factory(cache, "memoryStorageService", opts, async () => { if (BrowserApi.isManifestVersion(3)) { return new LocalBackedSessionStorageService( - await logServiceFactory(cache, opts), - await encryptServiceFactory(cache, opts), - await keyGenerationServiceFactory(cache, opts), + new Lazy(async () => { + const existingKey = await ( + await sessionStorageServiceFactory(cache, opts) + ).get("session-key"); + if (existingKey) { + return existingKey; + } + const { derivedKey } = await ( + await keyGenerationServiceFactory(cache, opts) + ).createKeyWithPurpose(128, "ephemeral", "bitwarden-ephemeral"); + await (await sessionStorageServiceFactory(cache, opts)).save("session-key", derivedKey); + return derivedKey; + }), await diskStorageServiceFactory(cache, opts), - await sessionStorageServiceFactory(cache, opts), + await encryptServiceFactory(cache, opts), await platformUtilsServiceFactory(cache, opts), - "serviceFactories", + await logServiceFactory(cache, opts), ); } return new MemoryStorageService(); diff --git a/apps/browser/src/platform/decorators/dev-flag.decorator.spec.ts b/apps/browser/src/platform/decorators/dev-flag.decorator.spec.ts index c5401f8a09..da00bc6fe3 100644 --- a/apps/browser/src/platform/decorators/dev-flag.decorator.spec.ts +++ b/apps/browser/src/platform/decorators/dev-flag.decorator.spec.ts @@ -9,7 +9,7 @@ jest.mock("../flags", () => ({ })); class TestClass { - @devFlag("storeSessionDecrypted") test() { + @devFlag("managedEnvironment") test() { return "test"; } } diff --git a/apps/browser/src/platform/flags.ts b/apps/browser/src/platform/flags.ts index 36aa698a7b..383e982f06 100644 --- a/apps/browser/src/platform/flags.ts +++ b/apps/browser/src/platform/flags.ts @@ -19,7 +19,6 @@ export type Flags = { // required to avoid linting errors when there are no flags // eslint-disable-next-line @typescript-eslint/ban-types export type DevFlags = { - storeSessionDecrypted?: boolean; managedEnvironment?: GroupPolicyEnvironment; } & SharedDevFlags; diff --git a/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts b/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts index a4581e6ac1..7114bda06e 100644 --- a/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts +++ b/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts @@ -1,412 +1,200 @@ import { mock, MockProxy } from "jest-mock-extended"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; -import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { - AbstractMemoryStorageService, - AbstractStorageService, - StorageUpdate, -} from "@bitwarden/common/platform/abstractions/storage.service"; +import { Lazy } from "@bitwarden/common/platform/misc/lazy"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; - -import { BrowserApi } from "../browser/browser-api"; +import { FakeStorageService, makeEncString } from "@bitwarden/common/spec"; import { LocalBackedSessionStorageService } from "./local-backed-session-storage.service"; -describe.skip("LocalBackedSessionStorage", () => { - const sendMessageWithResponseSpy: jest.SpyInstance = jest.spyOn( - BrowserApi, - "sendMessageWithResponse", +describe("LocalBackedSessionStorage", () => { + const sessionKey = new SymmetricCryptoKey( + Utils.fromUtf8ToArray("00000000000000000000000000000000"), ); - + let localStorage: FakeStorageService; let encryptService: MockProxy; - let keyGenerationService: MockProxy; - let localStorageService: MockProxy; - let sessionStorageService: MockProxy; - let logService: MockProxy; let platformUtilsService: MockProxy; - - let cache: Record; - const testObj = { a: 1, b: 2 }; - const stringifiedTestObj = JSON.stringify(testObj); - - const key = new SymmetricCryptoKey(Utils.fromUtf8ToArray("00000000000000000000000000000000")); - let getSessionKeySpy: jest.SpyInstance; - let sendUpdateSpy: jest.SpyInstance; - const mockEnc = (input: string) => Promise.resolve(new EncString("ENCRYPTED" + input)); + let logService: MockProxy; let sut: LocalBackedSessionStorageService; - const mockExistingSessionKey = (key: SymmetricCryptoKey) => { - sessionStorageService.get.mockImplementation((storageKey) => { - if (storageKey === "localEncryptionKey_test") { - return Promise.resolve(key?.toJSON()); - } - - return Promise.reject("No implementation for " + storageKey); - }); - }; - beforeEach(() => { - sendMessageWithResponseSpy.mockResolvedValue(null); - logService = mock(); + localStorage = new FakeStorageService(); encryptService = mock(); - keyGenerationService = mock(); - localStorageService = mock(); - sessionStorageService = mock(); + platformUtilsService = mock(); + logService = mock(); sut = new LocalBackedSessionStorageService( - logService, + new Lazy(async () => sessionKey), + localStorage, encryptService, - keyGenerationService, - localStorageService, - sessionStorageService, platformUtilsService, - "test", + logService, ); - - cache = sut["cachedSession"]; - - keyGenerationService.createKeyWithPurpose.mockResolvedValue({ - derivedKey: key, - salt: "bitwarden-ephemeral", - material: null, // Not used - }); - - getSessionKeySpy = jest.spyOn(sut, "getSessionEncKey"); - getSessionKeySpy.mockResolvedValue(key); - - // sendUpdateSpy = jest.spyOn(sut, "sendUpdate"); - // sendUpdateSpy.mockReturnValue(); }); describe("get", () => { - describe("in local cache or external context cache", () => { - it("should return from local cache", async () => { - cache["test"] = stringifiedTestObj; - const result = await sut.get("test"); - expect(result).toStrictEqual(testObj); - }); - - it("should return from external context cache when local cache is not available", async () => { - sendMessageWithResponseSpy.mockResolvedValue(stringifiedTestObj); - const result = await sut.get("test"); - expect(result).toStrictEqual(testObj); - }); + it("return the cached value when one is cached", async () => { + sut["cache"]["test"] = "cached"; + const result = await sut.get("test"); + expect(result).toEqual("cached"); }); - describe("not in cache", () => { - const session = { test: stringifiedTestObj }; + it("returns a decrypted value when one is stored in local storage", async () => { + const encrypted = makeEncString("encrypted"); + localStorage.internalStore["session_test"] = encrypted.encryptedString; + encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); + const result = await sut.get("test"); + expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encrypted, sessionKey); + expect(result).toEqual("decrypted"); + }); - beforeEach(() => { - mockExistingSessionKey(key); - }); + it("caches the decrypted value when one is stored in local storage", async () => { + const encrypted = makeEncString("encrypted"); + localStorage.internalStore["session_test"] = encrypted.encryptedString; + encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); + await sut.get("test"); + expect(sut["cache"]["test"]).toEqual("decrypted"); + }); + }); - describe("no session retrieved", () => { - let result: any; - let spy: jest.SpyInstance; - beforeEach(async () => { - spy = jest.spyOn(sut, "getLocalSession").mockResolvedValue(null); - localStorageService.get.mockResolvedValue(null); - result = await sut.get("test"); - }); + describe("getBypassCache", () => { + it("ignores cached values", async () => { + sut["cache"]["test"] = "cached"; + const encrypted = makeEncString("encrypted"); + localStorage.internalStore["session_test"] = encrypted.encryptedString; + encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); + const result = await sut.getBypassCache("test"); + expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encrypted, sessionKey); + expect(result).toEqual("decrypted"); + }); - it("should grab from session if not in cache", async () => { - expect(spy).toHaveBeenCalledWith(key); - }); + it("returns a decrypted value when one is stored in local storage", async () => { + const encrypted = makeEncString("encrypted"); + localStorage.internalStore["session_test"] = encrypted.encryptedString; + encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); + const result = await sut.getBypassCache("test"); + expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encrypted, sessionKey); + expect(result).toEqual("decrypted"); + }); - it("should return null if session is null", async () => { - expect(result).toBeNull(); - }); - }); + it("caches the decrypted value when one is stored in local storage", async () => { + const encrypted = makeEncString("encrypted"); + localStorage.internalStore["session_test"] = encrypted.encryptedString; + encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); + await sut.getBypassCache("test"); + expect(sut["cache"]["test"]).toEqual("decrypted"); + }); - describe("session retrieved from storage", () => { - beforeEach(() => { - jest.spyOn(sut, "getLocalSession").mockResolvedValue(session); - }); - - it("should return null if session does not have the key", async () => { - const result = await sut.get("DNE"); - expect(result).toBeNull(); - }); - - it("should return the value retrieved from session", async () => { - const result = await sut.get("test"); - expect(result).toEqual(session.test); - }); - - it("should set retrieved values in cache", async () => { - await sut.get("test"); - expect(cache["test"]).toBeTruthy(); - expect(cache["test"]).toEqual(session.test); - }); - - it("should use a deserializer if provided", async () => { - const deserializer = jest.fn().mockReturnValue(testObj); - const result = await sut.get("test", { deserializer: deserializer }); - expect(deserializer).toHaveBeenCalledWith(session.test); - expect(result).toEqual(testObj); - }); - }); + it("deserializes when a deserializer is provided", async () => { + const encrypted = makeEncString("encrypted"); + localStorage.internalStore["session_test"] = encrypted.encryptedString; + encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); + const deserializer = jest.fn().mockReturnValue("deserialized"); + const result = await sut.getBypassCache("test", { deserializer }); + expect(deserializer).toHaveBeenCalledWith("decrypted"); + expect(result).toEqual("deserialized"); }); }); describe("has", () => { - it("should be false if `get` returns null", async () => { - const spy = jest.spyOn(sut, "get"); - spy.mockResolvedValue(null); - expect(await sut.has("test")).toBe(false); + it("returns false when the key is not in cache", async () => { + const result = await sut.has("test"); + expect(result).toBe(false); + }); + + it("returns true when the key is in cache", async () => { + sut["cache"]["test"] = "cached"; + const result = await sut.has("test"); + expect(result).toBe(true); + }); + + it("returns true when the key is in local storage", async () => { + localStorage.internalStore["session_test"] = makeEncString("encrypted").encryptedString; + encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); + const result = await sut.has("test"); + expect(result).toBe(true); + }); + + it.each([null, undefined])("returns false when %s is cached", async (nullish) => { + sut["cache"]["test"] = nullish; + await expect(sut.has("test")).resolves.toBe(false); + }); + + it.each([null, undefined])( + "returns false when null is stored in local storage", + async (nullish) => { + localStorage.internalStore["session_test"] = nullish; + await expect(sut.has("test")).resolves.toBe(false); + expect(encryptService.decryptToUtf8).not.toHaveBeenCalled(); + }, + ); + }); + + describe("save", () => { + const encString = makeEncString("encrypted"); + beforeEach(() => { + encryptService.encrypt.mockResolvedValue(encString); + }); + + it("logs a warning when saving the same value twice and in a dev environment", async () => { + platformUtilsService.isDev.mockReturnValue(true); + sut["cache"]["test"] = "cached"; + await sut.save("test", "cached"); + expect(logService.warning).toHaveBeenCalled(); + }); + + it("does not log when saving the same value twice and not in a dev environment", async () => { + platformUtilsService.isDev.mockReturnValue(false); + sut["cache"]["test"] = "cached"; + await sut.save("test", "cached"); + expect(logService.warning).not.toHaveBeenCalled(); + }); + + it("removes the key when saving a null value", async () => { + const spy = jest.spyOn(sut, "remove"); + await sut.save("test", null); expect(spy).toHaveBeenCalledWith("test"); }); - it("should be true if `get` returns non-null", async () => { - const spy = jest.spyOn(sut, "get"); - spy.mockResolvedValue({}); - expect(await sut.has("test")).toBe(true); - expect(spy).toHaveBeenCalledWith("test"); + it("saves the value to cache", async () => { + await sut.save("test", "value"); + expect(sut["cache"]["test"]).toEqual("value"); + }); + + it("encrypts and saves the value to local storage", async () => { + await sut.save("test", "value"); + expect(encryptService.encrypt).toHaveBeenCalledWith(JSON.stringify("value"), sessionKey); + expect(localStorage.internalStore["session_test"]).toEqual(encString.encryptedString); + }); + + it("emits an update", async () => { + const spy = jest.spyOn(sut["updatesSubject"], "next"); + await sut.save("test", "value"); + expect(spy).toHaveBeenCalledWith({ key: "test", updateType: "save" }); }); }); describe("remove", () => { - describe("existing cache value is null", () => { - it("should not save null if the local cached value is already null", async () => { - cache["test"] = null; - await sut.remove("test"); - expect(sendUpdateSpy).not.toHaveBeenCalled(); - }); - - it("should not save null if the externally cached value is already null", async () => { - sendMessageWithResponseSpy.mockResolvedValue(null); - await sut.remove("test"); - expect(sendUpdateSpy).not.toHaveBeenCalled(); - }); - }); - - it("should save null", async () => { - cache["test"] = stringifiedTestObj; - + it("nulls the value in cache", async () => { + sut["cache"]["test"] = "cached"; await sut.remove("test"); - expect(sendUpdateSpy).toHaveBeenCalledWith({ key: "test", updateType: "remove" }); - }); - }); - - describe("save", () => { - describe("currently cached", () => { - it("does not save the value a local cached value exists which is an exact match", async () => { - cache["test"] = stringifiedTestObj; - await sut.save("test", testObj); - expect(sendUpdateSpy).not.toHaveBeenCalled(); - }); - - it("does not save the value if a local cached value exists, even if the keys not in the same order", async () => { - cache["test"] = JSON.stringify({ b: 2, a: 1 }); - await sut.save("test", testObj); - expect(sendUpdateSpy).not.toHaveBeenCalled(); - }); - - it("does not save the value a externally cached value exists which is an exact match", async () => { - sendMessageWithResponseSpy.mockResolvedValue(stringifiedTestObj); - await sut.save("test", testObj); - expect(sendUpdateSpy).not.toHaveBeenCalled(); - expect(cache["test"]).toBe(stringifiedTestObj); - }); - - it("saves the value if the currently cached string value evaluates to a falsy value", async () => { - cache["test"] = "null"; - await sut.save("test", testObj); - expect(sendUpdateSpy).toHaveBeenCalledWith({ key: "test", updateType: "save" }); - }); + expect(sut["cache"]["test"]).toBeNull(); }); - describe("caching", () => { - beforeEach(() => { - localStorageService.get.mockResolvedValue(null); - sessionStorageService.get.mockResolvedValue(null); - - localStorageService.save.mockResolvedValue(); - sessionStorageService.save.mockResolvedValue(); - - encryptService.encrypt.mockResolvedValue(mockEnc("{}")); - }); - - it("should remove key from cache if value is null", async () => { - cache["test"] = {}; - // const cacheSetSpy = jest.spyOn(cache, "set"); - expect(cache["test"]).toBe(true); - await sut.save("test", null); - // Don't remove from cache, just replace with null - expect(cache["test"]).toBe(null); - // expect(cacheSetSpy).toHaveBeenCalledWith("test", null); - }); - - it("should set cache if value is non-null", async () => { - expect(cache["test"]).toBe(false); - // const setSpy = jest.spyOn(cache, "set"); - await sut.save("test", testObj); - expect(cache["test"]).toBe(stringifiedTestObj); - // expect(setSpy).toHaveBeenCalledWith("test", stringifiedTestObj); - }); + it("removes the key from local storage", async () => { + localStorage.internalStore["session_test"] = makeEncString("encrypted").encryptedString; + await sut.remove("test"); + expect(localStorage.internalStore["session_test"]).toBeUndefined(); }); - describe("local storing", () => { - let setSpy: jest.SpyInstance; - - beforeEach(() => { - setSpy = jest.spyOn(sut, "setLocalSession").mockResolvedValue(); - }); - - it("should store a new session", async () => { - jest.spyOn(sut, "getLocalSession").mockResolvedValue(null); - await sut.save("test", testObj); - - expect(setSpy).toHaveBeenCalledWith({ test: testObj }, key); - }); - - it("should update an existing session", async () => { - const existingObj = { test: testObj }; - jest.spyOn(sut, "getLocalSession").mockResolvedValue(existingObj); - await sut.save("test2", testObj); - - expect(setSpy).toHaveBeenCalledWith({ test2: testObj, ...existingObj }, key); - }); - - it("should overwrite an existing item in session", async () => { - const existingObj = { test: {} }; - jest.spyOn(sut, "getLocalSession").mockResolvedValue(existingObj); - await sut.save("test", testObj); - - expect(setSpy).toHaveBeenCalledWith({ test: testObj }, key); - }); - }); - }); - - describe("getSessionKey", () => { - beforeEach(() => { - getSessionKeySpy.mockRestore(); - }); - - it("should return the stored symmetric crypto key", async () => { - sessionStorageService.get.mockResolvedValue({ ...key }); - const result = await sut.getSessionEncKey(); - - expect(result).toStrictEqual(key); - }); - - describe("new key creation", () => { - beforeEach(() => { - keyGenerationService.createKeyWithPurpose.mockResolvedValue({ - salt: "salt", - material: null, - derivedKey: key, - }); - jest.spyOn(sut, "setSessionEncKey").mockResolvedValue(); - }); - - it("should create a symmetric crypto key", async () => { - const result = await sut.getSessionEncKey(); - - expect(result).toStrictEqual(key); - expect(keyGenerationService.createKeyWithPurpose).toHaveBeenCalledTimes(1); - }); - - it("should store a symmetric crypto key if it makes one", async () => { - const spy = jest.spyOn(sut, "setSessionEncKey").mockResolvedValue(); - await sut.getSessionEncKey(); - - expect(spy).toHaveBeenCalledWith(key); - }); - }); - }); - - describe("getLocalSession", () => { - it("should return null if session is null", async () => { - const result = await sut.getLocalSession(key); - - expect(result).toBeNull(); - expect(localStorageService.get).toHaveBeenCalledWith("session_test"); - }); - - describe("non-null sessions", () => { - const session = { test: "test" }; - const encSession = new EncString(JSON.stringify(session)); - const decryptedSession = JSON.stringify(session); - - beforeEach(() => { - localStorageService.get.mockResolvedValue(encSession.encryptedString); - }); - - it("should decrypt returned sessions", async () => { - encryptService.decryptToUtf8 - .calledWith(expect.anything(), key) - .mockResolvedValue(decryptedSession); - await sut.getLocalSession(key); - expect(encryptService.decryptToUtf8).toHaveBeenNthCalledWith(1, encSession, key); - }); - - it("should parse session", async () => { - encryptService.decryptToUtf8 - .calledWith(expect.anything(), key) - .mockResolvedValue(decryptedSession); - const result = await sut.getLocalSession(key); - expect(result).toEqual(session); - }); - - it("should remove state if decryption fails", async () => { - encryptService.decryptToUtf8.mockResolvedValue(null); - const setSessionEncKeySpy = jest.spyOn(sut, "setSessionEncKey").mockResolvedValue(); - - const result = await sut.getLocalSession(key); - - expect(result).toBeNull(); - expect(setSessionEncKeySpy).toHaveBeenCalledWith(null); - expect(localStorageService.remove).toHaveBeenCalledWith("session_test"); - }); - }); - }); - - describe("setLocalSession", () => { - const testSession = { test: "a" }; - const testJSON = JSON.stringify(testSession); - - it("should encrypt a stringified session", async () => { - encryptService.encrypt.mockImplementation(mockEnc); - localStorageService.save.mockResolvedValue(); - await sut.setLocalSession(testSession, key); - - expect(encryptService.encrypt).toHaveBeenNthCalledWith(1, testJSON, key); - }); - - it("should remove local session if null", async () => { - encryptService.encrypt.mockResolvedValue(null); - await sut.setLocalSession(null, key); - - expect(localStorageService.remove).toHaveBeenCalledWith("session_test"); - }); - - it("should save encrypted string", async () => { - encryptService.encrypt.mockImplementation(mockEnc); - await sut.setLocalSession(testSession, key); - - expect(localStorageService.save).toHaveBeenCalledWith( - "session_test", - (await mockEnc(testJSON)).encryptedString, - ); - }); - }); - - describe("setSessionKey", () => { - it("should remove if null", async () => { - await sut.setSessionEncKey(null); - expect(sessionStorageService.remove).toHaveBeenCalledWith("localEncryptionKey_test"); - }); - - it("should save key when not null", async () => { - await sut.setSessionEncKey(key); - expect(sessionStorageService.save).toHaveBeenCalledWith("localEncryptionKey_test", key); + it("emits an update", async () => { + const spy = jest.spyOn(sut["updatesSubject"], "next"); + await sut.remove("test"); + expect(spy).toHaveBeenCalledWith({ key: "test", updateType: "remove" }); }); }); }); diff --git a/apps/browser/src/platform/services/local-backed-session-storage.service.ts b/apps/browser/src/platform/services/local-backed-session-storage.service.ts index 146eb11b2b..5432e8d918 100644 --- a/apps/browser/src/platform/services/local-backed-session-storage.service.ts +++ b/apps/browser/src/platform/services/local-backed-session-storage.service.ts @@ -2,7 +2,6 @@ import { Subject } from "rxjs"; import { Jsonify } from "type-fest"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; -import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { @@ -11,13 +10,12 @@ import { ObservableStorageService, StorageUpdate, } from "@bitwarden/common/platform/abstractions/storage.service"; +import { Lazy } from "@bitwarden/common/platform/misc/lazy"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { MemoryStorageOptions } from "@bitwarden/common/platform/models/domain/storage-options"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { BrowserApi } from "../browser/browser-api"; -import { devFlag } from "../decorators/dev-flag.decorator"; -import { devFlagEnabled } from "../flags"; import { MemoryStoragePortMessage } from "../storage/port-messages"; import { portName } from "../storage/port-name"; @@ -25,85 +23,64 @@ export class LocalBackedSessionStorageService extends AbstractMemoryStorageService implements ObservableStorageService { + private ports: Set = new Set([]); + private cache: Record = {}; private updatesSubject = new Subject(); - private commandName = `localBackedSessionStorage_${this.partitionName}`; - private encKey = `localEncryptionKey_${this.partitionName}`; - private sessionKey = `session_${this.partitionName}`; - private cachedSession: Record = {}; - private _ports: Set = new Set([]); - private knownNullishCacheKeys: Set = new Set([]); + readonly valuesRequireDeserialization = true; + updates$ = this.updatesSubject.asObservable(); constructor( - private logService: LogService, - private encryptService: EncryptService, - private keyGenerationService: KeyGenerationService, - private localStorage: AbstractStorageService, - private sessionStorage: AbstractStorageService, - private platformUtilsService: PlatformUtilsService, - private partitionName: string, + private readonly sessionKey: Lazy>, + private readonly localStorage: AbstractStorageService, + private readonly encryptService: EncryptService, + private readonly platformUtilsService: PlatformUtilsService, + private readonly logService: LogService, ) { super(); BrowserApi.addListener(chrome.runtime.onConnect, (port) => { - if (port.name !== `${portName(chrome.storage.session)}_${partitionName}`) { + if (port.name !== portName(chrome.storage.session)) { return; } - this._ports.add(port); + this.ports.add(port); const listenerCallback = this.onMessageFromForeground.bind(this); port.onDisconnect.addListener(() => { - this._ports.delete(port); + this.ports.delete(port); port.onMessage.removeListener(listenerCallback); }); port.onMessage.addListener(listenerCallback); // Initialize the new memory storage service with existing data this.sendMessageTo(port, { action: "initialization", - data: Array.from(Object.keys(this.cachedSession)), + data: Array.from(Object.keys(this.cache)), + }); + this.updates$.subscribe((update) => { + this.broadcastMessage({ + action: "subject_update", + data: update, + }); }); }); - this.updates$.subscribe((update) => { - this.broadcastMessage({ - action: "subject_update", - data: update, - }); - }); - } - - get valuesRequireDeserialization(): boolean { - return true; - } - - get updates$() { - return this.updatesSubject.asObservable(); } async get(key: string, options?: MemoryStorageOptions): Promise { - if (this.cachedSession[key] != null) { - return this.cachedSession[key] as T; - } - - if (this.knownNullishCacheKeys.has(key)) { - return null; + if (this.cache[key] !== undefined) { + return this.cache[key] as T; } return await this.getBypassCache(key, options); } async getBypassCache(key: string, options?: MemoryStorageOptions): Promise { - const session = await this.getLocalSession(await this.getSessionEncKey()); - if (session[key] == null) { - this.knownNullishCacheKeys.add(key); - return null; - } + let value = await this.getLocalSessionValue(await this.sessionKey.get(), key); - let value = session[key]; if (options?.deserializer != null) { value = options.deserializer(value as Jsonify); } - void this.save(key, value); + this.cache[key] = value; return value as T; } @@ -114,7 +91,7 @@ export class LocalBackedSessionStorageService async save(key: string, obj: T): Promise { // This is for observation purposes only. At some point, we don't want to write to local session storage if the value is the same. if (this.platformUtilsService.isDev()) { - const existingValue = this.cachedSession[key] as T; + const existingValue = this.cache[key] as T; if (this.compareValues(existingValue, obj)) { this.logService.warning(`Possible unnecessary write to local session storage. Key: ${key}`); this.logService.warning(obj as any); @@ -125,128 +102,42 @@ export class LocalBackedSessionStorageService return await this.remove(key); } - this.knownNullishCacheKeys.delete(key); - this.cachedSession[key] = obj; + this.cache[key] = obj; await this.updateLocalSessionValue(key, obj); this.updatesSubject.next({ key, updateType: "save" }); } async remove(key: string): Promise { - this.knownNullishCacheKeys.add(key); - delete this.cachedSession[key]; + this.cache[key] = null; await this.updateLocalSessionValue(key, null); this.updatesSubject.next({ key, updateType: "remove" }); } - private async updateLocalSessionValue(key: string, obj: T) { - const sessionEncKey = await this.getSessionEncKey(); - const localSession = (await this.getLocalSession(sessionEncKey)) ?? {}; - localSession[key] = obj; - void this.setLocalSession(localSession, sessionEncKey); - } - - async getLocalSession(encKey: SymmetricCryptoKey): Promise> { - if (Object.keys(this.cachedSession).length > 0) { - return this.cachedSession; - } - - this.cachedSession = {}; - const local = await this.localStorage.get(this.sessionKey); + private async getLocalSessionValue(encKey: SymmetricCryptoKey, key: string): Promise { + const local = await this.localStorage.get(this.sessionStorageKey(key)); if (local == null) { - return this.cachedSession; + return null; } - if (devFlagEnabled("storeSessionDecrypted")) { - return local as any as Record; + const valueJson = await this.encryptService.decryptToUtf8(new EncString(local), encKey); + if (valueJson == null) { + // error with decryption, value is lost, delete state and start over + await this.localStorage.remove(this.sessionStorageKey(key)); + return null; } - const sessionJson = await this.encryptService.decryptToUtf8(new EncString(local), encKey); - if (sessionJson == null) { - // Error with decryption -- session is lost, delete state and key and start over - await this.setSessionEncKey(null); - await this.localStorage.remove(this.sessionKey); - return this.cachedSession; - } - - this.cachedSession = JSON.parse(sessionJson); - return this.cachedSession; + return JSON.parse(valueJson); } - async setLocalSession(session: Record, key: SymmetricCryptoKey) { - if (devFlagEnabled("storeSessionDecrypted")) { - await this.setDecryptedLocalSession(session); - } else { - await this.setEncryptedLocalSession(session, key); - } - } - - @devFlag("storeSessionDecrypted") - async setDecryptedLocalSession(session: Record): Promise { - // Make sure we're storing the jsonified version of the session - const jsonSession = JSON.parse(JSON.stringify(session)); - if (session == null) { - await this.localStorage.remove(this.sessionKey); - } else { - await this.localStorage.save(this.sessionKey, jsonSession); - } - } - - async setEncryptedLocalSession(session: Record, key: SymmetricCryptoKey) { - const jsonSession = JSON.stringify(session); - const encSession = await this.encryptService.encrypt(jsonSession, key); - - if (encSession == null) { - return await this.localStorage.remove(this.sessionKey); - } - await this.localStorage.save(this.sessionKey, encSession.encryptedString); - } - - async getSessionEncKey(): Promise { - let storedKey = await this.sessionStorage.get(this.encKey); - if (storedKey == null || Object.keys(storedKey).length == 0) { - const generatedKey = await this.keyGenerationService.createKeyWithPurpose( - 128, - "ephemeral", - "bitwarden-ephemeral", - ); - storedKey = generatedKey.derivedKey; - await this.setSessionEncKey(storedKey); - return storedKey; - } else { - return SymmetricCryptoKey.fromJSON(storedKey); - } - } - - async setSessionEncKey(input: SymmetricCryptoKey): Promise { - if (input == null) { - await this.sessionStorage.remove(this.encKey); - } else { - await this.sessionStorage.save(this.encKey, input); - } - } - - private compareValues(value1: T, value2: T): boolean { - if (value1 == null && value2 == null) { - return true; + private async updateLocalSessionValue(key: string, value: unknown): Promise { + if (value == null) { + await this.localStorage.remove(this.sessionStorageKey(key)); + return; } - if (value1 && value2 == null) { - return false; - } - - if (value1 == null && value2) { - return false; - } - - if (typeof value1 !== "object" || typeof value2 !== "object") { - return value1 === value2; - } - - if (JSON.stringify(value1) === JSON.stringify(value2)) { - return true; - } - - return Object.entries(value1).sort().toString() === Object.entries(value2).sort().toString(); + const valueJson = JSON.stringify(value); + const encValue = await this.encryptService.encrypt(valueJson, await this.sessionKey.get()); + await this.localStorage.save(this.sessionStorageKey(key), encValue.encryptedString); } private async onMessageFromForeground( @@ -282,7 +173,7 @@ export class LocalBackedSessionStorageService } protected broadcastMessage(data: Omit) { - this._ports.forEach((port) => { + this.ports.forEach((port) => { this.sendMessageTo(port, data); }); } @@ -296,4 +187,32 @@ export class LocalBackedSessionStorageService originator: "background", }); } + + private sessionStorageKey(key: string) { + return `session_${key}`; + } + + private compareValues(value1: T, value2: T): boolean { + if (value1 == null && value2 == null) { + return true; + } + + if (value1 && value2 == null) { + return false; + } + + if (value1 == null && value2) { + return false; + } + + if (typeof value1 !== "object" || typeof value2 !== "object") { + return value1 === value2; + } + + if (JSON.stringify(value1) === JSON.stringify(value2)) { + return true; + } + + return Object.entries(value1).sort().toString() === Object.entries(value2).sort().toString(); + } } diff --git a/libs/common/spec/index.ts b/libs/common/spec/index.ts index 72bd28aca4..6e9af8400e 100644 --- a/libs/common/spec/index.ts +++ b/libs/common/spec/index.ts @@ -4,3 +4,4 @@ export * from "./matchers"; export * from "./fake-state-provider"; export * from "./fake-state"; export * from "./fake-account-service"; +export * from "./fake-storage.service"; diff --git a/libs/common/src/platform/misc/lazy.spec.ts b/libs/common/src/platform/misc/lazy.spec.ts new file mode 100644 index 0000000000..76ee085d3d --- /dev/null +++ b/libs/common/src/platform/misc/lazy.spec.ts @@ -0,0 +1,85 @@ +import { Lazy } from "./lazy"; + +describe("Lazy", () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + describe("async", () => { + let factory: jest.Mock>; + let lazy: Lazy>; + + beforeEach(() => { + factory = jest.fn(); + lazy = new Lazy(factory); + }); + + describe("get", () => { + it("should call the factory once", async () => { + await lazy.get(); + await lazy.get(); + + expect(factory).toHaveBeenCalledTimes(1); + }); + + it("should return the value from the factory", async () => { + factory.mockResolvedValue(42); + + const value = await lazy.get(); + + expect(value).toBe(42); + }); + }); + + describe("factory throws", () => { + it("should throw the error", async () => { + factory.mockRejectedValue(new Error("factory error")); + + await expect(lazy.get()).rejects.toThrow("factory error"); + }); + }); + + describe("factory returns undefined", () => { + it("should return undefined", async () => { + factory.mockResolvedValue(undefined); + + const value = await lazy.get(); + + expect(value).toBeUndefined(); + }); + }); + + describe("factory returns null", () => { + it("should return null", async () => { + factory.mockResolvedValue(null); + + const value = await lazy.get(); + + expect(value).toBeNull(); + }); + }); + }); + + describe("sync", () => { + const syncFactory = jest.fn(); + let lazy: Lazy; + + beforeEach(() => { + syncFactory.mockReturnValue(42); + lazy = new Lazy(syncFactory); + }); + + it("should return the value from the factory", () => { + const value = lazy.get(); + + expect(value).toBe(42); + }); + + it("should call the factory once", () => { + lazy.get(); + lazy.get(); + + expect(syncFactory).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/libs/common/src/platform/misc/lazy.ts b/libs/common/src/platform/misc/lazy.ts new file mode 100644 index 0000000000..fb85b93678 --- /dev/null +++ b/libs/common/src/platform/misc/lazy.ts @@ -0,0 +1,20 @@ +export class Lazy { + private _value: T | undefined = undefined; + private _isCreated = false; + + constructor(private readonly factory: () => T) {} + + /** + * Resolves the factory and returns the result. Guaranteed to resolve the value only once. + * + * @returns The value produced by your factory. + */ + get(): T { + if (!this._isCreated) { + this._value = this.factory(); + this._isCreated = true; + } + + return this._value as T; + } +}