diff --git a/.changeset/green-dragons-boil.md b/.changeset/green-dragons-boil.md new file mode 100644 index 0000000000000..47e9914c8dc19 --- /dev/null +++ b/.changeset/green-dragons-boil.md @@ -0,0 +1,6 @@ +--- +'@rocket.chat/rest-typings': patch +'@rocket.chat/meteor': patch +--- + +Fixes an issue where web clients could remain with a stale slashcommand list during a rolling workspace update diff --git a/apps/meteor/.meteor/packages b/apps/meteor/.meteor/packages index 4242b98f6bed3..000e897aa4ce5 100644 --- a/apps/meteor/.meteor/packages +++ b/apps/meteor/.meteor/packages @@ -10,7 +10,6 @@ rocketchat:version accounts-base@3.1.2 accounts-facebook@1.3.4 -accounts-github@1.5.1 accounts-google@1.4.1 accounts-meteor-developer@1.5.1 accounts-oauth@1.4.6 diff --git a/apps/meteor/.meteor/versions b/apps/meteor/.meteor/versions index 04fcb4d920bdd..29f3113919760 100644 --- a/apps/meteor/.meteor/versions +++ b/apps/meteor/.meteor/versions @@ -1,6 +1,5 @@ accounts-base@3.1.2 accounts-facebook@1.3.4 -accounts-github@1.5.1 accounts-google@1.4.1 accounts-meteor-developer@1.5.1 accounts-oauth@1.4.6 @@ -35,7 +34,6 @@ facebook-oauth@1.11.6 facts-base@1.0.2 fetch@0.1.6 geojson-utils@1.0.12 -github-oauth@1.4.2 google-oauth@1.4.5 hot-code-push@1.0.5 http@3.0.0 diff --git a/apps/meteor/app/api/server/v1/commands.ts b/apps/meteor/app/api/server/v1/commands.ts index fda27be1d0dd9..59b90baafa8a1 100644 --- a/apps/meteor/app/api/server/v1/commands.ts +++ b/apps/meteor/app/api/server/v1/commands.ts @@ -1,3 +1,4 @@ +import { Apps } from '@rocket.chat/apps'; import { Messages } from '@rocket.chat/models'; import { Random } from '@rocket.chat/random'; import objectPath from 'object-path'; @@ -143,6 +144,19 @@ API.v1.addRoute( { authRequired: true }, { async get() { + if (!Apps.self?.isLoaded()) { + return { + statusCode: 202, // Accepted - apps are not ready, so the list is incomplete. Retry later + body: { + commands: [], + appsLoaded: false, + offset: 0, + count: 0, + total: 0, + }, + }; + } + const params = this.queryParams as Record; const { offset, count } = await getPaginationItems(params); const { sort, query } = await this.parseJsonQuery(); @@ -161,6 +175,7 @@ API.v1.addRoute( skip: offset, limit: count, }), + appsLoaded: true, offset, count: commands.length, total: totalCount, diff --git a/apps/meteor/app/api/server/v1/emoji-custom.ts b/apps/meteor/app/api/server/v1/emoji-custom.ts index fa84d3cc6b995..15764b7f74e7d 100644 --- a/apps/meteor/app/api/server/v1/emoji-custom.ts +++ b/apps/meteor/app/api/server/v1/emoji-custom.ts @@ -135,8 +135,8 @@ API.v1.addRoute( }); await uploadEmojiCustomWithBuffer(this.userId, fileBuffer, mimetype, emojiData); - } catch (e) { - SystemLogger.error(e); + } catch (err) { + SystemLogger.error({ err }); return API.v1.failure(); } diff --git a/apps/meteor/app/api/server/v1/ldap.ts b/apps/meteor/app/api/server/v1/ldap.ts index 4b112cfbf3351..3f9a2c29deded 100644 --- a/apps/meteor/app/api/server/v1/ldap.ts +++ b/apps/meteor/app/api/server/v1/ldap.ts @@ -20,8 +20,8 @@ API.v1.addRoute( try { await LDAP.testConnection(); - } catch (error) { - SystemLogger.error(error); + } catch (err) { + SystemLogger.error({ err }); throw new Error('Connection_failed'); } diff --git a/apps/meteor/app/api/server/v1/misc.ts b/apps/meteor/app/api/server/v1/misc.ts index 403ffcc29b4ce..192a3e3579dc0 100644 --- a/apps/meteor/app/api/server/v1/misc.ts +++ b/apps/meteor/app/api/server/v1/misc.ts @@ -519,7 +519,7 @@ API.v1.addRoute( return API.v1.success(mountResult({ id, result })); } catch (err) { if (!(err as any).isClientSafe && !(err as any).meteorError) { - SystemLogger.error({ msg: `Exception while invoking method ${method}`, err }); + SystemLogger.error({ msg: 'Exception while invoking method', err, method }); } if (settings.get('Log_Level') === '2') { @@ -576,7 +576,7 @@ API.v1.addRoute( return API.v1.success(mountResult({ id, result })); } catch (err) { if (!(err as any).isClientSafe && !(err as any).meteorError) { - SystemLogger.error({ msg: `Exception while invoking method ${method}`, err }); + SystemLogger.error({ msg: 'Exception while invoking method', err, method }); } if (settings.get('Log_Level') === '2') { Meteor._debug(`Exception while invoking method ${method}`, err); diff --git a/apps/meteor/app/authentication/server/lib/logLoginAttempts.ts b/apps/meteor/app/authentication/server/lib/logLoginAttempts.ts index 0f97796aa4a75..3c50937599063 100644 --- a/apps/meteor/app/authentication/server/lib/logLoginAttempts.ts +++ b/apps/meteor/app/authentication/server/lib/logLoginAttempts.ts @@ -26,7 +26,12 @@ export const logFailedLoginAttempts = (login: ILoginAttempt): void => { if (!settings.get('Login_Logs_UserAgent')) { userAgent = '-'; } - SystemLogger.info( - `Failed login detected - Username[${user}] ClientAddress[${clientAddress}] ForwardedFor[${forwardedFor}] XRealIp[${realIp}] UserAgent[${userAgent}]`, - ); + SystemLogger.info({ + msg: 'Failed login detected', + user, + clientAddress, + forwardedFor, + realIp, + userAgent, + }); }; diff --git a/apps/meteor/app/cloud/server/index.ts b/apps/meteor/app/cloud/server/index.ts index e7214295225b1..7bc7696d5b0dc 100644 --- a/apps/meteor/app/cloud/server/index.ts +++ b/apps/meteor/app/cloud/server/index.ts @@ -23,36 +23,36 @@ Meteor.startup(async () => { } console.log('Successfully registered with token provided by REG_TOKEN!'); - } catch (e: any) { - SystemLogger.error('An error occurred registering with token.', e.message); + } catch (err: any) { + SystemLogger.error({ msg: 'An error occurred registering with token.', err }); } } setImmediate(async () => { try { await syncWorkspace(); - } catch (e: any) { - if (e instanceof CloudWorkspaceAccessTokenEmptyError) { + } catch (err: any) { + if (err instanceof CloudWorkspaceAccessTokenEmptyError) { return; } - if (e.type && e.type === 'AbortError') { + if (err.type && err.type === 'AbortError') { return; } - SystemLogger.error('An error occurred syncing workspace.', e.message); + SystemLogger.error({ msg: 'An error occurred syncing workspace.', err }); } }); const minute = Math.floor(Math.random() * 60); await cronJobs.add(licenseCronName, `${minute} */12 * * *`, async () => { try { await syncWorkspace(); - } catch (e: any) { - if (e instanceof CloudWorkspaceAccessTokenEmptyError) { + } catch (err: any) { + if (err instanceof CloudWorkspaceAccessTokenEmptyError) { return; } - if (e.type && e.type === 'AbortError') { + if (err.type && err.type === 'AbortError') { return; } - SystemLogger.error('An error occurred syncing workspace.', e.message); + SystemLogger.error({ msg: 'An error occurred syncing workspace.', err }); } }); }); diff --git a/apps/meteor/app/crowd/server/crowd.ts b/apps/meteor/app/crowd/server/crowd.ts index ac1467dedbe00..8e499b06f77c6 100644 --- a/apps/meteor/app/crowd/server/crowd.ts +++ b/apps/meteor/app/crowd/server/crowd.ts @@ -392,8 +392,8 @@ Accounts.registerLoginHandler('crowd', async function (this: typeof Accounts, lo return result; } catch (err: any) { - logger.debug({ err }); - logger.error('Crowd user not authenticated due to an error'); + logger.error({ msg: 'Crowd user not authenticated due to an error', err }); + throw new Meteor.Error('user-not-found', err.message); } }); diff --git a/apps/meteor/app/file-upload/server/methods/sendFileMessage.ts b/apps/meteor/app/file-upload/server/methods/sendFileMessage.ts index fd503b31644ce..fbad271ed0438 100644 --- a/apps/meteor/app/file-upload/server/methods/sendFileMessage.ts +++ b/apps/meteor/app/file-upload/server/methods/sendFileMessage.ts @@ -101,8 +101,8 @@ export const parseFileIntoMessageAttachments = async ( typeGroup: thumbnail.typeGroup || '', }); } - } catch (e) { - SystemLogger.error(e); + } catch (err) { + SystemLogger.error({ err }); } attachments.push(attachment); } else if (/^audio\/.+/.test(file.type as string)) { diff --git a/apps/meteor/app/file-upload/ufs/AmazonS3/server.ts b/apps/meteor/app/file-upload/ufs/AmazonS3/server.ts index 3cc3f04f9ccb3..9e00e4ea497f3 100644 --- a/apps/meteor/app/file-upload/ufs/AmazonS3/server.ts +++ b/apps/meteor/app/file-upload/ufs/AmazonS3/server.ts @@ -128,7 +128,7 @@ class AmazonS3Store extends UploadFS.Store { try { return s3.deleteObject(params).promise(); } catch (err: any) { - SystemLogger.error(err); + SystemLogger.error({ err }); } }; @@ -184,9 +184,9 @@ class AmazonS3Store extends UploadFS.Store { ContentType: file.type, Bucket: classOptions.connection.params.Bucket, }, - (error) => { - if (error) { - SystemLogger.error(error); + (err) => { + if (err) { + SystemLogger.error({ err }); } writeStream.emit('real_finish'); diff --git a/apps/meteor/app/file-upload/ufs/GoogleStorage/server.ts b/apps/meteor/app/file-upload/ufs/GoogleStorage/server.ts index 2034ea2135706..e2b71ac8052d7 100644 --- a/apps/meteor/app/file-upload/ufs/GoogleStorage/server.ts +++ b/apps/meteor/app/file-upload/ufs/GoogleStorage/server.ts @@ -103,7 +103,7 @@ class GoogleStorageStore extends UploadFS.Store { try { return bucket.file(this.getPath(file)).delete(); } catch (err: any) { - SystemLogger.error(err); + SystemLogger.error({ err }); } }; diff --git a/apps/meteor/app/file-upload/ufs/Webdav/server.ts b/apps/meteor/app/file-upload/ufs/Webdav/server.ts index 69ab18a4ebb08..e5a8a62b5d059 100644 --- a/apps/meteor/app/file-upload/ufs/Webdav/server.ts +++ b/apps/meteor/app/file-upload/ufs/Webdav/server.ts @@ -94,7 +94,7 @@ class WebdavStore extends UploadFS.Store { try { return client.deleteFile(this.getPath(file)); } catch (err: any) { - SystemLogger.error(err); + SystemLogger.error({ err }); } }; diff --git a/apps/meteor/app/github/server/index.ts b/apps/meteor/app/github/server/index.ts new file mode 100644 index 0000000000000..cf327e4971bb2 --- /dev/null +++ b/apps/meteor/app/github/server/index.ts @@ -0,0 +1 @@ +import './lib'; diff --git a/apps/meteor/app/github/server/lib.ts b/apps/meteor/app/github/server/lib.ts new file mode 100644 index 0000000000000..abdc87419956a --- /dev/null +++ b/apps/meteor/app/github/server/lib.ts @@ -0,0 +1,19 @@ +import type { OauthConfig } from '@rocket.chat/core-typings'; + +import { CustomOAuth } from '../../custom-oauth/server/custom_oauth_server'; + +const config: OauthConfig = { + serverURL: 'https://github.com', + identityPath: 'https://api.github.com/user', + tokenPath: 'https://github.com/login/oauth/access_token', + scope: 'user:email', + mergeUsers: false, + addAutopublishFields: { + forLoggedInUser: ['services.github'], + forOtherUsers: ['services.github.username'], + }, + accessTokenParam: 'access_token', + identityTokenSentVia: 'header', +}; + +export const Github = new CustomOAuth('github', config); diff --git a/apps/meteor/app/importer/server/classes/converters/MessageConverter.ts b/apps/meteor/app/importer/server/classes/converters/MessageConverter.ts index 8fa9eaba04534..825090147be8a 100644 --- a/apps/meteor/app/importer/server/classes/converters/MessageConverter.ts +++ b/apps/meteor/app/importer/server/classes/converters/MessageConverter.ts @@ -41,9 +41,8 @@ export class MessageConverter extends RecordConverter { for await (const rid of this.rids) { try { await Rooms.resetLastMessageById(rid, null); - } catch (e) { - this._logger.warn({ msg: 'Failed to update last message of room', roomId: rid }); - this._logger.error(e); + } catch (err) { + this._logger.error({ msg: 'Failed to update last message of room', roomId: rid, err }); } } } @@ -70,9 +69,8 @@ export class MessageConverter extends RecordConverter { try { await insertMessage(creator, msgObj as unknown as IDBMessage, rid, true); - } catch (e) { - this._logger.warn({ msg: 'Failed to import message', timestamp: msgObj.ts, roomId: rid }); - this._logger.error(e); + } catch (err) { + this._logger.error({ msg: 'Failed to import message', timestamp: msgObj.ts, roomId: rid, err }); } } @@ -167,7 +165,7 @@ export class MessageConverter extends RecordConverter { } if (!data.username) { - this._logger.debug(importId); + this._logger.debug({ msg: 'Mentioned user has no username', importId }); throw new Error('importer-message-mentioned-username-not-found'); } diff --git a/apps/meteor/app/integrations/server/lib/triggerHandler.ts b/apps/meteor/app/integrations/server/lib/triggerHandler.ts index 0a29396ec2c32..abf3437f1bed9 100644 --- a/apps/meteor/app/integrations/server/lib/triggerHandler.ts +++ b/apps/meteor/app/integrations/server/lib/triggerHandler.ts @@ -158,9 +158,10 @@ class RocketChatIntegrationHandler { // If no room could be found, we won't be sending any messages but we'll warn in the logs if (!tmpRoom) { - outgoingLogger.warn( - `The Integration "${trigger.name}" doesn't have a room configured nor did it provide a room to send the message to.`, - ); + outgoingLogger.warn({ + msg: 'The Integration doesnt have a room configured nor did it provide a room to send the message to.', + integrationName: trigger.name, + }); return; } diff --git a/apps/meteor/app/settings/server/CachedSettings.ts b/apps/meteor/app/settings/server/CachedSettings.ts index 6a16a4c761313..3c46dd05a6806 100644 --- a/apps/meteor/app/settings/server/CachedSettings.ts +++ b/apps/meteor/app/settings/server/CachedSettings.ts @@ -108,7 +108,7 @@ export class CachedSettings */ public override has(_id: ISetting['_id']): boolean { if (!this.ready && warn) { - SystemLogger.warn(`Settings not initialized yet. getting: ${_id}`); + SystemLogger.warn({ msg: 'Settings not initialized yet. getting', _id }); } return this.store.has(_id); } @@ -120,7 +120,7 @@ export class CachedSettings */ public getSetting(_id: ISetting['_id']): ISetting | undefined { if (!this.ready && warn) { - SystemLogger.warn(`Settings not initialized yet. getting: ${_id}`); + SystemLogger.warn({ msg: 'Settings not initialized yet. getting', _id }); } return this.store.get(_id); } @@ -134,7 +134,7 @@ export class CachedSettings */ public get(_id: ISetting['_id']): T { if (!this.ready && warn) { - SystemLogger.warn(`Settings not initialized yet. getting: ${_id}`); + SystemLogger.warn({ msg: 'Settings not initialized yet. getting', _id }); } return this.store.get(_id)?.value as T; } @@ -148,7 +148,7 @@ export class CachedSettings */ public getByRegexp(_id: RegExp): [string, T][] { if (!this.ready && warn) { - SystemLogger.warn(`Settings not initialized yet. getting: ${_id}`); + SystemLogger.warn({ msg: 'Settings not initialized yet. getting', _id }); } return [...this.store.entries()].filter(([key]) => _id.test(key)).map(([key, setting]) => [key, setting.value]) as [string, T][]; diff --git a/apps/meteor/app/settings/server/SettingsRegistry.ts b/apps/meteor/app/settings/server/SettingsRegistry.ts index cb6f9da20ab97..3b93ca5c0bfb6 100644 --- a/apps/meteor/app/settings/server/SettingsRegistry.ts +++ b/apps/meteor/app/settings/server/SettingsRegistry.ts @@ -132,7 +132,7 @@ export class SettingsRegistry { ); if (isSettingEnterprise(settingFromCode) && !('invalidValue' in settingFromCode)) { - SystemLogger.error(`Enterprise setting ${_id} is missing the invalidValue option`); + SystemLogger.error({ msg: 'Enterprise setting is missing the invalidValue option', _id }); throw new Error(`Enterprise setting ${_id} is missing the invalidValue option`); } @@ -145,7 +145,7 @@ export class SettingsRegistry { try { validateSetting(settingFromCode._id, settingFromCode.type, settingFromCode.value); } catch (e) { - IS_DEVELOPMENT && SystemLogger.error(`Invalid setting code ${_id}: ${(e as Error).message}`); + IS_DEVELOPMENT && SystemLogger.error({ msg: 'Invalid setting code', _id, err: e as Error }); } const isOverwritten = settingFromCode !== settingFromCodeOverwritten || (settingStored && settingStored !== settingStoredOverwritten); @@ -189,7 +189,7 @@ export class SettingsRegistry { try { validateSetting(settingFromCode._id, settingFromCode.type, settingStored?.value); } catch (e) { - IS_DEVELOPMENT && SystemLogger.error(`Invalid setting stored ${_id}: ${(e as Error).message}`); + IS_DEVELOPMENT && SystemLogger.error({ msg: 'Invalid setting stored', _id, err: e as Error }); } return; } diff --git a/apps/meteor/client/hooks/useAppSlashCommands.spec.tsx b/apps/meteor/client/hooks/useAppSlashCommands.spec.tsx index 058e2c8dc4d3f..0da074aa1de3d 100644 --- a/apps/meteor/client/hooks/useAppSlashCommands.spec.tsx +++ b/apps/meteor/client/hooks/useAppSlashCommands.spec.tsx @@ -5,6 +5,7 @@ import { renderHook, waitFor } from '@testing-library/react'; import { useAppSlashCommands } from './useAppSlashCommands'; import { slashCommands } from '../../app/utils/client/slashCommand'; +import { appsQueryKeys } from '../lib/queryKeys'; const mockSlashCommands: SlashCommand[] = [ { @@ -30,6 +31,7 @@ const mockSlashCommands: SlashCommand[] = [ const mockApiResponse = { commands: mockSlashCommands, total: mockSlashCommands.length, + appsLoaded: true, }; describe('useAppSlashCommands', () => { @@ -96,7 +98,7 @@ describe('useAppSlashCommands', () => { expect(slashCommands.commands['/test']).toBeUndefined(); await waitFor(() => { - expect(queryClient.invalidateQueries).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ['apps', 'slashCommands'] })); + expect(queryClient.invalidateQueries).toHaveBeenCalledWith(expect.objectContaining({ queryKey: appsQueryKeys.slashCommands() })); }); }); @@ -123,7 +125,7 @@ describe('useAppSlashCommands', () => { expect(slashCommands.commands['/test']).toBeUndefined(); await waitFor(() => { - expect(queryClient.invalidateQueries).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ['apps', 'slashCommands'] })); + expect(queryClient.invalidateQueries).toHaveBeenCalledWith(expect.objectContaining({ queryKey: appsQueryKeys.slashCommands() })); }); }); @@ -155,6 +157,7 @@ describe('useAppSlashCommands', () => { }, ], total: mockSlashCommands.length + 1, + appsLoaded: true, }); streamRef.controller?.emit('apps', [['command/added', ['/newcommand']]]); @@ -188,13 +191,39 @@ describe('useAppSlashCommands', () => { streamRef.controller?.emit('apps', [['command/updated', ['/test']]]); await waitFor(() => { - expect(queryClient.invalidateQueries).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ['apps', 'slashCommands'] })); + expect(queryClient.invalidateQueries).toHaveBeenCalledWith(expect.objectContaining({ queryKey: appsQueryKeys.slashCommands() })); }); expect(slashCommands.commands['/test']).toBeDefined(); expect(slashCommands.commands['/weather']).toBeDefined(); }); + it('should ignore events that do not start with command/', async () => { + const streamRef: StreamControllerRef<'apps'> = {}; + + renderHook(() => useAppSlashCommands(), { + wrapper: mockAppRoot() + .withJohnDoe() + .withQueryClient(queryClient) + .withStream('apps', streamRef) + .withEndpoint('GET', '/v1/commands.list', mockGetSlashCommands) + .build(), + }); + + expect(streamRef.controller).toBeDefined(); + + await waitFor(() => { + expect(Object.keys(slashCommands.commands)).toHaveLength(mockSlashCommands.length); + }); + + // @ts-expect-error - testing invalid event + streamRef.controller?.emit('apps', [['some/random/event', ['/test']]]); + + await new Promise((resolve) => setTimeout(resolve, 200)); + + expect(queryClient.invalidateQueries).not.toHaveBeenCalled(); + }); + it('should not set up stream listener when user ID is not available', () => { const streamRef: StreamControllerRef<'apps'> = {}; @@ -221,6 +250,7 @@ describe('useAppSlashCommands', () => { return Promise.resolve({ commands: largeMockCommands.slice(offset, offset + count), total: largeMockCommands.length, + appsLoaded: true, }); }); @@ -232,4 +262,24 @@ describe('useAppSlashCommands', () => { expect(Object.keys(slashCommands.commands)).toHaveLength(largeMockCommands.length); }); }); + + it('should not load commands when apps are not loaded', async () => { + mockGetSlashCommands.mockResolvedValue({ + commands: [], + total: 0, + appsLoaded: false, + }); + + expect(Object.keys(slashCommands.commands)).toHaveLength(0); + + renderHook(() => useAppSlashCommands(), { + wrapper: mockAppRoot().withJohnDoe().withEndpoint('GET', '/v1/commands.list', mockGetSlashCommands).build(), + }); + + await waitFor(() => { + expect(mockGetSlashCommands).toHaveBeenCalled(); + }); + + expect(Object.keys(slashCommands.commands)).toHaveLength(0); + }); }); diff --git a/apps/meteor/client/hooks/useAppSlashCommands.ts b/apps/meteor/client/hooks/useAppSlashCommands.ts index 0f2a1123c0b93..c1a6d5e33b7f8 100644 --- a/apps/meteor/client/hooks/useAppSlashCommands.ts +++ b/apps/meteor/client/hooks/useAppSlashCommands.ts @@ -48,10 +48,17 @@ export const useAppSlashCommands = () => { queryKey: appsQueryKeys.slashCommands(), enabled: !!uid, structuralSharing: false, + retry: true, + // Add a bit of randomness to avoid thundering herd problem + retryDelay: (attemptIndex) => Math.min(500 * Math.random() * 10 * 2 ** attemptIndex, 30000), queryFn: async () => { const fetchBatch = async (currentOffset: number, accumulator: SlashCommandBasicInfo[] = []): Promise => { const count = 50; - const { commands, total } = await getSlashCommands({ offset: currentOffset, count }); + const { commands, appsLoaded, total } = await getSlashCommands({ offset: currentOffset, count }); + + if (!appsLoaded) { + throw new Error('Apps not loaded, retry later'); + } const newAccumulator = [...accumulator, ...commands]; diff --git a/apps/meteor/client/meteor/login/github.ts b/apps/meteor/client/meteor/login/github.ts deleted file mode 100644 index 98c8fb8fea76d..0000000000000 --- a/apps/meteor/client/meteor/login/github.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { Random } from '@rocket.chat/random'; -import { Accounts } from 'meteor/accounts-base'; -// eslint-disable-next-line import/no-duplicates -import { Github } from 'meteor/github-oauth'; -import { Meteor } from 'meteor/meteor'; -// eslint-disable-next-line import/no-duplicates -import { OAuth } from 'meteor/oauth'; - -import { createOAuthTotpLoginMethod } from './oauth'; -import { overrideLoginMethod } from '../../lib/2fa/overrideLoginMethod'; -import { wrapRequestCredentialFn } from '../../lib/wrapRequestCredentialFn'; - -const { loginWithGithub } = Meteor; -const loginWithGithubAndTOTP = createOAuthTotpLoginMethod(Github); -Meteor.loginWithGithub = (options, callback) => { - overrideLoginMethod(loginWithGithub, [options], callback, loginWithGithubAndTOTP); -}; - -Github.requestCredential = wrapRequestCredentialFn('github', ({ config, loginStyle, options, credentialRequestCompleteCallback }) => { - const credentialToken = Random.secret(); - const scope = options?.requestPermissions || ['user:email']; - const flatScope = scope.map(encodeURIComponent).join('+'); - - let allowSignup = ''; - if (Accounts._options?.forbidClientAccountCreation) { - allowSignup = '&allow_signup=false'; - } - - const loginUrl = - `https://github.com/login/oauth/authorize` + - `?client_id=${config.clientId}` + - `&scope=${flatScope}` + - `&redirect_uri=${OAuth._redirectUri('github', config)}` + - `&state=${OAuth._stateParam(loginStyle, credentialToken, options.redirectUrl)}${allowSignup}`; - - OAuth.launchLogin({ - loginService: 'github', - loginStyle, - loginUrl, - credentialRequestCompleteCallback, - credentialToken, - popupOptions: { width: 900, height: 450 }, - }); -}); diff --git a/apps/meteor/client/meteor/login/index.ts b/apps/meteor/client/meteor/login/index.ts index cef3570085f43..d49f3653de683 100644 --- a/apps/meteor/client/meteor/login/index.ts +++ b/apps/meteor/client/meteor/login/index.ts @@ -1,7 +1,6 @@ import './cas'; import './crowd'; import './facebook'; -import './github'; import './google'; import './ldap'; import './meteorDeveloperAccount'; diff --git a/apps/meteor/client/views/root/AppLayout.tsx b/apps/meteor/client/views/root/AppLayout.tsx index 17c8561ca489b..7a62fff7b9c5b 100644 --- a/apps/meteor/client/views/root/AppLayout.tsx +++ b/apps/meteor/client/views/root/AppLayout.tsx @@ -8,6 +8,7 @@ import { useDolphinOAuth } from './hooks/customOAuth/useDolphinOAuth'; import { useDrupalOAuth } from './hooks/customOAuth/useDrupalOAuth'; import { useGitHubEnterpriseOAuth } from './hooks/customOAuth/useGitHubEnterpriseOAuth'; import { useGitLabOAuth } from './hooks/customOAuth/useGitLabOAuth'; +import { useGithubOAuth } from './hooks/customOAuth/useGithubOAuth'; import { useNextcloudOAuth } from './hooks/customOAuth/useNextcloudOAuth'; import { useWordPressOAuth } from './hooks/customOAuth/useWordPressOAuth'; import { useAnalytics } from './hooks/useAnalytics'; @@ -56,6 +57,7 @@ const AppLayout = () => { useLivechatEnterprise(); useNextcloudOAuth(); useGitLabOAuth(); + useGithubOAuth(); useGitHubEnterpriseOAuth(); useDrupalOAuth(); useDolphinOAuth(); diff --git a/apps/meteor/client/views/root/hooks/customOAuth/useGithubOAuth.ts b/apps/meteor/client/views/root/hooks/customOAuth/useGithubOAuth.ts new file mode 100644 index 0000000000000..acf799e27344f --- /dev/null +++ b/apps/meteor/client/views/root/hooks/customOAuth/useGithubOAuth.ts @@ -0,0 +1,33 @@ +import type { OauthConfig } from '@rocket.chat/core-typings'; +import { useSetting } from '@rocket.chat/ui-contexts'; +import { useEffect } from 'react'; + +import { CustomOAuth } from '../../../../lib/customOAuth/CustomOAuth'; + +const config: OauthConfig = { + authorizePath: 'https://github.com/login/oauth/authorize', + serverURL: 'https://github.com', + identityPath: 'https://api.github.com/user', + tokenPath: 'https://github.com/login/oauth/access_token', + scope: 'user:email', + mergeUsers: false, + addAutopublishFields: { + forLoggedInUser: ['services.github'], + forOtherUsers: ['services.github.username'], + }, + accessTokenParam: 'access_token', +} as const satisfies OauthConfig; + +const Github = CustomOAuth.configureOAuthService('github', config); + +export const useGithubOAuth = () => { + const enabled = useSetting('Accounts_OAuth_Github'); + + useEffect(() => { + if (enabled) { + Github.configure({ + ...config, + }); + } + }, [enabled]); +}; diff --git a/apps/meteor/definition/externals/meteor/github-oauth.d.ts b/apps/meteor/definition/externals/meteor/github-oauth.d.ts deleted file mode 100644 index ac67405bd4e4c..0000000000000 --- a/apps/meteor/definition/externals/meteor/github-oauth.d.ts +++ /dev/null @@ -1,3 +0,0 @@ -declare module 'meteor/github-oauth' { - export const Github: any; -} diff --git a/apps/meteor/ee/app/livechat-enterprise/server/hooks/afterOnHold.ts b/apps/meteor/ee/app/livechat-enterprise/server/hooks/afterOnHold.ts index bb38b53773602..89a28af413420 100644 --- a/apps/meteor/ee/app/livechat-enterprise/server/hooks/afterOnHold.ts +++ b/apps/meteor/ee/app/livechat-enterprise/server/hooks/afterOnHold.ts @@ -18,7 +18,7 @@ const handleAfterOnHold = async (room: Pick): Promise('Livechat_auto_close_on_hold_chats_custom_message') || i18n.t('Closed_automatically_because_chat_was_onhold_for_seconds', { diff --git a/apps/meteor/ee/app/livechat-enterprise/server/hooks/afterOnHoldChatResumed.ts b/apps/meteor/ee/app/livechat-enterprise/server/hooks/afterOnHoldChatResumed.ts index 9ebdfcacb539a..e114fd89ec435 100644 --- a/apps/meteor/ee/app/livechat-enterprise/server/hooks/afterOnHoldChatResumed.ts +++ b/apps/meteor/ee/app/livechat-enterprise/server/hooks/afterOnHoldChatResumed.ts @@ -13,7 +13,7 @@ const handleAfterOnHoldChatResumed = async (room: IRoom): Promise => { const { _id: roomId } = room; - cbLogger.debug(`Removing current on hold timers for room ${roomId}`); + cbLogger.debug({ msg: 'Removing current on hold timers for room', roomId }); await AutoCloseOnHoldScheduler.unscheduleRoom(roomId); return room; diff --git a/apps/meteor/ee/app/livechat-enterprise/server/hooks/checkAgentBeforeTakeInquiry.ts b/apps/meteor/ee/app/livechat-enterprise/server/hooks/checkAgentBeforeTakeInquiry.ts index a76cc7871f45b..349ad44652bf1 100644 --- a/apps/meteor/ee/app/livechat-enterprise/server/hooks/checkAgentBeforeTakeInquiry.ts +++ b/apps/meteor/ee/app/livechat-enterprise/server/hooks/checkAgentBeforeTakeInquiry.ts @@ -38,12 +38,12 @@ const validateMaxChats = async ({ } if (!settings.get('Livechat_waiting_queue')) { - cbLogger.info(`Chat can be taken by Agent ${agentId}: waiting queue is disabled`); + cbLogger.info({ msg: 'Chat can be taken by Agent: waiting queue is disabled', agentId }); return agent; } if (await allowAgentSkipQueue(agent)) { - cbLogger.info(`Chat can be taken by Agent ${agentId}: agent can skip queue`); + cbLogger.info({ msg: 'Chat can be taken by Agent: agent can skip queue', agentId }); return agent; } diff --git a/apps/meteor/ee/app/livechat-enterprise/server/hooks/resumeOnHold.ts b/apps/meteor/ee/app/livechat-enterprise/server/hooks/resumeOnHold.ts index 85239c4fe1fc9..b35b9e8acce5f 100644 --- a/apps/meteor/ee/app/livechat-enterprise/server/hooks/resumeOnHold.ts +++ b/apps/meteor/ee/app/livechat-enterprise/server/hooks/resumeOnHold.ts @@ -16,7 +16,7 @@ const resumeOnHoldCommentAndUser = async (room: IOmnichannelRoom): Promise<{ com projection: { name: 1, username: 1 }, }); if (!visitor) { - callbackLogger.error(`[afterOmnichannelSaveMessage] Visitor Not found for room ${rid} while trying to resume on hold`); + callbackLogger.error({ msg: '[afterOmnichannelSaveMessage] Visitor Not found for room while trying to resume on hold', rid }); throw new Error('Visitor not found while trying to resume on hold'); } @@ -26,7 +26,7 @@ const resumeOnHoldCommentAndUser = async (room: IOmnichannelRoom): Promise<{ com const resumedBy = await Users.findOneById('rocket.cat'); if (!resumedBy) { - callbackLogger.error(`[afterOmnichannelSaveMessage] User Not found for room ${rid} while trying to resume on hold`); + callbackLogger.error({ msg: '[afterOmnichannelSaveMessage] User Not found for room while trying to resume on hold', rid }); throw new Error(`User not found while trying to resume on hold`); } @@ -53,13 +53,13 @@ callbacks.add( } if (isMessageFromVisitor(message) && room.onHold) { - callbackLogger.debug(`[afterOmnichannelSaveMessage] Room ${rid} is on hold, resuming it now since visitor sent a message`); + callbackLogger.debug({ msg: '[afterOmnichannelSaveMessage] Room is on hold, resuming it now since visitor sent a message', rid }); try { const { comment: resumeChatComment, resumedBy } = await resumeOnHoldCommentAndUser(updatedRoom); await OmnichannelEEService.resumeRoomOnHold(updatedRoom, resumeChatComment, resumedBy); } catch (error) { - callbackLogger.error(`[afterOmnichannelSaveMessage] Error while resuming room ${rid} on hold: Error: `, error); + callbackLogger.error({ msg: '[afterOmnichannelSaveMessage] Error while resuming room on hold', rid, err: error }); return message; } } diff --git a/apps/meteor/server/importPackages.ts b/apps/meteor/server/importPackages.ts index 61cb32f15958d..49af9b1ca237a 100644 --- a/apps/meteor/server/importPackages.ts +++ b/apps/meteor/server/importPackages.ts @@ -79,3 +79,4 @@ import '../app/ui-utils/server'; import '../app/reactions/server'; import '../app/livechat/server'; import '../app/authentication/server'; +import '../app/github/server'; diff --git a/apps/meteor/server/lib/ldap/Manager.ts b/apps/meteor/server/lib/ldap/Manager.ts index 19dd6a516e789..9b747c0f78fd9 100644 --- a/apps/meteor/server/lib/ldap/Manager.ts +++ b/apps/meteor/server/lib/ldap/Manager.ts @@ -241,7 +241,7 @@ export class LDAPManager { // Do a search as the user and check if they have any result authLogger.debug('User authenticated successfully, performing additional search.'); if ((await ldap.searchAndCount(ldapUser.dn, {})) === 0) { - authLogger.debug(`Bind successful but user ${ldapUser.dn} was not found via search`); + authLogger.debug({ msg: 'Bind successful but user was not found via search', dn: ldapUser.dn }); } } return ldapUser; @@ -267,7 +267,7 @@ export class LDAPManager { // Do a search as the user and check if they have any result authLogger.debug('User authenticated successfully, performing additional search.'); if ((await ldap.searchAndCount(ldapUser.dn, {})) === 0) { - authLogger.debug(`Bind successful but user ${ldapUser.dn} was not found via search`); + authLogger.debug({ msg: 'Bind successful but user was not found via search', dn: ldapUser.dn }); } } diff --git a/apps/meteor/server/modules/streamer/streamer.module.ts b/apps/meteor/server/modules/streamer/streamer.module.ts index 01cdd6861600d..e55592fb2e907 100644 --- a/apps/meteor/server/modules/streamer/streamer.module.ts +++ b/apps/meteor/server/modules/streamer/streamer.module.ts @@ -78,7 +78,7 @@ export abstract class Streamer extends EventEmit } if (typeof fn === 'string' && ['all', 'none', 'logged'].indexOf(fn) === -1) { - SystemLogger.error(`${name} shortcut '${fn}' is invalid`); + SystemLogger.error({ msg: 'shortcut is invalid', name, fn }); } if (fn === 'all' || fn === true) { diff --git a/apps/meteor/server/services/omnichannel-analytics/service.ts b/apps/meteor/server/services/omnichannel-analytics/service.ts index 9ad567feaa188..24ce8ded79f2a 100644 --- a/apps/meteor/server/services/omnichannel-analytics/service.ts +++ b/apps/meteor/server/services/omnichannel-analytics/service.ts @@ -55,7 +55,7 @@ export class OmnichannelAnalyticsService extends ServiceClassInternal implements } if (!this.agentOverview.isActionAllowed(name)) { - serviceLogger.error(`AgentOverview.${name} is not a valid action`); + serviceLogger.error({ msg: 'AgentOverview action is not valid', name }); return; } @@ -74,7 +74,7 @@ export class OmnichannelAnalyticsService extends ServiceClassInternal implements // Check if function exists, prevent server error in case property altered if (!this.chart.isActionAllowed(chartLabel)) { - serviceLogger.error(`ChartData.${chartLabel} is not a valid action`); + serviceLogger.error({ msg: 'ChartData action is not valid', chartLabel }); return; } @@ -164,7 +164,7 @@ export class OmnichannelAnalyticsService extends ServiceClassInternal implements } if (!this.overview.isActionAllowed(name)) { - serviceLogger.error(`OverviewData.${name} is not a valid action`); + serviceLogger.error({ msg: 'OverviewData action is not valid', name }); return; } diff --git a/apps/meteor/server/services/omnichannel-integrations/providers/twilio.ts b/apps/meteor/server/services/omnichannel-integrations/providers/twilio.ts index 26b6dbaea1cf9..fe4f5e30241b0 100644 --- a/apps/meteor/server/services/omnichannel-integrations/providers/twilio.ts +++ b/apps/meteor/server/services/omnichannel-integrations/providers/twilio.ts @@ -78,7 +78,7 @@ export class Twilio implements ISMSProvider { } if (isNaN(numMedia)) { - SystemLogger.error(`Error parsing NumMedia ${data.NumMedia}`); + SystemLogger.error({ msg: 'Error parsing NumMedia', numMedia: data.NumMedia }); return returnData; } @@ -114,7 +114,7 @@ export class Twilio implements ISMSProvider { return twilio(sid, token); } catch (error) { await notifyAgent(userId, rid, i18n.t('SMS_Twilio_InvalidCredentials')); - SystemLogger.error(`(Twilio) -> ${error}`); + SystemLogger.error({ msg: '(Twilio) ->', err: error }); } } @@ -157,7 +157,7 @@ export class Twilio implements ISMSProvider { if (reason) { await notifyAgent(userId, rid, reason); - SystemLogger.error(`(Twilio) -> ${reason}`); + SystemLogger.error({ msg: '(Twilio) ->', reason }); return ''; } @@ -219,7 +219,7 @@ export class Twilio implements ISMSProvider { if (result.errorCode) { await notifyAgent(userId, rid, result.errorMessage); - SystemLogger.error(`(Twilio) -> ${result.errorCode}`); + SystemLogger.error({ msg: '(Twilio) ->', errorCode: result.errorCode }); } return { diff --git a/apps/meteor/server/services/omnichannel/queue.ts b/apps/meteor/server/services/omnichannel/queue.ts index 89acb5434702f..d522c78d7daaa 100644 --- a/apps/meteor/server/services/omnichannel/queue.ts +++ b/apps/meteor/server/services/omnichannel/queue.ts @@ -46,7 +46,7 @@ export class OmnichannelQueue implements IOmnichannelQueue { } const activeQueues = await this.getActiveQueues(); - queueLogger.debug(`Active queues: ${activeQueues.length}`); + queueLogger.debug({ msg: 'Active queues', count: activeQueues.length }); this.running = true; queueLogger.info('Service started'); @@ -118,22 +118,22 @@ export class OmnichannelQueue implements IOmnichannelQueue { } private async checkQueue(queue: string | null) { - queueLogger.debug(`Processing items for queue ${queue || 'Public'}`); + queueLogger.debug({ msg: 'Processing items for queue', queue: queue || 'Public' }); try { const nextInquiry = await LivechatInquiry.findNextAndLock(getOmniChatSortQuery(getInquirySortMechanismSetting()), queue); if (!nextInquiry) { - queueLogger.debug(`No more items for queue ${queue || 'Public'}`); + queueLogger.debug({ msg: 'No more items for queue', queue: queue || 'Public' }); return; } const result = await this.processWaitingQueue(queue, nextInquiry as InquiryWithAgentInfo); if (!result) { - queueLogger.debug(`Inquiry ${nextInquiry._id} not taken. Unlocking and re-queueing`); + queueLogger.debug({ msg: 'Inquiry not taken. Unlocking and re-queueing', inquiry: nextInquiry._id }); return await LivechatInquiry.unlock(nextInquiry._id); } - queueLogger.debug(`Inquiry ${nextInquiry._id} taken successfully. Unlocking`); + queueLogger.debug({ msg: 'Inquiry taken successfully. Unlocking', inquiry: nextInquiry._id }); await LivechatInquiry.unlock(nextInquiry._id); queueLogger.debug({ msg: 'Inquiry processed', @@ -264,7 +264,7 @@ export class OmnichannelQueue implements IOmnichannelQueue { _id: rid, servedBy: { _id: agentId }, } = room; - queueLogger.debug(`Inquiry ${inquiry._id} taken successfully by agent ${agentId}. Notifying`); + queueLogger.debug({ msg: 'Inquiry taken successfully by agent. Notifying', inquiry: inquiry._id, agentId }); setTimeout(() => { void dispatchAgentDelegated(rid, agentId); }, 1000); diff --git a/apps/meteor/server/startup/migrations/v317.ts b/apps/meteor/server/startup/migrations/v317.ts index 02867fcdb5cc7..383f2a47adb0b 100644 --- a/apps/meteor/server/startup/migrations/v317.ts +++ b/apps/meteor/server/startup/migrations/v317.ts @@ -70,7 +70,7 @@ addMigration({ return; } - SystemLogger.warn(`The default value of the setting ${key} has changed to ${newValue}. Please review your settings.`); + SystemLogger.warn({ msg: 'The default value of the setting has changed. Please review your settings.', key, newValue }); return Settings.updateOne({ _id: key }, { $set: { value: newValue } }); }) @@ -91,9 +91,11 @@ addMigration({ return; } - SystemLogger.warn( - `The default value of the custom setting ${_id} has changed to ${newDefaultButtonColor}. Please review your settings.`, - ); + SystemLogger.warn({ + msg: 'The default value of the custom setting has changed. Please review your settings.', + _id, + newValue: newDefaultButtonColor, + }); return Settings.updateOne({ _id }, { $set: { value: newDefaultButtonColor } }); }) @@ -105,9 +107,11 @@ addMigration({ return; } - SystemLogger.warn( - `The default value of the custom setting ${_id} has changed to ${newDefaultButtonLabelColor}. Please review your settings.`, - ); + SystemLogger.warn({ + msg: 'The default value of the custom setting has changed. Please review your settings.', + _id, + newValue: newDefaultButtonLabelColor, + }); return Settings.updateOne({ _id }, { $set: { value: newDefaultButtonLabelColor } }); }) diff --git a/apps/meteor/tests/end-to-end/api/incoming-integrations.ts b/apps/meteor/tests/end-to-end/api/incoming-integrations.ts index 1aa98bd560bf3..a8986b5bddcdc 100644 --- a/apps/meteor/tests/end-to-end/api/incoming-integrations.ts +++ b/apps/meteor/tests/end-to-end/api/incoming-integrations.ts @@ -49,8 +49,19 @@ describe('[Incoming Integrations]', () => { }); describe('[/integrations.create]', () => { - it('should return an error when the user DOES NOT have the permission "manage-incoming-integrations" to add an incoming integration', (done) => { - void updatePermission('manage-incoming-integrations', []).then(() => { + describe('Permission checks', () => { + before(async () => { + await Promise.all([updatePermission('manage-incoming-integrations', []), updatePermission('manage-own-incoming-integrations', [])]); + }); + + after(async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', ['admin']), + updatePermission('manage-own-incoming-integrations', ['admin']), + ]); + }); + + it('should return an error when the user DOES NOT have the permission "manage-incoming-integrations" to add an incoming integration', (done) => { void request .post(api('integrations.create')) .set(credentials) @@ -72,10 +83,8 @@ describe('[Incoming Integrations]', () => { }) .end(done); }); - }); - it('should return an error when the user DOES NOT have the permission "manage-own-incoming-integrations" to add an incoming integration', (done) => { - void updatePermission('manage-own-incoming-integrations', []).then(() => { + it('should return an error when the user DOES NOT have the permission "manage-own-incoming-integrations" to add an incoming integration', (done) => { void request .post(api('integrations.create')) .set(credentials) @@ -99,33 +108,21 @@ describe('[Incoming Integrations]', () => { }); }); - it('should return an error when the user sends an invalid type of integration', (done) => { - void request - .post(api('integrations.create')) - .set(credentials) - .send({ - type: 'webhook-incoming-invalid', - name: 'Incoming test', - enabled: true, - alias: 'test', - username: 'rocket.cat', - scriptEnabled: false, - overrideDestinationChannelEnabled: true, - channel: '#general', - }) - .expect('Content-Type', 'application/json') - .expect(400) - .expect((res) => { - expect(res.body).to.have.property('success', false); - expect(res.body).to.have.property('error', 'Invalid integration type.'); - }) - .end(done); - }); + describe('With manage-incoming-integrations permission', () => { + let tempIntegrationId: IIntegration['_id']; - it('should add the integration successfully when the user ONLY has the permission "manage-incoming-integrations" to add an incoming integration', (done) => { - let integrationId: IIntegration['_id']; - void updatePermission('manage-own-incoming-integrations', ['admin']).then(() => { - void request + before(async () => { + await updatePermission('manage-incoming-integrations', ['admin']); + }); + + after(async () => { + if (tempIntegrationId) { + await removeIntegration(tempIntegrationId, 'incoming'); + } + }); + + it('should add the integration successfully when the user ONLY has the permission "manage-incoming-integrations" to add an incoming integration', async () => { + const res = await request .post(api('integrations.create')) .set(credentials) .send({ @@ -139,339 +136,361 @@ describe('[Incoming Integrations]', () => { channel: '#general', }) .expect('Content-Type', 'application/json') + .expect(200); + + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('integration').and.to.be.an('object'); + tempIntegrationId = res.body.integration._id; + }); + + it('should set overrideDestinationChannelEnabled setting to false when it is not provided', async () => { + const res = await request + .post(api('integrations.create')) + .set(credentials) + .send({ + type: 'webhook-incoming', + name: 'Incoming test', + enabled: true, + alias: 'test', + username: 'rocket.cat', + scriptEnabled: false, + channel: '#general', + }) + .expect('Content-Type', 'application/json') + .expect(200); + + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('integration').and.to.be.an('object'); + expect(res.body.integration).to.have.property('overrideDestinationChannelEnabled', false); + const integrationId = res.body.integration._id; + await removeIntegration(integrationId, 'incoming'); + }); + }); + + describe('With manage-own-incoming-integrations permission', () => { + before(async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', []), + updatePermission('manage-own-incoming-integrations', ['admin']), + ]); + }); + + after(async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', ['admin']), + updatePermission('manage-own-incoming-integrations', ['admin']), + ]); + }); + + it('should add the integration successfully when the user ONLY has the permission "manage-own-incoming-integrations" to add an incoming integration', (done) => { + void request + .post(api('integrations.create')) + .set(credentials) + .send({ + type: 'webhook-incoming', + name: 'Incoming test 2', + enabled: true, + alias: 'test2', + username: 'rocket.cat', + scriptEnabled: false, + overrideDestinationChannelEnabled: false, + channel: '#general', + }) + .expect('Content-Type', 'application/json') .expect(200) .expect((res) => { expect(res.body).to.have.property('success', true); expect(res.body).to.have.property('integration').and.to.be.an('object'); - integrationId = res.body.integration._id; + integration = res.body.integration; }) - .end(() => removeIntegration(integrationId, 'incoming').then(done)); + .end(done); }); }); - it('should set overrideDestinationChannelEnabled setting to false when it is not provided', async () => { - const res = await request - .post(api('integrations.create')) - .set(credentials) - .send({ - type: 'webhook-incoming', - name: 'Incoming test', - enabled: true, - alias: 'test', - username: 'rocket.cat', - scriptEnabled: false, - channel: '#general', - }) - .expect('Content-Type', 'application/json') - .expect(200); + describe('Incoming Integration execution', () => { + it('should return an error when the user sends an invalid type of integration', (done) => { + void request + .post(api('integrations.create')) + .set(credentials) + .send({ + type: 'webhook-incoming-invalid', + name: 'Incoming test', + enabled: true, + alias: 'test', + username: 'rocket.cat', + scriptEnabled: false, + overrideDestinationChannelEnabled: true, + channel: '#general', + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'Invalid integration type.'); + }) + .end(done); + }); - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('integration').and.to.be.an('object'); - expect(res.body.integration).to.have.property('overrideDestinationChannelEnabled', false); - const integrationId = res.body.integration._id; - await removeIntegration(integrationId, 'incoming'); - }); + it('should execute the incoming integration', (done) => { + void request + .post(`/hooks/${integration._id}/${integration.token}`) + .send({ + text: 'Example message', + }) + .expect(200) + .end(done); + }); - it('should add the integration successfully when the user ONLY has the permission "manage-own-incoming-integrations" to add an incoming integration', (done) => { - void updatePermission('manage-incoming-integrations', []).then(() => { - void updatePermission('manage-own-incoming-integrations', ['admin']).then(() => { - void request - .post(api('integrations.create')) - .set(credentials) - .send({ - type: 'webhook-incoming', - name: 'Incoming test 2', - enabled: true, - alias: 'test2', - username: 'rocket.cat', - scriptEnabled: false, - overrideDestinationChannelEnabled: false, - channel: '#general', - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('integration').and.to.be.an('object'); - integration = res.body.integration; - }) - .end(done); - }); + it("should return an error when sending 'channel' field telling its configuration is disabled", (done) => { + void request + .post(`/hooks/${integration._id}/${integration.token}`) + .send({ + text: 'Example message', + channel: [testChannelName], + }) + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'overriding destination channel is disabled for this integration'); + }) + .end(done); }); - }); - it('should execute the incoming integration', (done) => { - void request - .post(`/hooks/${integration._id}/${integration.token}`) - .send({ - text: 'Example message', - }) - .expect(200) - .end(done); - }); + it("should return an error when sending 'roomId' field telling its configuration is disabled", (done) => { + void request + .post(`/hooks/${integration._id}/${integration.token}`) + .send({ + text: 'Example message', + roomId: channel._id, + }) + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'overriding destination channel is disabled for this integration'); + }) + .end(done); + }); + it('should send a message for a channel that is specified in the webhooks configuration', (done) => { + const successfulMessage = `Message sent successfully at #${Date.now()}`; + void request + .post(`/hooks/${integration._id}/${integration.token}`) + .send({ + text: successfulMessage, + }) + .expect(200) + .end(() => { + return request + .get(api('channels.messages')) + .set(credentials) + .query({ + roomId: 'GENERAL', + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('messages').and.to.be.an('array'); + expect(!!(res.body.messages as IMessage[]).find((m) => m.msg === successfulMessage)).to.be.true; + }) + .end(done); + }); + }); + it('should send a message for a channel that is not specified in the webhooks configuration', async () => { + await request + .put(api('integrations.update')) + .set(credentials) + .send({ + type: 'webhook-incoming', + overrideDestinationChannelEnabled: true, + integrationId: integration._id, + username: 'rocket.cat', + channel: '#general', + scriptEnabled: true, + enabled: true, + name: integration.name, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('integration'); + expect(res.body.integration.overrideDestinationChannelEnabled).to.be.equal(true); + }); + const successfulMessage = `Message sent successfully at #${Date.now()}`; + await request + .post(`/hooks/${integration._id}/${integration.token}`) + .send({ + text: successfulMessage, + channel: [testChannelName], + }) + .expect(200); - it("should return an error when sending 'channel' field telling its configuration is disabled", (done) => { - void request - .post(`/hooks/${integration._id}/${integration.token}`) - .send({ - text: 'Example message', - channel: [testChannelName], - }) - .expect(400) - .expect((res) => { - expect(res.body).to.have.property('success', false); - expect(res.body).to.have.property('error', 'overriding destination channel is disabled for this integration'); - }) - .end(done); - }); + return request + .get(api('channels.messages')) + .set(credentials) + .query({ + roomId: channel._id, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('messages').and.to.be.an('array'); + expect(!!(res.body.messages as IMessage[]).find((m) => m.msg === successfulMessage)).to.be.true; + }); + }); - it("should return an error when sending 'roomId' field telling its configuration is disabled", (done) => { - void request - .post(`/hooks/${integration._id}/${integration.token}`) - .send({ - text: 'Example message', - roomId: channel._id, - }) - .expect(400) - .expect((res) => { - expect(res.body).to.have.property('success', false); - expect(res.body).to.have.property('error', 'overriding destination channel is disabled for this integration'); - }) - .end(done); - }); - it('should send a message for a channel that is specified in the webhooks configuration', (done) => { - const successfulMesssage = `Message sent successfully at #${Date.now()}`; - void request - .post(`/hooks/${integration._id}/${integration.token}`) - .send({ - text: successfulMesssage, - }) - .expect(200) - .end(() => { - return request - .get(api('channels.messages')) - .set(credentials) - .query({ - roomId: 'GENERAL', - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('messages').and.to.be.an('array'); - expect(!!(res.body.messages as IMessage[]).find((m) => m.msg === successfulMesssage)).to.be.true; - }) - .end(done); - }); - }); - it('should send a message for a channel that is not specified in the webhooks configuration', async () => { - await request - .put(api('integrations.update')) - .set(credentials) - .send({ - type: 'webhook-incoming', - overrideDestinationChannelEnabled: true, - integrationId: integration._id, - username: 'rocket.cat', - channel: '#general', - scriptEnabled: true, - enabled: true, - name: integration.name, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('integration'); - expect(res.body.integration.overrideDestinationChannelEnabled).to.be.equal(true); - }); - const successfulMesssage = `Message sent successfully at #${Date.now()}`; - await request - .post(`/hooks/${integration._id}/${integration.token}`) - .send({ - text: successfulMesssage, - channel: [testChannelName], - }) - .expect(200); + it('should send a message if the payload is a application/x-www-form-urlencoded JSON', async () => { + const payload = { msg: `Message as x-www-form-urlencoded JSON sent successfully at #${Date.now()}` }; - return request - .get(api('channels.messages')) - .set(credentials) - .query({ - roomId: channel._id, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('messages').and.to.be.an('array'); - expect(!!(res.body.messages as IMessage[]).find((m) => m.msg === successfulMesssage)).to.be.true; - }); + await request + .post(`/hooks/${integration._id}/${integration.token}`) + .set('Content-Type', 'application/x-www-form-urlencoded') + .send(`payload=${JSON.stringify(payload)}`) + .expect(200) + .expect(async () => { + return request + .get(api('channels.messages')) + .set(credentials) + .query({ + roomId: 'GENERAL', + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('messages').and.to.be.an('array'); + expect(!!(res.body.messages as IMessage[]).find((m) => m.msg === payload.msg)).to.be.true; + }); + }); + }); }); - it('should send a message if the payload is a application/x-www-form-urlencoded JSON', async () => { - const payload = { msg: `Message as x-www-form-urlencoded JSON sent successfully at #${Date.now()}` }; + describe('Script integration tests', () => { + let withScript: IIntegration; + let withScriptDefaultContentType: IIntegration; - await request - .post(`/hooks/${integration._id}/${integration.token}`) - .set('Content-Type', 'application/x-www-form-urlencoded') - .send(`payload=${JSON.stringify(payload)}`) - .expect(200) - .expect(async () => { - return request - .get(api('channels.messages')) - .set(credentials) - .query({ - roomId: 'GENERAL', - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('messages').and.to.be.an('array'); - expect(!!(res.body.messages as IMessage[]).find((m) => m.msg === payload.msg)).to.be.true; - }); - }); - }); - - it('should send a message if the payload is a application/x-www-form-urlencoded JSON AND the integration has a valid script', async () => { - const payload = { msg: `Message as x-www-form-urlencoded JSON sent successfully at #${Date.now()}` }; - let withScript: IIntegration | undefined; + before(async () => { + await updatePermission('manage-incoming-integrations', ['admin']); - await updatePermission('manage-incoming-integrations', ['admin']); - await request - .post(api('integrations.create')) - .set(credentials) - .send({ - type: 'webhook-incoming', - name: 'Incoming test with script', - enabled: true, - alias: 'test', - username: 'rocket.cat', - scriptEnabled: true, - overrideDestinationChannelEnabled: false, - channel: '#general', - script: ` - class Script { - process_incoming_request({ request }) { - return { - content:{ - text: request.content.text - } - }; + const res1 = await request + .post(api('integrations.create')) + .set(credentials) + .send({ + type: 'webhook-incoming', + name: 'Incoming test with script', + enabled: true, + alias: 'test', + username: 'rocket.cat', + scriptEnabled: true, + overrideDestinationChannelEnabled: false, + channel: '#general', + script: ` + class Script { + process_incoming_request({ request }) { + return { + content:{ + text: request.content.text + } + }; + } } - } - `, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('integration').and.to.be.an('object'); - withScript = res.body.integration; - }); - - if (!withScript) { - throw new Error('Integration not created'); - } - - await request - .post(`/hooks/${withScript._id}/${withScript.token}`) - .set('Content-Type', 'application/x-www-form-urlencoded') - .send(`payload=${JSON.stringify(payload)}`) - .expect(200) - .expect(async () => { - return request - .get(api('channels.messages')) - .set(credentials) - .query({ - roomId: 'GENERAL', - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('messages').and.to.be.an('array'); - expect(!!(res.body.messages as IMessage[]).find((m) => m.msg === payload.msg)).to.be.true; - }); - }); + `, + }) + .expect(200); + withScript = res1.body.integration; - await removeIntegration(withScript._id, 'incoming'); - }); + const res2 = await request + .post(api('integrations.create')) + .set(credentials) + .send({ + type: 'webhook-incoming', + name: 'Incoming test with script and default content-type', + enabled: true, + alias: 'test', + username: 'rocket.cat', + scriptEnabled: true, + overrideDestinationChannelEnabled: false, + channel: '#general', + script: + 'const buildMessage = (obj) => {\n' + + ' \n' + + ' const template = `[#VALUE](${ obj.test })`;\n' + + ' \n' + + ' return {\n' + + ' text: template\n' + + ' };\n' + + ' };\n' + + ' \n' + + ' class Script {\n' + + ' process_incoming_request({ request }) {\n' + + ' msg = buildMessage(request.content);\n' + + ' \n' + + ' return {\n' + + ' content:{\n' + + ' text: msg.text\n' + + ' }\n' + + ' };\n' + + ' }\n' + + ' }\n' + + ' \n', + }) + .expect(200); + withScriptDefaultContentType = res2.body.integration; + }); - it('should send a message if the payload is a application/x-www-form-urlencoded JSON(when not set, default one) but theres no "payload" key, its just a string, the integration has a valid script', async () => { - const payload = { test: 'test' }; - let withScript: IIntegration | undefined; + after(async () => { + await Promise.all([removeIntegration(withScript._id, 'incoming'), removeIntegration(withScriptDefaultContentType._id, 'incoming')]); + }); - await updatePermission('manage-incoming-integrations', ['admin']); - await request - .post(api('integrations.create')) - .set(credentials) - .send({ - type: 'webhook-incoming', - name: 'Incoming test with script and default content-type', - enabled: true, - alias: 'test', - username: 'rocket.cat', - scriptEnabled: true, - overrideDestinationChannelEnabled: false, - channel: '#general', - script: - 'const buildMessage = (obj) => {\n' + - ' \n' + - ' const template = `[#VALUE](${ obj.test })`;\n' + - ' \n' + - ' return {\n' + - ' text: template\n' + - ' };\n' + - ' };\n' + - ' \n' + - ' class Script {\n' + - ' process_incoming_request({ request }) {\n' + - ' msg = buildMessage(request.content);\n' + - ' \n' + - ' return {\n' + - ' content:{\n' + - ' text: msg.text\n' + - ' }\n' + - ' };\n' + - ' }\n' + - ' }\n' + - ' \n', - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('integration').and.to.be.an('object'); - withScript = res.body.integration; - }); + it('should send a message if the payload is a application/x-www-form-urlencoded JSON AND the integration has a valid script', async () => { + const payload = { msg: `Message as x-www-form-urlencoded JSON sent successfully at #${Date.now()}` }; - if (!withScript) { - throw new Error('Integration not created'); - } + await request + .post(`/hooks/${withScript._id}/${withScript.token}`) + .set('Content-Type', 'application/x-www-form-urlencoded') + .send(`payload=${JSON.stringify(payload)}`) + .expect(200) + .expect(async () => { + return request + .get(api('channels.messages')) + .set(credentials) + .query({ + roomId: 'GENERAL', + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('messages').and.to.be.an('array'); + expect(!!(res.body.messages as IMessage[]).find((m) => m.msg === payload.msg)).to.be.true; + }); + }); + }); - await request - .post(`/hooks/${withScript._id}/${withScript.token}`) - .send(JSON.stringify(payload)) - .expect(200) - .expect(async () => { - return request - .get(api('channels.messages')) - .set(credentials) - .query({ - roomId: 'GENERAL', - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('messages').and.to.be.an('array'); - expect(!!(res.body.messages as IMessage[]).find((m) => m.msg === '[#VALUE](test)')).to.be.true; - }); - }); + it('should send a message if the payload is a application/x-www-form-urlencoded JSON(when not set, default one) but theres no "payload" key, its just a string, the integration has a valid script', async () => { + const payload = { test: 'test' }; - await removeIntegration(withScript._id, 'incoming'); + await request + .post(`/hooks/${withScriptDefaultContentType._id}/${withScriptDefaultContentType.token}`) + .send(JSON.stringify(payload)) + .expect(200) + .expect(async () => { + return request + .get(api('channels.messages')) + .set(credentials) + .query({ + roomId: 'GENERAL', + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('messages').and.to.be.an('array'); + expect(!!(res.body.messages as IMessage[]).find((m) => m.msg === '[#VALUE](test)')).to.be.true; + }); + }); + }); }); describe('With manage-own-incoming-integrations permission', () => { @@ -523,6 +542,17 @@ describe('[Incoming Integrations]', () => { }); describe('[/integrations.history]', () => { + before(async () => { + await Promise.all([updatePermission('manage-incoming-integrations', []), updatePermission('manage-own-incoming-integrations', [])]); + }); + + after(async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', ['admin']), + updatePermission('manage-own-incoming-integrations', ['admin']), + ]); + }); + it('should return an error when trying to get history of incoming integrations if user does NOT have enough permissions', (done) => { void request .get(api('integrations.history')) @@ -589,83 +619,120 @@ describe('[Incoming Integrations]', () => { .end(done); }); - it('should return the list of integrations created by the user only', (done) => { - void updatePermission('manage-incoming-integrations', []).then(() => { - void updatePermission('manage-own-incoming-integrations', ['user']).then(() => { - void request - .get(api('integrations.list')) - .set(userCredentials) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - const integrationCreatedByAdmin = (res.body.integrations as IIntegration[]).find( - (createdIntegration) => createdIntegration._id === integration._id, - ); - expect(integrationCreatedByAdmin).to.be.equal(undefined); - expect(res.body).to.have.property('offset'); - expect(res.body).to.have.property('items'); - expect(res.body).to.have.property('total'); - }) - .end(done); - }); + describe('With manage-own-incoming-integrations permission', () => { + before(async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', []), + updatePermission('manage-own-incoming-integrations', ['user']), + ]); + }); + + after(async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', ['admin']), + updatePermission('manage-own-incoming-integrations', ['admin']), + ]); + }); + + it('should return the list of integrations created by the user only', (done) => { + void request + .get(api('integrations.list')) + .set(userCredentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + const integrationCreatedByAdmin = (res.body.integrations as IIntegration[]).find( + (createdIntegration) => createdIntegration._id === integration._id, + ); + expect(integrationCreatedByAdmin).to.be.equal(undefined); + expect(res.body).to.have.property('offset'); + expect(res.body).to.have.property('items'); + expect(res.body).to.have.property('total'); + }) + .end(done); }); }); - it('should return unauthorized error when the user does not have any integrations permissions', async () => { - await Promise.all([ - updatePermission('manage-incoming-integrations', []), - updatePermission('manage-own-incoming-integrations', []), - updatePermission('manage-outgoing-integrations', []), - updatePermission('manage-outgoing-integrations', []), - ]); - await request - .get(api('integrations.list')) - .set(credentials) - .expect('Content-Type', 'application/json') - .expect(403) - .expect((res) => { - expect(res.body).to.have.property('success', false); - expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]'); - }); + describe('Without any permissions', () => { + before(async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', []), + updatePermission('manage-own-incoming-integrations', []), + updatePermission('manage-outgoing-integrations', []), + updatePermission('manage-own-outgoing-integrations', []), + ]); + }); + + after(async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', ['admin']), + updatePermission('manage-own-incoming-integrations', ['admin']), + updatePermission('manage-outgoing-integrations', ['admin']), + updatePermission('manage-own-outgoing-integrations', ['admin']), + ]); + }); + + it('should return unauthorized error when the user does not have any integrations permissions', async () => { + await request + .get(api('integrations.list')) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(403) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]'); + }); + }); }); }); describe('[/integrations.get]', () => { - it('should return an error when the required "integrationId" query parameters is not sent', (done) => { - void request - .get(api('integrations.get')) - .set(credentials) - .expect('Content-Type', 'application/json') - .expect(400) - .expect((res) => { - expect(res.body).to.have.property('success', false); - expect(res.body).to.have.property('error', `must have required property 'integrationId' [invalid-params]`); - }) - .end(done); - }); + describe('Invalid params', () => { + it('should return an error when the required "integrationId" query parameters is not sent', (done) => { + void request + .get(api('integrations.get')) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', `must have required property 'integrationId' [invalid-params]`); + }) + .end(done); + }); - it('should return an error when the user DOES NOT have the permission "manage-incoming-integrations" to get an incoming integration', (done) => { - void updatePermission('manage-incoming-integrations', []).then(() => { + it('should return an error when the user sends an invalid integration', (done) => { void request .get(api('integrations.get')) - .query({ integrationId: integration._id }) + .query({ integrationId: 'invalid' }) .set(credentials) .expect('Content-Type', 'application/json') .expect(400) .expect((res) => { expect(res.body).to.have.property('success', false); - expect(res.body).to.have.property('error', 'not-authorized'); + expect(res.body).to.have.property('error', 'The integration does not exists.'); }) .end(done); }); }); - it('should return an error when the user DOES NOT have the permission "manage-incoming-integrations" to get an incoming integration created by another user', (done) => { - void updatePermission('manage-incoming-integrations', []).then(() => { + describe('Without permissions', () => { + before(async () => { + await Promise.all([updatePermission('manage-incoming-integrations', []), updatePermission('manage-own-incoming-integrations', [])]); + }); + + after(async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', ['admin']), + updatePermission('manage-own-incoming-integrations', ['admin']), + ]); + }); + + it('should return an error when the user DOES NOT have the permission "manage-incoming-integrations" to get an incoming integration', (done) => { void request .get(api('integrations.get')) - .query({ integrationId: integrationCreatedByAnUser._id }) + .query({ integrationId: integration._id }) .set(credentials) .expect('Content-Type', 'application/json') .expect(400) @@ -675,45 +742,28 @@ describe('[Incoming Integrations]', () => { }) .end(done); }); - }); - it('should return an error when the user sends an invalid integration', (done) => { - void updatePermission('manage-incoming-integrations', ['admin']).then(() => { + it('should return an error when the user DOES NOT have the permission "manage-incoming-integrations" to get an incoming integration created by another user', (done) => { void request .get(api('integrations.get')) - .query({ integrationId: 'invalid' }) + .query({ integrationId: integrationCreatedByAnUser._id }) .set(credentials) .expect('Content-Type', 'application/json') .expect(400) .expect((res) => { expect(res.body).to.have.property('success', false); - expect(res.body).to.have.property('error', 'The integration does not exists.'); + expect(res.body).to.have.property('error', 'not-authorized'); }) .end(done); }); }); - it('should return the integration successfully when the user is able to see only your own integrations', (done) => { - void updatePermission('manage-incoming-integrations', []) - .then(() => updatePermission('manage-own-incoming-integrations', ['user'])) - .then(() => { - void request - .get(api('integrations.get')) - .query({ integrationId: integrationCreatedByAnUser._id }) - .set(userCredentials) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('integration'); - expect(res.body.integration._id).to.be.equal(integrationCreatedByAnUser._id); - }) - .end(done); - }); - }); + describe('With manage-incoming-integrations permission', () => { + before(async () => { + await updatePermission('manage-incoming-integrations', ['admin']); + }); - it('should return the integration successfully', (done) => { - void updatePermission('manage-incoming-integrations', ['admin']).then(() => { + it('should return the integration successfully', (done) => { void request .get(api('integrations.get')) .query({ integrationId: integration._id }) @@ -728,6 +778,37 @@ describe('[Incoming Integrations]', () => { .end(done); }); }); + + describe('With manage-own-incoming-integrations permission', () => { + before(async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', []), + updatePermission('manage-own-incoming-integrations', ['user']), + ]); + }); + + after(async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', ['admin']), + updatePermission('manage-own-incoming-integrations', ['admin']), + ]); + }); + + it('should return the integration successfully when the user is able to see only your own integrations', (done) => { + void request + .get(api('integrations.get')) + .query({ integrationId: integrationCreatedByAnUser._id }) + .set(userCredentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('integration'); + expect(res.body.integration._id).to.be.equal(integrationCreatedByAnUser._id); + }) + .end(done); + }); + }); }); describe('[/integrations.update]', () => { @@ -816,11 +897,11 @@ describe('[Incoming Integrations]', () => { }); it('should send messages to the channel under the updated username', async () => { - const successfulMesssage = `Message sent successfully at #${Random.id()}`; + const successfulMessage = `Message sent successfully at #${Random.id()}`; await request .post(`/hooks/${integration._id}/${integration.token}`) .send({ - text: successfulMesssage, + text: successfulMessage, }) .expect(200); @@ -835,15 +916,33 @@ describe('[Incoming Integrations]', () => { .expect((res) => { expect(res.body).to.have.property('success', true); expect(res.body).to.have.property('messages').and.to.be.an('array'); - const message = (res.body.messages as IMessage[]).find((m) => m.msg === successfulMesssage); + const message = (res.body.messages as IMessage[]).find((m) => m.msg === successfulMessage); expect(message?.u).have.property('username', senderUser.username); }); }); }); describe('[/integrations.remove]', () => { - it('should return an error when the user DOES NOT have the permission "manage-incoming-integrations" to remove an incoming integration', (done) => { - void updatePermission('manage-incoming-integrations', []).then(() => { + describe('Without permissions', () => { + before(async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', []), + updatePermission('manage-own-incoming-integrations', []), + updatePermission('manage-outgoing-integrations', []), + updatePermission('manage-own-outgoing-integrations', []), + ]); + }); + + after(async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', ['admin']), + updatePermission('manage-own-incoming-integrations', ['admin']), + updatePermission('manage-outgoing-integrations', ['admin']), + updatePermission('manage-own-outgoing-integrations', ['admin']), + ]); + }); + + it('should return an error when the user DOES NOT have the permission "manage-incoming-integrations" to remove an incoming integration', (done) => { void request .post(api('integrations.remove')) .set(credentials) @@ -859,10 +958,8 @@ describe('[Incoming Integrations]', () => { }) .end(done); }); - }); - it('should return an error when the user DOES NOT have the permission "manage-own-incoming-integrations" to remove an incoming integration', (done) => { - void updatePermission('manage-own-incoming-integrations', []).then(() => { + it('should return an error when the user DOES NOT have the permission "manage-own-incoming-integrations" to remove an incoming integration', (done) => { void request .post(api('integrations.remove')) .set(credentials) @@ -880,8 +977,12 @@ describe('[Incoming Integrations]', () => { }); }); - it('should return an error when the user sends an invalid type of integration', (done) => { - void updatePermission('manage-own-incoming-integrations', ['admin']).then(() => { + describe('Invalid params', () => { + before(async () => { + await updatePermission('manage-own-incoming-integrations', ['admin']); + }); + + it('should return an error when the user sends an invalid type of integration', (done) => { void request .post(api('integrations.remove')) .set(credentials) @@ -899,8 +1000,12 @@ describe('[Incoming Integrations]', () => { }); }); - it('should remove the integration successfully when the user at least one of the necessary permission to remove an incoming integration', (done) => { - void updatePermission('manage-incoming-integrations', ['admin']).then(() => { + describe('With manage-incoming-integrations permission', () => { + before(async () => { + await updatePermission('manage-incoming-integrations', ['admin']); + }); + + it('should remove the integration successfully when the user at least one of the necessary permission to remove an incoming integration', (done) => { void request .post(api('integrations.remove')) .set(credentials) @@ -917,8 +1022,16 @@ describe('[Incoming Integrations]', () => { }); }); - it('the normal user should remove the integration successfully when the user have the "manage-own-incoming-integrations" to remove an incoming integration', (done) => { - void updatePermission('manage-own-incoming-integrations', ['user']).then(() => { + describe('Normal user with manage-own-incoming-integrations', () => { + before(async () => { + await updatePermission('manage-own-incoming-integrations', ['user']); + }); + + after(async () => { + await updatePermission('manage-own-incoming-integrations', ['admin']); + }); + + it('the normal user should remove the integration successfully when the user have the "manage-own-incoming-integrations" to remove an incoming integration', (done) => { void request .post(api('integrations.remove')) .set(userCredentials) @@ -1102,11 +1215,11 @@ describe('[Incoming Integrations]', () => { }); it('should not send a message in public room if token is invalid', async () => { - const successfulMesssage = `Message sent successfully at #${Random.id()}`; + const successfulMessage = `Message sent successfully at #${Random.id()}`; await request .post(`/hooks/${integration4._id}/invalid-token`) .send({ - text: successfulMesssage, + text: successfulMessage, }) .expect(500) .expect((res) => { @@ -1123,16 +1236,16 @@ describe('[Incoming Integrations]', () => { .expect((res) => { expect(res.body).to.have.property('success', true); expect(res.body).to.have.property('messages').and.to.be.an('array'); - expect((res.body.messages as IMessage[]).find((m) => m.msg === successfulMesssage)).to.be.undefined; + expect((res.body.messages as IMessage[]).find((m) => m.msg === successfulMessage)).to.be.undefined; }); }); it('should not send a message in private room if token is invalid', async () => { - const successfulMesssage = `Message sent successfully at #${Random.id()}`; + const successfulMessage = `Message sent successfully at #${Random.id()}`; await request .post(`/hooks/${integration2._id}/invalid-token`) .send({ - text: successfulMesssage, + text: successfulMessage, }) .expect(500) .expect((res) => { @@ -1149,16 +1262,16 @@ describe('[Incoming Integrations]', () => { .expect((res) => { expect(res.body).to.have.property('success', true); expect(res.body).to.have.property('messages').and.to.be.an('array'); - expect((res.body.messages as IMessage[]).find((m) => m.msg === successfulMesssage)).to.be.undefined; + expect((res.body.messages as IMessage[]).find((m) => m.msg === successfulMessage)).to.be.undefined; }); }); it('should not send a message to a private rooms on behalf of a non member', async () => { - const successfulMesssage = `Message sent successfully at #${Random.id()}`; + const successfulMessage = `Message sent successfully at #${Random.id()}`; await request .post(`/hooks/${integration2._id}/${integration2.token}`) .send({ - text: successfulMesssage, + text: successfulMessage, }) .expect(400) .expect((res) => { @@ -1175,16 +1288,16 @@ describe('[Incoming Integrations]', () => { .expect((res) => { expect(res.body).to.have.property('success', true); expect(res.body).to.have.property('messages').and.to.be.an('array'); - expect((res.body.messages as IMessage[]).find((m) => m.msg === successfulMesssage)).to.be.undefined; + expect((res.body.messages as IMessage[]).find((m) => m.msg === successfulMessage)).to.be.undefined; }); }); it('should not add non member to private rooms when sending message', async () => { - const successfulMesssage = `Message sent successfully at #${Random.id()}`; + const successfulMessage = `Message sent successfully at #${Random.id()}`; await request .post(`/hooks/${integration2._id}/${integration2.token}`) .send({ - text: successfulMesssage, + text: successfulMessage, }) .expect(400) .expect((res) => { @@ -1206,11 +1319,11 @@ describe('[Incoming Integrations]', () => { }); it('should not send a message to public channel of a private team on behalf of a non team member', async () => { - const successfulMesssage = `Message sent successfully at #${Random.id()}`; + const successfulMessage = `Message sent successfully at #${Random.id()}`; await request .post(`/hooks/${integration3._id}/${integration3.token}`) .send({ - text: successfulMesssage, + text: successfulMessage, }) .expect(400) .expect((res) => { @@ -1227,16 +1340,16 @@ describe('[Incoming Integrations]', () => { .expect((res) => { expect(res.body).to.have.property('success', true); expect(res.body).to.have.property('messages').and.to.be.an('array'); - expect((res.body.messages as IMessage[]).find((m) => m.msg === successfulMesssage)).to.be.undefined; + expect((res.body.messages as IMessage[]).find((m) => m.msg === successfulMessage)).to.be.undefined; }); }); it('should not add non team member to the public channel in a private team when sending message', async () => { - const successfulMesssage = `Message sent successfully at #${Random.id()}`; + const successfulMessage = `Message sent successfully at #${Random.id()}`; await request .post(`/hooks/${integration3._id}/${integration3.token}`) .send({ - text: successfulMesssage, + text: successfulMessage, }) .expect(400) .expect((res) => { @@ -1258,11 +1371,11 @@ describe('[Incoming Integrations]', () => { }); it('should send messages from non-members to public rooms and add them as room members', async () => { - const successfulMesssage = `Message sent successfully at #${Random.id()}`; + const successfulMessage = `Message sent successfully at #${Random.id()}`; await request .post(`/hooks/${integration4._id}/${integration4.token}`) .send({ - text: successfulMesssage, + text: successfulMessage, }) .expect(200); @@ -1277,7 +1390,7 @@ describe('[Incoming Integrations]', () => { .expect((res) => { expect(res.body).to.have.property('success', true); expect(res.body).to.have.property('messages').and.to.be.an('array'); - expect((res.body.messages as IMessage[]).find((m) => m.msg === successfulMesssage)).not.to.be.undefined; + expect((res.body.messages as IMessage[]).find((m) => m.msg === successfulMessage)).not.to.be.undefined; }); await request diff --git a/apps/meteor/tests/unit/server/services/omnichannel/queue.tests.ts b/apps/meteor/tests/unit/server/services/omnichannel/queue.tests.ts index 4d498a204307f..1580620ae509f 100644 --- a/apps/meteor/tests/unit/server/services/omnichannel/queue.tests.ts +++ b/apps/meteor/tests/unit/server/services/omnichannel/queue.tests.ts @@ -384,7 +384,6 @@ describe('Omnichannel Queue processor', () => { await queue.execute(); expect(queue.getActiveQueues.calledOnce).to.be.true; - expect(queueLogger.debug.calledWith('Processing items for queue Public')).to.be.true; }); }); describe('start', () => { diff --git a/ee/packages/federation-matrix/src/FederationMatrix.ts b/ee/packages/federation-matrix/src/FederationMatrix.ts index 332d393c1d18c..e2a975bd22194 100644 --- a/ee/packages/federation-matrix/src/FederationMatrix.ts +++ b/ee/packages/federation-matrix/src/FederationMatrix.ts @@ -114,6 +114,10 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS this.logger.debug({ msg: 'Matrix room created', response: matrixRoomResult }); + if (room.topic) { + await federationSDK.setRoomTopic(matrixRoomResult.room_id, matrixUserId, room.topic); + } + await Rooms.setAsFederated(room._id, { mrid: matrixRoomResult.room_id, origin: this.serverName }); // Members are NOT invited here - invites are sent via beforeAddUserToRoom callback. diff --git a/ee/packages/federation-matrix/src/setup.ts b/ee/packages/federation-matrix/src/setup.ts index f307d9c1e5327..ccfffb3ad0da0 100644 --- a/ee/packages/federation-matrix/src/setup.ts +++ b/ee/packages/federation-matrix/src/setup.ts @@ -14,7 +14,7 @@ function validateDomain(domain: string): boolean { } if (value.toLowerCase() !== value) { - logger.error(`The Federation domain "${value}" cannot have uppercase letters`); + logger.error({ msg: 'The Federation domain cannot have uppercase letters', domain: value }); return false; } @@ -25,7 +25,7 @@ function validateDomain(domain: string): boolean { throw new Error(); } } catch { - logger.error(`The configured Federation domain "${value}" is not valid`); + logger.error({ msg: 'The configured Federation domain is not valid', domain: value }); return false; } diff --git a/ee/packages/federation-matrix/tests/end-to-end/room.spec.ts b/ee/packages/federation-matrix/tests/end-to-end/room.spec.ts index ff120a10f5764..4b51395eb8d84 100644 --- a/ee/packages/federation-matrix/tests/end-to-end/room.spec.ts +++ b/ee/packages/federation-matrix/tests/end-to-end/room.spec.ts @@ -1,4 +1,6 @@ import type { IMessage, IUser } from '@rocket.chat/core-typings'; +import type { Room } from 'matrix-js-sdk'; +import { EventTimeline } from 'matrix-js-sdk'; import { createRoom, @@ -713,6 +715,102 @@ import { SynapseClient } from '../helper/synapse-client'; }); }); + describe('Create a room with a topic', () => { + describe('Create a federated room with a topic', () => { + let channelName: string; + let channelTopic: string; + let federatedChannel: any; + + beforeAll(async () => { + channelName = `federated-channel-with-topic-${Date.now()}`; + channelTopic = 'This is a test topic for federation'; + + const createResponse = await createRoom({ + type: 'p', + name: channelName, + members: [federationConfig.hs1.adminMatrixUserId], + extraData: { + federated: true, + topic: channelTopic, + }, + config: rc1AdminRequestConfig, + }); + + federatedChannel = createResponse.body.group; + + expect(federatedChannel).toHaveProperty('_id'); + expect(federatedChannel).toHaveProperty('name', channelName); + expect(federatedChannel).toHaveProperty('t', 'p'); + expect(federatedChannel).toHaveProperty('federated', true); + + const acceptedRoomId = await hs1AdminApp.acceptInvitationForRoomName(channelName); + expect(acceptedRoomId).not.toBe(''); + }, 10000); + + it('should set the topic on the Rocket.Chat side', async () => { + // RC view: Verify the topic is set in Rocket.Chat + const roomInfo = await getRoomInfo(federatedChannel._id, rc1AdminRequestConfig); + expect(roomInfo.room).toHaveProperty('topic', channelTopic); + }); + + it('should set the topic on the Matrix side', async () => { + const hs1Room1 = (await hs1AdminApp.matrixClient.getRoom(federatedChannel.federation.mrid)) as Room; + + expect(hs1Room1).toBeDefined(); + + const [topic] = hs1Room1.getLiveTimeline().getState(EventTimeline.FORWARDS)?.getStateEvents('m.room.topic') || []; + + expect(topic.getContent().topic).toBe(channelTopic); + }); + }); + + describe('Create a federated room without a topic', () => { + let channelName: string; + let federatedChannel: any; + + beforeAll(async () => { + channelName = `federated-channel-no-topic-${Date.now()}`; + + const createResponse = await createRoom({ + type: 'p', + name: channelName, + members: [federationConfig.hs1.adminMatrixUserId], + extraData: { + federated: true, + }, + config: rc1AdminRequestConfig, + }); + + federatedChannel = createResponse.body.group; + + expect(federatedChannel).toHaveProperty('_id'); + expect(federatedChannel).toHaveProperty('name', channelName); + expect(federatedChannel).toHaveProperty('t', 'p'); + expect(federatedChannel).toHaveProperty('federated', true); + expect(federatedChannel).toHaveProperty('federation.mrid'); + + const acceptedRoomId = await hs1AdminApp.acceptInvitationForRoomName(channelName); + expect(acceptedRoomId).not.toBe(''); + }, 10000); + + it('should not set a topic on the Rocket.Chat side', async () => { + // RC view: Verify no topic is set in Rocket.Chat + const roomInfo = await getRoomInfo(federatedChannel._id, rc1AdminRequestConfig); + expect(roomInfo.room?.topic).toBeUndefined(); + }); + + it('should not set a topic on the Matrix side', async () => { + const hs1Room1 = (await hs1AdminApp.matrixClient.getRoom(federatedChannel.federation.mrid)) as Room; + + expect(hs1Room1).toBeDefined(); + + const [topic] = hs1Room1.getLiveTimeline().getState(EventTimeline.FORWARDS)?.getStateEvents('m.room.topic') || []; + + expect(topic).toBeUndefined(); + }); + }); + }); + describe('Create a room on RC as private and federated, then invite users', () => { describe('Go to the members list and', () => { describe('Add a federated user', () => { diff --git a/ee/packages/license/src/license.ts b/ee/packages/license/src/license.ts index 3e94af592066a..0e92af730b1cf 100644 --- a/ee/packages/license/src/license.ts +++ b/ee/packages/license/src/license.ts @@ -355,10 +355,8 @@ export abstract class LicenseManager extends Emitter { this.emit('installed'); return true; - } catch (e) { - logger.error('Invalid license'); - - logger.error({ msg: 'Invalid raw license', encryptedLicense, e }); + } catch (err) { + logger.error({ msg: 'Invalid raw license', encryptedLicense, err }); throw new InvalidLicenseError(); } diff --git a/ee/packages/omnichannel-services/src/QueueWorker.ts b/ee/packages/omnichannel-services/src/QueueWorker.ts index 3e18e4fea4b7a..69855f14071c2 100644 --- a/ee/packages/omnichannel-services/src/QueueWorker.ts +++ b/ee/packages/omnichannel-services/src/QueueWorker.ts @@ -46,7 +46,7 @@ export class QueueWorker extends ServiceClass implements IQueueWorkerService { await this.createIndexes(); this.registerWorkers(); } catch (e) { - this.logger.fatal(e, 'Fatal error occurred when registering workers'); + this.logger.fatal({ msg: 'Fatal error occurred when registering workers', err: e }); process.exit(1); } } @@ -75,24 +75,24 @@ export class QueueWorker extends ServiceClass implements IQueueWorkerService { } private async workerCallback(queueItem: Work<{ to: string; data: any }>): Promise { - this.logger.info(`Processing queue item ${queueItem._id} for work`); - this.logger.info(`Queue item is trying to call ${queueItem.message.to}`); + this.logger.info({ msg: 'Processing queue item for work', queueItemId: queueItem._id }); + this.logger.info({ msg: 'Queue item is trying to call', to: queueItem.message.to }); try { await api.call(queueItem.message.to, [queueItem.message]); - this.logger.info(`Queue item ${queueItem._id} completed`); + this.logger.info({ msg: 'Queue item completed', queueItemId: queueItem._id }); return 'Completed' as const; } catch (err: unknown) { const e = err as Error; - this.logger.error(`Queue item ${queueItem._id} errored: ${e.message}`); + this.logger.error({ msg: 'Queue item errored', queueItemId: queueItem._id, err: e }); queueItem.releasedReason = e.message; // Let's only retry for X times when the error is "service not found" // For any other error, we'll just reject the item if ((queueItem.retryCount || 0) < this.retryCount && this.isRetryableError(e.message)) { - this.logger.info(`Queue item ${queueItem._id} will be retried in 10 seconds`); + this.logger.info({ msg: 'Queue item will be retried', queueItemId: queueItem._id, retry: this.retryDelay }); queueItem.nextReceivableTime = new Date(Date.now() + this.retryDelay); return 'Retry' as const; } - this.logger.info(`Queue item ${queueItem._id} will be rejected`); + this.logger.info({ msg: 'Queue item will be rejected', queueItemId: queueItem._id }); return 'Rejected' as const; } } @@ -119,7 +119,7 @@ export class QueueWorker extends ServiceClass implements IQueueWorkerService { // `to` is a service name that will be called, including namespace + action // This is a "generic" job that allows you to call any service async queueWork>(queue: Actions, to: string, data: T): Promise { - this.logger.info(`Queueing work for ${to}`); + this.logger.info({ msg: 'Queueing work for', to }); if (!this.matchServiceCall(to)) { // We don't want to queue calls to invalid service names throw new Error(`Invalid service name ${to}`); diff --git a/packages/apps-engine/src/server/runtime/deno/AppsEngineDenoRuntime.ts b/packages/apps-engine/src/server/runtime/deno/AppsEngineDenoRuntime.ts index 8b4eb385a88f8..d445eeac0ccd8 100644 --- a/packages/apps-engine/src/server/runtime/deno/AppsEngineDenoRuntime.ts +++ b/packages/apps-engine/src/server/runtime/deno/AppsEngineDenoRuntime.ts @@ -287,7 +287,7 @@ export class DenoRuntimeSubprocessController extends EventEmitter implements IRu this.debug('Restarting app subprocess'); const logger = new AppConsole('runtime:restart'); - logger.info('Starting restart procedure for app subprocess...', this.livenessManager.getRuntimeData()); + logger.info({ msg: 'Starting restart procedure for app subprocess...', runtimeData: this.livenessManager.getRuntimeData() }); this.state = 'restarting'; @@ -297,13 +297,13 @@ export class DenoRuntimeSubprocessController extends EventEmitter implements IRu const hasKilled = await this.killProcess(); if (hasKilled) { - logger.debug('Process successfully terminated', { pid }); + logger.debug({ msg: 'Process successfully terminated', pid }); } else { - logger.warn('Could not terminate process. Maybe it was already dead?', { pid }); + logger.warn({ msg: 'Could not terminate process. Maybe it was already dead?', pid }); } await this.setupApp(); - logger.info('New subprocess successfully spawned', { pid: this.deno.pid }); + logger.info({ msg: 'New subprocess successfully spawned', pid: this.deno.pid }); // setupApp() changes the state to 'ready' - we'll need to workaround that for now this.state = 'restarting'; @@ -319,7 +319,7 @@ export class DenoRuntimeSubprocessController extends EventEmitter implements IRu logger.info('Successfully restarted app subprocess'); } catch (e) { - logger.error("Failed to restart app's subprocess", { error: e.message || e }); + logger.error({ msg: "Failed to restart app's subprocess", err: e }); throw e; } finally { await this.logStorage.storeEntries(AppConsole.toStorageEntry(this.getAppId(), logger)); diff --git a/packages/rest-typings/src/v1/commands.ts b/packages/rest-typings/src/v1/commands.ts index 41a08435bade5..bbf3788b2b475 100644 --- a/packages/rest-typings/src/v1/commands.ts +++ b/packages/rest-typings/src/v1/commands.ts @@ -15,6 +15,7 @@ export type CommandsEndpoints = { fields?: string; }>, ) => PaginatedResult<{ + appsLoaded: boolean; commands: Pick[]; }>; };