Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/meteor/app/slashcommand-asciiarts/client/gimme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { slashCommands } from '../../utils/client/slashCommand';
*/
async function Gimme({ message, params }: SlashCommandCallbackParams<'gimme'>): Promise<void> {
const msg = message;
await sdk.call('sendMessage', { ...msg, msg: `༼ つ ◕_◕ ༽つ ${params}` });
await sdk.rest.post('/v1/chat.sendMessage', { message: { ...msg, msg: `༼ つ ◕_◕ ༽つ ${params}` } });
}

slashCommands.add({
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/app/slashcommand-asciiarts/client/lenny.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { slashCommands } from '../../utils/client/slashCommand';

async function LennyFace({ message, params }: SlashCommandCallbackParams<'lenny'>): Promise<void> {
const msg = message;
await sdk.call('sendMessage', { ...msg, msg: `${params} ( ͡° ͜ʖ ͡°)` });
await sdk.rest.post('/v1/chat.sendMessage', { message: { ...msg, msg: `${params} ( ͡° ͜ʖ ͡°)` } });
}

slashCommands.add({
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/app/slashcommand-asciiarts/client/shrug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ slashCommands.add({
command: 'shrug',
callback: async ({ message, params }: SlashCommandCallbackParams<'shrug'>): Promise<void> => {
const msg = message;
await sdk.call('sendMessage', { ...msg, msg: `${params} ¯\\\\_(ツ)_/¯` });
await sdk.rest.post('/v1/chat.sendMessage', { message: { ...msg, msg: `${params} ¯\\\\_(ツ)_/¯` } });
},
options: {
description: 'Slash_Shrug_Description',
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/app/slashcommand-asciiarts/client/tableflip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ slashCommands.add({
command: 'tableflip',
callback: async ({ message, params }: SlashCommandCallbackParams<'tableflip'>): Promise<void> => {
const msg = message;
await sdk.call('sendMessage', { ...msg, msg: `${params} (╯°□°)╯︵ ┻━┻` });
await sdk.rest.post('/v1/chat.sendMessage', { message: { ...msg, msg: `${params} (╯°□°)╯︵ ┻━┻` } });
},
options: {
description: 'Slash_Tableflip_Description',
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/app/slashcommand-asciiarts/client/unflip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ slashCommands.add({
command: 'unflip',
callback: async ({ message, params }: SlashCommandCallbackParams<'unflip'>): Promise<void> => {
const msg = message;
await sdk.call('sendMessage', { ...msg, msg: `${params} ┬─┬ ノ( ゜-゜ノ)` });
await sdk.rest.post('/v1/chat.sendMessage', { message: { ...msg, msg: `${params} ┬─┬ ノ( ゜-゜ノ)` } });
},
options: {
description: 'Slash_TableUnflip_Description',
Expand Down
21 changes: 21 additions & 0 deletions apps/meteor/app/utils/client/lib/RestApiClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { RestClient } from '@rocket.chat/api-client';

import { invokeTwoFactorModal } from '../../../../client/lib/2fa/process2faReturn';
import { baseURI } from '../../../../client/lib/baseURI';
import { clearStoredCredentials } from '../../../../client/lib/sdk/ddpSdk';
import { STORAGE_KEYS, getStoredItem } from '../../../../client/lib/sdk/storage';

class RestApiClient extends RestClient {
Expand Down Expand Up @@ -37,11 +38,31 @@ APIClient.handleTwoFactorChallenge(invokeTwoFactorModal);
* This middleware will throw the error object instead.
* */

/**
* A 401 means the request reached an authRequired route without a session the server recognizes.
* That is only evidence of an expired session when the request was an action the user just took:
* boot-time reads (OmnichannelProvider's livechat/config/routing, custom-sounds.list) fire before
* the session is established and answer 401 with the same message, and clearing on those logs out
* a session that is coming up. Restricting the wipe to mutations mirrors the DDP path this replaced
* — ddpOverREST only clears credentials for method calls, never for background fetches — so a
* session that dies while idle is noticed on the user's next write, exactly as before.
*/
const isMutation = (method: string): boolean => method === 'POST' || method === 'PUT' || method === 'DELETE';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Include PATCH in mutation credential handling.

A 401 from a PATCH mutation will not clear stale credentials, unlike the other write methods.

-const isMutation = (method: string): boolean => method === 'POST' || method === 'PUT' || method === 'DELETE';
+const isMutation = (method: string): boolean => method === 'POST' || method === 'PUT' || method === 'PATCH' || method === 'DELETE';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const isMutation = (method: string): boolean => method === 'POST' || method === 'PUT' || method === 'DELETE';
const isMutation = (method: string): boolean => method === 'POST' || method === 'PUT' || method === 'PATCH' || method === 'DELETE';
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/meteor/app/utils/client/lib/RestApiClient.ts` at line 50, Update the
isMutation helper to classify PATCH alongside POST, PUT, and DELETE so PATCH
requests use the same mutation credential handling, including clearing stale
credentials after a 401.


APIClient.use(async (request, next) => {
const [, method] = request;
const tokenAtSend = getStoredItem(STORAGE_KEYS.LOGIN_TOKEN);

try {
return await next(...request);
} catch (error) {
if (error instanceof Response) {
// Never 403 (authenticated but lacking permission) — that must not log the user out.
// The token comparison keeps a request that was in flight across a re-login from
// evicting the session it does not belong to.
if (error.status === 401 && isMutation(method) && tokenAtSend && tokenAtSend === getStoredItem(STORAGE_KEYS.LOGIN_TOKEN)) {
clearStoredCredentials();
}
const e = await error.json();
throw e;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { sdk } from '../../../app/utils/client/lib/SDKClient';
import UserAutoCompleteMultiple from '../../components/UserAutoCompleteMultiple';
import { useOpenedRoom } from '../../lib/RoomManager';
import { roomCoordinator } from '../../lib/rooms/roomCoordinator';
import { callWithErrorHandling } from '../../lib/utils/callWithErrorHandling';

type Username = Exclude<IUser['username'], undefined>;

Expand All @@ -35,10 +34,12 @@ const GameCenterInvitePlayersModal = ({ game, onClose }: IGameCenterInvitePlayer
roomCoordinator.openRouteLink(group.t, { rid: group._id, name: group.name });

if (openedRoom === group._id) {
await callWithErrorHandling('sendMessage', {
_id: Random.id(),
rid: group._id,
msg: t('Apps_Game_Center_Play_Game_Together', { name }),
await sdk.rest.post('/v1/chat.sendMessage', {
message: {
_id: Random.id(),
rid: group._id,
msg: t('Apps_Game_Center_Play_Game_Together', { name }),
},
});
}
onClose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const useReadReceiptsDetailsAction = (message: IMessage): MessageActionCo
setModal(
<ReadReceiptsModal
messageId={message._id}
rid={message.rid}
onClose={() => {
setModal(null);
}}
Expand Down
10 changes: 6 additions & 4 deletions apps/meteor/client/hooks/notification/useNotification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,12 @@ export const useNotification = () => {
n.addEventListener(
'reply',
({ response }) =>
void sdk.call('sendMessage', {
_id: Random.id(),
rid,
msg: response,
void sdk.rest.post('/v1/chat.sendMessage', {
message: {
_id: Random.id(),
rid,
msg: response,
},
}),
);
}
Expand Down
15 changes: 12 additions & 3 deletions apps/meteor/client/lib/chats/flows/sendMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,13 @@ const process = async (chat: ChatAPI, message: IMessage, previewUrls?: string[],

chat.composer?.clear();
await runOptimisticSendMessage(message);
await sdk.call('sendMessage', message, previewUrls);

// after the request is complete we can go ahead and mark as sent
await sdk.rest.post('/v1/chat.sendMessage', { message, previewUrls });

// Clear the optimistic `temp` flag only if the messages stream hasn't already
// replaced the record. Overwriting with the server response can clobber stream
// updates that arrive first — e.g. read-receipt-driven `unread: false`, async
// URL/quote attachments, or E2EE decrypt — leading to stale UI state.
Comment on lines +53 to +56

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the added implementation comments.

  • apps/meteor/client/lib/chats/flows/sendMessage.ts#L53-L56: remove the optimistic-state rationale comment.
  • apps/meteor/client/lib/chats/flows/sendMessage.ts#L114-L119: remove the quote-dismissal rationale comment.
  • apps/meteor/app/utils/client/lib/RestApiClient.ts#L41-L49: remove the mutation-auth rationale comment.
  • apps/meteor/app/utils/client/lib/RestApiClient.ts#L60-L62: remove the inline credential-clearing rationale comment.

As per coding guidelines, “Avoid code comments in the implementation.”

📍 Affects 2 files
  • apps/meteor/client/lib/chats/flows/sendMessage.ts#L53-L56 (this comment)
  • apps/meteor/client/lib/chats/flows/sendMessage.ts#L114-L119
  • apps/meteor/app/utils/client/lib/RestApiClient.ts#L41-L49
  • apps/meteor/app/utils/client/lib/RestApiClient.ts#L60-L62
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/meteor/client/lib/chats/flows/sendMessage.ts` around lines 53 - 56,
Remove the added implementation rationale comments at
apps/meteor/client/lib/chats/flows/sendMessage.ts lines 53-56 and 114-119, and
at apps/meteor/app/utils/client/lib/RestApiClient.ts lines 41-49 and 60-62.
Leave the surrounding implementations unchanged.

Source: Coding guidelines

Messages.state.update(
(record) => record._id === message._id && record.temp === true,
({ temp: _, ...record }) => record,
Expand Down Expand Up @@ -107,8 +111,13 @@ export const sendMessage = async (
}

try {
await process(chat, message, previewUrls, isSlashCommandAllowed);
// Dismiss quoted messages optimistically — they are already baked into
// `message` by composeMessage above, so the composer preview must unmount
// regardless of whether the send request resolves. Keeping it coupled to a
// resolved request leaves the quote stuck in the composer when the REST call
// rejects even though the message was already broadcast over the stream.
chat.composer?.dismissAllQuotedMessages();
await process(chat, message, previewUrls, isSlashCommandAllowed);
await afterSendMessageCallback(message, message.rid);
} catch (error) {
dispatchToastMessage({ type: 'error', message: error });
Expand Down
Original file line number Diff line number Diff line change
@@ -1,28 +1,44 @@
import type { IMessage } from '@rocket.chat/core-typings';
import type { IMessage, IRoom } from '@rocket.chat/core-typings';
import { GenericModal, GenericModalSkeleton } from '@rocket.chat/ui-client';
import { useMethod, useToastMessageDispatch } from '@rocket.chat/ui-contexts';
import { useQuery } from '@tanstack/react-query';
import { useEndpoint, useStream, useToastMessageDispatch } from '@rocket.chat/ui-contexts';
import { useQuery, useQueryClient } from '@tanstack/react-query';
import type { ReactElement } from 'react';
import { useEffect } from 'react';
import { useTranslation } from 'react-i18next';

import ReadReceiptRow from './ReadReceiptRow';
import { mapReadReceiptFromApi } from '../../../../lib/utils/mapReadReceiptFromApi';

export type ReadReceiptsModalProps = {
type ReadReceiptsModalProps = {
messageId: IMessage['_id'];
rid?: IRoom['_id'];
onClose: () => void;
};

const ReadReceiptsModal = ({ messageId, onClose }: ReadReceiptsModalProps) => {
const readReceiptsQueryKey = (messageId: IMessage['_id']) => ['read-receipts', messageId] as const;

const ReadReceiptsModal = ({ messageId, rid, onClose }: ReadReceiptsModalProps): ReactElement => {
const { t } = useTranslation();
const dispatchToastMessage = useToastMessageDispatch();
const queryClient = useQueryClient();
const subscribeToNotifyRoom = useStream('notify-room');

const getReadReceipts = useMethod('getReadReceipts');
const getReadReceipts = useEndpoint('GET', '/v1/chat.getMessageReadReceipts');

const readReceiptsResult = useQuery({
queryKey: ['read-receipts', messageId],
queryFn: () => getReadReceipts({ messageId }),
queryKey: readReceiptsQueryKey(messageId),
queryFn: async () => (await getReadReceipts({ messageId })).receipts.map(mapReadReceiptFromApi),
});

useEffect(() => {
if (!rid) {
return;
}
return subscribeToNotifyRoom(`${rid}/messagesRead`, () => {
queryClient.invalidateQueries({ queryKey: readReceiptsQueryKey(messageId) });
});
}, [rid, messageId, queryClient, subscribeToNotifyRoom]);

useEffect(() => {
if (readReceiptsResult.isError) {
dispatchToastMessage({ type: 'error', message: readReceiptsResult.error });
Expand Down
6 changes: 3 additions & 3 deletions apps/meteor/ee/server/lib/message-read-receipt/ReadReceipt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class ReadReceiptClass {
return;
}

void this.storeReadReceipts(
await this.storeReadReceipts(
() => {
return Messages.findVisibleUnreadMessagesByRoomAndDate(roomId, userLastSeen).toArray();
},
Expand Down Expand Up @@ -80,7 +80,7 @@ class ReadReceiptClass {
}
}

void this.storeReadReceipts(
await this.storeReadReceipts(
() => {
return Promise.resolve([message]);
},
Expand All @@ -101,7 +101,7 @@ class ReadReceiptClass {
return;
}

void this.storeReadReceipts(
await this.storeReadReceipts(
() => {
return Messages.findUnreadThreadMessagesByDate(message.rid, tmid, userId, userLastSeen).toArray();
},
Expand Down
3 changes: 3 additions & 0 deletions apps/meteor/ee/server/meteor-methods/getReadReceipts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { check } from 'meteor/check';
import { Meteor } from 'meteor/meteor';

import { canAccessRoomIdAsync } from '../../../server/lib/authorization/canAccessRoom';
import { methodDeprecationLogger } from '../../../server/lib/deprecationWarningLogger';
import { ReadReceipt } from '../lib/message-read-receipt/ReadReceipt';

declare module '@rocket.chat/ddp-client' {
Expand Down Expand Up @@ -37,6 +38,8 @@ export const getReadReceiptsFunction = async function (messageId: IMessage['_id'

Meteor.methods<ServerMethods>({
async getReadReceipts({ messageId }) {
methodDeprecationLogger.method('getReadReceipts', '9.0.0', '/v1/chat.getMessageReadReceipts');

check(messageId, String);

const uid = Meteor.userId();
Expand Down
22 changes: 22 additions & 0 deletions apps/meteor/server/api/validation/ajv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,28 @@ if (components) {
(mad as Record<string, unknown>).additionalProperties = false;
}

// Lock down the catch-all "plain file" attachment branch. typia emits the
// FileAttachmentProps union as `(type: 'file') & (video | image | audio | base)`;
// the base branch only requires `type: 'file'`, so it also matches image/video/audio
// payloads. That makes the MessageAttachment oneOf ambiguous — a single image
// attachment satisfies both its specific branch and the base branch, violating
// oneOf's "exactly one" rule and failing response validation for any message with a
// file or quoted-file attachment. Reject unknown props on the base branch so only
// genuine plain-file attachments match it.
for (const key in components) {
if (!Object.prototype.hasOwnProperty.call(components, key)) {
continue;
}
const schema = components[key] as { properties?: Record<string, unknown> };
const props = schema?.properties;
const typeEnum = (props?.type as { enum?: unknown[] } | undefined)?.enum;
const isFileBranch = Array.isArray(typeEnum) && typeEnum.length === 1 && typeEnum[0] === 'file';
const hasMediaUrl = !!props && ('image_url' in props || 'video_url' in props || 'audio_url' in props);
if (isFileBranch && !hasMediaUrl) {
(schema as Record<string, unknown>).additionalProperties = false;
}
}

for (const key in components) {
if (Object.prototype.hasOwnProperty.call(components, key)) {
const uri = `#/components/schemas/${key}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Meteor } from 'meteor/meteor';

import { canAccessRoomAsync } from '../../lib/authorization';
import { callbacks } from '../../lib/callbacks';
import { methodDeprecationLogger } from '../../lib/deprecationWarningLogger';
import { readThread } from '../../lib/messaging/threads/functions';
import { settings } from '../../settings';

Expand All @@ -19,6 +20,8 @@ const MAX_LIMIT = 100;

Meteor.methods<ServerMethods>({
async getThreadMessages({ tmid, limit, skip }) {
methodDeprecationLogger.method('getThreadMessages', '9.0.0', '/v1/chat.getThreadMessages');

if ((limit ?? 0) > MAX_LIMIT) {
throw new Meteor.Error('error-not-allowed', `max limit: ${MAX_LIMIT}`, {
method: 'getThreadMessages',
Expand Down
3 changes: 3 additions & 0 deletions apps/meteor/server/meteor-methods/messages/sendMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { RateLimiterClass as RateLimiter } from '../../lib/RateLimiter';
import { canSendMessageAsync } from '../../lib/authorization/canSendMessage';
import { hasPermissionAsync } from '../../lib/authorization/hasPermission';
import { applyAirGappedRestrictionsValidation } from '../../lib/cloud/license/airGappedRestrictionsWrapper';
import { methodDeprecationLogger } from '../../lib/deprecationWarningLogger';
import { i18n } from '../../lib/i18n';
import { SystemLogger } from '../../lib/logger/system';
import { sendMessage } from '../../lib/messages/sendMessage';
Expand Down Expand Up @@ -136,6 +137,8 @@ declare module '@rocket.chat/ddp-client' {

Meteor.methods<ServerMethods>({
async sendMessage(message, previewUrls) {
methodDeprecationLogger.method('sendMessage', '9.0.0', '/v1/chat.sendMessage');

check(message, {
_id: Match.Maybe(String),
rid: Match.Maybe(String),
Expand Down
Loading