Extract functions for service worker usage, and add initial MSC3916 playwright test (when supported) (#12414)

* Send user credentials to service worker for MSC3916 authentication

* appease linter

* Add initial test

The test fails, seemingly because the service worker isn't being installed or because the network mock can't reach that far.

* Remove unsafe access token code

* Split out base IDB operations to avoid importing `document` in serviceworkers

* Use safe crypto access for service workers

* Fix tests/unsafe access

* Remove backwards compatibility layer & appease linter

* Add docs

* Fix tests

* Appease the linter

* Iterate tests

* Factor out pickle key handling for service workers

* Enable everything we can about service workers

* Appease the linter

* Add docs

* Rename win32 image to linux in hopes of it just working

* Use actual image

* Apply suggestions from code review

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

* Improve documentation

* Document `??` not working

* Try to appease the tests

* Add some notes

---------

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

playwright/e2e/timeline/timeline.spec.ts

@@ -70,6 +70,22 @@ const sendEvent = async (client: Client, roomId: string, html = false): Promise<
     return client.sendEvent(roomId, null, "m.room.message" as EventType, content);
 };
 
+const sendImage = async (
+    client: Client,
+    roomId: string,
+    pngBytes: Buffer,
+    additionalContent?: any,
+): Promise<ISendEventResponse> => {
+    const upload = await client.uploadContent(pngBytes, { name: "image.png", type: "image/png" });
+    return client.sendEvent(roomId, null, "m.room.message" as EventType, {
+        ...(additionalContent ?? {}),
+
+        msgtype: "m.image" as MsgType,
+        body: "image.png",
+        url: upload.content_uri,
+    });
+};
+
 test.describe("Timeline", () => {
     test.use({
         displayName: OLD_NAME,
@@ -1136,5 +1152,91 @@ test.describe("Timeline", () => {
                 screenshotOptions,
             );
         });
+
+        async function testImageRendering(page: Page, app: ElementAppPage, room: { roomId: string }) {
+            await app.viewRoomById(room.roomId);
+
+            // Reinstall the service workers to clear their implicit caches (global-level stuff)
+            await page.evaluate(async () => {
+                const registrations = await window.navigator.serviceWorker.getRegistrations();
+                registrations.forEach((r) => r.update());
+            });
+
+            await sendImage(app.client, room.roomId, NEW_AVATAR);
+            await expect(page.locator(".mx_MImageBody").first()).toBeVisible();
+
+            // Exclude timestamp and read marker from snapshot
+            const screenshotOptions = {
+                mask: [page.locator(".mx_MessageTimestamp")],
+                css: `
+                    .mx_TopUnreadMessagesBar, .mx_MessagePanel_myReadMarker {
+                        display: none !important;
+                    }
+                `,
+            };
+
+            await expect(page.locator(".mx_ScrollPanel")).toMatchScreenshot(
+                "image-in-timeline-default-layout.png",
+                screenshotOptions,
+            );
+        }
+
+        test("should render images in the timeline", async ({ page, app, room, context }) => {
+            await testImageRendering(page, app, room);
+        });
+
+        // XXX: This test doesn't actually work because the service worker relies on IndexedDB, which Playwright forces
+        // to be a localstorage implementation, which service workers cannot access.
+        // See https://github.com/microsoft/playwright/issues/11164
+        // See https://github.com/microsoft/playwright/issues/15684#issuecomment-2070862042
+        //
+        // In practice, this means this test will *always* succeed because it ends up relying on fallback behaviour tested
+        // above (unless of course the above tests are also broken).
+        test.describe("MSC3916 - Authenticated Media", () => {
+            test("should render authenticated images in the timeline", async ({ page, app, room, context }) => {
+                // Note: we have to use `context` instead of `page` for routing, otherwise we'll miss Service Worker events.
+                // See https://playwright.dev/docs/service-workers-experimental#network-events-and-routing
+
+                // Install our mocks and preventative measures
+                await context.route("**/_matrix/client/versions", async (route) => {
+                    // Force enable MSC3916, which may require the service worker's internal cache to be cleared later.
+                    const json = await (await route.fetch()).json();
+                    if (!json["unstable_features"]) json["unstable_features"] = {};
+                    json["unstable_features"]["org.matrix.msc3916"] = true;
+                    await route.fulfill({ json });
+                });
+                await context.route("**/_matrix/media/*/download/**", async (route) => {
+                    // should not be called. We don't use `abort` so that it's clearer in the logs what happened.
+                    await route.fulfill({
+                        status: 500,
+                        json: { errcode: "M_UNKNOWN", error: "Unexpected route called." },
+                    });
+                });
+                await context.route("**/_matrix/media/*/thumbnail/**", async (route) => {
+                    // should not be called. We don't use `abort` so that it's clearer in the logs what happened.
+                    await route.fulfill({
+                        status: 500,
+                        json: { errcode: "M_UNKNOWN", error: "Unexpected route called." },
+                    });
+                });
+                await context.route("**/_matrix/client/unstable/org.matrix.msc3916/download/**", async (route) => {
+                    expect(route.request().headers()["Authorization"]).toBeDefined();
+                    // we can't use route.continue() because no configured homeserver supports MSC3916 yet
+                    await route.fulfill({
+                        body: NEW_AVATAR,
+                    });
+                });
+                await context.route("**/_matrix/client/unstable/org.matrix.msc3916/thumbnail/**", async (route) => {
+                    expect(route.request().headers()["Authorization"]).toBeDefined();
+                    // we can't use route.continue() because no configured homeserver supports MSC3916 yet
+                    await route.fulfill({
+                        body: NEW_AVATAR,
+                    });
+                });
+
+                // We check the same screenshot because there should be no user-visible impact to using authentication.
+                await testImageRendering(page, app, room);
+            });
+        });
     });
 });

