Gracefully handle rate limit response when attempting to Sign in with…

… QR (#11809)

* Gracefully hand rate limit response when attempting to Sign in with QR

* Don't cancel after rate limit

* Update src/components/views/auth/LoginWithQR.tsx

Co-authored-by: Michael Telatynski <[email protected]>

* Add missing import

* Fix mock of error

---------

Co-authored-by: Michael Telatynski <[email protected]>

src/components/views/auth/LoginWithQR.tsx

@@ -19,7 +19,7 @@ import { MSC3906Rendezvous, MSC3906RendezvousPayload, RendezvousFailureReason }
 import { MSC3886SimpleHttpRendezvousTransport } from "matrix-js-sdk/src/rendezvous/transports";
 import { MSC3903ECDHPayload, MSC3903ECDHv2RendezvousChannel } from "matrix-js-sdk/src/rendezvous/channels";
 import { logger } from "matrix-js-sdk/src/logger";
-import { MatrixClient } from "matrix-js-sdk/src/matrix";
+import { HTTPError, MatrixClient } from "matrix-js-sdk/src/matrix";
 
 import { _t } from "../../../languageHandler";
 import { wrapRequestWithDialog } from "../../../utils/UserInteractiveAuth";
@@ -63,10 +63,16 @@ interface IState {
     phase: Phase;
     rendezvous?: MSC3906Rendezvous;
     confirmationDigits?: string;
-    failureReason?: RendezvousFailureReason;
+    failureReason?: FailureReason;
     mediaPermissionError?: boolean;
 }
 
+export enum LoginWithQRFailureReason {
+    RateLimited = "rate_limited",
+}
+
+export type FailureReason = RendezvousFailureReason | LoginWithQRFailureReason;
+
 /**
  * A component that allows sign in and E2EE set up with a QR code.
  *
@@ -136,16 +142,27 @@ export default class LoginWithQR extends React.Component<IProps, IState> {
                 // user denied
                 return;
             }
-            if (!this.props.client.crypto) {
+            if (!this.props.client.getCrypto()) {
                 // no E2EE to set up
                 this.props.onFinished(true);
                 return;
             }
             this.setState({ phase: Phase.Verifying });
             await this.state.rendezvous.verifyNewDeviceOnExistingDevice();
+            // clean up our state:
+            try {
+                await this.state.rendezvous.close();
+            } finally {
+                this.setState({ rendezvous: undefined });
+            }
             this.props.onFinished(true);
         } catch (e) {
             logger.error("Error whilst approving sign in", e);
+            if (e instanceof HTTPError && e.httpStatus === 429) {
+                // 429: rate limit
+                this.setState({ phase: Phase.Error, failureReason: LoginWithQRFailureReason.RateLimited });
+                return;
+            }
             this.setState({ phase: Phase.Error, failureReason: RendezvousFailureReason.Unknown });
         }
     };

src/components/views/auth/LoginWithQRFlow.tsx

@@ -25,13 +25,13 @@ import { Icon as BackButtonIcon } from "../../../../res/img/element-icons/back.s
 import { Icon as DevicesIcon } from "../../../../res/img/element-icons/devices.svg";
 import { Icon as WarningBadge } from "../../../../res/img/element-icons/warning-badge.svg";
 import { Icon as InfoIcon } from "../../../../res/img/element-icons/i.svg";
-import { Click, Phase } from "./LoginWithQR";
+import { Click, FailureReason, LoginWithQRFailureReason, Phase } from "./LoginWithQR";
 
 interface IProps {
     phase: Phase;
     code?: string;
     onClick(type: Click): Promise<void>;
-    failureReason?: RendezvousFailureReason;
+    failureReason?: FailureReason;
     confirmationDigits?: string;
 }
 
@@ -102,6 +102,9 @@ export default class LoginWithQRFlow extends React.Component<IProps> {
                     case RendezvousFailureReason.UserCancelled:
                         cancellationMessage = _t("auth|qr_code_login|error_request_cancelled");
                         break;
+                    case LoginWithQRFailureReason.RateLimited:
+                        cancellationMessage = _t("auth|qr_code_login|error_rate_limited");
+                        break;
                     case RendezvousFailureReason.Unknown:
                         cancellationMessage = _t("auth|qr_code_login|error_unexpected");
                         break;

src/i18n/strings/en_EN.json

@@ -256,6 +256,7 @@
             "error_homeserver_lacks_support": "The homeserver doesn't support signing in another device.",
             "error_invalid_scanned_code": "The scanned code is invalid.",
             "error_linking_incomplete": "The linking wasn't completed in the required time.",
+            "error_rate_limited": "Too many attempts in a short time. Wait some time before trying again.",
             "error_request_cancelled": "The request was cancelled.",
             "error_request_declined": "The request was declined on the other device.",
             "error_unexpected": "An unexpected error occurred.",

test/components/views/settings/devices/LoginWithQR-test.tsx

@@ -15,10 +15,10 @@ limitations under the License.
 */
 
 import { cleanup, render, waitFor } from "@testing-library/react";
-import { mocked } from "jest-mock";
+import { MockedObject, mocked } from "jest-mock";
 import React from "react";
 import { MSC3906Rendezvous, RendezvousFailureReason } from "matrix-js-sdk/src/rendezvous";
-import { LoginTokenPostResponse } from "matrix-js-sdk/src/matrix";
+import { HTTPError, LoginTokenPostResponse } from "matrix-js-sdk/src/matrix";
 
 import LoginWithQR, { Click, Mode, Phase } from "../../../../../src/components/views/auth/LoginWithQR";
 import type { MatrixClient } from "matrix-js-sdk/src/matrix";
@@ -52,6 +52,8 @@ function makeClient() {
             on: jest.fn(),
         },
         getClientWellKnown: jest.fn().mockReturnValue({}),
+        getCrypto: jest.fn().mockReturnValue({}),
+        crypto: {},
     } as unknown as MatrixClient);
 }
 
