diff --git a/playwright/e2e/oidc/oidc-native.spec.ts b/playwright/e2e/oidc/oidc-native.spec.ts index 8c9128c39b..2592311554 100644 --- a/playwright/e2e/oidc/oidc-native.spec.ts +++ b/playwright/e2e/oidc/oidc-native.spec.ts @@ -73,4 +73,33 @@ test.describe("OIDC Native", { tag: ["@no-firefox", "@no-webkit"] }, () => { await revokeAccessTokenPromise; await revokeRefreshTokenPromise; }); + + test( + "it should log out the user & wipe data when logging out via MAS", + { tag: "@screenshot" }, + async ({ mas, page, mailpitClient }, testInfo) => { + // We use this over the `user` fixture to ensure we get an OIDC session rather than a compatibility one + await page.goto("/#/login"); + await page.getByRole("button", { name: "Continue" }).click(); + + const userId = `alice_${testInfo.testId}`; + await registerAccountMas(page, mailpitClient, userId, "alice@email.com", "Pa$sW0rD!"); + + await expect(page.getByText("Welcome")).toBeVisible(); + await page.goto("about:blank"); + + // @ts-expect-error + const result = await mas.manage("kill-sessions", userId); + expect(result.output).toContain("Ended 1 active OAuth 2.0 session"); + + await page.goto("http://localhost:8080"); + await expect( + page.getByText("For security, this session has been signed out. Please sign in again."), + ).toBeVisible(); + await expect(page).toMatchScreenshot("token-expired.png", { includeDialogBackground: true }); + + const localStorageKeys = await page.evaluate(() => Object.keys(localStorage)); + expect(localStorageKeys).toHaveLength(0); + }, + ); }); diff --git a/playwright/snapshots/oidc/oidc-native.spec.ts/token-expired-linux.png b/playwright/snapshots/oidc/oidc-native.spec.ts/token-expired-linux.png new file mode 100644 index 0000000000..17479bb870 Binary files /dev/null and b/playwright/snapshots/oidc/oidc-native.spec.ts/token-expired-linux.png differ diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 279589ff5f..8ac235f78a 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -149,6 +149,7 @@ interface ILoadSessionOpts { ignoreGuest?: boolean; defaultDeviceDisplayName?: string; fragmentQueryParams?: QueryDict; + abortSignal?: AbortSignal; } /** @@ -196,7 +197,7 @@ export async function loadSession(opts: ILoadSessionOpts = {}): Promise if (enableGuest && guestHsUrl && fragmentQueryParams.guest_user_id && fragmentQueryParams.guest_access_token) { logger.log("Using guest access credentials"); - return doSetLoggedIn( + await doSetLoggedIn( { userId: fragmentQueryParams.guest_user_id as string, accessToken: fragmentQueryParams.guest_access_token as string, @@ -206,7 +207,8 @@ export async function loadSession(opts: ILoadSessionOpts = {}): Promise }, true, false, - ).then(() => true); + ); + return true; } const success = await restoreSessionFromStorage({ ignoreGuest: Boolean(opts.ignoreGuest), @@ -225,6 +227,11 @@ export async function loadSession(opts: ILoadSessionOpts = {}): Promise // fall back to welcome screen return false; } catch (e) { + // We may be aborted e.g. because our token expired, so don't show an error here + if (opts.abortSignal?.aborted) { + return false; + } + if (e instanceof AbortLoginAndRebuildStorage) { // If we're aborting login because of a storage inconsistency, we don't // need to show the general failure dialog. Instead, just go back to welcome. @@ -236,7 +243,7 @@ export async function loadSession(opts: ILoadSessionOpts = {}): Promise return false; } - return handleLoadSessionFailure(e); + return handleLoadSessionFailure(e, opts); } } @@ -656,7 +663,7 @@ export async function restoreSessionFromStorage(opts?: { ignoreGuest?: boolean } } } -async function handleLoadSessionFailure(e: unknown): Promise { +async function handleLoadSessionFailure(e: unknown, loadSessionOpts?: ILoadSessionOpts): Promise { logger.error("Unable to load session", e); const modal = Modal.createDialog(SessionRestoreErrorDialog, { @@ -671,7 +678,7 @@ async function handleLoadSessionFailure(e: unknown): Promise { } // try, try again - return loadSession(); + return loadSession(loadSessionOpts); } /** diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index bc0cd36762..0175943327 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -235,6 +235,7 @@ export default class MatrixChat extends React.PureComponent { private themeWatcher?: ThemeWatcher; private fontWatcher?: FontWatcher; private readonly stores: SdkContextClass; + private loadSessionAbortController = new AbortController(); public constructor(props: IProps) { super(props); @@ -327,7 +328,7 @@ export default class MatrixChat extends React.PureComponent { // When the session loads it'll be detected as soft logged out and a dispatch // will be sent out to say that, triggering this MatrixChat to show the soft // logout page. - Lifecycle.loadSession(); + Lifecycle.loadSession({ abortSignal: this.loadSessionAbortController.signal }); return; } @@ -552,6 +553,7 @@ export default class MatrixChat extends React.PureComponent { guestHsUrl: this.getServerProperties().serverConfig.hsUrl, guestIsUrl: this.getServerProperties().serverConfig.isUrl, defaultDeviceDisplayName: this.props.defaultDeviceDisplayName, + abortSignal: this.loadSessionAbortController.signal, }); }) .then((loadedSession) => { @@ -1565,26 +1567,33 @@ export default class MatrixChat extends React.PureComponent { dis.fire(Action.FocusSendMessageComposer); }); - cli.on(HttpApiEvent.SessionLoggedOut, function (errObj) { + cli.on(HttpApiEvent.SessionLoggedOut, (errObj) => { + this.loadSessionAbortController.abort(errObj); + this.loadSessionAbortController = new AbortController(); + if (Lifecycle.isLoggingOut()) return; // A modal might have been open when we were logged out by the server Modal.forceCloseAllModals(); - if (errObj.httpStatus === 401 && errObj.data && errObj.data["soft_logout"]) { + if (errObj.httpStatus === 401 && errObj.data?.["soft_logout"]) { logger.warn("Soft logout issued by server - avoiding data deletion"); Lifecycle.softLogout(); return; } + dis.dispatch( + { + action: "logout", + }, + true, + ); + + // The above dispatch closes all modals, so open the modal after calling it synchronously Modal.createDialog(ErrorDialog, { title: _t("auth|session_logged_out_title"), description: _t("auth|session_logged_out_description"), }); - - dis.dispatch({ - action: "logout", - }); }); cli.on(HttpApiEvent.NoConsent, function (message, consentUri) { Modal.createDialog( diff --git a/test/unit-tests/Lifecycle-test.ts b/test/unit-tests/Lifecycle-test.ts index 94751b98c4..62a93b6d38 100644 --- a/test/unit-tests/Lifecycle-test.ts +++ b/test/unit-tests/Lifecycle-test.ts @@ -15,7 +15,7 @@ import { mocked, type MockedObject } from "jest-mock"; import fetchMock from "fetch-mock-jest"; import StorageEvictedDialog from "../../src/components/views/dialogs/StorageEvictedDialog"; -import { logout, restoreSessionFromStorage, setLoggedIn } from "../../src/Lifecycle"; +import * as Lifecycle from "../../src/Lifecycle"; import { MatrixClientPeg } from "../../src/MatrixClientPeg"; import Modal from "../../src/Modal"; import * as StorageAccess from "../../src/utils/StorageAccess"; @@ -28,19 +28,25 @@ import { Action } from "../../src/dispatcher/actions"; import PlatformPeg from "../../src/PlatformPeg"; import { persistAccessTokenInStorage, persistRefreshTokenInStorage } from "../../src/utils/tokens/tokens"; import { encryptPickleKey } from "../../src/utils/tokens/pickling"; +import * as StorageManager from "../../src/utils/StorageManager.ts"; +import type BasePlatform from "../../src/BasePlatform.ts"; + +const { logout, restoreSessionFromStorage, setLoggedIn } = Lifecycle; const webCrypto = new Crypto(); const windowCrypto = window.crypto; describe("Lifecycle", () => { - const mockPlatform = mockPlatformPeg(); + let mockPlatform: MockedObject; const realLocalStorage = global.localStorage; let mockClient!: MockedObject; beforeEach(() => { + jest.restoreAllMocks(); + mockPlatform = mockPlatformPeg(); mockClient = getMockClientWithEventEmitter({ ...mockClientMethodsUser(), stopClient: jest.fn(), @@ -182,6 +188,32 @@ describe("Lifecycle", () => { mac: expect.any(String), }; + describe("loadSession", () => { + beforeEach(() => { + // stub this out + jest.spyOn(Modal, "createDialog").mockReturnValue( + // @ts-ignore allow bad mock + { finished: Promise.resolve([true]) }, + ); + }); + + it("should not show any error dialog when checkConsistency throws but abortSignal has triggered", async () => { + jest.spyOn(StorageManager, "checkConsistency").mockRejectedValue(new Error("test error")); + + const abortController = new AbortController(); + const prom = Lifecycle.loadSession({ + enableGuest: true, + guestHsUrl: "https://guest.server", + fragmentQueryParams: { guest_user_id: "a", guest_access_token: "b" }, + abortSignal: abortController.signal, + }); + abortController.abort(); + await expect(prom).resolves.toBeFalsy(); + + expect(Modal.createDialog).not.toHaveBeenCalled(); + }); + }); + describe("restoreSessionFromStorage()", () => { beforeEach(() => { initLocalStorageMock();