Skip to content

Commit 5144cd8

Browse files
committed
fix: remove permissive any overloads from model find typings
IBaseModel exposed the class implementation signatures (options?: any) as public overloads for findOneById, findOne and findPaginated, so invalid FindOptions compiled silently and were ignored by the MongoDB driver. Removing them surfaced and fixes: - rooms.bannedUsers passed { offset, count } to findPaginated: pagination never applied, every banned subscription returned with full documents - omnichannel department listings used offset instead of skip: every page returned the first page - projections typo (rooms.hide, private group lookup) and bare projection objects (direct email reply, auto-transfer scheduler) fetched full documents; group lookup keeps roles for the room access validators - omnichannel pagination helpers typed sort as Record<string, number>, which is not a valid mongo Sort
1 parent 0e7b205 commit 5144cd8

10 files changed

Lines changed: 24 additions & 21 deletions

File tree

.changeset/strict-find-options.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@rocket.chat/meteor': patch
3+
'@rocket.chat/model-typings': patch
4+
---
5+
6+
Fixes broken pagination on `rooms.bannedUsers` and on omnichannel department listing endpoints, which ignored the `offset` parameter and always returned results from the first page. Also reduces payload over-fetching on several endpoints that unintentionally loaded full documents (`rooms.hide`, private group lookups, direct email replies, omnichannel auto-transfer), and removes the permissive model query typings that allowed these invalid find options to compile unnoticed.

