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

Replace SecurityCustomisations with CryptoSetupExtension #12342

Merged
merged 41 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
328d301
Changed call sites from customisations/security to ModuleRunner.exten…
thoraj Feb 13, 2024
9b25236
Updated depenndecy and added tests
thoraj Mar 14, 2024
b84fe86
Fixed style and formatting with prettier
thoraj Mar 14, 2024
2bad8f1
Merge branch 'matrix-org:develop' into taj/cryptosetup
thoraj Mar 15, 2024
d3bbb4a
Fix according to Element PR comments
thoraj Apr 5, 2024
43aea4e
Fixing issues raised in PR review
thoraj Apr 5, 2024
78c7ea7
Removed commented code. Improved encapsulation. Removed noisy logging
thoraj Apr 6, 2024
c39c34d
Improved language of comment about calling the factory
thoraj Apr 6, 2024
b64f50c
Refactor to get better encapsulation
thoraj Apr 6, 2024
f7a2ec8
Find a better name. Provide explicit reset function. Provide more TSDoc
thoraj Apr 6, 2024
c905af8
Simplify mock for cryptoSetup, and add assertion for exception message.
thoraj Apr 7, 2024
6b79258
Merge branch 'matrix-org:develop' into taj/cryptosetup
thoraj Apr 7, 2024
158d977
Remove unused className property. Adjust TSDoc comments
thoraj Apr 7, 2024
5eccd0e
Fix linting and code style issues
thoraj Apr 7, 2024
f0973ef
Added test to ensure we canregister anduse experimental extensions
thoraj Apr 7, 2024
9e5b543
Fix linting and code-style issues
thoraj Apr 7, 2024
d1acc5f
Added test to ensure only on registration of experimental extensions
thoraj Apr 7, 2024
f6fc5d8
Added test toensure call to getDehydratedDeviceCallback()
thoraj Apr 7, 2024
a7c7388
Test what happens when there is no implementation
thoraj Apr 7, 2024
5f506e7
Iterating cryptoSetup tests
thoraj Apr 8, 2024
c55e3eb
Lint/prettier fix
thoraj Apr 8, 2024
de57bc7
Assert both branches when checking for dehydrationkey callback
thoraj Apr 8, 2024
a36e00e
Merge branch 'matrix-org:develop' into taj/cryptosetup
thoraj Apr 8, 2024
c70cfe9
Update src/modules/ModuleRunner.ts
thoraj Apr 8, 2024
e5ca4f6
Update src/modules/ModuleRunner.ts
thoraj Apr 8, 2024
3745e21
Update src/modules/ModuleRunner.ts
thoraj Apr 8, 2024
fba6f3e
Update test/MatrixClientPeg-test.ts
thoraj Apr 8, 2024
a38267b
Update src/modules/ModuleRunner.ts
thoraj Apr 8, 2024
a18c808
Update src/modules/ModuleRunner.ts
thoraj Apr 8, 2024
b939c2e
Update src/modules/ModuleRunner.ts
thoraj Apr 8, 2024
97f25ce
Simplify mock setup
thoraj Apr 8, 2024
fd9562f
Simplified mock and cleaned up a bit
thoraj Apr 8, 2024
8e7a2b4
Keeping track of extensions is an implementation detail internal to E…
thoraj Apr 8, 2024
82b4f82
Merge branch 'develop' into taj/cryptosetup
thoraj Apr 9, 2024
dbf7c29
Addressed issues and comments from PR review
thoraj Apr 9, 2024
c81ca70
Merge branch 'develop' into taj/cryptosetup
thoraj Apr 9, 2024
319dc09
Update src/modules/ModuleRunner.ts
thoraj Apr 11, 2024
dff0368
Merge branch 'develop' into taj/cryptosetup
thoraj Apr 11, 2024
7c7d238
Fix flattening of implementation map
thoraj Apr 11, 2024
ee14471
Update src/modules/ModuleRunner.ts
thoraj Apr 12, 2024
2ddf31f
Merge branch 'develop' into taj/cryptosetup
thoraj Apr 12, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions src/Lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,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";
Expand Down Expand Up @@ -863,7 +863,7 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise<void
localStorage.setItem("mx_device_id", credentials.deviceId);
}

SecurityCustomisations.persistCredentials?.(credentials);
ModuleRunner.instance.extensions.cryptoSetup?.persistCredentials(credentials);

logger.log(`Session persisted for ${credentials.userId}`);
}
Expand Down
4 changes: 2 additions & 2 deletions src/Login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
import { logger } from "matrix-js-sdk/src/logger";

