diff --git a/apps/meteor/app/invites/server/functions/useInviteToken.ts b/apps/meteor/app/invites/server/functions/useInviteToken.ts index f514736da59..10ccd147d5b 100644 --- a/apps/meteor/app/invites/server/functions/useInviteToken.ts +++ b/apps/meteor/app/invites/server/functions/useInviteToken.ts @@ -1,3 +1,4 @@ +import { isBannedSubscription } from '@rocket.chat/core-typings'; import { Invites, Subscriptions, Users } from '@rocket.chat/models'; import { Meteor } from 'meteor/meteor'; @@ -37,11 +38,15 @@ export const useInviteToken = async (userId: string, token: string) => { field: 'userId', }); } + const subscription = await Subscriptions.findOneByRoomIdAndUserId(room._id, user._id); + if (subscription && isBannedSubscription(subscription)) { + throw new Meteor.Error('error-user-is-banned', 'User is banned from this room', { + method: 'useInviteToken', + }); + } + await Users.updateInviteToken(user._id, token); - const subscription = await Subscriptions.findOneByRoomIdAndUserId(room._id, user._id, { - projection: { _id: 1 }, - }); if (!subscription) { await Invites.increaseUsageById(inviteData._id, 1); } diff --git a/apps/meteor/tests/end-to-end/api/invites.ts b/apps/meteor/tests/end-to-end/api/invites.ts index 85f0771f67d..9ae8163134e 100644 --- a/apps/meteor/tests/end-to-end/api/invites.ts +++ b/apps/meteor/tests/end-to-end/api/invites.ts @@ -1,8 +1,13 @@ -import type { IInvite } from '@rocket.chat/core-typings'; +import type { Credentials } from '@rocket.chat/api-client'; +import type { IInvite, IRoom, IUser } from '@rocket.chat/core-typings'; import { expect } from 'chai'; -import { before, describe, it } from 'mocha'; +import { after, before, describe, it } from 'mocha'; import { getCredentials, api, request, credentials } from '../../data/api-data'; +import { createRoom, deleteRoom } from '../../data/rooms.helper'; +import { password } from '../../data/user'; +import type { TestUser } from '../../data/users.helper'; +import { createUser, deleteUser, login } from '../../data/users.helper'; describe('Invites', () => { let testInviteID: IInvite['_id']; @@ -195,6 +200,64 @@ describe('Invites', () => { }); }); + describe('POST [/useInviteToken] - banned user', () => { + let room: IRoom; + let bannedUser: TestUser; + let bannedUserCredentials: Credentials; + let inviteId: IInvite['_id']; + + before(async () => { + bannedUser = await createUser(); + bannedUserCredentials = await login(bannedUser.username, password); + + const result = await createRoom({ type: 'p', name: `invite-ban-test-${Date.now()}` }); + room = result.body.group; + + // Add user then ban them + await request.post(api('groups.invite')).set(credentials).send({ roomId: room._id, userId: bannedUser._id }).expect(200); + await request.post(api('rooms.banUser')).set(credentials).send({ roomId: room._id, userId: bannedUser._id }).expect(200); + + // Create invite link for the room + const invite = await request + .post(api('findOrCreateInvite')) + .set(credentials) + .send({ rid: room._id, days: 1, maxUses: 10 }) + .expect(200); + inviteId = invite.body._id; + }); + + after(async () => { + await deleteRoom({ type: 'p', roomId: room._id }); + await deleteUser(bannedUser); + }); + + it('should fail if user is banned from the room', async () => { + await request + .post(api('useInviteToken')) + .set(bannedUserCredentials) + .send({ token: inviteId }) + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('errorType', 'error-user-is-banned'); + }); + }); + + it('should succeed after the user is unbanned', async () => { + await request.post(api('rooms.unbanUser')).set(credentials).send({ roomId: room._id, userId: bannedUser._id }).expect(200); + + await request + .post(api('useInviteToken')) + .set(bannedUserCredentials) + .send({ token: inviteId }) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('room').and.to.have.property('rid', room._id); + }); + }); + }); + describe('DELETE [/removeInvite]', () => { it('should fail if not logged in', (done) => { void request diff --git a/apps/meteor/tests/end-to-end/api/rooms.ts b/apps/meteor/tests/end-to-end/api/rooms.ts index 0b8ca498a6a..96d93855690 100644 --- a/apps/meteor/tests/end-to-end/api/rooms.ts +++ b/apps/meteor/tests/end-to-end/api/rooms.ts @@ -4628,130 +4628,4 @@ describe('[Rooms]', () => { }); }); }); - - describe('Re-inviting a banned user should preserve other subscriptions', () => { - let privateChannel: IRoom; - let otherChannel1: IRoom; - let otherChannel2: IRoom; - let userB: TestUser; - let userBCredentials: Credentials; - - before(async () => { - userB = await createUser(); - userBCredentials = await login(userB.username, password); - - // Create the private channel (owned by admin / User A) - const result = await createRoom({ type: 'p', name: `ban-reinvite-test-${Date.now()}` }); - privateChannel = result.body.group; - - // Create two additional channels that User B will be a member of - const ch1 = await createRoom({ type: 'c', name: `other-ch1-${Date.now()}` }); - otherChannel1 = ch1.body.channel; - - const ch2 = await createRoom({ type: 'c', name: `other-ch2-${Date.now()}` }); - otherChannel2 = ch2.body.channel; - - // Add User B to all three channels - await request.post(api('groups.invite')).set(credentials).send({ roomId: privateChannel._id, userId: userB._id }).expect(200); - await request.post(api('channels.invite')).set(credentials).send({ roomId: otherChannel1._id, userId: userB._id }).expect(200); - await request.post(api('channels.invite')).set(credentials).send({ roomId: otherChannel2._id, userId: userB._id }).expect(200); - }); - - after(async () => { - await deleteRoom({ type: 'p', roomId: privateChannel._id }); - await deleteRoom({ type: 'c', roomId: otherChannel1._id }); - await deleteRoom({ type: 'c', roomId: otherChannel2._id }); - await deleteUser(userB); - }); - - it('should confirm User B is a member of all three channels', async () => { - const res = await request.get(api('subscriptions.get')).set(userBCredentials).expect(200); - expect(res.body).to.have.property('success', true); - - const roomIds = res.body.update.map((sub: { rid: string }) => sub.rid); - expect(roomIds).to.include(privateChannel._id); - expect(roomIds).to.include(otherChannel1._id); - expect(roomIds).to.include(otherChannel2._id); - }); - - it('should ban User B from the private channel', async () => { - await request - .post(api('rooms.banUser')) - .set(credentials) - .send({ - roomId: privateChannel._id, - userId: userB._id, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - }); - }); - - it('should still have User B subscribed to the other channels after being banned', async () => { - const res = await request.get(api('subscriptions.get')).set(userBCredentials).expect(200); - expect(res.body).to.have.property('success', true); - - const roomIds = res.body.update.map((sub: { rid: string }) => sub.rid); - expect(roomIds).to.include(otherChannel1._id); - expect(roomIds).to.include(otherChannel2._id); - expect(roomIds).to.not.include(privateChannel._id); - }); - - it('should re-invite banned User B back to the private channel', async () => { - await request - .post(api('groups.invite')) - .set(credentials) - .send({ - roomId: privateChannel._id, - userId: userB._id, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - }); - }); - - it('should list User B as a member of the private channel again', async () => { - const res = await request - .get(api('groups.members')) - .set(credentials) - .query({ - roomId: privateChannel._id, - }) - .expect('Content-Type', 'application/json') - .expect(200); - - expect(res.body).to.have.property('success', true); - const usernames = res.body.members.map((m: IUser) => m.username); - expect(usernames).to.include(userB.username); - }); - - it('should no longer list User B as banned', async () => { - const res = await request - .get(api('rooms.bannedUsers')) - .set(credentials) - .query({ - roomId: privateChannel._id, - }) - .expect('Content-Type', 'application/json') - .expect(200); - - expect(res.body).to.have.property('success', true); - const userIds = res.body.bannedUsers.map((u: IUser) => u._id); - expect(userIds).to.not.include(userB._id); - }); - - it('should preserve all other channel subscriptions after re-invite', async () => { - const res = await request.get(api('subscriptions.get')).set(userBCredentials).expect(200); - expect(res.body).to.have.property('success', true); - - const roomIds = res.body.update.map((sub: { rid: string }) => sub.rid); - expect(roomIds).to.include(privateChannel._id, 'User B should be re-subscribed to the private channel'); - expect(roomIds).to.include(otherChannel1._id, 'User B should still be subscribed to otherChannel1'); - expect(roomIds).to.include(otherChannel2._id, 'User B should still be subscribed to otherChannel2'); - }); - }); }); diff --git a/docs/features/ban-user.md b/docs/features/ban-user.md index 10e5037b01f..1515b489dd6 100644 --- a/docs/features/ban-user.md +++ b/docs/features/ban-user.md @@ -34,14 +34,39 @@ Banning prevents a user from participating in a specific room. Unlike kicking (w **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. -## Re-entry After Unban +## Join / Invite / Re-entry Behavior -In `addUserToRoom`, if the user being added has a subscription with `status: 'BANNED'`: -- Removes the banned subscription. -- Saves a `user-unbanned` system message. -- Creates a new subscription normally. +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. -This means inviting/adding a banned user automatically unbans them. +### Invite via API / UI (`groups.invite`, `channels.invite`, "Add Users") + +`addUsersToRoom` checks for a `BANNED` subscription before calling `addUserToRoom`: +- Returns `error-user-is-banned` — the invite is rejected. +- The UI shows a warning modal asking the admin to unban first. +- Applies equally to normal and federated rooms (the check is in the method layer, before the room-type branch). + +### Invite link (`useInviteToken`) + +`useInviteToken` checks for a `BANNED` subscription before saving the invite token or calling `addUserToRoom`: +- Returns `error-user-is-banned` — the token is not consumed. +- Because the check runs before `Users.updateInviteToken`, the secondary path through `setUsername` (for users who register via invite link) is also blocked. + +### Direct join (`channels.join`, `joinRoom`) + +`Room.join` calls `canAccessRoom` before `addUserToRoom`: +- For **public rooms** and **public rooms inside teams**, the `canAccessRoom` validators explicitly check `findOneBannedSubscription` and deny access. +- For **private rooms**, `countByRoomIdAndUserId` excludes `BANNED` subscriptions (`status: { $exists: false }`), so the "already joined" validator returns false and access is denied. + +### Federation invite events + +When a Matrix homeserver sends an invite for a user who is banned locally: +- `handleInvite` in `federation-matrix/src/events/member.ts` finds the existing (banned) subscription and returns early without creating a new one. +- The user never receives an `INVITED` subscription, so `handleJoin` is never reached. + +### Expected flow + +1. **Unban** the user via `POST /v1/rooms.unbanUser`, `/unban @username`, or the "Banned Users" contextual bar. This deletes the banned subscription. +2. **Invite or join** — the user can now be invited (API, UI, invite link) or join (public rooms) normally. ## Access Control @@ -49,7 +74,7 @@ The `canAccessRoom` validators check for bans in two public room scenarios: - **Public rooms inside teams** — if banned, access is denied. - **Regular public rooms** — if banned, access is denied. -For private rooms, access is already controlled by the subscription (which is marked as `BANNED`). +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. ## UI