Skip to content
4 changes: 3 additions & 1 deletion apps/meteor/app/api/server/helpers/addUserToFileObj.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import type { IUpload, IUser } from '@rocket.chat/core-typings';
import { Users } from '@rocket.chat/models';

const isString = (value: unknown): value is string => typeof value === 'string';

export async function addUserToFileObj(files: IUpload[]): Promise<(IUpload & { user?: Pick<IUser, '_id' | 'name' | 'username'> })[]> {
const uids = files.map(({ userId }) => userId).filter(Boolean);
const uids = files.map(({ userId }) => userId).filter(isString);

const users = await Users.findByIds(uids, { projection: { name: 1, username: 1 } }).toArray();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-reviewError Handling

try {
  const users = await Users.findByIds(uids, { projection: { name: 1, username: 1 } }).toArray();
  return files.map((file) => {
    const user = users.find(({ _id }) => _id === file.userId);
    return { ...file, user };
  });
} catch (error) {
  console.error('Failed to fetch users:', error);
  throw new Error('Failed to fetch user data');
}

Multiple instances of missing or inadequate error handling in database queries and asynchronous operations, which could lead to unhandled promise rejections or runtime errors.

This issue appears in multiple locations:

  • apps/meteor/app/api/server/helpers/addUserToFileObj.ts: Lines 9-9
  • apps/meteor/app/api/server/v1/channels.ts: Lines 1011-1011
  • apps/meteor/app/api/server/v1/channels.ts: Lines 1050-1050
  • apps/meteor/server/lib/ldap/Manager.ts: Lines 504-511
  • packages/models/src/models/Users.ts: Lines 1588-1609

Please ensure all database queries and asynchronous operations are wrapped in try-catch blocks to handle potential errors and prevent unhandled promise rejections.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.


Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/app/api/server/lib/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { IUser } from '@rocket.chat/core-typings';
import { Users, Subscriptions } from '@rocket.chat/models';
import { escapeRegExp } from '@rocket.chat/string-helpers';
import type { Mongo } from 'meteor/mongo';
import type { Filter, RootFilterOperators } from 'mongodb';
import type { Filter, FindOptions, RootFilterOperators } from 'mongodb';

import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
import { settings } from '../../../settings/server';
Expand All @@ -21,7 +21,7 @@ export async function findUsersToAutocomplete({
const searchFields = settings.get<string>('Accounts_SearchFields').trim().split(',');
const exceptions = selector.exceptions || [];
const conditions = selector.conditions || {};
const options = {
const options: FindOptions<IUser> & { limit: number } = {
projection: {
name: 1,
username: 1,
Expand Down
8 changes: 4 additions & 4 deletions apps/meteor/app/api/server/v1/channels.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Team, Room } from '@rocket.chat/core-services';
import { TEAM_TYPE, type IRoom, type ISubscription, type IUser, type RoomType } from '@rocket.chat/core-typings';
import { TEAM_TYPE, type IRoom, type ISubscription, type IUser, type RoomType, type UserStatus } from '@rocket.chat/core-typings';
import { Integrations, Messages, Rooms, Subscriptions, Uploads, Users } from '@rocket.chat/models';
import {
isChannelsAddAllProps,
Expand Down Expand Up @@ -1013,7 +1013,7 @@ API.v1.addRoute(
},
];

const { cursor, totalCount } = await Rooms.findPaginated(ourQuery, {
const { cursor, totalCount } = Rooms.findPaginated(ourQuery, {
sort: sort || { name: 1 },
skip: offset,
limit: count,
Expand Down Expand Up @@ -1052,7 +1052,7 @@ API.v1.addRoute(
});
}

const { cursor, totalCount } = await Rooms.findPaginatedByTypeAndIds('c', rids, {
const { cursor, totalCount } = Rooms.findPaginatedByTypeAndIds('c', rids, {
sort: sort || { name: 1 },
skip: offset,
limit: count,
Expand Down Expand Up @@ -1103,7 +1103,7 @@ API.v1.addRoute(

const { cursor, totalCount } = await findUsersOfRoom({
rid: findResult._id,
...(status && { status: { $in: status } }),
...(status && { status: { $in: status as UserStatus[] } }),
skip,
limit,
filter,
Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/app/api/server/v1/groups.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Team, isMeteorError } from '@rocket.chat/core-services';
import type { IIntegration, IUser, IRoom, RoomType } from '@rocket.chat/core-typings';
import type { IIntegration, IUser, IRoom, RoomType, UserStatus } from '@rocket.chat/core-typings';
import { Integrations, Messages, Rooms, Subscriptions, Uploads, Users } from '@rocket.chat/models';
import { isGroupsOnlineProps, isGroupsMessagesProps } from '@rocket.chat/rest-typings';
import { check, Match } from 'meteor/check';
Expand Down Expand Up @@ -740,7 +740,7 @@ API.v1.addRoute(

const { cursor, totalCount } = await findUsersOfRoom({
rid: findResult.rid,
...(status && { status: { $in: status } }),
...(status && { status: { $in: status as UserStatus[] } }),
skip,
limit,
filter,
Expand Down
5 changes: 3 additions & 2 deletions apps/meteor/app/api/server/v1/im.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* Docs: https://github.com/RocketChat/developer-docs/blob/master/reference/api/rest-api/endpoints/team-collaboration-endpoints/im-endpoints
*/
import type { IMessage, IRoom, ISubscription } from '@rocket.chat/core-typings';
import type { IMessage, IRoom, ISubscription, IUser } from '@rocket.chat/core-typings';
import { Subscriptions, Uploads, Messages, Rooms, Users } from '@rocket.chat/models';
import {
isDmDeleteProps,
Expand All @@ -13,6 +13,7 @@ import {
} from '@rocket.chat/rest-typings';
import { Match, check } from 'meteor/check';
import { Meteor } from 'meteor/meteor';
import type { FindOptions } from 'mongodb';

import { eraseRoom } from '../../../../server/lib/eraseRoom';
import { openRoom } from '../../../../server/lib/openRoom';
Expand Down Expand Up @@ -338,7 +339,7 @@ API.v1.addRoute(
room._id,
);

const options = {
const options: FindOptions<IUser> = {
projection: {
_id: 1,
username: 1,
Expand Down
10 changes: 5 additions & 5 deletions apps/meteor/app/api/server/v1/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,7 @@ API.v1.addRoute(
});
}

let user = await (async (): Promise<
Pick<IUser, '_id' | 'roles' | 'username' | 'name' | 'status' | 'statusText'> | undefined | null
> => {
let user = await (async (): Promise<Pick<IUser, '_id' | 'username'> | undefined | null> => {
if (isUserFromParams(this.bodyParams, this.userId, this.user)) {
return Users.findOneById(this.userId);
}
Expand Down Expand Up @@ -269,9 +267,11 @@ API.v1.addRoute(
const sentTheUserByFormData = fields.userId || fields.username;
if (sentTheUserByFormData) {
if (fields.userId) {
user = await Users.findOneById(fields.userId, { projection: { username: 1 } });
user = await Users.findOneById<Pick<IUser, '_id' | 'username'>>(fields.userId, { projection: { username: 1 } });
} else if (fields.username) {
user = await Users.findOneByUsernameIgnoringCase(fields.username, { projection: { username: 1 } });
user = await Users.findOneByUsernameIgnoringCase<Pick<IUser, '_id' | 'username'>>(fields.username, {
projection: { username: 1 },
});
}

if (!user) {
Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/app/api/server/v1/voip/rooms.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { LivechatVoip } from '@rocket.chat/core-services';
import type { ILivechatAgent, IVoipRoom } from '@rocket.chat/core-typings';
import type { IVoipRoom } from '@rocket.chat/core-typings';
import { VoipRoom, LivechatVisitors, Users } from '@rocket.chat/models';
import { Random } from '@rocket.chat/random';
import { isVoipRoomProps, isVoipRoomsProps, isVoipRoomCloseProps } from '@rocket.chat/rest-typings';
Expand Down Expand Up @@ -128,7 +128,7 @@ API.v1.addRoute(
return API.v1.failure('agent-not-found');
}

const agentObj: ILivechatAgent = await Users.findOneAgentById(agentId, {
const agentObj = await Users.findOneAgentById(agentId, {
projection: { username: 1 },
});
if (!agentObj?.username) {
Expand Down
3 changes: 2 additions & 1 deletion apps/meteor/app/apps/server/bridges/activation.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { IAppServerOrchestrator, AppStatus } from '@rocket.chat/apps';
import type { ProxiedApp } from '@rocket.chat/apps-engine/server/ProxiedApp';
import { AppActivationBridge as ActivationBridge } from '@rocket.chat/apps-engine/server/bridges/AppActivationBridge';
import { UserStatus } from '@rocket.chat/core-typings';
import { Users } from '@rocket.chat/models';

export class AppActivationBridge extends ActivationBridge {
Expand Down Expand Up @@ -29,7 +30,7 @@ export class AppActivationBridge extends ActivationBridge {
}

protected async appStatusChanged(app: ProxiedApp, status: AppStatus): Promise<void> {
const userStatus = ['auto_enabled', 'manually_enabled'].includes(status) ? 'online' : 'offline';
const userStatus = ['auto_enabled', 'manually_enabled'].includes(status) ? UserStatus.ONLINE : UserStatus.OFFLINE;

await Users.updateStatusByAppId(app.getID(), userStatus);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ declare module '@rocket.chat/ddp-client' {
}
}

const isKeysResult = (result: any): result is { public_key: string; private_key: string } => {
return result.private_key && result.public_key;
};

Meteor.methods<ServerMethods>({
async 'e2e.setUserPublicAndPrivateKeys'(keyPair) {
const userId = Meteor.userId();
Expand All @@ -24,7 +28,7 @@ Meteor.methods<ServerMethods>({
if (!keyPair.force) {
const keys = await Users.fetchKeysByUserId(userId);

if (keys.private_key && keys.public_key) {
if (isKeysResult(keys)) {
throw new Meteor.Error('error-keys-already-set', 'Keys already set', {
method: 'e2e.setUserPublicAndPrivateKeys',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,9 @@ export class ConverterCache {
}

const user = await Users.findOneByUsername<Pick<IUser, '_id'>>(username, { projection: { _id: 1 } });
this.addUsernameToId(username, user?._id);
if (user) {
this.addUsernameToId(username, user._id);
}

return user?._id;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export class UserConverter extends RecordConverter<IImportUserRecord, UserConver
return newIds;
}

async findExistingUser(data: IImportUser): Promise<IUser | undefined> {
async findExistingUser(data: IImportUser): Promise<IUser | null | undefined> {
if (data.emails.length) {
const emailUser = await Users.findOneByEmailAddress(data.emails[0], {});

Expand All @@ -138,7 +138,7 @@ export class UserConverter extends RecordConverter<IImportUserRecord, UserConver

// If we couldn't find one by their email address, try to find an existing user by their username
if (data.username) {
return Users.findOneByUsernameIgnoringCase(data.username, {});
return Users.findOneByUsernameIgnoringCase<IUser>(data.username, {});
}
}

Expand Down Expand Up @@ -221,7 +221,7 @@ export class UserConverter extends RecordConverter<IImportUserRecord, UserConver
subset(userData.customFields, 'customFields');
}

async insertOrUpdateUser(existingUser: IUser | undefined, data: IImportUser): Promise<void> {
async insertOrUpdateUser(existingUser: IUser | null | undefined, data: IImportUser): Promise<void> {
if (!data.username && !existingUser?.username) {
const emails = data.emails.filter(Boolean).map((email) => ({ address: email }));
data.username = await generateUsernameSuggestion({
Expand Down
11 changes: 5 additions & 6 deletions apps/meteor/app/lib/server/functions/updateGroupDMsName.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { IUser } from '@rocket.chat/core-typings';
import { isNotUndefined } from '@rocket.chat/core-typings';
import { Rooms, Subscriptions, Users } from '@rocket.chat/models';
import type { ClientSession } from 'mongodb';

Expand All @@ -13,8 +14,8 @@ async function getUsersWhoAreInTheSameGroupDMsAs(user: IUser) {
return;
}

const userIds = new Set();
const users = new Map();
const userIds = new Set<string>();
const users = new Map<string, IUser>();

const rooms = Rooms.findGroupDMsByUids([user._id], { projection: { uids: 1 } });
await rooms.forEach((room) => {
Expand All @@ -25,9 +26,7 @@ async function getUsersWhoAreInTheSameGroupDMsAs(user: IUser) {
room.uids.forEach((uid) => uid !== user._id && userIds.add(uid));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-reviewKody Rules

room.uids.forEach((uid) => {
	if (uid !== user._id) {
		userIds.add(uid);
	}
});

Use of the comma operator in forEach loops and other expressions reduces readability and can make the code harder to debug.

This issue appears in multiple locations:

  • apps/meteor/app/lib/server/functions/updateGroupDMsName.ts: Lines 25-25
  • packages/models/src/models/Users.ts: Lines 598-598
  • packages/models/src/models/Users.ts: Lines 138-140

Please avoid using the comma operator in favor of separate statements or explicit control structures to improve code readability.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

});

(await Users.findByIds([...userIds], { projection: { username: 1, name: 1 } }).toArray()).forEach((user: IUser) =>
users.set(user._id, user),
);
(await Users.findByIds([...userIds], { projection: { username: 1, name: 1 } }).toArray()).forEach((user) => users.set(user._id, user));

return users;
}
Expand Down Expand Up @@ -59,7 +58,7 @@ export const updateGroupDMsName = async (
const rooms = Rooms.findGroupDMsByUids([userThatChangedName._id], { projection: { uids: 1 }, session });

// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
const getMembers = (uids: string[]) => uids.map((uid) => users.get(uid)).filter(Boolean);
const getMembers = (uids: string[]) => uids.map((uid) => users.get(uid)).filter(isNotUndefined);

// loop rooms to update the subscriptions from them all
for await (const room of rooms) {
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/app/livechat/server/api/lib/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ async function findUsers({
sortedResults,
totalCount: [{ total } = { total: 0 }],
},
] = await Users.findAgentsWithDepartments<ILivechatAgent>(role, query, {
] = await Users.findAgentsWithDepartments(role, query, {
sort: sort || { name: 1 },
skip: offset,
limit: count,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { AtLeast, ILivechatAgentStatus, ILivechatBusinessHour, ILivechatDepartment } from '@rocket.chat/core-typings';
import { UserStatus } from '@rocket.chat/core-typings';
import type { ILivechatBusinessHoursModel, IUsersModel } from '@rocket.chat/model-typings';
import { LivechatBusinessHours, Users } from '@rocket.chat/models';
import type { IWorkHoursCronJobsWrapper } from '@rocket.chat/models';
Expand Down Expand Up @@ -55,7 +56,7 @@ export abstract class AbstractBusinessHourBehavior {
status,
// Why this works: statusDefault is the property set when a user manually changes their status
// So if it's set to offline, we can be sure the user will be offline after login and we can skip the update
{ livechatStatusSystemModified: true, statusDefault: { $ne: 'offline' } },
{ livechatStatusSystemModified: true, statusDefault: { $ne: UserStatus.OFFLINE } },
{ livechatStatusSystemModified: true },
);

Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/app/livechat/server/business-hour/Helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export const createDefaultBusinessHourIfNotExists = async (): Promise<void> => {
}
};

export async function makeAgentsUnavailableBasedOnBusinessHour(agentIds: string[] | null = null) {
export async function makeAgentsUnavailableBasedOnBusinessHour(agentIds?: string[]) {
const results = await Users.findAgentsAvailableWithoutBusinessHours(agentIds).toArray();

const update = await Users.updateLivechatStatusByAgentIds(
Expand All @@ -75,7 +75,7 @@ export async function makeAgentsUnavailableBasedOnBusinessHour(agentIds: string[
);
}

export async function makeOnlineAgentsAvailable(agentIds: string[] | null = null) {
export async function makeOnlineAgentsAvailable(agentIds?: string[]) {
const results = await Users.findOnlineButNotAvailableAgents(agentIds).toArray();

const update = await Users.updateLivechatStatusByAgentIds(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { IUser } from '@rocket.chat/core-typings';
import { isNotUndefined } from '@rocket.chat/core-typings';
import { LivechatContacts, Users } from '@rocket.chat/models';
import type { PaginatedResult, ILivechatContactWithManagerData } from '@rocket.chat/rest-typings';
import type { FindCursor, Sort } from 'mongodb';
Expand All @@ -25,7 +26,7 @@ export async function getContacts(params: GetContactsParams): Promise<PaginatedR

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

const managerIds = [...new Set(rawContacts.map(({ contactManager }) => contactManager))];
const managerIds = [...new Set(rawContacts.map(({ contactManager }) => contactManager))].filter(isNotUndefined);
const managersCursor: FindCursor<[string, Pick<IUser, '_id' | 'name' | 'username'>]> = Users.findByIds(managerIds, {
projection: { name: 1, username: 1 },
}).map((manager) => [manager._id, manager]);
Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/app/livechat/server/lib/isMessageFromBot.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { IMessage } from '@rocket.chat/core-typings';
import type { IMessage, IUser } from '@rocket.chat/core-typings';
import { Users } from '@rocket.chat/models';

export async function isMessageFromBot(message: IMessage): Promise<boolean> {
export async function isMessageFromBot(message: IMessage): Promise<Pick<IUser, '_id' | 'roles'> | null> {
return Users.isUserInRole(message.u._id, 'bot');
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class LoadRotation {
ignoreAgentId,
settings.get<boolean>('Livechat_enabled_when_agent_idle'),
);
if (!nextAgent) {
if (!nextAgent?.username) {
return;
}

Expand Down
4 changes: 4 additions & 0 deletions apps/meteor/ee/server/lib/ldap/Manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,10 @@ export class LDAPEEManager extends LDAPManager {
const roomOwner = settings.get<string>('LDAP_Sync_User_Data_Channels_Admin') || '';

const user = await Users.findOneByUsernameIgnoringCase(roomOwner);
if (!user) {
logger.error(`Unable to find user '${roomOwner}' to be the owner of the channel '${channel}'.`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this should be just a log or a throw, because on the old code if user was not found then createRoom would actually throw a Meteor.Error

return;
}

const room = await createRoom('c', channel, user, [], false, false, {
customFields: { ldap: true },
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Meteor } from 'meteor/meteor';
import type { ServerMethods } from '@rocket.chat/ddp-client';
import { Users } from '@rocket.chat/models';
import { isPersonalAccessToken } from '@rocket.chat/core-typings';

import { hasPermissionAsync } from '../../../../../app/authorization/server/functions/hasPermission';
import { twoFactorRequired } from '../../../../../app/2fa/server/twoFactorRequired';
Expand Down Expand Up @@ -33,8 +34,14 @@ export const regeneratePersonalAccessTokenOfUser = async (tokenName: string, use

await removePersonalAccessTokenOfUser(tokenName, userId);

return generatePersonalAccessTokenOfUser({ tokenName, userId, bypassTwoFactor: tokenExist.bypassTwoFactor || false });
}
const tokenObject = tokenExist.services?.resume?.loginTokens?.find((token) => isPersonalAccessToken(token) && token.name === tokenName);

return generatePersonalAccessTokenOfUser({
tokenName,
userId,
bypassTwoFactor: (tokenObject && isPersonalAccessToken(tokenObject) && tokenObject.bypassTwoFactor) || false,
});
};

Meteor.methods<ServerMethods>({
'personalAccessTokens:regenerateToken': twoFactorRequired(async function ({ tokenName }) {
Expand All @@ -44,7 +51,7 @@ Meteor.methods<ServerMethods>({
method: 'personalAccessTokens:regenerateToken',
});
}

return regeneratePersonalAccessTokenOfUser(tokenName, uid);
}),
});
Loading
Loading