Fix unfederated invite dialog (#9618)

* clarify error message for unfederated room invites * hide external user suggesetions * rename some descriptors * fix i18n * add warning for unfederated spaces * i18n Signed-off-by: Michael Telatynski <[email protected]> * Add tests Signed-off-by: Michael Telatynski <[email protected]> --------- Signed-off-by: Michael Telatynski <[email protected]> Co-authored-by: Michael Telatynski <[email protected]>
matrix-org · Oct 25, 2023 · 4ff35f0 · 4ff35f0
1 parent a306a08
commit 4ff35f0
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 13 deletions.
diff --git a/src/components/views/dialogs/InviteDialog.tsx b/src/components/views/dialogs/InviteDialog.tsx
@@ -16,7 +16,7 @@ limitations under the License.
 
 import React, { createRef, ReactNode, SyntheticEvent } from "react";
 import classNames from "classnames";
-import { RoomMember, Room, MatrixError } from "matrix-js-sdk/src/matrix";
+import { RoomMember, Room, MatrixError, EventType } from "matrix-js-sdk/src/matrix";
 import { MatrixCall } from "matrix-js-sdk/src/webrtc/call";
 import { logger } from "matrix-js-sdk/src/logger";
 import { uniqBy } from "lodash";
@@ -368,26 +368,32 @@ export default class InviteDialog extends React.PureComponent<Props, IInviteDial
 
         this.profilesStore = SdkContextClass.instance.userProfilesStore;
 
-        const alreadyInvited = new Set([MatrixClientPeg.safeGet().getUserId()!]);
+        const excludedIds = new Set([MatrixClientPeg.safeGet().getUserId()!]);
         const welcomeUserId = SdkConfig.get("welcome_user_id");
-        if (welcomeUserId) alreadyInvited.add(welcomeUserId);
+        if (welcomeUserId) excludedIds.add(welcomeUserId);
 
         if (isRoomInvite(props)) {
             const room = MatrixClientPeg.safeGet().getRoom(props.roomId);
+            const isFederated = room?.currentState.getStateEvents(EventType.RoomCreate, "")?.getContent()["m.federate"];
             if (!room) throw new Error("Room ID given to InviteDialog does not look like a room");
-            room.getMembersWithMembership("invite").forEach((m) => alreadyInvited.add(m.userId));
-            room.getMembersWithMembership("join").forEach((m) => alreadyInvited.add(m.userId));
+            room.getMembersWithMembership("invite").forEach((m) => excludedIds.add(m.userId));
+            room.getMembersWithMembership("join").forEach((m) => excludedIds.add(m.userId));
             // add banned users, so we don't try to invite them
-            room.getMembersWithMembership("ban").forEach((m) => alreadyInvited.add(m.userId));
+            room.getMembersWithMembership("ban").forEach((m) => excludedIds.add(m.userId));
+            if (isFederated === false) {
+                // exclude users from external servers
+                const homeserver = props.roomId.split(":")[1];
+                this.excludeExternals(homeserver, excludedIds);
+            }
         }
 
         this.state = {
             targets: [], // array of Member objects (see interface above)
             filterText: this.props.initialText || "",
             // Mutates alreadyInvited set so that buildSuggestions doesn't duplicate any users
-            recents: InviteDialog.buildRecents(alreadyInvited),
+            recents: InviteDialog.buildRecents(excludedIds),
             numRecentsShown: INITIAL_ROOMS_SHOWN,
-            suggestions: this.buildSuggestions(alreadyInvited),
+            suggestions: this.buildSuggestions(excludedIds),
             numSuggestionsShown: INITIAL_ROOMS_SHOWN,
             serverResultsMixin: [],
             threepidResultsMixin: [],
@@ -418,6 +424,18 @@ export default class InviteDialog extends React.PureComponent<Props, IInviteDial
         this.setState({ consultFirst: ev.target.checked });
     };
 
+    private excludeExternals(homeserver: string, excludedTargetIds: Set<string>): void {
+        const client = MatrixClientPeg.safeGet();
+        // users with room membership
+        const members = Object.values(buildMemberScores(client)).map(({ member }) => member.userId);
+        // users with dm membership
+        const roomMembers = Object.keys(DMRoomMap.shared().getUniqueRoomsWithIndividuals());
+        roomMembers.forEach((id) => members.push(id));
+        // filter duplicates and user IDs from external servers
+        const externals = new Set(members.filter((id) => !id.includes(homeserver)));
+        externals.forEach((id) => excludedTargetIds.add(id));
+    }
+
     public static buildRecents(excludedTargetIds: Set<string>): Result[] {
         const rooms = DMRoomMap.shared().getUniqueRoomsWithIndividuals(); // map of userId => js-sdk Room
 

diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json
@@ -1260,6 +1260,8 @@
         "error_permissions_space": "You do not have permission to invite people to this space.",
         "error_profile_undisclosed": "User may or may not exist",
         "error_transfer_multiple_target": "A call can only be transferred to a single user.",
+        "error_unfederated_room": "This room is unfederated. You cannot invite people from external servers.",
+        "error_unfederated_space": "This space is unfederated. You cannot invite people from external servers.",
         "error_unknown": "Unknown server error",
         "error_user_not_found": "User does not exist",
         "error_version_unsupported_room": "The user's homeserver does not support the version of the room.",

diff --git a/src/utils/MultiInviter.ts b/src/utils/MultiInviter.ts
@@ -246,16 +246,26 @@ export default class MultiInviter {
 
                     logger.error(err);
 
-                    const isSpace = this.roomId && this.matrixClient.getRoom(this.roomId)?.isSpaceRoom();
+                    const room = this.roomId ? this.matrixClient.getRoom(this.roomId) : null;
+                    const isSpace = room?.isSpaceRoom();
+                    const isFederated = room?.currentState.getStateEvents(EventType.RoomCreate, "")?.getContent()[
+                        "m.federate"
+                    ];
 
                     let errorText: string | undefined;
                     let fatal = false;
                     switch (err.errcode) {
                         case "M_FORBIDDEN":
                             if (isSpace) {
-                                errorText = _t("invite|error_permissions_space");
+                                errorText =
+                                    isFederated === false
+                                        ? _t("invite|error_unfederated_space")
+                                        : _t("invite|error_permissions_space");
                             } else {
-                                errorText = _t("invite|error_permissions_room");
+                                errorText =
+                                    isFederated === false
+                                        ? _t("invite|error_unfederated_room")
+                                        : _t("invite|error_permissions_room");
                             }
                             fatal = true;
                             break;

diff --git a/test/components/views/dialogs/InviteDialog-test.tsx b/test/components/views/dialogs/InviteDialog-test.tsx
@@ -477,4 +477,20 @@ describe("InviteDialog", () => {
             ]);
         });
     });
