Merge pull request #25556 from overleaf/jel-group-audit-log-remove-from-group

[web] Log when user leaves or is removed from group

GitOrigin-RevId: 8a5042b21cbf4eb622d5ca35cc095d94fe5a8539

services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs

@@ -108,10 +108,16 @@ async function _removeUserFromGroup(
     })
   }
 
+  const groupAuditLog = {
+    initiatorId: loggedInUserId,
+    ipAddress: req.ip,
+  }
+
   try {
     await SubscriptionGroupHandler.promises.removeUserFromGroup(
       subscriptionId,
-      userToRemoveId
+      userToRemoveId,
+      groupAuditLog
     )
   } catch (error) {
     logger.err(

services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js

@@ -22,10 +22,11 @@ const {
 const EmailHelper = require('../Helpers/EmailHelper')
 const { InvalidEmailError } = require('../Errors/Errors')
 
-async function removeUserFromGroup(subscriptionId, userIdToRemove) {
+async function removeUserFromGroup(subscriptionId, userIdToRemove, auditLog) {
   await SubscriptionUpdater.promises.removeUserFromGroup(
     subscriptionId,
-    userIdToRemove
+    userIdToRemove,
+    auditLog
   )
 }
 
@@ -463,7 +464,9 @@ async function updateGroupMembersBulk(
       )
     }
     for (const user of membersToRemove) {
-      await removeUserFromGroup(subscription._id, user._id)
+      await removeUserFromGroup(subscription._id, user._id, {
+        initiatorId: inviterId,
+      })
     }
   }
 

services/web/app/src/Features/Subscription/SubscriptionUpdater.js

@@ -18,8 +18,31 @@ const Modules = require('../../infrastructure/Modules')
 /**
  * @typedef {import('../../../../types/subscription/dashboard/subscription').Subscription} Subscription
  * @typedef {import('../../../../types/subscription/dashboard/subscription').PaymentProvider} PaymentProvider
+ * @typedef {import('../../../../types/group-management/group-audit-log').GroupAuditLog} GroupAuditLog
  */
 
+/**
+ *
+ * @param {GroupAuditLog} auditLog
+ */
+async function subscriptionUpdateWithAuditLog(dbFilter, dbUpdate, auditLog) {
+  const session = await mongoose.startSession()
+
+  try {
+    await session.withTransaction(async () => {
+      await Subscription.updateOne(dbFilter, dbUpdate, { session }).exec()
+
+      await Modules.promises.hooks.fire(
+        'addGroupAuditLogEntry',
+        auditLog,
+        session
+      )
+    })
+  } finally {
+    await session.endSession()
+  }
+}
+
 /**
  * Change the admin of the given subscription.
  *
@@ -68,8 +91,6 @@ async function syncSubscription(
 }
 
 async function addUserToGroup(subscriptionId, userId, auditLog) {
-  const session = await mongoose.startSession()
-
   await UserAuditLogHandler.promises.addEntry(
     userId,
     'join-group-subscription',
@@ -78,28 +99,16 @@ async function addUserToGroup(subscriptionId, userId, auditLog) {
     { subscriptionId }
   )
 
-  try {
-    await session.withTransaction(async () => {
-      await Subscription.updateOne(
-        { _id: subscriptionId },
-        { $addToSet: { member_ids: userId } },
-        { session }
-      ).exec()
-
-      await Modules.promises.hooks.fire(
-        'addGroupAuditLogEntry',
-        {
-          initiatorId: auditLog?.initiatorId,
-          ipAddress: auditLog?.ipAddress,
-          groupId: subscriptionId,
-          operation: 'join-group',
-        },
-        session
-      )
-    })
-  } finally {
-    await session.endSession()
-  }
+  await subscriptionUpdateWithAuditLog(
+    { _id: subscriptionId },
+    { $addToSet: { member_ids: userId } },
+    {
+      initiatorId: auditLog?.initiatorId,
+      ipAddress: auditLog?.ipAddress,
+      groupId: subscriptionId,
+      operation: 'join-group',
+    }
+  )
 
   await FeaturesUpdater.promises.refreshFeatures(userId, 'add-to-group')
   await _sendUserGroupPlanCodeUserProperty(userId)
@@ -110,7 +119,7 @@ async function addUserToGroup(subscriptionId, userId, auditLog) {
   )
 }
 
-async function removeUserFromGroup(subscriptionId, userId) {
+async function removeUserFromGroup(subscriptionId, userId, auditLog) {
   await UserAuditLogHandler.promises.addEntry(
     userId,
     'leave-group-subscription',
@@ -118,6 +127,19 @@ async function removeUserFromGroup(subscriptionId, userId) {
     undefined,
     { subscriptionId }
   )
+
+  await subscriptionUpdateWithAuditLog(
+    { _id: subscriptionId },
+    { $pull: { member_ids: userId } },
+    {
+      initiatorId: auditLog?.initiatorId,
+      ipAddress: auditLog?.ipAddress,
+      groupId: subscriptionId,
+      operation: 'leave-group',
+      info: { userIdRemoved: userId },
+    }
+  )
+
   await Subscription.updateOne(
     { _id: subscriptionId },
     { $pull: { member_ids: userId } }

services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs

@@ -185,7 +185,10 @@ describe('SubscriptionGroupController', function () {
       const res = {
         sendStatus: () => {
           this.SubscriptionGroupHandler.promises.removeUserFromGroup
-            .calledWith(this.subscriptionId, userIdToRemove)
+            .calledWith(this.subscriptionId, userIdToRemove, {
+              initiatorId: this.req.session.user._id,
+              ipAddress: this.req.ip,
+            })
             .should.equal(true)
           done()
         },
@@ -277,7 +280,11 @@ describe('SubscriptionGroupController', function () {
           sinon.assert.calledWith(
             this.SubscriptionGroupHandler.promises.removeUserFromGroup,
             this.subscriptionId,
-            memberUserIdToremove
+            memberUserIdToremove,
+            {
+              initiatorId: this.req.session.user._id,
+              ipAddress: this.req.ip,
+            }
           )
           done()
         },

services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js

@@ -233,13 +233,15 @@ describe('SubscriptionGroupHandler', function () {
 
   describe('removeUserFromGroup', function () {
     it('should call the subscription updater to remove the user', async function () {
+      const auditLog = { ipAddress: '0:0:0:0', initiatorId: this.user._id }
       await this.Handler.promises.removeUserFromGroup(
         this.adminUser_id,
-        this.user._id
+        this.user._id,
+        auditLog
       )
 
       this.SubscriptionUpdater.promises.removeUserFromGroup
-        .calledWith(this.adminUser_id, this.user._id)
+        .calledWith(this.adminUser_id, this.user._id, auditLog)
         .should.equal(true)
     })
   })
@@ -1149,7 +1151,9 @@ describe('SubscriptionGroupHandler', function () {
 
           expect(
             this.SubscriptionUpdater.promises.removeUserFromGroup
-          ).to.have.been.calledWith(this.subscription._id, members[2]._id)
+          ).to.have.been.calledWith(this.subscription._id, members[2]._id, {
+            initiatorId: inviterId,
+          })
 
           expect(
             this.TeamInvitesHandler.promises.createInvite.callCount

services/web/types/group-management/group-audit-log.ts

@@ -0,0 +1,7 @@
+export type GroupAuditLog = {
+  groupId: string
+  operation: string
+  ipAddress?: string
+  initiatorId?: string
+  info?: object
+}