Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Offer to unban user during invite if inviter has sufficient permissions #11256

Merged
merged 4 commits into from
Jul 14, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,8 @@
"Not a valid %(brand)s keyfile": "Not a valid %(brand)s keyfile",
"Authentication check failed: incorrect password?": "Authentication check failed: incorrect password?",
"Unrecognised address": "Unrecognised address",
"Unban": "Unban",
"User cannot be invited until they are unbanned": "User cannot be invited until they are unbanned",
"You do not have permission to invite people to this space.": "You do not have permission to invite people to this space.",
"You do not have permission to invite people to this room.": "You do not have permission to invite people to this room.",
"User is already invited to the space": "User is already invited to the space",
Expand Down Expand Up @@ -1705,7 +1707,6 @@
"Upload custom sound": "Upload custom sound",
"Browse": "Browse",
"Failed to unban": "Failed to unban",
"Unban": "Unban",
"Banned by %(displayName)s": "Banned by %(displayName)s",
"Reason": "Reason",
"Error changing power level requirement": "Error changing power level requirement",
Expand Down
29 changes: 29 additions & 0 deletions src/utils/MultiInviter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { _t } from "../languageHandler";
import Modal from "../Modal";
import SettingsStore from "../settings/SettingsStore";
import AskInviteAnywayDialog from "../components/views/dialogs/AskInviteAnywayDialog";
import ConfirmUserActionDialog from "../components/views/dialogs/ConfirmUserActionDialog";

export enum InviteState {
Invited = "invited",
Expand All @@ -48,6 +49,7 @@ export type CompletionStates = Record<string, InviteState>;

const USER_ALREADY_JOINED = "IO.ELEMENT.ALREADY_JOINED";
const USER_ALREADY_INVITED = "IO.ELEMENT.ALREADY_INVITED";
const USER_BANNED = "IO.ELEMENT.BANNED";

/**
* Invites multiple addresses to a room, handling rate limiting from the server
Expand Down Expand Up @@ -170,6 +172,32 @@ export default class MultiInviter {
errcode: USER_ALREADY_INVITED,
error: "Member already invited",
});
} else if (member?.membership === "ban") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slight concern with checking before attempting the invite is it is vulnerable to Element's state drifting out of sync with the server. We could refuse to invite someone who isn't actually banned, etc. It might have been nicer to prompt to unban if the initial attempt to invite fails.

But I guess we already have this problem with the other checks here, so 🤷‍♂️

let proceed = false;
const ourMember = room.getMember(this.matrixClient.getSafeUserId());
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
if (
!!ourMember &&
member.powerLevel < ourMember.powerLevel &&
room.currentState.hasSufficientPowerLevelFor("ban", ourMember.powerLevel) &&
room.currentState.hasSufficientPowerLevelFor("kick", ourMember.powerLevel)
) {
Comment on lines +180 to +185
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might have been nice to pull this out to a canUnbanUser(ourMember, member) function, but whatever

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally it'd have been a util in the js-sdk but 🤷

const { finished } = Modal.createDialog(ConfirmUserActionDialog, {
member,
action: _t("Unban"),
title: _t("User cannot be invited until they are unbanned"),
});
[proceed = false] = await finished;
if (proceed) {
await this.matrixClient.unban(roomId, member.userId);
}
}

if (!proceed) {
throw new MatrixError({
errcode: USER_BANNED,
error: "Member is banned",
});
}
}

if (!ignoreProfile && SettingsStore.getValue("promptBeforeInviteUnknownUsers", this.roomId)) {
Expand Down Expand Up @@ -268,6 +296,7 @@ export default class MultiInviter {
}
break;
case "M_BAD_STATE":
case USER_BANNED:
errorText = _t("The user must be unbanned before they can be invited.");
break;
case "M_UNSUPPORTED_ROOM_VERSION":
Expand Down
35 changes: 34 additions & 1 deletion test/utils/MultiInviter-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ limitations under the License.
*/

import { mocked } from "jest-mock";
import { MatrixClient, MatrixError } from "matrix-js-sdk/src/matrix";
import { MatrixClient, MatrixError, Room, RoomMember } from "matrix-js-sdk/src/matrix";

import { MatrixClientPeg } from "../../src/MatrixClientPeg";
import Modal, { ComponentType, ComponentProps } from "../../src/Modal";
import SettingsStore from "../../src/settings/SettingsStore";
import MultiInviter, { CompletionStates } from "../../src/utils/MultiInviter";
import * as TestUtilsMatrix from "../test-utils";
import AskInviteAnywayDialog from "../../src/components/views/dialogs/AskInviteAnywayDialog";
import ConfirmUserActionDialog from "../../src/components/views/dialogs/ConfirmUserActionDialog";

const ROOMID = "!room:server";

Expand Down Expand Up @@ -89,6 +90,7 @@ describe("MultiInviter", () => {
client.getProfileInfo.mockImplementation((userId: string) => {
return MXID_PROFILE_STATES[userId] || Promise.reject();
});
client.unban = jest.fn();

inviter = new MultiInviter(client, ROOMID);
});
Expand Down Expand Up @@ -154,5 +156,36 @@ describe("MultiInviter", () => {
`"Cannot invite user by email without an identity server. You can connect to one under "Settings"."`,
);
});

it("should ask if user wants to unban user if they have permission", async () => {
mocked(Modal.createDialog).mockImplementation(
(Element: ComponentType, props?: ComponentProps<ComponentType>): any => {
// We stub out the modal with an immediate affirmative (proceed) return
return { finished: Promise.resolve([true]) };
},
);

const room = new Room(ROOMID, client, client.getSafeUserId());
mocked(client.getRoom).mockReturnValue(room);
const ourMember = new RoomMember(ROOMID, client.getSafeUserId());
ourMember.membership = "join";
ourMember.powerLevel = 100;
const member = new RoomMember(ROOMID, MXID1);
member.membership = "ban";
member.powerLevel = 0;
room.getMember = (userId: string) => {
if (userId === client.getSafeUserId()) return ourMember;
if (userId === MXID1) return member;
return null;
};

await inviter.invite([MXID1]);
expect(Modal.createDialog).toHaveBeenCalledWith(ConfirmUserActionDialog, {
member,
title: "User cannot be invited until they are unbanned",
action: "Unban",
});
expect(client.unban).toHaveBeenCalledWith(ROOMID, MXID1);
});
});
});
Loading