From b9f319a9f5b5607d60057c5918024313b8e4f17b Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Wed, 4 Jun 2025 23:09:19 +0530 Subject: [PATCH] Add sanity checks to prevent users from ignoring themselves (#30079) * Fix missing state * Throw error if membership event changes * Write test * Fix broken tests * Cache inviter when room is loaded * Translate error message for dialog --- src/components/structures/RoomView.tsx | 56 +++++++++++++++++-- src/i18n/strings/en_EN.json | 1 + .../components/structures/RoomView-test.tsx | 39 ++++++++++++- .../__snapshots__/RoomView-test.tsx.snap | 12 ++-- 4 files changed, 95 insertions(+), 13 deletions(-) diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index fbff50bd01..d64f4b7a36 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -370,6 +370,10 @@ export class RoomView extends React.Component { private unmounted = false; private permalinkCreators: Record = {}; + // The userId from which we received this invite. + // Only populated if the membership of our user is invite. + private inviter?: string; + private roomView = createRef(); private searchResultsPanel = createRef(); private messagePanel: TimelinePanel | null = null; @@ -1350,6 +1354,11 @@ export class RoomView extends React.Component { // after a successful peek, or after we join the room). private onRoomLoaded = (room: Room): void => { if (this.unmounted) return; + + // Store the inviter so that we can know who invited us to this room even if + // the membership event changes. + this.inviter = this.getInviterFromRoom(room); + // Attach a widget store listener only when we get a room this.context.widgetLayoutStore.on(WidgetLayoutStore.emissionForRoom(room), this.onWidgetLayoutChange); @@ -1729,8 +1738,20 @@ export class RoomView extends React.Component { }); }; + private getInviterFromRoom(room: Room): string | undefined { + const ownUserId = this.context.client?.getSafeUserId(); + if (!ownUserId) return; + + const myMember = room.getMember(ownUserId); + const memberEvent = myMember?.events.member; + const senderId = memberEvent?.getSender(); + + if (memberEvent?.getContent().membership === KnownMembership.Invite) return senderId; + } + private onDeclineAndBlockButtonClicked = async (): Promise => { if (!this.state.room || !this.context.client) return; + const [shouldReject, ignoreUser, reportRoom] = await Modal.createDialog(DeclineAndBlockInviteDialog, { roomName: this.state.room.name, }).finished; @@ -1745,11 +1766,20 @@ export class RoomView extends React.Component { const actions: Promise[] = []; if (ignoreUser) { - const myMember = this.state.room.getMember(this.context.client!.getSafeUserId()); - const inviteEvent = myMember!.events.member; - const ignoredUsers = this.context.client.getIgnoredUsers(); - ignoredUsers.push(inviteEvent!.getSender()!); // de-duped internally in the js-sdk - actions.push(this.context.client.setIgnoredUsers(ignoredUsers)); + const doIgnore = async (): Promise => { + const ownUserId = this.context.client!.getSafeUserId(); + if (!this.inviter || this.inviter === ownUserId) { + // This is unlikely to happen since we cache the inviter as early as possible. + // However, we still do this check here to be double sure. + throw new CannotDetermineUserError( + "Cannot determine which user to ignore since the member event has changed.", + ); + } + const ignoredUsers = this.context.client!.getIgnoredUsers(); + ignoredUsers.push(this.inviter); // de-duped internally in the js-sdk + await this.context.client!.setIgnoredUsers(ignoredUsers); + }; + actions.push(doIgnore()); } if (reportRoom !== false) { @@ -1766,7 +1796,14 @@ export class RoomView extends React.Component { } catch (error) { logger.error(`Failed to reject invite: ${error}`); - const msg = error instanceof Error ? error.message : JSON.stringify(error); + let msg: string = ""; + if (error instanceof CannotDetermineUserError) { + msg = _t("room|failed_determine_user"); + } else if (error instanceof Error) { + msg = error.message; + } else { + msg = JSON.stringify(error); + } Modal.createDialog(ErrorDialog, { title: _t("room|failed_reject_invite"), description: msg, @@ -1783,6 +1820,9 @@ export class RoomView extends React.Component { return; } try { + this.setState({ + rejecting: true, + }); await this.context.client.leave(this.state.room.roomId); defaultDispatcher.dispatch({ action: Action.ViewHomePage }); this.setState({ @@ -2612,3 +2652,7 @@ export class RoomView extends React.Component { ); } } + +class CannotDetermineUserError extends Error { + public name = "CannotDetermineUserError"; +} diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 13aa7b9da3..ee5378b11a 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1957,6 +1957,7 @@ }, "face_pile_tooltip_shortcut": "Including %(commaSeparatedMembers)s", "face_pile_tooltip_shortcut_joined": "Including you, %(commaSeparatedMembers)s", + "failed_determine_user": "Cannot determine which user to ignore since the member event has changed.", "failed_reject_invite": "Failed to reject invite", "forget_room": "Forget this room", "forget_space": "Forget this space", diff --git a/test/unit-tests/components/structures/RoomView-test.tsx b/test/unit-tests/components/structures/RoomView-test.tsx index 2d0d7f0d36..387027f30a 100644 --- a/test/unit-tests/components/structures/RoomView-test.tsx +++ b/test/unit-tests/components/structures/RoomView-test.tsx @@ -77,6 +77,7 @@ import { type ViewUserPayload } from "../../../../src/dispatcher/payloads/ViewUs import { CallStore } from "../../../../src/stores/CallStore.ts"; import MediaDeviceHandler, { MediaDeviceKindEnum } from "../../../../src/MediaDeviceHandler.ts"; import Modal from "../../../../src/Modal.tsx"; +import ErrorDialog from "../../../../src/components/views/dialogs/ErrorDialog.tsx"; // Used by group calls jest.spyOn(MediaDeviceHandler, "getDevices").mockResolvedValue({ @@ -237,6 +238,7 @@ describe("RoomView", () => { member.membership = KnownMembership.Invite; member.events.member = new MatrixEvent({ sender: "@bob:example.org", + content: { membership: KnownMembership.Invite }, }); room.getMyMembership = jest.fn().mockReturnValue(KnownMembership.Invite); room.getMember = jest.fn().mockReturnValue(member); @@ -271,10 +273,45 @@ describe("RoomView", () => { finished: Promise.resolve([true, true, false]), close: jest.fn(), }); - await fireEvent.click(getByRole("button", { name: "Decline and block" })); + await act(() => fireEvent.click(getByRole("button", { name: "Decline and block" }))); expect(cli.leave).toHaveBeenCalledWith(room.roomId); expect(cli.setIgnoredUsers).toHaveBeenCalledWith(["@carol:example.org", "@bob:example.org"]); }); + it("prevents ignoring own user", async () => { + const member = new RoomMember(room.roomId, cli.getSafeUserId()); + member.membership = KnownMembership.Invite; + member.events.member = new MatrixEvent({ + /* + It doesn't matter that this is an invite event coming from own user, we just + want to simulate a situation where the sender of the membership event somehow + ends up being own user. + */ + sender: cli.getSafeUserId(), + content: { membership: KnownMembership.Invite }, + }); + jest.spyOn(room, "getMyMembership").mockReturnValue(KnownMembership.Invite); + jest.spyOn(room, "getMember").mockReturnValue(member); + + const { getByRole } = await mountRoomView(); + cli.getIgnoredUsers.mockReturnValue(["@carol:example.org"]); + jest.spyOn(Modal, "createDialog").mockReturnValue({ + finished: Promise.resolve([true, true, false]), + close: jest.fn(), + }); + + await act(() => fireEvent.click(getByRole("button", { name: "Decline and block" }))); + + // Should show error in a modal dialog + await waitFor(() => { + expect(Modal.createDialog).toHaveBeenLastCalledWith(ErrorDialog, { + title: "Failed to reject invite", + description: "Cannot determine which user to ignore since the member event has changed.", + }); + }); + + // The ignore call should not go through + expect(cli.setIgnoredUsers).not.toHaveBeenCalled(); + }); it("handles declining an invite and reporting the room", async () => { const { getByRole } = await mountRoomView(); jest.spyOn(Modal, "createDialog").mockReturnValue({ diff --git a/test/unit-tests/components/structures/__snapshots__/RoomView-test.tsx.snap b/test/unit-tests/components/structures/__snapshots__/RoomView-test.tsx.snap index b99ca68c28..8d909a6797 100644 --- a/test/unit-tests/components/structures/__snapshots__/RoomView-test.tsx.snap +++ b/test/unit-tests/components/structures/__snapshots__/RoomView-test.tsx.snap @@ -1363,7 +1363,7 @@ exports[`RoomView should not display the timeline when the room encryption is lo aria-label="Open room settings" aria-live="off" class="_avatar_1qbcf_8 mx_BaseAvatar _avatar-imageless_1qbcf_52" - data-color="4" + data-color="5" data-testid="avatar-img" data-type="round" role="button" @@ -1390,7 +1390,7 @@ exports[`RoomView should not display the timeline when the room encryption is lo - !11:example.org + !12:example.org @@ -1571,7 +1571,7 @@ exports[`RoomView should not display the timeline when the room encryption is lo aria-label="Open room settings" aria-live="off" class="_avatar_1qbcf_8 mx_BaseAvatar _avatar-imageless_1qbcf_52" - data-color="4" + data-color="5" data-testid="avatar-img" data-type="round" role="button" @@ -1598,7 +1598,7 @@ exports[`RoomView should not display the timeline when the room encryption is lo - !11:example.org + !12:example.org @@ -1952,7 +1952,7 @@ exports[`RoomView video rooms should render joined video room view 1`] = ` aria-label="Open room settings" aria-live="off" class="_avatar_1qbcf_8 mx_BaseAvatar _avatar-imageless_1qbcf_52" - data-color="3" + data-color="4" data-testid="avatar-img" data-type="round" role="button" @@ -1979,7 +1979,7 @@ exports[`RoomView video rooms should render joined video room view 1`] = ` - !16:example.org + !17:example.org