Skip to content

AccessSecretStorageDialog: various fixes #30093

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

Merged
merged 5 commits into from
Jun 9, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Please see LICENSE files in the repository root for full details.
color: $alert;

&::before {
mask-image: url("@vector-im/compound-design-tokens/icons/close.svg");
mask-image: url("@vector-im/compound-design-tokens/icons/error-solid.svg");
background-color: $alert;
}
}
Expand Down
23 changes: 0 additions & 23 deletions src/SecurityManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import AccessSecretStorageDialog, {
type KeyParams,
} from "./components/views/dialogs/security/AccessSecretStorageDialog";
import { ModuleRunner } from "./modules/ModuleRunner";
import QuestionDialog from "./components/views/dialogs/QuestionDialog";
import InteractiveAuthDialog from "./components/views/dialogs/InteractiveAuthDialog";

// This stores the secret storage private keys in memory for the JS SDK. This is
Expand Down Expand Up @@ -50,17 +49,6 @@ export class AccessCancelledError extends Error {
}
}

async function confirmToDismiss(): Promise<boolean> {
const [sure] = await Modal.createDialog(QuestionDialog, {
title: _t("encryption|cancel_entering_passphrase_title"),
description: _t("encryption|cancel_entering_passphrase_description"),
danger: false,
button: _t("action|go_back"),
cancelButton: _t("action|cancel"),
}).finished;
return !sure;
}

function makeInputToKey(
keyInfo: SecretStorage.SecretStorageKeyDescription,
): (keyParams: KeyParams) => Promise<Uint8Array> {
Expand Down Expand Up @@ -134,17 +122,6 @@ async function getSecretStorageKey(
return MatrixClientPeg.safeGet().secretStorage.checkKey(key, keyInfo);
},
},
/* className= */ undefined,
/* isPriorityModal= */ false,
/* isStaticModal= */ false,
/* options= */ {
onBeforeClose: async (reason): Promise<boolean> => {
if (reason === "backgroundClick") {
return confirmToDismiss();
}
return true;
},
},
);
const [keyParams] = await finished;
if (!keyParams) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import React, { type ChangeEvent, type FormEvent } from "react";
import { type SecretStorage } from "matrix-js-sdk/src/matrix";

import Field from "../../elements/Field";
import { Flex } from "../../../utils/Flex";
import { _t } from "../../../../languageHandler";
import { EncryptionCard } from "../../settings/encryption/EncryptionCard";
import { EncryptionCardButtons } from "../../settings/encryption/EncryptionCardButtons";
Expand Down Expand Up @@ -83,6 +84,7 @@ export default class AccessSecretStorageDialog extends React.PureComponent<IProp
this.setState({
recoveryKeyCorrect: null,
});
return;
}

const hasPassphrase = this.props.keyInfo?.passphrase?.salt && this.props.keyInfo?.passphrase?.iterations;
Expand Down Expand Up @@ -139,30 +141,30 @@ export default class AccessSecretStorageDialog extends React.PureComponent<IProp
}
};

private getKeyValidationClasses(): string {
return classNames({
"mx_AccessSecretStorageDialog_recoveryKeyFeedback": this.state.recoveryKeyCorrect !== null,
"mx_AccessSecretStorageDialog_recoveryKeyFeedback--invalid": this.state.recoveryKeyCorrect === false,
});
}
private getRecoveryKeyFeedback(): React.ReactNode | null {
let validationText: string;
let classes: string | undefined;

private getKeyValidationText(): string | null {
if (this.state.recoveryKeyCorrect) {
return null;
// The recovery key is good. Empty feedback.
validationText = "\xA0"; // &nbsp;
} else if (this.state.recoveryKeyCorrect === null) {
return _t("encryption|access_secret_storage_dialog|alternatives");
// The input element is empty. Tell the user they can also use a passphrase.
validationText = _t("encryption|access_secret_storage_dialog|alternatives");
} else {
return _t("encryption|access_secret_storage_dialog|key_validation_text|wrong_security_key");
// The entered key is not (yet) correct. Tell them so.
validationText = _t("encryption|access_secret_storage_dialog|key_validation_text|wrong_security_key");
classes = classNames({
"mx_AccessSecretStorageDialog_recoveryKeyFeedback": true,
"mx_AccessSecretStorageDialog_recoveryKeyFeedback--invalid": true,
});
}
}

private getRecoveryKeyFeedback(): React.ReactNode | null {
const validationText = this.getKeyValidationText();
if (validationText === null) {
return null;
} else {
return <div className={this.getKeyValidationClasses()}>{validationText}</div>;
}
return (
<Flex align="center" className={classes}>
{validationText}
</Flex>
);
}

public render(): React.ReactNode {
Expand Down
2 changes: 0 additions & 2 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -920,8 +920,6 @@
"security_key_title": "Recovery key"
},
"bootstrap_title": "Setting up keys",
"cancel_entering_passphrase_description": "Are you sure you want to cancel entering passphrase?",
"cancel_entering_passphrase_title": "Cancel entering passphrase?",
"confirm_encryption_setup_body": "Click the button below to confirm setting up encryption.",
"confirm_encryption_setup_title": "Confirm encryption setup",
"cross_signing_room_normal": "This room is end-to-end encrypted",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ describe("AccessSecretStorageDialog", () => {
render(<AccessSecretStorageDialog {...defaultProps} {...props} />);
};

const enterRecoveryKey = (): void => {
act(() => {
const enterRecoveryKey = async (valueToEnter: string = recoveryKey): Promise<void> => {
await act(async () => {
fireEvent.change(screen.getByRole("textbox"), {
target: {
value: recoveryKey,
value: valueToEnter,
},
});
// wait for debounce
jest.advanceTimersByTime(250);
// wait for debounce, and then give `checkPrivateKey` a chance to complete
await jest.advanceTimersByTimeAsync(250);
});
};

Expand Down Expand Up @@ -116,4 +116,22 @@ describe("AccessSecretStorageDialog", () => {
await expect(screen.findByText("The recovery key you entered is not correct.")).resolves.toBeInTheDocument();
expect(screen.getByText("Continue")).toHaveAttribute("aria-disabled", "true");
});

it("Clears the 'invalid recovery key' notice when the input is cleared", async function () {
renderComponent({ onFinished: () => {}, checkPrivateKey: () => false });

jest.spyOn(mockClient.secretStorage, "checkKey").mockRejectedValue(new Error("invalid key"));

// First, enter the wrong recovery key
await enterRecoveryKey();
expect(screen.getByText("The recovery key you entered is not correct.")).toBeInTheDocument();

// Now, clear the input: the notice should be cleared.
await enterRecoveryKey("");
expect(screen.queryByText("The recovery key you entered is not correct.")).not.toBeInTheDocument();
expect(
screen.getByText("If you have a security key or security phrase, this will work too."),
).toBeInTheDocument();
expect(screen.getByText("Continue")).toHaveAttribute("aria-disabled", "true");
});
});
Loading