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 <7t3chguy@gmail.com>

* Add missing import

* Fix mock of error

---------

Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
pull/28788/head^2
Hugh Nimmo-Smith 2024-01-11 16:11:47 +00:00 committed by GitHub
parent 6187c8c884
commit 54b71140b8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 131 additions and 18 deletions

View File

@ -19,7 +19,7 @@ import { MSC3906Rendezvous, MSC3906RendezvousPayload, RendezvousFailureReason }
import { MSC3886SimpleHttpRendezvousTransport } from "matrix-js-sdk/src/rendezvous/transports"; import { MSC3886SimpleHttpRendezvousTransport } from "matrix-js-sdk/src/rendezvous/transports";
import { MSC3903ECDHPayload, MSC3903ECDHv2RendezvousChannel } from "matrix-js-sdk/src/rendezvous/channels"; import { MSC3903ECDHPayload, MSC3903ECDHv2RendezvousChannel } from "matrix-js-sdk/src/rendezvous/channels";
import { logger } from "matrix-js-sdk/src/logger"; 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 { _t } from "../../../languageHandler";
import { wrapRequestWithDialog } from "../../../utils/UserInteractiveAuth"; import { wrapRequestWithDialog } from "../../../utils/UserInteractiveAuth";
@ -63,10 +63,16 @@ interface IState {
phase: Phase; phase: Phase;
rendezvous?: MSC3906Rendezvous; rendezvous?: MSC3906Rendezvous;
confirmationDigits?: string; confirmationDigits?: string;
failureReason?: RendezvousFailureReason; failureReason?: FailureReason;
mediaPermissionError?: boolean; 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. * 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 // user denied
return; return;
} }
if (!this.props.client.crypto) { if (!this.props.client.getCrypto()) {
// no E2EE to set up // no E2EE to set up
this.props.onFinished(true); this.props.onFinished(true);
return; return;
} }
this.setState({ phase: Phase.Verifying }); this.setState({ phase: Phase.Verifying });
await this.state.rendezvous.verifyNewDeviceOnExistingDevice(); await this.state.rendezvous.verifyNewDeviceOnExistingDevice();
// clean up our state:
try {
await this.state.rendezvous.close();
} finally {
this.setState({ rendezvous: undefined });
}
this.props.onFinished(true); this.props.onFinished(true);
} catch (e) { } catch (e) {
logger.error("Error whilst approving sign in", 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 }); this.setState({ phase: Phase.Error, failureReason: RendezvousFailureReason.Unknown });
} }
}; };

View File

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

View File

@ -256,6 +256,7 @@
"error_homeserver_lacks_support": "The homeserver doesn't support signing in another device.", "error_homeserver_lacks_support": "The homeserver doesn't support signing in another device.",
"error_invalid_scanned_code": "The scanned code is invalid.", "error_invalid_scanned_code": "The scanned code is invalid.",
"error_linking_incomplete": "The linking wasn't completed in the required time.", "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_cancelled": "The request was cancelled.",
"error_request_declined": "The request was declined on the other device.", "error_request_declined": "The request was declined on the other device.",
"error_unexpected": "An unexpected error occurred.", "error_unexpected": "An unexpected error occurred.",

View File

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

View File

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

View File

@ -294,6 +294,55 @@ exports[`<LoginWithQRFlow /> errors renders other_device_not_signed_in 1`] = `
</div> </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`] = ` exports[`<LoginWithQRFlow /> errors renders unknown 1`] = `
<div> <div>
<div <div