+
+    it("should not suggest users from other server when room has m.federate=false", async () => {
+        SdkConfig.add({ welcome_user_id: "@bot:example.org" });
+        room.currentState.setStateEvents([mkRoomCreateEvent(bobId, roomId, { "m.federate": false })]);
+
+        render(
+            <InviteDialog
+                kind={InviteKind.Invite}
+                roomId={roomId}
+                onFinished={jest.fn()}
+                initialText="@localpart:server.tld"
+            />,
+        );
+        await flushPromises();
+        expect(screen.queryByText("@localpart:server.tld")).not.toBeInTheDocument();
+    });
 });
diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts
@@ -291,13 +291,14 @@ type MakeEventProps = MakeEventPassThruProps & {
     unsigned?: IUnsigned;
 };
 
-export const mkRoomCreateEvent = (userId: string, roomId: string): MatrixEvent => {
+export const mkRoomCreateEvent = (userId: string, roomId: string, content?: IContent): MatrixEvent => {
     return mkEvent({
         event: true,
         type: EventType.RoomCreate,
         content: {
             creator: userId,
             room_version: KNOWN_SAFE_ROOM_VERSION,
+            ...content,
         },
         skey: "",
         user: userId,

diff --git a/test/utils/MultiInviter-test.ts b/test/utils/MultiInviter-test.ts
@@ -15,7 +15,7 @@ limitations under the License.
 */
 
 import { mocked } from "jest-mock";
-import { MatrixClient, MatrixError, Room, RoomMember } from "matrix-js-sdk/src/matrix";
+import { EventType, MatrixClient, MatrixError, MatrixEvent, Room, RoomMember } from "matrix-js-sdk/src/matrix";
 
 import { MatrixClientPeg } from "../../src/MatrixClientPeg";
 import Modal, { ComponentType, ComponentProps } from "../../src/Modal";
@@ -187,5 +187,56 @@ describe("MultiInviter", () => {
             });
             expect(client.unban).toHaveBeenCalledWith(ROOMID, MXID1);
         });
+
+        it("should show sensible error when attempting to invite over federation with m.federate=false", async () => {
+            mocked(client.invite).mockRejectedValueOnce(
+                new MatrixError({
+                    errcode: "M_FORBIDDEN",
+                }),
+            );
+            const room = new Room(ROOMID, client, client.getSafeUserId());
+            room.currentState.setStateEvents([
+                new MatrixEvent({
+                    type: EventType.RoomCreate,
+                    state_key: "",
+                    content: {
+                        "m.federate": false,
+                    },
+                    room_id: ROOMID,
+                }),
+            ]);
+            mocked(client.getRoom).mockReturnValue(room);
+
+            await inviter.invite(["@user:other_server"]);
+            expect(inviter.getErrorText("@user:other_server")).toMatchInlineSnapshot(
+                `"This room is unfederated. You cannot invite people from external servers."`,
+            );
+        });
+
+        it("should show sensible error when attempting to invite over federation with m.federate=false to space", async () => {
+            mocked(client.invite).mockRejectedValueOnce(
+                new MatrixError({
+                    errcode: "M_FORBIDDEN",
+                }),
+            );
+            const room = new Room(ROOMID, client, client.getSafeUserId());
+            room.currentState.setStateEvents([
+                new MatrixEvent({
+                    type: EventType.RoomCreate,
+                    state_key: "",
+                    content: {
+                        "m.federate": false,
+                        "type": "m.space",
+                    },
+                    room_id: ROOMID,
+                }),
+            ]);
+            mocked(client.getRoom).mockReturnValue(room);
+
+            await inviter.invite(["@user:other_server"]);
+            expect(inviter.getErrorText("@user:other_server")).toMatchInlineSnapshot(
+                `"This space is unfederated. You cannot invite people from external servers."`,
+            );
+        });
     });
 });