diff --git a/.changeset/empty-readers-teach.md b/.changeset/empty-readers-teach.md new file mode 100644 index 000000000000..b4bd075ef654 --- /dev/null +++ b/.changeset/empty-readers-teach.md @@ -0,0 +1,8 @@ +--- +"@rocket.chat/meteor": patch +"@rocket.chat/tools": patch +"@rocket.chat/account-service": patch +--- + +Fixed an inconsistent evaluation of the `Accounts_LoginExpiration` setting over the codebase. In some places, it was being used as milliseconds while in others as days. Invalid values produced different results. A helper function was created to centralize the setting validation and the proper value being returned to avoid edge cases. +Negative values may be saved on the settings UI panel but the code will interpret any negative, NaN or 0 value to the default expiration which is 90 days. diff --git a/.changeset/grumpy-worms-appear.md b/.changeset/grumpy-worms-appear.md new file mode 100644 index 000000000000..fb9fab77b24c --- /dev/null +++ b/.changeset/grumpy-worms-appear.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/i18n": patch +--- + +Fixed wrong wording on a federation setting diff --git a/.changeset/happy-peaches-nail.md b/.changeset/happy-peaches-nail.md new file mode 100644 index 000000000000..2dfb2151ced0 --- /dev/null +++ b/.changeset/happy-peaches-nail.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": patch +--- + +Fixed issue with livechat agents not being able to leave omnichannel rooms if joining after a room has been closed by the visitor (due to race conditions) diff --git a/.changeset/sour-forks-breathe.md b/.changeset/sour-forks-breathe.md new file mode 100644 index 000000000000..2d1076845fa9 --- /dev/null +++ b/.changeset/sour-forks-breathe.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": minor +--- + +Extended apps-engine events for users leaving a room to also fire when being removed by another user. Also added the triggering user's information to the event's context payload. diff --git a/apps/meteor/app/api/server/v1/users.ts b/apps/meteor/app/api/server/v1/users.ts index c26957fa1991..041ef0410df0 100644 --- a/apps/meteor/app/api/server/v1/users.ts +++ b/apps/meteor/app/api/server/v1/users.ts @@ -19,6 +19,7 @@ import { isUsersCheckUsernameAvailabilityParamsGET, isUsersSendConfirmationEmailParamsPOST, } from '@rocket.chat/rest-typings'; +import { getLoginExpirationInMs } from '@rocket.chat/tools'; import { Accounts } from 'meteor/accounts-base'; import { Match, check } from 'meteor/check'; import { Meteor } from 'meteor/meteor'; @@ -1048,8 +1049,9 @@ API.v1.addRoute( const token = me.services?.resume?.loginTokens?.find((token) => token.hashedToken === hashedToken); - const tokenExpires = - (token && 'when' in token && new Date(token.when.getTime() + settings.get('Accounts_LoginExpiration') * 1000)) || undefined; + const loginExp = settings.get('Accounts_LoginExpiration'); + + const tokenExpires = (token && 'when' in token && new Date(token.when.getTime() + getLoginExpirationInMs(loginExp))) || undefined; return API.v1.success({ token: xAuthToken, diff --git a/apps/meteor/app/apps/server/bridges/listeners.js b/apps/meteor/app/apps/server/bridges/listeners.js index ab2632c912b0..13db1179310c 100644 --- a/apps/meteor/app/apps/server/bridges/listeners.js +++ b/apps/meteor/app/apps/server/bridges/listeners.js @@ -143,10 +143,11 @@ export class AppListenerBridge { }; case AppInterface.IPreRoomUserLeave: case AppInterface.IPostRoomUserLeave: - const [leavingUser] = payload; + const [leavingUser, removedBy] = payload; return { room: rm, leavingUser: this.orch.getConverters().get('users').convertToApp(leavingUser), + removedBy: this.orch.getConverters().get('users').convertToApp(removedBy), }; default: return rm; diff --git a/apps/meteor/app/authentication/server/startup/index.js b/apps/meteor/app/authentication/server/startup/index.js index bffbe1f9876d..2e4c599ce558 100644 --- a/apps/meteor/app/authentication/server/startup/index.js +++ b/apps/meteor/app/authentication/server/startup/index.js @@ -2,6 +2,7 @@ import { Apps, AppEvents } from '@rocket.chat/apps'; import { User } from '@rocket.chat/core-services'; import { Roles, Settings, Users } from '@rocket.chat/models'; import { escapeRegExp, escapeHTML } from '@rocket.chat/string-helpers'; +import { getLoginExpirationInDays } from '@rocket.chat/tools'; import { Accounts } from 'meteor/accounts-base'; import { Match } from 'meteor/check'; import { Meteor } from 'meteor/meteor'; @@ -31,7 +32,7 @@ Accounts.config({ Meteor.startup(() => { settings.watchMultiple(['Accounts_LoginExpiration', 'Site_Name', 'From_Email'], () => { - Accounts._options.loginExpirationInDays = settings.get('Accounts_LoginExpiration'); + Accounts._options.loginExpirationInDays = getLoginExpirationInDays(settings.get('Accounts_LoginExpiration')); Accounts.emailTemplates.siteName = settings.get('Site_Name'); diff --git a/apps/meteor/app/lib/server/functions/closeLivechatRoom.ts b/apps/meteor/app/lib/server/functions/closeLivechatRoom.ts new file mode 100644 index 000000000000..b716be044d57 --- /dev/null +++ b/apps/meteor/app/lib/server/functions/closeLivechatRoom.ts @@ -0,0 +1,81 @@ +import type { IUser, IRoom, IOmnichannelRoom } from '@rocket.chat/core-typings'; +import { isOmnichannelRoom } from '@rocket.chat/core-typings'; +import { LivechatRooms, Subscriptions } from '@rocket.chat/models'; + +import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission'; +import type { CloseRoomParams } from '../../../livechat/server/lib/LivechatTyped'; +import { Livechat } from '../../../livechat/server/lib/LivechatTyped'; + +export const closeLivechatRoom = async ( + user: IUser, + roomId: IRoom['_id'], + { + comment, + tags, + generateTranscriptPdf, + transcriptEmail, + }: { + comment?: string; + tags?: string[]; + generateTranscriptPdf?: boolean; + transcriptEmail?: + | { + sendToVisitor: false; + } + | { + sendToVisitor: true; + requestData: Pick, 'email' | 'subject'>; + }; + }, +): Promise => { + const room = await LivechatRooms.findOneById(roomId); + if (!room || !isOmnichannelRoom(room)) { + throw new Error('error-invalid-room'); + } + + if (!room.open) { + const subscriptionsLeft = await Subscriptions.countByRoomId(roomId); + if (subscriptionsLeft) { + await Subscriptions.removeByRoomId(roomId); + return; + } + throw new Error('error-room-already-closed'); + } + + const subscription = await Subscriptions.findOneByRoomIdAndUserId(roomId, user._id, { projection: { _id: 1 } }); + if (!subscription && !(await hasPermissionAsync(user._id, 'close-others-livechat-room'))) { + throw new Error('error-not-authorized'); + } + + const options: CloseRoomParams['options'] = { + clientAction: true, + tags, + ...(generateTranscriptPdf && { pdfTranscript: { requestedBy: user._id } }), + ...(transcriptEmail && { + ...(transcriptEmail.sendToVisitor + ? { + emailTranscript: { + sendToVisitor: true, + requestData: { + email: transcriptEmail.requestData.email, + subject: transcriptEmail.requestData.subject, + requestedAt: new Date(), + requestedBy: user, + }, + }, + } + : { + emailTranscript: { + sendToVisitor: false, + }, + }), + }), + }; + + await Livechat.closeRoom({ + room, + user, + options, + comment, + }); +}; diff --git a/apps/meteor/app/lib/server/functions/removeUserFromRoom.ts b/apps/meteor/app/lib/server/functions/removeUserFromRoom.ts index 3b065c68f15c..c55ee382f10c 100644 --- a/apps/meteor/app/lib/server/functions/removeUserFromRoom.ts +++ b/apps/meteor/app/lib/server/functions/removeUserFromRoom.ts @@ -10,11 +10,7 @@ import { beforeLeaveRoomCallback } from '../../../../lib/callbacks/beforeLeaveRo import { settings } from '../../../settings/server'; import { notifyOnRoomChangedById } from '../lib/notifyListener'; -export const removeUserFromRoom = async function ( - rid: string, - user: IUser, - options?: { byUser: Pick }, -): Promise { +export const removeUserFromRoom = async function (rid: string, user: IUser, options?: { byUser: IUser }): Promise { const room = await Rooms.findOneById(rid); if (!room) { @@ -22,7 +18,7 @@ export const removeUserFromRoom = async function ( } try { - await Apps.self?.triggerEvent(AppEvents.IPreRoomUserLeave, room, user); + await Apps.self?.triggerEvent(AppEvents.IPreRoomUserLeave, room, user, options?.byUser); } catch (error: any) { if (error.name === AppsEngineException.name) { throw new Meteor.Error('error-app-prevented', error.message); @@ -75,5 +71,5 @@ export const removeUserFromRoom = async function ( void notifyOnRoomChangedById(rid); - await Apps.self?.triggerEvent(AppEvents.IPostRoomUserLeave, room, user); + await Apps.self?.triggerEvent(AppEvents.IPostRoomUserLeave, room, user, options?.byUser); }; diff --git a/apps/meteor/app/livechat/server/api/v1/room.ts b/apps/meteor/app/livechat/server/api/v1/room.ts index 8a663fb0bd6d..94674f801ad5 100644 --- a/apps/meteor/app/livechat/server/api/v1/room.ts +++ b/apps/meteor/app/livechat/server/api/v1/room.ts @@ -1,7 +1,7 @@ import { Omnichannel } from '@rocket.chat/core-services'; import type { ILivechatAgent, IUser, SelectedAgent, TransferByData } from '@rocket.chat/core-typings'; import { isOmnichannelRoom, OmnichannelSourceType } from '@rocket.chat/core-typings'; -import { LivechatVisitors, Users, LivechatRooms, Subscriptions, Messages } from '@rocket.chat/models'; +import { LivechatVisitors, Users, LivechatRooms, Messages } from '@rocket.chat/models'; import { isLiveChatRoomForwardProps, isPOSTLivechatRoomCloseParams, @@ -21,6 +21,7 @@ import { isWidget } from '../../../../api/server/helpers/isWidget'; import { canAccessRoomAsync, roomAccessAttributes } from '../../../../authorization/server'; import { hasPermissionAsync } from '../../../../authorization/server/functions/hasPermission'; import { addUserToRoom } from '../../../../lib/server/functions/addUserToRoom'; +import { closeLivechatRoom } from '../../../../lib/server/functions/closeLivechatRoom'; import { settings as rcSettings } from '../../../../settings/server'; import { normalizeTransferredByData } from '../../lib/Helper'; import type { CloseRoomParams } from '../../lib/LivechatTyped'; @@ -178,51 +179,7 @@ API.v1.addRoute( async post() { const { rid, comment, tags, generateTranscriptPdf, transcriptEmail } = this.bodyParams; - const room = await LivechatRooms.findOneById(rid); - if (!room || !isOmnichannelRoom(room)) { - throw new Error('error-invalid-room'); - } - - if (!room.open) { - throw new Error('error-room-already-closed'); - } - - const subscription = await Subscriptions.findOneByRoomIdAndUserId(rid, this.userId, { projection: { _id: 1 } }); - if (!subscription && !(await hasPermissionAsync(this.userId, 'close-others-livechat-room'))) { - throw new Error('error-not-authorized'); - } - - const options: CloseRoomParams['options'] = { - clientAction: true, - tags, - ...(generateTranscriptPdf && { pdfTranscript: { requestedBy: this.userId } }), - ...(transcriptEmail && { - ...(transcriptEmail.sendToVisitor - ? { - emailTranscript: { - sendToVisitor: true, - requestData: { - email: transcriptEmail.requestData.email, - subject: transcriptEmail.requestData.subject, - requestedAt: new Date(), - requestedBy: this.user, - }, - }, - } - : { - emailTranscript: { - sendToVisitor: false, - }, - }), - }), - }; - - await LivechatTyped.closeRoom({ - room, - user: this.user, - options, - comment, - }); + await closeLivechatRoom(this.user, rid, { comment, tags, generateTranscriptPdf, transcriptEmail }); return API.v1.success(); }, diff --git a/apps/meteor/app/livechat/server/methods/closeRoom.ts b/apps/meteor/app/livechat/server/methods/closeRoom.ts index 1374d86ab9f7..5fdf9e7d504f 100644 --- a/apps/meteor/app/livechat/server/methods/closeRoom.ts +++ b/apps/meteor/app/livechat/server/methods/closeRoom.ts @@ -60,6 +60,16 @@ Meteor.methods({ }); } + const subscription = await SubscriptionRaw.findOneByRoomIdAndUserId(roomId, userId, { + projection: { + _id: 1, + }, + }); + if (!room.open && subscription) { + await SubscriptionRaw.removeByRoomId(roomId); + return; + } + if (!room.open) { throw new Meteor.Error('room-closed', 'Room closed', { method: 'livechat:closeRoom' }); } @@ -71,11 +81,6 @@ Meteor.methods({ }); } - const subscription = await SubscriptionRaw.findOneByRoomIdAndUserId(roomId, user._id, { - projection: { - _id: 1, - }, - }); if (!subscription && !(await hasPermissionAsync(userId, 'close-others-livechat-room'))) { throw new Meteor.Error('error-not-authorized', 'Not authorized', { method: 'livechat:closeRoom', diff --git a/apps/meteor/client/views/room/Header/Omnichannel/QuickActions/hooks/useQuickActions.tsx b/apps/meteor/client/views/room/Header/Omnichannel/QuickActions/hooks/useQuickActions.tsx index 7446d0630b09..edf5ffcbdc8a 100644 --- a/apps/meteor/client/views/room/Header/Omnichannel/QuickActions/hooks/useQuickActions.tsx +++ b/apps/meteor/client/views/room/Header/Omnichannel/QuickActions/hooks/useQuickActions.tsx @@ -10,6 +10,7 @@ import { useMethod, useTranslation, useRouter, + useUserSubscription, } from '@rocket.chat/ui-contexts'; import React, { useCallback, useState, useEffect } from 'react'; @@ -47,6 +48,7 @@ export const useQuickActions = (): { const visitorRoomId = room.v._id; const rid = room._id; const uid = useUserId(); + const subscription = useUserSubscription(rid); const roomLastMessage = room.lastMessage; const getVisitorInfo = useEndpoint('GET', '/v1/livechat/visitors.info'); @@ -330,7 +332,7 @@ export const useQuickActions = (): { case QuickActionsEnum.TranscriptPDF: return hasLicense && !isRoomOverMacLimit && canSendTranscriptPDF; case QuickActionsEnum.CloseChat: - return !!roomOpen && (canCloseRoom || canCloseOthersRoom); + return (subscription && (canCloseRoom || canCloseOthersRoom)) || (!!roomOpen && canCloseOthersRoom); case QuickActionsEnum.OnHoldChat: return !!roomOpen && canPlaceChatOnHold; default: diff --git a/apps/meteor/server/methods/removeUserFromRoom.ts b/apps/meteor/server/methods/removeUserFromRoom.ts index 08cd6c5b25e0..6662ae8d22cf 100644 --- a/apps/meteor/server/methods/removeUserFromRoom.ts +++ b/apps/meteor/server/methods/removeUserFromRoom.ts @@ -1,3 +1,5 @@ +import { Apps, AppEvents } from '@rocket.chat/apps'; +import { AppsEngineException } from '@rocket.chat/apps-engine/definition/exceptions'; import { Message, Team } from '@rocket.chat/core-services'; import { Subscriptions, Rooms, Users } from '@rocket.chat/models'; import type { ServerMethods } from '@rocket.chat/ui-contexts'; @@ -75,6 +77,16 @@ export const removeUserFromRoomMethod = async (fromId: string, data: { rid: stri } } + try { + await Apps.self?.triggerEvent(AppEvents.IPreRoomUserLeave, room, removedUser, fromUser); + } catch (error: any) { + if (error.name === AppsEngineException.name) { + throw new Meteor.Error('error-app-prevented', error.message); + } + + throw error; + } + await callbacks.run('beforeRemoveFromRoom', { removedUser, userWhoRemoved: fromUser }, room); await Subscriptions.removeByRoomIdAndUserId(data.rid, removedUser._id); @@ -99,6 +111,8 @@ export const removeUserFromRoomMethod = async (fromId: string, data: { rid: stri void notifyOnRoomChanged(room); }); + await Apps.self?.triggerEvent(AppEvents.IPostRoomUserLeave, room, removedUser, fromUser); + return true; }; diff --git a/apps/meteor/server/services/room/service.ts b/apps/meteor/server/services/room/service.ts index b112209b681b..4b0fe2d177b5 100644 --- a/apps/meteor/server/services/room/service.ts +++ b/apps/meteor/server/services/room/service.ts @@ -66,7 +66,7 @@ export class RoomService extends ServiceClassInternal implements IRoomService { return addUserToRoom(roomId, user, inviter, silenced); } - async removeUserFromRoom(roomId: string, user: IUser, options?: { byUser: Pick }): Promise { + async removeUserFromRoom(roomId: string, user: IUser, options?: { byUser: IUser }): Promise { return removeUserFromRoom(roomId, user, options); } diff --git a/apps/meteor/server/services/team/service.ts b/apps/meteor/server/services/team/service.ts index f81b21d7fa01..be153d565f18 100644 --- a/apps/meteor/server/services/team/service.ts +++ b/apps/meteor/server/services/team/service.ts @@ -771,7 +771,7 @@ export class TeamService extends ServiceClassInternal implements ITeamService { const usersToRemove = await Users.findByIds(membersIds, { projection: { _id: 1, username: 1 }, }).toArray(); - const byUser = (await Users.findOneById(uid, { projection: { _id: 1, username: 1 } })) as Pick; + const byUser = await Users.findOneById(uid); for await (const member of members) { if (!member.userId) { @@ -802,7 +802,7 @@ export class TeamService extends ServiceClassInternal implements ITeamService { await removeUserFromRoom( team.roomId, removedUser, - uid !== member.userId + uid !== member.userId && byUser ? { byUser, } diff --git a/apps/meteor/tests/e2e/message-mentions.spec.ts b/apps/meteor/tests/e2e/message-mentions.spec.ts index 9d5cd7e622ce..15abd9c36b40 100644 --- a/apps/meteor/tests/e2e/message-mentions.spec.ts +++ b/apps/meteor/tests/e2e/message-mentions.spec.ts @@ -56,7 +56,7 @@ test.describe.serial('Should not allow to send @all mention if permission to do await expect(page).toHaveURL(`/group/${targetChannel2}`); }); await test.step('receive notify message', async () => { - await adminPage.content.dispatchSlashCommand('@all'); + await adminPage.content.sendMessage('@all '); await expect(adminPage.content.lastUserMessage).toContainText('Notify all in this room is not allowed'); }); }); @@ -98,7 +98,7 @@ test.describe.serial('Should not allow to send @here mention if permission to do await expect(page).toHaveURL(`/group/${targetChannel2}`); }); await test.step('receive notify message', async () => { - await adminPage.content.dispatchSlashCommand('@here'); + await adminPage.content.sendMessage('@here '); await expect(adminPage.content.lastUserMessage).toContainText('Notify all in this room is not allowed'); }); }); diff --git a/apps/meteor/tests/unit/app/lib/server/functions/closeLivechatRoom.tests.ts b/apps/meteor/tests/unit/app/lib/server/functions/closeLivechatRoom.tests.ts new file mode 100644 index 000000000000..07ee437832d2 --- /dev/null +++ b/apps/meteor/tests/unit/app/lib/server/functions/closeLivechatRoom.tests.ts @@ -0,0 +1,154 @@ +import { expect } from 'chai'; +import { describe, it } from 'mocha'; +import proxyquire from 'proxyquire'; +import sinon from 'sinon'; + +import { createFakeRoom, createFakeSubscription, createFakeUser } from '../../../../../mocks/data'; + +const subscriptionsStub = { + findOneByRoomIdAndUserId: sinon.stub(), + removeByRoomId: sinon.stub(), + countByRoomId: sinon.stub(), +}; + +const livechatRoomsStub = { + findOneById: sinon.stub(), +}; + +const livechatStub = { + closeRoom: sinon.stub(), +}; + +const hasPermissionStub = sinon.stub(); + +const { closeLivechatRoom } = proxyquire.noCallThru().load('../../../../../../app/lib/server/functions/closeLivechatRoom.ts', { + '../../../livechat/server/lib/LivechatTyped': { + Livechat: livechatStub, + }, + '../../../authorization/server/functions/hasPermission': { + hasPermissionAsync: hasPermissionStub, + }, + '@rocket.chat/models': { + Subscriptions: subscriptionsStub, + LivechatRooms: livechatRoomsStub, + }, +}); + +describe('closeLivechatRoom', () => { + const user = createFakeUser(); + const room = createFakeRoom({ t: 'l', open: true }); + const subscription = createFakeSubscription({ rid: room._id, u: { username: user.username, _id: user._id } }); + + beforeEach(() => { + subscriptionsStub.findOneByRoomIdAndUserId.reset(); + subscriptionsStub.removeByRoomId.reset(); + subscriptionsStub.countByRoomId.reset(); + livechatRoomsStub.findOneById.reset(); + livechatStub.closeRoom.reset(); + hasPermissionStub.reset(); + }); + + it('should not perform any operation when an invalid room id is provided', async () => { + livechatRoomsStub.findOneById.resolves(null); + hasPermissionStub.resolves(true); + + await expect(closeLivechatRoom(user, room._id, {})).to.be.rejectedWith('error-invalid-room'); + expect(livechatStub.closeRoom.notCalled).to.be.true; + expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; + expect(subscriptionsStub.findOneByRoomIdAndUserId.notCalled).to.be.true; + expect(subscriptionsStub.removeByRoomId.notCalled).to.be.true; + }); + + it('should not perform any operation when a non-livechat room is provided', async () => { + livechatRoomsStub.findOneById.resolves({ ...room, t: 'c' }); + subscriptionsStub.findOneByRoomIdAndUserId.resolves(subscription); + hasPermissionStub.resolves(true); + + await expect(closeLivechatRoom(user, room._id, {})).to.be.rejectedWith('error-invalid-room'); + expect(livechatStub.closeRoom.notCalled).to.be.true; + expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; + expect(subscriptionsStub.findOneByRoomIdAndUserId.notCalled).to.be.true; + expect(subscriptionsStub.removeByRoomId.notCalled).to.be.true; + }); + + it('should not perform any operation when a closed room with no subscriptions is provided and the caller is not subscribed to it', async () => { + livechatRoomsStub.findOneById.resolves({ ...room, open: false }); + subscriptionsStub.countByRoomId.resolves(0); + subscriptionsStub.findOneByRoomIdAndUserId.resolves(null); + hasPermissionStub.resolves(true); + + await expect(closeLivechatRoom(user, room._id, {})).to.be.rejectedWith('error-room-already-closed'); + expect(livechatStub.closeRoom.notCalled).to.be.true; + expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; + expect(subscriptionsStub.findOneByRoomIdAndUserId.notCalled).to.be.true; + expect(subscriptionsStub.countByRoomId.calledOnceWith(room._id)).to.be.true; + expect(subscriptionsStub.removeByRoomId.notCalled).to.be.true; + }); + + it('should remove dangling subscription when a closed room with subscriptions is provided and the caller is not subscribed to it', async () => { + livechatRoomsStub.findOneById.resolves({ ...room, open: false }); + subscriptionsStub.countByRoomId.resolves(1); + subscriptionsStub.findOneByRoomIdAndUserId.resolves(null); + hasPermissionStub.resolves(true); + + await closeLivechatRoom(user, room._id, {}); + expect(livechatStub.closeRoom.notCalled).to.be.true; + expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; + expect(subscriptionsStub.findOneByRoomIdAndUserId.notCalled).to.be.true; + expect(subscriptionsStub.countByRoomId.calledOnceWith(room._id)).to.be.true; + expect(subscriptionsStub.removeByRoomId.calledOnceWith(room._id)).to.be.true; + }); + + it('should remove dangling subscription when a closed room is provided but the user is still subscribed to it', async () => { + livechatRoomsStub.findOneById.resolves({ ...room, open: false }); + subscriptionsStub.findOneByRoomIdAndUserId.resolves(subscription); + subscriptionsStub.countByRoomId.resolves(1); + hasPermissionStub.resolves(true); + + await closeLivechatRoom(user, room._id, {}); + expect(livechatStub.closeRoom.notCalled).to.be.true; + expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; + expect(subscriptionsStub.findOneByRoomIdAndUserId.notCalled).to.be.true; + expect(subscriptionsStub.countByRoomId.calledOnceWith(room._id)).to.be.true; + expect(subscriptionsStub.removeByRoomId.calledOnceWith(room._id)).to.be.true; + }); + + it('should not perform any operation when the caller is not subscribed to an open room and does not have the permission to close others rooms', async () => { + livechatRoomsStub.findOneById.resolves(room); + subscriptionsStub.findOneByRoomIdAndUserId.resolves(null); + subscriptionsStub.countByRoomId.resolves(1); + hasPermissionStub.resolves(false); + + await expect(closeLivechatRoom(user, room._id, {})).to.be.rejectedWith('error-not-authorized'); + expect(livechatStub.closeRoom.notCalled).to.be.true; + expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; + expect(subscriptionsStub.findOneByRoomIdAndUserId.calledOnceWith(room._id, user._id)).to.be.true; + expect(subscriptionsStub.removeByRoomId.notCalled).to.be.true; + }); + + it('should close the room when the caller is not subscribed to it but has the permission to close others rooms', async () => { + livechatRoomsStub.findOneById.resolves(room); + subscriptionsStub.findOneByRoomIdAndUserId.resolves(null); + subscriptionsStub.countByRoomId.resolves(1); + hasPermissionStub.resolves(true); + + await closeLivechatRoom(user, room._id, {}); + expect(livechatStub.closeRoom.calledOnceWith(sinon.match({ room, user }))).to.be.true; + expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; + expect(subscriptionsStub.findOneByRoomIdAndUserId.calledOnceWith(room._id, user._id)).to.be.true; + expect(subscriptionsStub.removeByRoomId.notCalled).to.be.true; + }); + + it('should close the room when the caller is subscribed to it and does not have the permission to close others rooms', async () => { + livechatRoomsStub.findOneById.resolves(room); + subscriptionsStub.findOneByRoomIdAndUserId.resolves(subscription); + subscriptionsStub.countByRoomId.resolves(1); + hasPermissionStub.resolves(false); + + await closeLivechatRoom(user, room._id, {}); + expect(livechatStub.closeRoom.calledOnceWith(sinon.match({ room, user }))).to.be.true; + expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; + expect(subscriptionsStub.findOneByRoomIdAndUserId.calledOnceWith(room._id, user._id)).to.be.true; + expect(subscriptionsStub.removeByRoomId.notCalled).to.be.true; + }); +}); diff --git a/ee/apps/account-service/Dockerfile b/ee/apps/account-service/Dockerfile index c662d8765300..acbc5b0371d2 100644 --- a/ee/apps/account-service/Dockerfile +++ b/ee/apps/account-service/Dockerfile @@ -43,6 +43,9 @@ COPY ./ee/packages/license/dist packages/license/dist COPY ./packages/ui-kit/package.json packages/ui-kit/package.json COPY ./packages/ui-kit/dist packages/ui-kit/dist +COPY ./packages/tools/package.json packages/tools/package.json +COPY ./packages/tools/dist packages/tools/dist + COPY ./ee/apps/${SERVICE}/dist . COPY ./package.json . diff --git a/ee/apps/account-service/package.json b/ee/apps/account-service/package.json index c41932159e0e..194a3e1760e5 100644 --- a/ee/apps/account-service/package.json +++ b/ee/apps/account-service/package.json @@ -22,6 +22,7 @@ "@rocket.chat/models": "workspace:^", "@rocket.chat/rest-typings": "workspace:^", "@rocket.chat/string-helpers": "~0.31.25", + "@rocket.chat/tools": "workspace:^", "@types/node": "^14.18.63", "bcrypt": "^5.0.1", "ejson": "^2.2.3", diff --git a/ee/apps/account-service/src/Account.ts b/ee/apps/account-service/src/Account.ts index 4521574c12c7..c71376d301f3 100644 --- a/ee/apps/account-service/src/Account.ts +++ b/ee/apps/account-service/src/Account.ts @@ -1,6 +1,7 @@ import { ServiceClass } from '@rocket.chat/core-services'; import type { IAccount, ILoginResult } from '@rocket.chat/core-services'; import { Settings } from '@rocket.chat/models'; +import { getLoginExpirationInDays } from '@rocket.chat/tools'; import { loginViaResume } from './lib/loginViaResume'; import { loginViaUsername } from './lib/loginViaUsername'; @@ -22,9 +23,8 @@ export class Account extends ServiceClass implements IAccount { if (_id !== 'Accounts_LoginExpiration') { return; } - if (typeof value === 'number') { - this.loginExpiration = value; - } + + this.loginExpiration = getLoginExpirationInDays(value as number); }); } @@ -46,8 +46,7 @@ export class Account extends ServiceClass implements IAccount { async started(): Promise { const expiry = await Settings.findOne({ _id: 'Accounts_LoginExpiration' }, { projection: { value: 1 } }); - if (expiry?.value) { - this.loginExpiration = expiry.value as number; - } + + this.loginExpiration = getLoginExpirationInDays(expiry?.value as number); } } diff --git a/ee/apps/account-service/src/lib/utils.ts b/ee/apps/account-service/src/lib/utils.ts index 1d4a3a6ad580..f52f323606de 100644 --- a/ee/apps/account-service/src/lib/utils.ts +++ b/ee/apps/account-service/src/lib/utils.ts @@ -1,5 +1,6 @@ import crypto from 'crypto'; +import { convertFromDaysToMilliseconds } from '@rocket.chat/tools'; import bcrypt from 'bcrypt'; import { v4 as uuidv4 } from 'uuid'; @@ -60,4 +61,4 @@ export const validatePassword = (password: string, bcryptPassword: string): Prom bcrypt.compare(getPassword(password), bcryptPassword); export const _tokenExpiration = (when: string | Date, expirationInDays: number): Date => - new Date(new Date(when).getTime() + expirationInDays * 60 * 60 * 24 * 1000); + new Date(new Date(when).getTime() + convertFromDaysToMilliseconds(expirationInDays)); diff --git a/packages/i18n/src/locales/en.i18n.json b/packages/i18n/src/locales/en.i18n.json index 90cb13688fc9..0b569b4ee564 100644 --- a/packages/i18n/src/locales/en.i18n.json +++ b/packages/i18n/src/locales/en.i18n.json @@ -2321,8 +2321,8 @@ "Federation_Matrix_not_allowed_to_change_moderator": "You are not allowed to change the moderator", "Federation_Matrix_not_allowed_to_change_owner": "You are not allowed to change the owner", "Federation_Matrix_join_public_rooms_is_premium": "Join federated rooms is a Premium feature", - "Federation_Matrix_max_size_of_public_rooms_users": "Maximum number of users when joining a public room in a remote server", - "Federation_Matrix_max_size_of_public_rooms_users_desc": "The number of the maximum users when joining a public room in a remote server. Public Rooms with more users will be ignored in the list of Public Rooms to join.", + "Federation_Matrix_max_size_of_public_rooms_users": "Maximum number of members when joining a public room in a remote server", + "Federation_Matrix_max_size_of_public_rooms_users_desc": "The user limit from a public room in a remote server that can still be joined. Rooms that exceed this setting will still be listed, but users won't be able to join them", "Federation_Matrix_max_size_of_public_rooms_users_Alert": "Keep in mind, that the bigger the room you allow for users to join, the more time it will take to join that room, besides the amount of resource it will use. Read more", "Federation_Matrix_serve_well_known": "Serve Well Known", "Federation_Matrix_serve_well_known_Description": "Serve /.well-known/matrix/server and /.well-known/matrix/client directly from within Rocket.Chat instead of reverse proxy for federation", diff --git a/packages/tools/jest.config.ts b/packages/tools/jest.config.ts new file mode 100644 index 000000000000..959a31a7c6bf --- /dev/null +++ b/packages/tools/jest.config.ts @@ -0,0 +1,3 @@ +export default { + preset: 'ts-jest', +}; diff --git a/packages/tools/package.json b/packages/tools/package.json index e6bbae9b5be6..ac79955314b8 100644 --- a/packages/tools/package.json +++ b/packages/tools/package.json @@ -13,7 +13,9 @@ "lint": "eslint --ext .js,.jsx,.ts,.tsx .", "lint:fix": "eslint --ext .js,.jsx,.ts,.tsx . --fix", "test": "jest", + "test:cov": "jest --coverage", "build": "rm -rf dist && tsc -p tsconfig.json", + "testunit": "jest", "dev": "tsc -p tsconfig.json --watch --preserveWatchOutput" }, "main": "./dist/index.js", diff --git a/packages/tools/src/converter.spec.ts b/packages/tools/src/converter.spec.ts new file mode 100644 index 000000000000..5a27b6a97b95 --- /dev/null +++ b/packages/tools/src/converter.spec.ts @@ -0,0 +1,15 @@ +import { convertFromDaysToMilliseconds } from './converter'; + +describe('convertFromDaysToMilliseconds', () => { + it('should throw an error when a non number is passed', () => { + // @ts-expect-error - Testing + expect(() => convertFromDaysToMilliseconds('90')).toThrow(); + }); + it('should return the value passed when its valid', () => { + expect(convertFromDaysToMilliseconds(85)).toBe(85 * 24 * 60 * 60 * 1000); + }); + it('should fail if anything but an integer is passed', () => { + expect(() => convertFromDaysToMilliseconds(85.5)).toThrow(); + expect(() => convertFromDaysToMilliseconds(-2.3)).toThrow(); + }); +}); diff --git a/packages/tools/src/converter.ts b/packages/tools/src/converter.ts new file mode 100644 index 000000000000..e71c264857dc --- /dev/null +++ b/packages/tools/src/converter.ts @@ -0,0 +1,7 @@ +export const convertFromDaysToMilliseconds = (days: number) => { + if (typeof days !== 'number' || !Number.isInteger(days)) { + throw new Error('days must be a number'); + } + + return days * 24 * 60 * 60 * 1000; +}; diff --git a/packages/tools/src/getLoginExpiration.spec.ts b/packages/tools/src/getLoginExpiration.spec.ts new file mode 100644 index 000000000000..edd652172a5e --- /dev/null +++ b/packages/tools/src/getLoginExpiration.spec.ts @@ -0,0 +1,35 @@ +import { getLoginExpirationInDays, getLoginExpirationInMs } from './getLoginExpiration'; + +describe('getLoginExpirationInDays', () => { + it('should return 90 by default', () => { + expect(getLoginExpirationInDays()).toBe(90); + }); + it('should return 90 when value is 0', () => { + expect(getLoginExpirationInDays(0)).toBe(90); + }); + it('should return 90 when value is NaN', () => { + expect(getLoginExpirationInDays(NaN)).toBe(90); + }); + it('should return 90 when value is negative', () => { + expect(getLoginExpirationInDays(-1)).toBe(90); + }); + it('should return 90 when value is undefined', () => { + expect(getLoginExpirationInDays(undefined)).toBe(90); + }); + it('should return 90 when value is not a number', () => { + // @ts-expect-error - Testing + expect(getLoginExpirationInDays('90')).toBe(90); + }); + it('should return the value passed when its valid', () => { + expect(getLoginExpirationInDays(85)).toBe(85); + }); +}); + +describe('getLoginExpirationInMs', () => { + it('should return 90 days in milliseconds when no value is passed', () => { + expect(getLoginExpirationInMs()).toBe(90 * 24 * 60 * 60 * 1000); + }); + it('should return the value passed when its valid', () => { + expect(getLoginExpirationInMs(85)).toBe(85 * 24 * 60 * 60 * 1000); + }); +}); diff --git a/packages/tools/src/getLoginExpiration.ts b/packages/tools/src/getLoginExpiration.ts new file mode 100644 index 000000000000..de21fdce21a8 --- /dev/null +++ b/packages/tools/src/getLoginExpiration.ts @@ -0,0 +1,16 @@ +import { convertFromDaysToMilliseconds } from './converter'; + +const ACCOUNTS_DEFAULT_LOGIN_EXPIRATION_DAYS = 90; + +// Given a value, validates if it mets the conditions to be a valid login expiration. +// Else, returns the default login expiration (which for Meteor is 90 days) +export const getLoginExpirationInDays = (expiry?: number) => { + if (expiry && typeof expiry === 'number' && !Number.isNaN(expiry) && expiry > 0) { + return expiry; + } + return ACCOUNTS_DEFAULT_LOGIN_EXPIRATION_DAYS; +}; + +export const getLoginExpirationInMs = (expiry?: number) => { + return convertFromDaysToMilliseconds(getLoginExpirationInDays(expiry)); +}; diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts index b1b53ab71a90..96faa4d55969 100644 --- a/packages/tools/src/index.ts +++ b/packages/tools/src/index.ts @@ -4,3 +4,5 @@ export * from './pick'; export * from './stream'; export * from './timezone'; export * from './wrapExceptions'; +export * from './getLoginExpiration'; +export * from './converter'; diff --git a/yarn.lock b/yarn.lock index 3b68dcde3806..add9a5e19468 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8432,6 +8432,7 @@ __metadata: "@rocket.chat/models": "workspace:^" "@rocket.chat/rest-typings": "workspace:^" "@rocket.chat/string-helpers": ~0.31.25 + "@rocket.chat/tools": "workspace:^" "@types/bcrypt": ^5.0.1 "@types/gc-stats": ^1.4.2 "@types/node": ^14.18.63