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

@@ -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 {

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<IProps, IState> {
         }
     }
 
-    private async updateMode(mode: Mode): Promise<void> {
-        this.setState({ phase: Phase.Loading });
+    private async updateMode(mode: Mode, showLoading = true): Promise<void> {
         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<IProps, IState> {
         }
     };
 
-    private onFailure = (reason: RendezvousFailureReason): void => {
+    private onFailure = async (reason: RendezvousFailureReason): Promise<void> => {
         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<IProps, IState> {
             failureReason: undefined,
             userCode: undefined,
             checkCode: undefined,
-            mediaPermissionError: false,
         });
     }
 

apps/web/src/components/views/auth/LoginWithQRFlow.tsx

@@ -226,39 +226,39 @@ export default class LoginWithQRFlow extends React.Component<Props> {
                     </>
                 );
                 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: <strong>{_t("auth|qr_code_login|scan_qr_code")}</strong>,
+                    }),
+                    _t("auth|qr_code_login|point_the_camera"),
+                    _t("auth|qr_code_login|follow_remaining_instructions"),
+                ];
 
-                    main = (
-                        <>
-                            <Heading as="h1" size="sm" weight="semibold">
-                                {_t("auth|qr_code_login|scan_code_instruction")}
-                            </Heading>
-                            <div className="mx_LoginWithQR_qrWrapper">
-                                <QRCode data={[{ data, mode: "byte" }]} className="mx_QRCode" />
-                            </div>
-                            <ol>
-                                <li>
-                                    {_t("auth|qr_code_login|open_element_other_device", {
-                                        brand: SdkConfig.get().brand,
-                                    })}
-                                </li>
-                                <li>
-                                    {_t("auth|qr_code_login|select_qr_code", {
-                                        scanQRCode: <strong>{_t("auth|qr_code_login|scan_qr_code")}</strong>,
-                                    })}
-                                </li>
-                                <li>{_t("auth|qr_code_login|point_the_camera")}</li>
-                                <li>{_t("auth|qr_code_login|follow_remaining_instructions")}</li>
-                            </ol>
-                        </>
-                    );
-                } else {
-                    main = this.simpleSpinner();
-                    buttons = this.cancelButton();
-                }
+                main = (
+                    <>
+                        <Heading as="h1" size="sm" weight="semibold">
+                            {_t("auth|qr_code_login|scan_code_instruction")}
+                        </Heading>
+                        <div className="mx_LoginWithQR_qrWrapper">
+                            {this.props.code ? (
+                                <QRCode data={[{ data: this.props.code, mode: "byte" }]} width={200} />
+                            ) : (
+                                <Spinner />
+                            )}
+                        </div>
+                        <ol>
+                            {steps.map((step, i) => (
+                                <li key={i}>{step}</li>
+                            ))}
+                        </ol>
+                    </>
+                );
                 break;
