diff --git a/services/web/app/src/Features/Project/ProjectAuditLogHandler.mjs b/services/web/app/src/Features/Project/ProjectAuditLogHandler.mjs index 6fd878267d..c7d7c1faec 100644 --- a/services/web/app/src/Features/Project/ProjectAuditLogHandler.mjs +++ b/services/web/app/src/Features/Project/ProjectAuditLogHandler.mjs @@ -2,6 +2,7 @@ import logger from '@overleaf/logger' import { ProjectAuditLogEntry } from '../../models/ProjectAuditLogEntry.mjs' import { callbackify } from '@overleaf/promise-utils' import SubscriptionLocator from '../Subscription/SubscriptionLocator.mjs' +import _ from 'lodash' const MANAGED_GROUP_PROJECT_EVENTS = [ 'send-invite', @@ -14,8 +15,35 @@ const MANAGED_GROUP_PROJECT_EVENTS = [ 'project-untrashed', 'project-restored', 'project-cloned', + 'transfer-ownership', ] +async function findManagedSubscriptions(entry) { + if (!MANAGED_GROUP_PROJECT_EVENTS.includes(entry.operation)) { + return + } + + // remove duplications and empty values + const userIds = _.uniq( + _.compact([ + entry.info?.previousOwnerId, + entry.info?.newOwnerId, + entry.initiatorId, + ]) + ) + + const managedSubscriptions = await Promise.all( + userIds.map(id => + SubscriptionLocator.promises.getUniqueManagedSubscriptionMemberOf(id) + ) + ) + const ids = managedSubscriptions.map(subscription => + subscription?._id.toString() + ) + + return _.uniq(_.compact(ids)) +} + export default { promises: { addEntry, @@ -29,13 +57,16 @@ export default { } /** - * Add an audit log entry + * Add an audit log entry. If the entry involves multiple managed subscriptions, + * adds multiple entries each with a different managedSubscriptionId. * * The entry should include at least the following fields: * - * - operation: a string identifying the type of operation - * - userId: the user on behalf of whom the operation was performed - * - message: a string detailing what happened + * @param {ObjectId} projectId - the project for which the operation was performed + * @param {string} operation - a string identifying the type of operation + * @param {ObjectId} initiatorId - the user on behalf of whom the operation was performed + * @param {string} ipAddress - the IP address of the initiator + * @param {object} info - any additional payload */ async function addEntry( projectId, @@ -51,20 +82,32 @@ async function addEntry( ipAddress, info, } - - if (MANAGED_GROUP_PROJECT_EVENTS.includes(operation)) { - const managedSubscription = - await SubscriptionLocator.promises.getUniqueManagedSubscriptionMemberOf( - info.userId || initiatorId - ) - - if (managedSubscription) { - entry.managedSubscriptionId = managedSubscription._id + const managedSubscriptions = await findManagedSubscriptions(entry) + if (managedSubscriptions?.length) { + for (const managedSubscriptionId of managedSubscriptions) { + await ProjectAuditLogEntry.create({ + ...entry, + managedSubscriptionId, + }) } + } else { + await ProjectAuditLogEntry.create(entry) } - await ProjectAuditLogEntry.create(entry) } +/** + * Add an audit log entry only if the entry is related to a managed subscription. + * If the entry involves multiple managed subscriptions, adds multiple entries each + * with a different managedSubscriptionId. + * + * The entry should include at least the following fields: + * + * @param {ObjectId} projectId - the project for which the operation was performed + * @param {string} operation - a string identifying the type of operation + * @param {ObjectId} initiatorId - the user on behalf of whom the operation was performed + * @param {string} ipAddress - the IP address of the initiator + * @param {object} info - any additional payload + */ async function addEntryIfManaged( projectId, operation, @@ -76,24 +119,25 @@ async function addEntryIfManaged( return } - const managedSubscription = - await SubscriptionLocator.promises.getUniqueManagedSubscriptionMemberOf( - info.userId || initiatorId - ) - if (!managedSubscription) { - return - } - const entry = { projectId, operation, initiatorId, ipAddress, info, - managedSubscriptionId: managedSubscription._id, } - await ProjectAuditLogEntry.create(entry) + const managedSubscriptions = await findManagedSubscriptions(entry) + if (!managedSubscriptions?.length) { + return + } + + for (const managedSubscriptionId of managedSubscriptions) { + await ProjectAuditLogEntry.create({ + ...entry, + managedSubscriptionId, + }) + } } /** @@ -116,6 +160,9 @@ function addEntryInBackground( }) } +/** + * Add an audit log entry in the background only if related to a managed subscription. + */ function addEntryIfManagedInBackground( projectId, operation, diff --git a/services/web/test/unit/src/Project/ProjectAuditLogHandler.test.mjs b/services/web/test/unit/src/Project/ProjectAuditLogHandler.test.mjs index 39702ff971..2659cadf37 100644 --- a/services/web/test/unit/src/Project/ProjectAuditLogHandler.test.mjs +++ b/services/web/test/unit/src/Project/ProjectAuditLogHandler.test.mjs @@ -9,6 +9,9 @@ const { ObjectId } = mongodb const projectId = new ObjectId() const userId = new ObjectId() const subscriptionId = new ObjectId() +const previousOwnerId = new ObjectId() +const newOwnerId = new ObjectId() +const subscriptionId2 = new ObjectId() describe('ProjectAuditLogHandler', function (ctx) { beforeEach(async function (ctx) { @@ -75,7 +78,7 @@ describe('ProjectAuditLogHandler', function (ctx) { '0:0:0:0' ) expect(ctx.createEntryMock).to.have.been.calledWithMatch({ - managedSubscriptionId: subscriptionId, + managedSubscriptionId: subscriptionId.toString(), }) }) @@ -93,6 +96,29 @@ describe('ProjectAuditLogHandler', function (ctx) { managedSubscriptionId: subscriptionId, }) }) + + it('adds multiple entries when the log involves multiple group subscriptions', async function (ctx) { + ctx.getUniqueManagedSubscriptionMemberOfMock.onFirstCall().resolves({ + _id: subscriptionId, + }) + ctx.getUniqueManagedSubscriptionMemberOfMock.onSecondCall().resolves({ + _id: subscriptionId2, + }) + await ctx.ProjectAuditLogHandler.promises.addEntry( + projectId, + 'transfer-ownership', + userId, + '0:0:0:0', + { previousOwnerId, newOwnerId } + ) + expect(ctx.createEntryMock).to.have.been.calledTwice + expect(ctx.createEntryMock).to.have.been.calledWithMatch({ + managedSubscriptionId: subscriptionId.toString(), + }) + expect(ctx.createEntryMock).to.have.been.calledWithMatch({ + managedSubscriptionId: subscriptionId2.toString(), + }) + }) }) describe('addEntryIfManaged', function () { @@ -116,7 +142,7 @@ describe('ProjectAuditLogHandler', function (ctx) { initiatorId: userId, ipAddress: '0:0:0:0', info: {}, - managedSubscriptionId: subscriptionId, + managedSubscriptionId: subscriptionId.toString(), }) }) @@ -142,5 +168,28 @@ describe('ProjectAuditLogHandler', function (ctx) { expect(ctx.createEntryMock).not.to.have.been.called }) }) + + it('adds multiple entries when the log involves multiple group subscriptions', async function (ctx) { + ctx.getUniqueManagedSubscriptionMemberOfMock.onFirstCall().resolves({ + _id: subscriptionId, + }) + ctx.getUniqueManagedSubscriptionMemberOfMock.onSecondCall().resolves({ + _id: subscriptionId2, + }) + await ctx.ProjectAuditLogHandler.promises.addEntryIfManaged( + projectId, + 'transfer-ownership', + userId, + '0:0:0:0', + { previousOwnerId, newOwnerId } + ) + expect(ctx.createEntryMock).to.have.been.calledTwice + expect(ctx.createEntryMock).to.have.been.calledWithMatch({ + managedSubscriptionId: subscriptionId.toString(), + }) + expect(ctx.createEntryMock).to.have.been.calledWithMatch({ + managedSubscriptionId: subscriptionId2.toString(), + }) + }) }) })