Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Commit 6392759

Browse files
thorajrichvdh
thoraj
and
richvdh
authored
Replace SecurityCustomisations with CryptoSetupExtension (#12342)
* Changed call sites from customisations/security to ModuleRunner.extensions * Updated depenndecy and added tests * Fixed style and formatting with prettier * Fix according to Element PR comments * Fixing issues raised in PR review * Removed commented code. Improved encapsulation. Removed noisy logging * Improved language of comment about calling the factory * Refactor to get better encapsulation * Find a better name. Provide explicit reset function. Provide more TSDoc * Simplify mock for cryptoSetup, and add assertion for exception message. * Remove unused className property. Adjust TSDoc comments * Fix linting and code style issues * Added test to ensure we canregister anduse experimental extensions * Fix linting and code-style issues * Added test to ensure only on registration of experimental extensions * Added test toensure call to getDehydratedDeviceCallback() * Test what happens when there is no implementation * Iterating cryptoSetup tests * Lint/prettier fix * Assert both branches when checking for dehydrationkey callback * Update src/modules/ModuleRunner.ts Language and formatting Co-authored-by: Richard van der Hoff <[email protected]> * Update src/modules/ModuleRunner.ts Reset by setting a fresh ExtensionsManager Co-authored-by: Richard van der Hoff <[email protected]> * Update src/modules/ModuleRunner.ts Use regular comment instead of TSDoc style comment Co-authored-by: Richard van der Hoff <[email protected]> * Update test/MatrixClientPeg-test.ts No need to extend the base class Co-authored-by: Richard van der Hoff <[email protected]> * Update src/modules/ModuleRunner.ts Fix spelling Co-authored-by: Richard van der Hoff <[email protected]> * Update src/modules/ModuleRunner.ts Fix spelling Co-authored-by: Richard van der Hoff <[email protected]> * Update src/modules/ModuleRunner.ts Fix TSDoc formatting Co-authored-by: Richard van der Hoff <[email protected]> * Simplify mock setup * Simplified mock and cleaned up a bit * Keeping track of extensions is an implementation detail internal to ExtensionsManager. Language and punctuation * Addressed issues and comments from PR review * Update src/modules/ModuleRunner.ts Keep the flags to track implementations as direct properties Co-authored-by: Richard van der Hoff <[email protected]> * Fix flattening of implementation map * Update src/modules/ModuleRunner.ts Fix whitespace Co-authored-by: Richard van der Hoff <[email protected]> --------- Co-authored-by: Richard van der Hoff <[email protected]>
1 parent 313b556 commit 6392759

File tree

13 files changed

+361
-28
lines changed

13 files changed

+361
-28
lines changed

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@
7171
"@matrix-org/emojibase-bindings": "^1.1.2",
7272
"@matrix-org/matrix-wysiwyg": "2.17.0",
7373
"@matrix-org/olm": "3.2.15",
74-
"@matrix-org/react-sdk-module-api": "^2.3.0",
74+
"@matrix-org/react-sdk-module-api": "^2.4.0",
7575
"@matrix-org/spec": "^1.7.0",
7676
"@sentry/browser": "^7.0.0",
7777
"@testing-library/react-hooks": "^8.0.1",

‎src/Lifecycle.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import { QueryDict } from "matrix-js-sdk/src/utils";
2424
import { logger } from "matrix-js-sdk/src/logger";
2525

2626
import { IMatrixClientCreds, MatrixClientPeg } from "./MatrixClientPeg";
27-
import SecurityCustomisations from "./customisations/Security";
27+
import { ModuleRunner } from "./modules/ModuleRunner";
2828
import EventIndexPeg from "./indexing/EventIndexPeg";
2929
import createMatrixClient from "./utils/createMatrixClient";
3030
import Notifier from "./Notifier";
@@ -863,7 +863,7 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise<void
863863
localStorage.setItem("mx_device_id", credentials.deviceId);
864864
}
865865

866-
SecurityCustomisations.persistCredentials?.(credentials);
866+
ModuleRunner.instance.extensions.cryptoSetup?.persistCredentials(credentials);
867867

868868
logger.log(`Session persisted for ${credentials.userId}`);
869869
}

