From 163e55b1ebddf771f51cedeb05ab96e99a6e5957 Mon Sep 17 00:00:00 2001 From: "Julio A." <52619625+julio-cfa@users.noreply.github.com> Date: Fri, 15 Nov 2024 06:07:01 +0100 Subject: [PATCH] fix: imported fixes (#33941) --- apps/meteor/app/api/server/v1/chat.ts | 15 ++ apps/meteor/app/api/server/v1/import.ts | 1 + apps/meteor/app/api/server/v1/rooms.ts | 7 +- .../server/methods/createDiscussion.ts | 8 + .../federation/server/endpoints/dispatch.js | 13 ++ .../server/endpoints/requestFromLatest.js | 13 ++ .../federation/server/endpoints/uploads.js | 13 ++ .../app/federation/server/endpoints/users.js | 25 +++ .../app/lib/server/methods/sendMessage.ts | 21 ++- apps/meteor/tests/end-to-end/api/chat.ts | 173 ++++++++++++++++++ .../tests/end-to-end/api/import.spec.ts | 32 ++++ apps/meteor/tests/end-to-end/api/methods.ts | 23 +++ 12 files changed, 341 insertions(+), 3 deletions(-) diff --git a/apps/meteor/app/api/server/v1/chat.ts b/apps/meteor/app/api/server/v1/chat.ts index b5cfd9e46ce..fdcd97e66c7 100644 --- a/apps/meteor/app/api/server/v1/chat.ts +++ b/apps/meteor/app/api/server/v1/chat.ts @@ -161,6 +161,21 @@ API.v1.addRoute( { authRequired: true }, { async post() { + const { text, attachments } = this.bodyParams; + const maxAllowedSize = settings.get('Message_MaxAllowedSize') ?? 0; + + if (text && text.length > maxAllowedSize) { + return API.v1.failure('error-message-size-exceeded'); + } + + if (attachments && attachments.length > 0) { + for (const attachment of attachments) { + if (attachment.text && attachment.text.length > maxAllowedSize) { + return API.v1.failure('error-message-size-exceeded'); + } + } + } + const messageReturn = (await applyAirGappedRestrictionsValidation(() => processWebhookMessage(this.bodyParams, this.user)))[0]; if (!messageReturn) { diff --git a/apps/meteor/app/api/server/v1/import.ts b/apps/meteor/app/api/server/v1/import.ts index d0557a81969..6104076c4fb 100644 --- a/apps/meteor/app/api/server/v1/import.ts +++ b/apps/meteor/app/api/server/v1/import.ts @@ -33,6 +33,7 @@ API.v1.addRoute( { authRequired: true, validateParams: isUploadImportFileParamsPOST, + permissionsRequired: ['run-import'], }, { async post() { diff --git a/apps/meteor/app/api/server/v1/rooms.ts b/apps/meteor/app/api/server/v1/rooms.ts index 355cce24d40..4a746f60ad6 100644 --- a/apps/meteor/app/api/server/v1/rooms.ts +++ b/apps/meteor/app/api/server/v1/rooms.ts @@ -459,9 +459,14 @@ API.v1.addRoute( }, ); +/* +TO-DO: 8.0.0 should use the ajv validation +which will change this endpoint's +response errors. +*/ API.v1.addRoute( 'rooms.createDiscussion', - { authRequired: true }, + { authRequired: true /* , validateParams: isRoomsCreateDiscussionProps */ }, { async post() { // eslint-disable-next-line @typescript-eslint/naming-convention diff --git a/apps/meteor/app/discussion/server/methods/createDiscussion.ts b/apps/meteor/app/discussion/server/methods/createDiscussion.ts index f2b4533489d..c4b8fb8e9a2 100644 --- a/apps/meteor/app/discussion/server/methods/createDiscussion.ts +++ b/apps/meteor/app/discussion/server/methods/createDiscussion.ts @@ -3,6 +3,7 @@ import type { IMessage, IRoom, IUser, MessageAttachmentDefault } from '@rocket.c import type { ServerMethods } from '@rocket.chat/ddp-client'; import { Messages, Rooms, Users } from '@rocket.chat/models'; import { Random } from '@rocket.chat/random'; +import { check, Match } from 'meteor/check'; import { Meteor } from 'meteor/meteor'; import { i18n } from '../../../../server/lib/i18n'; @@ -246,6 +247,13 @@ Meteor.methods({ * @param {boolean} encrypted - if the discussion's e2e encryption should be enabled. */ async createDiscussion({ prid, pmid, t_name: discussionName, reply, users, encrypted }: CreateDiscussionProperties) { + check(prid, Match.Maybe(String)); + check(pmid, Match.Maybe(String)); + check(reply, Match.Maybe(String)); + check(discussionName, String); + check(users, [String]); + check(encrypted, Match.Maybe(Boolean)); + const uid = Meteor.userId(); if (!uid) { throw new Meteor.Error('error-invalid-user', 'Invalid user', { diff --git a/apps/meteor/app/federation/server/endpoints/dispatch.js b/apps/meteor/app/federation/server/endpoints/dispatch.js index 4f2a197b25e..d80f74bf18d 100644 --- a/apps/meteor/app/federation/server/endpoints/dispatch.js +++ b/apps/meteor/app/federation/server/endpoints/dispatch.js @@ -6,6 +6,7 @@ import EJSON from 'ejson'; import { API } from '../../../api/server'; import { FileUpload } from '../../../file-upload/server'; import { deleteRoom } from '../../../lib/server/functions/deleteRoom'; +import { apiDeprecationLogger } from '../../../lib/server/lib/deprecationWarningLogger'; import { notifyOnMessageChange, notifyOnRoomChanged, @@ -551,6 +552,18 @@ API.v1.addRoute( { authRequired: false, rateLimiterOptions: { numRequestsAllowed: 30, intervalTimeInMS: 1000 } }, { async post() { + /* + The legacy federation has been deprecated for over a year + and no longer receives any updates. This feature also has + relevant security issues that weren't addressed. + Workspaces should migrate to the newer matrix federation. + */ + apiDeprecationLogger.endpoint(this.request.route, '8.0.0', this.response, 'Use Matrix Federation instead.'); + + if (!process.env.ENABLE_INSECURE_LEGACY_FEDERATION) { + return API.v1.failure('Deprecated. ENABLE_INSECURE_LEGACY_FEDERATION environment variable is needed to enable it.'); + } + if (!isFederationEnabled()) { return API.v1.failure('Federation not enabled'); } diff --git a/apps/meteor/app/federation/server/endpoints/requestFromLatest.js b/apps/meteor/app/federation/server/endpoints/requestFromLatest.js index b45e8944e46..8dca41d3f44 100644 --- a/apps/meteor/app/federation/server/endpoints/requestFromLatest.js +++ b/apps/meteor/app/federation/server/endpoints/requestFromLatest.js @@ -2,6 +2,7 @@ import { FederationRoomEvents } from '@rocket.chat/models'; import EJSON from 'ejson'; import { API } from '../../../api/server'; +import { apiDeprecationLogger } from '../../../lib/server/lib/deprecationWarningLogger'; import { dispatchEvents } from '../handler'; import { decryptIfNeeded } from '../lib/crypt'; import { isFederationEnabled } from '../lib/isFederationEnabled'; @@ -12,6 +13,18 @@ API.v1.addRoute( { authRequired: false }, { async post() { + /* + The legacy federation has been deprecated for over a year + and no longer receives any updates. This feature also has + relevant security issues that weren't addressed. + Workspaces should migrate to the newer matrix federation. + */ + apiDeprecationLogger.endpoint(this.request.route, '8.0.0', this.response, 'Use Matrix Federation instead.'); + + if (!process.env.ENABLE_INSECURE_LEGACY_FEDERATION) { + return API.v1.failure('Deprecated. ENABLE_INSECURE_LEGACY_FEDERATION environment variable is needed to enable it.'); + } + if (!isFederationEnabled()) { return API.v1.failure('Federation not enabled'); } diff --git a/apps/meteor/app/federation/server/endpoints/uploads.js b/apps/meteor/app/federation/server/endpoints/uploads.js index da5740f95f1..10c513b3741 100644 --- a/apps/meteor/app/federation/server/endpoints/uploads.js +++ b/apps/meteor/app/federation/server/endpoints/uploads.js @@ -2,6 +2,7 @@ import { Uploads } from '@rocket.chat/models'; import { API } from '../../../api/server'; import { FileUpload } from '../../../file-upload/server'; +import { apiDeprecationLogger } from '../../../lib/server/lib/deprecationWarningLogger'; import { isFederationEnabled } from '../lib/isFederationEnabled'; API.v1.addRoute( @@ -9,6 +10,18 @@ API.v1.addRoute( { authRequired: false }, { async get() { + /* + The legacy federation has been deprecated for over a year + and no longer receives any updates. This feature also has + relevant security issues that weren't addressed. + Workspaces should migrate to the newer matrix federation. + */ + apiDeprecationLogger.endpoint(this.request.route, '8.0.0', this.response, 'Use Matrix Federation instead.'); + + if (!process.env.ENABLE_INSECURE_LEGACY_FEDERATION) { + return API.v1.failure('Deprecated. ENABLE_INSECURE_LEGACY_FEDERATION environment variable is needed to enable it.'); + } + if (!isFederationEnabled()) { return API.v1.failure('Federation not enabled'); } diff --git a/apps/meteor/app/federation/server/endpoints/users.js b/apps/meteor/app/federation/server/endpoints/users.js index 0d9a4ff4b0c..fba9b1642e8 100644 --- a/apps/meteor/app/federation/server/endpoints/users.js +++ b/apps/meteor/app/federation/server/endpoints/users.js @@ -1,6 +1,7 @@ import { Users } from '@rocket.chat/models'; import { API } from '../../../api/server'; +import { apiDeprecationLogger } from '../../../lib/server/lib/deprecationWarningLogger'; import { isFederationEnabled } from '../lib/isFederationEnabled'; import { serverLogger } from '../lib/logger'; import { normalizers } from '../normalizers'; @@ -12,6 +13,18 @@ API.v1.addRoute( { authRequired: false }, { async get() { + /* + The legacy federation has been deprecated for over a year + and no longer receives any updates. This feature also has + relevant security issues that weren't addressed. + Workspaces should migrate to the newer matrix federation. + */ + apiDeprecationLogger.endpoint(this.request.route, '8.0.0', this.response, 'Use Matrix Federation instead.'); + + if (!process.env.ENABLE_INSECURE_LEGACY_FEDERATION) { + return API.v1.failure('Deprecated. ENABLE_INSECURE_LEGACY_FEDERATION environment variable is needed to enable it.'); + } + if (!isFederationEnabled()) { return API.v1.failure('Federation not enabled'); } @@ -39,6 +52,18 @@ API.v1.addRoute( { authRequired: false }, { async get() { + /* + The legacy federation has been deprecated for over a year + and no longer receives any updates. This feature also has + relevant security issues that weren't addressed. + Workspaces should migrate to the newer matrix federation. + */ + apiDeprecationLogger.endpoint(this.request.route, '8.0.0', this.response, 'Use Matrix Federation instead.'); + + if (!process.env.ENABLE_INSECURE_LEGACY_FEDERATION) { + return API.v1.failure('Deprecated. ENABLE_INSECURE_LEGACY_FEDERATION environment variable is needed to enable it.'); + } + if (!isFederationEnabled()) { return API.v1.failure('Federation not enabled'); } diff --git a/apps/meteor/app/lib/server/methods/sendMessage.ts b/apps/meteor/app/lib/server/methods/sendMessage.ts index 76134c81d0b..6ab03df337e 100644 --- a/apps/meteor/app/lib/server/methods/sendMessage.ts +++ b/apps/meteor/app/lib/server/methods/sendMessage.ts @@ -4,7 +4,7 @@ import type { ServerMethods } from '@rocket.chat/ddp-client'; import type { RocketchatI18nKeys } from '@rocket.chat/i18n'; import { Messages, Users } from '@rocket.chat/models'; import type { TOptions } from 'i18next'; -import { check } from 'meteor/check'; +import { check, Match } from 'meteor/check'; import { Meteor } from 'meteor/meteor'; import moment from 'moment'; @@ -123,7 +123,24 @@ declare module '@rocket.chat/ddp-client' { Meteor.methods({ async sendMessage(message, previewUrls) { - check(message, Object); + check(message, { + _id: Match.Maybe(String), + rid: Match.Maybe(String), + msg: Match.Maybe(String), + tmid: Match.Maybe(String), + tshow: Match.Maybe(Boolean), + ts: Match.Maybe(Date), + t: Match.Maybe(String), + otrAck: Match.Maybe(String), + bot: Match.Maybe(Boolean), + content: Match.Maybe(Object), + e2e: Match.Maybe(String), + e2eMentions: Match.Maybe(Object), + customFields: Match.Maybe(Object), + federation: Match.Maybe(Object), + groupable: Match.Maybe(Boolean), + sentByEmail: Match.Maybe(Boolean), + }); const uid = Meteor.userId(); if (!uid) { diff --git a/apps/meteor/tests/end-to-end/api/chat.ts b/apps/meteor/tests/end-to-end/api/chat.ts index cea8018b90e..d7c640fc10f 100644 --- a/apps/meteor/tests/end-to-end/api/chat.ts +++ b/apps/meteor/tests/end-to-end/api/chat.ts @@ -515,6 +515,179 @@ describe('[Chat]', () => { }) .end(done); }); + + describe('text message allowed size', () => { + before(async () => { + await updateSetting('Message_MaxAllowedSize', 10); + }); + + after(async () => { + await updateSetting('Message_MaxAllowedSize', 5000); + }); + + it('should return an error if text parameter surpasses the maximum allowed size', (done) => { + void request + .post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: '#general', + text: 'Text to test max limit allowed', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'http://res.guggy.com/logo_128.png', + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'error-message-size-exceeded'); + }) + .end(done); + }); + + it('should return an error if text parameter in the first attachment surpasses the maximum allowed size', (done) => { + void request + .post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: testChannel.name, + text: 'Yay!', + emoji: ':smirk:', + attachments: [ + { + color: '#ff0000', + text: 'Text to test max limit allowed', + ts: '2016-12-09T16:53:06.761Z', + thumb_url: 'http://res.guggy.com/logo_128.png', + message_link: 'https://google.com', + collapsed: false, + author_name: 'Bradley Hilton', + author_link: 'https://rocket.chat/', + author_icon: 'https://avatars.githubusercontent.com/u/850391?v=3', + title: 'Attachment Example', + title_link: 'https://youtube.com', + title_link_download: true, + image_url: 'http://res.guggy.com/logo_128.png', + audio_url: 'http://www.w3schools.com/tags/horse.mp3', + video_url: 'http://www.w3schools.com/tags/movie.mp4', + fields: [], + }, + ], + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'error-message-size-exceeded'); + }) + .end(done); + }); + + it('should return an error if text parameter in any of the attachments surpasses the maximum allowed size', (done) => { + void request + .post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: testChannel.name, + text: 'Yay!', + emoji: ':smirk:', + attachments: [ + { + color: '#ff0000', + text: 'Yay!', + ts: '2016-12-09T16:53:06.761Z', + thumb_url: 'http://res.guggy.com/logo_128.png', + message_link: 'https://google.com', + collapsed: false, + author_name: 'Bradley Hilton', + author_link: 'https://rocket.chat/', + author_icon: 'https://avatars.githubusercontent.com/u/850391?v=3', + title: 'Attachment Example', + title_link: 'https://youtube.com', + title_link_download: true, + image_url: 'http://res.guggy.com/logo_128.png', + audio_url: 'http://www.w3schools.com/tags/horse.mp3', + video_url: 'http://www.w3schools.com/tags/movie.mp4', + fields: [], + }, + { + color: '#ff0000', + text: 'Text to large to test max limit allowed', + ts: '2016-12-09T16:53:06.761Z', + thumb_url: 'http://res.guggy.com/logo_128.png', + message_link: 'https://google.com', + collapsed: false, + author_name: 'Bradley Hilton', + author_link: 'https://rocket.chat/', + author_icon: 'https://avatars.githubusercontent.com/u/850391?v=3', + title: 'Attachment Example', + title_link: 'https://youtube.com', + title_link_download: true, + image_url: 'http://res.guggy.com/logo_128.png', + audio_url: 'http://www.w3schools.com/tags/horse.mp3', + video_url: 'http://www.w3schools.com/tags/movie.mp4', + fields: [], + }, + ], + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'error-message-size-exceeded'); + }) + .end(done); + }); + + it('should pass if any text parameter length does not surpasses the maximum allowed size', (done) => { + void request + .post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: testChannel.name, + text: 'Sample', + emoji: ':smirk:', + attachments: [ + { + color: '#ff0000', + text: 'Sample', + ts: '2016-12-09T16:53:06.761Z', + thumb_url: 'http://res.guggy.com/logo_128.png', + message_link: 'https://google.com', + collapsed: false, + author_name: 'Bradley Hilton', + author_link: 'https://rocket.chat/', + author_icon: 'https://avatars.githubusercontent.com/u/850391?v=3', + title: 'Attachment Example', + title_link: 'https://youtube.com', + title_link_download: true, + image_url: 'http://res.guggy.com/logo_128.png', + audio_url: 'http://www.w3schools.com/tags/horse.mp3', + video_url: 'http://www.w3schools.com/tags/movie.mp4', + fields: [ + { + short: true, + title: 'Test', + value: 'Testing out something or other', + }, + { + short: true, + title: 'Another Test', + value: '[Link](https://google.com/) something and this and that.', + }, + ], + }, + ], + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.nested.property('message.msg', 'Sample'); + }) + .end(done); + }); + }); }); describe('/chat.getMessage', () => { diff --git a/apps/meteor/tests/end-to-end/api/import.spec.ts b/apps/meteor/tests/end-to-end/api/import.spec.ts index a3db33efff0..90019ffdba5 100644 --- a/apps/meteor/tests/end-to-end/api/import.spec.ts +++ b/apps/meteor/tests/end-to-end/api/import.spec.ts @@ -111,4 +111,36 @@ describe('Imports', () => { }); }); }); + + describe('[/uploadImportFile]', () => { + let testUser: any = {}; + before(async () => { + testUser = await createUser(); + }); + let testCredentials: any = {}; + before(async () => { + testCredentials = await login(testUser.username, password); + }); + after(async () => { + await deleteUser(testUser); + testUser = undefined; + }); + + it('should fail if the user is not authorized', async () => { + await request + .post(api('uploadImportFile')) + .set(testCredentials) + .send({ + binaryContent: 'ZXJzLmNzdlBLBQYAAAAAAQABADcAAAAmAQAAAAA=', + contentType: 'application/zip', + fileName: 'users11.zip', + importerKey: 'csv', + }) + .expect(403) + .expect((res: Response) => { + expect(res.body.success).to.be.false; + expect(res.body.error).to.equal('User does not have the permissions required for this action [error-unauthorized]'); + }); + }); + }); }); diff --git a/apps/meteor/tests/end-to-end/api/methods.ts b/apps/meteor/tests/end-to-end/api/methods.ts index 9e05fba88b4..e3e8a193d79 100644 --- a/apps/meteor/tests/end-to-end/api/methods.ts +++ b/apps/meteor/tests/end-to-end/api/methods.ts @@ -2059,6 +2059,29 @@ describe('Meteor.methods', () => { expect(res.body).to.have.property('success', false); }); }); + + it('should return an error if request includes unallowed parameters', (done) => { + void request + .post(methodCall('sendMessage')) + .set(credentials) + .send({ + message: JSON.stringify({ + method: 'sendMessage', + params: [{ _id: `${Date.now() + Math.random()}`, rid, msg: 'test message', _notAllowed: '1' }], + id: 1000, + msg: 'method', + }), + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + const data = JSON.parse(res.body.message); + expect(data).to.have.a.property('error').that.is.an('object'); + expect(data.error.sanitizedError).to.have.a.property('reason', 'Match failed'); + }) + .end(done); + }); }); describe('[@updateMessage]', () => {