apps/meteor/ee/server/lib/omnichannel/AutoTransferChatScheduler.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,7 @@ export class AutoTransferChatSchedulerClass {
7878

7979
private async transferRoom(roomId: string): Promise<void> {
8080
this.logger.debug({ msg: 'Transferring room', roomId });
81-
const room = await LivechatRooms.findOneById(roomId, {
82-
_id: 1,
83-
v: 1,
84-
servedBy: 1,
85-
open: 1,
86-
departmentId: 1,
87-
});
81+
const room = await LivechatRooms.findOneById(roomId);
8882
if (!room?.open || !room?.servedBy?._id) {
8983
throw new Error('Room is not open or is not being served by an agent');
9084
}

apps/meteor/ee/server/lib/omnichannel/Department.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export const findAllDepartmentsAvailable = async (
2424
query = await applyDepartmentRestrictions(query, uid);
2525
}
2626

27-
const { cursor, totalCount } = LivechatDepartment.findPaginated(query, { limit: count, offset, sort: { name: 1 } });
27+
const { cursor, totalCount } = LivechatDepartment.findPaginated(query, { limit: count, skip: offset, sort: { name: 1 } });
2828

2929
const [departments, total] = await Promise.all([cursor.toArray(), totalCount]);
3030

@@ -40,7 +40,7 @@ export const findAllDepartmentsByUnit = async (
4040
{
4141
ancestors: { $in: [unitId] },
4242
},
43-
{ limit: count, offset },
43+
{ limit: count, skip: offset },
4444
);
4545

4646
const [departments, total] = await Promise.all([cursor.toArray(), totalCount]);

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,8 @@ async function findPrivateGroupByIdOrName({
106106
}> {
107107
const room = await getRoomFromParams(params);
108108

109-
const user = await Users.findOneById(userId, { projections: { username: 1 } });
109+
// `roles` is required by the room access validators, which read it straight from the user object
110+
const user = await Users.findOneById(userId, { projection: { username: 1, roles: 1 } });
110111

111112
if (!room || !user || !(await canAccessRoomAsync(room, user))) {
112113
throw new Meteor.Error('error-room-not-found', 'The required "roomId" or "roomName" param provided does not match any group');

apps/meteor/server/api/v1/omnichannel/lib/customFields.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export async function findLivechatCustomFields({
99
pagination: { offset, count, sort },
1010
}: {
1111
text?: string;
12-
pagination: { offset: number; count: number; sort: Record<string, number> };
12+
pagination: { offset: number; count: number; sort: Record<string, 1 | -1> };
1313
}): Promise<PaginatedResult<{ customFields: Array<ILivechatCustomField> }>> {
1414
const query = {
1515
...(text && {

apps/meteor/server/api/v1/omnichannel/lib/transfer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export async function findLivechatTransferHistory({
1111
pagination: { offset, count, sort },
1212
}: {
1313
rid: string;
14-
pagination: { offset: number; count: number; sort: Record<string, number> };
14+
pagination: { offset: number; count: number; sort: Record<string, 1 | -1> };
1515
}): Promise<PaginatedResult<{ history: IOmnichannelSystemMessage['transferData'][] }>> {
1616
const { cursor, totalCount } = Messages.findPaginated(
1717
{ rid, t: 'livechat_transfer_history' },

apps/meteor/server/api/v1/omnichannel/lib/triggers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { PaginatedResult } from '@rocket.chat/rest-typings';
55
export async function findTriggers({
66
pagination: { offset, count, sort },
77
}: {
8-
pagination: { offset: number; count: number; sort: Record<string, number> };
8+
pagination: { offset: number; count: number; sort: Record<string, 1 | -1> };
99
}): Promise<PaginatedResult<{ triggers: Array<ILivechatTrigger> }>> {
1010
const { cursor, totalCount } = LivechatTrigger.findPaginated(
1111
{},

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1301,7 +1301,7 @@ API.v1.post(
13011301
return API.v1.unauthorized();
13021302
}
13031303

1304-
const user = await Users.findOneById(this.userId, { projections: { _id: 1 } });
1304+
const user = await Users.findOneById(this.userId, { projection: { _id: 1 } });
13051305

13061306
if (!user) {
13071307
return API.v1.failure('error-invalid-user');
@@ -1706,7 +1706,10 @@ export const roomEndpoints = API.v1
17061706

17071707
const { offset, count } = await getPaginationItems(this.queryParams);
17081708

1709-
const { cursor, totalCount } = Subscriptions.findPaginated({ rid: roomId, status: 'BANNED' as const }, { offset, count });
1709+
const { cursor, totalCount } = Subscriptions.findPaginated(
1710+
{ rid: roomId, status: 'BANNED' as const },
1711+
{ sort: { ts: 1 }, skip: offset, limit: count, projection: { 'u._id': 1 } },
1712+
);
17101713

17111714
const [bannedSubs, total] = await Promise.all([cursor.toArray(), totalCount]);
17121715

apps/meteor/server/lib/notifications/processDirectEmail.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,10 @@ export const processDirectEmail = async function (email: ParsedMail): Promise<vo
5050
}
5151

5252
const prevMessage = await Messages.findOneById(mid, {
53-
rid: 1,
54-
u: 1,
53+
projection: {
54+
rid: 1,
55+
u: 1,
56+
},
5557
});
5658

5759
if (!prevMessage) {

packages/model-typings/src/models/IBaseModel.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,9 @@ export interface IBaseModel<
6060

6161
findOneById(_id: T['_id'], options?: FindOptions<T> | undefined): Promise<T | null>;
6262
findOneById<P extends Document = T>(_id: T['_id'], options?: FindOptions<P>): Promise<P | null>;
63-
findOneById(_id: T['_id'], options?: any): Promise<T | null>;
6463

6564
findOne(query?: Filter<T> | T['_id'], options?: undefined): Promise<T | null>;
66-
findOne<P extends Document = T>(query: Filter<T> | T['_id'], options: FindOptions<P extends T ? T : P>): Promise<P | null>;
67-
findOne<P>(query: Filter<T> | T['_id'], options?: any): Promise<WithId<T> | WithId<P> | null>;
65+
findOne<P extends Document = T>(query: Filter<T> | T['_id'], options?: FindOptions<P extends T ? T : P>): Promise<P | null>;
6866

6967
find(query?: Filter<T>): FindCursor<ResultFields<T, C>>;
7068
find<P extends Document = T>(query: Filter<T>, options: FindOptions<P extends T ? T : P>): FindCursor<P>;
@@ -74,7 +72,6 @@ export interface IBaseModel<
7472
): FindCursor<WithId<P>> | FindCursor<WithId<T>>;
7573

7674
findPaginated<P extends Document = T>(query: Filter<T>, options?: FindOptions<P extends T ? T : P>): FindPaginated<FindCursor<WithId<P>>>;
77-
findPaginated(query: Filter<T>, options?: any): FindPaginated<FindCursor<WithId<T>>>;
7875

7976
update(
8077
filter: Filter<T>,

0 commit comments

Comments
 (0)