Просмотр исходного кода

Update delete user audit log for managed users

GitOrigin-RevId: 5b5b164ebf5b077ec783709d6846a52e94d21565
Simon Gardner 8 месяцев назад
Родитель
Сommit
377e431146

+ 2 - 0
services/web/app/src/Features/User/UserAuditLogHandler.mjs

@@ -40,6 +40,8 @@ const MANAGED_GROUP_USER_EVENTS = [
   'unlink-dropbox',
   'link-github',
   'unlink-github',
+  'delete-account',
+  'leave-group-subscription',
 ]
 
 /**

+ 10 - 5
services/web/app/src/Features/User/UserDeleter.mjs

@@ -45,17 +45,22 @@ async function deleteUser(userId, options) {
     const user = await User.findById(userId).exec()
     logger.info({ userId }, 'deleting user')
     await ensureCanDeleteUser(user)
-    logger.info({ userId }, 'cleaning up user')
-    await _cleanupUser(user)
-    logger.info({ userId }, 'firing deleteUser hook')
-    await Modules.promises.hooks.fire('deleteUser', userId)
+
+    // add audit log entry before _cleanUpUser removes any group subscriptions
     logger.info({ userId }, 'adding delete-account audit log entry')
     await UserAuditLogHandler.promises.addEntry(
       userId,
       'delete-account',
       options.deleterUser ? options.deleterUser._id : userId,
-      options.ipAddress
+      options.ipAddress,
+      {}
     )
+
+    logger.info({ userId }, 'cleaning up user')
+    await _cleanupUser(user)
+    logger.info({ userId }, 'firing deleteUser hook')
+    await Modules.promises.hooks.fire('deleteUser', userId)
+
     logger.info({ userId }, 'creating deleted user record')
     await _createDeletedUser(user, options)
     logger.info({ userId }, 'deleting user projects')

+ 80 - 2
services/web/test/unit/src/User/UserDeleter.test.mjs

@@ -69,6 +69,7 @@ describe('UserDeleter', function () {
     ctx.SubscriptionLocator = {
       promises: {
         getUsersSubscription: sinon.stub().resolves(),
+        getUniqueManagedSubscriptionMemberOf: sinon.stub().resolves(),
       },
     }
 
@@ -427,7 +428,8 @@ describe('UserDeleter', function () {
               ctx.userId,
               'delete-account',
               ctx.userId,
-              ctx.ipAddress
+              ctx.ipAddress,
+              {}
             )
           })
         })
@@ -508,7 +510,8 @@ describe('UserDeleter', function () {
             ctx.userId,
             'delete-account',
             ctx.deleterId,
-            ctx.ipAddress
+            ctx.ipAddress,
+            {}
           )
         })
 
@@ -524,6 +527,81 @@ describe('UserDeleter', function () {
           })
         })
       })
+
+      describe('when the user is part of a managed subscription', function () {
+        beforeEach(function (ctx) {
+          ctx.managedSubscriptionId = new ObjectId()
+          ctx.SubscriptionLocator.promises.getUniqueManagedSubscriptionMemberOf.resolves(
+            {
+              _id: ctx.managedSubscriptionId,
+            }
+          )
+
+          ctx.DeletedUserMock.expects('updateOne')
+            .withArgs(
+              { 'deleterData.deletedUserId': ctx.userId },
+              ctx.deletedUser,
+              { upsert: true }
+            )
+            .chain('exec')
+            .resolves()
+          ctx.UserMock.expects('deleteOne')
+            .withArgs({ _id: ctx.userId })
+            .chain('exec')
+            .resolves()
+        })
+
+        it('should include managedSubscriptionId in audit log', async function (ctx) {
+          await ctx.UserDeleter.promises.deleteUser(ctx.userId, {
+            ipAddress: ctx.ipAddress,
+          })
+          expect(
+            ctx.UserAuditLogHandler.promises.addEntry
+          ).to.have.been.calledWith(
+            ctx.userId,
+            'delete-account',
+            ctx.userId,
+            ctx.ipAddress,
+            {}
+          )
+        })
+      })
+
+      describe('when checking managed subscription fails', function () {
+        beforeEach(function (ctx) {
+          ctx.SubscriptionLocator.promises.getUniqueManagedSubscriptionMemberOf.rejects(
+            new Error('subscription lookup failed')
+          )
+
+          ctx.DeletedUserMock.expects('updateOne')
+            .withArgs(
+              { 'deleterData.deletedUserId': ctx.userId },
+              ctx.deletedUser,
+              { upsert: true }
+            )
+            .chain('exec')
+            .resolves()
+          ctx.UserMock.expects('deleteOne')
+            .withArgs({ _id: ctx.userId })
+            .chain('exec')
+            .resolves()
+        })
+
+        it('should continue with deletion and not include managedSubscriptionId', async function (ctx) {
+          await ctx.UserDeleter.promises.deleteUser(ctx.userId, {
+            ipAddress: ctx.ipAddress,
+          })
+          expect(
+            ctx.UserAuditLogHandler.promises.addEntry
+          ).to.have.been.calledWith(
+            ctx.userId,
+            'delete-account',
+            ctx.userId,
+            ctx.ipAddress,
+            {}
+          )
+        })
+      })
     })
 
     describe('when the user cannot be deleted because they are a subscription admin', function () {