Refactor store and fix bugs with it (#2348)

src/i18n/strings/en_EN.json

@@ -59,13 +59,11 @@
             "backend_no_encryption": "Your system has a supported keyring but encryption is not available.",
             "backend_no_encryption_detail": "Electron has detected that encryption is not available on your keyring %(backend)s. Please ensure that you have the keyring installed. If you do have the keyring installed, please reboot and try again. Optionally, you can allow %(brand)s to use a weaker form of encryption.",
             "backend_no_encryption_title": "No encryption support",
-            "unknown_backend_override": "Your system has an unsupported keyring meaning the database cannot be opened.",
-            "unknown_backend_override_details": "Please check the logs for more details.",
-            "unknown_backend_override_title": "Failed to load database",
             "unsupported_keyring": "Your system has an unsupported keyring meaning the database cannot be opened.",
-            "unsupported_keyring_cta": "Use weaker encryption",
             "unsupported_keyring_detail": "Electron's keyring detection did not find a supported backend. You can attempt to manually configure the backend by starting %(brand)s with a command-line argument, a one-time operation. See %(link)s.",
-            "unsupported_keyring_title": "System unsupported"
+            "unsupported_keyring_title": "System unsupported",
+            "unsupported_keyring_use_basic_text": "Use weaker encryption",
+            "unsupported_keyring_use_plaintext": "Use no encryption"
         }
     },
     "view_menu": {

src/store.ts

@@ -41,6 +41,10 @@ const LEGACY_KEYTAR_SERVICE = "riot.im";
  * + All other backends are linux-specific and are encrypted using the keychain
  */
 type SafeStorageBackend = ReturnType<SafeStorage["getSelectedStorageBackend"]> | "system" | "plaintext";
+/**
+ * The "unknown" backend is not a valid backend, so we exclude it from the type.
+ */
+type SaneSafeStorageBackend = Exclude<SafeStorageBackend, "unknown">;
 
 /**
  * Map of safeStorage backends to their command line arguments.
@@ -85,7 +89,7 @@ interface StoreData {
 /**
  * Fallback storage writer for secrets, mainly used for automated tests and systems without any safeStorage support.
  */
-class PlaintextStorageWriter {
+class StorageWriter {
     public constructor(protected readonly store: ElectronStore<StoreData>) {}
 
     public getKey(key: string): `safeStorage.${string}` {
@@ -108,7 +112,7 @@ class PlaintextStorageWriter {
 /**
  * Storage writer for secrets using safeStorage.
  */
-class SafeStorageWriter extends PlaintextStorageWriter {
+class SafeStorageWriter extends StorageWriter {
     public set(key: string, secret: string): void {
         this.store.set(this.getKey(key), safeStorage.encryptString(secret).toString("base64"));
     }
@@ -157,7 +161,10 @@ class Store extends ElectronStore<StoreData> {
         const store = new Store(mode ?? Mode.Encrypted);
         Store.internalInstance = store;
 
-        if (process.platform === "linux" && store.get("safeStorageBackendOverride")) {
+        if (
+            process.platform === "linux" &&
+            (store.get("safeStorageBackendOverride") || store.get("safeStorageBackendMigrate"))
+        ) {
             const backend = store.get("safeStorageBackend")!;
             if (backend in safeStorageBackendMap) {
                 // If the safeStorage backend which was used to write the data is one we can specify via the commandLine
@@ -174,7 +181,7 @@ class Store extends ElectronStore<StoreData> {
 
     // Provides "raw" access to the underlying secrets storage,
     // should be avoided in favour of the getSecret/setSecret/deleteSecret methods.
-    private secrets?: PlaintextStorageWriter | SafeStorageWriter;
+    private secrets?: StorageWriter;
 
     private constructor(private mode: Mode) {
         super({
@@ -228,6 +235,37 @@ class Store extends ElectronStore<StoreData> {
         await this.safeStorageReadyPromise;
     }
 
+    /**
+     * Normalise the backend to a sane value (exclude `unknown`), respect forcePlaintext mode,
+     * and ensure that if an encrypted backend is picked that encryption is available, falling back to plaintext if not.
+     * @param forcePlaintext - whether to force plaintext mode
+     * @private
+     */
+    private chooseBackend(forcePlaintext: boolean): SaneSafeStorageBackend {
+        if (forcePlaintext) {
+            return "plaintext";
+        }
+
+        if (process.platform === "linux") {
+            // The following enables plain text encryption if the backend used is basic_text.
+            // It has no significance for any other backend.
+            // We do this early so that in case we end up using the basic_text backend (either because that's the only one available
+            // or as a fallback when the configured backend lacks encryption support), encryption is already turned on.
+            safeStorage.setUsePlainTextEncryption(true);
+
+            // Linux safeStorage support is hellish, the support varies on the Desktop Environment used rather than the store itself.
+            // https://github.com/electron/electron/issues/39789 https://github.com/microsoft/vscode/issues/185212
+            const selectedBackend = safeStorage.getSelectedStorageBackend();
+
+            if (selectedBackend === "unknown" || !safeStorage.isEncryptionAvailable()) {
+                return "plaintext";
+            }
+            return selectedBackend;
+        }
+
+        return safeStorage.isEncryptionAvailable() ? "system" : "plaintext";
+    }
+
     /**
      * Prepare the safeStorage backend for use.
      * We don't eagerly import from keytar as that would bring in data for all Element profiles and not just the current one,
@@ -236,134 +274,110 @@ class Store extends ElectronStore<StoreData> {
     private async prepareSafeStorage(): Promise<void> {
         await app.whenReady();
 
-        let safeStorageBackend = this.get("safeStorageBackend");
+        // The backend the existing data is written with if any
-        if (process.platform === "linux") {
+        let existingSafeStorageBackend = this.get("safeStorageBackend");
-            // Linux safeStorage support is hellish, the support varies on the Desktop Environment used rather than the store itself.
+        // The backend and encryption status of the currently loaded backend
-            // https://github.com/electron/electron/issues/39789 https://github.com/microsoft/vscode/issues/185212
+        const backend = this.chooseBackend(this.mode === Mode.ForcePlaintext);
-            const selectedSafeStorageBackend = safeStorage.getSelectedStorageBackend();
 
-            // The following enables plain text encryption if the backend used is basic_text.
+        // Handle migrations
-            // It has no significance for any other backend.
+        if (existingSafeStorageBackend) {
-            // We do this early so that in case we end up using the basic_text backend (either because that's the only one available
+            if (existingSafeStorageBackend === "basic_text" && backend !== "plaintext" && backend !== "basic_text") {
-            // or as a fallback when the configured backend lacks encryption support), encryption is already turned on.
+                return this.prepareMigrateBasicTextToPlaintext();
-            safeStorage.setUsePlainTextEncryption(true);
-
-            const isEncryptionAvailable = safeStorage.isEncryptionAvailable();
-            console.info(
-                `safeStorage backend '${selectedSafeStorageBackend}' selected, '${safeStorageBackend}' in config, isEncryptionAvailable = ${isEncryptionAvailable}.`,
-            );
-
-            if (selectedSafeStorageBackend === "unknown") {
-                // This should never happen but good to be safe
-                await dialog.showMessageBox({
-                    title: _t("store|error|unknown_backend_override_title"),
-                    message: _t("store|error|unknown_backend_override"),
-                    detail: _t("store|error|unknown_backend_override_details"),
-                    type: "error",
-                });
-                throw new Error("safeStorage backend unknown");
             }
 
-            if (this.get("safeStorageBackendMigrate")) {
+            if (this.get("safeStorageBackendMigrate") && backend === "basic_text") {
-                return this.upgradeLinuxBackend2();
+                return this.migrateBasicTextToPlaintext();
             }
 
-            // Whether we were using basic_text as a fallback before
+            if (existingSafeStorageBackend === "plaintext" && backend !== "plaintext") {
-            const usingFallback = this.get("safeStorageBackendOverride") && safeStorageBackend === "basic_text";
+                this.migratePlaintextToEncrypted();
+                // Ensure we update existingSafeStorageBackend so we don't fall into the "backend changed" clause below
+                existingSafeStorageBackend = this.get("safeStorageBackend");
+            }
+        }
 
-            if (this.mode === Mode.Encrypted && !isEncryptionAvailable && !usingFallback) {
+        if (!existingSafeStorageBackend) {
-                // Sometimes we may have a working backend that for some reason does not support encryption at the moment.
+            // First launch of the app or first launch since the update
-                // This may be because electron reported an incorrect backend or because of some known issues with the keyring itself.
+            if (this.mode === Mode.Encrypted && (backend === "plaintext" || backend === "basic_text")) {
-                // In any case, when this happens, we give the user an option to use a weaker form of encryption.
+                // Ask the user for consent to use a degraded mode
-                const { response } = await dialog.showMessageBox({
+                await this.consultUserConsentDegradedMode(backend);
-                    title: _t("store|error|backend_no_encryption_title"),
+            }
-                    message: _t("store|error|backend_no_encryption"),
+            // Store the backend used for the safeStorage data so we can detect if it changes, and we know how the data is encoded
-                    detail: _t("store|error|backend_no_encryption_detail", {
+            this.recordSafeStorageBackend(backend);
-                        backend: safeStorageBackend,
+        } else if (existingSafeStorageBackend !== backend) {
-                        brand: global.vectorConfig.brand || "Element",
+            console.warn(`safeStorage backend changed from ${existingSafeStorageBackend} to ${backend}`);
-                    }),
+
-                    type: "error",
+            if (existingSafeStorageBackend in safeStorageBackendMap) {
-                    buttons: [_t("action|cancel"), _t("store|error|unsupported_keyring_cta")],
-                    defaultId: 0,
-                    cancelId: 0,
-                });
-                if (response === 0) {
-                    throw new Error(
-                        `Encryption support not available on backend ${safeStorageBackend} and user prohibits using weaker encryption.`,
-                    );
-                }
-                this.recordSafeStorageBackend("basic_text");
                 this.set("safeStorageBackendOverride", true);
-                relaunchApp();
+                return relaunchApp();
-            } else if (usingFallback) {
+            } else {
-                // On the next run, don't use the fallback.
+                await this.consultUserBackendChangedUnableToMigrate();
-                // This is so that we can check if the problems with the keyring fixed itself.
-                this.set("safeStorageBackendOverride", false);
-            } else if (!safeStorageBackend) {
-                if (selectedSafeStorageBackend === "basic_text" && this.mode === Mode.Encrypted) {
-                    const { response } = await dialog.showMessageBox({
-                        title: _t("store|error|unsupported_keyring_title"),
-                        message: _t("store|error|unsupported_keyring"),
-                        detail: _t("store|error|unsupported_keyring_detail", {
-                            brand: global.vectorConfig.brand || "Element",
-                            link: "https://www.electronjs.org/docs/latest/api/safe-storage#safestoragegetselectedstoragebackend-linux",
-                        }),
-                        type: "error",
-                        buttons: [_t("action|cancel"), _t("store|error|unsupported_keyring_cta")],
-                        defaultId: 0,
-                        cancelId: 0,
-                    });
-                    if (response === 0) {
-                        throw new Error("safeStorage backend basic_text and user rejected it");
-                    }
-                    this.mode = Mode.AllowPlaintext;
-                }
-
-                // Store the backend used for the safeStorage data so we can detect if it changes
-                this.recordSafeStorageBackend(selectedSafeStorageBackend);
-                safeStorageBackend = selectedSafeStorageBackend;
-            } else if (safeStorageBackend !== selectedSafeStorageBackend) {
-                console.warn(`safeStorage backend changed from ${safeStorageBackend} to ${selectedSafeStorageBackend}`);
-
-                if (safeStorageBackend === "basic_text") {
-                    return this.upgradeLinuxBackend1();
-                } else if (safeStorageBackend === "plaintext") {
-                    this.upgradeLinuxBackend3();
-                } else if (safeStorageBackend in safeStorageBackendMap) {
-                    this.set("safeStorageBackendOverride", true);
-                    relaunchApp();
-                    return;
-                } else {
-                    // Warn the user that the backend has changed and tell them that we cannot migrate
-                    const { response } = await dialog.showMessageBox({
-                        title: _t("store|error|backend_changed_title"),
-                        message: _t("store|error|backend_changed"),
-                        detail: _t("store|error|backend_changed_detail"),
-                        type: "question",
-                        buttons: [_t("common|no"), _t("common|yes")],
-                        defaultId: 0,
-                        cancelId: 0,
-                    });
-                    if (response === 0) {
-                        throw new Error("safeStorage backend changed and cannot migrate");
-                    }
-                    await clearDataAndRelaunch();
-                }
             }
-        } else if (!safeStorageBackend) {
-            safeStorageBackend = this.mode === Mode.Encrypted ? "system" : "plaintext";
-            this.recordSafeStorageBackend(safeStorageBackend);
         }
 
-        if (this.mode !== Mode.ForcePlaintext && safeStorage.isEncryptionAvailable()) {
+        console.info(`Using storage mode '${this.mode}' with backend '${backend}'`);
+        if (backend !== "plaintext") {
             this.secrets = new SafeStorageWriter(this);
-        } else if (this.mode !== Mode.Encrypted) {
-            this.secrets = new PlaintextStorageWriter(this);
         } else {
-            throw new Error(`safeStorage is not available`);
+            this.secrets = new StorageWriter(this);
         }
+    }
 
-        console.info(`Using storage mode '${this.mode}' with backend '${safeStorageBackend}'`);
+    private async consultUserBackendChangedUnableToMigrate(): Promise<void> {
+        const { response } = await dialog.showMessageBox({
+            title: _t("store|error|backend_changed_title"),
+            message: _t("store|error|backend_changed"),
+            detail: _t("store|error|backend_changed_detail"),
+            type: "question",
+            buttons: [_t("common|no"), _t("common|yes")],
+            defaultId: 0,
+            cancelId: 0,
+        });
+        if (response === 0) {
+            throw new Error("safeStorage backend changed and cannot migrate");
+        }
+        return clearDataAndRelaunch();
+    }
+
+    private async consultUserConsentDegradedMode(backend: "plaintext" | "basic_text"): Promise<void> {
+        if (backend === "plaintext") {
+            // Sometimes we may have a working backend that for some reason does not support encryption at the moment.
+            // This may be because electron reported an incorrect backend or because of some known issues with the keyring itself.
+            // Or the environment specified `--storage-mode=force-plaintext`.
+            // In any case, when this happens, we give the user an option to use a weaker form of encryption.
+            const { response } = await dialog.showMessageBox({
+                title: _t("store|error|backend_no_encryption_title"),
+                message: _t("store|error|backend_no_encryption"),
+                detail: _t("store|error|backend_no_encryption_detail", {
+                    backend: safeStorage.getSelectedStorageBackend(),
+                    brand: global.vectorConfig.brand || "Element",
+                }),
+                type: "error",
+                buttons: [_t("action|cancel"), _t("store|error|unsupported_keyring_use_plaintext")],
+                defaultId: 0,
+                cancelId: 0,
+            });
+            if (response === 0) {
+                throw new Error("isEncryptionAvailable=false and user rejected plaintext");
+            }
+        } else {
+            // Electron did not identify a compatible encrypted backend, ask user for consent to degraded mode
+            const { response } = await dialog.showMessageBox({
+                title: _t("store|error|unsupported_keyring_title"),
+                message: _t("store|error|unsupported_keyring"),
+                detail: _t("store|error|unsupported_keyring_detail", {
+                    brand: global.vectorConfig.brand || "Element",
+                    link: "https://www.electronjs.org/docs/latest/api/safe-storage#safestoragegetselectedstoragebackend-linux",
+                }),
+                type: "error",
+                buttons: [_t("action|cancel"), _t("store|error|unsupported_keyring_use_basic_text")],
+                defaultId: 0,
+                cancelId: 0,
+            });
+            if (response === 0) {
+                throw new Error("safeStorage backend basic_text and user rejected it");
+            }
+        }
     }
 
     private recordSafeStorageBackend(backend: SafeStorageBackend): void {
@@ -373,35 +387,37 @@ class Store extends ElectronStore<StoreData> {
     /**
      * Linux support for upgrading the backend from basic_text to one of the encrypted backends,
      * this is quite a tricky process as the backend is not known until the app is ready & cannot be changed once it is.
-     * First we restart the app in basic_text backend mode, then decrypt the data & restart back in default backend mode,
+     * 1. We restart the app in safeStorageBackendMigrate mode
-     * and re-encrypt the data.
+     * 2. Now that we are in the mode which our data is written in we decrypt the data, write it back in plaintext
+     *     & restart back in default backend mode,
+     * 3. Finally, we load the plaintext data & encrypt it.
      */
-    private upgradeLinuxBackend1(): void {
+    private prepareMigrateBasicTextToPlaintext(): void {
         console.info(`Starting safeStorage migration to ${safeStorage.getSelectedStorageBackend()}`);
         this.set("safeStorageBackendMigrate", true);
         relaunchApp();
     }
-    private upgradeLinuxBackend2(): void {
+    private migrateBasicTextToPlaintext(): void {
-        this.secrets = new PlaintextStorageWriter(this);
+        const secrets = new SafeStorageWriter(this);
         console.info("Performing safeStorage migration");
         const data = this.get("safeStorage");
         if (data) {
             for (const key in data) {
-                this.set(this.secrets.getKey(key), this.secrets!.get(key));
+                this.set(secrets.getKey(key), secrets.get(key));
             }
             this.recordSafeStorageBackend("plaintext");
         }
-        this.set("safeStorageBackendMigrate", false);
+        this.delete("safeStorageBackendMigrate");
         relaunchApp();
     }
-    private upgradeLinuxBackend3(): void {
+    private migratePlaintextToEncrypted(): void {
-        this.secrets = new PlaintextStorageWriter(this);
+        const secrets = new SafeStorageWriter(this);
         const selectedSafeStorageBackend = safeStorage.getSelectedStorageBackend();
         console.info(`Finishing safeStorage migration to ${selectedSafeStorageBackend}`);
         const data = this.get("safeStorage");
         if (data) {
             for (const key in data) {
-                this.secrets.set(key, data[key]);
+                secrets.set(key, data[key]);
             }
         }
         this.recordSafeStorageBackend(selectedSafeStorageBackend);