Skip to content

Commit a1c75ff

Browse files
aleksandernsilvaaleksandernsilva
committed
fix: Kick event not being propagated to federated servers
1 parent a286ad8 commit a1c75ff

File tree

3 files changed

+109
-77
lines changed

3 files changed

+109
-77
lines changed

‎packages/federation-sdk/src/repositories/event.repository.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ export class EventRepository {
154154
'nextEventId': '',
155155
'event.room_id': roomId,
156156
'rejectCode': { $exists: false },
157+
'outlier': { $ne: true },
157158
},
158159
{ sort: { 'event.depth': 1, 'createdAt': 1 } },
159160
)

‎packages/federation-sdk/src/services/room.service.spec.ts‎

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ describe('RoomService', async () => {
223223
});
224224

225225
describe('kickUser', () => {
226-
it('should ban user to room correctly', async () => {
226+
it('should kick user from room correctly', async () => {
227227
const username = '@alice:example.com' as UserID;
228228
const secondaryUsername = '@bob:example.com' as UserID;
229229
const { roomCreateEvent } = await createRoom(username, 'public');
@@ -242,10 +242,33 @@ describe('RoomService', async () => {
242242

243243
expect(secondaryState?.getContent().membership).toBe('leave');
244244
});
245+
246+
it('should reject kick when sender has insufficient power level', async () => {
247+
const admin = '@alice:example.com' as UserID;
248+
const regularUser = '@bob:example.com' as UserID;
249+
const target = '@charlie:example.com' as UserID;
250+
const { roomCreateEvent } = await createRoom(admin, 'public');
251+
const { roomId } = roomCreateEvent;
252+
253+
await roomService.joinUser(roomId, regularUser);
254+
255+
await expect(federationSDK.kickUser(roomId, target, regularUser)).rejects.toThrow();
256+
});
257+
258+
it('should reject kick when sender tries to kick a user with equal or higher power level', async () => {
259+
const admin = '@alice:example.com' as UserID;
260+
const coAdmin = '@bob:example.com' as UserID;
261+
const { roomCreateEvent } = await createRoom(admin, 'public', {
262+
users: { [coAdmin]: 100 },
263+
});
264+
const { roomId } = roomCreateEvent;
265+
266+
await expect(federationSDK.kickUser(roomId, coAdmin, admin)).rejects.toThrow();
267+
});
245268
});
246269

247270
describe('banUser', () => {
248-
it('should ban user to room correctly', async () => {
271+
it('should ban user from room correctly', async () => {
249272
const username = '@alice:example.com' as UserID;
250273
const secondaryUsername = '@bob:example.com' as UserID;
251274
const { roomCreateEvent } = await createRoom(username, 'public');
@@ -263,6 +286,29 @@ describe('RoomService', async () => {
263286

264287
expect(secondaryState?.getContent().membership).toBe('ban');
265288
});
289+
290+
it('should reject ban when sender has insufficient power level', async () => {
291+
const admin = '@alice:example.com' as UserID;
292+
const regularUser = '@bob:example.com' as UserID;
293+
const target = '@charlie:example.com' as UserID;
294+
const { roomCreateEvent } = await createRoom(admin, 'public');
295+
const { roomId } = roomCreateEvent;
296+
297+
await roomService.joinUser(roomId, regularUser);
298+
299+
await expect(federationSDK.banUser(roomId, target, regularUser)).rejects.toThrow();
300+
});
301+
302+
it('should reject ban when sender tries to ban a user with equal or higher power level', async () => {
303+
const admin = '@alice:example.com' as UserID;
304+
const coAdmin = '@bob:example.com' as UserID;
305+
const { roomCreateEvent } = await createRoom(admin, 'public', {
306+
users: { [coAdmin]: 100 },
307+
});
308+
const { roomId } = roomCreateEvent;
309+
310+
await expect(federationSDK.banUser(roomId, coAdmin, admin)).rejects.toThrow();
311+
});
266312
});
267313

268314
describe('updateUserPowerLevel', () => {
@@ -294,6 +340,29 @@ describe('RoomService', async () => {
294340
[secondaryUsername]: 50,
295341
});
296342
});
343+
344+
it('should reject power level update when sender has insufficient power level', async () => {
345+
const admin = '@alice:example.com' as UserID;
346+
const regularUser = '@bob:example.com' as UserID;
347+
const target = '@charlie:example.com' as UserID;
348+
const { roomCreateEvent } = await createRoom(admin, 'public');
349+
const { roomId } = roomCreateEvent;
350+
351+
await roomService.joinUser(roomId, regularUser);
352+
353+
await expect(federationSDK.updateUserPowerLevel(roomId, target, 50, regularUser)).rejects.toThrow();
354+
});
355+
356+
it('should reject power level update when sender tries to grant more power than they have', async () => {
357+
const admin = '@alice:example.com' as UserID;
358+
const secondaryUsername = '@bob:example.com' as UserID;
359+
const { roomCreateEvent } = await createRoom(admin, 'public');
360+
const { roomId } = roomCreateEvent;
361+
362+
await roomService.joinUser(roomId, secondaryUsername);
363+
// admin (100) cannot grant power level above their own (e.g. 200)
364+
await expect(federationSDK.updateUserPowerLevel(roomId, secondaryUsername, 200, admin)).rejects.toThrow();
365+
});
297366
});
298367

299368
describe('acceptInvite', () => {

‎packages/federation-sdk/src/services/room.service.ts‎

Lines changed: 37 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import {
22
EventBase,
33
EventStore,
4-
RoomPowerLevelsEvent,
54
SignedEvent,
65
TombstoneAuthEvents,
76
createLogger,
@@ -23,18 +22,18 @@ import {
2322
RoomVersion,
2423
UserID,
2524
extractDomainFromId,
25+
StateResolverAuthorizationError,
2626
} from '@rocket.chat/federation-room';
2727
import { delay, inject, singleton } from 'tsyringe';
2828

2929
import { ConfigService } from './config.service';
30-
import { EventAuthorizationService } from './event-authorization.service';
3130
import { EventEmitterService } from './event-emitter.service';
3231
import { EventFetcherService } from './event-fetcher.service';
3332
import { EventService } from './event.service';
3433
import { FederationValidationService } from './federation-validation.service';
3534
import { FederationService } from './federation.service';
3635
import { InviteService } from './invite.service';
37-
import { RoomInfoNotReadyError, StateService, UnknownRoomError } from './state.service';
36+
import { StateService, UnknownRoomError } from './state.service';
3837
import { EventStagingRepository } from '../repositories/event-staging.repository';
3938
import { EventRepository } from '../repositories/event.repository';
4039
import { RoomRepository } from '../repositories/room.repository';
@@ -105,42 +104,6 @@ export class RoomService {
105104
}
106105
}
107106

108-
private validateKickPermission(currentPowerLevelsContent: RoomPowerLevelsEvent['content'], senderId: string, kickedUserId: string): void {
109-
const senderPower = currentPowerLevelsContent.users?.[senderId] ?? currentPowerLevelsContent.users_default ?? 0;
110-
const kickedUserPower = currentPowerLevelsContent.users?.[kickedUserId] ?? currentPowerLevelsContent.users_default ?? 0;
111-
const kickLevel = currentPowerLevelsContent.kick ?? 50; // Default kick level if not specified
112-
113-
if (senderPower < kickLevel) {
114-
logger.warn(`Sender ${senderId} (power ${senderPower}) does not meet required power level (${kickLevel}) to kick users.`);
115-
throw new HttpException("You don't have permission to kick users from this room.", HttpStatus.FORBIDDEN);
116-
}
117-
118-
if (kickedUserPower >= senderPower) {
119-
logger.warn(
120-
`Sender ${senderId} (power ${senderPower}) cannot kick user ${kickedUserId} (power ${kickedUserPower}) who has equal or greater power.`,
121-
);
122-
throw new HttpException('You cannot kick a user with power greater than or equal to your own.', HttpStatus.FORBIDDEN);
123-
}
124-
}
125-
126-
private validateBanPermission(currentPowerLevelsContent: RoomPowerLevelsEvent['content'], senderId: string, bannedUserId: string): void {
127-
const senderPower = currentPowerLevelsContent.users?.[senderId] ?? currentPowerLevelsContent.users_default ?? 0;
128-
const bannedUserPower = currentPowerLevelsContent.users?.[bannedUserId] ?? currentPowerLevelsContent.users_default ?? 0;
129-
const banLevel = currentPowerLevelsContent.ban ?? 50; // Default ban level if not specified
130-
131-
if (senderPower < banLevel) {
132-
logger.warn(`Sender ${senderId} (power ${senderPower}) does not meet required power level (${banLevel}) to ban users.`);
133-
throw new HttpException("You don't have permission to ban users from this room.", HttpStatus.FORBIDDEN);
134-
}
135-
136-
if (bannedUserPower >= senderPower) {
137-
logger.warn(
138-
`Sender ${senderId} (power ${senderPower}) cannot ban user ${bannedUserId} (power ${bannedUserPower}) who has equal or greater power.`,
139-
);
140-
throw new HttpException('You cannot ban a user with power greater than or equal to your own.', HttpStatus.FORBIDDEN);
141-
}
142-
}
143-
144107
async upsertRoom(roomId: string, state: EventBase[]) {
145108
logger.info(`Upserting room ${roomId} with ${state.length} state events`);
146109

@@ -447,7 +410,14 @@ export class RoomService {
447410
PersistentEventFactory.defaultRoomVersion,
448411
);
449412

450-
await this.stateService.handlePdu(event);
413+
try {
414+
await this.stateService.handlePdu(event);
415+
} catch (error) {
416+
if (error instanceof StateResolverAuthorizationError) {
417+
throw new HttpException(error.reason, HttpStatus.FORBIDDEN);
418+
}
419+
throw error;
420+
}
451421

452422
logger.info(`Successfully created and stored m.room.power_levels event ${event.eventId} for room ${roomId}`);
453423

@@ -557,21 +527,6 @@ export class RoomService {
557527

558528
const roomInfo = await this.stateService.getRoomInformation(roomId);
559529

560-
const authEventIdsForPowerLevels = await this.eventService.getAuthEventIds('m.room.power_levels', { roomId, senderId });
561-
const powerLevelsEventId = this.getEventByType(authEventIdsForPowerLevels, 'm.room.power_levels')?._id;
562-
563-
if (!powerLevelsEventId) {
564-
logger.warn(`No power_levels event found for room ${roomId}, cannot verify permission to kick.`);
565-
throw new HttpException('Cannot verify permission to kick user.', HttpStatus.FORBIDDEN);
566-
}
567-
const powerLevelsEvent = await this.eventService.getEventById(powerLevelsEventId, 'm.room.power_levels');
568-
if (!powerLevelsEvent) {
569-
logger.error(`Power levels event ${powerLevelsEventId} not found despite ID being retrieved.`);
570-
throw new HttpException('Internal server error: Power levels event data missing.', HttpStatus.INTERNAL_SERVER_ERROR);
571-
}
572-
573-
this.validateKickPermission(powerLevelsEvent.event.content, senderId, kickedUserId);
574-
575530
const kickEvent = await this.stateService.buildEvent<'m.room.member'>(
576531
{
577532
type: 'm.room.member',
@@ -590,14 +545,28 @@ export class RoomService {
590545
roomInfo.room_version,
591546
);
592547

593-
await this.stateService.handlePdu(kickEvent);
548+
try {
549+
await this.stateService.handlePdu(kickEvent);
550+
} catch (error) {
551+
if (error instanceof StateResolverAuthorizationError) {
552+
logger.warn(`User ${senderId} failed to kick ${kickedUserId} from room ${roomId}: ${error.reason}`);
553+
554+
throw new HttpException(error.reason, HttpStatus.FORBIDDEN);
555+
}
556+
throw error;
557+
}
558+
559+
void this.federationService.sendEventToAllServersInRoom(kickEvent);
560+
561+
await this.emitterService.emit('homeserver.matrix.membership', {
562+
event_id: kickEvent.eventId,
563+
event: kickEvent.event,
564+
});
594565

595566
logger.info(
596567
`Successfully created and stored m.room.member (kick) event ${kickEvent.eventId} for user ${kickedUserId} in room ${roomId}`,
597568
);
598569

599-
void this.federationService.sendEventToAllServersInRoom(kickEvent);
600-
601570
return kickEvent.eventId;
602571
}
603572

@@ -648,22 +617,6 @@ export class RoomService {
648617

649618
const roomInfo = await this.stateService.getRoomInformation(roomId);
650619

651-
const authEventIdsForPowerLevels = await this.eventService.getAuthEventIds('m.room.power_levels', { roomId, senderId });
652-
653-
const powerLevelsEventId = this.getEventByType(authEventIdsForPowerLevels, 'm.room.power_levels')?._id;
654-
655-
if (!powerLevelsEventId) {
656-
logger.warn(`No power_levels event found for room ${roomId}, cannot verify permission to ban.`);
657-
throw new HttpException('Cannot verify permission to ban user.', HttpStatus.FORBIDDEN);
658-
}
659-
const powerLevelsEvent = await this.eventService.getEventById(powerLevelsEventId, 'm.room.power_levels');
660-
if (!powerLevelsEvent) {
661-
logger.error(`Power levels event ${powerLevelsEventId} not found despite ID being retrieved.`);
662-
throw new HttpException('Internal server error: Power levels event data missing.', HttpStatus.INTERNAL_SERVER_ERROR);
663-
}
664-
665-
this.validateBanPermission(powerLevelsEvent.event.content, senderId, bannedUserId);
666-
667620
const banEvent = await this.stateService.buildEvent<'m.room.member'>(
668621
{
669622
type: 'm.room.member',
@@ -682,7 +635,16 @@ export class RoomService {
682635
roomInfo.room_version,
683636
);
684637

685-
await this.stateService.handlePdu(banEvent);
638+
try {
639+
await this.stateService.handlePdu(banEvent);
640+
} catch (error) {
641+
if (error instanceof StateResolverAuthorizationError) {
642+
logger.warn(`User ${senderId} failed to ban ${bannedUserId} from room ${roomId}: ${error.reason}`);
643+
644+
throw new HttpException(error.reason, HttpStatus.FORBIDDEN);
645+
}
646+
throw error;
647+
}
686648

687649
logger.info(`Successfully created and stored m.room.member (ban) event ${banEvent.eventId} for user ${bannedUserId} in room ${roomId}`);
688650

0 commit comments

Comments
 (0)