@@ -60,7 +62,7 @@ function unresolvedPromise<T>(): Promise<T> {
 }
 
 describe("<LoginWithQR />", () => {
-    let client = makeClient();
+    let client!: MockedObject<MatrixClient>;
     const defaultProps = {
         mode: Mode.Show,
         onFinished: jest.fn(),
@@ -78,6 +80,7 @@ describe("<LoginWithQR />", () => {
     beforeEach(() => {
         mockedFlow.mockReset();
         jest.resetAllMocks();
+        client = makeClient();
         jest.spyOn(MSC3906Rendezvous.prototype, "generateCode").mockResolvedValue();
         // @ts-ignore
         // workaround for https://github.com/facebook/jest/issues/9675
@@ -233,10 +236,9 @@ describe("<LoginWithQR />", () => {
     });
 
     test("approve - no crypto", async () => {
-        // @ts-ignore
-        client.crypto = undefined;
+        (client as any).crypto = undefined;
+        (client as any).getCrypto = () => undefined;
         const onFinished = jest.fn();
-        // jest.spyOn(MSC3906Rendezvous.prototype, 'approveLoginOnExistingDevice').mockReturnValue(unresolvedPromise());
         render(getComponent({ client, onFinished }));
         const rendezvous = mocked(MSC3906Rendezvous).mock.instances[0];
 
@@ -274,8 +276,6 @@ describe("<LoginWithQR />", () => {
 
     test("approve + verifying", async () => {
         const onFinished = jest.fn();
-        // @ts-ignore
-        client.crypto = {};
         jest.spyOn(MSC3906Rendezvous.prototype, "verifyNewDeviceOnExistingDevice").mockImplementation(() =>
             unresolvedPromise(),
         );
@@ -316,8 +316,6 @@ describe("<LoginWithQR />", () => {
 
     test("approve + verify", async () => {
         const onFinished = jest.fn();
-        // @ts-ignore
-        client.crypto = {};
         render(getComponent({ client, onFinished }));
         const rendezvous = mocked(MSC3906Rendezvous).mock.instances[0];
 
@@ -341,6 +339,43 @@ describe("<LoginWithQR />", () => {
         await onClick(Click.Approve);
         expect(rendezvous.approveLoginOnExistingDevice).toHaveBeenCalledWith("token");
         expect(rendezvous.verifyNewDeviceOnExistingDevice).toHaveBeenCalled();
+        expect(rendezvous.close).toHaveBeenCalled();
         expect(onFinished).toHaveBeenCalledWith(true);
     });
+
+    test("approve - rate limited", async () => {
+        mocked(client.requestLoginToken).mockRejectedValue(new HTTPError("rate limit reached", 429));
+        const onFinished = jest.fn();
+        render(getComponent({ client, onFinished }));
+        const rendezvous = mocked(MSC3906Rendezvous).mock.instances[0];
+
+        await waitFor(() =>
+            expect(mockedFlow).toHaveBeenLastCalledWith(
+                expect.objectContaining({
+                    phase: Phase.Connected,
+                }),
+            ),
+        );
+        expect(mockedFlow).toHaveBeenLastCalledWith({
+            phase: Phase.Connected,
+            confirmationDigits: mockConfirmationDigits,
+            onClick: expect.any(Function),
+        });
+        expect(rendezvous.generateCode).toHaveBeenCalled();
+        expect(rendezvous.startAfterShowingCode).toHaveBeenCalled();
+
+        // approve
+        const onClick = mockedFlow.mock.calls[0][0].onClick;
+        await onClick(Click.Approve);
+
+        // the 429 error should be handled and mapped
+        await waitFor(() =>
+            expect(mockedFlow).toHaveBeenLastCalledWith(
+                expect.objectContaining({
+                    phase: Phase.Error,
+                    failureReason: "rate_limited",
+                }),
+            ),
+        );
+    });
 });

test/components/views/settings/devices/LoginWithQRFlow-test.tsx

@@ -19,7 +19,12 @@ import React from "react";
 import { RendezvousFailureReason } from "matrix-js-sdk/src/rendezvous";
 
 import LoginWithQRFlow from "../../../../../src/components/views/auth/LoginWithQRFlow";
-import { Click, Phase } from "../../../../../src/components/views/auth/LoginWithQR";
+import {
+    Click,
+    Phase,
+    LoginWithQRFailureReason,
+    FailureReason,
+} from "../../../../../src/components/views/auth/LoginWithQR";
 
 describe("<LoginWithQRFlow />", () => {
     const onClick = jest.fn();
@@ -31,7 +36,7 @@ describe("<LoginWithQRFlow />", () => {
     const getComponent = (props: {
         phase: Phase;
         onClick?: () => Promise<void>;
-        failureReason?: RendezvousFailureReason;
+        failureReason?: FailureReason;
         code?: string;
         confirmationDigits?: string;
     }) => <LoginWithQRFlow {...defaultProps} {...props} />;
@@ -97,7 +102,10 @@ describe("<LoginWithQRFlow />", () => {
     });
 
     describe("errors", () => {
-        for (const failureReason of Object.values(RendezvousFailureReason)) {
+        for (const failureReason of [
+            ...Object.values(RendezvousFailureReason),
+            ...Object.values(LoginWithQRFailureReason),
+        ]) {
             it(`renders ${failureReason}`, async () => {
                 const { container } = render(
                     getComponent({

test/components/views/settings/devices/__snapshots__/LoginWithQRFlow-test.tsx.snap

@@ -294,6 +294,55 @@ exports[`<LoginWithQRFlow /> errors renders other_device_not_signed_in 1`] = `
 </div>
 `;
 
+exports[`<LoginWithQRFlow /> errors renders rate_limited 1`] = `
+<div>
+  <div
+    class="mx_LoginWithQR"
+    data-testid="login-with-qr"
+  >
+    <div
+      class="mx_LoginWithQR_centreTitle"
+    >
+      <h1>
+        <div
+          class="error"
+        />
+        Connection failed
+      </h1>
+    </div>
+    <div
+      class="mx_LoginWithQR_main"
+    >
+      <p
+        data-testid="cancellation-message"
+      >
+        Too many attempts in a short time. Wait some time before trying again.
+      </p>
+    </div>
+    <div
+      class="mx_LoginWithQR_buttons"
+    >
+      <div
+        class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary"
+        data-testid="try-again-button"
+        role="button"
+        tabindex="0"
+      >
+        Try again
+      </div>
+      <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>
+`;
+
 exports[`<LoginWithQRFlow /> errors renders unknown 1`] = `
 <div>
   <div