Fix canUploadKeysWithPasswordOnly for MSC3967

Being able to upload keys without any auth works too, so return true.
Otherwise we have to go the same route via the InteractiveAuthDialog
which is way more complex.

src/CreateCrossSigning.ts

@@ -23,11 +23,11 @@ import InteractiveAuthDialog from "./components/views/dialogs/InteractiveAuthDia
 async function canUploadKeysWithPasswordOnly(cli: MatrixClient): Promise<boolean> {
     try {
         await cli.uploadDeviceSigningKeys(undefined, {} as CrossSigningKeys);
-        // We should never get here: the server should always require
+        // If we get here, it's because the server is allowing us to upload keys without
-        // UI auth to upload device signing keys. If we do, we upload
+        // auth the first time due to MSC3967. Therefore, yes, we can upload keys
-        // no keys which would be a no-op.
+        // (with or without password, technically, but that's fine).
         logger.log("uploadDeviceSigningKeys unexpectedly succeeded without UI auth!");
-        return false;
+        return true;
     } catch (error) {
         if (!(error instanceof MatrixError) || !error.data || !error.data.flows) {
             logger.log("uploadDeviceSigningKeys advertised no flows!");

src/DeviceListener.ts

@@ -295,13 +295,20 @@ export default class DeviceListener {
             await crypto.getUserDeviceInfo([cli.getSafeUserId()]);
 
             // cross signing isn't enabled - nag to enable it
-            // There are 2 different toasts for:
+            // There are 3 different toasts for:
             if (!(await crypto.getCrossSigningKeyId()) && (await crypto.userHasCrossSigningKeys())) {
-                // Cross-signing on account but this device doesn't trust the master key (verify this session)
+                // Toast 1. Cross-signing on account but this device doesn't trust the master key (verify this session)
                 showSetupEncryptionToast(SetupKind.VERIFY_THIS_SESSION);
                 this.checkKeyBackupStatus();
             } else {
-                // No cross-signing or key backup on account (set up encryption)
+                const backupInfo = await this.getKeyBackupInfo();
+                if (backupInfo) {
+                    // Toast 2: Key backup is enabled but recovery (4S) is not set up: prompt user to set up recovery.
+                    // Since we now enable key backup at registration time, this will be the common case for
+                    // new users.
+                    showSetupEncryptionToast(SetupKind.SET_UP_RECOVERY);
+                } else {
+                    // Toast 3: No cross-signing or key backup on account (set up encryption)
                     await cli.waitForClientWellKnown();
                     if (isSecureBackupRequired(cli) && isLoggedIn()) {
                         // If we're meant to set up, and Secure Backup is required,
@@ -313,6 +320,7 @@ export default class DeviceListener {
                     }
                 }
             }
+        }
 
         // This needs to be done after awaiting on getUserDeviceInfo() above, so
         // we make sure we get the devices after the fetch is done.

src/components/structures/auth/E2eSetup.tsx

@@ -11,7 +11,7 @@ import { MatrixClient } from "matrix-js-sdk/src/matrix";
 
 import AuthPage from "../../views/auth/AuthPage";
 import CompleteSecurityBody from "../../views/auth/CompleteSecurityBody";
-import CreateCrossSigningDialog from "../../views/dialogs/security/CreateCrossSigningDialog";
+import InitialCryptoSetupDialog from "../../views/dialogs/security/InitialCryptoSetupDialog";
 
 interface IProps {
     matrixClient: MatrixClient;
@@ -25,7 +25,7 @@ export default class E2eSetup extends React.Component<IProps> {
         return (
             <AuthPage>
                 <CompleteSecurityBody>
-                    <CreateCrossSigningDialog
+                    <InitialCryptoSetupDialog
                         matrixClient={this.props.matrixClient}
                         onFinished={this.props.onFinished}
                         accountPassword={this.props.accountPassword}

src/components/views/dialogs/security/InitialCryptoSetupDialog.tsx

@@ -25,14 +25,14 @@ interface Props {
 }
 
 /*
- * Walks the user through the process of creating a cross-signing keys. In most
+ * Walks the user through the process of creating a cross-signing keys and setting
- * cases, only a spinner is shown, but for more complex auth like SSO, the user
+ * up message key backup. In most cases, only a spinner is shown, but for more
- * may need to complete some steps to proceed.
+ * complex auth like SSO, the user may need to complete some steps to proceed.
  */
-const CreateCrossSigningDialog: React.FC<Props> = ({ matrixClient, accountPassword, tokenLogin, onFinished }) => {
+const InitialCryptoSetupDialog: React.FC<Props> = ({ matrixClient, accountPassword, tokenLogin, onFinished }) => {
     const [error, setError] = useState(false);
 
-    const bootstrapCrossSigning = useCallback(async () => {
+    const doSetup = useCallback(async () => {
         const cryptoApi = matrixClient.getCrypto();
         if (!cryptoApi) return;
 
@@ -40,6 +40,12 @@ const CreateCrossSigningDialog: React.FC<Props> = ({ matrixClient, accountPasswo
 
         try {
             await createCrossSigning(matrixClient, tokenLogin, accountPassword);
+
+            const backupInfo = await matrixClient.getKeyBackupVersion();
+            if (backupInfo === null) {
+                await cryptoApi.resetKeyBackup();
+            }
+
             onFinished(true);
         } catch (e) {
             if (tokenLogin) {
@@ -58,8 +64,8 @@ const CreateCrossSigningDialog: React.FC<Props> = ({ matrixClient, accountPasswo
     }, [onFinished]);
 
     useEffect(() => {
-        bootstrapCrossSigning();
+        doSetup();
-    }, [bootstrapCrossSigning]);
+    }, [doSetup]);
 
     let content;
     if (error) {
@@ -69,7 +75,7 @@ const CreateCrossSigningDialog: React.FC<Props> = ({ matrixClient, accountPasswo
                 <div className="mx_Dialog_buttons">
                     <DialogButtons
                         primaryButton={_t("action|retry")}
-                        onPrimaryButtonClick={bootstrapCrossSigning}
+                        onPrimaryButtonClick={doSetup}
                         onCancel={onCancel}
                     />
                 </div>
@@ -96,4 +102,4 @@ const CreateCrossSigningDialog: React.FC<Props> = ({ matrixClient, accountPasswo
     );
 };
 
-export default CreateCrossSigningDialog;
+export default InitialCryptoSetupDialog;

src/i18n/strings/en_EN.json

@@ -914,6 +914,9 @@
             "warning": "If you didn't remove the recovery method, an attacker may be trying to access your account. Change your account password and set a new recovery method immediately in Settings."
         },
         "reset_all_button": "Forgotten or lost all recovery methods? <a>Reset all</a>",
+        "set_up_recovery": "Set up recovery",
+        "set_up_recovery_later": "Not now",
+        "set_up_recovery_toast_title": "Set up recovery to protect your account",
         "set_up_toast_description": "Safeguard against losing access to encrypted messages & data",
         "set_up_toast_title": "Set up Secure Backup",
         "setup_secure_backup": {

src/toasts/SetupEncryptionToast.ts

@@ -6,6 +6,9 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
 Please see LICENSE files in the repository root for full details.
 */
 
+import KeyboardIcon from "@vector-im/compound-design-tokens/assets/web/icons/key";
+import { ComponentType } from "react";
+
 import Modal from "../Modal";
 import { _t } from "../languageHandler";
 import DeviceListener from "../DeviceListener";
@@ -23,33 +26,61 @@ const getTitle = (kind: Kind): string => {
     switch (kind) {
         case Kind.SET_UP_ENCRYPTION:
             return _t("encryption|set_up_toast_title");
+        case Kind.SET_UP_RECOVERY:
+            return _t("encryption|set_up_recovery_toast_title");
         case Kind.VERIFY_THIS_SESSION:
             return _t("encryption|verify_toast_title");
     }
 };
 
-const getIcon = (kind: Kind): string => {
+const getIcon = (kind: Kind): string | undefined => {
     switch (kind) {
         case Kind.SET_UP_ENCRYPTION:
             return "secure_backup";
+        case Kind.SET_UP_RECOVERY:
+            return undefined;
         case Kind.VERIFY_THIS_SESSION:
             return "verification_warning";
     }
 };
 
+// Gets the icon displayed on the prinary button
+const getPrimaryIcon = (kind: Kind): ComponentType<React.SVGAttributes<SVGElement>> | undefined => {
+    switch (kind) {
+        case Kind.SET_UP_RECOVERY:
+            return KeyboardIcon;
+        default:
+            return undefined;
+    }
+};
+
 const getSetupCaption = (kind: Kind): string => {
     switch (kind) {
         case Kind.SET_UP_ENCRYPTION:
             return _t("action|continue");
+        case Kind.SET_UP_RECOVERY:
+            return _t("encryption|set_up_recovery");
         case Kind.VERIFY_THIS_SESSION:
             return _t("action|verify");
     }
 };
 
+const getSecondaryButtonLabel = (kind: Kind): string => {
+    switch (kind) {
+        case Kind.SET_UP_RECOVERY:
+            return _t("encryption|set_up_recovery_later");
+        case Kind.SET_UP_ENCRYPTION:
+        case Kind.VERIFY_THIS_SESSION:
+            return _t("encryption|verification|unverified_sessions_toast_reject");
+    }
+};
+
 const getDescription = (kind: Kind): string => {
     switch (kind) {
         case Kind.SET_UP_ENCRYPTION:
             return _t("encryption|set_up_toast_description");
+        case Kind.SET_UP_RECOVERY:
+            return _t("encryption|set_up_recovery_toast_title");
         case Kind.VERIFY_THIS_SESSION:
             return _t("encryption|verify_toast_description");
     }
@@ -57,6 +88,7 @@ const getDescription = (kind: Kind): string => {
 
 export enum Kind {
     SET_UP_ENCRYPTION = "set_up_encryption",
+    SET_UP_RECOVERY = "set_up_recovery",
     VERIFY_THIS_SESSION = "verify_this_session",
 }
 
@@ -101,9 +133,9 @@ export const showToast = (kind: Kind): void => {
             description: getDescription(kind),
             primaryLabel: getSetupCaption(kind),
             onPrimaryClick: onAccept,
-            secondaryLabel: _t("encryption|verification|unverified_sessions_toast_reject"),
+            secondaryLabel: getSecondaryButtonLabel(kind),
             onSecondaryClick: onReject,
-            destructive: "secondary",
+            PrimaryIcon: getPrimaryIcon(kind),
         },
         component: GenericToast,
         priority: kind === Kind.VERIFY_THIS_SESSION ? 95 : 40,

test/components/views/dialogs/security/InitialCryptoSetupDialog-test.tsx

@@ -12,14 +12,14 @@ import { mocked } from "jest-mock";
 import { MatrixClient } from "matrix-js-sdk/src/matrix";
 
 import { createCrossSigning } from "../../../../../src/CreateCrossSigning";
-import CreateCrossSigningDialog from "../../../../../src/components/views/dialogs/security/CreateCrossSigningDialog";
+import InitialCryptoSetupDialog from "../../../../../src/components/views/dialogs/security/InitialCryptoSetupDialog";
 import { createTestClient } from "../../../../test-utils";
 
 jest.mock("../../../../../src/CreateCrossSigning", () => ({
     createCrossSigning: jest.fn(),
 }));
 
-describe("CreateCrossSigningDialog", () => {
+describe("InitialCryptoSetupDialog", () => {
     let client: MatrixClient;
     let createCrossSigningResolve: () => void;
     let createCrossSigningReject: (e: Error) => void;
@@ -43,7 +43,7 @@ describe("CreateCrossSigningDialog", () => {
         const onFinished = jest.fn();
 
         render(
-            <CreateCrossSigningDialog
+            <InitialCryptoSetupDialog
                 matrixClient={client}
                 accountPassword="hunter2"
                 tokenLogin={false}
@@ -61,7 +61,7 @@ describe("CreateCrossSigningDialog", () => {
 
     it("should display an error if createCrossSigning fails", async () => {
         render(
-            <CreateCrossSigningDialog
+            <InitialCryptoSetupDialog
                 matrixClient={client}
                 accountPassword="hunter2"
                 tokenLogin={false}
@@ -78,7 +78,7 @@ describe("CreateCrossSigningDialog", () => {
         const onFinished = jest.fn();
 
         render(
-            <CreateCrossSigningDialog
+            <InitialCryptoSetupDialog
                 matrixClient={client}
                 accountPassword="hunter2"
                 tokenLogin={true}
@@ -95,7 +95,7 @@ describe("CreateCrossSigningDialog", () => {
         const onFinished = jest.fn();
 
         render(
-            <CreateCrossSigningDialog
+            <InitialCryptoSetupDialog
                 matrixClient={client}
                 accountPassword="hunter2"
                 tokenLogin={false}
@@ -113,7 +113,7 @@ describe("CreateCrossSigningDialog", () => {
 
     it("should retry when the retry button is clicked", async () => {
         render(
-            <CreateCrossSigningDialog
+            <InitialCryptoSetupDialog
                 matrixClient={client}
                 accountPassword="hunter2"
                 tokenLogin={false}

test/test-utils/test-utils.ts

@@ -281,6 +281,7 @@ export function createTestClient(): MatrixClient {
         getLocalAliases: jest.fn().mockReturnValue([]),
         uploadDeviceSigningKeys: jest.fn(),
         isKeyBackupKeyStored: jest.fn().mockResolvedValue(null),
+        getKeyBackupVersion: jest.fn(),
     } as unknown as MatrixClient;
 
     client.reEmitter = new ReEmitter(client);

test/unit-tests/DeviceListener-test.ts

@@ -352,13 +352,13 @@ describe("DeviceListener", () => {
                     mockCrypto!.getCrossSigningKeyId.mockResolvedValue("abc");
                 });
 
-                it("shows set up encryption toast when user has a key backup available", async () => {
+                it("shows set up recovery toast when user has a key backup available", async () => {
                     // non falsy response
                     mockCrypto.getKeyBackupInfo.mockResolvedValue({} as unknown as KeyBackupInfo);
                     await createAndStart();
 
                     expect(SetupEncryptionToast.showToast).toHaveBeenCalledWith(
-                        SetupEncryptionToast.Kind.SET_UP_ENCRYPTION,
+                        SetupEncryptionToast.Kind.SET_UP_RECOVERY,
                     );
                 });
             });

test/unit-tests/components/structures/MatrixChat-test.tsx

@@ -148,6 +148,7 @@ describe("<MatrixChat />", () => {
         whoami: jest.fn(),
         logout: jest.fn(),
         getDeviceId: jest.fn(),
+        getKeyBackupVersion: jest.fn(),
     });
     let mockClient: Mocked<MatrixClient>;
     const serverConfig = {
@@ -1003,6 +1004,7 @@ describe("<MatrixChat />", () => {
                     userHasCrossSigningKeys: jest.fn().mockResolvedValue(false),
                     // This needs to not finish immediately because we need to test the screen appears
                     bootstrapCrossSigning: jest.fn().mockImplementation(() => bootstrapDeferred.promise),
+                    resetKeyBackup: jest.fn(),
                     isEncryptionEnabledInRoom: jest.fn().mockResolvedValue(false),
                 };
                 loginClient.getCrypto.mockReturnValue(mockCrypto as any);