Skip to content

Commit f21ed3a

Browse files
authored
fix: require unban before using inviting link (RocketChat#40087)
1 parent 3340757 commit f21ed3a

File tree

4 files changed

+105
-138
lines changed

4 files changed

+105
-138
lines changed

apps/meteor/app/invites/server/functions/useInviteToken.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { isBannedSubscription } from '@rocket.chat/core-typings';
12
import { Invites, Subscriptions, Users } from '@rocket.chat/models';
23
import { Meteor } from 'meteor/meteor';
34

@@ -37,11 +38,15 @@ export const useInviteToken = async (userId: string, token: string) => {
3738
field: 'userId',
3839
});
3940
}
41+
const subscription = await Subscriptions.findOneByRoomIdAndUserId(room._id, user._id);
42+
if (subscription && isBannedSubscription(subscription)) {
43+
throw new Meteor.Error('error-user-is-banned', 'User is banned from this room', {
44+
method: 'useInviteToken',
45+
});
46+
}
47+
4048
await Users.updateInviteToken(user._id, token);
4149

42-
const subscription = await Subscriptions.findOneByRoomIdAndUserId(room._id, user._id, {
43-
projection: { _id: 1 },
44-
});
4550
if (!subscription) {
4651
await Invites.increaseUsageById(inviteData._id, 1);
4752
}

apps/meteor/tests/end-to-end/api/invites.ts

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
1-
import type { IInvite } from '@rocket.chat/core-typings';
1+
import type { Credentials } from '@rocket.chat/api-client';
2+
import type { IInvite, IRoom, IUser } from '@rocket.chat/core-typings';
23
import { expect } from 'chai';
3-
import { before, describe, it } from 'mocha';
4+
import { after, before, describe, it } from 'mocha';
45

56
import { getCredentials, api, request, credentials } from '../../data/api-data';
7+
import { createRoom, deleteRoom } from '../../data/rooms.helper';
8+
import { password } from '../../data/user';
9+
import type { TestUser } from '../../data/users.helper';
10+
import { createUser, deleteUser, login } from '../../data/users.helper';
611

712
describe('Invites', () => {
813
let testInviteID: IInvite['_id'];
@@ -195,6 +200,64 @@ describe('Invites', () => {
195200
});
196201
});
197202

