Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Factor out crypto setup process into a store #28675

Merged
merged 7 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 0 deletions src/@types/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import { IConfigOptions } from "../IConfigOptions";
import { MatrixDispatcher } from "../dispatcher/dispatcher";
import { DeepReadonly } from "./common";
import MatrixChat from "../components/structures/MatrixChat";
import { InitialCryptoSetupStore } from "../stores/InitialCryptoSetupStore";

/* eslint-disable @typescript-eslint/naming-convention */

Expand Down Expand Up @@ -117,6 +118,7 @@ declare global {
mxPerformanceEntryNames: any;
mxUIStore: UIStore;
mxSetupEncryptionStore?: SetupEncryptionStore;
mxInitialCryptoStore?: InitialCryptoSetupStore;
mxRoomScrollStateStore?: RoomScrollStateStore;
mxActiveWidgetStore?: ActiveWidgetStore;
mxOnRecaptchaLoaded?: () => void;
Expand Down
16 changes: 8 additions & 8 deletions src/components/structures/MatrixChat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ import { SessionLockStolenView } from "./auth/SessionLockStolenView";
import { ConfirmSessionLockTheftView } from "./auth/ConfirmSessionLockTheftView";
import { LoginSplashView } from "./auth/LoginSplashView";
import { cleanUpDraftsIfRequired } from "../../DraftCleaner";
import { InitialCryptoSetupStore } from "../../stores/InitialCryptoSetupStore";

// legacy export
export { default as Views } from "../../Views";
Expand Down Expand Up @@ -428,6 +429,12 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
!(await shouldSkipSetupEncryption(cli))
) {
// if cross-signing is not yet set up, do so now if possible.
InitialCryptoSetupStore.sharedInstance().startInitialCryptoSetup(
cli,
Boolean(this.tokenLogin),
this.stores,
this.onCompleteSecurityE2eSetupFinished,
);
this.setStateForNewView({ view: Views.E2E_SETUP });
} else {
this.onLoggedIn();
Expand Down Expand Up @@ -2073,14 +2080,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
} else if (this.state.view === Views.COMPLETE_SECURITY) {
view = <CompleteSecurity onFinished={this.onCompleteSecurityE2eSetupFinished} />;
} else if (this.state.view === Views.E2E_SETUP) {
view = (
<E2eSetup
matrixClient={MatrixClientPeg.safeGet()}
onFinished={this.onCompleteSecurityE2eSetupFinished}
accountPassword={this.stores.accountPasswordStore.getPassword()}
tokenLogin={!!this.tokenLogin}
/>
);
view = <E2eSetup onFinished={this.onCompleteSecurityE2eSetupFinished} />;
} else if (this.state.view === Views.LOGGED_IN) {
// `ready` and `view==LOGGED_IN` may be set before `page_type` (because the
// latter is set via the dispatcher). If we don't yet have a `page_type`,
Expand Down
11 changes: 1 addition & 10 deletions src/components/structures/auth/E2eSetup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,21 @@ Please see LICENSE files in the repository root for full details.
*/

import React from "react";
import { MatrixClient } from "matrix-js-sdk/src/matrix";

import AuthPage from "../../views/auth/AuthPage";
import CompleteSecurityBody from "../../views/auth/CompleteSecurityBody";
import { InitialCryptoSetupDialog } from "../../views/dialogs/security/InitialCryptoSetupDialog";

interface IProps {
matrixClient: MatrixClient;
onFinished: () => void;
accountPassword?: string;
tokenLogin: boolean;
}

export default class E2eSetup extends React.Component<IProps> {
public render(): React.ReactNode {
return (
<AuthPage>
<CompleteSecurityBody>
<InitialCryptoSetupDialog
matrixClient={this.props.matrixClient}
onFinished={this.props.onFinished}
accountPassword={this.props.accountPassword}
tokenLogin={this.props.tokenLogin}
/>
<InitialCryptoSetupDialog onFinished={this.props.onFinished} />
</CompleteSecurityBody>
</AuthPage>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,15 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
Please see LICENSE files in the repository root for full details.
*/

import React, { useCallback, useEffect, useState } from "react";
import { logger } from "matrix-js-sdk/src/logger";
import { MatrixClient } from "matrix-js-sdk/src/matrix";
import React, { useCallback } from "react";

import { _t } from "../../../../languageHandler";
import DialogButtons from "../../elements/DialogButtons";
import BaseDialog from "../BaseDialog";
import Spinner from "../../elements/Spinner";
import { createCrossSigning } from "../../../../CreateCrossSigning";
import { InitialCryptoSetupStore, useInitialCryptoSetupStatus } from "../../../../stores/InitialCryptoSetupStore";

interface Props {
matrixClient: MatrixClient;
accountPassword?: string;
tokenLogin: boolean;
onFinished: (success?: boolean) => void;
}

Expand All @@ -29,54 +24,27 @@ interface Props {
* In most cases, only a spinner is shown, but for more
* complex auth like SSO, the user may need to complete some steps to proceed.
*/
export const InitialCryptoSetupDialog: React.FC<Props> = ({
matrixClient,
accountPassword,
tokenLogin,
onFinished,
}) => {
const [error, setError] = useState(false);
export const InitialCryptoSetupDialog: React.FC<Props> = ({ onFinished }) => {
const onRetryClick = useCallback(() => {
InitialCryptoSetupStore.sharedInstance().retry();
}, []);

const doSetup = useCallback(async () => {
const cryptoApi = matrixClient.getCrypto();
if (!cryptoApi) return;

setError(false);

try {
await createCrossSigning(matrixClient, tokenLogin, accountPassword);

onFinished(true);
} catch (e) {
if (tokenLogin) {
// ignore any failures, we are relying on grace period here
onFinished(false);
return;
}

setError(true);
logger.error("Error bootstrapping cross-signing", e);
}
}, [matrixClient, tokenLogin, accountPassword, onFinished]);

const onCancel = useCallback(() => {
const onCancelClick = useCallback(() => {
onFinished(false);
}, [onFinished]);

useEffect(() => {
doSetup();
}, [doSetup]);
const status = useInitialCryptoSetupStatus(InitialCryptoSetupStore.sharedInstance());

let content;
if (error) {
if (status === "error") {
content = (
<div>
<p>{_t("encryption|unable_to_setup_keys_error")}</p>
<div className="mx_Dialog_buttons">
<DialogButtons
primaryButton={_t("action|retry")}
onPrimaryButtonClick={doSetup}
onCancel={onCancel}
onPrimaryButtonClick={onRetryClick}
onCancel={onCancelClick}
/>
</div>
</div>
Expand Down
140 changes: 140 additions & 0 deletions src/stores/InitialCryptoSetupStore.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/*
Copyright 2024 New Vector Ltd.

SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
Please see LICENSE files in the repository root for full details.
*/

import EventEmitter from "events";
import { MatrixClient } from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";
import { useEffect, useState } from "react";

import { createCrossSigning } from "../CreateCrossSigning";
import { SdkContextClass } from "../contexts/SDKContext";

type Status = "in_progress" | "complete" | "error" | undefined;

export const useInitialCryptoSetupStatus = (store: InitialCryptoSetupStore): Status => {
const [status, setStatus] = useState<Status>(store.getStatus());

useEffect(() => {
const update = (): void => {
setStatus(store.getStatus());
};

store.on("update", update);

return () => {
store.off("update", update);
};
}, [store]);

return status;
};

/**
* Logic for setting up crypto state that's done immediately after
* a user registers. Should be transparent to the user, not requiring
* interaction in most cases.
* As distinct from SetupEncryptionStore which is for setting up
* 4S or verifying the device, will always require interaction
* from the user in some form.
*/
export class InitialCryptoSetupStore extends EventEmitter {
private status: Status = undefined;

private client?: MatrixClient;
private isTokenLogin?: boolean;
private stores?: SdkContextClass;
private onFinished?: (success: boolean) => void;

public static sharedInstance(): InitialCryptoSetupStore {
if (!window.mxInitialCryptoStore) window.mxInitialCryptoStore = new InitialCryptoSetupStore();
return window.mxInitialCryptoStore;
}

public getStatus(): Status {
return this.status;
}

/**
* Start the initial crypto setup process.
*
* @param {MatrixClient} client The client to use for the setup
* @param {boolean} isTokenLogin True if the user logged in via a token login, otherwise false
* @param {SdkContextClass} stores The stores to use for the setup
*/
public startInitialCryptoSetup(
client: MatrixClient,
isTokenLogin: boolean,
stores: SdkContextClass,
onFinished: (success: boolean) => void,
): void {
this.client = client;
this.isTokenLogin = isTokenLogin;
this.stores = stores;
this.onFinished = onFinished;

// We just start this process: it's progress is tracked by the events rather
// than returning a promise, so we don't bother.
this.doSetup().catch(() => logger.error("Initial crypto setup failed"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we are not waiting for doSetup to finish?

Copy link
Member Author

@dbkr dbkr Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just seemed unnecessary and would conflate the interface I think. Added a comment.

}

/**
* Retry the initial crypto setup process.
*
* If no crypto setup is currently in process, this will return false.
*
* @returns {boolean} True if a retry was initiated, otherwise false
*/
public retry(): boolean {
if (this.client === undefined || this.isTokenLogin === undefined || this.stores == undefined) return false;

this.doSetup().catch(() => logger.error("Initial crypto setup failed"));

return true;
}

private reset(): void {
this.client = undefined;
this.isTokenLogin = undefined;
this.stores = undefined;
}

private async doSetup(): Promise<void> {
if (this.client === undefined || this.isTokenLogin === undefined || this.stores == undefined) {
throw new Error("No setup is in progress");
}

const cryptoApi = this.client.getCrypto();
if (!cryptoApi) throw new Error("No crypto module found!");

this.status = "in_progress";
this.emit("update");

try {
await createCrossSigning(this.client, this.isTokenLogin, this.stores.accountPasswordStore.getPassword());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The password thing look like something that should be removed. Because MSC3967 has been merged and re-auth will not be required to setup cross signing for the first time.
And also anyhow the grace period is managed by the server.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was hoping we could remove this soon. I wouldn't do that in a refactor though.


this.reset();

this.status = "complete";
this.emit("update");
this.onFinished?.(true);
} catch (e) {
if (this.isTokenLogin) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the status is not "error" if there is a login token?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish I knew: this comment is all there was so I moved it over. Possibly because if the user's using token login and the grace period didn't work, nothing else will so it's better to just carry on and bug the user later to set up crypto rather than worry them? Maybe the real answer is that we should drop support for HSes without the key upload grace period.

// ignore any failures, we are relying on grace period here
this.reset();

this.status = "complete";
this.emit("update");
this.onFinished?.(true);

return;
}
logger.error("Error bootstrapping cross-signing", e);
this.status = "error";
this.emit("update");
}
}
}
5 changes: 5 additions & 0 deletions src/stores/SetupEncryptionStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ export enum Phase {
ConfirmReset = 6,
}

/**
* Logic for setting up 4S and/or verifying the user's device: a process requiring
* ongoing interaction with the user, as distinct from InitialCryptoSetupStore which
* a (usually) non-interactive process that happens immediately after registration.
*/
export class SetupEncryptionStore extends EventEmitter {
private started?: boolean;
public phase?: Phase;
Expand Down
Loading
Loading