diff --git a/apps/meteor/jest.config.ts b/apps/meteor/jest.config.ts index 5dd247ce4406e..01d1fc9559519 100644 --- a/apps/meteor/jest.config.ts +++ b/apps/meteor/jest.config.ts @@ -40,6 +40,7 @@ export default { '/app/cloud/server/functions/supportedVersionsToken/**.spec.ts', '/app/utils/lib/**.spec.ts', '/server/lib/auditServerEvents/**.spec.ts', + '/server/settings/lib/**.spec.ts', '/server/cron/**.spec.ts', '/app/api/server/**.spec.ts', '/app/api/server/helpers/**.spec.ts', diff --git a/apps/meteor/server/settings/lib/auditedSettingUpdates.spec.ts b/apps/meteor/server/settings/lib/auditedSettingUpdates.spec.ts new file mode 100644 index 0000000000000..f7c1d7fd9f3ec --- /dev/null +++ b/apps/meteor/server/settings/lib/auditedSettingUpdates.spec.ts @@ -0,0 +1,307 @@ +import type { ISetting, SettingValue } from '@rocket.chat/core-typings'; + +import { updateAuditedByUser, updateAuditedBySystem, updateAuditedByApp, resetAuditedSettingByUser } from './auditedSettingUpdates'; + +const mockCreateAuditServerEvent = jest.fn(); +jest.mock('@rocket.chat/models', () => ({ + ServerEvents: { + createAuditServerEvent: (...args: any[]) => mockCreateAuditServerEvent(...args), + }, +})); + +const mockSettings = new Map(); + +jest.mock('../../../app/settings/server/cached', () => { + const mockGetSetting = jest.fn((key: string) => mockSettings.get(key)); + return { + settings: { + getSetting: mockGetSetting, + }, + }; +}); + +describe('auditedSettingUpdates', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockSettings.clear(); + }); + + describe('Masking sensitive settings', () => { + it('should mask password type settings with more than 8 characters', () => { + const settingId = 'SMTP_Password'; + const originalValue = 'secretpassword123'; + const newValue = 'newpassword456'; + + mockSettings.set(settingId, { + _id: settingId, + type: 'password', + value: originalValue, + } as ISetting); + + const actor = { + _id: 'user1234567', + username: 'testuser133532', + ip: '127.0.0.1', + useragent: 'test-agent', + }; + + const mockFn = jest.fn((_key: string, value: SettingValue) => value); + const auditedFn = updateAuditedByUser(actor)(mockFn, settingId, newValue); + + expect(auditedFn).toBe(newValue); + expect(mockCreateAuditServerEvent).toHaveBeenCalledWith( + 'settings.changed', + { + id: settingId, + previous: 'sec**************', + current: 'new***********', + }, + { + type: 'user', + ...actor, + }, + ); + }); + + it('should mask password type settings with 8 or fewer characters completely', () => { + const settingId = 'Short_Password'; + + mockSettings.set(settingId, { + _id: settingId, + type: 'password', + value: 'abc', + } as ISetting); + + const actor = { + _id: 'user123', + username: 'testuser', + ip: '127.0.0.1', + useragent: 'test-agent', + }; + + const mockFn = jest.fn((_key: string, value: SettingValue) => value); + const auditedFn = updateAuditedByUser(actor)(mockFn, settingId, 'xy'); + + expect(auditedFn).toBe('xy'); + expect(mockCreateAuditServerEvent).toHaveBeenCalledWith( + 'settings.changed', + { + id: settingId, + previous: '***', + current: '**', + }, + { + type: 'user', + ...actor, + }, + ); + }); + + it('should mask secret type settings', () => { + const settingId = 'SMTP_Username'; + + mockSettings.set(settingId, { + _id: settingId, + type: 'string', + secret: true, + value: 'myusername', + } as ISetting); + + const actor = { + _id: 'user123', + username: 'testuser', + ip: '127.0.0.1', + useragent: 'test-agent', + }; + + const mockFn = jest.fn((_key: string, value: SettingValue) => value); + const auditedFn = updateAuditedByUser(actor)(mockFn, settingId, 'newusername'); + + expect(auditedFn).toBe('newusername'); + expect(mockCreateAuditServerEvent).toHaveBeenCalledWith( + 'settings.changed', + { + id: settingId, + previous: 'myu*******', + current: 'new********', + }, + { + type: 'user', + ...actor, + }, + ); + }); + }); + + describe('updateAuditedBySystem', () => { + it('should mask sensitive settings for system actor', () => { + const settingId = 'SMTP_Password'; + + mockSettings.set(settingId, { + _id: settingId, + type: 'password', + value: 'oldpassword', + } as ISetting); + + const mockFn = jest.fn((_key: string, value: SettingValue) => value); + const auditedFn = updateAuditedBySystem({}); + + auditedFn(mockFn, settingId, 'newpassword123'); + + expect(mockCreateAuditServerEvent).toHaveBeenCalledWith( + 'settings.changed', + { + id: settingId, + previous: 'old********', + current: 'new***********', + }, + { + type: 'system', + }, + ); + }); + }); + + describe('updateAuditedByApp', () => { + it('should mask sensitive settings for app actor', () => { + const settingId = 'LDAP_Authentication_Password'; + + mockSettings.set(settingId, { + _id: settingId, + type: 'password', + value: 'oldpass', + } as ISetting); + + const actor = { + type: 'app' as const, + _id: 'app123', + }; + + const mockFn = jest.fn((_key: string, value: SettingValue) => value); + const auditedFn = updateAuditedByApp(actor); + + auditedFn(mockFn, settingId, 'ldappass'); + + expect(mockCreateAuditServerEvent).toHaveBeenCalledWith( + 'settings.changed', + { + id: settingId, + previous: '*******', + current: '********', + }, + { + type: 'app', + _id: 'app123', + }, + ); + }); + }); + + describe('resetAuditedSettingByUser', () => { + it('should mask sensitive settings when resetting', () => { + const settingId = 'SMTP_Password'; + + mockSettings.set(settingId, { + _id: settingId, + type: 'password', + value: 'currentpassword123', + packageValue: 'packagepassword456', + } as ISetting); + + const actor = { + _id: 'user123', + username: 'testuser', + ip: '127.0.0.1', + useragent: 'test-agent', + }; + + const mockFn = jest.fn((key: string) => key); + const auditedFn = resetAuditedSettingByUser(actor); + + auditedFn(mockFn, settingId); + + expect(mockCreateAuditServerEvent).toHaveBeenCalledWith( + 'settings.changed', + { + id: settingId, + previous: 'cur***************', + current: 'pac***************', + }, + { + type: 'user', + ...actor, + }, + ); + }); + }); + + describe('Edge cases', () => { + it('should handle exactly 8 character values', () => { + const settingId = 'Three_Char_Password'; + + mockSettings.set(settingId, { + _id: settingId, + type: 'password', + value: 'abc', + } as ISetting); + + const actor = { + _id: 'user1234', + username: 'testuser', + ip: '127.0.0.1', + useragent: 'test-agent', + }; + + const mockFn = jest.fn((_key: string, value: SettingValue) => value); + const auditedFn = updateAuditedByUser(actor)(mockFn, settingId, 'xyz'); + + expect(auditedFn).toBe('xyz'); + expect(mockCreateAuditServerEvent).toHaveBeenCalledWith( + 'settings.changed', + { + id: settingId, + previous: '***', + current: '***', + }, + { + type: 'user', + ...actor, + }, + ); + }); + + it('should handle non-string values (numbers)', () => { + const settingId = 'Numeric_Password'; + + mockSettings.set(settingId, { + _id: settingId, + type: 'password', + value: 12345, + } as ISetting); + + const actor = { + _id: 'user123', + username: 'testuser', + ip: '127.0.0.1', + useragent: 'test-agent', + }; + + const mockFn = jest.fn((_key: string, value: SettingValue) => value); + const auditedFn = updateAuditedByUser(actor)(mockFn, settingId, 67890); + + expect(auditedFn).toBe(67890); + expect(mockCreateAuditServerEvent).toHaveBeenCalledWith( + 'settings.changed', + { + id: settingId, + previous: '*****', + current: '*****', + }, + { + type: 'user', + ...actor, + }, + ); + }); + }); +}); diff --git a/apps/meteor/server/settings/lib/auditedSettingUpdates.ts b/apps/meteor/server/settings/lib/auditedSettingUpdates.ts index 19f646641eaaf..4b90278e6facd 100644 --- a/apps/meteor/server/settings/lib/auditedSettingUpdates.ts +++ b/apps/meteor/server/settings/lib/auditedSettingUpdates.ts @@ -1,8 +1,42 @@ -import type { IAuditServerAppActor, IAuditServerSystemActor, IAuditServerUserActor, ISetting } from '@rocket.chat/core-typings'; +import type { + IAuditServerAppActor, + IAuditServerSystemActor, + IAuditServerUserActor, + ISetting, + SettingValue, +} from '@rocket.chat/core-typings'; import { ServerEvents } from '@rocket.chat/models'; import { settings } from '../../../app/settings/server/cached'; +const shouldMaskSettingInAudit = (settingId: ISetting['_id']): boolean => { + const setting = settings.getSetting(settingId); + return Boolean(setting && (setting.type === 'password' || (setting.type === 'string' && setting.secret === true))); +}; + +const maskIfNeeded = (settingId: ISetting['_id'], value: SettingValue): SettingValue => { + if (!shouldMaskSettingInAudit(settingId)) { + return value; + } + + if (value === undefined || value === null || value === '') { + return value; + } + + const valueString = String(value); + const valueLength = valueString.length; + + let maskedValue: string; + + if (valueLength <= 8) { + maskedValue = '*'.repeat(valueLength); + } else { + maskedValue = valueString.substring(0, 3) + '*'.repeat(valueLength - 3); + } + + return maskedValue; +}; + export const resetAuditedSettingByUser = (actor: Omit) => any>(fn: F, key: ISetting['_id']): ReturnType => { @@ -12,8 +46,8 @@ export const resetAuditedSettingByUser = 'settings.changed', { id: key, - previous: value, - current: packageValue, + previous: maskIfNeeded(key, value), + current: maskIfNeeded(key, packageValue), }, { type: 'user', @@ -38,8 +72,8 @@ export const updateAuditedByUser = 'settings.changed', { id: key, - previous, - current: value, + previous: maskIfNeeded(key, previous), + current: maskIfNeeded(key, value), }, { type: 'user', @@ -64,8 +98,8 @@ export const updateAuditedBySystem = 'settings.changed', { id: key, - previous, - current: value, + previous: maskIfNeeded(key, previous), + current: maskIfNeeded(key, value), }, { type: 'system', @@ -88,8 +122,8 @@ export const updateAuditedByApp = 'settings.changed', { id: key, - previous, - current: value, + previous: maskIfNeeded(key, previous), + current: maskIfNeeded(key, value), }, { type: 'app', diff --git a/apps/meteor/tests/end-to-end/api/settings.ts b/apps/meteor/tests/end-to-end/api/settings.ts index 51a871dacd31a..341cba62e1fc5 100644 --- a/apps/meteor/tests/end-to-end/api/settings.ts +++ b/apps/meteor/tests/end-to-end/api/settings.ts @@ -3,7 +3,7 @@ import { expect } from 'chai'; import { before, describe, it, after } from 'mocha'; import { getCredentials, api, request, credentials } from '../../data/api-data'; -import { updatePermission, updateSetting } from '../../data/permissions.helper'; +import { updatePermission, updateSetting, getSettingValueById } from '../../data/permissions.helper'; import { IS_EE } from '../../e2e/config/constants'; describe('[Settings]', () => { @@ -423,5 +423,117 @@ describe('[Settings]', () => { }); }); }); + + describe('Masking sensitive settings', () => { + let originalSmtpPassword: string | undefined; + let originalSmtpUsername: string | undefined; + let maskingStart: Date; + let maskingEnd: Date; + const testPassword = 'testpassword123'; + const testUsername = 'testuser@example.com'; + + before(async () => { + maskingStart = new Date(); + originalSmtpPassword = (await getSettingValueById('SMTP_Password')) as string | undefined; + originalSmtpUsername = (await getSettingValueById('SMTP_Username')) as string | undefined; + + await updateSetting('SMTP_Password', testPassword); + await updateSetting('SMTP_Username', testUsername); + maskingEnd = new Date(); + }); + + after(async () => { + await updateSetting('SMTP_Password', originalSmtpPassword || ''); + await updateSetting('SMTP_Username', originalSmtpUsername || ''); + }); + + it('should mask sensitive settings in audit logs', async () => { + await request + .get(api('audit.settings')) + .query({ settingId: 'SMTP_Password', start: formatDate(maskingStart), end: formatDate(maskingEnd) }) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('events').and.to.be.an('array'); + expect(res.body.events.length).to.be.greaterThanOrEqual(1); + + const passwordEvents = res.body.events as IServerEvents['settings.changed'][]; + const passwordEvent = passwordEvents.find((event) => { + const settingId = event.data.find((item) => item.key === 'id')?.value; + return settingId === 'SMTP_Password'; + }); + + expect(passwordEvent).to.exist; + + if (passwordEvent) { + const previous = passwordEvent.data.find((item) => item.key === 'previous')?.value; + const current = passwordEvent.data.find((item) => item.key === 'current')?.value; + + if (previous && typeof previous === 'string' && previous.length > 0) { + expect(previous).to.include('*'); + expect(previous).to.not.equal(testPassword); + if (previous.length > 8) { + expect(previous.substring(0, 3)).to.equal(originalSmtpPassword?.substring(0, 3)); + expect(previous.substring(3)).to.match(/^\*+$/); + } + } + + if (current && typeof current === 'string' && current.length > 0) { + expect(current).to.include('*'); + expect(current).to.not.equal(testPassword); + if (testPassword.length > 8) { + expect(current.substring(0, 3)).to.equal(testPassword.substring(0, 3)); + expect(current.substring(3)).to.match(/^\*+$/); + } else { + expect(current).to.match(/^\*+$/); + } + } + } + }); + + await request + .get(api('audit.settings')) + .query({ settingId: 'SMTP_Username', start: formatDate(maskingStart), end: formatDate(maskingEnd) }) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('events').and.to.be.an('array'); + expect(res.body.events.length).to.be.greaterThanOrEqual(1); + + const usernameEvents = res.body.events as IServerEvents['settings.changed'][]; + const usernameEvent = usernameEvents.find((event) => { + const settingId = event.data.find((item) => item.key === 'id')?.value; + return settingId === 'SMTP_Username'; + }); + + expect(usernameEvent).to.exist; + + if (usernameEvent) { + const previous = usernameEvent.data.find((item) => item.key === 'previous')?.value; + const current = usernameEvent.data.find((item) => item.key === 'current')?.value; + + if (previous && typeof previous === 'string' && previous.length > 0) { + expect(previous).to.include('*'); + expect(previous).to.not.equal(originalSmtpUsername); + } + + if (current && typeof current === 'string' && current.length > 0) { + expect(current).to.include('*'); + expect(current).to.not.equal(testUsername); + if (testUsername.length > 8) { + expect(current.substring(0, 3)).to.equal(testUsername.substring(0, 3)); + expect(current.substring(3)).to.match(/^\*+$/); + } else { + expect(current).to.match(/^\*+$/); + } + } + } + }); + }); + }); }); });