From 1de809a8f0b4b542af08127c829dcf2f4ba99f72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cau=C3=AA=20Felchar?= <11652381+cauefcr@users.noreply.github.com> Date: Thu, 25 Aug 2022 11:59:40 -0300 Subject: [PATCH] [FIX] Correct IMAP configuration for email inbox (#25789) Co-authored-by: Murtaza Patrawala <34130764+murtaza98@users.noreply.github.com> Co-authored-by: Kevin Aleman <11577696+KevLehman@users.noreply.github.com> --- apps/meteor/app/api/server/v1/email-inbox.ts | 3 + .../app/models/server/models/LivechatRooms.js | 16 +++- .../views/admin/emailInbox/EmailInboxForm.js | 15 ++- .../rocketchat-i18n/i18n/en.i18n.json | 1 + .../rocketchat-i18n/i18n/pt-BR.i18n.json | 1 + apps/meteor/server/email/IMAPInterceptor.ts | 82 ++++++++++------ .../server/features/EmailInbox/EmailInbox.ts | 14 ++- .../EmailInbox/EmailInbox_Incoming.ts | 14 ++- .../EmailInbox/EmailInbox_Outgoing.ts | 18 ++-- .../end-to-end/api/livechat/11-email-inbox.ts | 93 +++++++++++++++++++ packages/core-typings/src/IEmailInbox.ts | 1 + packages/core-typings/src/IRoom.ts | 2 +- 12 files changed, 207 insertions(+), 53 deletions(-) create mode 100644 apps/meteor/tests/end-to-end/api/livechat/11-email-inbox.ts diff --git a/apps/meteor/app/api/server/v1/email-inbox.ts b/apps/meteor/app/api/server/v1/email-inbox.ts index e94370acf3f3..293ae2e6b8aa 100644 --- a/apps/meteor/app/api/server/v1/email-inbox.ts +++ b/apps/meteor/app/api/server/v1/email-inbox.ts @@ -29,6 +29,7 @@ API.v1.addRoute( if (!hasPermission(this.userId, 'manage-email-inbox')) { throw new Error('error-not-allowed'); } + check(this.bodyParams, { _id: Match.Maybe(String), active: Boolean, @@ -50,6 +51,7 @@ API.v1.addRoute( username: String, password: String, secure: Boolean, + maxRetries: Number, }), }); @@ -126,6 +128,7 @@ API.v1.addRoute( const { email } = this.queryParams; // TODO: Chapter day backend - check if user has permission to view this email inbox instead of null values + // TODO: Chapter day: Remove this endpoint and move search to GET /email-inbox const emailInbox = await EmailInbox.findOne({ email }); return API.v1.success({ emailInbox }); diff --git a/apps/meteor/app/models/server/models/LivechatRooms.js b/apps/meteor/app/models/server/models/LivechatRooms.js index 6d17be960dbf..b0ebd2369e05 100644 --- a/apps/meteor/app/models/server/models/LivechatRooms.js +++ b/apps/meteor/app/models/server/models/LivechatRooms.js @@ -198,7 +198,7 @@ export class LivechatRooms extends Base { const query = { 't': 'l', 'v.token': visitorToken, - 'email.thread': emailThread, + '$or': [{ 'email.thread': { $elemMatch: { $in: emailThread } } }, { 'email.thread': new RegExp(emailThread.join('|')) }], }; return this.findOne(query, options); @@ -208,7 +208,7 @@ export class LivechatRooms extends Base { const query = { 't': 'l', 'v.token': visitorToken, - 'email.thread': emailThread, + '$or': [{ 'email.thread': { $elemMatch: { $in: emailThread } } }, { 'email.thread': new RegExp(emailThread.join('|')) }], ...(departmentId && { departmentId }), }; @@ -220,12 +220,22 @@ export class LivechatRooms extends Base { 't': 'l', 'open': true, 'v.token': visitorToken, - 'email.thread': emailThread, + '$or': [{ 'email.thread': { $elemMatch: { $in: emailThread } } }, { 'email.thread': new RegExp(emailThread.join('|')) }], }; return this.findOne(query, options); } + updateEmailThreadByRoomId(roomId, threadIds) { + const query = { + $addToSet: { + 'email.thread': threadIds, + }, + }; + + return this.update({ _id: roomId }, query); + } + findOneLastServedAndClosedByVisitorToken(visitorToken, options = {}) { const query = { 't': 'l', diff --git a/apps/meteor/client/views/admin/emailInbox/EmailInboxForm.js b/apps/meteor/client/views/admin/emailInbox/EmailInboxForm.js index 47cf1d63df4c..582e3c7de124 100644 --- a/apps/meteor/client/views/admin/emailInbox/EmailInboxForm.js +++ b/apps/meteor/client/views/admin/emailInbox/EmailInboxForm.js @@ -40,6 +40,7 @@ const initialValues = { imapUsername: '', imapPassword: '', imapSecure: false, + imapRetries: 10, }; const getInitialValues = (data) => { @@ -68,6 +69,7 @@ const getInitialValues = (data) => { imapUsername: imap.username ?? '', imapPassword: imap.password ?? '', imapSecure: imap.secure ?? false, + imapRetries: imap.maxRetries ?? 10, }; }; @@ -96,6 +98,7 @@ function EmailInboxForm({ id, data }) { handleImapPort, handleImapUsername, handleImapPassword, + handleImapRetries, handleImapSecure, } = handlers; const { @@ -116,6 +119,7 @@ function EmailInboxForm({ id, data }) { imapPort, imapUsername, imapPassword, + imapRetries, imapSecure, } = values; @@ -174,15 +178,16 @@ function EmailInboxForm({ id, data }) { username: imapUsername, password: imapPassword, secure: imapSecure, + maxRetries: parseInt(imapRetries), }; - const departmentValue = department.value; + const payload = { active, name, email, description, senderInfo, - department: departmentValue, + department: typeof department === 'string' ? department : department.value, smtp, imap, }; @@ -331,6 +336,12 @@ function EmailInboxForm({ id, data }) { + + {t('Max_Retry')}* + + + + {t('Connect_SSL_TLS')} diff --git a/apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json b/apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json index 1b4783f86748..65428c1e721f 100644 --- a/apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json +++ b/apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json @@ -3090,6 +3090,7 @@ "Max_number_of_chats_per_agent": "Max. number of simultaneous chats", "Max_number_of_chats_per_agent_description": "The max. number of simultaneous chats that the agents can attend", "Max_number_of_uses": "Max number of uses", + "Max_Retry": "Maximum attemps to reconnect to the server", "Maximum": "Maximum", "Maximum_number_of_guests_reached": "Maximum number of guests reached", "Me": "Me", diff --git a/apps/meteor/packages/rocketchat-i18n/i18n/pt-BR.i18n.json b/apps/meteor/packages/rocketchat-i18n/i18n/pt-BR.i18n.json index 42b3bd73cc1a..31cbda9faa0d 100644 --- a/apps/meteor/packages/rocketchat-i18n/i18n/pt-BR.i18n.json +++ b/apps/meteor/packages/rocketchat-i18n/i18n/pt-BR.i18n.json @@ -2869,6 +2869,7 @@ "Max_number_of_chats_per_agent": "Número máximo de conversas simultâneas", "Max_number_of_chats_per_agent_description": "Número máximo de conversas simultâneas de que um agente pode participar", "Max_number_of_uses": "Número máximo de usos", + "Max_Retry": "Número máximo de tentativas de conexão com o servidor", "Maximum": "Máximo", "Maximum_number_of_guests_reached": "Número máximo de visitantes atingido", "Me": "Eu", diff --git a/apps/meteor/server/email/IMAPInterceptor.ts b/apps/meteor/server/email/IMAPInterceptor.ts index 5806ab79de87..00646f3ba844 100644 --- a/apps/meteor/server/email/IMAPInterceptor.ts +++ b/apps/meteor/server/email/IMAPInterceptor.ts @@ -5,11 +5,14 @@ import type Connection from 'imap'; import type { ParsedMail } from 'mailparser'; import { simpleParser } from 'mailparser'; +import { logger } from '../features/EmailInbox/logger'; + type IMAPOptions = { deleteAfterRead: boolean; filter: any[]; rejectBeforeTS?: Date; markSeen: boolean; + maxRetries: number; }; export declare interface IMAPInterceptor { @@ -19,30 +22,42 @@ export declare interface IMAPInterceptor { export class IMAPInterceptor extends EventEmitter { private imap: IMAP; - private options: IMAPOptions; + private config: IMAP.Config; + + private initialBackoffDurationMS = 30000; + + private backoff: NodeJS.Timeout; - constructor(imapConfig: IMAP.Config, options?: Partial) { + private retries = 0; + + constructor( + imapConfig: IMAP.Config, + private options: IMAPOptions = { + deleteAfterRead: false, + filter: ['UNSEEN'], + markSeen: true, + maxRetries: 10, + }, + ) { super(); + this.config = imapConfig; + this.imap = new IMAP({ - connTimeout: 30000, + connTimeout: 10000, keepalive: true, ...(imapConfig.tls && { tlsOptions: { servername: imapConfig.host } }), ...imapConfig, }); - this.options = { - deleteAfterRead: false, - filter: ['UNSEEN'], - markSeen: true, - ...options, - }; - // On successfully connected. this.imap.on('ready', () => { if (this.imap.state !== 'disconnected') { + clearTimeout(this.backoff); + this.retries = 0; this.openInbox((err) => { if (err) { + logger.error(`Error occurred during imap on inbox ${this.config.user}: `, err); throw err; } // fetch new emails & wait [IDLE] @@ -54,19 +69,20 @@ export class IMAPInterceptor extends EventEmitter { }); }); } else { - this.log('IMAP did not connected.'); + logger.error(`IMAP did not connect on inbox ${this.config.user}`); this.imap.end(); + this.reconnect(); } }); this.imap.on('error', (err: Error) => { - this.log('Error occurred ...', err); - throw err; + logger.error(`Error occurred on inbox ${this.config.user}: `, err); + this.stop(() => this.reconnect()); }); - } - log(...msg: any[]): void { - console.log(...msg); + this.imap.on('close', () => { + this.reconnect(); + }); } openInbox(cb: (error: Error, mailbox: Connection.Box) => void): void { @@ -86,30 +102,37 @@ export class IMAPInterceptor extends EventEmitter { } stop(callback = new Function()): void { - this.log('IMAP stop called'); + logger.info('IMAP stop called'); this.imap.end(); this.imap.once('end', () => { - this.log('IMAP stopped'); + logger.info('IMAP stopped'); callback?.(); }); - callback?.(); } - restart(): void { - this.stop(() => { - this.log('Restarting IMAP ....'); + reconnect(): void { + const loop = (): void => { this.start(); - }); + if (this.retries < this.options.maxRetries) { + this.retries += 1; + this.initialBackoffDurationMS *= 2; + this.backoff = setTimeout(loop, this.initialBackoffDurationMS); + } else { + logger.error(`IMAP reconnection failed on inbox ${this.config.user}`); + clearTimeout(this.backoff); + } + }; + this.backoff = setTimeout(loop, this.initialBackoffDurationMS); } // Fetch all UNSEEN messages and pass them for further processing getEmails(): void { this.imap.search(this.options.filter, (err, newEmails) => { + logger.debug(`IMAP search on inbox ${this.config.user} returned ${newEmails.length} new emails: `, newEmails); if (err) { - this.log(err); + logger.error(err); throw err; } - // newEmails => array containing serials of unseen messages if (newEmails.length > 0) { const fetch = this.imap.fetch(newEmails, { @@ -119,6 +142,8 @@ export class IMAPInterceptor extends EventEmitter { }); fetch.on('message', (msg, seqno) => { + logger.debug('E-mail received', seqno, msg); + msg.on('body', (stream, type) => { if (type.which !== '') { return; @@ -126,10 +151,9 @@ export class IMAPInterceptor extends EventEmitter { simpleParser(stream, (_err, email) => { if (this.options.rejectBeforeTS && email.date && email.date < this.options.rejectBeforeTS) { - this.log('Rejecting email', email.subject); + logger.error(`Rejecting email on inbox ${this.config.user}`, email.subject); return; } - this.emit('email', email); }); }); @@ -140,7 +164,7 @@ export class IMAPInterceptor extends EventEmitter { if (this.options.deleteAfterRead) { this.imap.seq.addFlags(seqno, 'Deleted', (err) => { if (err) { - this.log(`Mark deleted error: ${err}`); + logger.error(`Mark deleted error: ${err}`); } }); } @@ -148,7 +172,7 @@ export class IMAPInterceptor extends EventEmitter { }); fetch.once('error', (err) => { - this.log(`Fetch error: ${err}`); + logger.error(`Fetch error: ${err}`); }); } }); diff --git a/apps/meteor/server/features/EmailInbox/EmailInbox.ts b/apps/meteor/server/features/EmailInbox/EmailInbox.ts index 12e122a56ba2..739888aa29dc 100644 --- a/apps/meteor/server/features/EmailInbox/EmailInbox.ts +++ b/apps/meteor/server/features/EmailInbox/EmailInbox.ts @@ -38,17 +38,21 @@ export async function configureEmailInboxes(): Promise { user: emailInboxRecord.imap.username, host: emailInboxRecord.imap.server, port: emailInboxRecord.imap.port, - tls: emailInboxRecord.imap.secure, - tlsOptions: { - rejectUnauthorized: false, - }, - // debug: (...args: any[]): void => logger.debug(args), + ...(emailInboxRecord.imap.secure + ? { + tls: emailInboxRecord.imap.secure, + tlsOptions: { + rejectUnauthorized: false, + }, + } + : {}), }, { deleteAfterRead: false, filter: [['UNSEEN'], ['SINCE', emailInboxRecord._updatedAt]], rejectBeforeTS: emailInboxRecord._updatedAt, markSeen: true, + maxRetries: emailInboxRecord.imap.maxRetries, }, ); diff --git a/apps/meteor/server/features/EmailInbox/EmailInbox_Incoming.ts b/apps/meteor/server/features/EmailInbox/EmailInbox_Incoming.ts index f2bad29a632b..4105b301b45d 100644 --- a/apps/meteor/server/features/EmailInbox/EmailInbox_Incoming.ts +++ b/apps/meteor/server/features/EmailInbox/EmailInbox_Incoming.ts @@ -2,7 +2,7 @@ import stripHtml from 'string-strip-html'; import { Random } from 'meteor/random'; import type { ParsedMail, Attachment } from 'mailparser'; import { TAPi18n } from 'meteor/rocketchat:tap-i18n'; -import type { ILivechatVisitor } from '@rocket.chat/core-typings'; +import type { ILivechatVisitor, IOmnichannelRoom } from '@rocket.chat/core-typings'; import { OmnichannelSourceType } from '@rocket.chat/core-typings'; import { LivechatVisitors } from '@rocket.chat/models'; @@ -127,14 +127,16 @@ async function uploadAttachment(attachment: Attachment, rid: string, visitorToke } export async function onEmailReceived(email: ParsedMail, inbox: string, department = ''): Promise { - logger.debug(`New email conversation received on inbox ${inbox}. Will be assigned to department ${department}`); + logger.debug(`New email conversation received on inbox ${inbox}. Will be assigned to department ${department}`, email); if (!email.from?.value?.[0]?.address) { return; } const references = typeof email.references === 'string' ? [email.references] : email.references; + const initialRef = [email.messageId, email.inReplyTo].filter(Boolean) as string[]; + const thread = (references?.length ? references : []).flatMap((t: string) => t.split(',')).concat(initialRef); - const thread = references?.[0] ?? email.messageId; + logger.debug(`Received new email conversation with thread ${thread} on inbox ${inbox} from ${email.from.value[0].address}`); logger.debug(`Fetching guest for visitor ${email.from.value[0].address}`); const guest = await getGuestByEmail(email.from.value[0].address, email.from.value[0].name, department); @@ -146,7 +148,7 @@ export async function onEmailReceived(email: ParsedMail, inbox: string, departme logger.debug(`Guest ${guest._id} obtained. Attempting to find or create a room on department ${department}`); - let room = LivechatRooms.findOneByVisitorTokenAndEmailThreadAndDepartment(guest.token, thread, department, {}); + let room: IOmnichannelRoom = LivechatRooms.findOneByVisitorTokenAndEmailThreadAndDepartment(guest.token, thread, department, {}); logger.debug({ msg: 'Room found for guest', @@ -170,6 +172,7 @@ export async function onEmailReceived(email: ParsedMail, inbox: string, departme const msgId = Random.id(); logger.debug(`Sending email message to room ${rid} for visitor ${guest._id}. Conversation assigned to department ${department}`); + Livechat.sendMessage({ guest, message: { @@ -211,7 +214,7 @@ export async function onEmailReceived(email: ParsedMail, inbox: string, departme ], rid, email: { - references, + thread, messageId: email.messageId, }, }, @@ -258,6 +261,7 @@ export async function onEmailReceived(email: ParsedMail, inbox: string, departme }, }, ); + LivechatRooms.updateEmailThreadByRoomId(room._id, thread); }) .catch((err) => { Livechat.logger.error({ diff --git a/apps/meteor/server/features/EmailInbox/EmailInbox_Outgoing.ts b/apps/meteor/server/features/EmailInbox/EmailInbox_Outgoing.ts index 97a2e711531d..67088c5a2f31 100644 --- a/apps/meteor/server/features/EmailInbox/EmailInbox_Outgoing.ts +++ b/apps/meteor/server/features/EmailInbox/EmailInbox_Outgoing.ts @@ -7,11 +7,12 @@ import { Uploads } from '@rocket.chat/models'; import { callbacks } from '../../../lib/callbacks'; import { FileUpload } from '../../../app/file-upload/server'; import { slashCommands } from '../../../app/utils/server'; -import { Messages, Rooms, Users } from '../../../app/models/server'; +import { Messages, Rooms, Users, LivechatRooms } from '../../../app/models/server'; import type { Inbox } from './EmailInbox'; import { inboxes } from './EmailInbox'; import { sendMessage } from '../../../app/lib/server/functions/sendMessage'; import { settings } from '../../../app/settings/server'; +import { logger } from './logger'; const livechatQuoteRegExp = /^\[\s\]\(https?:\/\/.+\/live\/.+\?msg=(?.+?)\)\s(?.+)/s; @@ -36,8 +37,8 @@ const sendErrorReplyMessage = (error: string, options: any): void => { sendMessage(user, message, { _id: options.rid }); }; -function sendEmail(inbox: Inbox, mail: Mail.Options, options?: any): void { - inbox.smtp +async function sendEmail(inbox: Inbox, mail: Mail.Options, options?: any): Promise<{ messageId: string }> { + return inbox.smtp .sendMail({ from: inbox.config.senderInfo ? { @@ -48,10 +49,11 @@ function sendEmail(inbox: Inbox, mail: Mail.Options, options?: any): void { ...mail, }) .then((info) => { - console.log('Message sent: %s', info.messageId); + logger.info('Message sent: %s', info.messageId); + return info; }) .catch((error) => { - console.log('Error sending Email reply: %s', error.message); + logger.error('Error sending Email reply: %s', error.message); if (!options?.msgId) { return; @@ -116,7 +118,7 @@ slashCommands.add({ sender: message.u.username, rid: message.rid, }, - ); + ).then((info) => LivechatRooms.updateEmailThreadByRoomId(room._id, info.messageId)); }); Messages.update( @@ -216,7 +218,7 @@ callbacks.add( sender: message.u.username, rid: room._id, }, - ); + ).then((info) => LivechatRooms.updateEmailThreadByRoomId(room._id, info.messageId)); message.msg = match.groups.text; @@ -269,7 +271,7 @@ export async function sendTestEmailToInbox(emailInboxRecord: IEmailInbox, user: throw new Error('user-without-verified-email'); } - console.log(`Sending testing email to ${address}`); + logger.info(`Sending testing email to ${address}`); sendEmail(inbox, { to: address, subject: 'Test of inbox configuration', diff --git a/apps/meteor/tests/end-to-end/api/livechat/11-email-inbox.ts b/apps/meteor/tests/end-to-end/api/livechat/11-email-inbox.ts new file mode 100644 index 000000000000..2cb4e6e3db49 --- /dev/null +++ b/apps/meteor/tests/end-to-end/api/livechat/11-email-inbox.ts @@ -0,0 +1,93 @@ +import type { IEmailInbox } from '@rocket.chat/core-typings'; +import { expect } from 'chai'; +import type { Response } from 'supertest'; + +import { getCredentials, api, request, credentials } from '../../../data/api-data'; +import { createDepartment } from '../../../data/livechat/rooms'; + +// TODO: Add tests with actual e-mail servers involved + +describe('Email inbox', () => { + before((done) => getCredentials(done)); + let testInbox = ''; + before((done) => { + createDepartment() + .then((dept) => + request + .post(api('email-inbox')) + .set(credentials) + .send({ + active: true, + name: 'test-email-inbox##', + email: 'test-email@example.com', + description: 'test email inbox', + senderInfo: 'test email inbox', + department: dept.name, + smtp: { + server: 'smtp.example.com', + port: 587, + username: 'example@example.com', + password: 'not-a-real-password', + secure: true, + }, + imap: { + server: 'imap.example.com', + port: 993, + username: 'example@example.com', + password: 'not-a-real-password', + secure: true, + maxRetries: 10, + }, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res: Response) => { + expect(res.body).to.have.property('success'); + if (res.body.success === true) { + testInbox = res.body._id; + } else { + expect(res.body).to.have.property('error'); + expect(res.body.error.includes('E11000')).to.be.eq(true); + } + }), + ) + .finally(done); + }); + after((done) => { + if (testInbox) { + request + .delete(api(`email-inbox/${testInbox}`)) + .set(credentials) + .send() + .expect(200) + .end(() => done()); + return; + } + done(); + }); + describe('GET email-inbox.list', () => { + it('should return a list of email inboxes', (done) => { + request + .get(api('email-inbox.list')) + .set(credentials) + .send() + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res: Response) => { + expect(res.body).to.have.property('emailInboxes'); + expect(res.body.emailInboxes).to.be.an('array'); + expect(res.body.emailInboxes).to.have.length.of.at.least(1); + expect(res.body.emailInboxes.filter((ibx: IEmailInbox) => ibx.email === 'test-email@example.com')).to.have.length.gte(1); + // make sure we delete the test inbox, even if creation failed on this test run + testInbox = res.body.emailInboxes.filter((ibx: IEmailInbox) => ibx.email === 'test-email@example.com')[0]._id; + expect(res.body).to.have.property('total'); + expect(res.body.total).to.be.a('number'); + expect(res.body).to.have.property('count'); + expect(res.body.count).to.be.a('number'); + expect(res.body).to.have.property('offset'); + expect(res.body.offset).to.be.a('number'); + }) + .end(() => done()); + }); + }); +}); diff --git a/packages/core-typings/src/IEmailInbox.ts b/packages/core-typings/src/IEmailInbox.ts index 2f0ff390297a..5dbd7394ead7 100644 --- a/packages/core-typings/src/IEmailInbox.ts +++ b/packages/core-typings/src/IEmailInbox.ts @@ -19,6 +19,7 @@ export interface IEmailInbox { username: string; password: string; secure: boolean; + maxRetries: number; }; _createdAt: Date; _createdBy: { diff --git a/packages/core-typings/src/IRoom.ts b/packages/core-typings/src/IRoom.ts index 1c025318ff21..f80b5c95cb77 100644 --- a/packages/core-typings/src/IRoom.ts +++ b/packages/core-typings/src/IRoom.ts @@ -138,7 +138,7 @@ export interface IOmnichannelGenericRoom extends Omit