From a25920fdc67a7fa4fa3d825135ac8dbd313d3650 Mon Sep 17 00:00:00 2001 From: Mikhail Aheichyk Date: Wed, 1 Mar 2023 16:09:51 +0300 Subject: [PATCH] Fixes authentication when user is registered via module API Signed-off-by: Mikhail Aheichyk --- src/Lifecycle.ts | 18 ++++-- src/modules/ProxiedModuleApi.ts | 18 ++++++ test/Lifecycle-test.ts | 86 +++++++++++++++++++++++++++ test/modules/ProxiedModuleApi-test.ts | 26 ++++++++ test/test-utils/test-utils.ts | 3 + 5 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 test/Lifecycle-test.ts diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 590b7eb1801..8937fe355b5 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -73,8 +73,9 @@ dis.register((payload) => { onLoggedOut(); } else if (payload.action === Action.OverwriteLogin) { const typed = payload; + // is invoked without dispatching of "on_logging_in" // noinspection JSIgnoredPromiseFromCall - we don't care if it fails - doSetLoggedIn(typed.credentials, true); + doSetLoggedIn(typed.credentials, true, false); } }); @@ -573,10 +574,14 @@ export async function hydrateSession(credentials: IMatrixClientCreds): Promise { +async function doSetLoggedIn( + credentials: IMatrixClientCreds, + clearStorageEnabled: boolean, + dispatchOnLoggingIn = true, +): Promise { credentials.guest = Boolean(credentials.guest); const softLogout = isSoftLogout(); @@ -602,7 +607,12 @@ async function doSetLoggedIn(credentials: IMatrixClientCreds, clearStorageEnable // // we fire it *synchronously* to make sure it fires before on_logged_in. // (dis.dispatch uses `window.setTimeout`, which does not guarantee ordering.) - dis.dispatch({ action: "on_logging_in" }, true); + // + // can be disabled to resolve "Cannot dispatch in the middle of a dispatch." + // error when it is invoked via another dispatch that is not yet finished. + if (dispatchOnLoggingIn) { + dis.dispatch({ action: "on_logging_in" }, true); + } if (clearStorageEnabled) { await clearStorage(); diff --git a/src/modules/ProxiedModuleApi.ts b/src/modules/ProxiedModuleApi.ts index 818a1f9fcdf..2f3c5a519a0 100644 --- a/src/modules/ProxiedModuleApi.ts +++ b/src/modules/ProxiedModuleApi.ts @@ -36,6 +36,7 @@ import { MatrixClientPeg } from "../MatrixClientPeg"; import { getCachedRoomIDForAlias } from "../RoomAliasCache"; import { Action } from "../dispatcher/actions"; import { OverwriteLoginPayload } from "../dispatcher/payloads/OverwriteLoginPayload"; +import { ActionPayload } from "../dispatcher/payloads"; /** * Glue between the `ModuleApi` interface and the react-sdk. Anticipates one instance @@ -44,6 +45,18 @@ import { OverwriteLoginPayload } from "../dispatcher/payloads/OverwriteLoginPayl export class ProxiedModuleApi implements ModuleApi { private cachedTranslations: Optional; + private overrideLoginResolve?: () => void; + + public constructor() { + dispatcher.register(this.onAction); + } + + private onAction = (payload: ActionPayload): void => { + if (payload.action === Action.OnLoggedIn) { + this.overrideLoginResolve?.(); + } + }; + /** * All custom translations used by the associated module. */ @@ -154,6 +167,11 @@ export class ProxiedModuleApi implements ModuleApi { }, true, ); // require to be sync to match inherited interface behaviour + + // wait for login to complete + await new Promise((resolve) => { + this.overrideLoginResolve = resolve; + }); } /** diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts new file mode 100644 index 00000000000..bce03e6830a --- /dev/null +++ b/test/Lifecycle-test.ts @@ -0,0 +1,86 @@ +/* +Copyright 2023 Mikhail Aheichyk +Copyright 2023 Nordeck IT + Consulting GmbH. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { AccountAuthInfo } from "@matrix-org/react-sdk-module-api/lib/types/AccountAuthInfo"; +import { logger } from "matrix-js-sdk/src/logger"; + +import defaultDispatcher from "../src/dispatcher/dispatcher"; +import { OverwriteLoginPayload } from "../src/dispatcher/payloads/OverwriteLoginPayload"; +import { Action } from "../src/dispatcher/actions"; +import { setLoggedIn } from "../src/Lifecycle"; +import { stubClient } from "./test-utils"; +import { ActionPayload } from "../src/dispatcher/payloads"; + +jest.mock("../src/utils/createMatrixClient", () => ({ + __esModule: true, + default: jest.fn().mockReturnValue({ + clearStores: jest.fn(), + }), +})); + +describe("Lifecycle", () => { + stubClient(); + + jest.spyOn(logger, "error").mockReturnValue(undefined); + jest.spyOn(logger, "warn").mockReturnValue(undefined); + jest.spyOn(logger, "log").mockImplementation(undefined); + + it("should call 'overwrite_login' callback and receive 'on_logged_in'", async () => { + // promise to wait 'on_logged_in' + const loggedInPromise = new Promise((resolve, reject) => { + defaultDispatcher.register((payload: ActionPayload) => { + if (payload.action === Action.OnLoggedIn) { + resolve(undefined); + } + }); + }); + + // dispatch 'overwrite_login' + const accountInfo = {} as unknown as AccountAuthInfo; + defaultDispatcher.dispatch( + { + action: Action.OverwriteLogin, + credentials: { + ...accountInfo, + guest: false, + }, + }, + true, + ); + + await expect(loggedInPromise).resolves.toBeUndefined(); + }); + + it("should setLoggedIn", async () => { + // promise to wait 'on_logging_in' + const loggingInPromise = new Promise((resolve, reject) => { + defaultDispatcher.register((payload: ActionPayload) => { + if (payload.action === "on_logging_in") { + resolve(undefined); + } + }); + }); + + await setLoggedIn({ + accessToken: "some_token", + homeserverUrl: "https://example.com", + userId: "@some_user_id", + }); + + await expect(loggingInPromise).resolves.toBeUndefined(); + }); +}); diff --git a/test/modules/ProxiedModuleApi-test.ts b/test/modules/ProxiedModuleApi-test.ts index 4a29e37453f..881e98c9dff 100644 --- a/test/modules/ProxiedModuleApi-test.ts +++ b/test/modules/ProxiedModuleApi-test.ts @@ -15,12 +15,15 @@ limitations under the License. */ import { TranslationStringsObject } from "@matrix-org/react-sdk-module-api/lib/types/translations"; +import { AccountAuthInfo } from "@matrix-org/react-sdk-module-api/lib/types/AccountAuthInfo"; import { ProxiedModuleApi } from "../../src/modules/ProxiedModuleApi"; import { stubClient } from "../test-utils"; import { setLanguage } from "../../src/languageHandler"; import { ModuleRunner } from "../../src/modules/ModuleRunner"; import { registerMockModule } from "./MockModule"; +import defaultDispatcher from "../../src/dispatcher/dispatcher"; +import { Action } from "../../src/dispatcher/actions"; describe("ProxiedApiModule", () => { afterEach(() => { @@ -44,6 +47,29 @@ describe("ProxiedApiModule", () => { expect(api.translations).toBe(translations); }); + it("should overwriteAccountAuth", async () => { + const dispatchSpy = jest.spyOn(defaultDispatcher, "dispatch"); + + const api = new ProxiedModuleApi(); + const accountInfo = {} as unknown as AccountAuthInfo; + const promise = api.overwriteAccountAuth(accountInfo); + + expect(dispatchSpy).toHaveBeenCalledWith( + expect.objectContaining({ + action: Action.OverwriteLogin, + credentials: { + ...accountInfo, + guest: false, + }, + }), + expect.anything(), + ); + + defaultDispatcher.fire(Action.OnLoggedIn); + + await expect(promise).resolves.toBeUndefined(); + }); + describe("integration", () => { it("should translate strings using translation system", async () => { // Test setup diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 2e700c2b185..9596d0fd10b 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -231,6 +231,9 @@ export function createTestClient(): MatrixClient { room_id: roomId, }); }), + startClient: jest.fn().mockResolvedValue(undefined), + stopClient: jest.fn().mockReturnValue(undefined), + removeAllListeners: jest.fn().mockReturnValue(undefined), } as unknown as MatrixClient; client.reEmitter = new ReEmitter(client);