+            }
             case Phase.Loading:
                 main = this.simpleSpinner();
                 break;

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("<LoginWithQR />", () => {
         mockedFlow.mockReset();
         jest.resetAllMocks();
         client = makeClient();
+        jest.useFakeTimers();
     });
 
     afterEach(() => {
@@ -105,6 +106,29 @@ describe("<LoginWithQR />", () => {
             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("<LoginWithQR />", () => {
             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");
 

apps/web/test/unit-tests/components/views/settings/devices/LoginWithQRFlow-test.tsx

@@ -42,10 +42,8 @@ describe("<LoginWithQRFlow />", () => {
 
     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 () => {

apps/web/test/unit-tests/components/views/settings/devices/__snapshots__/LoginWithQRFlow-test.tsx.snap

@@ -813,12 +813,12 @@ exports[`<LoginWithQRFlow /> renders QR code 1`] = `
         class="mx_LoginWithQR_qrWrapper"
       >
         <div
-          class="mx_QRCode mx_QRCode"
+          class="mx_QRCode"
         >
           <img
             alt="QR Code"
             class="mx_VerificationQRCode"
-            src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAHQAAAB0CAYAAABUmhYnAAAAAklEQVR4AewaftIAAAKxSURBVO3BQW7kQAwEwSxC//9yro88NSBI4/UQjIg/WGMUa5RijVKsUYo1SrFGKdYoxRqlWKMUa5RijVKsUYo1SrFGKdYoxRrl4qEk/CaVkyR0Kl0STlS6JPwmlSeKNUqxRinWKBcvU3lTEp5IwonKHSpvSsKbijVKsUYp1igXH5aEO1TuSMKJSpeELgmdyh1JuEPlk4o1SrFGKdYoF19OpUvCHSqTFGuUYo1SrFEuhlHpktCpdEnoVL5ZsUYp1ijFGuXiw1T+J5VPUvlLijVKsUYp1igXL0vCb0pCp9IloVN5Igl/WbFGKdYoxRrl4iGVvyQJnUqXhDtUvkmxRinWKMUa5eKhJNyh0iXhk5JwotIl4UTljiR0Kp9UrFGKNUqxRok/eFESOpUuCZ3KN0nCiUqXhDtUnijWKMUapVijxB88kIQ3qXRJeELlJAmdSpeEJ1S6JJyoPFGsUYo1SrFGuXiZSpeETqVLwonKSRKeUDlROUlCp3Ki8knFGqVYoxRrlPiDB5LwhMpJEjqVO5LQqXRJ6FROktCp3JGETuVNxRqlWKMUa5T4gy+WhE7lJAmdyh1JuEOlS8KJyhPFGqVYoxRrlIuHkvCbVO5Iwh1J+GbFGqVYoxRrlIuXqbwpCXckoVPpktAl4UTlJAmdyonKJxVrlGKNUqxRLj4sCXeovCkJJyonSehU/rJijVKsUYo1ysWXUzlJQqfyRBJOknCHypuKNUqxRinWKBdfLgmdSqdykoROpVPpknCHykkSOpUnijVKsUYp1igXH6byPyWhU+lUTpLQqZwk4SQJncqbijVKsUYp1igXL0vC/5SEO5LQqZwkoVPpVO5IQqfyRLFGKdYoxRol/mCNUaxRijVKsUYp1ijFGqVYoxRrlGKNUqxRijVKsUYp1ijFGqVYoxRrlH+MbAvtLaAtKAAAAABJRU5ErkJggg=="
+            src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAMgAAADICAYAAACtWK6eAAAAAklEQVR4AewaftIAAAPWSURBVO3BQY5jBxYDweSD7n9lTu+8aQJu+Y+gsjMi/QVJv3VImg5J0yFpOiRNh6TpkDQdkqZD0nRImg5J0yFpOiRNh6TpkDQdkqZD0nRImg5J0yFpOiRNh6TpkDQdkqZD0nRImg5J0yFpevFhSfjJ2vKOJCxtWZLwjrYsSfjJ2vIph6TpkDQdkqZD0nRImg5J04sv0pZvkIRPSsI72vK0tnyDJHyDQ9J0SJoOSdMhaTokTYek6cUPkYSnteVJSXhHW5YkLElY2vKkJDytLd/ukDQdkqZD0nRImg5J0yFpeqHHtGVJwtPaov+vQ9J0SJoOSdMhaTokTYek6YU+oi1LEpa2LElY2qJ/7pA0HZKmQ9J0SJoOSdMhaXrxQ7Tl36ot364t/0WHpOmQNB2SpkPSdEiaXnyRJPxkSVjasiRhacsnJUF/OSRNh6TpkDQdkqZD0nRIml58WFv+i5KwtGVJwtPaor/nkDQdkqZD0nRImg5J0yFpevFhSXhaW5YkfLskvKMtSxLe0ZYnJWFpy7c7JE2HpOmQNB2SpkPSdEia0l/4EklY2rIkYWmL/r4k/Km2LEl4Wls+5ZA0HZKmQ9J0SJoOSdMhaUp/4YOS8C3a8jtJ+KS2vCMJS1uWJHxKW5YkvKMtn3JImg5J0yFpOiRNh6TpkDS9+CJtWZKwtGVJwp9qyzuS8ElteUdb/lQSlra8oy3f7pA0HZKmQ9J0SJoOSdMhaUp/4YOS8Elt+VNJWNrytCQsbVmSsLTlHUn4nbY8LQlLW77BIWk6JE2HpOmQNB2SpkPSlP6CHpGEpS3vSMLSlqcl4UltWZLwjrZ8yiFpOiRNh6TpkDQdkqZD0vTiw5Lwk7XlaUl4WhL0zx2SpkPSdEiaDknTIWk6JE0vvkhbvkESnpaEpS1LEpYkvKMtfyoJS1ve0ZZvd0iaDknTIWk6JE2HpOnFD5GEp7XlGyThHW15RxKWtugvh6TpkDQdkqZD0nRImg5J0ws9pi3vSMLSlk9Kwp9KwtPa8g0OSdMhaTokTYek6ZA0HZKmF3pMEpa2LG15RxKWtixtWZLwpLa8IwlLWz7lkDQdkqZD0nRImg5J0yFpevFDtOXfKglLW5a2vCMJS1v+VBLekYSlLd/gkDQdkqZD0nRImg5J0yFpevFFkvBvlYSnJWFpyzuS8DttWdrytCQsbfmUQ9J0SJoOSdMhaTokTYekKf0FSb91SJoOSdMhaTokTYek6ZA0HZKmQ9J0SJoOSdMhaTokTYek6ZA0HZKmQ9J0SJoOSdMhaTokTYek6ZA0HZKmQ9J0SJoOSdP/AHKVDJXnVp/qAAAAAElFTkSuQmCC"
           />
         </div>
       </div>
@@ -1203,47 +1203,61 @@ exports[`<LoginWithQRFlow /> renders spinner whilst QR generating 1`] = `
     <div
       class="mx_LoginWithQR_main"
     >
-      <div
-        class="mx_LoginWithQR_spinner"
+      <h1
+        class="_typography_6v6n8_153 _font-heading-sm-semibold_6v6n8_93"
       >
-        <div>
-          <div
-            class="mx_Spinner"
+        Scan the QR code with another device
+      </h1>
+      <div
+        class="mx_LoginWithQR_qrWrapper"
+      >
+        <div
+          class="mx_Spinner"
+        >
+          <svg
+            aria-label="Loading…"
+            class="_icon_1855a_18"
+            data-testid="spinner"
+            fill="currentColor"
+            height="1em"
+            role="progressbar"
+            style="width: 32px; height: 32px;"
+            viewBox="0 0 24 24"
+            width="1em"
+            xmlns="http://www.w3.org/2000/svg"
           >
-            <svg
-              aria-label="Loading…"
-              class="_icon_1855a_18"
-              data-testid="spinner"
-              fill="currentColor"
-              height="1em"
-              role="progressbar"
-              style="width: 32px; height: 32px;"
-              viewBox="0 0 24 24"
-              width="1em"
-              xmlns="http://www.w3.org/2000/svg"
-            >
-              <path
-                clip-rule="evenodd"
-                d="M12 4.031a8 8 0 1 0 8 8 1 1 0 0 1 2 0c0 5.523-4.477 10-10 10s-10-4.477-10-10 4.477-10 10-10a1 1 0 1 1 0 2"
-                fill-rule="evenodd"
-              />
-            </svg>
-          </div>
+            <path
+              clip-rule="evenodd"
+              d="M12 4.031a8 8 0 1 0 8 8 1 1 0 0 1 2 0c0 5.523-4.477 10-10 10s-10-4.477-10-10 4.477-10 10-10a1 1 0 1 1 0 2"
+              fill-rule="evenodd"
+            />
+          </svg>
         </div>
       </div>
+      <ol>
+        <li>
+          Open Element on your other device
+        </li>
+        <li>
+          <span>
+            Select "
+            <strong>
+              Sign in with QR code
+            </strong>
+            "
+          </span>
+        </li>
+        <li>
+          Scan the QR code shown here
+        </li>
+        <li>
+          Follow the remaining instructions
+        </li>
+      </ol>
     </div>
     <div
       class="mx_LoginWithQR_buttons"
-    >
-      <div
-        class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary_outline"
-        data-testid="cancel-button"
-        role="button"
-        tabindex="0"
-      >
-        Cancel
-      </div>
-    </div>
+    />
   </div>
 </div>
 `;