import { IMatrixClientCreds } from "./MatrixClientPeg";
import SecurityCustomisations from "./customisations/Security";
import { ModuleRunner } from "./modules/ModuleRunner";
import { getOidcClientId } from "./utils/oidc/registerClient";
import { IConfigOptions } from "./IConfigOptions";
import SdkConfig from "./SdkConfig";
Expand Down Expand Up @@ -291,7 +291,7 @@ export async function sendLoginRequest(
accessToken: data.access_token,
};

SecurityCustomisations.examineLoginResponse?.(data, creds);
ModuleRunner.instance.extensions.cryptoSetup.examineLoginResponse(data, creds);

return creds;
}
7 changes: 4 additions & 3 deletions src/MatrixClientPeg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import MatrixClientBackedSettingsHandler from "./settings/handlers/MatrixClientB
import * as StorageManager from "./utils/StorageManager";
import IdentityAuthClient from "./IdentityAuthClient";
import { crossSigningCallbacks, tryToUnlockSecretStorageWithDehydrationKey } from "./SecurityManager";
import SecurityCustomisations from "./customisations/Security";
import { ModuleRunner } from "./modules/ModuleRunner";
import { SlidingSyncManager } from "./SlidingSyncManager";
import CryptoStoreTooNewDialog from "./components/views/dialogs/CryptoStoreTooNewDialog";
import { _t, UserFriendlyError } from "./languageHandler";
Expand Down Expand Up @@ -463,8 +463,9 @@ class MatrixClientPegClass implements IMatrixClientPeg {
},
};

