diff --git a/services/web/app/src/Features/User/UserAuditLogHandler.mjs b/services/web/app/src/Features/User/UserAuditLogHandler.mjs index 919dc30d8a..21e4ac85c2 100644 --- a/services/web/app/src/Features/User/UserAuditLogHandler.mjs +++ b/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', ] /** diff --git a/services/web/app/src/Features/User/UserDeleter.mjs b/services/web/app/src/Features/User/UserDeleter.mjs index 60704ae572..ee220304b6 100644 --- a/services/web/app/src/Features/User/UserDeleter.mjs +++ b/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') diff --git a/services/web/test/unit/src/User/UserDeleter.test.mjs b/services/web/test/unit/src/User/UserDeleter.test.mjs index 5e41c89a80..70b5b4e721 100644 --- a/services/web/test/unit/src/User/UserDeleter.test.mjs +++ b/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 () {