203+
describe('POST [/useInviteToken] - banned user', () => {
204+
let room: IRoom;
205+
let bannedUser: TestUser<IUser>;
206+
let bannedUserCredentials: Credentials;
207+
let inviteId: IInvite['_id'];
208+
209+
before(async () => {
210+
bannedUser = await createUser();
211+
bannedUserCredentials = await login(bannedUser.username, password);
212+
213+
const result = await createRoom({ type: 'p', name: `invite-ban-test-${Date.now()}` });
214+
room = result.body.group;
215+
216+
// Add user then ban them
217+
await request.post(api('groups.invite')).set(credentials).send({ roomId: room._id, userId: bannedUser._id }).expect(200);
218+
await request.post(api('rooms.banUser')).set(credentials).send({ roomId: room._id, userId: bannedUser._id }).expect(200);
219+
220+
// Create invite link for the room
221+
const invite = await request
222+
.post(api('findOrCreateInvite'))
223+
.set(credentials)
224+
.send({ rid: room._id, days: 1, maxUses: 10 })
225+
.expect(200);
226+
inviteId = invite.body._id;
227+
});
228+
229+
after(async () => {
230+
await deleteRoom({ type: 'p', roomId: room._id });
231+
await deleteUser(bannedUser);
232+
});
233+
234+
it('should fail if user is banned from the room', async () => {
235+
await request
236+
.post(api('useInviteToken'))
237+
.set(bannedUserCredentials)
238+
.send({ token: inviteId })
239+
.expect(400)
240+
.expect((res) => {
241+
expect(res.body).to.have.property('success', false);
242+
expect(res.body).to.have.property('errorType', 'error-user-is-banned');
243+
});
244+
});
245+
246+
it('should succeed after the user is unbanned', async () => {
247+
await request.post(api('rooms.unbanUser')).set(credentials).send({ roomId: room._id, userId: bannedUser._id }).expect(200);
248+
249+
await request
250+
.post(api('useInviteToken'))
251+
.set(bannedUserCredentials)
252+
.send({ token: inviteId })
253+
.expect(200)
254+
.expect((res) => {
255+
expect(res.body).to.have.property('success', true);
256+
expect(res.body).to.have.property('room').and.to.have.property('rid', room._id);
257+
});
258+
});
259+
});
260+
198261
describe('DELETE [/removeInvite]', () => {
199262
it('should fail if not logged in', (done) => {
200263
void request

apps/meteor/tests/end-to-end/api/rooms.ts

Lines changed: 0 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -4628,130 +4628,4 @@ describe('[Rooms]', () => {
46284628
});
46294629
});
46304630
});
4631-
4632-
describe('Re-inviting a banned user should preserve other subscriptions', () => {
4633-
let privateChannel: IRoom;
4634-
let otherChannel1: IRoom;
4635-
let otherChannel2: IRoom;
4636-
let userB: TestUser<IUser>;
4637-
let userBCredentials: Credentials;
4638-
4639-
before(async () => {
4640-
userB = await createUser();
4641-
userBCredentials = await login(userB.username, password);
4642-
4643-
// Create the private channel (owned by admin / User A)
4644-
const result = await createRoom({ type: 'p', name: `ban-reinvite-test-${Date.now()}` });
4645-
privateChannel = result.body.group;
4646-
4647-
// Create two additional channels that User B will be a member of
4648-
const ch1 = await createRoom({ type: 'c', name: `other-ch1-${Date.now()}` });
4649-
otherChannel1 = ch1.body.channel;
4650-
4651-
const ch2 = await createRoom({ type: 'c', name: `other-ch2-${Date.now()}` });
4652-
otherChannel2 = ch2.body.channel;
4653-
4654-
// Add User B to all three channels
4655-
await request.post(api('groups.invite')).set(credentials).send({ roomId: privateChannel._id, userId: userB._id }).expect(200);
4656-
await request.post(api('channels.invite')).set(credentials).send({ roomId: otherChannel1._id, userId: userB._id }).expect(200);
4657-
await request.post(api('channels.invite')).set(credentials).send({ roomId: otherChannel2._id, userId: userB._id }).expect(200);
4658-
});
4659-
4660-
after(async () => {
4661-
await deleteRoom({ type: 'p', roomId: privateChannel._id });
4662-
await deleteRoom({ type: 'c', roomId: otherChannel1._id });
4663-
await deleteRoom({ type: 'c', roomId: otherChannel2._id });
4664-
await deleteUser(userB);
4665-
});
4666-
4667-
it('should confirm User B is a member of all three channels', async () => {
4668-
const res = await request.get(api('subscriptions.get')).set(userBCredentials).expect(200);
4669-
expect(res.body).to.have.property('success', true);
4670-
4671-
const roomIds = res.body.update.map((sub: { rid: string }) => sub.rid);
4672-
expect(roomIds).to.include(privateChannel._id);
4673-
expect(roomIds).to.include(otherChannel1._id);
4674-
expect(roomIds).to.include(otherChannel2._id);
4675-
});
4676-
4677-
it('should ban User B from the private channel', async () => {
4678-
await request
4679-
.post(api('rooms.banUser'))
4680-
.set(credentials)
4681-
.send({
4682-
roomId: privateChannel._id,
4683-
userId: userB._id,
4684-
})
4685-
.expect('Content-Type', 'application/json')
4686-
.expect(200)
4687-
.expect((res) => {
4688-
expect(res.body).to.have.property('success', true);
4689-
});
4690-
});
4691-
4692-
it('should still have User B subscribed to the other channels after being banned', async () => {
4693-
const res = await request.get(api('subscriptions.get')).set(userBCredentials).expect(200);
4694-
expect(res.body).to.have.property('success', true);
4695-
4696-
const roomIds = res.body.update.map((sub: { rid: string }) => sub.rid);
4697-
expect(roomIds).to.include(otherChannel1._id);
4698-
expect(roomIds).to.include(otherChannel2._id);
4699-
expect(roomIds).to.not.include(privateChannel._id);
4700-
});
4701-
4702-
it('should re-invite banned User B back to the private channel', async () => {
4703-
await request
4704-
.post(api('groups.invite'))
4705-
.set(credentials)
4706-
.send({
4707-
roomId: privateChannel._id,
4708-
userId: userB._id,
4709-
})
4710-
.expect('Content-Type', 'application/json')
4711-
.expect(200)
4712-
.expect((res) => {
4713-
expect(res.body).to.have.property('success', true);
4714-
});
4715-
});
4716-
4717-
it('should list User B as a member of the private channel again', async () => {
4718-
const res = await request
4719-
.get(api('groups.members'))
4720-
.set(credentials)
4721-
.query({
4722-
roomId: privateChannel._id,
4723-
})
4724-
.expect('Content-Type', 'application/json')
4725-
.expect(200);
4726-
4727-
expect(res.body).to.have.property('success', true);
4728-
const usernames = res.body.members.map((m: IUser) => m.username);
4729-
expect(usernames).to.include(userB.username);
4730-
});
4731-
4732-
it('should no longer list User B as banned', async () => {
4733-
const res = await request
4734-
.get(api('rooms.bannedUsers'))
4735-
.set(credentials)
4736-
.query({
4737-
roomId: privateChannel._id,
4738-
})
4739-
.expect('Content-Type', 'application/json')
4740-
.expect(200);
4741-
4742-
expect(res.body).to.have.property('success', true);
4743-
const userIds = res.body.bannedUsers.map((u: IUser) => u._id);
4744-
expect(userIds).to.not.include(userB._id);
4745-
});
4746-
4747-
it('should preserve all other channel subscriptions after re-invite', async () => {
4748-
const res = await request.get(api('subscriptions.get')).set(userBCredentials).expect(200);
4749-
expect(res.body).to.have.property('success', true);
4750-
4751-
const roomIds = res.body.update.map((sub: { rid: string }) => sub.rid);
4752-
expect(roomIds).to.include(privateChannel._id, 'User B should be re-subscribed to the private channel');
4753-
expect(roomIds).to.include(otherChannel1._id, 'User B should still be subscribed to otherChannel1');
4754-
expect(roomIds).to.include(otherChannel2._id, 'User B should still be subscribed to otherChannel2');
4755-
});
4756-
});
47574631
});

