Fix room joining over federation not specifying vias or using aliases

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);
 }

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

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">

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,
                 });
             }
         }

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({
-                action: Action.ViewRoom,
-                room_id: roomId,
-                via_servers: servers,
-            });
-
-            if (andJoin) {
-                dispatcher.dispatch({
-                    action: Action.JoinRoom,
-                    canAskToJoin: SettingsStore.getValue("feature_ask_to_join"),
+        if (parts?.roomIdOrAlias) {
+            if (parts.roomIdOrAlias.startsWith("#")) {
+                dispatcher.dispatch<ViewRoomPayload>({
+                    action: Action.ViewRoom,
+                    room_alias: parts.roomIdOrAlias,
+                    via_servers: parts.viaServers ?? undefined,
+                    auto_join: andJoin ?? false,
+                    metricsTrigger: undefined,
+                });
+            } else {
+                dispatcher.dispatch<ViewRoomPayload>({
+                    action: Action.ViewRoom,
+                    room_id: parts.roomIdOrAlias,
+                    via_servers: parts.viaServers ?? undefined,
+                    auto_join: andJoin ?? false,
+                    metricsTrigger: undefined,
                 });
             }
         }

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,17 +438,24 @@ 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"),
                 });
             }
 
             if (room) {
                 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 });
             }
         }

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

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,
                 }),
             );
         });

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,
+                }),
+            );
+        });
+    });
 });