From 92137901585101a8f8ef496fa6b839cb3b0c80b6 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 28 Apr 2026 09:26:56 +0100 Subject: [PATCH] Re-generate QR code if the channel expires before scan (#33303) * Re-generate QR code if the channel expires before scan * Tweak styling * Remove unused state variable * Update tests * Add tests --- apps/web/res/css/views/auth/_LoginWithQR.pcss | 48 +++++------ .../src/components/views/auth/LoginWithQR.tsx | 25 ++++-- .../components/views/auth/LoginWithQRFlow.tsx | 62 ++++++------- .../settings/devices/LoginWithQR-test.tsx | 54 +++++++++++- .../settings/devices/LoginWithQRFlow-test.tsx | 4 +- .../LoginWithQRFlow-test.tsx.snap | 86 +++++++++++-------- 6 files changed, 179 insertions(+), 100 deletions(-) diff --git a/apps/web/res/css/views/auth/_LoginWithQR.pcss b/apps/web/res/css/views/auth/_LoginWithQR.pcss index e4e41d496e..82703e08e2 100644 --- a/apps/web/res/css/views/auth/_LoginWithQR.pcss +++ b/apps/web/res/css/views/auth/_LoginWithQR.pcss @@ -34,45 +34,45 @@ Please see LICENSE files in the repository root for full details. font-size: $font-15px; } -.mx_UserSettingsDialog .mx_LoginWithQR { +.mx_LoginWithQR { + min-height: 350px; + display: flex; + flex-direction: column; font: var(--cpd-font-body-md-regular); h1 { font-size: $font-24px; margin-bottom: 0; + + svg { + &.normal { + color: $secondary-content; + } + &.error { + color: $alert; + } + &.success { + color: $accent; + } + height: 1.3em; + margin-right: $spacing-8; + vertical-align: middle; + } } h2 { margin-top: $spacing-24; } - .mx_QRCode { - margin: $spacing-28 0; - } - .mx_LoginWithQR_qrWrapper { display: flex; - } -} + padding: $spacing-28 0; -.mx_LoginWithQR { - min-height: 350px; - display: flex; - flex-direction: column; - - h1 > svg { - &.normal { - color: $secondary-content; + .mx_Spinner { + /* Match the size of the QR code to prevent jumps */ + height: 200px; + width: 200px; } - &.error { - color: $alert; - } - &.success { - color: $accent; - } - height: 1.3em; - margin-right: $spacing-8; - vertical-align: middle; } .mx_LoginWithQR_confirmationDigits { diff --git a/apps/web/src/components/views/auth/LoginWithQR.tsx b/apps/web/src/components/views/auth/LoginWithQR.tsx index 1519a1fb45..4e1906b25e 100644 --- a/apps/web/src/components/views/auth/LoginWithQR.tsx +++ b/apps/web/src/components/views/auth/LoginWithQR.tsx @@ -19,6 +19,7 @@ import { } from "matrix-js-sdk/src/rendezvous"; import { logger } from "matrix-js-sdk/src/logger"; import { type MatrixClient } from "matrix-js-sdk/src/matrix"; +import { sleep } from "matrix-js-sdk/src/utils"; import { Click, Mode, Phase } from "./LoginWithQR-types"; import LoginWithQRFlow from "./LoginWithQRFlow"; @@ -32,7 +33,6 @@ interface IProps { interface IState { phase: Phase; rendezvous?: MSC4108SignInWithQR; - mediaPermissionError?: boolean; verificationUri?: string; userCode?: string; checkCode?: string; @@ -78,13 +78,15 @@ export default class LoginWithQR extends React.Component { } } - private async updateMode(mode: Mode): Promise { - this.setState({ phase: Phase.Loading }); + private async updateMode(mode: Mode, showLoading = true): Promise { if (this.state.rendezvous) { const rendezvous = this.state.rendezvous; rendezvous.onFailure = undefined; this.setState({ rendezvous: undefined }); } + if (showLoading) { + this.setState({ phase: Phase.Loading }); + } if (mode === Mode.Show) { await this.generateAndShowCode(); } @@ -187,9 +189,23 @@ export default class LoginWithQR extends React.Component { } }; - private onFailure = (reason: RendezvousFailureReason): void => { + private onFailure = async (reason: RendezvousFailureReason): Promise => { if (this.state.phase === Phase.Error) return; // Already in failed state logger.info(`Rendezvous failed: ${reason}`); + + // Generate a new rendezvous channel & qr code if we hit expiry whilst still showing the QR code + if (reason === ClientRendezvousFailureReason.Expired && this.state.phase === Phase.ShowingQR) { + try { + this.reset(); + // Add a sleep to make the UX looks less flickery and more intentional + await sleep(1000); + await this.updateMode(Mode.Show, false); + return; + } catch (e) { + logger.warn("Failed to re-roll qr code on expiry", e); + } + } + this.setState({ phase: Phase.Error, failureReason: reason }); }; @@ -200,7 +216,6 @@ export default class LoginWithQR extends React.Component { failureReason: undefined, userCode: undefined, checkCode: undefined, - mediaPermissionError: false, }); } diff --git a/apps/web/src/components/views/auth/LoginWithQRFlow.tsx b/apps/web/src/components/views/auth/LoginWithQRFlow.tsx index a432baac72..34a3b22db4 100644 --- a/apps/web/src/components/views/auth/LoginWithQRFlow.tsx +++ b/apps/web/src/components/views/auth/LoginWithQRFlow.tsx @@ -226,39 +226,39 @@ export default class LoginWithQRFlow extends React.Component { ); break; - case Phase.ShowingQR: - if (this.props.code) { - const data = this.props.code; + case Phase.ShowingQR: { + const steps = [ + _t("auth|qr_code_login|open_element_other_device", { + brand: SdkConfig.get().brand, + }), + _t("auth|qr_code_login|select_qr_code", { + scanQRCode: {_t("auth|qr_code_login|scan_qr_code")}, + }), + _t("auth|qr_code_login|point_the_camera"), + _t("auth|qr_code_login|follow_remaining_instructions"), + ]; - main = ( - <> - - {_t("auth|qr_code_login|scan_code_instruction")} - -
- -
-
    -
  1. - {_t("auth|qr_code_login|open_element_other_device", { - brand: SdkConfig.get().brand, - })} -
    2. -
  2. - {_t("auth|qr_code_login|select_qr_code", { - scanQRCode: {_t("auth|qr_code_login|scan_qr_code")}, - })} -
    3. -
  3. {_t("auth|qr_code_login|point_the_camera")}
    4. -
  4. {_t("auth|qr_code_login|follow_remaining_instructions")}
    5. -
- - ); - } else { - main = this.simpleSpinner(); - buttons = this.cancelButton(); - } + main = ( + <> + + {_t("auth|qr_code_login|scan_code_instruction")} + +
+ {this.props.code ? ( + + ) : ( + + )} +
+
    + {steps.map((step, i) => ( +
  1. {step}
    2. + ))} +
+ + ); break; + } case Phase.Loading: main = this.simpleSpinner(); break; diff --git a/apps/web/test/unit-tests/components/views/settings/devices/LoginWithQR-test.tsx b/apps/web/test/unit-tests/components/views/settings/devices/LoginWithQR-test.tsx index d9e9b3e391..fb8d07af82 100644 --- a/apps/web/test/unit-tests/components/views/settings/devices/LoginWithQR-test.tsx +++ b/apps/web/test/unit-tests/components/views/settings/devices/LoginWithQR-test.tsx @@ -17,7 +17,7 @@ import { } from "matrix-js-sdk/src/rendezvous"; import { HTTPError, type MatrixClient } from "matrix-js-sdk/src/matrix"; -import LoginWithQR from "../../../../../../src/components/views/auth/LoginWithQR"; +import LoginWithQR, { LoginWithQRFailureReason } from "../../../../../../src/components/views/auth/LoginWithQR"; import { Click, Mode, Phase } from "../../../../../../src/components/views/auth/LoginWithQR-types"; jest.mock("matrix-js-sdk/src/rendezvous"); @@ -68,6 +68,7 @@ describe("", () => { mockedFlow.mockReset(); jest.resetAllMocks(); client = makeClient(); + jest.useFakeTimers(); }); afterEach(() => { @@ -105,6 +106,29 @@ describe("", () => { expect(rendezvous.cancel).toHaveBeenCalledWith(MSC4108FailureReason.UserCancelled); }); + test("should open a new channel if expires before qr scan", async () => { + const onFinished = jest.fn(); + jest.spyOn(MSC4108SignInWithQR.prototype, "negotiateProtocols").mockReturnValue(unresolvedPromise()); + render(getComponent({ client, onFinished })); + + await waitFor(() => + expect(mockedFlow).toHaveBeenLastCalledWith({ + phase: Phase.ShowingQR, + onClick: expect.any(Function), + }), + ); + + const rendezvous = mocked(MSC4108SignInWithQR).mock.instances[0]; + expect(rendezvous.generateCode).toHaveBeenCalled(); + expect(rendezvous.negotiateProtocols).toHaveBeenCalled(); + + // Expire the channel + const onFailure = mocked(MSC4108SignInWithQR).mock.calls[0][3]; + onFailure!(ClientRendezvousFailureReason.Expired); + await jest.runAllTimersAsync(); + await waitFor(() => expect(mocked(MSC4108SignInWithQR).mock.instances).toHaveLength(2)); + }); + test("failed to connect", async () => { render(getComponent({ client })); jest.spyOn(MSC4108SignInWithQR.prototype, "negotiateProtocols").mockResolvedValue({}); @@ -115,6 +139,34 @@ describe("", () => { await waitFor(() => expect(fn).toHaveBeenLastCalledWith(ClientRendezvousFailureReason.Unknown)); }); + test("should show error if check code doesn't match", async () => { + jest.spyOn(global.window, "open"); + + render(getComponent({ client })); + jest.spyOn(MSC4108SignInWithQR.prototype, "negotiateProtocols").mockResolvedValue({}); + jest.spyOn(MSC4108SignInWithQR.prototype, "deviceAuthorizationGrant").mockResolvedValue({ + verificationUri: "mock-verification-uri", + }); + + await waitFor(() => + expect(mockedFlow).toHaveBeenLastCalledWith({ + phase: Phase.OutOfBandConfirmation, + onClick: expect.any(Function), + }), + ); + + const onClick = mockedFlow.mock.calls[0][0].onClick; + await onClick(Click.Approve, "12"); + + await waitFor(() => + expect(mockedFlow).toHaveBeenLastCalledWith({ + phase: Phase.OutOfBandConfirmation, + failureReason: LoginWithQRFailureReason.CheckCodeMismatch, + onClick: expect.any(Function), + }), + ); + }); + test("reciprocates login", async () => { jest.spyOn(global.window, "open"); diff --git a/apps/web/test/unit-tests/components/views/settings/devices/LoginWithQRFlow-test.tsx b/apps/web/test/unit-tests/components/views/settings/devices/LoginWithQRFlow-test.tsx index 9d9c5763be..5de2e70f1b 100644 --- a/apps/web/test/unit-tests/components/views/settings/devices/LoginWithQRFlow-test.tsx +++ b/apps/web/test/unit-tests/components/views/settings/devices/LoginWithQRFlow-test.tsx @@ -42,10 +42,8 @@ describe("", () => { it("renders spinner whilst QR generating", async () => { const { container } = render(getComponent({ phase: Phase.ShowingQR })); - expect(screen.getAllByTestId("cancel-button")).toHaveLength(1); + expect(screen.getAllByTestId("spinner")).toHaveLength(1); expect(container).toMatchSnapshot(); - fireEvent.click(screen.getByTestId("cancel-button")); - expect(onClick).toHaveBeenCalledWith(Click.Cancel, undefined); }); it("renders QR code", async () => { diff --git a/apps/web/test/unit-tests/components/views/settings/devices/__snapshots__/LoginWithQRFlow-test.tsx.snap b/apps/web/test/unit-tests/components/views/settings/devices/__snapshots__/LoginWithQRFlow-test.tsx.snap index f584af643b..63eb09fca7 100644 --- a/apps/web/test/unit-tests/components/views/settings/devices/__snapshots__/LoginWithQRFlow-test.tsx.snap +++ b/apps/web/test/unit-tests/components/views/settings/devices/__snapshots__/LoginWithQRFlow-test.tsx.snap @@ -813,12 +813,12 @@ exports[` renders QR code 1`] = ` class="mx_LoginWithQR_qrWrapper" >
QR Code
@@ -1203,47 +1203,61 @@ exports[` renders spinner whilst QR generating 1`] = `
-
`;