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 13 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 src/Lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise<void
localStorage.setItem("mx_device_id", credentials.deviceId);
}

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

logger.log(`Session persisted for ${credentials.userId}`);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ export async function sendLoginRequest(
accessToken: data.access_token,
};

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

return creds;
}
2 changes: 1 addition & 1 deletion src/MatrixClientPeg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ class MatrixClientPegClass implements IMatrixClientPeg {
},
};

const dehydrationKeyCallback = ModuleRunner.instance.extensions.cryptoSetup?.getDehydrationKeyCallback();
const dehydrationKeyCallback = ModuleRunner.instance.extensions.cryptoSetup.getDehydrationKeyCallback();
if (dehydrationKeyCallback) {
opts.cryptoCallbacks!.getDehydrationKey = dehydrationKeyCallback;
}
Expand Down
6 changes: 3 additions & 3 deletions src/SecurityManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ async function getSecretStorageKey({
}
}

const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup?.getSecretStorageKey();
const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup.getSecretStorageKey();
if (keyFromCustomisations) {
logger.log("CryptoSetupExtension: Using key from extension (secret storage)");
cacheSecretStorageKey(keyId, keyInfo, keyFromCustomisations);
Expand Down Expand Up @@ -187,7 +187,7 @@ export async function getDehydrationKey(
keyInfo: SecretStorage.SecretStorageKeyDescription,
checkFunc: (data: Uint8Array) => void,
): Promise<Uint8Array> {
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;
Expand Down Expand Up @@ -430,7 +430,7 @@ async function doAccessSecretStorage(func: () => Promise<void>, 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export default class CreateSecretStorageDialog extends React.PureComponent<IProp
}

private getInitialPhase(): void {
const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup?.createSecretStorageKey();
const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup.createSecretStorageKey();
if (keyFromCustomisations) {
logger.log("CryptoSetupExtension: Created key via extension, jumping to bootstrap step");
this.recoveryKey = {
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/MatrixChat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
// 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 });
Expand Down
91 changes: 39 additions & 52 deletions src/modules/ModuleRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,91 +31,80 @@ 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 {
richvdh marked this conversation as resolved.
Show resolved Hide resolved
private _cryptoSetup: ProvideCryptoSetupExtensions;
private _experimental: ProvideExperimentalExtensions;
private _implementionMap: ExtensionImplementationMap = new ExtensionImplementationMap();
// Private backing fields for extensions
private cryptoSetupExtension: ProvideCryptoSetupExtensions;
private experimentalExtension: ProvideExperimentalExtensions;

// Map to keep track of which extensions has been provided by modules.
private implementionMap = {
hasDefaultCryptoSetupExtension: true,
hasDefaultExperimentalExtension: true,
};

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

/**
* 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
* 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;
return this.experimentalExtension;
}

/**
* 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
*
* @param module - The appModule to check for extensions
* Add any extensions provided by the module.
*
* @throws if an extension is provided by more than one 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;

/* 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._implementionMap.hasDefaultCryptoSetupExtension = false;
if (this.implementionMap.hasDefaultCryptoSetupExtension) {
this.cryptoSetupExtension = 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`,
`adding cryptoSetup extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided.`,
);
}
}

/* 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._implementionMap.hasDefaultExperimentalExtension = false;
if (this.implementionMap.hasDefaultExperimentalExtension) {
this.experimentalExtension = 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`,
`adding experimental extension implementation from module ${runtimeModule.moduleName} but an implementation was already provided.`,
);
}
}
Expand All @@ -128,7 +117,7 @@ class ExtensionsManager {
export class ModuleRunner {
public static readonly instance = new ModuleRunner();

private _extensions = new ExtensionsManager();
private extensionsManager = new ExtensionsManager();

private modules: AppModule[] = [];

Expand All @@ -137,12 +126,12 @@ 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 An `ExtensionsManager` which exposes the extensions.
*/
public get extensions(): ExtensionsManager {
return this._extensions;
return this.extensionsManager;
}

/**
Expand All @@ -152,7 +141,7 @@ export class ModuleRunner {
*/
public reset(): void {
this.modules = [];
this._extensions.reset();
this.extensionsManager = new ExtensionsManager();
}

/**
Expand Down Expand Up @@ -187,10 +176,8 @@ 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);
// 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
2 changes: 1 addition & 1 deletion src/toasts/SetupEncryptionToast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() },
})
Expand Down
41 changes: 20 additions & 21 deletions test/MatrixClientPeg-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -94,16 +93,16 @@ describe("MatrixClientPeg", () => {

describe("cryptoSetup extension", () => {
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();
createSecretStorageKey = jest.fn();
catchAccessSecretStorageError = jest.fn();
setupEncryptionNeeded = jest.fn();
getDehydrationKeyCallback = jest.fn().mockReturnValue(null);
})() 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(null),
} as ProvideCryptoSetupExtensions;

// Ensure we have an instance before we set up spies
const instance = ModuleRunner.instance;
Expand All @@ -122,16 +121,16 @@ describe("MatrixClientPeg", () => {
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;
Expand Down
Loading
Loading