docs/features/ban-user.md

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,22 +34,47 @@ Banning prevents a user from participating in a specific room. Unlike kicking (w
3434

3535
**Important:** after unban the user **does not become a member** of the room again. The banned subscription is deleted. The user must be invited or join again.
3636

37-
## Re-entry After Unban
37+
## Join / Invite / Re-entry Behavior
3838

39-
In `addUserToRoom`, if the user being added has a subscription with `status: 'BANNED'`:
40-
- Removes the banned subscription.
41-
- Saves a `user-unbanned` system message.
42-
- Creates a new subscription normally.
39+
A banned user **cannot** re-enter the room through any path. The ban must be explicitly lifted first. Below is how each entry point enforces this for both normal and federated rooms.
4340

44-
This means inviting/adding a banned user automatically unbans them.
41+
### Invite via API / UI (`groups.invite`, `channels.invite`, "Add Users")
42+
43+
`addUsersToRoom` checks for a `BANNED` subscription before calling `addUserToRoom`:
44+
- Returns `error-user-is-banned` — the invite is rejected.
45+
- The UI shows a warning modal asking the admin to unban first.
46+
- Applies equally to normal and federated rooms (the check is in the method layer, before the room-type branch).
47+
48+
### Invite link (`useInviteToken`)
49+
50+
`useInviteToken` checks for a `BANNED` subscription before saving the invite token or calling `addUserToRoom`:
51+
- Returns `error-user-is-banned` — the token is not consumed.
52+
- Because the check runs before `Users.updateInviteToken`, the secondary path through `setUsername` (for users who register via invite link) is also blocked.
53+
54+
### Direct join (`channels.join`, `joinRoom`)
55+
56+
`Room.join` calls `canAccessRoom` before `addUserToRoom`:
57+
- For **public rooms** and **public rooms inside teams**, the `canAccessRoom` validators explicitly check `findOneBannedSubscription` and deny access.
58+
- For **private rooms**, `countByRoomIdAndUserId` excludes `BANNED` subscriptions (`status: { $exists: false }`), so the "already joined" validator returns false and access is denied.
59+
60+
### Federation invite events
61+
62+
When a Matrix homeserver sends an invite for a user who is banned locally:
63+
- `handleInvite` in `federation-matrix/src/events/member.ts` finds the existing (banned) subscription and returns early without creating a new one.
64+
- The user never receives an `INVITED` subscription, so `handleJoin` is never reached.
65+
66+
### Expected flow
67+
68+
1. **Unban** the user via `POST /v1/rooms.unbanUser`, `/unban @username`, or the "Banned Users" contextual bar. This deletes the banned subscription.
69+
2. **Invite or join** — the user can now be invited (API, UI, invite link) or join (public rooms) normally.
4570

4671
## Access Control
4772

4873
The `canAccessRoom` validators check for bans in two public room scenarios:
4974
- **Public rooms inside teams** — if banned, access is denied.
5075
- **Regular public rooms** — if banned, access is denied.
5176

52-
For private rooms, access is already controlled by the subscription (which is marked as `BANNED`).
77+
For private rooms, access is controlled by the subscription: `countByRoomIdAndUserId` excludes `BANNED` subscriptions, so a banned user has no valid subscription and cannot access the room.
5378

5479
## UI
5580

0 commit comments

Comments
 (0)