Skip to content

Commit 3e5b9a8

Browse files
ggazzoggazzo
authored
chore: reduce user finds over rest operations (#39618)
1 parent a834e17 commit 3e5b9a8

File tree

8 files changed

+40
-42
lines changed

8 files changed

+40
-42
lines changed

‎apps/meteor/app/api/server/v1/channels.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ API.v1.addRoute(
205205
async get() {
206206
const findResult = await findChannelByIdOrName({ params: this.queryParams });
207207

208-
const roles = await executeGetRoomRoles(findResult._id, this.userId);
208+
const roles = await executeGetRoomRoles(findResult._id, this.user);
209209

210210
return API.v1.success({
211211
roles,

‎apps/meteor/app/api/server/v1/groups.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ async function getRoomFromParams(params: { roomId?: string } | { roomName?: stri
6767
}
6868
})();
6969

70-
if (!room || room.t !== 'p') {
70+
if (room?.t !== 'p') {
7171
throw new Meteor.Error('error-room-not-found', 'The required "roomId" or "roomName" param provided does not match any group');
7272
}
7373

@@ -273,7 +273,7 @@ API.v1.addRoute(
273273
room = await Rooms.findOneByName(params.roomName || '');
274274
}
275275

276-
if (!room || room.t !== 'p') {
276+
if (room?.t !== 'p') {
277277
throw new Meteor.Error('error-room-not-found', 'The required "roomId" or "roomName" param provided does not match any group');
278278
}
279279

@@ -791,7 +791,7 @@ API.v1.addRoute(
791791
rid: findResult.rid,
792792
...parseIds(mentionIds, 'mentions._id'),
793793
...parseIds(starredIds, 'starred._id'),
794-
...(pinned && pinned.toLowerCase() === 'true' ? { pinned: true } : {}),
794+
...(pinned?.toLowerCase() === 'true' ? { pinned: true } : {}),
795795
_hidden: { $ne: true },
796796
};
797797

@@ -1192,7 +1192,7 @@ API.v1.addRoute(
11921192
userId: this.userId,
11931193
});
11941194

1195-
const roles = await executeGetRoomRoles(findResult.rid, this.userId);
1195+
const roles = await executeGetRoomRoles(findResult.rid, this.user);
11961196

11971197
return API.v1.success({
11981198
roles,

‎apps/meteor/app/api/server/v1/rooms.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
isRoomsInviteProps,
1919
validateBadRequestErrorResponse,
2020
validateUnauthorizedErrorResponse,
21+
validateForbiddenErrorResponse,
2122
} from '@rocket.chat/rest-typings';
2223
import { isTruthy } from '@rocket.chat/tools';
2324
import { Meteor } from 'meteor/meteor';
@@ -455,7 +456,6 @@ API.v1.addRoute(
455456
{ authRequired: true /* , validateParams: isRoomsCreateDiscussionProps */ },
456457
{
457458
async post() {
458-
// eslint-disable-next-line @typescript-eslint/naming-convention
459459
const { prid, pmid, reply, t_name, users, encrypted, topic } = this.bodyParams;
460460
if (!prid) {
461461
return API.v1.failure('Body parameter "prid" is required.');
@@ -550,7 +550,7 @@ API.v1.addRoute(
550550
const [files, total] = await Promise.all([cursor.toArray(), totalCount]);
551551

552552
// If the initial image was not returned in the query, insert it as the first element of the list
553-
if (initialImage && !files.find(({ _id }) => _id === (initialImage as IUpload)._id)) {
553+
if (initialImage && !files.find(({ _id }) => _id === initialImage._id)) {
554554
files.splice(0, 0, initialImage);
555555
}
556556

@@ -767,7 +767,7 @@ API.v1.addRoute(
767767
void dataExport.sendFile(
768768
{
769769
rid,
770-
format: format as 'html' | 'json',
770+
format,
771771
dateFrom: convertedDateFrom,
772772
dateTo: convertedDateTo,
773773
},
@@ -815,7 +815,7 @@ API.v1.addRoute(
815815
const [room, user] = await Promise.all([
816816
findRoomByIdOrName({
817817
params: { roomId },
818-
}) as Promise<IRoom>,
818+
}),
819819
Users.findOneByIdOrUsername(userId || username),
820820
]);
821821

@@ -1070,11 +1070,14 @@ export const roomEndpoints = API.v1
10701070
},
10711071
required: ['roles'],
10721072
}),
1073+
400: validateBadRequestErrorResponse,
1074+
401: validateUnauthorizedErrorResponse,
1075+
403: validateForbiddenErrorResponse,
10731076
},
10741077
},
10751078
async function () {
10761079
const { rid } = this.queryParams;
1077-
const roles = await executeGetRoomRoles(rid, this.userId);
1080+
const roles = await executeGetRoomRoles(rid, this.user);
10781081

10791082
return API.v1.success({
10801083
roles,
@@ -1246,7 +1249,6 @@ export const roomEndpoints = API.v1
12461249
);
12471250

12481251
type RoomEndpoints = ExtractRoutesFromAPI<typeof roomEndpoints> &
1249-
ExtractRoutesFromAPI<typeof roomEndpoints> &
12501252
ExtractRoutesFromAPI<typeof roomDeleteEndpoint> &
12511253
ExtractRoutesFromAPI<typeof roomsSaveNotificationEndpoint>;
12521254

‎apps/meteor/app/lib/server/methods/getRoomRoles.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { IRoom } from '@rocket.chat/core-typings';
1+
import type { IRoom, IUser } from '@rocket.chat/core-typings';
22
import { Rooms } from '@rocket.chat/models';
33
import { check } from 'meteor/check';
44
import { Meteor } from 'meteor/meteor';
@@ -15,10 +15,10 @@ declare module '@rocket.chat/ddp-client' {
1515
}
1616
}
1717

18-
export const executeGetRoomRoles = async (rid: IRoom['_id'], fromUserId?: string | null) => {
18+
export const executeGetRoomRoles = async (rid: IRoom['_id'], fromUser?: IUser | null) => {
1919
check(rid, String);
2020

21-
if (!fromUserId && settings.get('Accounts_AllowAnonymousRead') === false) {
21+
if (!fromUser && settings.get('Accounts_AllowAnonymousRead') === false) {
2222
throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'getRoomRoles' });
2323
}
2424

@@ -27,7 +27,7 @@ export const executeGetRoomRoles = async (rid: IRoom['_id'], fromUserId?: string
2727
throw new Meteor.Error('error-invalid-room', 'Invalid room', { method: 'getRoomRoles' });
2828
}
2929

30-
if (fromUserId && !(await canAccessRoomAsync(room, { _id: fromUserId }))) {
30+
if (fromUser && !(await canAccessRoomAsync(room, fromUser))) {
3131
throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'getRoomRoles' });
3232
}
3333

‎apps/meteor/server/methods/loadHistory.ts‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,36 +31,35 @@ declare module '@rocket.chat/ddp-client' {
3131
Meteor.methods<ServerMethods>({
3232
async loadHistory(rid, end, limit = 20, ls, showThreadMessages = true) {
3333
check(rid, String);
34+
const fromUser = await Meteor.userAsync();
3435

35-
if (!Meteor.userId() && settings.get('Accounts_AllowAnonymousRead') === false) {
36+
if (!fromUser && settings.get('Accounts_AllowAnonymousRead') === false) {
3637
throw new Meteor.Error('error-invalid-user', 'Invalid user', {
3738
method: 'loadHistory',
3839
});
3940
}
4041

41-
const fromId = Meteor.userId();
42-
4342
const room = await Rooms.findOneById(rid, { projection: { ...roomAccessAttributes, t: 1 } });
4443
if (!room) {
4544
return false;
4645
}
4746

4847
// this checks the Allow Anonymous Read setting, so no need to check again
49-
if (!(await canAccessRoomAsync(room, fromId ? { _id: fromId } : undefined))) {
48+
if (!(await canAccessRoomAsync(room, fromUser || undefined))) {
5049
return false;
5150
}
5251

5352
// if fromId is undefined and it passed the previous check, the user is reading anonymously
54-
if (!fromId) {
53+
if (!fromUser) {
5554
return loadMessageHistory({ rid, end, limit, ls, showThreadMessages });
5655
}
5756

58-
const canPreview = await hasPermissionAsync(fromId, 'preview-c-room');
57+
const canPreview = await hasPermissionAsync(fromUser._id, 'preview-c-room');
5958

60-
if (room.t === 'c' && !canPreview && !(await Subscriptions.findOneByRoomIdAndUserId(rid, fromId, { projection: { _id: 1 } }))) {
59+
if (room.t === 'c' && !canPreview && !(await Subscriptions.findOneByRoomIdAndUserId(rid, fromUser._id, { projection: { _id: 1 } }))) {
6160
return false;
6261
}
6362

64-
return loadMessageHistory({ userId: fromId, rid, end, limit, ls, showThreadMessages });
63+
return loadMessageHistory({ userId: fromUser._id, rid, end, limit, ls, showThreadMessages });
6564
},
6665
});

‎apps/meteor/server/publications/room/index.ts‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ Meteor.methods<ServerMethods>({
5757
});
5858
}
5959

60-
const userId = Meteor.userId();
61-
const isAnonymous = !userId;
60+
const user = await Meteor.userAsync();
61+
const isAnonymous = !user?._id;
6262

6363
if (isAnonymous) {
6464
const allowAnon = settings.get('Accounts_AllowAnonymousRead');
@@ -80,21 +80,17 @@ Meteor.methods<ServerMethods>({
8080
}
8181

8282
if (
83-
userId &&
84-
!(await canAccessRoomAsync(
85-
room,
86-
{ _id: userId },
87-
{
88-
includeInvitations: true,
89-
},
90-
))
83+
user &&
84+
!(await canAccessRoomAsync(room, user, {
85+
includeInvitations: true,
86+
}))
9187
) {
9288
throw new Meteor.Error('error-no-permission', 'No permission', {
9389
method: 'getRoomByTypeAndName',
9490
});
9591
}
9692

97-
if (settings.get('Store_Last_Message') && userId && !(await hasPermissionAsync(userId, 'preview-c-room'))) {
93+
if (settings.get('Store_Last_Message') && user && !(await hasPermissionAsync(user._id, 'preview-c-room'))) {
9894
delete room.lastMessage;
9995
}
10096

‎apps/meteor/server/services/authorization/canAccessRoom.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ const roomAccessValidators: RoomAccessValidatorConverted[] = [
9595
canAccessRoomLivechat,
9696
];
9797

98-
const isPartialUser = (user: IUser | Pick<IUser, '_id'> | undefined): user is Pick<IUser, '_id'> => {
98+
export const isPartialUser = (user: IUser | Pick<IUser, '_id'> | undefined): user is Pick<IUser, '_id'> => {
9999
return Boolean(user && Object.keys(user).length === 1 && '_id' in user);
100100
};
101101

‎apps/meteor/server/services/authorization/canReadRoom.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,22 @@
11
import type { RoomAccessValidator } from '@rocket.chat/core-services';
22
import { Authorization } from '@rocket.chat/core-services';
3-
import { Subscriptions } from '@rocket.chat/models';
3+
import { Subscriptions, Users } from '@rocket.chat/models';
44

5-
import { canAccessRoom } from './canAccessRoom';
5+
import { canAccessRoom, isPartialUser } from './canAccessRoom';
66

77
export const canReadRoom: RoomAccessValidator = async (...args) => {
8+
const [room, user] = args;
9+
const userArgument = isPartialUser(user) ? await Users.findOneById(user._id) : user;
10+
811
if (!(await canAccessRoom(...args))) {
912
return false;
1013
}
1114

12-
const [room, user] = args;
13-
1415
if (
15-
user?._id &&
16+
userArgument &&
1617
room?.t === 'c' &&
17-
!(await Authorization.hasPermission(user._id, 'preview-c-room')) &&
18-
!(await Subscriptions.findOneByRoomIdAndUserId(room?._id, user._id, { projection: { _id: 1 } }))
18+
!(await Authorization.hasPermission(userArgument, 'preview-c-room')) &&
19+
!(await Subscriptions.findOneByRoomIdAndUserId(room?._id, userArgument._id, { projection: { _id: 1 } }))
1920
) {
2021
return false;
2122
}

0 commit comments

Comments
 (0)