Remove unused sessionStore (#2455)

* Remove unused sessionStorage layer

* Move pending event abstraction into its temporary home

* Add test coverage

* Tweak

* Fix tests mocks

* Add coverage

* Add coverage

spec/TestClient.ts

@@ -23,7 +23,6 @@ import MockHttpBackend from 'matrix-mock-request';
 
 import { LocalStorageCryptoStore } from '../src/crypto/store/localStorage-crypto-store';
 import { logger } from '../src/logger';
-import { WebStorageSessionStore } from "../src/store/session/webstorage";
 import { syncPromise } from "./test-utils/test-utils";
 import { createClient } from "../src/matrix";
 import { ICreateClientOpts, IDownloadKeyResult, MatrixClient, PendingEventOrdering } from "../src/client";
@@ -53,7 +52,6 @@ export class TestClient {
         if (sessionStoreBackend === undefined) {
             sessionStoreBackend = new MockStorageApi();
         }
-        const sessionStore = new WebStorageSessionStore(sessionStoreBackend);
 
         this.httpBackend = new MockHttpBackend();
 
@@ -62,7 +60,6 @@ export class TestClient {
             userId: userId,
             accessToken: accessToken,
             deviceId: deviceId,
-            sessionStore: sessionStore,
             request: this.httpBackend.requestFn as IHttpOpts["request"],
             ...options,
         };

spec/unit/crypto.spec.js

@@ -3,7 +3,6 @@ import '../olm-loader';
 import { EventEmitter } from "events";
 
 import { Crypto } from "../../src/crypto";
-import { WebStorageSessionStore } from "../../src/store/session/webstorage";
 import { MemoryCryptoStore } from "../../src/crypto/store/memory-crypto-store";
 import { MockStorageApi } from "../MockStorageApi";
 import { TestClient } from "../TestClient";
@@ -14,6 +13,7 @@ import { sleep } from "../../src/utils";
 import { CRYPTO_ENABLED } from "../../src/client";
 import { DeviceInfo } from "../../src/crypto/deviceinfo";
 import { logger } from '../../src/logger';
+import { MemoryStore } from "../../src";
 
 const Olm = global.Olm;
 
@@ -153,7 +153,7 @@ describe("Crypto", function() {
 
         beforeEach(async function() {
             const mockStorage = new MockStorageApi();
-            const sessionStore = new WebStorageSessionStore(mockStorage);
+            const clientStore = new MemoryStore({ localStorage: mockStorage });
             const cryptoStore = new MemoryCryptoStore(mockStorage);
 
             cryptoStore.storeEndToEndDeviceData({
@@ -180,10 +180,9 @@ describe("Crypto", function() {
 
             crypto = new Crypto(
                 mockBaseApis,
-                sessionStore,
                 "@alice:home.server",
                 "FLIBBLE",
-                sessionStore,
+                clientStore,
                 cryptoStore,
                 mockRoomList,
             );

spec/unit/crypto/backup.spec.js

@@ -21,7 +21,6 @@ import * as olmlib from "../../../src/crypto/olmlib";
 import { MatrixClient } from "../../../src/client";
 import { MatrixEvent } from "../../../src/models/event";
 import * as algorithms from "../../../src/crypto/algorithms";
-import { WebStorageSessionStore } from "../../../src/store/session/webstorage";
 import { MemoryCryptoStore } from "../../../src/crypto/store/memory-crypto-store";
 import { MockStorageApi } from "../../MockStorageApi";
 import * as testUtils from "../../test-utils/test-utils";
@@ -118,7 +117,7 @@ function saveCrossSigningKeys(k) {
     Object.assign(keys, k);
 }
 
-function makeTestClient(sessionStore, cryptoStore) {
+function makeTestClient(cryptoStore) {
     const scheduler = [
         "getQueueForEvent", "queueEvent", "removeEventFromQueue",
         "setProcessFunction",
@@ -141,7 +140,6 @@ function makeTestClient(sessionStore, cryptoStore) {
         scheduler: scheduler,
         userId: "@alice:bar",
         deviceId: "device",
-        sessionStore: sessionStore,
         cryptoStore: cryptoStore,
         cryptoCallbacks: { getCrossSigningKey, saveCrossSigningKeys },
     });
@@ -161,7 +159,6 @@ describe("MegolmBackup", function() {
     let mockOlmLib;
     let mockCrypto;
     let mockStorage;
-    let sessionStore;
     let cryptoStore;
     let megolmDecryption;
     beforeEach(async function() {
@@ -174,7 +171,6 @@ describe("MegolmBackup", function() {
         mockCrypto.backupInfo = CURVE25519_BACKUP_INFO;
 
         mockStorage = new MockStorageApi();
-        sessionStore = new WebStorageSessionStore(mockStorage);
         cryptoStore = new MemoryCryptoStore(mockStorage);
 
         olmDevice = new OlmDevice(cryptoStore);
@@ -261,7 +257,7 @@ describe("MegolmBackup", function() {
             const ibGroupSession = new Olm.InboundGroupSession();
             ibGroupSession.create(groupSession.session_key());
 
-            const client = makeTestClient(sessionStore, cryptoStore);
+            const client = makeTestClient(cryptoStore);
 
             megolmDecryption = new MegolmDecryption({
                 userId: '@user:id',
@@ -340,7 +336,7 @@ describe("MegolmBackup", function() {
             const ibGroupSession = new Olm.InboundGroupSession();
             ibGroupSession.create(groupSession.session_key());
 
-            const client = makeTestClient(sessionStore, cryptoStore);
+            const client = makeTestClient(cryptoStore);
 
             megolmDecryption = new MegolmDecryption({
                 userId: '@user:id',
@@ -423,7 +419,7 @@ describe("MegolmBackup", function() {
             const ibGroupSession = new Olm.InboundGroupSession();
             ibGroupSession.create(groupSession.session_key());
 
-            const client = makeTestClient(sessionStore, cryptoStore);
+            const client = makeTestClient(cryptoStore);
 
             megolmDecryption = new MegolmDecryption({
                 userId: '@user:id',
@@ -520,7 +516,6 @@ describe("MegolmBackup", function() {
                 scheduler: scheduler,
                 userId: "@alice:bar",
                 deviceId: "device",
-                sessionStore: sessionStore,
                 cryptoStore: cryptoStore,
             });
 
@@ -606,7 +601,7 @@ describe("MegolmBackup", function() {
         let client;
 
         beforeEach(function() {
-            client = makeTestClient(sessionStore, cryptoStore);
+            client = makeTestClient(cryptoStore);
 
             megolmDecryption = new MegolmDecryption({
                 userId: '@user:id',

spec/unit/room.spec.ts

@@ -1555,6 +1555,8 @@ describe("Room", function() {
                         return Promise.resolve();
                     },
                     getSyncToken: () => "sync_token",
+                    getPendingEvents: jest.fn().mockResolvedValue([]),
+                    setPendingEvents: jest.fn().mockResolvedValue(undefined),
                 },
             };
         }

spec/unit/stores/indexeddb.spec.ts

@@ -17,13 +17,17 @@ limitations under the License.
 import 'fake-indexeddb/auto';
 import 'jest-localstorage-mock';
 
-import { IndexedDBStore, IStateEventWithRoomId } from "../../../src";
+import { IndexedDBStore, IStateEventWithRoomId, MemoryStore } from "../../../src";
 import { emitPromise } from "../../test-utils/test-utils";
 import { LocalIndexedDBStoreBackend } from "../../../src/store/indexeddb-local-backend";
 
 describe("IndexedDBStore", () => {
+    afterEach(() => {
+        jest.clearAllMocks();
+    });
+
+    const roomId = "!room:id";
     it("should degrade to MemoryStore on IDB errors", async () => {
-        const roomId = "!room:id";
         const store = new IndexedDBStore({
             indexedDB: indexedDB,
             dbName: "database",
@@ -69,4 +73,42 @@ describe("IndexedDBStore", () => {
         ]);
         expect(await store.getOutOfBandMembers(roomId)).toHaveLength(2);
     });
+
+    it("should use MemoryStore methods for pending events if no localStorage", async () => {
+        jest.spyOn(MemoryStore.prototype, "setPendingEvents");
+        jest.spyOn(MemoryStore.prototype, "getPendingEvents");
+
+        const store = new IndexedDBStore({
+            indexedDB: indexedDB,
+            dbName: "database",
+            localStorage: undefined,
+        });
+
+        const events = [{ type: "test" }];
+        await store.setPendingEvents(roomId, events);
+        expect(MemoryStore.prototype.setPendingEvents).toHaveBeenCalledWith(roomId, events);
+        await expect(store.getPendingEvents(roomId)).resolves.toEqual(events);
+        expect(MemoryStore.prototype.getPendingEvents).toHaveBeenCalledWith(roomId);
+    });
+
+    it("should persist pending events to localStorage if available", async () => {
+        jest.spyOn(MemoryStore.prototype, "setPendingEvents");
+        jest.spyOn(MemoryStore.prototype, "getPendingEvents");
+
+        const store = new IndexedDBStore({
+            indexedDB: indexedDB,
+            dbName: "database",
+            localStorage,
+        });
+
+        await expect(store.getPendingEvents(roomId)).resolves.toEqual([]);
+        const events = [{ type: "test" }];
+        await store.setPendingEvents(roomId, events);
+        expect(MemoryStore.prototype.setPendingEvents).not.toHaveBeenCalled();
+        await expect(store.getPendingEvents(roomId)).resolves.toEqual(events);
+        expect(MemoryStore.prototype.getPendingEvents).not.toHaveBeenCalled();
+        expect(localStorage.getItem("mx_pending_events_" + roomId)).toBe(JSON.stringify(events));
+        await store.setPendingEvents(roomId, []);
+        expect(localStorage.getItem("mx_pending_events_" + roomId)).toBeNull();
+    });
 });

src/client.ts

@@ -168,7 +168,6 @@ import {
 import { IAbortablePromise, IdServerUnbindResult, IImageInfo, Preset, Visibility } from "./@types/partials";
 import { EventMapper, eventMapperFor, MapperOpts } from "./event-mapper";
 import { randomString } from "./randomstring";
-import { WebStorageSessionStore } from "./store/session/webstorage";
 import { BackupManager, IKeyBackup, IKeyBackupCheck, IPreparedKeyBackupVersion, TrustInfo } from "./crypto/backup";
 import { DEFAULT_TREE_POWER_LEVELS_TEMPLATE, MSC3089TreeSpace } from "./models/MSC3089TreeSpace";
 import { ISignatures } from "./@types/signed";
@@ -195,7 +194,6 @@ import { Thread, THREAD_RELATION_TYPE } from "./models/thread";
 import { MBeaconInfoEventContent, M_BEACON_INFO } from "./@types/beacon";
 
 export type Store = IStore;
-export type SessionStore = WebStorageSessionStore;
 
 export type Callback<T = any> = (err: Error | any | null, data?: T) => void;
 export type ResetTimelineCallback = (roomId: string) => boolean;
@@ -315,14 +313,6 @@ export interface ICreateClientOpts {
      */
     pickleKey?: string;
 
-    /**
-     * A store to be used for end-to-end crypto session data. Most data has been
-     * migrated out of here to `cryptoStore` instead. If not specified,
-     * end-to-end crypto will be disabled. The `createClient` helper
-     * _will not_ create this store at the moment.
-     */
-    sessionStore?: SessionStore;
-
     verificationMethods?: Array<VerificationMethod>;
 
     /**
@@ -897,7 +887,6 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
     public timelineSupport = false;
     public urlPreviewCache: { [key: string]: Promise<IPreviewUrlResponse> } = {};
     public identityServer: IIdentityServerProvider;
-    public sessionStore: SessionStore; // XXX: Intended private, used in code.
     public http: MatrixHttpApi; // XXX: Intended private, used in code.
     public crypto: Crypto; // XXX: Intended private, used in code.
     public cryptoCallbacks: ICryptoCallbacks; // XXX: Intended private, used in code.
@@ -1029,7 +1018,6 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         this.timelineSupport = Boolean(opts.timelineSupport);
 
         this.cryptoStore = opts.cryptoStore;
-        this.sessionStore = opts.sessionStore;
         this.verificationMethods = opts.verificationMethods;
         this.cryptoCallbacks = opts.cryptoCallbacks || {};
 
@@ -1654,10 +1642,6 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
             return;
         }
 
-        if (!this.sessionStore) {
-            // this is temporary, the sessionstore is supposed to be going away
-            throw new Error(`Cannot enable encryption: no sessionStore provided`);
-        }
         if (!this.cryptoStore) {
             // the cryptostore is provided by sdk.createClient, so this shouldn't happen
             throw new Error(`Cannot enable encryption: no cryptoStore provided`);
@@ -1686,8 +1670,8 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
 
         const crypto = new Crypto(
             this,
-            this.sessionStore,
-            userId, this.deviceId,
+            userId,
+            this.deviceId,
             this.store,
             this.cryptoStore,
             this.roomList,

src/crypto/index.ts

@@ -75,7 +75,6 @@ import {
     ISignedKey,
     IUploadKeySignaturesResponse,
     MatrixClient,
-    SessionStore,
 } from "../client";
 import type { IRoomEncryption, RoomList } from "./RoomList";
 import { IKeyBackupInfo } from "./keybackup";
@@ -323,9 +322,6 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
      *
      * @param {MatrixClient} baseApis base matrix api interface
      *
-     * @param {module:store/session/webstorage~WebStorageSessionStore} sessionStore
-     *    Store to be used for end-to-end crypto session data
-     *
      * @param {string} userId The user ID for the local user
      *
      * @param {string} deviceId The identifier for this device.
@@ -343,7 +339,6 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
      */
     constructor(
         public readonly baseApis: MatrixClient,
-        public readonly sessionStore: SessionStore,
         public readonly userId: string,
         private readonly deviceId: string,
         private readonly clientStore: IStore,
@@ -1725,13 +1720,6 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
         logger.info(`Finished device verification upgrade for ${userId}`);
     }
 
-    public async setTrustedBackupPubKey(trustedPubKey: string): Promise<void> {
-        // This should be redundant post cross-signing is a thing, so just
-        // plonk it in localStorage for now.
-        this.sessionStore.setLocalTrustedBackupPubKey(trustedPubKey);
-        await this.backupManager.checkKeyBackup();
-    }
-
     /**
      */
     public enableLazyLoading(): void {

src/crypto/store/indexeddb-crypto-store.ts

@@ -157,7 +157,7 @@ export class IndexedDBCryptoStore implements CryptoStore {
             }
         }).then(backend => {
             this.backend = backend;
-            return backend as CryptoStore;
+            return backend;
         });
 
         return this.backendPromise;

src/matrix.ts

@@ -41,7 +41,6 @@ export * from "./interactive-auth";
 export * from "./service-types";
 export * from "./store/memory";
 export * from "./store/indexeddb";
-export * from "./store/session/webstorage";
 export * from "./crypto/store/memory-crypto-store";
 export * from "./crypto/store/indexeddb-crypto-store";
 export * from "./content-repo";