‎src/Login.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {
2727
import { logger } from "matrix-js-sdk/src/logger";
2828

2929
import { IMatrixClientCreds } from "./MatrixClientPeg";
30-
import SecurityCustomisations from "./customisations/Security";
30+
import { ModuleRunner } from "./modules/ModuleRunner";
3131
import { getOidcClientId } from "./utils/oidc/registerClient";
3232
import { IConfigOptions } from "./IConfigOptions";
3333
import SdkConfig from "./SdkConfig";
@@ -291,7 +291,7 @@ export async function sendLoginRequest(
291291
accessToken: data.access_token,
292292
};
293293

294-
SecurityCustomisations.examineLoginResponse?.(data, creds);
294+
ModuleRunner.instance.extensions.cryptoSetup.examineLoginResponse(data, creds);
295295

296296
return creds;
297297
}

‎src/MatrixClientPeg.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import MatrixClientBackedSettingsHandler from "./settings/handlers/MatrixClientB
4141
import * as StorageManager from "./utils/StorageManager";
4242
import IdentityAuthClient from "./IdentityAuthClient";
4343
import { crossSigningCallbacks, tryToUnlockSecretStorageWithDehydrationKey } from "./SecurityManager";
44-
import SecurityCustomisations from "./customisations/Security";
44+
import { ModuleRunner } from "./modules/ModuleRunner";
4545
import { SlidingSyncManager } from "./SlidingSyncManager";
4646
import CryptoStoreTooNewDialog from "./components/views/dialogs/CryptoStoreTooNewDialog";
4747
import { _t, UserFriendlyError } from "./languageHandler";
@@ -463,8 +463,9 @@ class MatrixClientPegClass implements IMatrixClientPeg {
463463
},
464464
};
465465

