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 <7t3chguy@gmail.com>

* Add tests

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
This commit is contained in:
Ole Wieners 2023-10-25 12:08:10 +02:00 committed by GitHub
parent a306a08780
commit 4ff35f0471
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 111 additions and 13 deletions

View file

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

View file

@ -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.",

View file

@ -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;

View file

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

View file

@ -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,

View file

@ -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."`,
);
});
});
});