From b1aaae72e34ea85ad1f7d6d5878dbec706fd0b22 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 13 Sep 2024 17:01:37 +0100 Subject: [PATCH 01/22] Force verification even for refreshed cients Set a flag on login to remember that the device needs to be verified so that we don't forget if the user refreshes the page, but still allow user with an existing unverified session to stay logged in. --- src/Lifecycle.ts | 11 +++++++++-- src/SdkConfig.ts | 1 + src/components/structures/MatrixChat.tsx | 18 ++++++++++++++++++ .../structures/auth/CompleteSecurity.tsx | 2 +- 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index e83b8df20d..0a83efcc88 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -824,8 +824,6 @@ async function doSetLoggedIn( } checkSessionLock(); - dis.fire(Action.OnLoggedIn); - const clientPegOpts: MatrixClientPegAssignOpts = {}; if (credentials.pickleKey) { // The pickleKey, if provided, is probably a base64-encoded 256-bit key, so can be used for the crypto store. @@ -846,6 +844,15 @@ 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"); + } + + // Fire this at the end, now that we've also set up crypto & started the sync running + dis.fire(Action.OnLoggedIn); + return client; } diff --git a/src/SdkConfig.ts b/src/SdkConfig.ts index 96b33923c0..abf9933e2b 100644 --- a/src/SdkConfig.ts +++ b/src/SdkConfig.ts @@ -24,6 +24,7 @@ export const DEFAULTS: DeepReadonly = { 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", diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index cae0a549b7..e6f210e387 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -1302,6 +1302,20 @@ export default class MatrixChat extends React.PureComponent { } } + private async shouldForceVerification(): Promise { + 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 */ @@ -1310,6 +1324,8 @@ export default class MatrixChat extends React.PureComponent { this.themeWatcher.recheck(); StorageManager.tryPersistStorage(); + const shouldForceVerification = await this.shouldForceVerification(); + if (MatrixClientPeg.currentUserIsJustRegistered() && SettingsStore.getValue("FTUE.useCaseSelection") === null) { this.setStateForNewView({ view: Views.USE_CASE_SELECTION }); @@ -1331,6 +1347,8 @@ export default class MatrixChat extends React.PureComponent { } }, ); + } else if (shouldForceVerification) { + this.setStateForNewView({ view: Views.COMPLETE_SECURITY }); } else { return this.onShowPostLoginScreen(); } diff --git a/src/components/structures/auth/CompleteSecurity.tsx b/src/components/structures/auth/CompleteSecurity.tsx index 568b7bffbd..a74e07692d 100644 --- a/src/components/structures/auth/CompleteSecurity.tsx +++ b/src/components/structures/auth/CompleteSecurity.tsx @@ -83,7 +83,7 @@ export default class CompleteSecurity extends React.Component { throw new Error(`Unknown phase ${phase}`); } - const forceVerification = SdkConfig.get("force_verification") ?? false; + const forceVerification = SdkConfig.get("force_verification"); let skipButton; if (!forceVerification && (phase === Phase.Intro || phase === Phase.ConfirmReset)) { From 2b10580026a908b86fcfbe406ff8a80b1c3d2d9a Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 20 Sep 2024 16:01:09 +0100 Subject: [PATCH 02/22] Hopefully make matrixchat tests pass? Much, much tweaking to make the matrixchat tests pass again. Should hopefully make them a bit more solid in general with judicious use of waitFor rather than flushPromises(). Also lots of fun to stop the state bleeding between tests. --- package.json | 1 + src/components/structures/MatrixChat.tsx | 13 +- src/components/views/auth/Welcome.tsx | 1 + .../components/structures/MatrixChat-test.tsx | 122 ++++++++---------- .../__snapshots__/MatrixChat-test.tsx.snap | 1 + .../settings/AddRemoveThreepids-test.tsx | 21 ++- yarn.lock | 36 ++---- 7 files changed, 95 insertions(+), 100 deletions(-) diff --git a/package.json b/package.json index 00301bef50..b1cd53f750 100644 --- a/package.json +++ b/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.0", "eslint-config-google": "^0.14.0", "eslint-config-prettier": "^9.0.0", diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index e6f210e387..954517b242 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/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; } interface IState { @@ -307,7 +313,12 @@ export default class MatrixChat extends React.PureComponent { * 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); }); diff --git a/src/components/views/auth/Welcome.tsx b/src/components/views/auth/Welcome.tsx index 8b2a04d54b..13755f6ca1 100644 --- a/src/components/views/auth/Welcome.tsx +++ b/src/components/views/auth/Welcome.tsx @@ -47,6 +47,7 @@ export default class Welcome extends React.PureComponent { className={classNames("mx_Welcome", { mx_WelcomePage_registrationDisabled: !SettingsStore.getValue(UIFeature.Registration), })} + data-testid="mx_welcome_screen" > diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index bae633b159..8e4e0758e5 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -6,6 +6,10 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only Please see LICENSE files in the repository root for full details. */ +// fake-indexeddb needs this and the tests crash without it +// https://github.com/dumbmatter/fakeIndexedDB?tab=readme-ov-file#jsdom-often-used-with-jest +import "core-js/stable/structured-clone"; +import "fake-indexeddb/auto"; import React, { ComponentProps } from "react"; import { fireEvent, render, RenderResult, screen, waitFor, within } from "@testing-library/react"; import fetchMock from "fetch-mock-jest"; @@ -67,12 +71,16 @@ describe("", () => { const userId = "@alice:server.org"; const deviceId = "qwertyui"; const accessToken = "abc123"; + const refreshToken = "def456"; // reused in createClient mock below const getMockClientMethods = () => ({ ...mockClientMethodsUser(userId), ...mockClientMethodsServer(), getVersions: jest.fn().mockResolvedValue({ versions: SERVER_SUPPORTED_MATRIX_VERSIONS }), - startClient: jest.fn(), + startClient: function () { + // @ts-ignore + this.emit(ClientEvent.Sync, SyncState.Prepared, null); + }, stopClient: jest.fn(), setCanResetTimelineCallback: jest.fn(), isInitialSyncComplete: jest.fn(), @@ -105,7 +113,9 @@ describe("", () => { getAccountData: jest.fn(), doesServerSupportUnstableFeature: jest.fn(), getDevices: jest.fn().mockResolvedValue({ devices: [] }), - getProfileInfo: jest.fn(), + getProfileInfo: jest.fn().mockResolvedValue({ + displayname: "Ernie", + }), getVisibleRooms: jest.fn().mockReturnValue([]), getRooms: jest.fn().mockReturnValue([]), userHasCrossSigningKeys: jest.fn(), @@ -132,6 +142,7 @@ describe("", () => { isNameResolvable: true, warning: "", }; + let initPromise: Promise | undefined; const defaultProps: ComponentProps = { config: { brand: "Test", @@ -147,6 +158,7 @@ describe("", () => { onNewScreen: jest.fn(), onTokenLoginCompleted: jest.fn(), realQueryParams: {}, + initPromiseCallback: (p: Promise) => (initPromise = p), }; const getComponent = (props: Partial> = {}) => render(); @@ -179,10 +191,6 @@ describe("", () => { // need to wait for different elements depending on which flow // without security setup we go to a loading page if (withoutSecuritySetup) { - // we think we are logged in, but are still waiting for the /sync to complete - await screen.findByText("Logout"); - // initial sync - client.emit(ClientEvent.Sync, SyncState.Prepared, null); // wait for logged in view to load await screen.findByLabelText("User menu"); @@ -202,6 +210,7 @@ describe("", () => { }; beforeEach(async () => { + initPromise = undefined; mockClient = getMockClientWithEventEmitter(getMockClientMethods()); fetchMock.get("https://test.com/_matrix/client/versions", { unstable_features: {}, @@ -213,8 +222,8 @@ describe("", () => { headers: { "content-type": "application/json" }, }); - jest.spyOn(StorageAccess, "idbLoad").mockReset(); - jest.spyOn(StorageAccess, "idbSave").mockResolvedValue(undefined); + //jest.spyOn(StorageAccess, "idbLoad").mockReset(); + //jest.spyOn(StorageAccess, "idbSave").mockResolvedValue(undefined); jest.spyOn(defaultDispatcher, "dispatch").mockClear(); jest.spyOn(defaultDispatcher, "fire").mockClear(); @@ -223,18 +232,27 @@ describe("", () => { await clearAllModals(); }); - resetJsDomAfterEach(); + afterEach(async () => { + // Wait for the promise that MatrixChat gives us to complete so that we know + // it's finished running its login code. We either need to do this or make the + // login code abort halfway through once the test finishes testing whatever it + // needs to test. If we do nothing, the login code will just continue running + // and interfere with the subsequent tests. + await initPromise; - afterEach(() => { // @ts-ignore DMRoomMap.setShared(null); jest.restoreAllMocks(); + fetchMock.mockReset(); // emit a loggedOut event so that all of the Store singletons forget about their references to the mock client - defaultDispatcher.dispatch({ action: Action.OnLoggedOut }); + // (must be sync otherwise the next test will start before it happens) + defaultDispatcher.dispatch({ action: Action.OnLoggedOut }, true); }); + resetJsDomAfterEach(); + it("should render spinner while app is loading", () => { const { container } = getComponent(); @@ -293,7 +311,7 @@ describe("", () => { expect(within(dialog).getByText(errorMessage)).toBeInTheDocument(); // just check we're back on welcome page - expect(document.querySelector(".mx_Welcome")!).toBeInTheDocument(); + await expect(await screen.findByTestId("mx_welcome_screen")).toBeInTheDocument(); }; beforeEach(() => { @@ -321,6 +339,7 @@ describe("", () => { beforeEach(() => { loginClient = getMockClientWithEventEmitter(getMockClientMethods()); + (loginClient as any).this_is_the_login_client = true; // this is used to create a temporary client during login jest.spyOn(MatrixJs, "createClient").mockReturnValue(loginClient); @@ -357,6 +376,8 @@ describe("", () => { await flushPromises(); expect(completeAuthorizationCodeGrant).toHaveBeenCalledWith(code, state); + + //await waitFor(() => expect(screen.queryByTestId("spinner")).not.toBeInTheDocument()); }); it("should look up userId using access token", async () => { @@ -390,9 +411,7 @@ describe("", () => { const onTokenLoginCompleted = jest.fn(); getComponent({ realQueryParams, onTokenLoginCompleted }); - await flushPromises(); - - expect(onTokenLoginCompleted).toHaveBeenCalled(); + await waitFor(() => expect(onTokenLoginCompleted).toHaveBeenCalled()); }); describe("when login fails", () => { @@ -456,17 +475,12 @@ describe("", () => { jest.spyOn(StorageAccess, "idbLoad").mockImplementation( async (_table: string, key: string | string[]) => (key === "mx_access_token" ? accessToken : null), ); - loginClient.getProfileInfo.mockResolvedValue({ - displayname: "Ernie", - }); }); it("should persist login credentials", async () => { getComponent({ realQueryParams }); - await flushPromises(); - - expect(localStorage.getItem("mx_hs_url")).toEqual(homeserverUrl); + await waitFor(() => expect(localStorage.getItem("mx_hs_url")).toEqual(homeserverUrl)); expect(localStorage.getItem("mx_user_id")).toEqual(userId); expect(localStorage.getItem("mx_has_access_token")).toEqual("true"); expect(localStorage.getItem("mx_device_id")).toEqual(deviceId); @@ -475,34 +489,17 @@ describe("", () => { it("should store clientId and issuer in session storage", async () => { getComponent({ realQueryParams }); - await flushPromises(); - - expect(localStorage.getItem("mx_oidc_client_id")).toEqual(clientId); + await waitFor(() => expect(localStorage.getItem("mx_oidc_client_id")).toEqual(clientId)); expect(localStorage.getItem("mx_oidc_token_issuer")).toEqual(issuer); }); it("should set logged in and start MatrixClient", async () => { getComponent({ realQueryParams }); - await flushPromises(); - await flushPromises(); - - expect(logger.log).toHaveBeenCalledWith( - "setLoggedIn: mxid: " + - userId + - " deviceId: " + - deviceId + - " guest: " + - false + - " hs: " + - homeserverUrl + - " softLogout: " + - false, - " freshLogin: " + true, - ); - // client successfully started - expect(defaultDispatcher.dispatch).toHaveBeenCalledWith({ action: "client_started" }); + await waitFor(() => + expect(defaultDispatcher.dispatch).toHaveBeenCalledWith({ action: "client_started" }), + ); // check we get to logged in view await waitForSyncAndLoad(loginClient, true); @@ -540,8 +537,9 @@ describe("", () => { describe("with an existing session", () => { const mockidb: Record> = { - acccount: { + account: { mx_access_token: accessToken, + mx_refresh_token: refreshToken, }, }; @@ -585,10 +583,10 @@ describe("", () => { // wait for logged in view to load await screen.findByLabelText("User menu"); - // let things settle - await flushPromises(); + expect(screen.queryByRole("progressbar")).not.toBeInTheDocument(); - expect(screen.getByText(`Welcome ${userId}`)).toBeInTheDocument(); + const h1Element = screen.getByRole("heading", { level: 1 }); + expect(h1Element).toHaveTextContent(`Welcome ${userId}`); }); describe("clean up drafts", () => { @@ -883,7 +881,7 @@ describe("", () => { // stuff that happens in onloggedout expect(defaultDispatcher.fire).toHaveBeenCalledWith(Action.OnLoggedOut, true); - expect(logoutClient.clearStores).toHaveBeenCalled(); + await waitFor(() => expect(logoutClient.clearStores).toHaveBeenCalled()); }); it("should do post-logout cleanup", async () => { @@ -892,7 +890,7 @@ describe("", () => { // stuff that happens in onloggedout expect(defaultDispatcher.fire).toHaveBeenCalledWith(Action.OnLoggedOut, true); - expect(EventIndexPeg.deleteEventIndex).toHaveBeenCalled(); + await waitFor(() => expect(EventIndexPeg.deleteEventIndex).toHaveBeenCalled()); expect(logoutClient.clearStores).toHaveBeenCalled(); }); }); @@ -980,10 +978,6 @@ describe("", () => { user_id: userId, }); loginClient.loginFlows.mockClear().mockResolvedValue({ flows: [{ type: "m.login.password" }] }); - - loginClient.getProfileInfo.mockResolvedValue({ - displayname: "Ernie", - }); }); it("should render login page", async () => { @@ -1171,9 +1165,7 @@ describe("", () => { const onTokenLoginCompleted = jest.fn(); getComponent({ realQueryParams, onTokenLoginCompleted }); - await flushPromises(); - - expect(onTokenLoginCompleted).toHaveBeenCalled(); + await waitFor(() => expect(onTokenLoginCompleted).toHaveBeenCalled()); }); describe("when login fails", () => { @@ -1230,9 +1222,7 @@ describe("", () => { it("should persist login credentials", async () => { getComponent({ realQueryParams }); - await flushPromises(); - - expect(localStorage.getItem("mx_hs_url")).toEqual(serverConfig.hsUrl); + await waitFor(() => expect(localStorage.getItem("mx_hs_url")).toEqual(serverConfig.hsUrl)); expect(localStorage.getItem("mx_user_id")).toEqual(userId); expect(localStorage.getItem("mx_has_access_token")).toEqual("true"); expect(localStorage.getItem("mx_device_id")).toEqual(deviceId); @@ -1242,8 +1232,7 @@ describe("", () => { const sessionStorageSetSpy = jest.spyOn(sessionStorage.__proto__, "setItem"); getComponent({ realQueryParams }); - await flushPromises(); - expect(sessionStorageSetSpy).toHaveBeenCalledWith("mx_fresh_login", "true"); + await waitFor(() => expect(sessionStorageSetSpy).toHaveBeenCalledWith("mx_fresh_login", "true")); }); it("should override hsUrl in creds when login response wellKnown differs from config", async () => { @@ -1310,8 +1299,10 @@ describe("", () => { screen: "start_sso", }, }); - await flushPromises(); - expect(ssoClient.getSsoLoginUrl).toHaveBeenCalledWith("http://localhost/", "sso", undefined, undefined); + + await waitFor(() => + expect(ssoClient.getSsoLoginUrl).toHaveBeenCalledWith("http://localhost/", "sso", undefined, undefined), + ); expect(window.localStorage.getItem(SSO_HOMESERVER_URL_KEY)).toEqual("matrix.example.com"); expect(window.localStorage.getItem(SSO_ID_SERVER_URL_KEY)).toEqual("ident.example.com"); expect(hrefSetter).toHaveBeenCalledWith("http://my-sso-url"); @@ -1323,8 +1314,10 @@ describe("", () => { screen: "start_cas", }, }); - await flushPromises(); - expect(ssoClient.getSsoLoginUrl).toHaveBeenCalledWith("http://localhost/", "cas", undefined, undefined); + + await waitFor(() => + expect(ssoClient.getSsoLoginUrl).toHaveBeenCalledWith("http://localhost/", "cas", undefined, undefined), + ); expect(window.localStorage.getItem(SSO_HOMESERVER_URL_KEY)).toEqual("matrix.example.com"); expect(window.localStorage.getItem(SSO_ID_SERVER_URL_KEY)).toEqual("ident.example.com"); expect(hrefSetter).toHaveBeenCalledWith("http://my-sso-url"); @@ -1388,7 +1381,6 @@ describe("", () => { const client = getMockClientWithEventEmitter(getMockClientMethods()); jest.spyOn(MatrixJs, "createClient").mockReturnValue(client); - client.getProfileInfo.mockResolvedValue({ displayname: "Ernie" }); const rendered = getComponent({}); await waitForSyncAndLoad(client, true); diff --git a/test/components/structures/__snapshots__/MatrixChat-test.tsx.snap b/test/components/structures/__snapshots__/MatrixChat-test.tsx.snap index 3234c45741..71bde418ff 100644 --- a/test/components/structures/__snapshots__/MatrixChat-test.tsx.snap +++ b/test/components/structures/__snapshots__/MatrixChat-test.tsx.snap @@ -117,6 +117,7 @@ exports[` Multi-tab lockout waits for other tab to stop during sta >
{ await userEvent.type(input, PHONE1_LOCALNUM); const addButton = screen.getByRole("button", { name: "Add" }); - await userEvent.click(addButton); + userEvent.click(addButton); + + const continueButton = await screen.findByRole("button", { name: "Continue" }); + + await expect(continueButton).toHaveAttribute("aria-disabled", "true"); + + await expect( + await screen.findByText( + `A text message has been sent to +${PHONE1.address}. Please enter the verification code it contains.`, + ), + ).toBeInTheDocument(); expect(client.requestAdd3pidMsisdnToken).toHaveBeenCalledWith( "GB", @@ -226,15 +236,14 @@ describe("AddRemoveThreepids", () => { client.generateClientSecret(), 1, ); - const continueButton = screen.getByRole("button", { name: "Continue" }); - - expect(continueButton).toHaveAttribute("aria-disabled", "true"); const verificationInput = screen.getByRole("textbox", { name: "Verification code" }); await userEvent.type(verificationInput, "123456"); expect(continueButton).not.toHaveAttribute("aria-disabled", "true"); - await userEvent.click(continueButton); + userEvent.click(continueButton); + + await waitFor(() => expect(continueButton).toHaveAttribute("aria-disabled", "true")); expect(client.addThreePidOnly).toHaveBeenCalledWith({ client_secret: client.generateClientSecret(), diff --git a/yarn.lock b/yarn.lock index f872acbe5f..93edc8cfb7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3976,6 +3976,11 @@ core-js@^3.0.0: resolved "https://registry.yarnpkg.com/core-js/-/core-js-3.30.0.tgz#64ac6f83bc7a49fd42807327051701d4b1478dea" integrity sha512-hQotSSARoNh1mYPi9O2YaWeiq/cEB95kOrFb4NCrO4RIFt1qqNpKsaE+vy/L3oiqvND5cThqXzUU3r9F7Efztg== +core-js@^3.38.1: + version "3.38.1" + resolved "https://registry.yarnpkg.com/core-js/-/core-js-3.38.1.tgz#aa375b79a286a670388a1a363363d53677c0383e" + integrity sha512-OP35aUorbU3Zvlx7pjsFdu1rGNnD4pgw/CWoYzRY3t2EzoVT7shKHY1dlAy3f41cGIO7ZDPQimhGFTlEYkG/Hw== + core-util-is@~1.0.0: version "1.0.3" resolved "https://registry.yarnpkg.com/core-util-is/-/core-util-is-1.0.3.tgz#a6042d3634c2b27e9328f837b965fac83808db85" @@ -8670,16 +8675,7 @@ string-length@^4.0.1: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0": - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - -string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -8775,14 +8771,7 @@ string_decoder@~1.1.1: dependencies: safe-buffer "~5.1.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - -strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -9544,7 +9533,7 @@ which@^2.0.1: dependencies: isexe "^2.0.0" -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -9562,15 +9551,6 @@ wrap-ansi@^6.2.0: string-width "^4.1.0" strip-ansi "^6.0.0" -wrap-ansi@^7.0.0: - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214" From 255c7dc8ed24962ce81469019790ddfaaf5c85c7 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 20 Sep 2024 16:45:45 +0100 Subject: [PATCH 03/22] Manual yarn.lock manipulation to hopefully resolve infinite package sadness --- yarn.lock | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index 178e5a396e..5f502910dc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8724,7 +8724,16 @@ string-length@^4.0.1: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0": + version "4.2.3" + resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" + integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== + dependencies: + emoji-regex "^8.0.0" + is-fullwidth-code-point "^3.0.0" + strip-ansi "^6.0.1" + +string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -8820,7 +8829,14 @@ string_decoder@~1.1.1: dependencies: safe-buffer "~5.1.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1": + version "6.0.1" + resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" + integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== + dependencies: + ansi-regex "^5.0.1" + +strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -9582,7 +9598,8 @@ which@^2.0.1: dependencies: isexe "^2.0.0" -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": + name wrap-ansi-cjs version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -9600,6 +9617,15 @@ wrap-ansi@^6.2.0: string-width "^4.1.0" strip-ansi "^6.0.0" +wrap-ansi@^7.0.0: + version "7.0.0" + resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" + integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== + dependencies: + ansi-styles "^4.0.0" + string-width "^4.1.0" + strip-ansi "^6.0.0" + wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214" From e98656229c53921839c5b87ad0347beddb7c451f Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 23 Sep 2024 18:33:06 +0100 Subject: [PATCH 04/22] Make final test pass(?) Mock out the createClient method to return the same client, because we've mocked the peg to always return that client, so if we let the code make another one having still overridden the peg, everything becomes cursed. Also mock out the autodiscovery stuff rather than relying on fetch-mock. --- .../components/structures/MatrixChat-test.tsx | 31 +++++-------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index d24fbffe21..a92a26e752 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -60,6 +60,8 @@ import DMRoomMap from "../../../src/utils/DMRoomMap"; import { ReleaseAnnouncementStore } from "../../../src/stores/ReleaseAnnouncementStore"; import { DRAFT_LAST_CLEANUP_KEY } from "../../../src/DraftCleaner"; import { UIFeature } from "../../../src/settings/UIFeature"; +import AutoDiscoveryUtils from "../../../src/utils/AutoDiscoveryUtils"; +import { ValidatedServerConfig } from "../../../src/utils/ValidatedServerConfig"; jest.mock("matrix-js-sdk/src/oidc/authorize", () => ({ completeAuthorizationCodeGrant: jest.fn(), @@ -213,23 +215,17 @@ describe("", () => { beforeEach(async () => { initPromise = undefined; mockClient = getMockClientWithEventEmitter(getMockClientMethods()); - fetchMock.get("https://test.com/_matrix/client/versions", { - unstable_features: {}, - versions: SERVER_SUPPORTED_MATRIX_VERSIONS, - }); - fetchMock.catch({ - status: 404, - body: '{"errcode": "M_UNRECOGNIZED", "error": "Unrecognized request"}', - headers: { "content-type": "application/json" }, - }); + jest.spyOn(MatrixJs, "createClient").mockReturnValue(mockClient); - //jest.spyOn(StorageAccess, "idbLoad").mockReset(); - //jest.spyOn(StorageAccess, "idbSave").mockResolvedValue(undefined); jest.spyOn(defaultDispatcher, "dispatch").mockClear(); jest.spyOn(defaultDispatcher, "fire").mockClear(); DMRoomMap.makeShared(mockClient); + jest.spyOn(AutoDiscoveryUtils, "validateServerConfigWithStaticUrls").mockResolvedValue( + {} as ValidatedServerConfig, + ); + await clearAllModals(); }); @@ -245,7 +241,6 @@ describe("", () => { DMRoomMap.setShared(null); jest.restoreAllMocks(); - fetchMock.mockReset(); // emit a loggedOut event so that all of the Store singletons forget about their references to the mock client // (must be sync otherwise the next test will start before it happens) @@ -340,7 +335,6 @@ describe("", () => { beforeEach(() => { loginClient = getMockClientWithEventEmitter(getMockClientMethods()); - (loginClient as any).this_is_the_login_client = true; // this is used to create a temporary client during login jest.spyOn(MatrixJs, "createClient").mockReturnValue(loginClient); @@ -573,21 +567,12 @@ describe("", () => { it("should render welcome page after login", async () => { getComponent(); - // we think we are logged in, but are still waiting for the /sync to complete - const logoutButton = await screen.findByText("Logout"); - - expect(logoutButton).toBeInTheDocument(); - expect(screen.getByRole("progressbar")).toBeInTheDocument(); - - // initial sync - mockClient.emit(ClientEvent.Sync, SyncState.Prepared, null); - // wait for logged in view to load await screen.findByLabelText("User menu"); expect(screen.queryByRole("progressbar")).not.toBeInTheDocument(); const h1Element = screen.getByRole("heading", { level: 1 }); - expect(h1Element).toHaveTextContent(`Welcome ${userId}`); + expect(h1Element).toHaveTextContent(`Welcome Ernie`); }); describe("clean up drafts", () => { From 80c75d4d5bdecddba3da34c0356723ea3c974408 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 24 Sep 2024 11:08:28 +0100 Subject: [PATCH 05/22] another waitFor --- test/components/structures/MatrixChat-test.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index a92a26e752..d2070ab828 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -1197,10 +1197,8 @@ describe("", () => { getComponent({ realQueryParams }); - await flushPromises(); - // just check we called the clearStorage function - expect(loginClient.clearStores).toHaveBeenCalled(); + await waitFor(() => expect(loginClient.clearStores).toHaveBeenCalled()); expect(localStorage.getItem("mx_sso_hs_url")).toBe(null); expect(localStorageClearSpy).toHaveBeenCalled(); }); From b07203e909e6d6ddd0077c5a8df92eb3034b4a02 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 24 Sep 2024 11:21:00 +0100 Subject: [PATCH 06/22] death to flushPromises --- test/components/structures/MatrixChat-test.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index d2070ab828..e05c93424b 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -1232,9 +1232,7 @@ describe("", () => { loginClient.login.mockResolvedValue(loginResponseWithWellKnown); getComponent({ realQueryParams }); - await flushPromises(); - - expect(localStorage.getItem("mx_hs_url")).toEqual(hsUrlFromWk); + await waitFor(() => expect(localStorage.getItem("mx_hs_url")).toEqual(hsUrlFromWk)); }); it("should continue to post login setup when no session is found in local storage", async () => { From f8c7db445f3d8eea007233c644af4ab0e07bb093 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 24 Sep 2024 17:11:25 +0100 Subject: [PATCH 07/22] Put the logged in dispatch back Actually it breaks all sorts of other things too, having fixed all the MatrixChat tests (although this is useful anyway). --- src/Lifecycle.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 0a83efcc88..177b712769 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -824,6 +824,10 @@ 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 = {}; if (credentials.pickleKey) { // The pickleKey, if provided, is probably a base64-encoded 256-bit key, so can be used for the crypto store. @@ -850,9 +854,6 @@ async function doSetLoggedIn( localStorage.setItem("must_verify_device", "true"); } - // Fire this at the end, now that we've also set up crypto & started the sync running - dis.fire(Action.OnLoggedIn); - return client; } From bc88a9ae364ea341d73663874ed7e80a74d6c277 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 24 Sep 2024 18:48:45 +0100 Subject: [PATCH 08/22] Try displaying the screen in onClientStarted instead --- src/components/structures/MatrixChat.tsx | 26 ++++++++++++------- .../components/structures/MatrixChat-test.tsx | 4 ++- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index e0731ac1cc..4ba29eabcb 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -892,7 +892,7 @@ export default class MatrixChat extends React.PureComponent { }); break; case "client_started": - this.onClientStarted(); + this.onClientStarted().then(); break; case "send_event": this.onSendEvent(payload.room_id, payload.event); @@ -1353,8 +1353,6 @@ export default class MatrixChat extends React.PureComponent { this.themeWatcher.recheck(); StorageManager.tryPersistStorage(); - const shouldForceVerification = await this.shouldForceVerification(); - if (MatrixClientPeg.currentUserIsJustRegistered() && SettingsStore.getValue("FTUE.useCaseSelection") === null) { this.setStateForNewView({ view: Views.USE_CASE_SELECTION }); @@ -1376,10 +1374,6 @@ export default class MatrixChat extends React.PureComponent { } }, ); - } else if (shouldForceVerification) { - this.setStateForNewView({ view: Views.COMPLETE_SECURITY }); - } else { - return this.onShowPostLoginScreen(); } } @@ -1731,9 +1725,23 @@ export default class MatrixChat extends React.PureComponent { * setting up anything that requires the client to be started. * @private */ - private onClientStarted(): void { + private async onClientStarted(): Promise { const cli = MatrixClientPeg.safeGet(); + // 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)) { + const shouldForceVerification = await this.shouldForceVerification(); + + if (shouldForceVerification) { + this.setStateForNewView({ view: Views.COMPLETE_SECURITY }); + } else { + await this.onShowPostLoginScreen(); + } + } + if (cli.isCryptoEnabled()) { const blacklistEnabled = SettingsStore.getValueAt(SettingLevel.DEVICE, "blacklistUnverifiedDevices"); cli.setGlobalBlacklistUnverifiedDevices(blacklistEnabled); @@ -2042,7 +2050,7 @@ export default class MatrixChat extends React.PureComponent { // complete security / e2e setup has finished private onCompleteSecurityE2eSetupFinished = (): void => { - this.onLoggedIn(); + this.onShowPostLoginScreen().then(); }; private getFragmentAfterLogin(): string { diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index e05c93424b..4fa4d01fd2 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -1027,7 +1027,9 @@ describe("", () => { }, }); - loginClient.isRoomEncrypted.mockImplementation((roomId) => roomId === encryptedRoom.roomId); + loginClient.isRoomEncrypted.mockImplementation((roomId) => { + return roomId === encryptedRoom.roomId; + }); }); it("should go straight to logged in view when user is not in any encrypted rooms", async () => { From bd2ca31240a43340d375e2fcf3d6a55e8fe7ae84 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 25 Sep 2024 13:38:26 +0100 Subject: [PATCH 09/22] Put post login screen back in logged in but move ready transition to avoid flash of main UI --- src/components/structures/MatrixChat.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 4ba29eabcb..f6f1f2298f 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -1374,6 +1374,8 @@ export default class MatrixChat extends React.PureComponent { } }, ); + } else { + await this.onShowPostLoginScreen(); } } @@ -1580,9 +1582,6 @@ export default class MatrixChat extends React.PureComponent { } dis.fire(Action.FocusSendMessageComposer); - this.setState({ - ready: true, - }); }); cli.on(HttpApiEvent.SessionLoggedOut, function (errObj) { @@ -1737,8 +1736,6 @@ export default class MatrixChat extends React.PureComponent { if (shouldForceVerification) { this.setStateForNewView({ view: Views.COMPLETE_SECURITY }); - } else { - await this.onShowPostLoginScreen(); } } @@ -1759,6 +1756,10 @@ export default class MatrixChat extends React.PureComponent { if (PosthogAnalytics.instance.isEnabled() && SettingsStore.isLevelSupported(SettingLevel.ACCOUNT)) { this.initPosthogAnalyticsToast(); } + + this.setState({ + ready: true, + }); } public showScreen(screen: string, params?: { [key: string]: any }): void { From 14718628397ebe4a7e2b0d1617f88d7fd6e94e28 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 25 Sep 2024 14:20:28 +0100 Subject: [PATCH 10/22] Rejig more in the hope it does the right thing --- src/components/structures/MatrixChat.tsx | 53 ++++++++++++------------ 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index f6f1f2298f..dc7935c4a6 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -1353,30 +1353,7 @@ export default class MatrixChat extends React.PureComponent { 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 { - await this.onShowPostLoginScreen(); - } + this.onShowPostLoginScreen().then(); } private async onShowPostLoginScreen(useCase?: UseCase): Promise { @@ -1727,13 +1704,12 @@ export default class MatrixChat extends React.PureComponent { private async onClientStarted(): Promise { 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)) { - const shouldForceVerification = await this.shouldForceVerification(); - if (shouldForceVerification) { this.setStateForNewView({ view: Views.COMPLETE_SECURITY }); } @@ -2051,7 +2027,30 @@ export default class MatrixChat extends React.PureComponent { // complete security / e2e setup has finished private onCompleteSecurityE2eSetupFinished = (): void => { - this.onShowPostLoginScreen().then(); + 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 { From e88d1e047590dbe059752a26402f348d32071fa6 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 26 Sep 2024 14:55:08 +0100 Subject: [PATCH 11/22] Make hook work before push rules are fetched --- src/hooks/useUserOnboardingContext.ts | 31 +++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/hooks/useUserOnboardingContext.ts b/src/hooks/useUserOnboardingContext.ts index 37d9650e25..15f5eff67b 100644 --- a/src/hooks/useUserOnboardingContext.ts +++ b/src/hooks/useUserOnboardingContext.ts @@ -77,14 +77,37 @@ function useUserOnboardingContextValue(defaultValue: T, callback: (cli: Matri } function useShowNotificationsPrompt(): boolean { - const [value, setValue] = useState(Notifier.shouldShowPrompt()); + const client = useMatrixClientContext(); + + const [value, setValue] = useState(client.pushRules ? Notifier.shouldShowPrompt() : true); + + const updateValue = useCallback(() => { + setValue(client.pushRules ? Notifier.shouldShowPrompt() : true); + }, [client]); + useEventEmitter(Notifier, NotifierEvent.NotificationHiddenChange, () => { - setValue(Notifier.shouldShowPrompt()); + updateValue(); }); + const setting = useSettingValue("notificationsEnabled"); useEffect(() => { - setValue(Notifier.shouldShowPrompt()); - }, [setting]); + updateValue(); + }, [setting, updateValue]); + + // shouldShowPrompt is dependent on the client having push rules. There isn't an event for the client + // fetching its push rules, but we'll know it has them by the time it sync, so we update this on sync. + useEffect(() => { + const onSync = (): void => { + updateValue(); + }; + + client.on(ClientEvent.Sync, onSync); + + return () => { + client.removeListener(ClientEvent.Sync, onSync); + }; + }, [client, updateValue]); + return value; } From 579d507fea34ac2787d70858a69ab473cb0219cf Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 26 Sep 2024 17:52:35 +0100 Subject: [PATCH 12/22] Add test for unskippable verification --- .../components/structures/MatrixChat-test.tsx | 56 +++++++++++++------ 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 4fa4d01fd2..12b0a2783d 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -124,7 +124,13 @@ describe("", () => { userHasCrossSigningKeys: jest.fn(), setGlobalBlacklistUnverifiedDevices: jest.fn(), setGlobalErrorOnUnknownDevices: jest.fn(), - getCrypto: jest.fn(), + getCrypto: jest.fn().mockReturnValue({ + getVerificationRequestsToDeviceInProgress: jest.fn().mockReturnValue([]), + isCrossSigningReady: jest.fn().mockReturnValue(false), + getUserDeviceInfo: jest.fn().mockReturnValue(new Map()), + getUserVerificationStatus: jest.fn().mockResolvedValue(new UserVerificationStatus(false, false, false)), + getVersion: jest.fn().mockReturnValue("1"), + }), secretStorage: { isStored: jest.fn().mockReturnValue(null), }, @@ -146,23 +152,7 @@ describe("", () => { warning: "", }; let initPromise: Promise | undefined; - const defaultProps: ComponentProps = { - config: { - brand: "Test", - help_url: "help_url", - help_encryption_url: "help_encryption_url", - element_call: {}, - feedback: { - existing_issues_url: "https://feedback.org/existing", - new_issue_url: "https://feedback.org/new", - }, - validated_server_config: serverConfig, - }, - onNewScreen: jest.fn(), - onTokenLoginCompleted: jest.fn(), - realQueryParams: {}, - initPromiseCallback: (p: Promise) => (initPromise = p), - }; + let defaultProps: ComponentProps; const getComponent = (props: Partial> = {}) => render(); @@ -213,6 +203,24 @@ describe("", () => { }; beforeEach(async () => { + defaultProps = { + config: { + brand: "Test", + help_url: "help_url", + help_encryption_url: "help_encryption_url", + element_call: {}, + feedback: { + existing_issues_url: "https://feedback.org/existing", + new_issue_url: "https://feedback.org/new", + }, + validated_server_config: serverConfig, + }, + onNewScreen: jest.fn(), + onTokenLoginCompleted: jest.fn(), + realQueryParams: {}, + initPromiseCallback: (p: Promise) => (initPromise = p), + }; + initPromise = undefined; mockClient = getMockClientWithEventEmitter(getMockClientMethods()); jest.spyOn(MatrixJs, "createClient").mockReturnValue(mockClient); @@ -245,6 +253,8 @@ describe("", () => { // emit a loggedOut event so that all of the Store singletons forget about their references to the mock client // (must be sync otherwise the next test will start before it happens) defaultDispatcher.dispatch({ action: Action.OnLoggedOut }, true); + + localStorage.clear(); }); resetJsDomAfterEach(); @@ -882,6 +892,16 @@ describe("", () => { }); }); }); + + describe("unskippable verification", () => { + it("should show the complete security screen if unskippable verification is enabled", async () => { + defaultProps.config.force_verification = true; + localStorage.setItem("must_verify_device", "true"); + getComponent(); + + await screen.findByRole("heading", { name: "Unable to verify this device", level: 1 }); + }); + }); }); describe("with a soft-logged-out session", () => { From 60b8eaf7b2da3f6d0ec1a3a554c2d67ab161c927 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 27 Sep 2024 11:33:11 +0100 Subject: [PATCH 13/22] Add test for use case selection --- test/components/structures/MatrixChat-test.tsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 12b0a2783d..f4f1e52e42 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -55,7 +55,7 @@ import * as Lifecycle from "../../../src/Lifecycle"; import { SSO_HOMESERVER_URL_KEY, SSO_ID_SERVER_URL_KEY } from "../../../src/BasePlatform"; import SettingsStore from "../../../src/settings/SettingsStore"; import { SettingLevel } from "../../../src/settings/SettingLevel"; -import { MatrixClientPeg as peg } from "../../../src/MatrixClientPeg"; +import { MatrixClientPeg, MatrixClientPeg as peg } from "../../../src/MatrixClientPeg"; import DMRoomMap from "../../../src/utils/DMRoomMap"; import { ReleaseAnnouncementStore } from "../../../src/stores/ReleaseAnnouncementStore"; import { DRAFT_LAST_CLEANUP_KEY } from "../../../src/DraftCleaner"; @@ -131,6 +131,7 @@ describe("", () => { getUserVerificationStatus: jest.fn().mockResolvedValue(new UserVerificationStatus(false, false, false)), getVersion: jest.fn().mockReturnValue("1"), }), + bootstrapCrossSigning: jest.fn(), secretStorage: { isStored: jest.fn().mockReturnValue(null), }, @@ -1110,6 +1111,15 @@ describe("", () => { // set up keys screen is rendered expect(screen.getByText("Setting up keys")).toBeInTheDocument(); }); + + it("should go to use case selection if user just registered", async () => { + loginClient.doesServerSupportUnstableFeature.mockResolvedValue(true); + MatrixClientPeg.setJustRegisteredUserId(userId); + + await getComponentAndLogin(); + + await expect(await screen.findByRole("heading", { name: "You're in", level: 1 })).toBeInTheDocument(); + }); }); }); From 1e2525b8d2f9ce568f416056ef92efd0f07c205a Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 27 Sep 2024 13:30:03 +0100 Subject: [PATCH 14/22] Fix test --- test/components/structures/MatrixChat-test.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index f4f1e52e42..71811d580e 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -21,7 +21,7 @@ import { completeAuthorizationCodeGrant } from "matrix-js-sdk/src/oidc/authorize import { logger } from "matrix-js-sdk/src/logger"; import { OidcError } from "matrix-js-sdk/src/oidc/error"; import { BearerTokenResponse } from "matrix-js-sdk/src/oidc/validate"; -import { defer, sleep } from "matrix-js-sdk/src/utils"; +import { defer, IDeferred, sleep } from "matrix-js-sdk/src/utils"; import { UserVerificationStatus } from "matrix-js-sdk/src/crypto-api"; import MatrixChat from "../../../src/components/structures/MatrixChat"; @@ -75,6 +75,7 @@ describe("", () => { const deviceId = "qwertyui"; const accessToken = "abc123"; const refreshToken = "def456"; + let bootstrapDeferred: IDeferred; // reused in createClient mock below const getMockClientMethods = () => ({ ...mockClientMethodsUser(userId), @@ -131,7 +132,8 @@ describe("", () => { getUserVerificationStatus: jest.fn().mockResolvedValue(new UserVerificationStatus(false, false, false)), getVersion: jest.fn().mockReturnValue("1"), }), - bootstrapCrossSigning: jest.fn(), + // This needs to not finish immediately because we need to test the screen appears + bootstrapCrossSigning: jest.fn().mockImplementation(() => bootstrapDeferred.promise), secretStorage: { isStored: jest.fn().mockReturnValue(null), }, @@ -235,6 +237,8 @@ describe("", () => { {} as ValidatedServerConfig, ); + bootstrapDeferred = defer(); + await clearAllModals(); }); @@ -1079,10 +1083,8 @@ describe("", () => { expect(loginClient.userHasCrossSigningKeys).toHaveBeenCalled(); - await flushPromises(); - // set up keys screen is rendered - expect(screen.getByText("Setting up keys")).toBeInTheDocument(); + await expect(await screen.findByText("Setting up keys")).toBeInTheDocument(); }); }); @@ -1118,6 +1120,8 @@ describe("", () => { await getComponentAndLogin(); + bootstrapDeferred.resolve(); + await expect(await screen.findByRole("heading", { name: "You're in", level: 1 })).toBeInTheDocument(); }); }); From 860deddc9b8ac40425648b8643caf326d9a330dd Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 27 Sep 2024 19:07:47 +0100 Subject: [PATCH 15/22] Add playwright test for unskippable verification --- playwright/e2e/login/login.spec.ts | 166 +++++++++++++++++++++++++++-- 1 file changed, 156 insertions(+), 10 deletions(-) diff --git a/playwright/e2e/login/login.spec.ts b/playwright/e2e/login/login.spec.ts index 9337cc57e8..38bee61e0c 100644 --- a/playwright/e2e/login/login.spec.ts +++ b/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,98 @@ 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, + }) => { + 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); + + 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 From ee7a3e8cfd8bd7efd638e09a9d53e9e5816d54f7 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 30 Sep 2024 11:01:21 +0100 Subject: [PATCH 16/22] Remove console log --- playwright/e2e/login/login.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/playwright/e2e/login/login.spec.ts b/playwright/e2e/login/login.spec.ts index 38bee61e0c..5b251c9bb4 100644 --- a/playwright/e2e/login/login.spec.ts +++ b/playwright/e2e/login/login.spec.ts @@ -171,7 +171,6 @@ test.describe("Login", () => { 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 }, From c2c3e4a64a6c73e683135f1ba1741dc131a63901 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 2 Oct 2024 13:47:55 +0100 Subject: [PATCH 17/22] Add log message to log line --- playwright/e2e/login/login.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/playwright/e2e/login/login.spec.ts b/playwright/e2e/login/login.spec.ts index 5b251c9bb4..696ecb183c 100644 --- a/playwright/e2e/login/login.spec.ts +++ b/playwright/e2e/login/login.spec.ts @@ -147,7 +147,7 @@ test.describe("Login", () => { { headers: { Authorization: `Bearer ${creds.accessToken}` }, data: DEVICE_SIGNING_KEYS_BODY }, ); if (res.status() / 100 !== 2) { - console.log(await res.json()); + console.log("Uploading dummy keys failed", await res.json()); } expect(res.status() / 100).toEqual(2); @@ -176,7 +176,7 @@ test.describe("Login", () => { { headers: { Authorization: `Bearer ${creds.accessToken}` }, data: DEVICE_SIGNING_KEYS_BODY }, ); if (res.status() / 100 !== 2) { - console.log(await res.json()); + console.log("Uploading dummy keys failed", await res.json()); } expect(res.status() / 100).toEqual(2); @@ -207,7 +207,7 @@ test.describe("Login", () => { { headers: { Authorization: `Bearer ${creds.accessToken}` }, data: DEVICE_SIGNING_KEYS_BODY }, ); if (res.status() / 100 !== 2) { - console.log(await res.json()); + console.log("Uploading dummy keys failed", await res.json()); } expect(res.status() / 100).toEqual(2); From 71249a316d26586a95803bd13dde1b22c1ff43a9 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 2 Oct 2024 13:56:34 +0100 Subject: [PATCH 18/22] Add tsdoc --- src/components/structures/MatrixChat.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index dc7935c4a6..693a9ae503 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -1331,6 +1331,11 @@ export default class MatrixChat extends React.PureComponent { } } + /** + * Returns true if the user must go through the device verification process before they + * can use the app. + * @returns true if the user must verify + */ private async shouldForceVerification(): Promise { if (!SdkConfig.get("force_verification")) return false; const mustVerifyFlag = localStorage.getItem("must_verify_device"); From 1b04bf6ff5f9327b70e4ab5ccfcd357b88c730eb Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 2 Oct 2024 14:13:28 +0100 Subject: [PATCH 19/22] Use useTypedEventEmitter --- src/hooks/useUserOnboardingContext.ts | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/hooks/useUserOnboardingContext.ts b/src/hooks/useUserOnboardingContext.ts index 15f5eff67b..8bd93603d4 100644 --- a/src/hooks/useUserOnboardingContext.ts +++ b/src/hooks/useUserOnboardingContext.ts @@ -14,7 +14,7 @@ import { Notifier, NotifierEvent } from "../Notifier"; import DMRoomMap from "../utils/DMRoomMap"; import { useMatrixClientContext } from "../contexts/MatrixClientContext"; import { useSettingValue } from "./useSettings"; -import { useEventEmitter } from "./useEventEmitter"; +import { useEventEmitter, useTypedEventEmitter } from "./useEventEmitter"; export interface UserOnboardingContext { hasAvatar: boolean; @@ -96,17 +96,7 @@ function useShowNotificationsPrompt(): boolean { // shouldShowPrompt is dependent on the client having push rules. There isn't an event for the client // fetching its push rules, but we'll know it has them by the time it sync, so we update this on sync. - useEffect(() => { - const onSync = (): void => { - updateValue(); - }; - - client.on(ClientEvent.Sync, onSync); - - return () => { - client.removeListener(ClientEvent.Sync, onSync); - }; - }, [client, updateValue]); + useTypedEventEmitter(client, ClientEvent.Sync, updateValue); return value; } From 677f6160aa0767fc0c9c4609be686226289006d6 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 2 Oct 2024 14:14:14 +0100 Subject: [PATCH 20/22] Remove commented code --- test/components/structures/MatrixChat-test.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 71811d580e..79aa0c709d 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -386,8 +386,6 @@ describe("", () => { await flushPromises(); expect(completeAuthorizationCodeGrant).toHaveBeenCalledWith(code, state); - - //await waitFor(() => expect(screen.queryByTestId("spinner")).not.toBeInTheDocument()); }); it("should look up userId using access token", async () => { From 4a84e3d98fb7ba54ab07465930ca686258bf71e1 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 2 Oct 2024 14:30:32 +0100 Subject: [PATCH 21/22] Use catch instead of empty then on unawaited promises or in one case just await it because the caller was async anyway --- src/components/structures/MatrixChat.tsx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 693a9ae503..f3b75dfce8 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -892,7 +892,10 @@ export default class MatrixChat extends React.PureComponent { }); break; case "client_started": - this.onClientStarted().then(); + // No need to make this handler async to wait for the result of this + this.onClientStarted().catch((e) => { + logger.error("Exception in onClientStarted", e); + }); break; case "send_event": this.onSendEvent(payload.room_id, payload.event); @@ -1358,7 +1361,7 @@ export default class MatrixChat extends React.PureComponent { this.themeWatcher.recheck(); StorageManager.tryPersistStorage(); - this.onShowPostLoginScreen().then(); + await this.onShowPostLoginScreen(); } private async onShowPostLoginScreen(useCase?: UseCase): Promise { @@ -2054,7 +2057,10 @@ export default class MatrixChat extends React.PureComponent { }, ); } else { - this.onShowPostLoginScreen().then(); + // This is async but we makign this function async to wait for it isn't useful + this.onShowPostLoginScreen().catch((e) => { + logger.error("Exception showing post-login screen", e); + }); } }; From 3e11360b9f7479dc1a14d438612f0e95604dc438 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 2 Oct 2024 14:57:33 +0100 Subject: [PATCH 22/22] Add new mock --- test/components/structures/MatrixChat-test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 53080cbb88..8876fdc802 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -135,6 +135,7 @@ describe("", () => { getUserDeviceInfo: jest.fn().mockReturnValue(new Map()), getUserVerificationStatus: jest.fn().mockResolvedValue(new UserVerificationStatus(false, false, false)), getVersion: jest.fn().mockReturnValue("1"), + setDeviceIsolationMode: jest.fn(), }), // This needs to not finish immediately because we need to test the screen appears bootstrapCrossSigning: jest.fn().mockImplementation(() => bootstrapDeferred.promise),