466-
if (SecurityCustomisations.getDehydrationKey) {
467-
opts.cryptoCallbacks!.getDehydrationKey = SecurityCustomisations.getDehydrationKey;
466+
const dehydrationKeyCallback = ModuleRunner.instance.extensions.cryptoSetup.getDehydrationKeyCallback();
467+
if (dehydrationKeyCallback) {
468+
opts.cryptoCallbacks!.getDehydrationKey = dehydrationKeyCallback;
468469
}
469470

470471
this.matrixClient = createMatrixClient(opts);

‎src/SecurityManager.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import { isSecureBackupRequired } from "./utils/WellKnownUtils";
3333
import AccessSecretStorageDialog, { KeyParams } from "./components/views/dialogs/security/AccessSecretStorageDialog";
3434
import RestoreKeyBackupDialog from "./components/views/dialogs/security/RestoreKeyBackupDialog";
3535
import SettingsStore from "./settings/SettingsStore";
36-
import SecurityCustomisations from "./customisations/Security";
36+
import { ModuleRunner } from "./modules/ModuleRunner";
3737
import QuestionDialog from "./components/views/dialogs/QuestionDialog";
3838
import InteractiveAuthDialog from "./components/views/dialogs/InteractiveAuthDialog";
3939

@@ -137,9 +137,9 @@ async function getSecretStorageKey({
137137
}
138138
}
139139

140-
const keyFromCustomisations = SecurityCustomisations.getSecretStorageKey?.();
140+
const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup.getSecretStorageKey();
141141
if (keyFromCustomisations) {
142-
logger.log("Using key from security customisations (secret storage)");
142+
logger.log("CryptoSetupExtension: Using key from extension (secret storage)");
143143
cacheSecretStorageKey(keyId, keyInfo, keyFromCustomisations);
144144
return [keyId, keyFromCustomisations];
145145
}
@@ -187,9 +187,9 @@ export async function getDehydrationKey(
187187
keyInfo: SecretStorage.SecretStorageKeyDescription,
188188
checkFunc: (data: Uint8Array) => void,
189189
): Promise<Uint8Array> {
190-
const keyFromCustomisations = SecurityCustomisations.getSecretStorageKey?.();
190+
const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup.getSecretStorageKey();
191191
if (keyFromCustomisations) {
192-
logger.log("Using key from security customisations (dehydration)");
192+
logger.log("CryptoSetupExtension: Using key from extension (dehydration)");
193193
return keyFromCustomisations;
194194
}
195195

@@ -430,7 +430,7 @@ async function doAccessSecretStorage(func: () => Promise<void>, forceReset: bool
430430
// inner operation completes.
431431
return await func();
432432
} catch (e) {
433-
SecurityCustomisations.catchAccessSecretStorageError?.(e);
433+
ModuleRunner.instance.extensions.cryptoSetup.catchAccessSecretStorageError(e as Error);
434434
logger.error(e);
435435
// Re-throw so that higher level logic can abort as needed
436436
throw e;

‎src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import {
4040
isSecureBackupRequired,
4141
SecureBackupSetupMethod,
4242
} from "../../../../utils/WellKnownUtils";
43-
import SecurityCustomisations from "../../../../customisations/Security";
43+
import { ModuleRunner } from "../../../../modules/ModuleRunner";
4444
import Field from "../../../../components/views/elements/Field";
4545
import BaseDialog from "../../../../components/views/dialogs/BaseDialog";
4646
import Spinner from "../../../../components/views/elements/Spinner";
@@ -180,9 +180,9 @@ export default class CreateSecretStorageDialog extends React.PureComponent<IProp
180180
}
181181

182182
private getInitialPhase(): void {
183-
const keyFromCustomisations = SecurityCustomisations.createSecretStorageKey?.();
183+
const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup.createSecretStorageKey();
184184
if (keyFromCustomisations) {
185-
logger.log("Created key via customisations, jumping to bootstrap step");
185+
logger.log("CryptoSetupExtension: Created key via extension, jumping to bootstrap step");
186186
this.recoveryKey = {
187187
privateKey: keyFromCustomisations,
188188
};

‎src/components/structures/MatrixChat.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ import { showToast as showMobileGuideToast } from "../../toasts/MobileGuideToast
8888
import { shouldUseLoginForWelcome } from "../../utils/pages";
8989
import RoomListStore from "../../stores/room-list/RoomListStore";
9090
import { RoomUpdateCause } from "../../stores/room-list/models";
91-
import SecurityCustomisations from "../../customisations/Security";
91+
import { ModuleRunner } from "../../modules/ModuleRunner";
9292
import Spinner from "../views/elements/Spinner";
9393
import QuestionDialog from "../views/dialogs/QuestionDialog";
9494
import UserSettingsDialog from "../views/dialogs/UserSettingsDialog";
@@ -442,7 +442,9 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
442442
if (crossSigningIsSetUp) {
443443
// if the user has previously set up cross-signing, verify this device so we can fetch the
444444
// private keys.
445-
if (SecurityCustomisations.SHOW_ENCRYPTION_SETUP_UI === false) {
445+
446+
const cryptoExtension = ModuleRunner.instance.extensions.cryptoSetup;
447+
if (cryptoExtension.SHOW_ENCRYPTION_SETUP_UI == false) {
446448
this.onLoggedIn();
447449
} else {
448450
this.setStateForNewView({ view: Views.COMPLETE_SECURITY });

‎src/modules/ModuleRunner.ts‎

Lines changed: 107 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,31 +17,131 @@ limitations under the License.
1717
import { safeSet } from "matrix-js-sdk/src/utils";
1818
import { TranslationStringsObject } from "@matrix-org/react-sdk-module-api/lib/types/translations";
1919
import { AnyLifecycle } from "@matrix-org/react-sdk-module-api/lib/lifecycles/types";
20+
import {
21+
DefaultCryptoSetupExtensions,
22+
ProvideCryptoSetupExtensions,
23+
} from "@matrix-org/react-sdk-module-api/lib/lifecycles/CryptoSetupExtensions";
24+
import {
25+
DefaultExperimentalExtensions,
26+
ProvideExperimentalExtensions,
27+
} from "@matrix-org/react-sdk-module-api/lib/lifecycles/ExperimentalExtensions";
2028

2129
import { AppModule } from "./AppModule";
2230
import { ModuleFactory } from "./ModuleFactory";
2331

2432
import "./ModuleComponents";
2533

34+
/**
35+
* Handles and manages extensions provided by modules.
36+
*/
37+
class ExtensionsManager {
38+
// Private backing fields for extensions
39+
private cryptoSetupExtension: ProvideCryptoSetupExtensions;
40+
private experimentalExtension: ProvideExperimentalExtensions;
41+
42+
/** `true` if `cryptoSetupExtension` is the default implementation; `false` if it is implemented by a module. */
43+
private hasDefaultCryptoSetupExtension = true;
44+
45+
/** `true` if `experimentalExtension` is the default implementation; `false` if it is implemented by a module. */
46+
private hasDefaultExperimentalExtension = true;
47+
48+
/**
49+
* Create a new instance.
50+
*/
51+
public constructor() {
52+
// Set up defaults
53+
this.cryptoSetupExtension = new DefaultCryptoSetupExtensions();
54+
this.experimentalExtension = new DefaultExperimentalExtensions();
55+
}
56+
57+
/**
58+
* Provides a crypto setup extension.
59+
*
60+
* @returns The registered extension. If no module provides this extension, a default implementation is returned.
61+
*/
62+
public get cryptoSetup(): ProvideCryptoSetupExtensions {
63+
return this.cryptoSetupExtension;
64+
}
65+
66+
/**
67+
* Provides an experimental extension.
68+
*
69+
* @remarks
70+
* This method extension is provided to simplify experimentation and development, and is not intended for production code.
71+
*
72+
* @returns The registered extension. If no module provides this extension, a default implementation is returned.
73+
*/
74+
public get experimental(): ProvideExperimentalExtensions {
75+
return this.experimentalExtension;
76+
}
77+
78+
/**
79+
* Add any extensions provided by the module.
80+
*
81+
* @param module - The appModule to check for extensions.
82+
*
83+
* @throws if an extension is provided by more than one module.
84+
*/
85+
public addExtensions(module: AppModule): void {
86+
const runtimeModule = module.module;
87+
88+
/* Add the cryptoSetup extension if any */
89+
if (runtimeModule.extensions?.cryptoSetup) {
90+
if (this.hasDefaultCryptoSetupExtension) {
91+
this.cryptoSetupExtension = runtimeModule.extensions?.cryptoSetup;
92+
this.hasDefaultCryptoSetupExtension = false;
93+
} else {
94+
throw new Error(
95+
`adding cryptoSetup extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided.`,
96+
);
97+
}
98+
}
99+
100+
/* Add the experimental extension if any */
101+
if (runtimeModule.extensions?.experimental) {
102+
if (this.hasDefaultExperimentalExtension) {
103+
this.experimentalExtension = runtimeModule.extensions?.experimental;
104+
this.hasDefaultExperimentalExtension = false;
105+
} else {
106+
throw new Error(
107+
`adding experimental extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided.`,
108+
);
109+
}
110+
}
111+
}
112+
}
113+
26114
/**
27115
* Handles and coordinates the operation of modules.
28116
*/
29117
export class ModuleRunner {
30118
public static readonly instance = new ModuleRunner();
31119

120+
private extensionsManager = new ExtensionsManager();
121+
32122
private modules: AppModule[] = [];
33123

34124
private constructor() {
35125
// we only want one instance
36126
}
37127

38128
/**
39-
* Resets the runner, clearing all known modules.
129+
* Exposes all extensions which may be overridden/provided by modules.
130+
*
131+
* @returns An `ExtensionsManager` which exposes the extensions.
132+
*/
133+
public get extensions(): ExtensionsManager {
134+
return this.extensionsManager;
135+
}
136+
137+
/**
138+
* Resets the runner, clearing all known modules, and all extensions
40139
*
41140
* Intended for test usage only.
42141
*/
43142
public reset(): void {
44143
this.modules = [];
144+
this.extensionsManager = new ExtensionsManager();
45145
}
46146

47147
/**
@@ -72,7 +172,12 @@ export class ModuleRunner {
72172
* @param factory The module factory.
73173
*/
74174
public registerModule(factory: ModuleFactory): void {
75-
this.modules.push(new AppModule(factory));
175+
const appModule = new AppModule(factory);
176+
177+
this.modules.push(appModule);
178+
179+
// Check if the new module provides any extensions, and also ensure a given extension is only provided by a single runtime module.
180+
this.extensionsManager.addExtensions(appModule);
76181
}
77182

78183
/**

‎src/toasts/SetupEncryptionToast.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ import SetupEncryptionDialog from "../components/views/dialogs/security/SetupEnc
2121
import { accessSecretStorage } from "../SecurityManager";
2222
import ToastStore from "../stores/ToastStore";
2323
import GenericToast from "../components/views/toasts/GenericToast";
24-
import SecurityCustomisations from "../customisations/Security";
24+
import { ModuleRunner } from "../modules/ModuleRunner";
25+
import { SetupEncryptionStore } from "../stores/SetupEncryptionStore";
2526
import Spinner from "../components/views/elements/Spinner";
2627

2728
const TOAST_KEY = "setupencryption";
@@ -79,7 +80,12 @@ const onReject = (): void => {
7980
};
8081

8182
export const showToast = (kind: Kind): void => {
82-
if (SecurityCustomisations.setupEncryptionNeeded?.(kind)) {
83+
if (
84+
ModuleRunner.instance.extensions.cryptoSetup.setupEncryptionNeeded({
85+
kind: kind as any,
86+
storeProvider: { getInstance: () => SetupEncryptionStore.sharedInstance() },
87+
})
88+
) {
8389
return;
8490
}
8591

0 commit comments

Comments
 (0)