playwright/element-web-test.ts

@@ -33,6 +33,10 @@ import { Bot, CreateBotOpts } from "./pages/bot";
 import { ProxyInstance, SlidingSyncProxy } from "./plugins/sliding-sync-proxy";
 import { Webserver } from "./plugins/webserver";
 
+// Enable experimental service worker support
+// See https://playwright.dev/docs/service-workers-experimental#how-to-enable
+process.env["PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS"] = "1";
+
 const CONFIG_JSON: Partial<IConfigOptions> = {
     // This is deliberately quite a minimal config.json, so that we can test that the default settings
     // actually work.

src/BasePlatform.ts

@@ -34,10 +34,11 @@ import { CheckUpdatesPayload } from "./dispatcher/payloads/CheckUpdatesPayload";
 import { Action } from "./dispatcher/actions";
 import { hideToast as hideUpdateToast } from "./toasts/UpdateToast";
 import { MatrixClientPeg } from "./MatrixClientPeg";
-import { idbLoad, idbSave, idbDelete } from "./utils/StorageManager";
+import { idbLoad, idbSave, idbDelete } from "./utils/StorageAccess";
 import { ViewRoomPayload } from "./dispatcher/payloads/ViewRoomPayload";
 import { IConfigOptions } from "./IConfigOptions";
 import SdkConfig from "./SdkConfig";
+import { buildAndEncodePickleKey, getPickleAdditionalData } from "./utils/tokens/pickling";
 
 export const SSO_HOMESERVER_URL_KEY = "mx_sso_hs_url";
 export const SSO_ID_SERVER_URL_KEY = "mx_sso_is_url";
@@ -352,55 +353,21 @@ export default abstract class BasePlatform {
 
     /**
      * Get a previously stored pickle key.  The pickle key is used for
-     * encrypting libolm objects.
+     * encrypting libolm objects and react-sdk-crypto data.
      * @param {string} userId the user ID for the user that the pickle key is for.
-     * @param {string} userId the device ID that the pickle key is for.
+     * @param {string} deviceId the device ID that the pickle key is for.
      * @returns {string|null} the previously stored pickle key, or null if no
      *     pickle key has been stored.
      */
     public async getPickleKey(userId: string, deviceId: string): Promise<string | null> {
-        if (!window.crypto || !window.crypto.subtle) {
+        let data: { encrypted?: BufferSource; iv?: BufferSource; cryptoKey?: CryptoKey } | undefined;
-            return null;
-        }
-        let data;
         try {
             data = await idbLoad("pickleKey", [userId, deviceId]);
         } catch (e) {
             logger.error("idbLoad for pickleKey failed", e);
         }
-        if (!data) {
-            return null;
-        }
-        if (!data.encrypted || !data.iv || !data.cryptoKey) {
-            logger.error("Badly formatted pickle key");
-            return null;
-        }
 
-        const additionalData = this.getPickleAdditionalData(userId, deviceId);
+        return (await buildAndEncodePickleKey(data, userId, deviceId)) ?? null;
-
-        try {
-            const key = await crypto.subtle.decrypt(
-                { name: "AES-GCM", iv: data.iv, additionalData },
-                data.cryptoKey,
-                data.encrypted,
-            );
-            return encodeUnpaddedBase64(key);
-        } catch (e) {
-            logger.error("Error decrypting pickle key");
-            return null;
-        }
-    }
-
-    private getPickleAdditionalData(userId: string, deviceId: string): Uint8Array {
-        const additionalData = new Uint8Array(userId.length + deviceId.length + 1);
-        for (let i = 0; i < userId.length; i++) {
-            additionalData[i] = userId.charCodeAt(i);
-        }
-        additionalData[userId.length] = 124; // "|"
-        for (let i = 0; i < deviceId.length; i++) {
-            additionalData[userId.length + 1 + i] = deviceId.charCodeAt(i);
-        }
-        return additionalData;
     }
 
     /**
@@ -424,7 +391,7 @@ export default abstract class BasePlatform {
         const iv = new Uint8Array(32);
         crypto.getRandomValues(iv);
 
-        const additionalData = this.getPickleAdditionalData(userId, deviceId);
+        const additionalData = getPickleAdditionalData(userId, deviceId);
         const encrypted = await crypto.subtle.encrypt({ name: "AES-GCM", iv, additionalData }, cryptoKey, randomArray);
 
         try {

src/Lifecycle.ts

@@ -37,6 +37,7 @@ import ActiveWidgetStore from "./stores/ActiveWidgetStore";
 import PlatformPeg from "./PlatformPeg";
 import { sendLoginRequest } from "./Login";
 import * as StorageManager from "./utils/StorageManager";
+import * as StorageAccess from "./utils/StorageAccess";
 import SettingsStore from "./settings/SettingsStore";
 import { SettingLevel } from "./settings/SettingLevel";
 import ToastStore from "./stores/ToastStore";
@@ -493,7 +494,7 @@ export interface IStoredSession {
 async function getStoredToken(storageKey: string): Promise<string | undefined> {
     let token: string | undefined;
     try {
-        token = await StorageManager.idbLoad("account", storageKey);
+        token = await StorageAccess.idbLoad("account", storageKey);
     } catch (e) {
         logger.error(`StorageManager.idbLoad failed for account:${storageKey}`, e);
     }
@@ -502,7 +503,7 @@ async function getStoredToken(storageKey: string): Promise<string | undefined> {
         if (token) {
             try {
                 // try to migrate access token to IndexedDB if we can
-                await StorageManager.idbSave("account", storageKey, token);
+                await StorageAccess.idbSave("account", storageKey, token);
                 localStorage.removeItem(storageKey);
             } catch (e) {
                 logger.error(`migration of token ${storageKey} to IndexedDB failed`, e);
@@ -1064,7 +1065,7 @@ async function clearStorage(opts?: { deleteEverything?: boolean }): Promise<void
         AbstractLocalStorageSettingsHandler.clear();
 
         try {
-            await StorageManager.idbDelete("account", ACCESS_TOKEN_STORAGE_KEY);
+            await StorageAccess.idbDelete("account", ACCESS_TOKEN_STORAGE_KEY);
         } catch (e) {
             logger.error("idbDelete failed for account:mx_access_token", e);
         }

src/utils/StorageAccess.ts

@@ -0,0 +1,132 @@
+/*
+Copyright 2019-2021, 2024 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+/**
+ * Retrieves the IndexedDB factory object.
+ *
+ * @returns {IDBFactory | undefined} The IndexedDB factory object if available, or undefined if it is not supported.
+ */
+export function getIDBFactory(): IDBFactory | undefined {
+    // IndexedDB loading is lazy for easier testing.
+
+    // just *accessing* _indexedDB throws an exception in firefox with
+    // indexeddb disabled.
+    try {
+        // `self` is preferred for service workers, which access this file's functions.
+        // We check `self` first because `window` returns something which doesn't work for service workers.
+        // Note: `self?.indexedDB ?? window.indexedDB` breaks in service workers for unknown reasons.
+        return self?.indexedDB ? self.indexedDB : window.indexedDB;
+    } catch (e) {}
+}
+
+let idb: IDBDatabase | null = null;
+
+async function idbInit(): Promise<void> {
+    if (!getIDBFactory()) {
+        throw new Error("IndexedDB not available");
+    }
+    idb = await new Promise((resolve, reject) => {
+        const request = getIDBFactory()!.open("matrix-react-sdk", 1);
+        request.onerror = reject;
+        request.onsuccess = (): void => {
+            resolve(request.result);
+        };
+        request.onupgradeneeded = (): void => {
+            const db = request.result;
+            db.createObjectStore("pickleKey");
+            db.createObjectStore("account");
+        };
+    });
+}
+
+/**
+ * Loads an item from an IndexedDB table within the underlying `matrix-react-sdk` database.
+ *
+ * If IndexedDB access is not supported in the environment, an error is thrown.
+ *
+ * @param {string} table The name of the object store in IndexedDB.
+ * @param {string | string[]} key The key where the data is stored.
+ * @returns {Promise<any>} A promise that resolves with the retrieved item from the table.
+ */
+export async function idbLoad(table: string, key: string | string[]): Promise<any> {
+    if (!idb) {
+        await idbInit();
+    }
+    return new Promise((resolve, reject) => {
+        const txn = idb!.transaction([table], "readonly");
+        txn.onerror = reject;
+
+        const objectStore = txn.objectStore(table);
+        const request = objectStore.get(key);
+        request.onerror = reject;
+        request.onsuccess = (event): void => {
+            resolve(request.result);
+        };
+    });
+}
+
+/**
+ * Saves data to an IndexedDB table within the underlying `matrix-react-sdk` database.
+ *
+ * If IndexedDB access is not supported in the environment, an error is thrown.
+ *
+ * @param {string} table The name of the object store in the IndexedDB.
+ * @param {string|string[]} key The key to use for storing the data.
+ * @param {*} data The data to be saved.
+ * @returns {Promise<void>} A promise that resolves when the data is saved successfully.
+ */
+export async function idbSave(table: string, key: string | string[], data: any): Promise<void> {
+    if (!idb) {
+        await idbInit();
+    }
+    return new Promise((resolve, reject) => {
+        const txn = idb!.transaction([table], "readwrite");
+        txn.onerror = reject;
+
+        const objectStore = txn.objectStore(table);
+        const request = objectStore.put(data, key);
+        request.onerror = reject;
+        request.onsuccess = (event): void => {
+            resolve();
+        };
+    });
+}
+
+/**
+ * Deletes a record from an IndexedDB table within the underlying `matrix-react-sdk` database.
+ *
+ * If IndexedDB access is not supported in the environment, an error is thrown.
+ *
+ * @param {string} table The name of the object store where the record is stored.
+ * @param {string|string[]} key The key of the record to be deleted.
+ * @returns {Promise<void>} A Promise that resolves when the record(s) have been successfully deleted.
+ */
+export async function idbDelete(table: string, key: string | string[]): Promise<void> {
+    if (!idb) {
+        await idbInit();
+    }
+    return new Promise((resolve, reject) => {
+        const txn = idb!.transaction([table], "readwrite");
+        txn.onerror = reject;
+
+        const objectStore = txn.objectStore(table);
+        const request = objectStore.delete(key);
+        request.onerror = reject;
+        request.onsuccess = (): void => {
+            resolve();
+        };
+    });
+}

src/utils/StorageManager.ts

@@ -19,18 +19,10 @@ import { logger } from "matrix-js-sdk/src/logger";
 
 import SettingsStore from "../settings/SettingsStore";
 import { Features } from "../settings/Settings";
+import { getIDBFactory } from "./StorageAccess";
 
 const localStorage = window.localStorage;
 
-// make this lazy in order to make testing easier
-function getIndexedDb(): IDBFactory | undefined {
-    // just *accessing* _indexedDB throws an exception in firefox with
-    // indexeddb disabled.
-    try {
-        return window.indexedDB;
-    } catch (e) {}
-}
-
 // The JS SDK will add a prefix of "matrix-js-sdk:" to the sync store name.
 const SYNC_STORE_NAME = "riot-web-sync";
 const LEGACY_CRYPTO_STORE_NAME = "matrix-js-sdk:crypto";
@@ -68,7 +60,7 @@ export async function checkConsistency(): Promise<{
 }> {
     log("Checking storage consistency");
     log(`Local storage supported? ${!!localStorage}`);
-    log(`IndexedDB supported? ${!!getIndexedDb()}`);
+    log(`IndexedDB supported? ${!!getIDBFactory()}`);
 
     let dataInLocalStorage = false;
     let dataInCryptoStore = false;
@@ -86,7 +78,7 @@ export async function checkConsistency(): Promise<{
         error("Local storage cannot be used on this browser");
     }
 
-    if (getIndexedDb() && localStorage) {
+    if (getIDBFactory() && localStorage) {
         const results = await checkSyncStore();
         if (!results.healthy) {
             healthy = false;
@@ -96,7 +88,7 @@ export async function checkConsistency(): Promise<{
         error("Sync store cannot be used on this browser");
     }
 
-    if (getIndexedDb()) {
+    if (getIDBFactory()) {
         const results = await checkCryptoStore();
         dataInCryptoStore = results.exists;
         if (!results.healthy) {
@@ -138,7 +130,7 @@ interface StoreCheck {
 async function checkSyncStore(): Promise<StoreCheck> {
     let exists = false;
     try {
-        exists = await IndexedDBStore.exists(getIndexedDb()!, SYNC_STORE_NAME);
+        exists = await IndexedDBStore.exists(getIDBFactory()!, SYNC_STORE_NAME);
         log(`Sync store using IndexedDB contains data? ${exists}`);
         return { exists, healthy: true };
     } catch (e) {
@@ -152,7 +144,7 @@ async function checkCryptoStore(): Promise<StoreCheck> {
     if (await SettingsStore.getValue(Features.RustCrypto)) {
         // check first if there is a rust crypto store
         try {
-            const rustDbExists = await IndexedDBCryptoStore.exists(getIndexedDb()!, RUST_CRYPTO_STORE_NAME);
+            const rustDbExists = await IndexedDBCryptoStore.exists(getIDBFactory()!, RUST_CRYPTO_STORE_NAME);
             log(`Rust Crypto store using IndexedDB contains data? ${rustDbExists}`);
 
             if (rustDbExists) {
@@ -162,7 +154,7 @@ async function checkCryptoStore(): Promise<StoreCheck> {
                 // No rust store, so let's check if there is a legacy store not yet migrated.
                 try {
                     const legacyIdbExists = await IndexedDBCryptoStore.existsAndIsNotMigrated(
-                        getIndexedDb()!,
+                        getIDBFactory()!,
                         LEGACY_CRYPTO_STORE_NAME,
                     );
                     log(`Legacy Crypto store using IndexedDB contains non migrated data? ${legacyIdbExists}`);
@@ -183,7 +175,7 @@ async function checkCryptoStore(): Promise<StoreCheck> {
         let exists = false;
         // legacy checks
         try {
-            exists = await IndexedDBCryptoStore.exists(getIndexedDb()!, LEGACY_CRYPTO_STORE_NAME);
+            exists = await IndexedDBCryptoStore.exists(getIDBFactory()!, LEGACY_CRYPTO_STORE_NAME);
             log(`Crypto store using IndexedDB contains data? ${exists}`);
             return { exists, healthy: true };
         } catch (e) {
@@ -214,77 +206,3 @@ async function checkCryptoStore(): Promise<StoreCheck> {
 export function setCryptoInitialised(cryptoInited: boolean): void {
     localStorage.setItem("mx_crypto_initialised", String(cryptoInited));
 }
-
-/* Simple wrapper functions around IndexedDB.
- */
-
-let idb: IDBDatabase | null = null;
-
-async function idbInit(): Promise<void> {
-    if (!getIndexedDb()) {
-        throw new Error("IndexedDB not available");
-    }
-    idb = await new Promise((resolve, reject) => {
-        const request = getIndexedDb()!.open("matrix-react-sdk", 1);
-        request.onerror = reject;
-        request.onsuccess = (): void => {
-            resolve(request.result);
-        };
-        request.onupgradeneeded = (): void => {
-            const db = request.result;
-            db.createObjectStore("pickleKey");
-            db.createObjectStore("account");
-        };
-    });
-}
-
-export async function idbLoad(table: string, key: string | string[]): Promise<any> {
-    if (!idb) {
-        await idbInit();
-    }
-    return new Promise((resolve, reject) => {
-        const txn = idb!.transaction([table], "readonly");
-        txn.onerror = reject;
-
-        const objectStore = txn.objectStore(table);
-        const request = objectStore.get(key);
-        request.onerror = reject;
-        request.onsuccess = (event): void => {
-            resolve(request.result);
-        };
-    });
-}
-
-export async function idbSave(table: string, key: string | string[], data: any): Promise<void> {
-    if (!idb) {
-        await idbInit();
-    }
-    return new Promise((resolve, reject) => {
-        const txn = idb!.transaction([table], "readwrite");
-        txn.onerror = reject;
-
-        const objectStore = txn.objectStore(table);
-        const request = objectStore.put(data, key);
-        request.onerror = reject;
-        request.onsuccess = (event): void => {
-            resolve();
-        };
-    });
-}
-
-export async function idbDelete(table: string, key: string | string[]): Promise<void> {
-    if (!idb) {
-        await idbInit();
-    }
-    return new Promise((resolve, reject) => {
-        const txn = idb!.transaction([table], "readwrite");
-        txn.onerror = reject;
-
-        const objectStore = txn.objectStore(table);
-        const request = objectStore.delete(key);
-        request.onerror = reject;
-        request.onsuccess = (): void => {
-            resolve();
-        };
-    });
-}

src/utils/tokens/pickling.ts

@@ -0,0 +1,88 @@
+/*
+Copyright 2016 Aviral Dasgupta
+Copyright 2016 OpenMarket Ltd
+Copyright 2018 New Vector Ltd
+Copyright 2020, 2024 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { encodeUnpaddedBase64 } from "matrix-js-sdk/src/matrix";
+import { logger } from "matrix-js-sdk/src/logger";
+
+/**
+ * Calculates the `additionalData` for the AES-GCM key used by the pickling processes. This
+ * additional data is *not* encrypted, but *is* authenticated. The additional data is constructed
+ * from the user ID and device ID provided.
+ *
+ * The later-constructed pickle key is used to decrypt values, such as access tokens, from IndexedDB.
+ *
+ * See https://developer.mozilla.org/en-US/docs/Web/API/AesGcmParams for more information on
+ * `additionalData`.
+ *
+ * @param {string} userId The user ID who owns the pickle key.
+ * @param {string} deviceId The device ID which owns the pickle key.
+ * @return {Uint8Array} The additional data as a Uint8Array.
+ */
+export function getPickleAdditionalData(userId: string, deviceId: string): Uint8Array {
+    const additionalData = new Uint8Array(userId.length + deviceId.length + 1);
+    for (let i = 0; i < userId.length; i++) {
+        additionalData[i] = userId.charCodeAt(i);
+    }
+    additionalData[userId.length] = 124; // "|"
+    for (let i = 0; i < deviceId.length; i++) {
+        additionalData[userId.length + 1 + i] = deviceId.charCodeAt(i);
+    }
+    return additionalData;
+}
+
+/**
+ * Decrypts the provided data into a pickle key and base64-encodes it ready for use elsewhere.
+ *
+ * If `data` is undefined in part or in full, returns undefined.
+ *
+ *  If crypto functions are not available, returns undefined regardless of input.
+ *
+ * @param data An object containing the encrypted pickle key data: encrypted payload, initialization vector (IV), and crypto key. Typically loaded from indexedDB.
+ * @param userId The user ID the pickle key belongs to.
+ * @param deviceId The device ID the pickle key belongs to.
+ * @returns A promise that resolves to the encoded pickle key, or undefined if the key cannot be built and encoded.
+ */
+export async function buildAndEncodePickleKey(
+    data: { encrypted?: BufferSource; iv?: BufferSource; cryptoKey?: CryptoKey } | undefined,
+    userId: string,
+    deviceId: string,
+): Promise<string | undefined> {
+    if (!crypto?.subtle) {
+        return undefined;
+    }
+    if (!data || !data.encrypted || !data.iv || !data.cryptoKey) {
+        return undefined;
+    }
+
+    try {
+        const additionalData = getPickleAdditionalData(userId, deviceId);
+        const pickleKeyBuf = await crypto.subtle.decrypt(
+            { name: "AES-GCM", iv: data.iv, additionalData },
+            data.cryptoKey,
+            data.encrypted,
+        );
+        if (pickleKeyBuf) {
+            return encodeUnpaddedBase64(pickleKeyBuf);
+        }
+    } catch (e) {
+        logger.error("Error decrypting pickle key");
+    }
+
+    return undefined;
+}

src/utils/tokens/tokens.ts

@@ -17,7 +17,7 @@ limitations under the License.
 import { decryptAES, encryptAES, IEncryptedPayload } from "matrix-js-sdk/src/crypto/aes";
 import { logger } from "matrix-js-sdk/src/logger";
 
-import * as StorageManager from "../StorageManager";
+import * as StorageAccess from "../StorageAccess";
 
 /**
  * Utility functions related to the storage and retrieval of access tokens
@@ -50,10 +50,10 @@ async function pickleKeyToAesKey(pickleKey: string): Promise<Uint8Array> {
     for (let i = 0; i < pickleKey.length; i++) {
         pickleKeyBuffer[i] = pickleKey.charCodeAt(i);
     }
-    const hkdfKey = await window.crypto.subtle.importKey("raw", pickleKeyBuffer, "HKDF", false, ["deriveBits"]);
+    const hkdfKey = await crypto.subtle.importKey("raw", pickleKeyBuffer, "HKDF", false, ["deriveBits"]);
     pickleKeyBuffer.fill(0);
     return new Uint8Array(
-        await window.crypto.subtle.deriveBits(
+        await crypto.subtle.deriveBits(
             {
                 name: "HKDF",
                 hash: "SHA-256",
@@ -142,7 +142,7 @@ export async function persistTokenInStorage(
             // Save either the encrypted access token, or the plain access
             // token if there is no token or we were unable to encrypt (e.g. if the browser doesn't
             // have WebCrypto).
-            await StorageManager.idbSave("account", storageKey, encryptedToken || token);
+            await StorageAccess.idbSave("account", storageKey, encryptedToken || token);
         } catch (e) {
             // if we couldn't save to indexedDB, fall back to localStorage.  We
             // store the access token unencrypted since localStorage only saves
@@ -155,7 +155,7 @@ export async function persistTokenInStorage(
         }
     } else {
         try {
-            await StorageManager.idbSave("account", storageKey, token);
+            await StorageAccess.idbSave("account", storageKey, token);
         } catch (e) {
             if (!!token) {
                 localStorage.setItem(storageKey, token);

test/Lifecycle-test.ts

@@ -26,7 +26,7 @@ import StorageEvictedDialog from "../src/components/views/dialogs/StorageEvicted
 import { logout, restoreFromLocalStorage, setLoggedIn } from "../src/Lifecycle";
 import { MatrixClientPeg } from "../src/MatrixClientPeg";
 import Modal from "../src/Modal";
-import * as StorageManager from "../src/utils/StorageManager";
+import * as StorageAccess from "../src/utils/StorageAccess";
 import { flushPromises, getMockClientWithEventEmitter, mockClientMethodsUser, mockPlatformPeg } from "./test-utils";
 import { OidcClientStore } from "../src/stores/oidc/OidcClientStore";
 import { makeDelegatedAuthConfig } from "./test-utils/oidc";
@@ -128,13 +128,13 @@ describe("Lifecycle", () => {
     };
 
     const initIdbMock = (mockStore: Record<string, Record<string, unknown>> = {}): void => {
-        jest.spyOn(StorageManager, "idbLoad")
+        jest.spyOn(StorageAccess, "idbLoad")
             .mockClear()
             .mockImplementation(
                 // @ts-ignore mock type
                 async (table: string, key: string) => mockStore[table]?.[key] ?? null,
             );
-        jest.spyOn(StorageManager, "idbSave")
+        jest.spyOn(StorageAccess, "idbSave")
             .mockClear()
             .mockImplementation(
                 // @ts-ignore mock type
@@ -144,7 +144,7 @@ describe("Lifecycle", () => {
                     mockStore[tableKey] = table;
                 },
             );
-        jest.spyOn(StorageManager, "idbDelete").mockClear().mockResolvedValue(undefined);
+        jest.spyOn(StorageAccess, "idbDelete").mockClear().mockResolvedValue(undefined);
     };
 
     const homeserverUrl = "https://server.org";
@@ -258,16 +258,16 @@ describe("Lifecycle", () => {
                     expect(localStorage.setItem).toHaveBeenCalledWith("mx_is_guest", "false");
                     expect(localStorage.setItem).toHaveBeenCalledWith("mx_device_id", deviceId);
 
-                    expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken);
+                    expect(StorageAccess.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken);
                     // dont put accessToken in localstorage when we have idb
                     expect(localStorage.setItem).not.toHaveBeenCalledWith("mx_access_token", accessToken);
                 });
 
                 it("should persist access token when idb is not available", async () => {
-                    jest.spyOn(StorageManager, "idbSave").mockRejectedValue("oups");
+                    jest.spyOn(StorageAccess, "idbSave").mockRejectedValue("oups");
                     expect(await restoreFromLocalStorage()).toEqual(true);
 
-                    expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken);
+                    expect(StorageAccess.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken);
                     // put accessToken in localstorage as fallback
                     expect(localStorage.setItem).toHaveBeenCalledWith("mx_access_token", accessToken);
                 });
@@ -316,11 +316,7 @@ describe("Lifecycle", () => {
 
                         // refresh token from storage is re-persisted
                         expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_refresh_token", "true");
-                        expect(StorageManager.idbSave).toHaveBeenCalledWith(
+                        expect(StorageAccess.idbSave).toHaveBeenCalledWith("account", "mx_refresh_token", refreshToken);
-                            "account",
-                            "mx_refresh_token",
-                            refreshToken,
-                        );
                     });
 
                     it("should create new matrix client with credentials", async () => {
@@ -359,7 +355,7 @@ describe("Lifecycle", () => {
                     expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_access_token", "true");
 
                     // token encrypted and persisted
-                    expect(StorageManager.idbSave).toHaveBeenCalledWith(
+                    expect(StorageAccess.idbSave).toHaveBeenCalledWith(
                         "account",
                         "mx_access_token",
                         encryptedTokenShapedObject,
@@ -368,7 +364,7 @@ describe("Lifecycle", () => {
 
                 it("should persist access token when idb is not available", async () => {
                     // dont fail for pickle key persist
-                    jest.spyOn(StorageManager, "idbSave").mockImplementation(
+                    jest.spyOn(StorageAccess, "idbSave").mockImplementation(
                         async (table: string, key: string | string[]) => {
                             if (table === "account" && key === "mx_access_token") {
                                 throw new Error("oups");
@@ -378,7 +374,7 @@ describe("Lifecycle", () => {
 
                     expect(await restoreFromLocalStorage()).toEqual(true);
 
-                    expect(StorageManager.idbSave).toHaveBeenCalledWith(
+                    expect(StorageAccess.idbSave).toHaveBeenCalledWith(
                         "account",
                         "mx_access_token",
                         encryptedTokenShapedObject,
@@ -422,7 +418,7 @@ describe("Lifecycle", () => {
 
                         // refresh token from storage is re-persisted
                         expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_refresh_token", "true");
-                        expect(StorageManager.idbSave).toHaveBeenCalledWith(
+                        expect(StorageAccess.idbSave).toHaveBeenCalledWith(
                             "account",
                             "mx_refresh_token",
                             encryptedTokenShapedObject,
@@ -502,7 +498,7 @@ describe("Lifecycle", () => {
                 expect(localStorage.setItem).toHaveBeenCalledWith("mx_is_guest", "false");
                 expect(localStorage.setItem).toHaveBeenCalledWith("mx_device_id", deviceId);
 
-                expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken);
+                expect(StorageAccess.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken);
                 // dont put accessToken in localstorage when we have idb
                 expect(localStorage.setItem).not.toHaveBeenCalledWith("mx_access_token", accessToken);
             });
@@ -513,14 +509,14 @@ describe("Lifecycle", () => {
                     refreshToken,
                 });
 
-                expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken);
+                expect(StorageAccess.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken);
-                expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_refresh_token", refreshToken);
+                expect(StorageAccess.idbSave).toHaveBeenCalledWith("account", "mx_refresh_token", refreshToken);
                 // dont put accessToken in localstorage when we have idb
                 expect(localStorage.setItem).not.toHaveBeenCalledWith("mx_access_token", accessToken);
             });
 
             it("should remove any access token from storage when there is none in credentials and idb save fails", async () => {
-                jest.spyOn(StorageManager, "idbSave").mockRejectedValue("oups");
+                jest.spyOn(StorageAccess, "idbSave").mockRejectedValue("oups");
                 await setLoggedIn({
                     ...credentials,
                     // @ts-ignore
@@ -534,7 +530,7 @@ describe("Lifecycle", () => {
             it("should clear stores", async () => {
                 await setLoggedIn(credentials);
 
-                expect(StorageManager.idbDelete).toHaveBeenCalledWith("account", "mx_access_token");
+                expect(StorageAccess.idbDelete).toHaveBeenCalledWith("account", "mx_access_token");
                 expect(sessionStorage.clear).toHaveBeenCalled();
                 expect(mockClient.clearStores).toHaveBeenCalled();
             });
@@ -566,7 +562,7 @@ describe("Lifecycle", () => {
                 });
 
                 // unpickled access token saved
-                expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken);
+                expect(StorageAccess.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken);
                 expect(mockPlatform.createPickleKey).not.toHaveBeenCalled();
             });
 
@@ -585,16 +581,12 @@ describe("Lifecycle", () => {
                 expect(localStorage.setItem).toHaveBeenCalledWith("mx_device_id", deviceId);
 
                 expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_pickle_key", "true");
-                expect(StorageManager.idbSave).toHaveBeenCalledWith(
+                expect(StorageAccess.idbSave).toHaveBeenCalledWith(
                     "account",
                     "mx_access_token",
                     encryptedTokenShapedObject,
                 );
-                expect(StorageManager.idbSave).toHaveBeenCalledWith(
+                expect(StorageAccess.idbSave).toHaveBeenCalledWith("pickleKey", [userId, deviceId], expect.any(Object));
-                    "pickleKey",
-                    [userId, deviceId],
-                    expect.any(Object),
-                );
                 // dont put accessToken in localstorage when we have idb
                 expect(localStorage.setItem).not.toHaveBeenCalledWith("mx_access_token", accessToken);
             });
@@ -604,12 +596,12 @@ describe("Lifecycle", () => {
                 await setLoggedIn(credentials);
 
                 // persist the unencrypted token
-                expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken);
+                expect(StorageAccess.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken);
             });
 
             it("should persist token in localStorage when idb fails to save token", async () => {
                 // dont fail for pickle key persist
-                jest.spyOn(StorageManager, "idbSave").mockImplementation(
+                jest.spyOn(StorageAccess, "idbSave").mockImplementation(
                     async (table: string, key: string | string[]) => {
                         if (table === "account" && key === "mx_access_token") {
                             throw new Error("oups");
@@ -624,7 +616,7 @@ describe("Lifecycle", () => {
 
             it("should remove any access token from storage when there is none in credentials and idb save fails", async () => {
                 // dont fail for pickle key persist
-                jest.spyOn(StorageManager, "idbSave").mockImplementation(
+                jest.spyOn(StorageAccess, "idbSave").mockImplementation(
                     async (table: string, key: string | string[]) => {
                         if (table === "account" && key === "mx_access_token") {
                             throw new Error("oups");

test/components/structures/MatrixChat-test.tsx

@@ -29,7 +29,7 @@ import { defer, sleep } from "matrix-js-sdk/src/utils";
 import { UserVerificationStatus } from "matrix-js-sdk/src/crypto-api";
 
 import MatrixChat from "../../../src/components/structures/MatrixChat";
-import * as StorageManager from "../../../src/utils/StorageManager";
+import * as StorageAccess from "../../../src/utils/StorageAccess";
 import defaultDispatcher from "../../../src/dispatcher/dispatcher";
 import { Action } from "../../../src/dispatcher/actions";
 import { UserTab } from "../../../src/components/views/dialogs/UserTab";
@@ -220,8 +220,8 @@ describe("<MatrixChat />", () => {
             headers: { "content-type": "application/json" },
         });
 
-        jest.spyOn(StorageManager, "idbLoad").mockReset();
+        jest.spyOn(StorageAccess, "idbLoad").mockReset();
-        jest.spyOn(StorageManager, "idbSave").mockResolvedValue(undefined);
+        jest.spyOn(StorageAccess, "idbSave").mockResolvedValue(undefined);
         jest.spyOn(defaultDispatcher, "dispatch").mockClear();
         jest.spyOn(defaultDispatcher, "fire").mockClear();
 
@@ -459,7 +459,7 @@ describe("<MatrixChat />", () => {
 
         describe("when login succeeds", () => {
             beforeEach(() => {
-                jest.spyOn(StorageManager, "idbLoad").mockImplementation(
+                jest.spyOn(StorageAccess, "idbLoad").mockImplementation(
                     async (_table: string, key: string | string[]) => (key === "mx_access_token" ? accessToken : null),
                 );
                 loginClient.getProfileInfo.mockResolvedValue({
@@ -553,7 +553,7 @@ describe("<MatrixChat />", () => {
 
         beforeEach(async () => {
             await populateStorageForSession();
-            jest.spyOn(StorageManager, "idbLoad").mockImplementation(async (table, key) => {
+            jest.spyOn(StorageAccess, "idbLoad").mockImplementation(async (table, key) => {
                 const safeKey = Array.isArray(key) ? key[0] : key;
                 return mockidb[table]?.[safeKey];
             });
@@ -868,7 +868,7 @@ describe("<MatrixChat />", () => {
 
             mockClient.loginFlows.mockResolvedValue({ flows: [{ type: "m.login.password" }] });
 
-            jest.spyOn(StorageManager, "idbLoad").mockImplementation(async (table, key) => {
+            jest.spyOn(StorageAccess, "idbLoad").mockImplementation(async (table, key) => {
                 const safeKey = Array.isArray(key) ? key[0] : key;
                 return mockidb[table]?.[safeKey];
             });
@@ -1164,7 +1164,7 @@ describe("<MatrixChat />", () => {
 
         describe("when login succeeds", () => {
             beforeEach(() => {
-                jest.spyOn(StorageManager, "idbLoad").mockImplementation(
+                jest.spyOn(StorageAccess, "idbLoad").mockImplementation(
                     async (_table: string, key: string | string[]) => {
                         if (key === "mx_access_token") {
                             return accessToken as any;

test/utils/StorageAccess-test.ts

@@ -0,0 +1,55 @@
+/*
+Copyright 2024 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import "core-js/stable/structured-clone"; // for idb access
+import "fake-indexeddb/auto";
+
+import { idbDelete, idbLoad, idbSave } from "../../src/utils/StorageAccess";
+
+const NONEXISTENT_TABLE = "this_is_not_a_table_we_use_ever_and_so_we_can_use_it_in_tests";
+const KNOWN_TABLES = ["account", "pickleKey"];
+
+describe("StorageAccess", () => {
+    it.each(KNOWN_TABLES)("should save, load, and delete from known table '%s'", async (tableName: string) => {
+        const key = ["a", "b"];
+        const data = { hello: "world" };
+
+        // Should start undefined
+        let loaded = await idbLoad(tableName, key);
+        expect(loaded).toBeUndefined();
+
+        // ... then define a value
+        await idbSave(tableName, key, data);
+
+        // ... then check that value
+        loaded = await idbLoad(tableName, key);
+        expect(loaded).toEqual(data);
+
+        // ... then set it back to undefined
+        await idbDelete(tableName, key);
+
+        // ... which we then check again
+        loaded = await idbLoad(tableName, key);
+        expect(loaded).toBeUndefined();
+    });
+
+    it("should fail to save, load, and delete from a non-existent table", async () => {
+        // Regardless of validity on the key/data, or write order, these should all fail.
+        await expect(() => idbSave(NONEXISTENT_TABLE, "whatever", "value")).rejects.toThrow();
+        await expect(() => idbLoad(NONEXISTENT_TABLE, "whatever")).rejects.toThrow();
+        await expect(() => idbDelete(NONEXISTENT_TABLE, "whatever")).rejects.toThrow();
+    });
+});