Remove the ability to skip device verification during login

playwright/e2e/crypto/decryption-failure-messages.spec.ts

@@ -30,6 +30,12 @@ test.describe("Cryptography", function () {
     test.describe("decryption failure messages", () => {
         test.skip(isDendrite, "Dendrite lacks support for MSC3967 so requires additional auth here");
 
+        test.use({
+            config: {
+                force_verification: false,
+            },
+        });
+
         test("should handle device-relative historical messages", async ({
             homeserver,
             page,

playwright/e2e/crypto/device-verification.spec.ts

@@ -185,6 +185,12 @@ test.describe("Device verification", { tag: "@no-webkit" }, () => {
         await enterRecoveryKeyAndCheckVerified(page, app, recoveryKey);
     });
 
+    test.use({
+        config: {
+            force_verification: false,
+        },
+    });
+
     test("Verify device with Recovery Key from settings", async ({ page, app, credentials }) => {
         const recoveryKey = (await aliceBotClient.getRecoveryKey()).encodedPrivateKey;
 

playwright/e2e/login/login-consent.spec.ts

@@ -168,7 +168,7 @@ test.describe("Login", () => {
         });
 
         test.describe("verification after login", () => {
-            test("Shows verification prompt after login if signing keys are set up, skippable by default", async ({
+            test("Shows verification prompt after login if signing keys are set up, unskippable by default", async ({
                 page,
                 homeserver,
                 request,
@@ -186,9 +186,10 @@ test.describe("Login", () => {
                 await page.goto("/");
                 await login(page, homeserver, credentials);
 
-                await expect(page.getByRole("heading", { name: "Confirm your identity", level: 2 })).toBeVisible();
+                const h2 = page.getByRole("heading", { name: "Confirm your identity", level: 2 });
+                await expect(h2).toBeVisible();
 
-                await expect(page.getByRole("button", { name: "Skip verification for now" })).toBeVisible();
+                await expect(h2.locator(".mx_CompleteSecurity_skip")).toHaveCount(0);
             });
 
             test.describe("with force_verification off", () => {

src/DeviceListener.ts

@@ -318,12 +318,6 @@ export default class DeviceListener {
 
         const cli = this.client;
 
-        // cross-signing support was added to Matrix in MSC1756, which landed in spec v1.1
-        if (!(await cli.isVersionSupported("v1.1"))) {
-            logSpan.debug("cross-signing not supported");
-            return;
-        }
-
         const crypto = cli.getCrypto();
         if (!crypto) {
             logSpan.debug("crypto not enabled");

src/SdkConfig.ts

@@ -24,7 +24,7 @@ export const DEFAULTS: DeepReadonly<IConfigOptions> = {
     integrations_rest_url: "https://scalar.vector.im/api",
     uisi_autorageshake_app: "element-auto-uisi",
     show_labs_settings: false,
-    force_verification: false,
+    force_verification: true,
 
     jitsi: {
         preferred_domain: "meet.element.io",

src/components/structures/MatrixChat.tsx

@@ -424,10 +424,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
             } else {
                 this.setStateForNewView({ view: Views.COMPLETE_SECURITY });
             }
-        } else if (
-            (await cli.doesServerSupportUnstableFeature("org.matrix.e2e_cross_signing")) &&
-            !(await shouldSkipSetupEncryption(cli))
-        ) {
+        } else if (!(await shouldSkipSetupEncryption(cli))) {
             // if cross-signing is not yet set up, do so now if possible.
             InitialCryptoSetupStore.sharedInstance().startInitialCryptoSetup(
                 cli,
@@ -1367,11 +1364,20 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
         if (!mustVerifyFlag) return false;
 
         const client = MatrixClientPeg.safeGet();
+
+        // Guests won't have a cross-signing identity to confirm.
         if (client.isGuest()) return false;
 
+        // If we don't have crypto support, we can't verify.
         const crypto = client.getCrypto();
-        const crossSigningReady = await crypto?.isCrossSigningReady();
+        if (!crypto) return false;
+
+        // If we skip setting up encryption, this takes priority over forcing
+        // verification.
+        if (await shouldSkipSetupEncryption(client)) return false;
 
+        // Force verification if this device hasn't already verified.
+        const crossSigningReady = await crypto.isCrossSigningReady();
         return !crossSigningReady;
     }
 

src/components/viewmodels/right_panel/user_info/UserInfoHeaderVerificationViewModel.tsx

@@ -29,16 +29,6 @@ export interface UserInfoVerificationSectionState {
     verifySelectedUser: () => Promise<void>;
 }
 
-const useHomeserverSupportsCrossSigning = (cli: MatrixClient): boolean => {
-    return useAsyncMemo<boolean>(
-        async () => {
-            return cli.doesServerSupportUnstableFeature("org.matrix.e2e_cross_signing");
-        },
-        [cli],
-        false,
-    );
-};
-
 const useHasCrossSigningKeys = (cli: MatrixClient, member: User, canVerify: boolean): boolean | undefined => {
     return useAsyncMemo(async () => {
         if (!canVerify) return undefined;
@@ -56,8 +46,6 @@ export const useUserInfoVerificationViewModel = (
 ): UserInfoVerificationSectionState => {
     const cli = useContext(MatrixClientContext);
 
-    const homeserverSupportsCrossSigning = useHomeserverSupportsCrossSigning(cli);
-
     const userTrust = useAsyncMemo<UserVerificationStatus | undefined>(
         async () => cli.getCrypto()?.getUserVerificationStatus(member.userId),
         [member.userId],
@@ -67,13 +55,7 @@ export const useUserInfoVerificationViewModel = (
     const hasUserVerificationStatus = Boolean(userTrust);
     const isUserVerified = Boolean(userTrust?.isVerified());
     const isMe = member.userId === cli.getUserId();
-    const canVerify =
-        hasUserVerificationStatus &&
-        homeserverSupportsCrossSigning &&
-        !isUserVerified &&
-        !isMe &&
-        devices &&
-        devices.length > 0;
+    const canVerify = hasUserVerificationStatus && !isUserVerified && !isMe && devices && devices.length > 0;
 
     const hasCrossSigningKeys = useHasCrossSigningKeys(cli, member as User, canVerify);
     const verifySelectedUser = (): Promise<void> => verifyUser(cli, member as User);

test/unit-tests/DeviceListener-test.ts

@@ -263,13 +263,6 @@ describe("DeviceListener", () => {
     });
 
     describe("recheck", () => {
-        it("does nothing when cross signing feature is not supported", async () => {
-            mockClient!.isVersionSupported.mockResolvedValue(false);
-            await createAndStart();
-
-            expect(mockClient!.isVersionSupported).toHaveBeenCalledWith("v1.1");
-            expect(mockCrypto!.isCrossSigningReady).not.toHaveBeenCalled();
-        });
         it("does nothing when crypto is not enabled", async () => {
             mockClient!.getCrypto.mockReturnValue(undefined);
             await createAndStart();

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

@@ -129,7 +129,7 @@ describe("<MatrixChat />", () => {
         setGuest: jest.fn(),
         setNotifTimelineSet: jest.fn(),
         getAccountData: jest.fn(),
-        doesServerSupportUnstableFeature: jest.fn().mockResolvedValue(false),
+        doesServerSupportUnstableFeature: jest.fn().mockResolvedValue(true),
         getDevices: jest.fn().mockResolvedValue({ devices: [] }),
         getProfileInfo: jest.fn().mockResolvedValue({
             displayname: "Ernie",
@@ -554,8 +554,8 @@ describe("<MatrixChat />", () => {
                     expect(defaultDispatcher.dispatch).toHaveBeenCalledWith({ action: "client_started" }),
                 );
 
-                // check we get to logged in view
-                await waitForSyncAndLoad(loginClient, true);
+                // set up keys screen is rendered
+                expect(screen.getByText("Setting up keys")).toBeInTheDocument();
             });
 
             it("should persist device language when available", async () => {
@@ -1166,6 +1166,8 @@ describe("<MatrixChat />", () => {
                     // Given force_verification is on (outer describe)
                     // And we just logged in via OIDC (inner describe)
 
+                    mocked(loginClient.getCrypto()!.userHasCrossSigningKeys).mockResolvedValue(true);
+
                     // When we load the page
                     getComponent({ realQueryParams });
 
@@ -1322,6 +1324,7 @@ describe("<MatrixChat />", () => {
                         .mockResolvedValue(new UserVerificationStatus(false, false, false)),
                     setDeviceIsolationMode: jest.fn(),
                     userHasCrossSigningKeys: jest.fn().mockResolvedValue(false),
+                    isCrossSigningReady: 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(),
@@ -1340,22 +1343,8 @@ describe("<MatrixChat />", () => {
                 expect(screen.getByRole("heading", { name: "Welcome Ernie" })).toBeInTheDocument();
             });
 
-            it("should go straight to logged in view when user does not have cross signing keys and server does not support cross signing", async () => {
-                loginClient.doesServerSupportUnstableFeature.mockResolvedValue(false);
-
-                await getComponentAndLogin(false);
-
-                expect(loginClient.doesServerSupportUnstableFeature).toHaveBeenCalledWith(
-                    "org.matrix.e2e_cross_signing",
-                );
-
-                // logged in
-                await screen.findByLabelText("User menu");
-            });
-
-            describe("when server supports cross signing and user does not have cross signing setup", () => {
+            describe("when user does not have cross signing setup", () => {
                 beforeEach(() => {
-                    loginClient.doesServerSupportUnstableFeature.mockResolvedValue(true);
                     jest.spyOn(loginClient.getCrypto()!, "userHasCrossSigningKeys").mockResolvedValue(false);
                 });
 
@@ -1400,8 +1389,6 @@ describe("<MatrixChat />", () => {
                 });
 
                 it("should go to setup e2e screen", async () => {
-                    loginClient.doesServerSupportUnstableFeature.mockResolvedValue(true);
-
                     await getComponentAndLogin();
 
                     expect(loginClient.getCrypto()!.userHasCrossSigningKeys).toHaveBeenCalled();
@@ -1424,9 +1411,7 @@ describe("<MatrixChat />", () => {
                 expect(screen.getByText("Confirm your identity")).toBeInTheDocument();
             });
 
-            it("should setup e2e when server supports cross signing", async () => {
-                loginClient.doesServerSupportUnstableFeature.mockResolvedValue(true);
-
+            it("should setup e2e", async () => {
                 await getComponentAndLogin();
 
                 expect(loginClient.getCrypto()!.userHasCrossSigningKeys).toHaveBeenCalled();
@@ -1587,8 +1572,8 @@ describe("<MatrixChat />", () => {
                     action: "will_start_client",
                 });
 
-                // logged in but waiting for sync screen
-                await screen.findByText("Logout");
+                // set up keys screen is rendered
+                expect(await screen.findByText("Setting up keys")).toBeInTheDocument();
             });
         });
     });

test/unit-tests/components/structures/auth/CompleteSecurity-test.tsx

@@ -47,10 +47,10 @@ describe("CompleteSecurity", () => {
         jest.restoreAllMocks();
     });
 
-    it("Renders with a cancel button by default", () => {
+    it("Renders without a cancel button by default", () => {
         render(<CompleteSecurity onFinished={() => {}} />);
 
-        expect(screen.getByRole("button", { name: "Skip verification for now" })).toBeInTheDocument();
+        expect(screen.queryByRole("button", { name: "Skip verification for now" })).not.toBeInTheDocument();
     });
 
     it("Renders with a cancel button if forceVerification false", () => {

test/unit-tests/components/structures/auth/__snapshots__/CompleteSecurity-test.tsx.snap

@@ -21,14 +21,7 @@ exports[`CompleteSecurity Allows verifying with another device if one is availab
           >
             <h1
               class="mx_CompleteSecurity_header"
-            >
-              <div
-                aria-label="Skip verification for now"
-                class="mx_AccessibleButton mx_CompleteSecurity_skip"
-                role="button"
-                tabindex="0"
-              />
-            </h1>
+            />
             <div
               class="mx_CompleteSecurity_body"
             >
@@ -187,14 +180,7 @@ exports[`CompleteSecurity Allows verifying with recovery key if one is available
           >
             <h1
               class="mx_CompleteSecurity_header"
-            >
-              <div
-                aria-label="Skip verification for now"
-                class="mx_AccessibleButton mx_CompleteSecurity_skip"
-                role="button"
-                tabindex="0"
-              />
-            </h1>
+            />
             <div
               class="mx_CompleteSecurity_body"
             >

test/unit-tests/components/views/right_panel/UserInfo-test.tsx

@@ -118,6 +118,7 @@ beforeEach(() => {
         on: jest.fn(),
         off: jest.fn(),
         isSynapseAdministrator: jest.fn().mockResolvedValue(false),
+        isVersionSupported: jest.fn().mockResolvedValue(false),
         doesServerSupportUnstableFeature: jest.fn().mockReturnValue(false),
         doesServerSupportExtendedProfiles: jest.fn().mockResolvedValue(false),
         getExtendedProfileProperty: jest.fn().mockRejectedValue(new Error("Not supported")),