From 209c1cfbfbc379457b0f48323fbfbfaa4ce0c06e Mon Sep 17 00:00:00 2001 From: Simon Gardner Date: Thu, 5 Feb 2026 12:40:25 +0000 Subject: [PATCH] Add audit log entries for project-role-changed and group-role-changed GitOrigin-RevId: 4c326dd922bede6f218a6d89e4f18c312a9abf98 --- .../Collaborators/CollaboratorsController.mjs | 9 +- .../Collaborators/CollaboratorsHandler.mjs | 28 +++- .../app/src/Features/Helpers/StringHelper.mjs | 5 + .../Project/ProjectAuditLogHandler.mjs | 1 + .../TokenAccess/TokenAccessController.mjs | 6 +- .../UserMembershipController.mjs | 14 +- .../UserMembership/UserMembershipHandler.mjs | 54 ++++++- .../CollaboratorsHandler.test.mjs | 35 +++++ .../UserMembershipController.test.mjs | 7 +- .../UserMembershipHandler.test.mjs | 145 ++++++++++++++++++ 10 files changed, 295 insertions(+), 9 deletions(-) diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsController.mjs b/services/web/app/src/Features/Collaborators/CollaboratorsController.mjs index 6441a4b8b9..849c10d5c5 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsController.mjs +++ b/services/web/app/src/Features/Collaborators/CollaboratorsController.mjs @@ -112,10 +112,17 @@ async function setCollaboratorInfo(req, res, next) { ) } + const auditInfo = { + ipAddress: req.ip, + initiatorId: SessionManager.getLoggedInUserId(req.session), + } + await CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel( projectId, userId, - privilegeLevel + privilegeLevel, + {}, + auditInfo ) EditorRealTimeController.emitToRoom( projectId, diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsHandler.mjs b/services/web/app/src/Features/Collaborators/CollaboratorsHandler.mjs index a25837f57e..afca4cf248 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsHandler.mjs +++ b/services/web/app/src/Features/Collaborators/CollaboratorsHandler.mjs @@ -10,6 +10,7 @@ import CollaboratorsGetter from './CollaboratorsGetter.mjs' import Errors from '../Errors/Errors.js' import TpdsUpdateSender from '../ThirdPartyDataStore/TpdsUpdateSender.mjs' import EditorRealTimeController from '../Editor/EditorRealTimeController.mjs' +import ProjectAuditLogHandler from '../Project/ProjectAuditLogHandler.mjs' export default { userIsTokenMember: callbackify(userIsTokenMember), @@ -268,7 +269,8 @@ async function setCollaboratorPrivilegeLevel( projectId, userId, privilegeLevel, - { pendingEditor, pendingReviewer } = {} + { pendingEditor, pendingReviewer } = {}, + auditInfo = {} ) { // Make sure we're only updating the project if the user is already a // collaborator @@ -352,6 +354,17 @@ async function setCollaboratorPrivilegeLevel( throw new Errors.NotFoundError('project or collaborator not found') } + ProjectAuditLogHandler.addEntryInBackground( + projectId, + 'project-role-changed', + auditInfo.initiatorId, + auditInfo.ipAddress, + { + userId, + role: _privilegeLevelToRole(privilegeLevel), + } + ) + if (update.$set?.track_changes) { EditorRealTimeController.emitToRoom( projectId, @@ -361,6 +374,19 @@ async function setCollaboratorPrivilegeLevel( } } +function _privilegeLevelToRole(privilegeLevel) { + switch (privilegeLevel) { + case 'readOnly': + return 'Viewer' + case 'readAndWrite': + return 'Editor' + case 'review': + return 'Reviewer' + default: + return privilegeLevel + } +} + async function userIsTokenMember(userId, projectId) { if (!userId) { return false diff --git a/services/web/app/src/Features/Helpers/StringHelper.mjs b/services/web/app/src/Features/Helpers/StringHelper.mjs index 2e4f141c0f..f3be97111e 100644 --- a/services/web/app/src/Features/Helpers/StringHelper.mjs +++ b/services/web/app/src/Features/Helpers/StringHelper.mjs @@ -15,6 +15,11 @@ const JSON_ESCAPE = { '\u2029': '\\u2029', } +export function capitalise(str) { + if (!str || str.length === 0 || typeof str !== 'string') return str + return str.substring(0, 1).toUpperCase() + str.slice(1) +} + /** * Converts a snake_case string into a user friendly string with each word capitalized. * @param {string} snakecaseStr diff --git a/services/web/app/src/Features/Project/ProjectAuditLogHandler.mjs b/services/web/app/src/Features/Project/ProjectAuditLogHandler.mjs index fb34a5a2fa..a9415a59d5 100644 --- a/services/web/app/src/Features/Project/ProjectAuditLogHandler.mjs +++ b/services/web/app/src/Features/Project/ProjectAuditLogHandler.mjs @@ -15,6 +15,7 @@ const MANAGED_GROUP_PROJECT_EVENTS = [ 'project-untrashed', 'project-restored', 'project-cloned', + 'project-role-changed', 'project-history-version-restored', 'project-history-version-downloaded', 'transfer-ownership', diff --git a/services/web/app/src/Features/TokenAccess/TokenAccessController.mjs b/services/web/app/src/Features/TokenAccess/TokenAccessController.mjs index 16d627e128..416b6b4eb6 100644 --- a/services/web/app/src/Features/TokenAccess/TokenAccessController.mjs +++ b/services/web/app/src/Features/TokenAccess/TokenAccessController.mjs @@ -542,13 +542,17 @@ async function moveReadWriteToCollaborators(req, res, next) { userId, projectId ) + + const auditInfo = { ipAddress: req.ip, initiatorId: userId } + await CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel( projectId, userId, pendingEditor ? PrivilegeLevels.READ_ONLY : PrivilegeLevels.READ_AND_WRITE, - { pendingEditor } + { pendingEditor }, + auditInfo ) } else { // Normal case, not invited, joining via link sharing diff --git a/services/web/app/src/Features/UserMembership/UserMembershipController.mjs b/services/web/app/src/Features/UserMembership/UserMembershipController.mjs index 01b93f16e3..9df369c5ac 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipController.mjs +++ b/services/web/app/src/Features/UserMembership/UserMembershipController.mjs @@ -202,10 +202,15 @@ async function add(req, res) { } let user try { + const auditInfo = { + ipAddress: req.ip, + initiatorId: SessionManager.getLoggedInUserId(req.session), + } user = await UserMembershipHandler.promises.addUser( entity, entityConfig, - email + email, + auditInfo ) } catch (err) { if (err instanceof UserMembershipErrors.UserAlreadyAddedError) { @@ -245,10 +250,15 @@ async function remove(req, res) { }) } try { + const auditInfo = { + ipAddress: req.ip, + initiatorId: loggedInUserId, + } await UserMembershipHandler.promises.removeUser( entity, entityConfig, - userId + userId, + auditInfo ) } catch (err) { if (err instanceof UserMembershipErrors.UserIsManagerError) { diff --git a/services/web/app/src/Features/UserMembership/UserMembershipHandler.mjs b/services/web/app/src/Features/UserMembership/UserMembershipHandler.mjs index f34937df82..2eb1d5721a 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipHandler.mjs +++ b/services/web/app/src/Features/UserMembership/UserMembershipHandler.mjs @@ -6,6 +6,8 @@ import { Publisher } from '../../models/Publisher.mjs' import UserMembershipViewModel from './UserMembershipViewModel.mjs' import UserGetter from '../User/UserGetter.mjs' import UserMembershipErrors from './UserMembershipErrors.mjs' +import Modules from '../../infrastructure/Modules.mjs' +import mongoose from '../../infrastructure/Mongoose.mjs' const { ObjectId } = mongodb @@ -27,7 +29,7 @@ const UserMembershipHandler = { return await getPopulatedListOfMembers(entity, attributes) }, - async addUser(entity, entityConfig, email) { + async addUser(entity, entityConfig, email, auditInfo) { const attribute = entityConfig.fields.write const user = await UserGetter.promises.getUserByAnyEmail(email) @@ -39,15 +41,63 @@ const UserMembershipHandler = { throw new UserMembershipErrors.UserAlreadyAddedError() } + // if the entity is a Subscription with managed users enabled, then audit log the event + if ( + entityConfig.modelName === 'Subscription' && + entity.managedUsersEnabled + ) { + const session = await mongoose.startSession() + try { + await session.withTransaction(async () => { + const auditLog = { + groupId: entity._id, + operation: 'group-role-changed', + initiatorId: auditInfo.initiatorId, + ipAddress: auditInfo.ipAddress, + info: { + userId: user._id, + role: 'manager', + }, + } + await Modules.promises.hooks.fire( + 'addGroupAuditLogEntry', + auditLog, + session + ) + }) + } finally { + await session.endSession() + } + } + await addUserToEntity(entity, attribute, user) return UserMembershipViewModel.build(user) }, - async removeUser(entity, entityConfig, userId) { + async removeUser(entity, entityConfig, userId, auditInfo) { const attribute = entityConfig.fields.write if (entity.admin_id ? entity.admin_id.equals(userId) : undefined) { throw new UserMembershipErrors.UserIsManagerError() } + + // if the entity is a Subscription with managed users enabled, then audit log the event + if ( + entityConfig.modelName === 'Subscription' && + entity.managedUsersEnabled + ) { + const auditLog = { + groupId: entity._id, + operation: 'group-role-changed', + initiatorId: auditInfo.initiatorId, + ipAddress: auditInfo.ipAddress, + info: { + userId, + role: 'member', + }, + } + await Modules.promises.hooks.fire('addGroupAuditLogEntry', auditLog) + } + return await removeUserFromEntity(entity, attribute, userId) }, } diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsHandler.test.mjs b/services/web/test/unit/src/Collaborators/CollaboratorsHandler.test.mjs index f9e571449b..0035d3595a 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsHandler.test.mjs +++ b/services/web/test/unit/src/Collaborators/CollaboratorsHandler.test.mjs @@ -104,6 +104,17 @@ describe('CollaboratorsHandler', function () { }) ) + ctx.ProjectAuditLogHandler = { + addEntryInBackground: sinon.stub(), + } + + vi.doMock( + '../../../../app/src/Features/Project/ProjectAuditLogHandler', + () => ({ + default: ctx.ProjectAuditLogHandler, + }) + ) + ctx.CollaboratorsHandler = (await import(MODULE_PATH)).default }) @@ -812,5 +823,29 @@ describe('CollaboratorsHandler', function () { ) ).to.be.rejectedWith(Errors.NotFoundError) }) + + it('should write a project audit log', async function (ctx) { + ctx.ProjectMock.expects('updateOne') + .chain('exec') + .resolves({ matchedCount: 1 }) + const auditInfo = { + initiatorId: new ObjectId(), + ipAddress: '192.168.1.1', + } + await ctx.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel( + ctx.project._id, + ctx.userId, + 'readOnly', + {}, + auditInfo + ) + ctx.ProjectAuditLogHandler.addEntryInBackground.should.have.been.calledWith( + ctx.project._id, + 'project-role-changed', + auditInfo.initiatorId, + auditInfo.ipAddress, + { userId: ctx.userId, role: 'Viewer' } + ) + }) }) }) diff --git a/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs b/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs index d9ce158cbc..83dafae38a 100644 --- a/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs +++ b/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs @@ -27,6 +27,7 @@ describe('UserMembershipController', () => { recurlySubscriptionId = 'mock-recurly-subscription-id' ctx.req = new MockRequest(vi) ctx.req.params.id = 'mock-entity-id' + ctx.req.ip = '1.2.3.4' ctx.user = { _id: 'mock-user-id' } ctx.newUser = { _id: 'mock-new-user-id', email: 'new-user-email@foo.bar' } ctx.subscription = { @@ -495,7 +496,8 @@ describe('UserMembershipController', () => { write: 'manager_ids', }, }, - newUser.email + newUser.email, + { initiatorId: 'mock-user-id', ipAddress: '1.2.3.4' } ) }, }) @@ -613,7 +615,8 @@ describe('UserMembershipController', () => { write: 'manager_ids', }, }, - newUser._id + newUser._id, + { initiatorId: 'mock-user-id', ipAddress: '1.2.3.4' } ) }, }) diff --git a/services/web/test/unit/src/UserMembership/UserMembershipHandler.test.mjs b/services/web/test/unit/src/UserMembership/UserMembershipHandler.test.mjs index 4ac56dc250..ef820588de 100644 --- a/services/web/test/unit/src/UserMembership/UserMembershipHandler.test.mjs +++ b/services/web/test/unit/src/UserMembership/UserMembershipHandler.test.mjs @@ -35,6 +35,9 @@ describe('UserMembershipHandler', function () { update: vi.fn().mockReturnValue({ exec: vi.fn().mockResolvedValue(), }), + updateOne: vi.fn().mockReturnValue({ + exec: vi.fn().mockResolvedValue(), + }), } ctx.institution = { _id: 'mock-institution-id', @@ -83,6 +86,21 @@ describe('UserMembershipHandler', function () { }), } + ctx.Modules = { + promises: { + hooks: { + fire: vi.fn().mockResolvedValue(), + }, + }, + } + + ctx.mongoose = { + startSession: vi.fn().mockResolvedValue({ + withTransaction: vi.fn(async callback => await callback()), + endSession: vi.fn().mockResolvedValue(), + }), + } + vi.doMock('mongodb-legacy', () => ({ default: { ObjectId }, })) @@ -110,6 +128,14 @@ describe('UserMembershipHandler', function () { Publisher: ctx.Publisher, })) + vi.doMock('../../../../app/src/infrastructure/Modules', () => ({ + default: ctx.Modules, + })) + + vi.doMock('../../../../app/src/infrastructure/Mongoose', () => ({ + default: ctx.mongoose, + })) + ctx.UserMembershipHandler = (await import(modulePath)).default }) @@ -254,6 +280,66 @@ describe('UserMembershipHandler', function () { expect(user).to.equal(ctx.newUser) }) }) + + describe('group managers', function () { + it('add user to group managers', async function (ctx) { + await ctx.UserMembershipHandler.promises.addUser( + ctx.subscription, + EntityConfigs.groupManagers, + ctx.email + ) + expect(ctx.subscription.updateOne).toHaveBeenCalledWith({ + $addToSet: { manager_ids: ctx.newUser._id }, + }) + }) + + it('should write a group audit log when subscription has managed users enabled', async function (ctx) { + ctx.subscription.managedUsersEnabled = true + const auditInfo = { + initiatorId: new ObjectId(), + ipAddress: '192.168.1.1', + } + + await ctx.UserMembershipHandler.promises.addUser( + ctx.subscription, + EntityConfigs.groupManagers, + ctx.email, + auditInfo + ) + + expect(ctx.Modules.promises.hooks.fire).toHaveBeenCalledWith( + 'addGroupAuditLogEntry', + { + groupId: ctx.subscription._id, + operation: 'group-role-changed', + initiatorId: auditInfo.initiatorId, + ipAddress: auditInfo.ipAddress, + info: { + userId: ctx.newUser._id, + role: 'manager', + }, + }, + expect.anything() // session object + ) + }) + + it('should not write a group audit log when subscription does not have managed users enabled', async function (ctx) { + ctx.subscription.managedUsersEnabled = false + const auditInfo = { + initiatorId: new ObjectId(), + ipAddress: '192.168.1.1', + } + + await ctx.UserMembershipHandler.promises.addUser( + ctx.subscription, + EntityConfigs.groupManagers, + ctx.email, + auditInfo + ) + + expect(ctx.Modules.promises.hooks.fire).not.toHaveBeenCalled() + }) + }) }) describe('removeUser', function () { @@ -283,5 +369,64 @@ describe('UserMembershipHandler', function () { } }) }) + + describe('group managers', function () { + it('remove user from group managers', async function (ctx) { + await ctx.UserMembershipHandler.promises.removeUser( + ctx.subscription, + EntityConfigs.groupManagers, + ctx.newUser._id + ) + expect(ctx.subscription.updateOne).toHaveBeenCalledWith({ + $pull: { manager_ids: ctx.newUser._id }, + }) + }) + + it('should write a group audit log when subscription has managed users enabled', async function (ctx) { + ctx.subscription.managedUsersEnabled = true + const auditInfo = { + initiatorId: new ObjectId(), + ipAddress: '192.168.1.1', + } + + await ctx.UserMembershipHandler.promises.removeUser( + ctx.subscription, + EntityConfigs.groupManagers, + ctx.newUser._id, + auditInfo + ) + + expect(ctx.Modules.promises.hooks.fire).toHaveBeenCalledWith( + 'addGroupAuditLogEntry', + { + groupId: ctx.subscription._id, + operation: 'group-role-changed', + initiatorId: auditInfo.initiatorId, + ipAddress: auditInfo.ipAddress, + info: { + userId: ctx.newUser._id, + role: 'member', + }, + } + ) + }) + + it('should not write a group audit log when subscription does not have managed users enabled', async function (ctx) { + ctx.subscription.managedUsersEnabled = false + const auditInfo = { + initiatorId: new ObjectId(), + ipAddress: '192.168.1.1', + } + + await ctx.UserMembershipHandler.promises.removeUser( + ctx.subscription, + EntityConfigs.groupManagers, + ctx.newUser._id, + auditInfo + ) + + expect(ctx.Modules.promises.hooks.fire).not.toHaveBeenCalled() + }) + }) }) })