if (SecurityCustomisations.getDehydrationKey) {
opts.cryptoCallbacks!.getDehydrationKey = SecurityCustomisations.getDehydrationKey;
const dehydrationKeyCallback = ModuleRunner.instance.extensions.cryptoSetup.getDehydrationKeyCallback();
if (dehydrationKeyCallback) {
opts.cryptoCallbacks!.getDehydrationKey = dehydrationKeyCallback;
}

this.matrixClient = createMatrixClient(opts);
Expand Down
12 changes: 6 additions & 6 deletions src/SecurityManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { isSecureBackupRequired } from "./utils/WellKnownUtils";
import AccessSecretStorageDialog, { KeyParams } from "./components/views/dialogs/security/AccessSecretStorageDialog";
import RestoreKeyBackupDialog from "./components/views/dialogs/security/RestoreKeyBackupDialog";
import SettingsStore from "./settings/SettingsStore";
import SecurityCustomisations from "./customisations/Security";
import { ModuleRunner } from "./modules/ModuleRunner";
import QuestionDialog from "./components/views/dialogs/QuestionDialog";
import InteractiveAuthDialog from "./components/views/dialogs/InteractiveAuthDialog";

Expand Down Expand Up @@ -137,9 +137,9 @@ async function getSecretStorageKey({
}
}

const keyFromCustomisations = SecurityCustomisations.getSecretStorageKey?.();
const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup.getSecretStorageKey();
if (keyFromCustomisations) {
logger.log("Using key from security customisations (secret storage)");
logger.log("CryptoSetupExtension: Using key from extension (secret storage)");
cacheSecretStorageKey(keyId, keyInfo, keyFromCustomisations);
return [keyId, keyFromCustomisations];
}
Expand Down Expand Up @@ -187,9 +187,9 @@ export async function getDehydrationKey(
keyInfo: SecretStorage.SecretStorageKeyDescription,
checkFunc: (data: Uint8Array) => void,
): Promise<Uint8Array> {
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;
}

Expand Down Expand Up @@ -430,7 +430,7 @@ async function doAccessSecretStorage(func: () => Promise<void>, forceReset: bool
// inner operation completes.
return await func();
} catch (e) {
SecurityCustomisations.catchAccessSecretStorageError?.(e);
ModuleRunner.instance.extensions.cryptoSetup.catchAccessSecretStorageError(e as Error);
logger.error(e);
// Re-throw so that higher level logic can abort as needed
throw e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,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";
Expand Down Expand Up @@ -180,9 +180,9 @@ export default class CreateSecretStorageDialog extends React.PureComponent<IProp
}

private getInitialPhase(): void {
const keyFromCustomisations = SecurityCustomisations.createSecretStorageKey?.();
const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup.createSecretStorageKey();
if (keyFromCustomisations) {
logger.log("Created key via customisations, jumping to bootstrap step");
logger.log("CryptoSetupExtension: Created key via extension, jumping to bootstrap step");
this.recoveryKey = {
privateKey: keyFromCustomisations,
};
Expand Down
6 changes: 4 additions & 2 deletions src/components/structures/MatrixChat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ import { showToast as showMobileGuideToast } from "../../toasts/MobileGuideToast
import { shouldUseLoginForWelcome } from "../../utils/pages";
import RoomListStore from "../../stores/room-list/RoomListStore";
import { RoomUpdateCause } from "../../stores/room-list/models";
import SecurityCustomisations from "../../customisations/Security";
import { ModuleRunner } from "../../modules/ModuleRunner";
import Spinner from "../views/elements/Spinner";
import QuestionDialog from "../views/dialogs/QuestionDialog";
import UserSettingsDialog from "../views/dialogs/UserSettingsDialog";
Expand Down Expand Up @@ -442,7 +442,9 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
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) {

const cryptoExtension = ModuleRunner.instance.extensions.cryptoSetup;
if (cryptoExtension.SHOW_ENCRYPTION_SETUP_UI == false) {
this.onLoggedIn();
} else {
this.setStateForNewView({ view: Views.COMPLETE_SECURITY });
Expand Down
108 changes: 106 additions & 2 deletions src/modules/ModuleRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,130 @@ 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 { AppModule } from "./AppModule";
import { ModuleFactory } from "./ModuleFactory";

import "./ModuleComponents";

/**
* Handles and manages extensions provided by modules.
*/
class ExtensionsManager {
richvdh marked this conversation as resolved.
Show resolved Hide resolved
// Private backing fields for extensions
private cryptoSetupExtension: ProvideCryptoSetupExtensions;
private experimentalExtension: ProvideExperimentalExtensions;

/** `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;
/**
thoraj marked this conversation as resolved.
Show resolved Hide resolved
* Create a new instance.
*/
public constructor() {
// Set up defaults
this.cryptoSetupExtension = new DefaultCryptoSetupExtensions();
this.experimentalExtension = 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.cryptoSetupExtension;
}

/**
* Provides an experimental extension.
*
* @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.
*/
public get experimental(): ProvideExperimentalExtensions {
return this.experimentalExtension;
}

/**
* Add any extensions provided by the module.
*
* @param module - The appModule to check for extensions.
*
thoraj marked this conversation as resolved.
Show resolved Hide resolved
* @throws if an extension is provided by more than one module.
*/
public addExtensions(module: AppModule): void {
const runtimeModule = module.module;

/* Add the cryptoSetup extension if any */
if (runtimeModule.extensions?.cryptoSetup) {
if (this.hasDefaultCryptoSetupExtension) {
this.cryptoSetupExtension = runtimeModule.extensions?.cryptoSetup;
this.hasDefaultCryptoSetupExtension = false;
} else {
throw new Error(
`adding cryptoSetup extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided.`,
);
}
}

/* Add the experimental extension if any */
if (runtimeModule.extensions?.experimental) {
if (this.hasDefaultExperimentalExtension) {
this.experimentalExtension = runtimeModule.extensions?.experimental;
this.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.
*/
export class ModuleRunner {
public static readonly instance = new ModuleRunner();

private extensionsManager = new ExtensionsManager();

private modules: AppModule[] = [];

private constructor() {
// we only want one instance
}

/**
* Resets the runner, clearing all known modules.
* Exposes all extensions which may be overridden/provided by modules.
*
* @returns An `ExtensionsManager` which exposes the extensions.
*/
public get extensions(): ExtensionsManager {
return this.extensionsManager;
}

/**
* Resets the runner, clearing all known modules, and all extensions
*
* Intended for test usage only.
*/
public reset(): void {
this.modules = [];
this.extensionsManager = new ExtensionsManager();
}

/**
Expand Down Expand Up @@ -72,7 +171,12 @@ export class ModuleRunner {
* @param factory The module factory.
*/
public registerModule(factory: ModuleFactory): void {
this.modules.push(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.
this.extensionsManager.addExtensions(appModule);
}

/**
Expand Down
10 changes: 8 additions & 2 deletions src/toasts/SetupEncryptionToast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -79,7 +80,12 @@ const onReject = (): void => {
};

export const showToast = (kind: Kind): void => {
if (SecurityCustomisations.setupEncryptionNeeded?.(kind)) {
if (
ModuleRunner.instance.extensions.cryptoSetup.setupEncryptionNeeded({
kind: kind as any,
storeProvider: { getInstance: () => SetupEncryptionStore.sharedInstance() },
})
) {
return;
}

Expand Down
Loading
Loading