From 328d30146378e981b0cc5c793664dcd830ed0728 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Tue, 13 Feb 2024 16:49:43 +0100 Subject: [PATCH 01/34] Changed call sites from customisations/security to ModuleRunner.extensions --- src/Lifecycle.ts | 5 +- src/Login.ts | 5 +- src/MatrixClientPeg.ts | 13 +++- src/SecurityManager.ts | 15 ++-- .../security/CreateSecretStorageDialog.tsx | 7 +- src/components/structures/MatrixChat.tsx | 10 ++- src/modules/ModuleRunner.ts | 71 +++++++++++++++++++ src/toasts/SetupEncryptionToast.ts | 12 +++- 8 files changed, 117 insertions(+), 21 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index b6fce3b6365..e0cd6be0a59 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -25,7 +25,7 @@ import { QueryDict } from "matrix-js-sdk/src/utils"; import { logger } from "matrix-js-sdk/src/logger"; import { IMatrixClientCreds, MatrixClientPeg } from "./MatrixClientPeg"; -import SecurityCustomisations from "./customisations/Security"; +import { ModuleRunner } from "./modules/ModuleRunner"; import EventIndexPeg from "./indexing/EventIndexPeg"; import createMatrixClient from "./utils/createMatrixClient"; import Notifier from "./Notifier"; @@ -899,7 +899,8 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise void, ): Promise { - const keyFromCustomisations = SecurityCustomisations.getSecretStorageKey?.(); + // const keyFromCustomisations = SecurityCustomisations.getSecretStorageKey?.(); + const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup?.getSecretStorageKey(); if (keyFromCustomisations) { - logger.log("Using key from security customisations (dehydration)"); + logger.log("CryptoSetupExtension: Using key from extension (dehydration)"); return keyFromCustomisations; } @@ -419,7 +421,8 @@ async function doAccessSecretStorage(func: () => Promise, forceReset: bool // inner operation completes. return await func(); } catch (e) { - SecurityCustomisations.catchAccessSecretStorageError?.(e); + // SecurityCustomisations.catchAccessSecretStorageError?.(e as Error); + ModuleRunner.instance.extensions.cryptoSetup?.catchAccessSecretStorageError(e as Error); logger.error(e); // Re-throw so that higher level logic can abort as needed throw e; diff --git a/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx b/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx index 036fb5038b3..5f638b74521 100644 --- a/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx @@ -41,7 +41,7 @@ import { isSecureBackupRequired, SecureBackupSetupMethod, } from "../../../../utils/WellKnownUtils"; -import SecurityCustomisations from "../../../../customisations/Security"; +import { ModuleRunner } from "../../../../modules/ModuleRunner"; import Field from "../../../../components/views/elements/Field"; import BaseDialog from "../../../../components/views/dialogs/BaseDialog"; import Spinner from "../../../../components/views/elements/Spinner"; @@ -181,9 +181,10 @@ export default class CreateSecretStorageDialog extends React.PureComponent { if (crossSigningIsSetUp) { // if the user has previously set up cross-signing, verify this device so we can fetch the // private keys. - if (SecurityCustomisations.SHOW_ENCRYPTION_SETUP_UI === false) { - this.onLoggedIn(); + + + // if (SecurityCustomisations.SHOW_ENCRYPTION_SETUP_UI === false) { + const cryptoExtension = ModuleRunner.instance.extensions.cryptoSetup; + if (cryptoExtension !== undefined && cryptoExtension.SHOW_ENCRYPTION_SETUP_UI == false) { + this.onLoggedIn(); } else { this.setStateForNewView({ view: Views.COMPLETE_SECURITY }); } diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index 1a94eb16b38..e1abe40d35e 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -17,6 +17,10 @@ limitations under the License. import { safeSet } from "matrix-js-sdk/src/utils"; import { TranslationStringsObject } from "@matrix-org/react-sdk-module-api/lib/types/translations"; import { AnyLifecycle } from "@matrix-org/react-sdk-module-api/lib/lifecycles/types"; +import { AllExtensions } from "@matrix-org/react-sdk-module-api/lib/types/extensions"; +import { DefaultCryptoSetupExtensions } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; +import { DefaultExperimentalExtensions } from "@matrix-org/react-sdk-module-api/lib/lifecycles/ExperimentalExtensions"; +import { RuntimeModule } from "@matrix-org/react-sdk-module-api"; import { AppModule } from "./AppModule"; import { ModuleFactory } from "./ModuleFactory"; @@ -29,6 +33,13 @@ import "./ModuleComponents"; export class ModuleRunner { public static readonly instance = new ModuleRunner(); + public className: string = ModuleRunner.name; + + public extensions: AllExtensions = { + cryptoSetup: new DefaultCryptoSetupExtensions(), + experimental: new DefaultExperimentalExtensions(), + }; + private modules: AppModule[] = []; private constructor() { @@ -42,6 +53,11 @@ export class ModuleRunner { */ public reset(): void { this.modules = []; + + this.extensions = { + cryptoSetup: new DefaultCryptoSetupExtensions(), + experimental: new DefaultExperimentalExtensions(), + }; } /** @@ -66,6 +82,52 @@ export class ModuleRunner { return merged; } + + /** + * Ensure we register extensions provided by the modules + */ + private updateExtensions(): void { + const cryptoSetupExtensions: Array = []; + const experimentalExtensions: Array = []; + + this.modules.forEach((m) => { + /* Record the cryptoSetup extensions if any */ + if (m.module.extensions?.cryptoSetup) { + cryptoSetupExtensions.push(m.module); + } + + /* Record the experimantal extensions if any */ + if (m.module.extensions?.experimental) { + experimentalExtensions.push(m.module); + } + }); + + /* Enforce rule that only a single module may provide a given extension */ + if (cryptoSetupExtensions.length > 1) { + throw new Error( + `cryptoSetup extension is provided by modules ${cryptoSetupExtensions + .map((m) => m.moduleName) + .join(", ")}, but can only be provided by a single module`, + ); + } + if (experimentalExtensions.length > 1) { + throw new Error( + `experimental extension is provided by modules ${experimentalExtensions + .map((m) => m.moduleName) + .join(", ")}, but can only be provided by a single module`, + ); + } + + /* Override the default extension if extension was provided by a module */ + if (cryptoSetupExtensions.length == 1) { + this.extensions.cryptoSetup = cryptoSetupExtensions[0].extensions?.cryptoSetup; + } + + if (experimentalExtensions.length == 1) { + this.extensions.experimental = cryptoSetupExtensions[0].extensions?.experimental; + } + } + /** * Registers a factory which creates a module for later loading. The factory * will be called immediately. @@ -73,6 +135,15 @@ export class ModuleRunner { */ public registerModule(factory: ModuleFactory): void { this.modules.push(new AppModule(factory)); + + /** + * Check if the new module provides any extensions, and also ensure a given extension is only provided by a single runtime module + * Slightly inefficient to do this on each registration, but avoids changes to element-web installer code + * Also note that this require that the statement in the comment above, about immediately calling the factory, is in fact true + * (otherwise wrapped RuntimeModules will not be available) + */ + + this.updateExtensions(); } /** diff --git a/src/toasts/SetupEncryptionToast.ts b/src/toasts/SetupEncryptionToast.ts index e55e665a27f..c0f4cc49779 100644 --- a/src/toasts/SetupEncryptionToast.ts +++ b/src/toasts/SetupEncryptionToast.ts @@ -21,7 +21,8 @@ import SetupEncryptionDialog from "../components/views/dialogs/security/SetupEnc import { accessSecretStorage } from "../SecurityManager"; import ToastStore from "../stores/ToastStore"; import GenericToast from "../components/views/toasts/GenericToast"; -import SecurityCustomisations from "../customisations/Security"; +import { ModuleRunner } from "../modules/ModuleRunner"; +import { SetupEncryptionStore } from "../stores/SetupEncryptionStore"; import Spinner from "../components/views/elements/Spinner"; const TOAST_KEY = "setupencryption"; @@ -79,7 +80,14 @@ const onReject = (): void => { }; export const showToast = (kind: Kind): void => { - if (SecurityCustomisations.setupEncryptionNeeded?.(kind)) { + // if (SecurityCustomisations.setupEncryptionNeeded?.(kind)) { + // return; + // } + + if (ModuleRunner.instance.extensions.cryptoSetup?.setupEncryptionNeeded({ + kind: kind as any, + storeProvider: { getInstance: () => SetupEncryptionStore.sharedInstance() }, + })) { return; } From 9b25236fcae250b3d32de5d2ceac189a2b8c9cc2 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Thu, 14 Mar 2024 16:15:43 +0100 Subject: [PATCH 02/34] Updated depenndecy and added tests --- package.json | 2 +- test/modules/MockModule.ts | 69 +++++++++++++++++++++++++++++++ test/modules/ModuleRunner-test.ts | 28 ++++++++++++- yarn.lock | 39 ++++------------- 4 files changed, 104 insertions(+), 34 deletions(-) diff --git a/package.json b/package.json index 4046fd0a8e2..40879285c3f 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "@matrix-org/emojibase-bindings": "^1.1.2", "@matrix-org/matrix-wysiwyg": "2.17.0", "@matrix-org/olm": "3.2.15", - "@matrix-org/react-sdk-module-api": "^2.3.0", + "@matrix-org/react-sdk-module-api": "^2.4.0", "@matrix-org/spec": "^1.7.0", "@sentry/browser": "^7.0.0", "@testing-library/react-hooks": "^8.0.1", diff --git a/test/modules/MockModule.ts b/test/modules/MockModule.ts index ab29b025086..8c6910688fc 100644 --- a/test/modules/MockModule.ts +++ b/test/modules/MockModule.ts @@ -16,6 +16,13 @@ limitations under the License. import { RuntimeModule } from "@matrix-org/react-sdk-module-api/lib/RuntimeModule"; import { ModuleApi } from "@matrix-org/react-sdk-module-api/lib/ModuleApi"; +import { AllExtensions } from "@matrix-org/react-sdk-module-api/lib/types/extensions"; +import { + CryptoSetupExtensionsBase, + ExtendedMatrixClientCreds, + SecretStorageKeyDescriptionAesV1, + CryptoSetupArgs, +} from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; import { ModuleRunner } from "../../src/modules/ModuleRunner"; @@ -43,3 +50,65 @@ export function registerMockModule(): MockModule { } return module; } + +export class MockModuleWithCryptoSetupExtension extends RuntimeModule { + public get apiInstance(): ModuleApi { + return this.moduleApi; + } + + moduleName: string = MockModuleWithCryptoSetupExtension.name; + + extensions: AllExtensions = { + cryptoSetup: new (class extends CryptoSetupExtensionsBase { + SHOW_ENCRYPTION_SETUP_UI = true; + + examineLoginResponse(response: any, credentials: ExtendedMatrixClientCreds): void { + throw new Error("Method not implemented."); + } + persistCredentials(credentials: ExtendedMatrixClientCreds): void { + throw new Error("Method not implemented."); + } + getSecretStorageKey(): Uint8Array | null { + return Uint8Array.from([0x11, 0x22, 0x99]); + } + createSecretStorageKey(): Uint8Array | null { + throw new Error("Method not implemented."); + } + catchAccessSecretStorageError(e: Error): void { + throw new Error("Method not implemented."); + } + setupEncryptionNeeded(args: CryptoSetupArgs): boolean { + throw new Error("Method not implemented."); + } + getDehydrationKeyCallback(): + | (( + keyInfo: SecretStorageKeyDescriptionAesV1, + checkFunc: (key: Uint8Array) => void, + ) => Promise) + | null { + throw new Error("Method not implemented."); + } + })(), + }; + + public constructor(moduleApi: ModuleApi) { + super(moduleApi); + } +} + +export function registerMockModuleWithCryptoSetupExtension(): MockModuleWithCryptoSetupExtension { + let module: MockModuleWithCryptoSetupExtension | undefined; + + ModuleRunner.instance.registerModule((api) => { + if (module) { + throw new Error("State machine error: ModuleRunner created the module twice"); + } + module = new MockModuleWithCryptoSetupExtension(api); + return module; + }); + if (!module) { + throw new Error("State machine error: ModuleRunner did not create module"); + } + return module; +} + diff --git a/test/modules/ModuleRunner-test.ts b/test/modules/ModuleRunner-test.ts index 175c62c9e6b..737ff885dd6 100644 --- a/test/modules/ModuleRunner-test.ts +++ b/test/modules/ModuleRunner-test.ts @@ -16,7 +16,7 @@ limitations under the License. import { RoomPreviewOpts, RoomViewLifecycle } from "@matrix-org/react-sdk-module-api/lib/lifecycles/RoomViewLifecycle"; -import { MockModule, registerMockModule } from "./MockModule"; +import { MockModule, registerMockModule, registerMockModuleWithCryptoSetupExtension } from "./MockModule"; import { ModuleRunner } from "../../src/modules/ModuleRunner"; describe("ModuleRunner", () => { @@ -49,4 +49,30 @@ describe("ModuleRunner", () => { ]); }); }); + + describe("extensions", () => { + it("should return default values when no crypto-setup extensions are provided by a registered module", async () => { + registerMockModule(); + const result = ModuleRunner.instance.extensions.cryptoSetup?.getSecretStorageKey(); + expect(result).toBeNull(); + }); + + it("should return default values when no experimental extensions are provided by a registered module", async () => { + registerMockModule(); + const result = ModuleRunner.instance.extensions?.experimental?.experimentalMethod(); + expect(result).toBeNull(); + }); + + it("should return value from crypto-setup-extensions provided by a registered module", async () => { + registerMockModuleWithCryptoSetupExtension(); + const result = ModuleRunner.instance.extensions.cryptoSetup?.getSecretStorageKey(); + expect(result).toEqual(Uint8Array.from([0x11, 0x22, 0x99])); + }); + + it("must not allow multiple modules to provide a given extension", async () => { + registerMockModuleWithCryptoSetupExtension(); + const t = () => registerMockModuleWithCryptoSetupExtension(); + expect(t).toThrow(Error); + }); + }); }); diff --git a/yarn.lock b/yarn.lock index 87089aec890..e886b2c623e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1841,10 +1841,10 @@ resolved "https://registry.yarnpkg.com/@matrix-org/olm/-/olm-3.2.15.tgz#55f3c1b70a21bbee3f9195cecd6846b1083451ec" integrity sha512-S7lOrndAK9/8qOtaTq/WhttJC/o4GAzdfK0MUPpo8ApzsJEC0QjtwrkC3KBXdFP1cD1MXi/mlKR7aaoVMKgs6Q== -"@matrix-org/react-sdk-module-api@^2.3.0": - version "2.3.0" - resolved "https://registry.yarnpkg.com/@matrix-org/react-sdk-module-api/-/react-sdk-module-api-2.3.0.tgz#85be5cfc73be0494c13d4dc9050cb70c58d7a08b" - integrity sha512-x/ie44yaXNtE5AKcmQiW5yINVEIJ7IjjEc35vj6j52fM8tZ9XbJx9PANKSWsdd0NJp3OqyaeHftmN6ESfx4YoQ== +"@matrix-org/react-sdk-module-api@^2.4.0": + version "2.4.0" + resolved "https://registry.yarnpkg.com/@matrix-org/react-sdk-module-api/-/react-sdk-module-api-2.4.0.tgz#5e4552acbe728141f42c1d54d75dcb4efea9301c" + integrity sha512-cPb+YaqllfJkRX0ofcG/0YdHxCvcMAvUbdNMO2olpGL8vwbBP6mHdhbZ87z9pgsRIVOqfFuLUE3WeW0hxWrklQ== dependencies: "@babel/runtime" "^7.17.9" @@ -8465,16 +8465,7 @@ string-length@^4.0.1: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0": - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - -string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -8549,14 +8540,7 @@ string_decoder@~1.1.1: dependencies: safe-buffer "~5.1.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - -strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -9337,7 +9321,7 @@ which@^2.0.1: dependencies: isexe "^2.0.0" -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -9355,15 +9339,6 @@ wrap-ansi@^6.2.0: string-width "^4.1.0" strip-ansi "^6.0.0" -wrap-ansi@^7.0.0: - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214" From b84fe8619ee747c9bfb2df66f2c3d1890d3862d4 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Thu, 14 Mar 2024 16:21:10 +0100 Subject: [PATCH 03/34] Fixed style and formatting with prettier --- src/Login.ts | 2 +- src/MatrixClientPeg.ts | 4 ++-- src/components/structures/MatrixChat.tsx | 3 +-- src/modules/ModuleRunner.ts | 1 - src/toasts/SetupEncryptionToast.ts | 6 ++++-- test/modules/MockModule.ts | 1 - 6 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Login.ts b/src/Login.ts index e8336e20b1a..16a50d2f7ed 100644 --- a/src/Login.ts +++ b/src/Login.ts @@ -291,7 +291,7 @@ export async function sendLoginRequest( accessToken: data.access_token, }; - // SecurityCustomisations.examineLoginResponse?.(data, creds); + // SecurityCustomisations.examineLoginResponse?.(data, creds); ModuleRunner.instance.extensions.cryptoSetup?.examineLoginResponse(data, creds); return creds; diff --git a/src/MatrixClientPeg.ts b/src/MatrixClientPeg.ts index d47329012a4..66e04708491 100644 --- a/src/MatrixClientPeg.ts +++ b/src/MatrixClientPeg.ts @@ -468,9 +468,9 @@ class MatrixClientPegClass implements IMatrixClientPeg { // opts.cryptoCallbacks!.getDehydrationKey = SecurityCustomisations.getDehydrationKey; // } - console.log("CryptoSetupExtensions: Executing getDehydrationKeyCallback...") + console.log("CryptoSetupExtensions: Executing getDehydrationKeyCallback..."); const dehydrationKeyCallback = ModuleRunner.instance.extensions.cryptoSetup?.getDehydrationKeyCallback(); - console.log("CryptoSetupExtensions: Executing getDehydrationKeyCallback...Done") + console.log("CryptoSetupExtensions: Executing getDehydrationKeyCallback...Done"); if (dehydrationKeyCallback) { opts.cryptoCallbacks!.getDehydrationKey = dehydrationKeyCallback; } diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index b794087dde5..383ba106d13 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -444,11 +444,10 @@ export default class MatrixChat extends React.PureComponent { // if the user has previously set up cross-signing, verify this device so we can fetch the // private keys. - // if (SecurityCustomisations.SHOW_ENCRYPTION_SETUP_UI === false) { const cryptoExtension = ModuleRunner.instance.extensions.cryptoSetup; if (cryptoExtension !== undefined && cryptoExtension.SHOW_ENCRYPTION_SETUP_UI == false) { - this.onLoggedIn(); + this.onLoggedIn(); } else { this.setStateForNewView({ view: Views.COMPLETE_SECURITY }); } diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index e1abe40d35e..e249c111a94 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -82,7 +82,6 @@ export class ModuleRunner { return merged; } - /** * Ensure we register extensions provided by the modules */ diff --git a/src/toasts/SetupEncryptionToast.ts b/src/toasts/SetupEncryptionToast.ts index c0f4cc49779..06363e387e3 100644 --- a/src/toasts/SetupEncryptionToast.ts +++ b/src/toasts/SetupEncryptionToast.ts @@ -84,10 +84,12 @@ export const showToast = (kind: Kind): void => { // return; // } - if (ModuleRunner.instance.extensions.cryptoSetup?.setupEncryptionNeeded({ + if ( + ModuleRunner.instance.extensions.cryptoSetup?.setupEncryptionNeeded({ kind: kind as any, storeProvider: { getInstance: () => SetupEncryptionStore.sharedInstance() }, - })) { + }) + ) { return; } diff --git a/test/modules/MockModule.ts b/test/modules/MockModule.ts index 8c6910688fc..84d074292d6 100644 --- a/test/modules/MockModule.ts +++ b/test/modules/MockModule.ts @@ -111,4 +111,3 @@ export function registerMockModuleWithCryptoSetupExtension(): MockModuleWithCryp } return module; } - From d3bbb4a0772a5a73b84a26bb277b981576f9df94 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Fri, 5 Apr 2024 12:25:03 +0200 Subject: [PATCH 04/34] Fix according to Element PR comments --- src/Lifecycle.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index e0cd6be0a59..07edc6954c1 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -899,7 +899,6 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise Date: Fri, 5 Apr 2024 12:25:53 +0200 Subject: [PATCH 05/34] Fixing issues raised in PR review --- src/Login.ts | 1 - src/modules/ModuleRunner.ts | 14 +++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/Login.ts b/src/Login.ts index 16a50d2f7ed..738e1c4b7a0 100644 --- a/src/Login.ts +++ b/src/Login.ts @@ -291,7 +291,6 @@ export async function sendLoginRequest( accessToken: data.access_token, }; - // SecurityCustomisations.examineLoginResponse?.(data, creds); ModuleRunner.instance.extensions.cryptoSetup?.examineLoginResponse(data, creds); return creds; diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index e249c111a94..5a7aa47644f 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -18,8 +18,8 @@ import { safeSet } from "matrix-js-sdk/src/utils"; import { TranslationStringsObject } from "@matrix-org/react-sdk-module-api/lib/types/translations"; import { AnyLifecycle } from "@matrix-org/react-sdk-module-api/lib/lifecycles/types"; import { AllExtensions } from "@matrix-org/react-sdk-module-api/lib/types/extensions"; -import { DefaultCryptoSetupExtensions } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; -import { DefaultExperimentalExtensions } from "@matrix-org/react-sdk-module-api/lib/lifecycles/ExperimentalExtensions"; +import { DefaultCryptoSetupExtensions, ProvideCryptoSetupExtensions } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; +import { DefaultExperimentalExtensions, ProvideExperimentalExtensions } from "@matrix-org/react-sdk-module-api/lib/lifecycles/ExperimentalExtensions"; import { RuntimeModule } from "@matrix-org/react-sdk-module-api"; import { AppModule } from "./AppModule"; @@ -35,7 +35,7 @@ export class ModuleRunner { public className: string = ModuleRunner.name; - public extensions: AllExtensions = { + private extensions: AllExtensions = { cryptoSetup: new DefaultCryptoSetupExtensions(), experimental: new DefaultExperimentalExtensions(), }; @@ -46,6 +46,14 @@ export class ModuleRunner { // we only want one instance } + public get cryptoSetupExtension(): ProvideCryptoSetupExtensions | undefined { + return this.extensions.cryptoSetup; + } + + public get experimentalExtension(): ProvideExperimentalExtensions | undefined { + return this.extensions.experimental; + } + /** * Resets the runner, clearing all known modules. * From 78c7ea7c46ba049336d7328a865f11e5613bd8b5 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Sat, 6 Apr 2024 10:21:29 +0200 Subject: [PATCH 06/34] Removed commented code. Improved encapsulation. Removed noisy logging --- src/MatrixClientPeg.ts | 6 ----- src/SecurityManager.ts | 3 --- .../security/CreateSecretStorageDialog.tsx | 1 - src/components/structures/MatrixChat.tsx | 1 - src/modules/ModuleRunner.ts | 26 +++++++++++-------- src/toasts/SetupEncryptionToast.ts | 3 --- 6 files changed, 15 insertions(+), 25 deletions(-) diff --git a/src/MatrixClientPeg.ts b/src/MatrixClientPeg.ts index 66e04708491..0c4f426b4f3 100644 --- a/src/MatrixClientPeg.ts +++ b/src/MatrixClientPeg.ts @@ -464,13 +464,7 @@ class MatrixClientPegClass implements IMatrixClientPeg { }, }; - // if (SecurityCustomisations.getDehydrationKey) { - // opts.cryptoCallbacks!.getDehydrationKey = SecurityCustomisations.getDehydrationKey; - // } - - console.log("CryptoSetupExtensions: Executing getDehydrationKeyCallback..."); const dehydrationKeyCallback = ModuleRunner.instance.extensions.cryptoSetup?.getDehydrationKeyCallback(); - console.log("CryptoSetupExtensions: Executing getDehydrationKeyCallback...Done"); if (dehydrationKeyCallback) { opts.cryptoCallbacks!.getDehydrationKey = dehydrationKeyCallback; } diff --git a/src/SecurityManager.ts b/src/SecurityManager.ts index 5c991209a75..3c87e71ef37 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -130,7 +130,6 @@ async function getSecretStorageKey({ } } - // const keyFromCustomisations = SecurityCustomisations.getSecretStorageKey?.(); const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup?.getSecretStorageKey(); if (keyFromCustomisations) { logger.log("CryptoSetupExtension: Using key from extension (secret storage)"); @@ -181,7 +180,6 @@ export async function getDehydrationKey( keyInfo: ISecretStorageKeyInfo, checkFunc: (data: Uint8Array) => void, ): Promise { - // const keyFromCustomisations = SecurityCustomisations.getSecretStorageKey?.(); const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup?.getSecretStorageKey(); if (keyFromCustomisations) { logger.log("CryptoSetupExtension: Using key from extension (dehydration)"); @@ -421,7 +419,6 @@ async function doAccessSecretStorage(func: () => Promise, forceReset: bool // inner operation completes. return await func(); } catch (e) { - // SecurityCustomisations.catchAccessSecretStorageError?.(e as Error); ModuleRunner.instance.extensions.cryptoSetup?.catchAccessSecretStorageError(e as Error); logger.error(e); // Re-throw so that higher level logic can abort as needed diff --git a/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx b/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx index 5f638b74521..8225dd0b367 100644 --- a/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx @@ -181,7 +181,6 @@ export default class CreateSecretStorageDialog extends React.PureComponent { // if the user has previously set up cross-signing, verify this device so we can fetch the // private keys. - // if (SecurityCustomisations.SHOW_ENCRYPTION_SETUP_UI === false) { const cryptoExtension = ModuleRunner.instance.extensions.cryptoSetup; if (cryptoExtension !== undefined && cryptoExtension.SHOW_ENCRYPTION_SETUP_UI == false) { this.onLoggedIn(); diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index 5a7aa47644f..97b9536a241 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -35,9 +35,13 @@ export class ModuleRunner { public className: string = ModuleRunner.name; - private extensions: AllExtensions = { - cryptoSetup: new DefaultCryptoSetupExtensions(), - experimental: new DefaultExperimentalExtensions(), + private _extensions: AllExtensions = { + cryptoSetup: new DefaultCryptoSetupExtensions(), + experimental: new DefaultExperimentalExtensions(), + }; + + public get extensions(): AllExtensions { + return this._extensions; }; private modules: AppModule[] = []; @@ -46,13 +50,13 @@ export class ModuleRunner { // we only want one instance } - public get cryptoSetupExtension(): ProvideCryptoSetupExtensions | undefined { - return this.extensions.cryptoSetup; - } + // public get cryptoSetupExtension(): ProvideCryptoSetupExtensions | undefined { + // return this.extensions.cryptoSetup; + // } - public get experimentalExtension(): ProvideExperimentalExtensions | undefined { - return this.extensions.experimental; - } + // public get experimentalExtension(): ProvideExperimentalExtensions | undefined { + // return this.extensions.experimental; + // } /** * Resets the runner, clearing all known modules. @@ -62,7 +66,7 @@ export class ModuleRunner { public reset(): void { this.modules = []; - this.extensions = { + this._extensions = { cryptoSetup: new DefaultCryptoSetupExtensions(), experimental: new DefaultExperimentalExtensions(), }; @@ -103,7 +107,7 @@ export class ModuleRunner { cryptoSetupExtensions.push(m.module); } - /* Record the experimantal extensions if any */ + /* Record the experimental extensions if any */ if (m.module.extensions?.experimental) { experimentalExtensions.push(m.module); } diff --git a/src/toasts/SetupEncryptionToast.ts b/src/toasts/SetupEncryptionToast.ts index 06363e387e3..04152341c86 100644 --- a/src/toasts/SetupEncryptionToast.ts +++ b/src/toasts/SetupEncryptionToast.ts @@ -80,9 +80,6 @@ const onReject = (): void => { }; export const showToast = (kind: Kind): void => { - // if (SecurityCustomisations.setupEncryptionNeeded?.(kind)) { - // return; - // } if ( ModuleRunner.instance.extensions.cryptoSetup?.setupEncryptionNeeded({ From c39c34dd5f851d3d5448f18452a381d22526ca8c Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Sat, 6 Apr 2024 10:29:43 +0200 Subject: [PATCH 07/34] Improved language of comment about calling the factory --- src/modules/ModuleRunner.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index 97b9536a241..c3d9556597a 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -150,8 +150,8 @@ export class ModuleRunner { /** * Check if the new module provides any extensions, and also ensure a given extension is only provided by a single runtime module * Slightly inefficient to do this on each registration, but avoids changes to element-web installer code - * Also note that this require that the statement in the comment above, about immediately calling the factory, is in fact true - * (otherwise wrapped RuntimeModules will not be available) + * + * Also note that this will break if the current behavior of calling the factory immediately changes. */ this.updateExtensions(); From b64f50c1a90ce376214a7bab60b7e0ddacea1fd5 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Sat, 6 Apr 2024 14:33:12 +0200 Subject: [PATCH 08/34] Refactor to get better encapsulation --- src/modules/ModuleRunner.ts | 141 +++++++++++++++++++----------------- 1 file changed, 75 insertions(+), 66 deletions(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index c3d9556597a..eeb6d831f1a 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -17,16 +17,76 @@ limitations under the License. import { safeSet } from "matrix-js-sdk/src/utils"; import { TranslationStringsObject } from "@matrix-org/react-sdk-module-api/lib/types/translations"; import { AnyLifecycle } from "@matrix-org/react-sdk-module-api/lib/lifecycles/types"; -import { AllExtensions } from "@matrix-org/react-sdk-module-api/lib/types/extensions"; import { DefaultCryptoSetupExtensions, ProvideCryptoSetupExtensions } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; import { DefaultExperimentalExtensions, ProvideExperimentalExtensions } from "@matrix-org/react-sdk-module-api/lib/lifecycles/ExperimentalExtensions"; -import { RuntimeModule } from "@matrix-org/react-sdk-module-api"; import { AppModule } from "./AppModule"; import { ModuleFactory } from "./ModuleFactory"; import "./ModuleComponents"; + + +class ExtensionImplementationMap { + public hasDefaultCryptoSetupExtension: boolean = true; + public hasDefaultExperimentalExtension: boolean = true; +} + +class ModuleRunnerExtensions { + + private _cryptoSetup: ProvideCryptoSetupExtensions; + private _experimental: ProvideExperimentalExtensions; + private _implementionMap: ExtensionImplementationMap = new ExtensionImplementationMap(); + + constructor() { + // Set up defaults + this._cryptoSetup = new DefaultCryptoSetupExtensions(); + this._experimental = new DefaultExperimentalExtensions(); + } + + public get cryptoSetup(): ProvideCryptoSetupExtensions{ + return this._cryptoSetup; + } + + public get experimental(): ProvideExperimentalExtensions{ + return this._experimental; + } + + /** + * Ensure we register extensions provided by the module + */ + public addExtensions(module: AppModule): void { + + var runtimeModule = module.module; + + /* Record the cryptoSetup extensions if any */ + if (runtimeModule.extensions?.cryptoSetup) { + if(this._implementionMap.hasDefaultCryptoSetupExtension){ + this._cryptoSetup = runtimeModule.extensions?.cryptoSetup; + this._implementionMap.hasDefaultCryptoSetupExtension = false; + } + else { + throw new Error( + `adding cryptoSetup extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided`, + ); + } + } + + if (runtimeModule.extensions?.experimental) { + if(this._implementionMap.hasDefaultExperimentalExtension){ + this._experimental = runtimeModule.extensions?.experimental; + this._implementionMap.hasDefaultExperimentalExtension = false; + } + else { + throw new Error( + `adding experimental extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided`, + ); + } + } + } +} + + /** * Handles and coordinates the operation of modules. */ @@ -35,12 +95,14 @@ export class ModuleRunner { public className: string = ModuleRunner.name; - private _extensions: AllExtensions = { - cryptoSetup: new DefaultCryptoSetupExtensions(), - experimental: new DefaultExperimentalExtensions(), - }; + // private _extensions: AllExtensions = { + // cryptoSetup: new DefaultCryptoSetupExtensions(), + // experimental: new DefaultExperimentalExtensions(), + // }; + + private _extensions = new ModuleRunnerExtensions(); - public get extensions(): AllExtensions { + public get extensions(): ModuleRunnerExtensions { return this._extensions; }; @@ -50,14 +112,6 @@ export class ModuleRunner { // we only want one instance } - // public get cryptoSetupExtension(): ProvideCryptoSetupExtensions | undefined { - // return this.extensions.cryptoSetup; - // } - - // public get experimentalExtension(): ProvideExperimentalExtensions | undefined { - // return this.extensions.experimental; - // } - /** * Resets the runner, clearing all known modules. * @@ -65,11 +119,7 @@ export class ModuleRunner { */ public reset(): void { this.modules = []; - - this._extensions = { - cryptoSetup: new DefaultCryptoSetupExtensions(), - experimental: new DefaultExperimentalExtensions(), - }; + this._extensions = new ModuleRunnerExtensions(); } /** @@ -94,50 +144,6 @@ export class ModuleRunner { return merged; } - /** - * Ensure we register extensions provided by the modules - */ - private updateExtensions(): void { - const cryptoSetupExtensions: Array = []; - const experimentalExtensions: Array = []; - - this.modules.forEach((m) => { - /* Record the cryptoSetup extensions if any */ - if (m.module.extensions?.cryptoSetup) { - cryptoSetupExtensions.push(m.module); - } - - /* Record the experimental extensions if any */ - if (m.module.extensions?.experimental) { - experimentalExtensions.push(m.module); - } - }); - - /* Enforce rule that only a single module may provide a given extension */ - if (cryptoSetupExtensions.length > 1) { - throw new Error( - `cryptoSetup extension is provided by modules ${cryptoSetupExtensions - .map((m) => m.moduleName) - .join(", ")}, but can only be provided by a single module`, - ); - } - if (experimentalExtensions.length > 1) { - throw new Error( - `experimental extension is provided by modules ${experimentalExtensions - .map((m) => m.moduleName) - .join(", ")}, but can only be provided by a single module`, - ); - } - - /* Override the default extension if extension was provided by a module */ - if (cryptoSetupExtensions.length == 1) { - this.extensions.cryptoSetup = cryptoSetupExtensions[0].extensions?.cryptoSetup; - } - - if (experimentalExtensions.length == 1) { - this.extensions.experimental = cryptoSetupExtensions[0].extensions?.experimental; - } - } /** * Registers a factory which creates a module for later loading. The factory @@ -145,7 +151,10 @@ export class ModuleRunner { * @param factory The module factory. */ public registerModule(factory: ModuleFactory): void { - this.modules.push(new AppModule(factory)); + + var appModule = new AppModule(factory) + + this.modules.push(appModule); /** * Check if the new module provides any extensions, and also ensure a given extension is only provided by a single runtime module @@ -154,7 +163,7 @@ export class ModuleRunner { * Also note that this will break if the current behavior of calling the factory immediately changes. */ - this.updateExtensions(); + this._extensions.addExtensions(appModule); } /** From f7a2ec86d941187e71c830fb37fea4b7ffbd5127 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Sat, 6 Apr 2024 15:05:34 +0200 Subject: [PATCH 09/34] Find a better name. Provide explicit reset function. Provide more TSDoc --- src/modules/ModuleRunner.ts | 70 +++++++++++++++++++++++++++---------- 1 file changed, 51 insertions(+), 19 deletions(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index eeb6d831f1a..0edf30d307e 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -32,7 +32,10 @@ class ExtensionImplementationMap { public hasDefaultExperimentalExtension: boolean = true; } -class ModuleRunnerExtensions { +/** + * Handles and manages any extensions provided by modules + */ +class ExtensionsManager { private _cryptoSetup: ProvideCryptoSetupExtensions; private _experimental: ProvideExperimentalExtensions; @@ -44,16 +47,46 @@ class ModuleRunnerExtensions { this._experimental = new DefaultExperimentalExtensions(); } + /** + * Provides a crypto setup extension. + * + * @returns The registered extension. If no module provides this extension, a default implementation is returned + */ public get cryptoSetup(): ProvideCryptoSetupExtensions{ return this._cryptoSetup; } - public get experimental(): ProvideExperimentalExtensions{ + /** + * Provides a n experimental extension. + * + * @remarks + * This method extension is provided to simplify experimentaion an development, and is not intended for production code + * + * @returns The registered extension. If no module provides this extension, a default implementation is returned + */ + public get experimental(): ProvideExperimentalExtensions{ return this._experimental; } /** - * Ensure we register extensions provided by the module + * Resets the extension to defaults + * + * Intended for test usage only. + */ + public reset(): void { + this._implementionMap = new ExtensionImplementationMap(); + this._cryptoSetup = new DefaultCryptoSetupExtensions(); + this._experimental = new DefaultExperimentalExtensions(); + } + + + /** + * Add any extensions provided by the module + * + * @param module - The module. + * + * @throws if an extension is provided by more than one module + * */ public addExtensions(module: AppModule): void { @@ -72,6 +105,7 @@ class ModuleRunnerExtensions { } } + /* Record the experimental extensions if any */ if (runtimeModule.extensions?.experimental) { if(this._implementionMap.hasDefaultExperimentalExtension){ this._experimental = runtimeModule.extensions?.experimental; @@ -95,16 +129,7 @@ export class ModuleRunner { public className: string = ModuleRunner.name; - // private _extensions: AllExtensions = { - // cryptoSetup: new DefaultCryptoSetupExtensions(), - // experimental: new DefaultExperimentalExtensions(), - // }; - - private _extensions = new ModuleRunnerExtensions(); - - public get extensions(): ModuleRunnerExtensions { - return this._extensions; - }; + private _extensions = new ExtensionsManager(); private modules: AppModule[] = []; @@ -112,6 +137,15 @@ export class ModuleRunner { // we only want one instance } + /** + * Exposes all extensions which may be overridden/provided by modules + * + * @returns En extensionsmanager which exposes the extensions + */ + public get extensions(): ExtensionsManager { + return this._extensions; + }; + /** * Resets the runner, clearing all known modules. * @@ -119,7 +153,7 @@ export class ModuleRunner { */ public reset(): void { this.modules = []; - this._extensions = new ModuleRunnerExtensions(); + this._extensions.reset(); } /** @@ -157,15 +191,13 @@ export class ModuleRunner { this.modules.push(appModule); /** - * Check if the new module provides any extensions, and also ensure a given extension is only provided by a single runtime module - * Slightly inefficient to do this on each registration, but avoids changes to element-web installer code - * - * Also note that this will break if the current behavior of calling the factory immediately changes. + * Check if the new module provides any extensions, and also ensure a given extension is only provided by a single runtime module * + * @param appModule The app module to inspect for extensions. */ - this._extensions.addExtensions(appModule); } + /** * Invokes a lifecycle event, notifying registered modules. * @param lifecycleEvent The lifecycle event. From c905af8775b3ab13dadb0471a29025e757f07c55 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Sun, 7 Apr 2024 13:31:41 +0200 Subject: [PATCH 10/34] Simplify mock for cryptoSetup, and add assertion for exception message. --- test/modules/MockModule.ts | 41 ++++++------------------------- test/modules/ModuleRunner-test.ts | 3 ++- 2 files changed, 10 insertions(+), 34 deletions(-) diff --git a/test/modules/MockModule.ts b/test/modules/MockModule.ts index 84d074292d6..1c549cc2eb8 100644 --- a/test/modules/MockModule.ts +++ b/test/modules/MockModule.ts @@ -17,12 +17,7 @@ limitations under the License. import { RuntimeModule } from "@matrix-org/react-sdk-module-api/lib/RuntimeModule"; import { ModuleApi } from "@matrix-org/react-sdk-module-api/lib/ModuleApi"; import { AllExtensions } from "@matrix-org/react-sdk-module-api/lib/types/extensions"; -import { - CryptoSetupExtensionsBase, - ExtendedMatrixClientCreds, - SecretStorageKeyDescriptionAesV1, - CryptoSetupArgs, -} from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; +import { CryptoSetupExtensionsBase, } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; import { ModuleRunner } from "../../src/modules/ModuleRunner"; @@ -61,33 +56,13 @@ export class MockModuleWithCryptoSetupExtension extends RuntimeModule { extensions: AllExtensions = { cryptoSetup: new (class extends CryptoSetupExtensionsBase { SHOW_ENCRYPTION_SETUP_UI = true; - - examineLoginResponse(response: any, credentials: ExtendedMatrixClientCreds): void { - throw new Error("Method not implemented."); - } - persistCredentials(credentials: ExtendedMatrixClientCreds): void { - throw new Error("Method not implemented."); - } - getSecretStorageKey(): Uint8Array | null { - return Uint8Array.from([0x11, 0x22, 0x99]); - } - createSecretStorageKey(): Uint8Array | null { - throw new Error("Method not implemented."); - } - catchAccessSecretStorageError(e: Error): void { - throw new Error("Method not implemented."); - } - setupEncryptionNeeded(args: CryptoSetupArgs): boolean { - throw new Error("Method not implemented."); - } - getDehydrationKeyCallback(): - | (( - keyInfo: SecretStorageKeyDescriptionAesV1, - checkFunc: (key: Uint8Array) => void, - ) => Promise) - | null { - throw new Error("Method not implemented."); - } + examineLoginResponse = jest.fn(); + persistCredentials = jest.fn(); + getSecretStorageKey = jest.fn().mockReturnValue(Uint8Array.from([0x11, 0x22, 0x99])); + createSecretStorageKey = jest.fn() + catchAccessSecretStorageError = jest.fn() + setupEncryptionNeeded = jest.fn() + getDehydrationKeyCallback = jest.fn(); })(), }; diff --git a/test/modules/ModuleRunner-test.ts b/test/modules/ModuleRunner-test.ts index 737ff885dd6..bf79ea71c54 100644 --- a/test/modules/ModuleRunner-test.ts +++ b/test/modules/ModuleRunner-test.ts @@ -72,7 +72,8 @@ describe("ModuleRunner", () => { it("must not allow multiple modules to provide a given extension", async () => { registerMockModuleWithCryptoSetupExtension(); const t = () => registerMockModuleWithCryptoSetupExtension(); - expect(t).toThrow(Error); + expect(t).toThrow(Error) + expect(t).toThrow("adding cryptoSetup extension implementation from module MockModuleWithCryptoSetupExtension but an implementation was already provided"); }); }); }); From 158d977330a5bda354dd8e7036ea4698eab1e5e5 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Sun, 7 Apr 2024 15:38:11 +0200 Subject: [PATCH 11/34] Remove unused className property. Adjust TSDoc comments --- src/modules/ModuleRunner.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index 0edf30d307e..2b6885e53b6 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -69,7 +69,7 @@ class ExtensionsManager { } /** - * Resets the extension to defaults + * Resets the extension to the defaults * * Intended for test usage only. */ @@ -83,7 +83,7 @@ class ExtensionsManager { /** * Add any extensions provided by the module * - * @param module - The module. + * @param module - The appModule to check for extensions * * @throws if an extension is provided by more than one module * @@ -127,8 +127,6 @@ class ExtensionsManager { export class ModuleRunner { public static readonly instance = new ModuleRunner(); - public className: string = ModuleRunner.name; - private _extensions = new ExtensionsManager(); private modules: AppModule[] = []; @@ -147,7 +145,7 @@ export class ModuleRunner { }; /** - * Resets the runner, clearing all known modules. + * Resets the runner, clearing all known modules, and all extensions * * Intended for test usage only. */ @@ -191,8 +189,7 @@ export class ModuleRunner { this.modules.push(appModule); /** - * Check if the new module provides any extensions, and also ensure a given extension is only provided by a single runtime module * - * @param appModule The app module to inspect for extensions. + * Check if the new module provides any extensions, and also ensure a given extension is only provided by a single runtime module */ this._extensions.addExtensions(appModule); } From 5eccd0ea7349ea50cc341c5a92f156600a0c9cb5 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Sun, 7 Apr 2024 15:49:38 +0200 Subject: [PATCH 12/34] Fix linting and code style issues --- src/modules/ModuleRunner.ts | 70 +++++++++++++++--------------- src/toasts/SetupEncryptionToast.ts | 1 - test/modules/MockModule.ts | 12 ++--- test/modules/ModuleRunner-test.ts | 6 ++- 4 files changed, 44 insertions(+), 45 deletions(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index 2b6885e53b6..a749d8ae20d 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -17,16 +17,20 @@ limitations under the License. import { safeSet } from "matrix-js-sdk/src/utils"; import { TranslationStringsObject } from "@matrix-org/react-sdk-module-api/lib/types/translations"; import { AnyLifecycle } from "@matrix-org/react-sdk-module-api/lib/lifecycles/types"; -import { DefaultCryptoSetupExtensions, ProvideCryptoSetupExtensions } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; -import { DefaultExperimentalExtensions, ProvideExperimentalExtensions } from "@matrix-org/react-sdk-module-api/lib/lifecycles/ExperimentalExtensions"; +import { + DefaultCryptoSetupExtensions, + ProvideCryptoSetupExtensions, +} from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; +import { + DefaultExperimentalExtensions, + ProvideExperimentalExtensions, +} from "@matrix-org/react-sdk-module-api/lib/lifecycles/ExperimentalExtensions"; import { AppModule } from "./AppModule"; import { ModuleFactory } from "./ModuleFactory"; import "./ModuleComponents"; - - class ExtensionImplementationMap { public hasDefaultCryptoSetupExtension: boolean = true; public hasDefaultExperimentalExtension: boolean = true; @@ -36,36 +40,38 @@ class ExtensionImplementationMap { * Handles and manages any extensions provided by modules */ class ExtensionsManager { - private _cryptoSetup: ProvideCryptoSetupExtensions; private _experimental: ProvideExperimentalExtensions; private _implementionMap: ExtensionImplementationMap = new ExtensionImplementationMap(); - constructor() { - // Set up defaults + /** + * Create a new instance + */ + public constructor() { + // Set up defaults this._cryptoSetup = new DefaultCryptoSetupExtensions(); this._experimental = new DefaultExperimentalExtensions(); } - /** - * Provides a crypto setup extension. + /** + * Provides a crypto setup extension. * - * @returns The registered extension. If no module provides this extension, a default implementation is returned + * @returns The registered extension. If no module provides this extension, a default implementation is returned */ - public get cryptoSetup(): ProvideCryptoSetupExtensions{ - return this._cryptoSetup; + public get cryptoSetup(): ProvideCryptoSetupExtensions { + return this._cryptoSetup; } - /** - * Provides a n experimental extension. + /** + * Provides a n experimental extension. * * @remarks * This method extension is provided to simplify experimentaion an development, and is not intended for production code * - * @returns The registered extension. If no module provides this extension, a default implementation is returned + * @returns The registered extension. If no module provides this extension, a default implementation is returned */ - public get experimental(): ProvideExperimentalExtensions{ - return this._experimental; + public get experimental(): ProvideExperimentalExtensions { + return this._experimental; } /** @@ -79,7 +85,6 @@ class ExtensionsManager { this._experimental = new DefaultExperimentalExtensions(); } - /** * Add any extensions provided by the module * @@ -89,16 +94,14 @@ class ExtensionsManager { * */ public addExtensions(module: AppModule): void { - - var runtimeModule = module.module; + const runtimeModule = module.module; /* Record the cryptoSetup extensions if any */ if (runtimeModule.extensions?.cryptoSetup) { - if(this._implementionMap.hasDefaultCryptoSetupExtension){ + if (this._implementionMap.hasDefaultCryptoSetupExtension) { this._cryptoSetup = runtimeModule.extensions?.cryptoSetup; this._implementionMap.hasDefaultCryptoSetupExtension = false; - } - else { + } else { throw new Error( `adding cryptoSetup extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided`, ); @@ -107,20 +110,18 @@ class ExtensionsManager { /* Record the experimental extensions if any */ if (runtimeModule.extensions?.experimental) { - if(this._implementionMap.hasDefaultExperimentalExtension){ + if (this._implementionMap.hasDefaultExperimentalExtension) { this._experimental = runtimeModule.extensions?.experimental; this._implementionMap.hasDefaultExperimentalExtension = false; - } - else { + } else { throw new Error( `adding experimental extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided`, ); } } - } + } } - /** * Handles and coordinates the operation of modules. */ @@ -136,13 +137,13 @@ export class ModuleRunner { } /** - * Exposes all extensions which may be overridden/provided by modules + * Exposes all extensions which may be overridden/provided by modules * - * @returns En extensionsmanager which exposes the extensions + * @returns En extensionsmanager which exposes the extensions */ public get extensions(): ExtensionsManager { return this._extensions; - }; + } /** * Resets the runner, clearing all known modules, and all extensions @@ -176,25 +177,22 @@ export class ModuleRunner { return merged; } - /** * Registers a factory which creates a module for later loading. The factory * will be called immediately. * @param factory The module factory. */ public registerModule(factory: ModuleFactory): void { - - var appModule = new AppModule(factory) + const appModule = new AppModule(factory); this.modules.push(appModule); /** - * Check if the new module provides any extensions, and also ensure a given extension is only provided by a single runtime module + * Check if the new module provides any extensions, and also ensure a given extension is only provided by a single runtime module */ this._extensions.addExtensions(appModule); } - /** * Invokes a lifecycle event, notifying registered modules. * @param lifecycleEvent The lifecycle event. diff --git a/src/toasts/SetupEncryptionToast.ts b/src/toasts/SetupEncryptionToast.ts index 04152341c86..b2d37748551 100644 --- a/src/toasts/SetupEncryptionToast.ts +++ b/src/toasts/SetupEncryptionToast.ts @@ -80,7 +80,6 @@ const onReject = (): void => { }; export const showToast = (kind: Kind): void => { - if ( ModuleRunner.instance.extensions.cryptoSetup?.setupEncryptionNeeded({ kind: kind as any, diff --git a/test/modules/MockModule.ts b/test/modules/MockModule.ts index 1c549cc2eb8..97bef5ae13b 100644 --- a/test/modules/MockModule.ts +++ b/test/modules/MockModule.ts @@ -17,7 +17,7 @@ limitations under the License. import { RuntimeModule } from "@matrix-org/react-sdk-module-api/lib/RuntimeModule"; import { ModuleApi } from "@matrix-org/react-sdk-module-api/lib/ModuleApi"; import { AllExtensions } from "@matrix-org/react-sdk-module-api/lib/types/extensions"; -import { CryptoSetupExtensionsBase, } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; +import { CryptoSetupExtensionsBase } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; import { ModuleRunner } from "../../src/modules/ModuleRunner"; @@ -57,11 +57,11 @@ export class MockModuleWithCryptoSetupExtension extends RuntimeModule { cryptoSetup: new (class extends CryptoSetupExtensionsBase { SHOW_ENCRYPTION_SETUP_UI = true; examineLoginResponse = jest.fn(); - persistCredentials = jest.fn(); - getSecretStorageKey = jest.fn().mockReturnValue(Uint8Array.from([0x11, 0x22, 0x99])); - createSecretStorageKey = jest.fn() - catchAccessSecretStorageError = jest.fn() - setupEncryptionNeeded = jest.fn() + persistCredentials = jest.fn(); + getSecretStorageKey = jest.fn().mockReturnValue(Uint8Array.from([0x11, 0x22, 0x99])); + createSecretStorageKey = jest.fn(); + catchAccessSecretStorageError = jest.fn(); + setupEncryptionNeeded = jest.fn(); getDehydrationKeyCallback = jest.fn(); })(), }; diff --git a/test/modules/ModuleRunner-test.ts b/test/modules/ModuleRunner-test.ts index bf79ea71c54..fd57eac2c21 100644 --- a/test/modules/ModuleRunner-test.ts +++ b/test/modules/ModuleRunner-test.ts @@ -72,8 +72,10 @@ describe("ModuleRunner", () => { it("must not allow multiple modules to provide a given extension", async () => { registerMockModuleWithCryptoSetupExtension(); const t = () => registerMockModuleWithCryptoSetupExtension(); - expect(t).toThrow(Error) - expect(t).toThrow("adding cryptoSetup extension implementation from module MockModuleWithCryptoSetupExtension but an implementation was already provided"); + expect(t).toThrow(Error); + expect(t).toThrow( + "adding cryptoSetup extension implementation from module MockModuleWithCryptoSetupExtension but an implementation was already provided", + ); }); }); }); From f0973ef484ecd20bf34bae1cba4b9d68646186b7 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Sun, 7 Apr 2024 16:08:55 +0200 Subject: [PATCH 13/34] Added test to ensure we canregister anduse experimental extensions --- test/modules/MockModule.ts | 35 +++++++++++++++++++++++++++++++ test/modules/ModuleRunner-test.ts | 8 ++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/test/modules/MockModule.ts b/test/modules/MockModule.ts index 97bef5ae13b..0e57b060bb3 100644 --- a/test/modules/MockModule.ts +++ b/test/modules/MockModule.ts @@ -20,6 +20,7 @@ import { AllExtensions } from "@matrix-org/react-sdk-module-api/lib/types/extens import { CryptoSetupExtensionsBase } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; import { ModuleRunner } from "../../src/modules/ModuleRunner"; +import { ExperimentalExtensionsBase } from "@matrix-org/react-sdk-module-api/lib/lifecycles/ExperimentalExtensions"; export class MockModule extends RuntimeModule { public get apiInstance(): ModuleApi { @@ -71,6 +72,24 @@ export class MockModuleWithCryptoSetupExtension extends RuntimeModule { } } +export class MockModuleWithExperimentalExtension extends RuntimeModule { + public get apiInstance(): ModuleApi { + return this.moduleApi; + } + + moduleName: string = MockModuleWithExperimentalExtension.name; + + extensions: AllExtensions = { + experimental: new (class extends ExperimentalExtensionsBase { + experimentalMethod = jest.fn().mockReturnValue(Uint8Array.from([0x22, 0x44, 0x88])); + })(), + }; + + public constructor(moduleApi: ModuleApi) { + super(moduleApi); + } +} + export function registerMockModuleWithCryptoSetupExtension(): MockModuleWithCryptoSetupExtension { let module: MockModuleWithCryptoSetupExtension | undefined; @@ -86,3 +105,19 @@ export function registerMockModuleWithCryptoSetupExtension(): MockModuleWithCryp } return module; } + +export function registerMockModuleWithExperimentalExtension(): MockModuleWithExperimentalExtension { + let module: MockModuleWithExperimentalExtension | undefined; + + ModuleRunner.instance.registerModule((api) => { + if (module) { + throw new Error("State machine error: ModuleRunner created the module twice"); + } + module = new MockModuleWithExperimentalExtension(api); + return module; + }); + if (!module) { + throw new Error("State machine error: ModuleRunner did not create module"); + } + return module; +} diff --git a/test/modules/ModuleRunner-test.ts b/test/modules/ModuleRunner-test.ts index fd57eac2c21..f652c8759e7 100644 --- a/test/modules/ModuleRunner-test.ts +++ b/test/modules/ModuleRunner-test.ts @@ -16,7 +16,7 @@ limitations under the License. import { RoomPreviewOpts, RoomViewLifecycle } from "@matrix-org/react-sdk-module-api/lib/lifecycles/RoomViewLifecycle"; -import { MockModule, registerMockModule, registerMockModuleWithCryptoSetupExtension } from "./MockModule"; +import { MockModule, registerMockModule, registerMockModuleWithCryptoSetupExtension, registerMockModuleWithExperimentalExtension } from "./MockModule"; import { ModuleRunner } from "../../src/modules/ModuleRunner"; describe("ModuleRunner", () => { @@ -69,6 +69,12 @@ describe("ModuleRunner", () => { expect(result).toEqual(Uint8Array.from([0x11, 0x22, 0x99])); }); + it("should return value from experimental-extensions provided by a registered module", async () => { + registerMockModuleWithExperimentalExtension(); + const result = ModuleRunner.instance.extensions.experimental?.experimentalMethod(); + expect(result).toEqual(Uint8Array.from([0x22, 0x44, 0x88])); + }); + it("must not allow multiple modules to provide a given extension", async () => { registerMockModuleWithCryptoSetupExtension(); const t = () => registerMockModuleWithCryptoSetupExtension(); From 9e5b543a2a4d7ddd3767e1c7bc0331d6746f398d Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Sun, 7 Apr 2024 16:19:41 +0200 Subject: [PATCH 14/34] Fix linting and code-style issues --- test/modules/MockModule.ts | 4 ++-- test/modules/ModuleRunner-test.ts | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/test/modules/MockModule.ts b/test/modules/MockModule.ts index 0e57b060bb3..ba08dd95706 100644 --- a/test/modules/MockModule.ts +++ b/test/modules/MockModule.ts @@ -18,9 +18,9 @@ import { RuntimeModule } from "@matrix-org/react-sdk-module-api/lib/RuntimeModul import { ModuleApi } from "@matrix-org/react-sdk-module-api/lib/ModuleApi"; import { AllExtensions } from "@matrix-org/react-sdk-module-api/lib/types/extensions"; import { CryptoSetupExtensionsBase } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; +import { ExperimentalExtensionsBase } from "@matrix-org/react-sdk-module-api/lib/lifecycles/ExperimentalExtensions"; import { ModuleRunner } from "../../src/modules/ModuleRunner"; -import { ExperimentalExtensionsBase } from "@matrix-org/react-sdk-module-api/lib/lifecycles/ExperimentalExtensions"; export class MockModule extends RuntimeModule { public get apiInstance(): ModuleApi { @@ -81,7 +81,7 @@ export class MockModuleWithExperimentalExtension extends RuntimeModule { extensions: AllExtensions = { experimental: new (class extends ExperimentalExtensionsBase { - experimentalMethod = jest.fn().mockReturnValue(Uint8Array.from([0x22, 0x44, 0x88])); + experimentalMethod = jest.fn().mockReturnValue(Uint8Array.from([0x22, 0x44, 0x88])); })(), }; diff --git a/test/modules/ModuleRunner-test.ts b/test/modules/ModuleRunner-test.ts index f652c8759e7..f7b431df3f7 100644 --- a/test/modules/ModuleRunner-test.ts +++ b/test/modules/ModuleRunner-test.ts @@ -16,7 +16,12 @@ limitations under the License. import { RoomPreviewOpts, RoomViewLifecycle } from "@matrix-org/react-sdk-module-api/lib/lifecycles/RoomViewLifecycle"; -import { MockModule, registerMockModule, registerMockModuleWithCryptoSetupExtension, registerMockModuleWithExperimentalExtension } from "./MockModule"; +import { + MockModule, + registerMockModule, + registerMockModuleWithCryptoSetupExtension, + registerMockModuleWithExperimentalExtension, +} from "./MockModule"; import { ModuleRunner } from "../../src/modules/ModuleRunner"; describe("ModuleRunner", () => { From d1acc5f3b4265f375d0edde7b1d1cc225b3f1547 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Sun, 7 Apr 2024 16:44:40 +0200 Subject: [PATCH 15/34] Added test to ensure only on registration of experimental extensions --- test/modules/ModuleRunner-test.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/modules/ModuleRunner-test.ts b/test/modules/ModuleRunner-test.ts index f7b431df3f7..07f16d67613 100644 --- a/test/modules/ModuleRunner-test.ts +++ b/test/modules/ModuleRunner-test.ts @@ -80,7 +80,7 @@ describe("ModuleRunner", () => { expect(result).toEqual(Uint8Array.from([0x22, 0x44, 0x88])); }); - it("must not allow multiple modules to provide a given extension", async () => { + it("must not allow multiple modules to provide cryptoSetup extension", async () => { registerMockModuleWithCryptoSetupExtension(); const t = () => registerMockModuleWithCryptoSetupExtension(); expect(t).toThrow(Error); @@ -88,5 +88,14 @@ describe("ModuleRunner", () => { "adding cryptoSetup extension implementation from module MockModuleWithCryptoSetupExtension but an implementation was already provided", ); }); + + it("must not allow multiple modules to provide experimental extension", async () => { + registerMockModuleWithExperimentalExtension(); + const t = () => registerMockModuleWithExperimentalExtension(); + expect(t).toThrow(Error); + expect(t).toThrow( + "adding experimental extension implementation from module MockModuleWithExperimentalExtension but an implementation was already provided", + ); + }); }); }); From f6fc5d8f98a6089a51428dee90d5e06d88d7cbe2 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Sun, 7 Apr 2024 18:53:25 +0200 Subject: [PATCH 16/34] Added test toensure call to getDehydratedDeviceCallback() --- test/MatrixClientPeg-test.ts | 42 ++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts index b94ec7cb018..55d51dc1c70 100644 --- a/test/MatrixClientPeg-test.ts +++ b/test/MatrixClientPeg-test.ts @@ -17,6 +17,10 @@ limitations under the License. import { logger } from "matrix-js-sdk/src/logger"; import fetchMockJest from "fetch-mock-jest"; import EventEmitter from "events"; +import { + CryptoSetupExtensionsBase, + ProvideCryptoSetupExtensions, +} from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; import { advanceDateAndTime, stubClient } from "./test-utils"; import { IMatrixClientPeg, MatrixClientPeg as peg } from "../src/MatrixClientPeg"; @@ -25,6 +29,7 @@ import Modal from "../src/Modal"; import PlatformPeg from "../src/PlatformPeg"; import { SettingLevel } from "../src/settings/SettingLevel"; import { Features } from "../src/settings/Settings"; +import { ModuleRunner } from "../src/modules/ModuleRunner"; jest.useFakeTimers(); @@ -77,6 +82,43 @@ describe("MatrixClientPeg", () => { expect(peg.userRegisteredWithinLastHours(24)).toBe(false); }); + describe(".start extensions", () => { + let testPeg: IMatrixClientPeg; + + beforeEach(() => { + // instantiate a MatrixClientPegClass instance, with a new MatrixClient + testPeg = new PegClass(); + fetchMockJest.get("http://example.com/_matrix/client/versions", {}); + }); + + describe("cryptoSetup extension", () => { + it("should call cryptoSetup.getDehydrationKeyCallback", async () => { + const mockCryptoSetup = new (class extends CryptoSetupExtensionsBase { + SHOW_ENCRYPTION_SETUP_UI = true; + examineLoginResponse = jest.fn(); + persistCredentials = jest.fn(); + getSecretStorageKey = jest.fn().mockReturnValue(Uint8Array.from([0x11, 0x22, 0x33])); + createSecretStorageKey = jest.fn(); + catchAccessSecretStorageError = jest.fn(); + setupEncryptionNeeded = jest.fn(); + getDehydrationKeyCallback = jest.fn(); + })() as ProvideCryptoSetupExtensions; + + // Ensure we have an instance before we set up spies + const instance = ModuleRunner.instance; + jest.spyOn(instance.extensions, "cryptoSetup", "get").mockReturnValue(mockCryptoSetup); + + testPeg.replaceUsingCreds({ + accessToken: "SEKRET", + homeserverUrl: "http://example.com", + userId: "@user:example.com", + deviceId: "TEST_DEVICE_ID", + }); + expect(mockCryptoSetup.getDehydrationKeyCallback).toHaveBeenCalledTimes(1); + }); + }); + }); + describe(".start", () => { let testPeg: IMatrixClientPeg; From a7c73886ed230069c226ca486cde59444fbd8897 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Sun, 7 Apr 2024 19:38:31 +0200 Subject: [PATCH 17/34] Test what happens when there is no implementation --- test/MatrixClientPeg-test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts index 55d51dc1c70..b5dfb56f950 100644 --- a/test/MatrixClientPeg-test.ts +++ b/test/MatrixClientPeg-test.ts @@ -108,6 +108,30 @@ describe("MatrixClientPeg", () => { const instance = ModuleRunner.instance; jest.spyOn(instance.extensions, "cryptoSetup", "get").mockReturnValue(mockCryptoSetup); + testPeg.replaceUsingCreds({ + accessToken: "SEKRET", + homeserverUrl: "http://example.com", + userId: "@user:example.com", + deviceId: "TEST_DEVICE_ID", + }); + expect(mockCryptoSetup.getDehydrationKeyCallback).toHaveBeenCalledTimes(1); + }); + it("should fallback to default when cryptoSetup.getDehydrationKeyCallback return null", async () => { + const mockCryptoSetup = new (class extends CryptoSetupExtensionsBase { + SHOW_ENCRYPTION_SETUP_UI = true; + examineLoginResponse = jest.fn(); + persistCredentials = jest.fn(); + getSecretStorageKey = jest.fn().mockReturnValue(null); + createSecretStorageKey = jest.fn(); + catchAccessSecretStorageError = jest.fn(); + setupEncryptionNeeded = jest.fn(); + getDehydrationKeyCallback = jest.fn(); + })() as ProvideCryptoSetupExtensions; + + // Ensure we have an instance before we set up spies + const instance = ModuleRunner.instance; + jest.spyOn(instance.extensions, "cryptoSetup", "get").mockReturnValue(mockCryptoSetup); + testPeg.replaceUsingCreds({ accessToken: "SEKRET", homeserverUrl: "http://example.com", From 5f506e7f97d30bab6fc92f5cce7e6db5a21759c5 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Mon, 8 Apr 2024 08:28:49 +0200 Subject: [PATCH 18/34] Iterating cryptoSetup tests --- test/MatrixClientPeg-test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts index b5dfb56f950..bb5a870bd26 100644 --- a/test/MatrixClientPeg-test.ts +++ b/test/MatrixClientPeg-test.ts @@ -92,12 +92,12 @@ describe("MatrixClientPeg", () => { }); describe("cryptoSetup extension", () => { - it("should call cryptoSetup.getDehydrationKeyCallback", async () => { + it("should call default cryptoSetup.getDehydrationKeyCallback", async () => { const mockCryptoSetup = new (class extends CryptoSetupExtensionsBase { SHOW_ENCRYPTION_SETUP_UI = true; examineLoginResponse = jest.fn(); persistCredentials = jest.fn(); - getSecretStorageKey = jest.fn().mockReturnValue(Uint8Array.from([0x11, 0x22, 0x33])); + getSecretStorageKey = jest.fn(); createSecretStorageKey = jest.fn(); catchAccessSecretStorageError = jest.fn(); setupEncryptionNeeded = jest.fn(); @@ -116,16 +116,16 @@ describe("MatrixClientPeg", () => { }); expect(mockCryptoSetup.getDehydrationKeyCallback).toHaveBeenCalledTimes(1); }); - it("should fallback to default when cryptoSetup.getDehydrationKeyCallback return null", async () => { + it("should call overridden cryptoSetup.getDehydrationKeyCallback", async () => { const mockCryptoSetup = new (class extends CryptoSetupExtensionsBase { SHOW_ENCRYPTION_SETUP_UI = true; examineLoginResponse = jest.fn(); persistCredentials = jest.fn(); - getSecretStorageKey = jest.fn().mockReturnValue(null); + getSecretStorageKey = jest.fn(); createSecretStorageKey = jest.fn(); catchAccessSecretStorageError = jest.fn(); setupEncryptionNeeded = jest.fn(); - getDehydrationKeyCallback = jest.fn(); + getDehydrationKeyCallback = jest.fn().mockReturnValue(() => Uint8Array.from([0x11, 0x22, 0x33])); })() as ProvideCryptoSetupExtensions; // Ensure we have an instance before we set up spies From c55e3ebe8b82d762795c9c907351216207705fff Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Mon, 8 Apr 2024 08:57:17 +0200 Subject: [PATCH 19/34] Lint/prettier fix --- test/MatrixClientPeg-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts index bb5a870bd26..39cd13266d1 100644 --- a/test/MatrixClientPeg-test.ts +++ b/test/MatrixClientPeg-test.ts @@ -118,7 +118,7 @@ describe("MatrixClientPeg", () => { }); it("should call overridden cryptoSetup.getDehydrationKeyCallback", async () => { const mockCryptoSetup = new (class extends CryptoSetupExtensionsBase { - SHOW_ENCRYPTION_SETUP_UI = true; + SHOW_ENCRYPTION_SETUP_UI = true; examineLoginResponse = jest.fn(); persistCredentials = jest.fn(); getSecretStorageKey = jest.fn(); From de57bc7bf1f0835056bb06a094201b8b68ee6c04 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Mon, 8 Apr 2024 12:13:51 +0200 Subject: [PATCH 20/34] Assert both branches when checking for dehydrationkey callback --- test/MatrixClientPeg-test.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts index 39cd13266d1..2ae72d084b6 100644 --- a/test/MatrixClientPeg-test.ts +++ b/test/MatrixClientPeg-test.ts @@ -20,6 +20,7 @@ import EventEmitter from "events"; import { CryptoSetupExtensionsBase, ProvideCryptoSetupExtensions, + SecretStorageKeyDescription, } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; import { advanceDateAndTime, stubClient } from "./test-utils"; @@ -101,7 +102,7 @@ describe("MatrixClientPeg", () => { createSecretStorageKey = jest.fn(); catchAccessSecretStorageError = jest.fn(); setupEncryptionNeeded = jest.fn(); - getDehydrationKeyCallback = jest.fn(); + getDehydrationKeyCallback = jest.fn().mockReturnValue(null); })() as ProvideCryptoSetupExtensions; // Ensure we have an instance before we set up spies @@ -114,9 +115,13 @@ describe("MatrixClientPeg", () => { userId: "@user:example.com", deviceId: "TEST_DEVICE_ID", }); + expect(mockCryptoSetup.getDehydrationKeyCallback).toHaveBeenCalledTimes(1); }); + it("should call overridden cryptoSetup.getDehydrationKeyCallback", async () => { + const mockDehydrationKeyCallback = () => Uint8Array.from([0x11, 0x22, 0x33]); + const mockCryptoSetup = new (class extends CryptoSetupExtensionsBase { SHOW_ENCRYPTION_SETUP_UI = true; examineLoginResponse = jest.fn(); @@ -125,7 +130,7 @@ describe("MatrixClientPeg", () => { createSecretStorageKey = jest.fn(); catchAccessSecretStorageError = jest.fn(); setupEncryptionNeeded = jest.fn(); - getDehydrationKeyCallback = jest.fn().mockReturnValue(() => Uint8Array.from([0x11, 0x22, 0x33])); + getDehydrationKeyCallback = jest.fn().mockReturnValue(mockDehydrationKeyCallback); })() as ProvideCryptoSetupExtensions; // Ensure we have an instance before we set up spies @@ -139,6 +144,13 @@ describe("MatrixClientPeg", () => { deviceId: "TEST_DEVICE_ID", }); expect(mockCryptoSetup.getDehydrationKeyCallback).toHaveBeenCalledTimes(1); + + const client = testPeg.get(); + const dehydrationKey = await client?.cryptoCallbacks.getDehydrationKey!( + {} as SecretStorageKeyDescription, + (key: Uint8Array) => true, + ); + expect(dehydrationKey).toEqual(Uint8Array.from([0x11, 0x22, 0x33])); }); }); }); From c70cfe9cd42b3d6505707fe6a836d9242bdfcd96 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Mon, 8 Apr 2024 14:54:15 +0200 Subject: [PATCH 21/34] Update src/modules/ModuleRunner.ts Language and formatting Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/modules/ModuleRunner.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index a749d8ae20d..9dbb3915a4d 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -139,7 +139,7 @@ export class ModuleRunner { /** * Exposes all extensions which may be overridden/provided by modules * - * @returns En extensionsmanager which exposes the extensions + * @returns An `ExtensionsManager` which exposes the extensions. */ public get extensions(): ExtensionsManager { return this._extensions; From e5ca4f6a0504ccc405c31b43083df17e79f853c3 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Mon, 8 Apr 2024 14:56:18 +0200 Subject: [PATCH 22/34] Update src/modules/ModuleRunner.ts Reset by setting a fresh ExtensionsManager Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/modules/ModuleRunner.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index 9dbb3915a4d..87c17c10da9 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -152,7 +152,7 @@ export class ModuleRunner { */ public reset(): void { this.modules = []; - this._extensions.reset(); + this._extensions = new ExtensionsManager(); } /** From 3745e21f7c82e4893b53868d34efc341beccc21a Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Mon, 8 Apr 2024 14:57:24 +0200 Subject: [PATCH 23/34] Update src/modules/ModuleRunner.ts Use regular comment instead of TSDoc style comment Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/modules/ModuleRunner.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index 87c17c10da9..f4bdcb33461 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -187,9 +187,7 @@ export class ModuleRunner { this.modules.push(appModule); - /** - * Check if the new module provides any extensions, and also ensure a given extension is only provided by a single runtime module - */ + // Check if the new module provides any extensions, and also ensure a given extension is only provided by a single runtime module. this._extensions.addExtensions(appModule); } From fba6f3e4e8771b4ffed41a60b32bae16cb1e9c35 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Mon, 8 Apr 2024 14:59:20 +0200 Subject: [PATCH 24/34] Update test/MatrixClientPeg-test.ts No need to extend the base class Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- test/MatrixClientPeg-test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts index 2ae72d084b6..bfbaa27a955 100644 --- a/test/MatrixClientPeg-test.ts +++ b/test/MatrixClientPeg-test.ts @@ -94,7 +94,7 @@ describe("MatrixClientPeg", () => { describe("cryptoSetup extension", () => { it("should call default cryptoSetup.getDehydrationKeyCallback", async () => { - const mockCryptoSetup = new (class extends CryptoSetupExtensionsBase { + const mockCryptoSetup = { SHOW_ENCRYPTION_SETUP_UI = true; examineLoginResponse = jest.fn(); persistCredentials = jest.fn(); @@ -103,7 +103,7 @@ describe("MatrixClientPeg", () => { catchAccessSecretStorageError = jest.fn(); setupEncryptionNeeded = jest.fn(); getDehydrationKeyCallback = jest.fn().mockReturnValue(null); - })() as ProvideCryptoSetupExtensions; + } as ProvideCryptoSetupExtensions; // Ensure we have an instance before we set up spies const instance = ModuleRunner.instance; From a38267bbda2f5ef52887e47150e4f5c5203c5ac3 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Mon, 8 Apr 2024 15:00:07 +0200 Subject: [PATCH 25/34] Update src/modules/ModuleRunner.ts Fix spelling Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/modules/ModuleRunner.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index f4bdcb33461..9285596ed95 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -63,7 +63,7 @@ class ExtensionsManager { } /** - * Provides a n experimental extension. + * Provides an experimental extension. * * @remarks * This method extension is provided to simplify experimentaion an development, and is not intended for production code From a18c80880a44a3272e2fb6475a804bcdf0e1e2c1 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Mon, 8 Apr 2024 15:00:44 +0200 Subject: [PATCH 26/34] Update src/modules/ModuleRunner.ts Fix spelling Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/modules/ModuleRunner.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index 9285596ed95..2a7b9ee26b5 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -66,7 +66,7 @@ class ExtensionsManager { * Provides an experimental extension. * * @remarks - * This method extension is provided to simplify experimentaion an development, and is not intended for production code + * This method extension is provided to simplify experimentation and development, and is not intended for production code. * * @returns The registered extension. If no module provides this extension, a default implementation is returned */ From b939c2e58103dbda97d6da50e729cbf72cd79d95 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Mon, 8 Apr 2024 15:01:29 +0200 Subject: [PATCH 27/34] Update src/modules/ModuleRunner.ts Fix TSDoc formatting Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/modules/ModuleRunner.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index 2a7b9ee26b5..9b98d8edced 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -91,7 +91,6 @@ class ExtensionsManager { * @param module - The appModule to check for extensions * * @throws if an extension is provided by more than one module - * */ public addExtensions(module: AppModule): void { const runtimeModule = module.module; From 97f25ce6b3b1622228a649a987cfcc02e13a5bb4 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Mon, 8 Apr 2024 15:08:56 +0200 Subject: [PATCH 28/34] Simplify mock setup --- test/MatrixClientPeg-test.ts | 40 +++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts index bfbaa27a955..0e0ba8716eb 100644 --- a/test/MatrixClientPeg-test.ts +++ b/test/MatrixClientPeg-test.ts @@ -94,15 +94,16 @@ describe("MatrixClientPeg", () => { describe("cryptoSetup extension", () => { it("should call default cryptoSetup.getDehydrationKeyCallback", async () => { + const mockCryptoSetup = { - SHOW_ENCRYPTION_SETUP_UI = true; - examineLoginResponse = jest.fn(); - persistCredentials = jest.fn(); - getSecretStorageKey = jest.fn(); - createSecretStorageKey = jest.fn(); - catchAccessSecretStorageError = jest.fn(); - setupEncryptionNeeded = jest.fn(); - getDehydrationKeyCallback = jest.fn().mockReturnValue(null); + SHOW_ENCRYPTION_SETUP_UI: true, + examineLoginResponse: jest.fn(), + persistCredentials: jest.fn(), + getSecretStorageKey: jest.fn(), + createSecretStorageKey: jest.fn(), + catchAccessSecretStorageError: jest.fn(), + setupEncryptionNeeded: jest.fn(), + getDehydrationKeyCallback: jest.fn().mockReturnValue(null), } as ProvideCryptoSetupExtensions; // Ensure we have an instance before we set up spies @@ -119,19 +120,20 @@ describe("MatrixClientPeg", () => { expect(mockCryptoSetup.getDehydrationKeyCallback).toHaveBeenCalledTimes(1); }); - it("should call overridden cryptoSetup.getDehydrationKeyCallback", async () => { + it("should call overridden cryptoSetup.getDehydrationKeyCallback", async () => { + const mockDehydrationKeyCallback = () => Uint8Array.from([0x11, 0x22, 0x33]); - const mockCryptoSetup = new (class extends CryptoSetupExtensionsBase { - SHOW_ENCRYPTION_SETUP_UI = true; - examineLoginResponse = jest.fn(); - persistCredentials = jest.fn(); - getSecretStorageKey = jest.fn(); - createSecretStorageKey = jest.fn(); - catchAccessSecretStorageError = jest.fn(); - setupEncryptionNeeded = jest.fn(); - getDehydrationKeyCallback = jest.fn().mockReturnValue(mockDehydrationKeyCallback); - })() as ProvideCryptoSetupExtensions; + const mockCryptoSetup = { + SHOW_ENCRYPTION_SETUP_UI: true, + examineLoginResponse: jest.fn(), + persistCredentials: jest.fn(), + getSecretStorageKey: jest.fn(), + createSecretStorageKey: jest.fn(), + catchAccessSecretStorageError: jest.fn(), + setupEncryptionNeeded: jest.fn(), + getDehydrationKeyCallback: jest.fn().mockReturnValue(mockDehydrationKeyCallback); + } as ProvideCryptoSetupExtensions; // Ensure we have an instance before we set up spies const instance = ModuleRunner.instance; From fd9562f1137f3f24a6fa60182a6bfe1d217a7d3a Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Mon, 8 Apr 2024 16:17:24 +0200 Subject: [PATCH 29/34] Simplified mock and cleaned up a bit --- test/MatrixClientPeg-test.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts index 0e0ba8716eb..2ed08e0a21f 100644 --- a/test/MatrixClientPeg-test.ts +++ b/test/MatrixClientPeg-test.ts @@ -18,7 +18,6 @@ import { logger } from "matrix-js-sdk/src/logger"; import fetchMockJest from "fetch-mock-jest"; import EventEmitter from "events"; import { - CryptoSetupExtensionsBase, ProvideCryptoSetupExtensions, SecretStorageKeyDescription, } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; @@ -94,13 +93,12 @@ describe("MatrixClientPeg", () => { describe("cryptoSetup extension", () => { it("should call default cryptoSetup.getDehydrationKeyCallback", async () => { - const mockCryptoSetup = { SHOW_ENCRYPTION_SETUP_UI: true, examineLoginResponse: jest.fn(), persistCredentials: jest.fn(), getSecretStorageKey: jest.fn(), - createSecretStorageKey: jest.fn(), + createSecretStorageKey: jest.fn(), catchAccessSecretStorageError: jest.fn(), setupEncryptionNeeded: jest.fn(), getDehydrationKeyCallback: jest.fn().mockReturnValue(null), @@ -120,19 +118,18 @@ describe("MatrixClientPeg", () => { expect(mockCryptoSetup.getDehydrationKeyCallback).toHaveBeenCalledTimes(1); }); - it("should call overridden cryptoSetup.getDehydrationKeyCallback", async () => { - + it("should call overridden cryptoSetup.getDehydrationKeyCallback", async () => { const mockDehydrationKeyCallback = () => Uint8Array.from([0x11, 0x22, 0x33]); - const mockCryptoSetup = { + const mockCryptoSetup = { SHOW_ENCRYPTION_SETUP_UI: true, examineLoginResponse: jest.fn(), persistCredentials: jest.fn(), getSecretStorageKey: jest.fn(), - createSecretStorageKey: jest.fn(), + createSecretStorageKey: jest.fn(), catchAccessSecretStorageError: jest.fn(), setupEncryptionNeeded: jest.fn(), - getDehydrationKeyCallback: jest.fn().mockReturnValue(mockDehydrationKeyCallback); + getDehydrationKeyCallback: jest.fn().mockReturnValue(mockDehydrationKeyCallback), } as ProvideCryptoSetupExtensions; // Ensure we have an instance before we set up spies From 8e7a2b44cbcf0b05ef292ce85e03ee4c6703d9b7 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Mon, 8 Apr 2024 16:18:46 +0200 Subject: [PATCH 30/34] Keeping track of extensions is an implementation detail internal to ExtensionsManager. Language and punctuation --- src/modules/ModuleRunner.ts | 54 +++++++++++++++---------------------- 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index 9b98d8edced..a645ee9da6f 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -31,21 +31,22 @@ import { ModuleFactory } from "./ModuleFactory"; import "./ModuleComponents"; -class ExtensionImplementationMap { - public hasDefaultCryptoSetupExtension: boolean = true; - public hasDefaultExperimentalExtension: boolean = true; -} - /** - * Handles and manages any extensions provided by modules + * Handles and manages extensions provided by modules. */ -class ExtensionsManager { +export class ExtensionsManager { + // Private backing fields for extensions private _cryptoSetup: ProvideCryptoSetupExtensions; private _experimental: ProvideExperimentalExtensions; - private _implementionMap: ExtensionImplementationMap = new ExtensionImplementationMap(); + + // Map to keep track of which extensions has been provided by modules. + private implementionMap = { + hasDefaultCryptoSetupExtension: true, + hasDefaultExperimentalExtension: true, + }; /** - * Create a new instance + * Create a new instance. */ public constructor() { // Set up defaults @@ -56,7 +57,7 @@ class ExtensionsManager { /** * Provides a crypto setup extension. * - * @returns The registered extension. If no module provides this extension, a default implementation is returned + * @returns The registered extension. If no module provides this extension, a default implementation is returned. */ public get cryptoSetup(): ProvideCryptoSetupExtensions { return this._cryptoSetup; @@ -68,53 +69,42 @@ class ExtensionsManager { * @remarks * This method extension is provided to simplify experimentation and development, and is not intended for production code. * - * @returns The registered extension. If no module provides this extension, a default implementation is returned + * @returns The registered extension. If no module provides this extension, a default implementation is returned. */ public get experimental(): ProvideExperimentalExtensions { return this._experimental; } /** - * Resets the extension to the defaults - * - * Intended for test usage only. - */ - public reset(): void { - this._implementionMap = new ExtensionImplementationMap(); - this._cryptoSetup = new DefaultCryptoSetupExtensions(); - this._experimental = new DefaultExperimentalExtensions(); - } - - /** - * Add any extensions provided by the module + * Add any extensions provided by the module. * - * @param module - The appModule to check for extensions + * @param module - The appModule to check for extensions. * - * @throws if an extension is provided by more than one module + * @throws if an extension is provided by more than one module. */ public addExtensions(module: AppModule): void { const runtimeModule = module.module; /* Record the cryptoSetup extensions if any */ if (runtimeModule.extensions?.cryptoSetup) { - if (this._implementionMap.hasDefaultCryptoSetupExtension) { + if (this.implementionMap.hasDefaultCryptoSetupExtension) { this._cryptoSetup = runtimeModule.extensions?.cryptoSetup; - this._implementionMap.hasDefaultCryptoSetupExtension = false; + this.implementionMap.hasDefaultCryptoSetupExtension = false; } else { throw new Error( - `adding cryptoSetup extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided`, + `adding cryptoSetup extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided.`, ); } } /* Record the experimental extensions if any */ if (runtimeModule.extensions?.experimental) { - if (this._implementionMap.hasDefaultExperimentalExtension) { + if (this.implementionMap.hasDefaultExperimentalExtension) { this._experimental = runtimeModule.extensions?.experimental; - this._implementionMap.hasDefaultExperimentalExtension = false; + this.implementionMap.hasDefaultExperimentalExtension = false; } else { throw new Error( - `adding experimental extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided`, + `adding experimental extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided.`, ); } } @@ -136,7 +126,7 @@ export class ModuleRunner { } /** - * Exposes all extensions which may be overridden/provided by modules + * Exposes all extensions which may be overridden/provided by modules. * * @returns An `ExtensionsManager` which exposes the extensions. */ From dbf7c2906c5ea4134312a250e88e67672b38fa0e Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Tue, 9 Apr 2024 09:57:55 +0200 Subject: [PATCH 31/34] Addressed issues and comments from PR review --- src/Lifecycle.ts | 2 +- src/Login.ts | 2 +- src/MatrixClientPeg.ts | 2 +- src/SecurityManager.ts | 6 +-- .../security/CreateSecretStorageDialog.tsx | 2 +- src/components/structures/MatrixChat.tsx | 2 +- src/modules/ModuleRunner.ts | 30 ++++++------ src/toasts/SetupEncryptionToast.ts | 2 +- test/modules/MockModule.ts | 49 ++++++++++++------- test/modules/ModuleRunner-test.ts | 8 +-- 10 files changed, 60 insertions(+), 45 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 1f987509fba..61097c13c21 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -863,7 +863,7 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise void, ): Promise { - const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup?.getSecretStorageKey(); + const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup.getSecretStorageKey(); if (keyFromCustomisations) { logger.log("CryptoSetupExtension: Using key from extension (dehydration)"); return keyFromCustomisations; @@ -430,7 +430,7 @@ async function doAccessSecretStorage(func: () => Promise, forceReset: bool // inner operation completes. return await func(); } catch (e) { - ModuleRunner.instance.extensions.cryptoSetup?.catchAccessSecretStorageError(e as Error); + ModuleRunner.instance.extensions.cryptoSetup.catchAccessSecretStorageError(e as Error); logger.error(e); // Re-throw so that higher level logic can abort as needed throw e; diff --git a/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx b/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx index acb1c826368..be49b43851e 100644 --- a/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx @@ -180,7 +180,7 @@ export default class CreateSecretStorageDialog extends React.PureComponent { // private keys. const cryptoExtension = ModuleRunner.instance.extensions.cryptoSetup; - if (cryptoExtension !== undefined && cryptoExtension.SHOW_ENCRYPTION_SETUP_UI == false) { + if (cryptoExtension.SHOW_ENCRYPTION_SETUP_UI == false) { this.onLoggedIn(); } else { this.setStateForNewView({ view: Views.COMPLETE_SECURITY }); diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index a645ee9da6f..68d5ca6fc21 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -34,10 +34,10 @@ import "./ModuleComponents"; /** * Handles and manages extensions provided by modules. */ -export class ExtensionsManager { +class ExtensionsManager { // Private backing fields for extensions - private _cryptoSetup: ProvideCryptoSetupExtensions; - private _experimental: ProvideExperimentalExtensions; + private cryptoSetupExtension: ProvideCryptoSetupExtensions; + private experimentalExtension: ProvideExperimentalExtensions; // Map to keep track of which extensions has been provided by modules. private implementionMap = { @@ -50,8 +50,8 @@ export class ExtensionsManager { */ public constructor() { // Set up defaults - this._cryptoSetup = new DefaultCryptoSetupExtensions(); - this._experimental = new DefaultExperimentalExtensions(); + this.cryptoSetupExtension = new DefaultCryptoSetupExtensions(); + this.experimentalExtension = new DefaultExperimentalExtensions(); } /** @@ -60,7 +60,7 @@ export class ExtensionsManager { * @returns The registered extension. If no module provides this extension, a default implementation is returned. */ public get cryptoSetup(): ProvideCryptoSetupExtensions { - return this._cryptoSetup; + return this.cryptoSetupExtension; } /** @@ -72,7 +72,7 @@ export class ExtensionsManager { * @returns The registered extension. If no module provides this extension, a default implementation is returned. */ public get experimental(): ProvideExperimentalExtensions { - return this._experimental; + return this.experimentalExtension; } /** @@ -85,10 +85,10 @@ export class ExtensionsManager { public addExtensions(module: AppModule): void { const runtimeModule = module.module; - /* Record the cryptoSetup extensions if any */ + /* Record the cryptoSetup extension if any */ if (runtimeModule.extensions?.cryptoSetup) { if (this.implementionMap.hasDefaultCryptoSetupExtension) { - this._cryptoSetup = runtimeModule.extensions?.cryptoSetup; + this.cryptoSetupExtension = runtimeModule.extensions?.cryptoSetup; this.implementionMap.hasDefaultCryptoSetupExtension = false; } else { throw new Error( @@ -97,10 +97,10 @@ export class ExtensionsManager { } } - /* Record the experimental extensions if any */ + /* Record the experimental extension if any */ if (runtimeModule.extensions?.experimental) { if (this.implementionMap.hasDefaultExperimentalExtension) { - this._experimental = runtimeModule.extensions?.experimental; + this.experimentalExtension = runtimeModule.extensions?.experimental; this.implementionMap.hasDefaultExperimentalExtension = false; } else { throw new Error( @@ -117,7 +117,7 @@ export class ExtensionsManager { export class ModuleRunner { public static readonly instance = new ModuleRunner(); - private _extensions = new ExtensionsManager(); + private extensionsManager = new ExtensionsManager(); private modules: AppModule[] = []; @@ -131,7 +131,7 @@ export class ModuleRunner { * @returns An `ExtensionsManager` which exposes the extensions. */ public get extensions(): ExtensionsManager { - return this._extensions; + return this.extensionsManager; } /** @@ -141,7 +141,7 @@ export class ModuleRunner { */ public reset(): void { this.modules = []; - this._extensions = new ExtensionsManager(); + this.extensionsManager = new ExtensionsManager(); } /** @@ -177,7 +177,7 @@ export class ModuleRunner { this.modules.push(appModule); // Check if the new module provides any extensions, and also ensure a given extension is only provided by a single runtime module. - this._extensions.addExtensions(appModule); + this.extensionsManager.addExtensions(appModule); } /** diff --git a/src/toasts/SetupEncryptionToast.ts b/src/toasts/SetupEncryptionToast.ts index b2d37748551..3f78ad0c925 100644 --- a/src/toasts/SetupEncryptionToast.ts +++ b/src/toasts/SetupEncryptionToast.ts @@ -81,7 +81,7 @@ const onReject = (): void => { export const showToast = (kind: Kind): void => { if ( - ModuleRunner.instance.extensions.cryptoSetup?.setupEncryptionNeeded({ + ModuleRunner.instance.extensions.cryptoSetup.setupEncryptionNeeded({ kind: kind as any, storeProvider: { getInstance: () => SetupEncryptionStore.sharedInstance() }, }) diff --git a/test/modules/MockModule.ts b/test/modules/MockModule.ts index ba08dd95706..67c2aeeb7b9 100644 --- a/test/modules/MockModule.ts +++ b/test/modules/MockModule.ts @@ -17,8 +17,8 @@ limitations under the License. import { RuntimeModule } from "@matrix-org/react-sdk-module-api/lib/RuntimeModule"; import { ModuleApi } from "@matrix-org/react-sdk-module-api/lib/ModuleApi"; import { AllExtensions } from "@matrix-org/react-sdk-module-api/lib/types/extensions"; -import { CryptoSetupExtensionsBase } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; -import { ExperimentalExtensionsBase } from "@matrix-org/react-sdk-module-api/lib/lifecycles/ExperimentalExtensions"; +import { ProvideCryptoSetupExtensions } from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions"; +import { ProvideExperimentalExtensions } from "@matrix-org/react-sdk-module-api/lib/lifecycles/ExperimentalExtensions"; import { ModuleRunner } from "../../src/modules/ModuleRunner"; @@ -32,6 +32,11 @@ export class MockModule extends RuntimeModule { } } +/** + * Register a mock module + * + * @returns The registered module. + */ export function registerMockModule(): MockModule { let module: MockModule | undefined; ModuleRunner.instance.registerModule((api) => { @@ -47,7 +52,7 @@ export function registerMockModule(): MockModule { return module; } -export class MockModuleWithCryptoSetupExtension extends RuntimeModule { +class MockModuleWithCryptoSetupExtension extends RuntimeModule { public get apiInstance(): ModuleApi { return this.moduleApi; } @@ -55,16 +60,16 @@ export class MockModuleWithCryptoSetupExtension extends RuntimeModule { moduleName: string = MockModuleWithCryptoSetupExtension.name; extensions: AllExtensions = { - cryptoSetup: new (class extends CryptoSetupExtensionsBase { - SHOW_ENCRYPTION_SETUP_UI = true; - examineLoginResponse = jest.fn(); - persistCredentials = jest.fn(); - getSecretStorageKey = jest.fn().mockReturnValue(Uint8Array.from([0x11, 0x22, 0x99])); - createSecretStorageKey = jest.fn(); - catchAccessSecretStorageError = jest.fn(); - setupEncryptionNeeded = jest.fn(); - getDehydrationKeyCallback = jest.fn(); - })(), + cryptoSetup: { + SHOW_ENCRYPTION_SETUP_UI: true, + examineLoginResponse: jest.fn(), + persistCredentials: jest.fn(), + getSecretStorageKey: jest.fn().mockReturnValue(Uint8Array.from([0x11, 0x22, 0x99])), + createSecretStorageKey: jest.fn(), + catchAccessSecretStorageError: jest.fn(), + setupEncryptionNeeded: jest.fn(), + getDehydrationKeyCallback: jest.fn(), + } as ProvideCryptoSetupExtensions, }; public constructor(moduleApi: ModuleApi) { @@ -72,7 +77,7 @@ export class MockModuleWithCryptoSetupExtension extends RuntimeModule { } } -export class MockModuleWithExperimentalExtension extends RuntimeModule { +class MockModuleWithExperimentalExtension extends RuntimeModule { public get apiInstance(): ModuleApi { return this.moduleApi; } @@ -80,9 +85,9 @@ export class MockModuleWithExperimentalExtension extends RuntimeModule { moduleName: string = MockModuleWithExperimentalExtension.name; extensions: AllExtensions = { - experimental: new (class extends ExperimentalExtensionsBase { - experimentalMethod = jest.fn().mockReturnValue(Uint8Array.from([0x22, 0x44, 0x88])); - })(), + experimental: { + experimentalMethod: jest.fn().mockReturnValue(Uint8Array.from([0x22, 0x44, 0x88])), + } as ProvideExperimentalExtensions, }; public constructor(moduleApi: ModuleApi) { @@ -90,6 +95,11 @@ export class MockModuleWithExperimentalExtension extends RuntimeModule { } } +/** + * Register a mock module which implements the cryptoSetup extension. + * + * @returns The registered module. + */ export function registerMockModuleWithCryptoSetupExtension(): MockModuleWithCryptoSetupExtension { let module: MockModuleWithCryptoSetupExtension | undefined; @@ -106,6 +116,11 @@ export function registerMockModuleWithCryptoSetupExtension(): MockModuleWithCryp return module; } +/** + * Register a mock module which implements the experimental extension. + * + * @returns The registered module. + */ export function registerMockModuleWithExperimentalExtension(): MockModuleWithExperimentalExtension { let module: MockModuleWithExperimentalExtension | undefined; diff --git a/test/modules/ModuleRunner-test.ts b/test/modules/ModuleRunner-test.ts index 07f16d67613..27358ff88af 100644 --- a/test/modules/ModuleRunner-test.ts +++ b/test/modules/ModuleRunner-test.ts @@ -58,25 +58,25 @@ describe("ModuleRunner", () => { describe("extensions", () => { it("should return default values when no crypto-setup extensions are provided by a registered module", async () => { registerMockModule(); - const result = ModuleRunner.instance.extensions.cryptoSetup?.getSecretStorageKey(); + const result = ModuleRunner.instance.extensions.cryptoSetup.getSecretStorageKey(); expect(result).toBeNull(); }); it("should return default values when no experimental extensions are provided by a registered module", async () => { registerMockModule(); - const result = ModuleRunner.instance.extensions?.experimental?.experimentalMethod(); + const result = ModuleRunner.instance.extensions?.experimental.experimentalMethod(); expect(result).toBeNull(); }); it("should return value from crypto-setup-extensions provided by a registered module", async () => { registerMockModuleWithCryptoSetupExtension(); - const result = ModuleRunner.instance.extensions.cryptoSetup?.getSecretStorageKey(); + const result = ModuleRunner.instance.extensions.cryptoSetup.getSecretStorageKey(); expect(result).toEqual(Uint8Array.from([0x11, 0x22, 0x99])); }); it("should return value from experimental-extensions provided by a registered module", async () => { registerMockModuleWithExperimentalExtension(); - const result = ModuleRunner.instance.extensions.experimental?.experimentalMethod(); + const result = ModuleRunner.instance.extensions.experimental.experimentalMethod(); expect(result).toEqual(Uint8Array.from([0x22, 0x44, 0x88])); }); From 319dc091d5259f01fa70477cf8fa5152fa2c1841 Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Thu, 11 Apr 2024 15:18:11 +0200 Subject: [PATCH 32/34] Update src/modules/ModuleRunner.ts Keep the flags to track implementations as direct properties Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/modules/ModuleRunner.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index 68d5ca6fc21..7d6553ba5ec 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -39,12 +39,11 @@ class ExtensionsManager { private cryptoSetupExtension: ProvideCryptoSetupExtensions; private experimentalExtension: ProvideExperimentalExtensions; - // Map to keep track of which extensions has been provided by modules. - private implementionMap = { - hasDefaultCryptoSetupExtension: true, - hasDefaultExperimentalExtension: true, - }; + /** `true` if `cryptoSetupExtension` is the default implementation; `false` if it is implemented by a module. */ + private hasDefaultCryptoSetupExtension = true; + /** `true` if `experimentalExtension` is the default implementation; `false` if it is implemented by a module. */ + private hasDefaultExperimentalExtension: true, /** * Create a new instance. */ From 7c7d238eb532c7fc8da20f6f9304f6c82f90203d Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Thu, 11 Apr 2024 15:32:14 +0200 Subject: [PATCH 33/34] Fix flattening of implementation map --- src/modules/ModuleRunner.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index 7d6553ba5ec..cefe584437e 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -42,8 +42,8 @@ class ExtensionsManager { /** `true` if `cryptoSetupExtension` is the default implementation; `false` if it is implemented by a module. */ private hasDefaultCryptoSetupExtension = true; - /** `true` if `experimentalExtension` is the default implementation; `false` if it is implemented by a module. */ - private hasDefaultExperimentalExtension: true, + /** `true` if `experimentalExtension` is the default implementation; `false` if it is implemented by a module. */ + private hasDefaultExperimentalExtension = true; /** * Create a new instance. */ @@ -84,11 +84,11 @@ class ExtensionsManager { public addExtensions(module: AppModule): void { const runtimeModule = module.module; - /* Record the cryptoSetup extension if any */ + /* Add the cryptoSetup extension if any */ if (runtimeModule.extensions?.cryptoSetup) { - if (this.implementionMap.hasDefaultCryptoSetupExtension) { + if (this.hasDefaultCryptoSetupExtension) { this.cryptoSetupExtension = runtimeModule.extensions?.cryptoSetup; - this.implementionMap.hasDefaultCryptoSetupExtension = false; + this.hasDefaultCryptoSetupExtension = false; } else { throw new Error( `adding cryptoSetup extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided.`, @@ -96,11 +96,11 @@ class ExtensionsManager { } } - /* Record the experimental extension if any */ + /* Add the experimental extension if any */ if (runtimeModule.extensions?.experimental) { - if (this.implementionMap.hasDefaultExperimentalExtension) { + if (this.hasDefaultExperimentalExtension) { this.experimentalExtension = runtimeModule.extensions?.experimental; - this.implementionMap.hasDefaultExperimentalExtension = false; + this.hasDefaultExperimentalExtension = false; } else { throw new Error( `adding experimental extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided.`, From ee144718a3a37f30d324ba24c93d8e141a9b581d Mon Sep 17 00:00:00 2001 From: Thor Arne Johansen Date: Fri, 12 Apr 2024 12:17:13 +0200 Subject: [PATCH 34/34] Update src/modules/ModuleRunner.ts Fix whitespace Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/modules/ModuleRunner.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/modules/ModuleRunner.ts b/src/modules/ModuleRunner.ts index cefe584437e..0c609f6f66a 100644 --- a/src/modules/ModuleRunner.ts +++ b/src/modules/ModuleRunner.ts @@ -44,6 +44,7 @@ class ExtensionsManager { /** `true` if `experimentalExtension` is the default implementation; `false` if it is implemented by a module. */ private hasDefaultExperimentalExtension = true; + /** * Create a new instance. */