Fix room joining over federation not specifying vias or using aliases (#30641)

* Fix room joining over federation not specifying vias or using aliases Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Be consistent with viaServers Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Simplify modules Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Only consider canAskToJoin on 403 as per spec Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Remove unused helper which I only just added =( Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Update tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Add tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Add test Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Add tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
2025-11-08 12:11:07 +01:00 · 2025-09-02 12:10:10 +01:00 · 2025-09-02 12:10:10 +01:00 · 8fa3d7e4b7
commit 8fa3d7e4b7
parent 1b4a979b6c
11 changed files with 133 additions and 77 deletions
--- a/src/RoomAliasCache.ts
+++ b/src/RoomAliasCache.ts
@ -6,6 +6,8 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
 Please see LICENSE files in the repository root for full details.
 */

+type CacheResult = { roomId: string; viaServers: string[] };
+
 /**
 * This is meant to be a cache of room alias to room ID so that moving between
 * rooms happens smoothly (for example using browser back / forward buttons).
@ -16,12 +18,12 @@ Please see LICENSE files in the repository root for full details.
 * A similar thing could also be achieved via `pushState` with a state object,
 * but keeping it separate like this seems easier in case we do want to extend.
 */
-const aliasToIDMap = new Map<string, string>();
+const cache = new Map<string, CacheResult>();

-export function storeRoomAliasInCache(alias: string, id: string): void {
-    aliasToIDMap.set(alias, id);
+export function storeRoomAliasInCache(alias: string, roomId: string, viaServers: string[]): void {
+    cache.set(alias, { roomId, viaServers });
 }

-export function getCachedRoomIDForAlias(alias: string): string | undefined {
-    return aliasToIDMap.get(alias);
+export function getCachedRoomIdForAlias(alias: string): CacheResult | undefined {
+    return cache.get(alias);
 }
--- a/src/components/structures/MatrixChat.tsx
+++ b/src/components/structures/MatrixChat.tsx
@ -51,7 +51,7 @@ import ThemeController from "../../settings/controllers/ThemeController";
 import { startAnyRegistrationFlow } from "../../Registration";
 import ResizeNotifier from "../../utils/ResizeNotifier";
 import AutoDiscoveryUtils from "../../utils/AutoDiscoveryUtils";
-import { makeRoomPermalink } from "../../utils/permalinks/Permalinks";
+import { calculateRoomVia, makeRoomPermalink } from "../../utils/permalinks/Permalinks";
 import ThemeWatcher, { ThemeWatcherEvent } from "../../settings/watchers/ThemeWatcher";
 import { FontWatcher } from "../../settings/watchers/FontWatcher";
 import { storeRoomAliasInCache } from "../../RoomAliasCache";
@ -1026,7 +1026,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
                presentedId = theAlias;
                // Store display alias of the presented room in cache to speed future
                // navigation.
-                storeRoomAliasInCache(theAlias, room.roomId);
+                storeRoomAliasInCache(theAlias, room.roomId, calculateRoomVia(room));
            }

            // Store this as the ID of the last room accessed. This is so that we can
--- a/src/components/views/dialogs/spotlight/SpotlightDialog.tsx
+++ b/src/components/views/dialogs/spotlight/SpotlightDialog.tsx
@ -53,7 +53,7 @@ import { getKeyBindingsManager } from "../../../../KeyBindingsManager";
 import { _t } from "../../../../languageHandler";
 import { MatrixClientPeg } from "../../../../MatrixClientPeg";
 import { PosthogAnalytics } from "../../../../PosthogAnalytics";
-import { getCachedRoomIDForAlias } from "../../../../RoomAliasCache";
+import { getCachedRoomIdForAlias } from "../../../../RoomAliasCache";
 import { showStartChatInviteDialog } from "../../../../RoomInvite";
 import { SettingLevel } from "../../../../settings/SettingLevel";
 import SettingsStore from "../../../../settings/SettingsStore";
@ -912,7 +912,7 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
        if (
            trimmedQuery.startsWith("#") &&
            trimmedQuery.includes(":") &&
-            (!getCachedRoomIDForAlias(trimmedQuery) || !cli.getRoom(getCachedRoomIDForAlias(trimmedQuery)))
+            (!getCachedRoomIdForAlias(trimmedQuery) || !cli.getRoom(getCachedRoomIdForAlias(trimmedQuery)!.roomId))
        ) {
            joinRoomSection = (
                <div className="mx_SpotlightDialog_section mx_SpotlightDialog_otherSearches" role="group">
--- a/src/modules/Navigation.ts
+++ b/src/modules/Navigation.ts
@ -9,33 +9,31 @@ import { type NavigationApi as INavigationApi } from "@element-hq/element-web-mo

 import { navigateToPermalink } from "../utils/permalinks/navigator.ts";
 import { parsePermalink } from "../utils/permalinks/Permalinks.ts";
-import { getCachedRoomIDForAlias } from "../RoomAliasCache.ts";
-import { MatrixClientPeg } from "../MatrixClientPeg.ts";
 import dispatcher from "../dispatcher/dispatcher.ts";
 import { Action } from "../dispatcher/actions.ts";
-import SettingsStore from "../settings/SettingsStore.ts";
+import type { ViewRoomPayload } from "../dispatcher/payloads/ViewRoomPayload.ts";

 export class NavigationApi implements INavigationApi {
    public async toMatrixToLink(link: string, join = false): Promise<void> {
        navigateToPermalink(link);

        const parts = parsePermalink(link);
-        if (parts?.roomIdOrAlias && join) {
-            let roomId: string | undefined = parts.roomIdOrAlias;
-            if (roomId.startsWith("#")) {
-                roomId = getCachedRoomIDForAlias(parts.roomIdOrAlias);
-                if (!roomId) {
-                    // alias resolution failed
-                    const result = await MatrixClientPeg.safeGet().getRoomIdForAlias(parts.roomIdOrAlias);
-                    roomId = result.room_id;
-                }
-            }
-
-            if (roomId) {
-                dispatcher.dispatch({
-                    action: Action.JoinRoom,
-                    canAskToJoin: SettingsStore.getValue("feature_ask_to_join"),
-                    roomId,
+        if (parts?.roomIdOrAlias) {
+            if (parts.roomIdOrAlias.startsWith("#")) {
+                dispatcher.dispatch<ViewRoomPayload>({
+                    action: Action.ViewRoom,
+                    room_alias: parts.roomIdOrAlias,
+                    via_servers: parts.viaServers ?? undefined,
+                    auto_join: join,
+                    metricsTrigger: undefined,
+                });
+            } else {
+                dispatcher.dispatch<ViewRoomPayload>({
+                    action: Action.ViewRoom,
+                    room_id: parts.roomIdOrAlias,
+                    via_servers: parts.viaServers ?? undefined,
+                    auto_join: join,
+                    metricsTrigger: undefined,
                });
            }
        }
--- a/src/modules/ProxiedModuleApi.ts
+++ b/src/modules/ProxiedModuleApi.ts
@ -28,13 +28,12 @@ import dispatcher from "../dispatcher/dispatcher";
 import { navigateToPermalink } from "../utils/permalinks/navigator";
 import { parsePermalink } from "../utils/permalinks/Permalinks";
 import { MatrixClientPeg } from "../MatrixClientPeg";
-import { getCachedRoomIDForAlias } from "../RoomAliasCache";
 import { Action } from "../dispatcher/actions";
 import { type OverwriteLoginPayload } from "../dispatcher/payloads/OverwriteLoginPayload";
 import { type ActionPayload } from "../dispatcher/payloads";
-import SettingsStore from "../settings/SettingsStore";
 import WidgetStore, { type IApp } from "../stores/WidgetStore";
 import { type Container, WidgetLayoutStore } from "../stores/widgets/WidgetLayoutStore";
+import type { ViewRoomPayload } from "../dispatcher/payloads/ViewRoomPayload.ts";

 /**
 * Glue between the `ModuleApi` interface and the react-sdk. Anticipates one instance
@ -183,28 +182,22 @@ export class ProxiedModuleApi implements ModuleApi {
        navigateToPermalink(uri);

        const parts = parsePermalink(uri);
-        if (parts?.roomIdOrAlias && andJoin) {
-            let roomId: string | undefined = parts.roomIdOrAlias;
-            let servers = parts.viaServers;
-            if (roomId.startsWith("#")) {
-                roomId = getCachedRoomIDForAlias(parts.roomIdOrAlias);
-                if (!roomId) {
-                    // alias resolution failed
-                    const result = await MatrixClientPeg.safeGet().getRoomIdForAlias(parts.roomIdOrAlias);
-                    roomId = result.room_id;
-                    if (!servers) servers = result.servers; // use provided servers first, if available
-                }
-            }
-            dispatcher.dispatch({
+        if (parts?.roomIdOrAlias) {
+            if (parts.roomIdOrAlias.startsWith("#")) {
+                dispatcher.dispatch<ViewRoomPayload>({
                    action: Action.ViewRoom,
-                room_id: roomId,
-                via_servers: servers,
+                    room_alias: parts.roomIdOrAlias,
+                    via_servers: parts.viaServers ?? undefined,
+                    auto_join: andJoin ?? false,
+                    metricsTrigger: undefined,
                });
-
-            if (andJoin) {
-                dispatcher.dispatch({
-                    action: Action.JoinRoom,
-                    canAskToJoin: SettingsStore.getValue("feature_ask_to_join"),
+            } else {
+                dispatcher.dispatch<ViewRoomPayload>({
+                    action: Action.ViewRoom,
+                    room_id: parts.roomIdOrAlias,
+                    via_servers: parts.viaServers ?? undefined,
+                    auto_join: andJoin ?? false,
+                    metricsTrigger: undefined,
                });
            }
        }
--- a/src/stores/RoomViewStore.tsx
+++ b/src/stores/RoomViewStore.tsx
@ -26,7 +26,7 @@ import { type MatrixDispatcher } from "../dispatcher/dispatcher";
 import { MatrixClientPeg } from "../MatrixClientPeg";
 import Modal from "../Modal";
 import { _t } from "../languageHandler";
-import { getCachedRoomIDForAlias, storeRoomAliasInCache } from "../RoomAliasCache";
+import { getCachedRoomIdForAlias, storeRoomAliasInCache } from "../RoomAliasCache";
 import { Action } from "../dispatcher/actions";
 import { retry } from "../utils/promise";
 import { TimelineRenderingType } from "../contexts/RoomContext";
@ -438,6 +438,7 @@ export class RoomViewStore extends EventEmitter {
                    action: Action.JoinRoom,
                    roomId: payload.room_id,
                    metricsTrigger: payload.metricsTrigger as JoinRoomPayload["metricsTrigger"],
+                    canAskToJoin: SettingsStore.getValue("feature_ask_to_join"),
                });
            }

@ -445,10 +446,16 @@ export class RoomViewStore extends EventEmitter {
                await setMarkedUnreadState(room, MatrixClientPeg.safeGet(), false);
            }
        } else if (payload.room_alias) {
+            let roomId: string;
+            let viaServers: string[] | undefined;
+
            // Try the room alias to room ID navigation cache first to avoid
            // blocking room navigation on the homeserver.
-            let roomId = getCachedRoomIDForAlias(payload.room_alias);
-            if (!roomId) {
+            const cachedResult = getCachedRoomIdForAlias(payload.room_alias);
+            if (cachedResult) {
+                roomId = cachedResult.roomId;
+                viaServers = cachedResult.viaServers;
+            } else {
                // Room alias cache miss, so let's ask the homeserver. Resolve the alias
                // and then do a second dispatch with the room ID acquired.
                this.setState({
@ -467,8 +474,9 @@ export class RoomViewStore extends EventEmitter {
                });
                try {
                    const result = await MatrixClientPeg.safeGet().getRoomIdForAlias(payload.room_alias);
-                    storeRoomAliasInCache(payload.room_alias, result.room_id);
+                    storeRoomAliasInCache(payload.room_alias, result.room_id, result.servers);
                    roomId = result.room_id;
+                    viaServers = result.servers;
                } catch (err) {
                    logger.error("RVS failed to get room id for alias: ", err);
                    this.dis?.dispatch<ViewRoomErrorPayload>({
@ -485,6 +493,7 @@ export class RoomViewStore extends EventEmitter {
            this.dis?.dispatch({
                ...payload,
                room_id: roomId,
+                via_servers: viaServers,
            });
        }
    }
@ -509,12 +518,13 @@ export class RoomViewStore extends EventEmitter {
            joining: true,
        });

-        // take a copy of roomAlias & roomId as they may change by the time the join is complete
-        const { roomAlias, roomId } = this.state;
-        const address = payload.roomId || roomAlias || roomId!;
+        // take a copy of roomAlias, roomId & viaServers as they may change by the time the join is complete
+        const { roomAlias, roomId = payload.roomId, viaServers = [] } = this.state;
+        // prefer the room alias if we have one as it allows joining over federation even with no viaServers
+        const address = roomAlias || roomId!;

        const joinOpts: IJoinRoomOpts = {
-            viaServers: this.state.viaServers || [],
+            viaServers,
            ...(payload.opts ?? {}),
        };
        if (SettingsStore.getValue("feature_share_history_on_invite")) {
@ -547,7 +557,7 @@ export class RoomViewStore extends EventEmitter {
                canAskToJoin: payload.canAskToJoin,
            });

-            if (payload.canAskToJoin) {
+            if (payload.canAskToJoin && err instanceof MatrixError && err.httpStatus === 403) {
                this.dis?.dispatch({ action: Action.PromptAskToJoin });
            }
        }
--- a/src/stores/spaces/SpaceStore.ts
+++ b/src/stores/spaces/SpaceStore.ts
@ -49,7 +49,7 @@ import {
    UPDATE_SUGGESTED_ROOMS,
    UPDATE_TOP_LEVEL_SPACES,
 } from ".";
-import { getCachedRoomIDForAlias } from "../../RoomAliasCache";
+import { getCachedRoomIdForAlias } from "../../RoomAliasCache";
 import { EffectiveMembership, getEffectiveMembership } from "../../utils/membership";
 import {
    flattenSpaceHierarchyWithCache,
@ -1249,7 +1249,8 @@ export class SpaceStoreClass extends AsyncStoreWithClient<EmptyObject> {
                let roomId = payload.room_id;

                if (payload.room_alias && !roomId) {
-                    roomId = getCachedRoomIDForAlias(payload.room_alias);
+                    const result = getCachedRoomIdForAlias(payload.room_alias);
+                    if (result) roomId = result.roomId;
                }

                if (!roomId) return; // we'll get re-fired with the room ID shortly
--- a/test/unit-tests/modules/Navigation-test.ts
+++ b/test/unit-tests/modules/Navigation-test.ts
@ -5,37 +5,35 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
 Please see LICENSE files in the repository root for full details.
 */

-import { mocked } from "jest-mock";
-
 import * as navigator from "../../../src/utils/permalinks/navigator";
 import { NavigationApi } from "../../../src/modules/Navigation.ts";
-import { stubClient } from "../../test-utils";
 import defaultDispatcher from "../../../src/dispatcher/dispatcher.ts";

 describe("NavigationApi", () => {
    const api = new NavigationApi();

    describe("toMatrixToLink", () => {
-        it("should call navigateToPermalink with the correct parameters", async () => {
-            const link = "https://matrix.to/#/!roomId:server.com";
+        it.each([
+            ["roomId", "https://matrix.to/#/!roomId:server.com"],
+            ["roomAlias", "https://matrix.to/#/#alias:server.com"],
+            ["user", "https://matrix.to/#/@user:server.com"],
+        ])("should call navigateToPermalink with the correct parameters for %s", async (_type, link) => {
            const spy = jest.spyOn(navigator, "navigateToPermalink");

            await api.toMatrixToLink(link);
            expect(spy).toHaveBeenCalledWith(link);
        });

-        it("should resolve the room alias to a room id when join=true", async () => {
-            const cli = stubClient();
-            mocked(cli.getRoomIdForAlias).mockResolvedValue({ room_id: "!roomId:server.com", servers: [] });
-
-            const link = "https://matrix.to/#/#alias:server.com";
+        it("should set auto_join to true when join=true", async () => {
+            const link = "https://matrix.to/#/#alias:server.com?via=server.com";
            const spy = jest.spyOn(defaultDispatcher, "dispatch");

            await api.toMatrixToLink(link, true);
            expect(spy).toHaveBeenCalledWith(
                expect.objectContaining({
-                    action: "join_room",
-                    roomId: "!roomId:server.com",
+                    action: "view_room",
+                    room_alias: "#alias:server.com",
+                    auto_join: true,
                }),
            );
        });
--- a/test/unit-tests/modules/ProxiedModuleApi-test.tsx
+++ b/test/unit-tests/modules/ProxiedModuleApi-test.tsx
@ -24,6 +24,7 @@ import defaultDispatcher from "../../../src/dispatcher/dispatcher";
 import { Action } from "../../../src/dispatcher/actions";
 import WidgetStore, { type IApp } from "../../../src/stores/WidgetStore";
 import { Container, WidgetLayoutStore } from "../../../src/stores/widgets/WidgetLayoutStore";
+import * as navigator from "../../../src/utils/permalinks/navigator.ts";

 describe("ProxiedApiModule", () => {
    afterEach(() => {
@ -361,4 +362,30 @@ describe("ProxiedApiModule", () => {
            expect(WidgetLayoutStore.instance.moveToContainer).toHaveBeenCalledWith(room, app, Container.Top);
        });
    });
+
+    describe("navigatePermalink", () => {
+        const api = new ProxiedModuleApi();
+
+        it("should call navigateToPermalink with the correct parameters", async () => {
+            const link = "https://matrix.to/#/!roomId:server.com";
+            const spy = jest.spyOn(navigator, "navigateToPermalink");
+
+            await api.navigatePermalink(link);
+            expect(spy).toHaveBeenCalledWith(link);
+        });
+
+        it("should set auto_join to true when join=true", async () => {
+            const link = "https://matrix.to/#/#alias:server.com";
+            const spy = jest.spyOn(defaultDispatcher, "dispatch");
+
+            await api.navigatePermalink(link, true);
+            expect(spy).toHaveBeenCalledWith(
+                expect.objectContaining({
+                    action: "view_room",
+                    room_alias: "#alias:server.com",
+                    auto_join: true,
+                }),
+            );
+        });
+    });
 });
--- a/test/unit-tests/stores/RoomViewStore-test.ts
+++ b/test/unit-tests/stores/RoomViewStore-test.ts
@ -43,6 +43,7 @@ import { type IApp } from "../../../src/utils/WidgetUtils-types";
 import { CallStore } from "../../../src/stores/CallStore";
 import { MatrixClientPeg } from "../../../src/MatrixClientPeg";
 import MediaDeviceHandler, { MediaDeviceKindEnum } from "../../../src/MediaDeviceHandler";
+import { storeRoomAliasInCache } from "../../../src/RoomAliasCache.ts";

 jest.mock("../../../src/Modal");

@ -211,6 +212,22 @@ describe("RoomViewStore", function () {
        expect(roomViewStore.isJoining()).toBe(true);
    });

+    it("can be used to view a room by alias with auto_join", async () => {
+        const alias = "#alias12345:server";
+        storeRoomAliasInCache(alias, roomId, ["server1"]);
+        dis.dispatch({ action: Action.ViewRoom, room_alias: alias, auto_join: true }, true);
+        await expect(untilDispatch(Action.ViewRoom, dis)).resolves.toEqual(
+            expect.objectContaining({
+                action: Action.ViewRoom,
+                room_id: roomId,
+                auto_join: true,
+            }),
+        );
+        await untilDispatch(Action.JoinRoomReady, dis);
+        expect(mockClient.joinRoom).toHaveBeenCalledWith(alias, { viaServers: ["server1"] });
+        expect(roomViewStore.isJoining()).toBe(true);
+    });
+
    it("can auto-join a room", async () => {
        dis.dispatch({ action: Action.ViewRoom, room_id: roomId, auto_join: true });
        await untilDispatch(Action.JoinRoomReady, dis);
@ -422,8 +439,8 @@ describe("RoomViewStore", function () {
    });

    describe("Action.JoinRoom", () => {
-        it("dispatches Action.JoinRoomError and Action.AskToJoin when the join fails", async () => {
-            const err = new MatrixError();
+        it("dispatches Action.JoinRoomError and Action.AskToJoin when the join fails with 403", async () => {
+            const err = new MatrixError({}, 403);

            jest.spyOn(dis, "dispatch");
            jest.spyOn(mockClient, "joinRoom").mockRejectedValueOnce(err);
--- a/test/unit-tests/stores/SpaceStore-test.ts
+++ b/test/unit-tests/stores/SpaceStore-test.ts
@ -41,6 +41,7 @@ import RoomListStore from "../../../src/stores/room-list/RoomListStore";
 import { DefaultTagID } from "../../../src/stores/room-list/models";
 import { RoomNotificationStateStore } from "../../../src/stores/notifications/RoomNotificationStateStore";
 import { NotificationLevel } from "../../../src/stores/notifications/NotificationLevel";
+import { storeRoomAliasInCache } from "../../../src/RoomAliasCache.ts";

 jest.useFakeTimers();

@ -1217,6 +1218,15 @@ describe("SpaceStore", () => {
            viewRoom(room1);
            expect(store.activeSpace).toBe(MetaSpace.Home);
        });
+
+        it("should check alias cache if switching to a room by alias", async () => {
+            viewRoom(room2);
+            storeRoomAliasInCache("#alias:server", room3, []);
+
+            store.setActiveSpace(space2, false);
+            defaultDispatcher.dispatch({ action: Action.ViewRoom, room_alias: "#alias:server" }, true);
+            expect(store.activeSpace).toBe(space1);
+        });
    });

    describe("traverseSpace", () => {