Skip to content

Commit

Permalink
Fix: No feedback when waiting for the server on a /delete_devices req…
Browse files Browse the repository at this point in the history
…uest with SSO (#10795)

* add spinner while requests in progress during sso interactive auth

* strict
  • Loading branch information
Kerry committed May 7, 2023
1 parent de16d34 commit b8482b6
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 12 deletions.
17 changes: 11 additions & 6 deletions src/components/structures/InteractiveAuth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -209,19 +209,24 @@ export default class InteractiveAuthComponent<T> extends React.Component<Interac

private onBusyChanged = (busy: boolean): void => {
// if we've started doing stuff, reset the error messages
// The JS SDK eagerly reports itself as "not busy" right after any
// immediate work has completed, but that's not really what we want at
// the UI layer, so we ignore this signal and show a spinner until
// there's a new screen to show the user. This is implemented by setting
// `busy: false` in `authStateUpdated`.
// See also https://github.com/vector-im/element-web/issues/12546
if (busy) {
this.setState({
busy: true,
errorText: undefined,
errorCode: undefined,
});
}
// The JS SDK eagerly reports itself as "not busy" right after any
// immediate work has completed, but that's not really what we want at
// the UI layer, so we ignore this signal and show a spinner until
// there's a new screen to show the user. This is implemented by setting
// `busy: false` in `authStateUpdated`.
// See also https://github.com/vector-im/element-web/issues/12546

// authStateUpdated is not called during sso flows
if (!busy && (this.state.authStage === AuthType.Sso || this.state.authStage === AuthType.SsoUnstable)) {
this.setState({ busy });
}
};

private setFocus(): void {
Expand Down
11 changes: 9 additions & 2 deletions src/components/views/auth/InteractiveAuthEntryComponents.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,7 @@ export class SSOAuthEntry extends React.Component<ISSOAuthEntryProps, ISSOAuthEn

public render(): React.ReactNode {
let continueButton: JSX.Element;

const cancelButton = (
<AccessibleButton
onClick={this.props.onCancel ?? null}
Expand Down Expand Up @@ -902,8 +903,14 @@ export class SSOAuthEntry extends React.Component<ISSOAuthEntryProps, ISSOAuthEn
<Fragment>
{errorSection}
<div className="mx_InteractiveAuthEntryComponents_sso_buttons">
{cancelButton}
{continueButton}
{this.props.busy ? (
<Spinner w={24} h={24} />
) : (
<>
{cancelButton}
{continueButton}
</>
)}
</div>
</Fragment>
);
Expand Down
106 changes: 102 additions & 4 deletions test/components/views/dialogs/InteractiveAuthDialog-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,20 @@ limitations under the License.
*/

import React from "react";
import { render, screen } from "@testing-library/react";
import { fireEvent, render, screen, act } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { mocked } from "jest-mock";

import InteractiveAuthDialog from "../../../../src/components/views/dialogs/InteractiveAuthDialog";
import { flushPromises, getMockClientWithEventEmitter, unmockClientPeg } from "../../../test-utils";
import { clearAllModals, flushPromises, getMockClientWithEventEmitter, unmockClientPeg } from "../../../test-utils";

describe("InteractiveAuthDialog", function () {
const homeserverUrl = "https://matrix.org";
const authUrl = "https://auth.com";
const mockClient = getMockClientWithEventEmitter({
generateClientSecret: jest.fn().mockReturnValue("t35tcl1Ent5ECr3T"),
getFallbackAuthUrl: jest.fn().mockReturnValue(authUrl),
getHomeserverUrl: jest.fn().mockReturnValue(homeserverUrl),
});

const defaultProps = {
Expand All @@ -37,13 +42,15 @@ describe("InteractiveAuthDialog", function () {
const getPasswordField = () => screen.getByLabelText("Password");
const getSubmitButton = () => screen.getByRole("button", { name: "Continue" });

beforeEach(function () {
beforeEach(async function () {
jest.clearAllMocks();
mockClient.credentials = { userId: null };
await clearAllModals();
});

afterAll(() => {
afterAll(async () => {
unmockClientPeg();
await clearAllModals();
});

it("Should successfully complete a password flow", async () => {
Expand Down Expand Up @@ -94,4 +101,95 @@ describe("InteractiveAuthDialog", function () {
expect(onFinished).toHaveBeenCalledTimes(1);
expect(onFinished).toHaveBeenCalledWith(true, { a: 1 });
});

describe("SSO flow", () => {
it("should close on cancel", () => {
const onFinished = jest.fn();
const makeRequest = jest.fn().mockResolvedValue({ a: 1 });

mockClient.credentials = { userId: "@user:id" };
const authData = {
session: "sess",
flows: [{ stages: ["m.login.sso"] }],
};

renderComponent({ makeRequest, onFinished, authData });

expect(screen.getByText("To continue, use Single Sign On to prove your identity.")).toBeInTheDocument();

fireEvent.click(screen.getByText("Cancel"));

expect(onFinished).toHaveBeenCalledWith(false, null);
});

it("should complete an sso flow", async () => {
jest.spyOn(global.window, "addEventListener");
// @ts-ignore
jest.spyOn(global.window, "open").mockImplementation(() => {});
const onFinished = jest.fn();
const successfulResult = { test: 1 };
const makeRequest = jest
.fn()
.mockRejectedValueOnce({ httpStatus: 401, data: { flows: [{ stages: ["m.login.sso"] }] } })
.mockResolvedValue(successfulResult);

mockClient.credentials = { userId: "@user:id" };
const authData = {
session: "sess",
flows: [{ stages: ["m.login.sso"] }],
};

renderComponent({ makeRequest, onFinished, authData });

await flushPromises();

expect(screen.getByText("To continue, use Single Sign On to prove your identity.")).toBeInTheDocument();
fireEvent.click(screen.getByText("Single Sign On"));

// no we're on the sso auth screen
expect(screen.getByText("Click the button below to confirm your identity.")).toBeInTheDocument();

// launch sso
fireEvent.click(screen.getByText("Confirm"));
expect(global.window.open).toHaveBeenCalledWith(authUrl, "_blank");

const onWindowReceiveMessageCall = mocked(window.addEventListener).mock.calls.find(
(args) => args[0] === "message",
);
expect(onWindowReceiveMessageCall).toBeTruthy();
// get the handle from SSO auth component
// so we can pretend sso auth was completed
const onWindowReceiveMessage = onWindowReceiveMessageCall![1];

// complete sso successfully
act(() => {
// @ts-ignore
onWindowReceiveMessage({ data: "authDone", origin: homeserverUrl });
});

// expect(makeRequest).toHaveBeenCalledWith({ session: authData.session })

// spinner displayed
expect(screen.getByRole("progressbar")).toBeInTheDocument();
// cancel/confirm buttons hidden while request in progress
expect(screen.queryByText("Confirm")).not.toBeInTheDocument();

await flushPromises();
await flushPromises();

// nothing in progress
expect(screen.queryByRole("progressbar")).not.toBeInTheDocument();

// auth completed, now make the request again with auth
fireEvent.click(screen.getByText("Confirm"));
// loading while making request
expect(screen.getByRole("progressbar")).toBeInTheDocument();

expect(makeRequest).toHaveBeenCalledTimes(2);

await flushPromises();

expect(onFinished).toHaveBeenCalledWith(true, successfulResult);
});
});
});

0 comments on commit b8482b6

Please sign in to comment.