Force verification even for refreshed clients

package.json

@@ -198,6 +198,7 @@
         "axe-core": "4.10.0",
         "babel-jest": "^29.0.0",
         "blob-polyfill": "^9.0.0",
+        "core-js": "^3.38.1",
         "eslint": "8.57.1",
         "eslint-config-google": "^0.14.0",
         "eslint-config-prettier": "^9.0.0",

playwright/e2e/login/login.spec.ts

@@ -6,20 +6,85 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
 Please see LICENSE files in the repository root for full details.
 */
 
+import { Page } from "playwright-core";
+
 import { expect, test } from "../../element-web-test";
 import { doTokenRegistration } from "./utils";
 import { isDendrite } from "../../plugins/homeserver/dendrite";
 import { selectHomeserver } from "../utils";
+import { Credentials, HomeserverInstance } from "../../plugins/homeserver";
+
+const username = "user1234";
+const password = "p4s5W0rD";
+
+// Pre-generated dummy signing keys to create an account that has signing keys set.
+// Note the signatures are specific to the username and must be valid or the HS will reject the keys.
+const DEVICE_SIGNING_KEYS_BODY = {
+    master_key: {
+        keys: {
+            "ed25519:6qCouJsi2j7DzOmpxPTBALpvDTqa8p2mjrQR2P8wEbg": "6qCouJsi2j7DzOmpxPTBALpvDTqa8p2mjrQR2P8wEbg",
+        },
+        signatures: {
+            "@user1234:localhost": {
+                "ed25519:6qCouJsi2j7DzOmpxPTBALpvDTqa8p2mjrQR2P8wEbg":
+                    "mvwqsYiGa2gPH6ueJsiJnceHMrZhf1pqIMGxkvKisN3ucz8sU7LwyzndbYaLkUKEDx1JuOKFfZ9Mb3mqc7PMBQ",
+                "ed25519:SRHVWTNVBH":
+                    "HVGmVIzsJe3d+Un/6S9tXPsU7YA8HjZPdxogVzdjEFIU8OjLyElccvjupow0rVWgkEqU8sO21LIHw9cWRZEmDw",
+            },
+        },
+        usage: ["master"],
+        user_id: "@user1234:localhost",
+    },
+    self_signing_key: {
+        keys: {
+            "ed25519:eqzRly4S1GvTA36v48hOKokHMtYBLm02zXRgPHue5/8": "eqzRly4S1GvTA36v48hOKokHMtYBLm02zXRgPHue5/8",
+        },
+        signatures: {
+            "@user1234:localhost": {
+                "ed25519:6qCouJsi2j7DzOmpxPTBALpvDTqa8p2mjrQR2P8wEbg":
+                    "M2rt5xs+23egbVUwUcZuU7pMpn0chBNC5rpdyZGayfU3FDlx1DbopbakIcl5v4uOSGMbqUotyzkE6CchB+dgDw",
+            },
+        },
+        usage: ["self_signing"],
+        user_id: "@user1234:localhost",
+    },
+    user_signing_key: {
+        keys: {
+            "ed25519:h6C7sonjKSSa/VMvmpmFnwMA02H2rKIMSYZ2ddwgJn4": "h6C7sonjKSSa/VMvmpmFnwMA02H2rKIMSYZ2ddwgJn4",
+        },
+        signatures: {
+            "@user1234:localhost": {
+                "ed25519:6qCouJsi2j7DzOmpxPTBALpvDTqa8p2mjrQR2P8wEbg":
+                    "5ZMJ7SG2qr76vU2nITKap88AxLZ/RZQmF/mBcAcVZ9Bknvos3WQp8qN9jKuiqOHCq/XpPORA6XBmiDIyPqTFAA",
+            },
+        },
+        usage: ["user_signing"],
+        user_id: "@user1234:localhost",
+    },
+    auth: {
+        type: "m.login.password",
+        identifier: { type: "m.id.user", user: "@user1234:localhost" },
+        password: password,
+    },
+};
+
+async function login(page: Page, homeserver: HomeserverInstance) {
+    await page.getByRole("link", { name: "Sign in" }).click();
+    await selectHomeserver(page, homeserver.config.baseUrl);
+
+    await page.getByRole("textbox", { name: "Username" }).fill(username);
+    await page.getByPlaceholder("Password").fill(password);
+    await page.getByRole("button", { name: "Sign in" }).click();
+}
 
 test.describe("Login", () => {
     test.describe("Password login", () => {
         test.use({ startHomeserverOpts: "consent" });
 
-        const username = "user1234";
-        const password = "p4s5W0rD";
+        let creds: Credentials;
 
         test.beforeEach(async ({ homeserver }) => {
-            await homeserver.registerUser(username, password);
+            creds = await homeserver.registerUser(username, password);
         });
 
         test("Loads the welcome page by default; then logs in with an existing account and lands on the home screen", async ({
@@ -65,17 +130,97 @@ test.describe("Login", () => {
 
         test("Follows the original link after login", async ({ page, homeserver }) => {
             await page.goto("/#/room/!room:id"); // should redirect to the welcome page
-            await page.getByRole("link", { name: "Sign in" }).click();
-
-            await selectHomeserver(page, homeserver.config.baseUrl);
-
-            await page.getByRole("textbox", { name: "Username" }).fill(username);
-            await page.getByPlaceholder("Password").fill(password);
-            await page.getByRole("button", { name: "Sign in" }).click();
+            await login(page, homeserver);
 
             await expect(page).toHaveURL(/\/#\/room\/!room:id$/);
             await expect(page.getByRole("button", { name: "Join the discussion" })).toBeVisible();
         });
+
+        test.describe("verification after login", () => {
+            test("Shows verification prompt after login if signing keys are set up, skippable by default", async ({
+                page,
+                homeserver,
+                request,
+            }) => {
+                const res = await request.post(
+                    `${homeserver.config.baseUrl}/_matrix/client/v3/keys/device_signing/upload`,
+                    { headers: { Authorization: `Bearer ${creds.accessToken}` }, data: DEVICE_SIGNING_KEYS_BODY },
+                );
+                if (res.status() / 100 !== 2) {
+                    console.log(await res.json());
+                }
+                expect(res.status() / 100).toEqual(2);
+
+                await page.goto("/");
+                await login(page, homeserver);
+
+                await expect(page.getByRole("heading", { name: "Verify this device", level: 1 })).toBeVisible();
+
+                await expect(page.getByRole("button", { name: "Skip verification for now" })).toBeVisible();
+            });
+
+            test.describe("with force_verification off", () => {
+                test.use({
+                    config: {
+                        force_verification: false,
+                    },
+                });
+
+                test("Shows skippable verification prompt after login if signing keys are set up", async ({
+                    page,
+                    homeserver,
+                    request,
+                }) => {
+                    const res = await request.post(
+                        `${homeserver.config.baseUrl}/_matrix/client/v3/keys/device_signing/upload`,
+                        { headers: { Authorization: `Bearer ${creds.accessToken}` }, data: DEVICE_SIGNING_KEYS_BODY },
+                    );
+                    if (res.status() / 100 !== 2) {
+                        console.log(await res.json());
+                    }
+                    expect(res.status() / 100).toEqual(2);
+
+                    await page.goto("/");
+                    await login(page, homeserver);
+
+                    await expect(page.getByRole("heading", { name: "Verify this device", level: 1 })).toBeVisible();
+
+                    await expect(page.getByRole("button", { name: "Skip verification for now" })).toBeVisible();
+                });
+            });
+
+            test.describe("with force_verification on", () => {
+                test.use({
+                    config: {
+                        force_verification: true,
+                    },
+                });
+
+                test("Shows unskippable verification prompt after login if signing keys are set up", async ({
+                    page,
+                    homeserver,
+                    request,
+                }) => {
+                    console.log(`uid ${creds.userId} body`, DEVICE_SIGNING_KEYS_BODY);
+                    const res = await request.post(
+                        `${homeserver.config.baseUrl}/_matrix/client/v3/keys/device_signing/upload`,
+                        { headers: { Authorization: `Bearer ${creds.accessToken}` }, data: DEVICE_SIGNING_KEYS_BODY },
+                    );
+                    if (res.status() / 100 !== 2) {
+                        console.log(await res.json());
+                    }
+                    expect(res.status() / 100).toEqual(2);
+
+                    await page.goto("/");
+                    await login(page, homeserver);
+
+                    const h1 = await page.getByRole("heading", { name: "Verify this device", level: 1 });
+                    await expect(h1).toBeVisible();
+
+                    expect(h1.locator(".mx_CompleteSecurity_skip")).not.toBeVisible();
+                });
+            });
+        });
     });
 
     // tests for old-style SSO login, in which we exchange tokens with Synapse, and Synapse talks to an auth server

src/Lifecycle.ts

@@ -824,6 +824,8 @@ async function doSetLoggedIn(
     }
     checkSessionLock();
 
+    // We are now logged in, so fire this. We have yet to start the client but the
+    // client_started dispatch is for that.
     dis.fire(Action.OnLoggedIn);
 
     const clientPegOpts: MatrixClientPegAssignOpts = {};
@@ -846,6 +848,12 @@ async function doSetLoggedIn(
     // Run the migrations after the MatrixClientPeg has been assigned
     SettingsStore.runMigrations(isFreshLogin);
 
+    if (isFreshLogin && !credentials.guest) {
+        // For newly registered users, set a flag so that we force them to verify,
+        // (we don't want to force users with existing sessions to verify though)
+        localStorage.setItem("must_verify_device", "true");
+    }
+
     return client;
 }
 

src/SdkConfig.ts

@@ -24,6 +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,
 
     jitsi: {
         preferred_domain: "meet.element.io",

src/components/structures/MatrixChat.tsx

@@ -166,6 +166,12 @@ interface IProps {
     initialScreenAfterLogin?: IScreen;
     // displayname, if any, to set on the device when logging in/registering.
     defaultDeviceDisplayName?: string;
+
+    // Used by tests, this function is called when session initialisation starts
+    // with a promise that resolves or rejects once the initialiation process
+    // has finished, so that tests can wait for this to avoid them executing over
+    // each other.
+    initPromiseCallback?: (p: Promise<void>) => void;
 }
 
 interface IState {
@@ -309,7 +315,12 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
      * Kick off a call to {@link initSession}, and handle any errors
      */
     private startInitSession = (): void => {
-        this.initSession().catch((err) => {
+        const initProm = this.initSession();
+        if (this.props.initPromiseCallback) {
+            this.props.initPromiseCallback(initProm);
+        }
+
+        initProm.catch((err) => {
             // TODO: show an error screen, rather than a spinner of doom
             logger.error("Error initialising Matrix session", err);
         });
@@ -881,7 +892,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
                 });
                 break;
             case "client_started":
-                this.onClientStarted();
+                this.onClientStarted().then();
                 break;
             case "send_event":
                 this.onSendEvent(payload.room_id, payload.event);
@@ -1320,6 +1331,20 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
         }
     }
 
+    private async shouldForceVerification(): Promise<boolean> {
+        if (!SdkConfig.get("force_verification")) return false;
+        const mustVerifyFlag = localStorage.getItem("must_verify_device");
+        if (!mustVerifyFlag) return false;
+
+        const client = MatrixClientPeg.safeGet();
+        if (client.isGuest()) return false;
+
+        const crypto = client.getCrypto();
+        const crossSigningReady = await crypto?.isCrossSigningReady();
+
+        return !crossSigningReady;
+    }
+
     /**
      * Called when a new logged in session has started
      */
@@ -1328,30 +1353,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
         this.themeWatcher.recheck();
         StorageManager.tryPersistStorage();
 
-        if (MatrixClientPeg.currentUserIsJustRegistered() && SettingsStore.getValue("FTUE.useCaseSelection") === null) {
-            this.setStateForNewView({ view: Views.USE_CASE_SELECTION });
-
-            // Listen to changes in settings and hide the use case screen if appropriate - this is necessary because
-            // account settings can still be changing at this point in app init (due to the initial sync being cached,
-            // then subsequent syncs being received from the server)
-            //
-            // This seems unlikely for something that should happen directly after registration, but if a user does
-            // their initial login on another device/browser than they registered on, we want to avoid asking this
-            // question twice
-            //
-            // initPosthogAnalyticsToast pioneered this technique, we’re just reusing it here.
-            SettingsStore.watchSetting(
-                "FTUE.useCaseSelection",
-                null,
-                (originalSettingName, changedInRoomId, atLevel, newValueAtLevel, newValue) => {
-                    if (newValue !== null && this.state.view === Views.USE_CASE_SELECTION) {
-                        this.onShowPostLoginScreen();
-                    }
-                },
-            );
-        } else {
-            return this.onShowPostLoginScreen();
-        }
+        this.onShowPostLoginScreen().then();
     }
 
     private async onShowPostLoginScreen(useCase?: UseCase): Promise<void> {
@@ -1557,9 +1559,6 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
             }
 
             dis.fire(Action.FocusSendMessageComposer);
-            this.setState({
-                ready: true,
-            });
         });
 
         cli.on(HttpApiEvent.SessionLoggedOut, function (errObj) {
@@ -1702,9 +1701,20 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
      * setting up anything that requires the client to be started.
      * @private
      */
-    private onClientStarted(): void {
+    private async onClientStarted(): Promise<void> {
         const cli = MatrixClientPeg.safeGet();
 
+        const shouldForceVerification = await this.shouldForceVerification();
+        // XXX: Don't replace the screen if it's already one of these: postLoginSetup
+        // changes to these screens in certain circumstances so we shouldn't clobber it.
+        // We should probably have one place where we decide what the next screen is after
+        // login.
+        if (![Views.COMPLETE_SECURITY, Views.E2E_SETUP].includes(this.state.view)) {
+            if (shouldForceVerification) {
+                this.setStateForNewView({ view: Views.COMPLETE_SECURITY });
+            }
+        }
+
         if (cli.isCryptoEnabled()) {
             const blacklistEnabled = SettingsStore.getValueAt(SettingLevel.DEVICE, "blacklistUnverifiedDevices");
             cli.setGlobalBlacklistUnverifiedDevices(blacklistEnabled);
@@ -1722,6 +1732,10 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
         if (PosthogAnalytics.instance.isEnabled() && SettingsStore.isLevelSupported(SettingLevel.ACCOUNT)) {
             this.initPosthogAnalyticsToast();
         }
+
+        this.setState({
+            ready: true,
+        });
     }
 
     public showScreen(screen: string, params?: { [key: string]: any }): void {
@@ -2013,7 +2027,30 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
 
     // complete security / e2e setup has finished
     private onCompleteSecurityE2eSetupFinished = (): void => {
-        this.onLoggedIn();
+        if (MatrixClientPeg.currentUserIsJustRegistered() && SettingsStore.getValue("FTUE.useCaseSelection") === null) {
+            this.setStateForNewView({ view: Views.USE_CASE_SELECTION });
+
+            // Listen to changes in settings and hide the use case screen if appropriate - this is necessary because
+            // account settings can still be changing at this point in app init (due to the initial sync being cached,
+            // then subsequent syncs being received from the server)
+            //
+            // This seems unlikely for something that should happen directly after registration, but if a user does
+            // their initial login on another device/browser than they registered on, we want to avoid asking this
+            // question twice
+            //
+            // initPosthogAnalyticsToast pioneered this technique, we’re just reusing it here.
+            SettingsStore.watchSetting(
+                "FTUE.useCaseSelection",
+                null,
+                (originalSettingName, changedInRoomId, atLevel, newValueAtLevel, newValue) => {
+                    if (newValue !== null && this.state.view === Views.USE_CASE_SELECTION) {
+                        this.onShowPostLoginScreen();
+                    }
+                },
+            );
+        } else {
+            this.onShowPostLoginScreen().then();
+        }
     };
 
     private getFragmentAfterLogin(): string {