Skip to content

Commit cf748a6

Browse files
ggazzosampaiodiego
andauthored
regression(federation): enhance user authorization checks for federation access (#37965)
Co-authored-by: Diego Sampaio <chinello@gmail.com>
1 parent 466137a commit cf748a6

File tree

10 files changed

+321
-25
lines changed

10 files changed

+321
-25
lines changed

apps/meteor/app/lib/server/functions/createRoom.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { AppEvents, Apps } from '@rocket.chat/apps';
22
import { AppsEngineException } from '@rocket.chat/apps-engine/definition/exceptions';
33
import { FederationMatrix, Message, Room, Team } from '@rocket.chat/core-services';
44
import type { ICreateRoomParams, ISubscriptionExtraData } from '@rocket.chat/core-services';
5-
import type { ICreatedRoom, IUser, IRoom, RoomType } from '@rocket.chat/core-typings';
5+
import { type ICreatedRoom, type IUser, type IRoom, type RoomType, isUserNativeFederated } from '@rocket.chat/core-typings';
66
import { Rooms, Subscriptions, Users } from '@rocket.chat/models';
77
import { Meteor } from 'meteor/meteor';
88

@@ -184,7 +184,12 @@ export const createRoom = async <T extends RoomType>(
184184

185185
const shouldBeHandledByFederation = extraData.federated === true;
186186

187-
if (shouldBeHandledByFederation && owner && !(await hasPermissionAsync(owner._id, 'access-federation'))) {
187+
if (
188+
shouldBeHandledByFederation &&
189+
owner &&
190+
!isUserNativeFederated(owner) &&
191+
!(await hasPermissionAsync(owner._id, 'access-federation'))
192+
) {
188193
throw new Meteor.Error('error-not-authorized-federation', 'Not authorized to access federation', {
189194
method: 'createRoom',
190195
});

apps/meteor/app/lib/server/functions/getRoomByNameOrIdWithOptionToJoin.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export const getRoomByNameOrIdWithOptionToJoin = async ({
1414
joinChannel = true,
1515
errorOnEmpty = true,
1616
}: {
17-
user: Pick<IUser, '_id' | 'username'>;
17+
user: Pick<IUser, '_id' | 'username' | 'federated' | 'federation'>;
1818
nameOrId: string;
1919
type?: RoomType;
2020
tryDirectByUserIdOnly?: boolean;

apps/meteor/app/lib/server/methods/joinRoom.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ Meteor.methods<ServerMethods>({
1616
async joinRoom(rid, code) {
1717
check(rid, String);
1818

19-
const userId = await Meteor.userId();
20-
if (!userId) {
19+
const user = await Meteor.userAsync();
20+
if (!user) {
2121
throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'joinRoom' });
2222
}
2323

@@ -26,6 +26,6 @@ Meteor.methods<ServerMethods>({
2626
throw new Meteor.Error('error-invalid-room', 'Invalid room', { method: 'joinRoom' });
2727
}
2828

29-
return Room.join({ room, user: { _id: userId }, ...(code ? { joinCode: code } : {}) });
29+
return Room.join({ room, user, ...(code ? { joinCode: code } : {}) });
3030
},
3131
});

apps/meteor/app/slashcommands-join/server/server.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { api, Room } from '@rocket.chat/core-services';
22
import type { SlashCommandCallbackParams } from '@rocket.chat/core-typings';
3-
import { Rooms, Subscriptions } from '@rocket.chat/models';
3+
import { Rooms, Subscriptions, Users } from '@rocket.chat/models';
44
import { Meteor } from 'meteor/meteor';
55

66
import { i18n } from '../../../server/lib/i18n';
@@ -43,7 +43,13 @@ slashCommands.add({
4343
});
4444
}
4545

46-
await Room.join({ room, user: { _id: userId } });
46+
const user = await Users.findOneById(userId, { projection: { federated: 1, federation: 1 } });
47+
if (!user) {
48+
throw new Meteor.Error('error-invalid-user', 'Invalid user', {
49+
method: 'slashCommands',
50+
});
51+
}
52+
await Room.join({ room, user });
4753
},
4854
options: {
4955
description: 'Join_the_given_channel',

apps/meteor/ee/server/hooks/federation/index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,7 @@ beforeAddUserToRoom.add(
112112
return;
113113
}
114114

115-
// TODO should we really check for "user" here? it is potentially an external user
116-
if (!(await Authorization.hasPermission(user._id, 'access-federation'))) {
115+
if (!isUserNativeFederated(user) && !(await Authorization.hasPermission(user._id, 'access-federation'))) {
117116
throw new MeteorError('error-not-authorized-federation', 'Not authorized to access federation');
118117
}
119118

apps/meteor/server/services/room/service.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
isOmnichannelRoom,
1010
isRoomWithJoinCode,
1111
} from '@rocket.chat/core-typings';
12+
import { isUserNativeFederated } from '@rocket.chat/core-typings';
1213
import { Rooms, Subscriptions, Users } from '@rocket.chat/models';
1314

1415
import { getNameForDMs } from './getNameForDMs';
@@ -148,7 +149,7 @@ export class RoomService extends ServiceClassInternal implements IRoomService {
148149
/**
149150
* Method called by users to join a room.
150151
*/
151-
async join({ room, user, joinCode }: { room: IRoom; user: Pick<IUser, '_id'>; joinCode?: string }) {
152+
async join({ room, user, joinCode }: { room: IRoom; user: Pick<IUser, '_id' | 'federated' | 'federation'>; joinCode?: string }) {
152153
if (!(await roomCoordinator.getRoomDirectives(room.t)?.allowMemberAction(room, RoomMemberActions.JOIN, user._id))) {
153154
throw new MeteorError('error-not-allowed', 'Not allowed', { method: 'joinRoom' });
154155
}
@@ -161,7 +162,11 @@ export class RoomService extends ServiceClassInternal implements IRoomService {
161162
throw new MeteorError('error-not-allowed', 'Not allowed', { method: 'joinRoom' });
162163
}
163164

164-
if (FederationActions.shouldPerformFederationAction(room) && !(await Authorization.hasPermission(user._id, 'access-federation'))) {
165+
if (
166+
FederationActions.shouldPerformFederationAction(room) &&
167+
!isUserNativeFederated(user) &&
168+
!(await Authorization.hasPermission(user._id, 'access-federation'))
169+
) {
165170
throw new MeteorError('error-not-authorized-federation', 'Not authorized to access federation', { method: 'joinRoom' });
166171
}
167172

ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,7 @@ import { IS_EE } from '../../../../../apps/meteor/tests/e2e/config/constants';
1010
import { retry } from '../../../../../apps/meteor/tests/end-to-end/api/helpers/retry';
1111
import { federationConfig } from '../helper/config';
1212
import { SynapseClient } from '../helper/synapse-client';
13-
14-
function withTimeout<T>(fn: (signal: AbortSignal) => Promise<T>, ms: number): Promise<T> {
15-
const controller = new AbortController();
16-
17-
const timeoutId = setTimeout(() => {
18-
controller.abort();
19-
}, ms);
20-
21-
return fn(controller.signal).finally(() => {
22-
clearTimeout(timeoutId);
23-
});
24-
}
13+
import { withTimeout } from '../helper/withTimeout';
2514

2615
const waitForRoomEvent = async (
2716
room: Room,

0 commit comments

Comments
 (0)