Merge pull request #15001 from overleaf/em-invite-audit-logs

Project audit logs for invite operations

GitOrigin-RevId: c2db4bc719f508c5bf33be2c59eddfb63fcdae25

services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js

@@ -12,6 +12,8 @@ const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const SessionManager = require('../Authentication/SessionManager')
 const { RateLimiter } = require('../../infrastructure/RateLimiter')
 const { expressify } = require('../../util/promises')
+const ProjectAuditLogHandler = require('../Project/ProjectAuditLogHandler')
+const Errors = require('../Errors/Errors')
 
 // This rate limiter allows a different number of requests depending on the
 // number of callaborators a user is allowed. This is implemented by providing
@@ -138,7 +140,20 @@ const CollaboratorsInviteController = {
       email,
       privileges
     )
+
+    ProjectAuditLogHandler.addEntryInBackground(
+      projectId,
+      'send-invite',
+      sendingUserId,
+      req.ip,
+      {
+        inviteId: invite._id,
+        privileges,
+      }
+    )
+
     logger.debug({ projectId, email, sendingUserId }, 'invite created')
+
     EditorRealTimeController.emitToRoom(
       projectId,
       'project:membership:changed',
@@ -150,21 +165,40 @@ const CollaboratorsInviteController = {
   async revokeInvite(req, res) {
     const projectId = req.params.Project_id
     const inviteId = req.params.invite_id
+    const user = SessionManager.getSessionUser(req.session)
 
     logger.debug({ projectId, inviteId }, 'revoking invite')
-    await CollaboratorsInviteHandler.promises.revokeInvite(projectId, inviteId)
-    EditorRealTimeController.emitToRoom(
+
+    const invite = await CollaboratorsInviteHandler.promises.revokeInvite(
       projectId,
-      'project:membership:changed',
-      { invites: true }
+      inviteId
     )
 
-    res.sendStatus(201)
+    if (invite != null) {
+      ProjectAuditLogHandler.addEntryInBackground(
+        projectId,
+        'revoke-invite',
+        user._id,
+        req.ip,
+        {
+          inviteId: invite._id,
+          privileges: invite.privileges,
+        }
+      )
+      EditorRealTimeController.emitToRoom(
+        projectId,
+        'project:membership:changed',
+        { invites: true }
+      )
+    }
+
+    res.sendStatus(204)
   },
 
   async resendInvite(req, res) {
     const projectId = req.params.Project_id
     const inviteId = req.params.invite_id
+    const user = SessionManager.getSessionUser(req.session)
 
     logger.debug({ projectId, inviteId }, 'resending invite')
     const sendingUser = SessionManager.getSessionUser(req.session)
@@ -175,12 +209,25 @@ const CollaboratorsInviteController = {
       return res.sendStatus(429)
     }
 
-    await CollaboratorsInviteHandler.promises.resendInvite(
+    const invite = await CollaboratorsInviteHandler.promises.resendInvite(
       projectId,
       sendingUser,
       inviteId
     )
 
+    if (invite != null) {
+      ProjectAuditLogHandler.addEntryInBackground(
+        projectId,
+        'resend-invite',
+        user._id,
+        req.ip,
+        {
+          inviteId: invite._id,
+          privileges: invite.privileges,
+        }
+      )
+    }
+
     res.sendStatus(201)
   },
 
@@ -248,21 +295,40 @@ const CollaboratorsInviteController = {
   },
 
   async acceptInvite(req, res) {
-    const projectId = req.params.Project_id
-    const { token } = req.params
+    const { Project_id: projectId, token } = req.params
     const currentUser = SessionManager.getSessionUser(req.session)
     logger.debug(
       { projectId, userId: currentUser._id },
       'got request to accept invite'
     )
 
+    const invite = await CollaboratorsInviteHandler.promises.getInviteByToken(
+      projectId,
+      token
+    )
+
+    if (invite == null) {
+      throw new Errors.NotFoundError('no matching invite found')
+    }
+
+    await ProjectAuditLogHandler.promises.addEntry(
+      projectId,
+      'accept-invite',
+      currentUser._id,
+      req.ip,
+      {
+        inviteId: invite._id,
+        privileges: invite.privileges,
+      }
+    )
+
     await CollaboratorsInviteHandler.promises.acceptInvite(
+      invite,
       projectId,
-      token,
       currentUser
     )
 
-    EditorRealTimeController.emitToRoom(
+    await EditorRealTimeController.emitToRoom(
       projectId,
       'project:membership:changed',
       { invites: true, members: true }

services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.js

@@ -5,7 +5,6 @@ const CollaboratorsEmailHandler = require('./CollaboratorsEmailHandler')
 const CollaboratorsHandler = require('./CollaboratorsHandler')
 const UserGetter = require('../User/UserGetter')
 const ProjectGetter = require('../Project/ProjectGetter')
-const Errors = require('../Errors/Errors')
 const Crypto = require('crypto')
 const NotificationsBuilder = require('../Notifications/NotificationsBuilder')
 
@@ -107,7 +106,10 @@ const CollaboratorsInviteHandler = {
 
   async revokeInvite(projectId, inviteId) {
     logger.debug({ projectId, inviteId }, 'removing invite')
-    await ProjectInvite.deleteOne({ projectId, _id: inviteId }).exec()
+    const invite = await ProjectInvite.findOneAndDelete({
+      projectId,
+      _id: inviteId,
+    }).exec()
     CollaboratorsInviteHandler._tryCancelInviteNotification(inviteId).catch(
       err => {
         logger.err(
@@ -116,6 +118,7 @@ const CollaboratorsInviteHandler = {
         )
       }
     )
+    return invite
   },
 
   async resendInvite(projectId, sendingUser, inviteId) {
@@ -127,7 +130,7 @@ const CollaboratorsInviteHandler = {
 
     if (invite == null) {
       logger.warn({ projectId, inviteId }, 'no invite found, nothing to resend')
-      return
+      return null
     }
 
     await CollaboratorsInviteHandler._sendMessages(
@@ -135,6 +138,8 @@ const CollaboratorsInviteHandler = {
       sendingUser,
       invite
     )
+
+    return invite
   },
 
   async getInviteByToken(projectId, tokenString) {
@@ -152,17 +157,7 @@ const CollaboratorsInviteHandler = {
     return invite
   },
 
-  async acceptInvite(projectId, tokenString, user) {
-    logger.debug({ projectId, userId: user._id }, 'accepting invite')
-    const invite = await CollaboratorsInviteHandler.getInviteByToken(
-      projectId,
-      tokenString
-    )
-
-    if (!invite) {
-      throw new Errors.NotFoundError('no matching invite found')
-    }
-    const inviteId = invite._id
+  async acceptInvite(invite, projectId, user) {
     CollaboratorsHandler.promises.addUserIdToProject(
       projectId,
       invite.sendingUserId,
@@ -171,6 +166,7 @@ const CollaboratorsInviteHandler = {
     )
 
     // Remove invite
+    const inviteId = invite._id
     logger.debug({ projectId, inviteId }, 'removing invite')
     await ProjectInvite.deleteOne({ _id: inviteId }).exec()
     CollaboratorsInviteHandler._tryCancelInviteNotification(inviteId).catch(

services/web/app/src/Features/Collaborators/OwnershipTransferHandler.js

@@ -49,6 +49,7 @@ async function transferOwnership(projectId, newOwnerId, options = {}) {
     projectId,
     'transfer-ownership',
     sessionUserId,
+    '', // IP address
     { previousOwnerId, newOwnerId }
   )
   await _transferOwnership(projectId, previousOwnerId, newOwnerId)

services/web/app/src/Features/Project/ProjectAuditLogHandler.js

@@ -1,3 +1,4 @@
+const logger = require('@overleaf/logger')
 const { ProjectAuditLogEntry } = require('../../models/ProjectAuditLogEntry')
 const { callbackify } = require('../../util/promises')
 
@@ -5,7 +6,8 @@ module.exports = {
   promises: {
     addEntry,
   },
-  addEntry: callbackify(addEntry), // callback version of adEntry
+  addEntry: callbackify(addEntry), // callback version of addEntry
+  addEntryInBackground,
 }
 
 /**
@@ -17,12 +19,39 @@ module.exports = {
  * - userId: the user on behalf of whom the operation was performed
  * - message: a string detailing what happened
  */
-async function addEntry(projectId, operation, initiatorId, info = {}) {
+async function addEntry(
+  projectId,
+  operation,
+  initiatorId,
+  ipAddress,
+  info = {}
+) {
   const entry = {
     projectId,
     operation,
     initiatorId,
+    ipAddress,
     info,
   }
   await ProjectAuditLogEntry.create(entry)
 }
+
+/**
+ * Add an audit log entry in the background
+ *
+ * This function doesn't return a promise. Instead, it catches any error and logs it.
+ */
+function addEntryInBackground(
+  projectId,
+  operation,
+  initiatorId,
+  ipAddress,
+  info = {}
+) {
+  addEntry(projectId, operation, initiatorId, ipAddress, info).catch(err => {
+    logger.error(
+      { err, projectId, operation, initiatorId, ipAddress, info },
+      'Failed to write audit log'
+    )
+  })
+}

services/web/app/src/Features/Project/ProjectController.js

@@ -133,6 +133,7 @@ const ProjectController = {
           projectId,
           'toggle-access-level',
           user._id,
+          req.ip,
           { publicAccessLevel: req.body.publicAccessLevel, status: 'OK' },
           callback
         )

services/web/app/src/models/ProjectAuditLogEntry.js

@@ -6,6 +6,7 @@ const ProjectAuditLogEntrySchema = new Schema(
     projectId: { type: Schema.Types.ObjectId, index: true },
     operation: { type: String },
     initiatorId: { type: Schema.Types.ObjectId },
+    ipAddress: { type: String },
     timestamp: { type: Date, default: Date.now },
     info: { type: Object },
   },

services/web/test/unit/src/Collaborators/CollaboratorsInviteHandlerTests.js

@@ -3,7 +3,6 @@ const { expect } = require('chai')
 const SandboxedModule = require('sandboxed-module')
 const { ObjectId } = require('mongodb')
 const Crypto = require('crypto')
-const Errors = require('../../../../app/src/Features/Errors/Errors')
 
 const MODULE_PATH =
   '../../../../app/src/Features/Collaborators/CollaboratorsInviteHandler.js'
@@ -26,6 +25,7 @@ describe('CollaboratorsInviteHandler', function () {
     this.ProjectInvite.findOne = sinon.stub()
     this.ProjectInvite.find = sinon.stub()
     this.ProjectInvite.deleteOne = sinon.stub()
+    this.ProjectInvite.findOneAndDelete = sinon.stub()
     this.ProjectInvite.countDocuments = sinon.stub()
 
     this.Crypto = {
@@ -177,8 +177,6 @@ describe('CollaboratorsInviteHandler', function () {
     })
 
     describe('when all goes well', function () {
-      beforeEach(function () {})
-
       it('should produce the invite object', async function () {
         const invite = await this.call()
         expect(invite).to.not.equal(null)
@@ -296,13 +294,13 @@ describe('CollaboratorsInviteHandler', function () {
 
   describe('revokeInvite', function () {
     beforeEach(function () {
-      this.ProjectInvite.deleteOne.returns({
-        exec: sinon.stub().resolves(),
+      this.ProjectInvite.findOneAndDelete.returns({
+        exec: sinon.stub().resolves(this.fakeInvite),
       })
       this.CollaboratorsInviteHandler.promises._tryCancelInviteNotification =
         sinon.stub().resolves()
       this.call = async () => {
-        await this.CollaboratorsInviteHandler.promises.revokeInvite(
+        return await this.CollaboratorsInviteHandler.promises.revokeInvite(
           this.projectId,
           this.inviteId
         )
@@ -310,14 +308,13 @@ describe('CollaboratorsInviteHandler', function () {
     })
 
     describe('when all goes well', function () {
-      beforeEach(function () {})
-
-      it('should call ProjectInvite.deleteOne', async function () {
+      it('should call ProjectInvite.findOneAndDelete', async function () {
         await this.call()
-        this.ProjectInvite.deleteOne.callCount.should.equal(1)
-        this.ProjectInvite.deleteOne
-          .calledWith({ projectId: this.projectId, _id: this.inviteId })
-          .should.equal(true)
+        this.ProjectInvite.findOneAndDelete.should.have.been.calledOnce
+        this.ProjectInvite.findOneAndDelete.should.have.been.calledWith({
+          projectId: this.projectId,
+          _id: this.inviteId,
+        })
       })
 
       it('should call _tryCancelInviteNotification', async function () {
@@ -329,11 +326,16 @@ describe('CollaboratorsInviteHandler', function () {
           .calledWith(this.inviteId)
           .should.equal(true)
       })
+
+      it('should return the deleted invite', async function () {
+        const invite = await this.call()
+        expect(invite).to.deep.equal(this.fakeInvite)
+      })
     })
 
     describe('when remove produces an error', function () {
       beforeEach(function () {
-        this.ProjectInvite.deleteOne.returns({
+        this.ProjectInvite.findOneAndDelete.returns({
           exec: sinon.stub().rejects(new Error('woops')),
         })
       })
@@ -353,7 +355,7 @@ describe('CollaboratorsInviteHandler', function () {
         .stub()
         .resolves()
       this.call = async () => {
-        await this.CollaboratorsInviteHandler.promises.resendInvite(
+        return await this.CollaboratorsInviteHandler.promises.resendInvite(
           this.projectId,
           this.sendingUser,
           this.inviteId
@@ -362,8 +364,6 @@ describe('CollaboratorsInviteHandler', function () {
     })
 
     describe('when all goes well', function () {
-      beforeEach(function () {})
-
       it('should call ProjectInvite.findOne', async function () {
         await this.call()
         this.ProjectInvite.findOne.callCount.should.equal(1)
@@ -381,6 +381,11 @@ describe('CollaboratorsInviteHandler', function () {
           .calledWith(this.projectId, this.sendingUser, this.fakeInvite)
           .should.equal(true)
       })
+
+      it('should return the invite', async function () {
+        const invite = await this.call()
+        expect(invite).to.deep.equal(this.fakeInvite)
+      })
     })
 
     describe('when findOne produces an error', function () {
@@ -480,49 +485,30 @@ describe('CollaboratorsInviteHandler', function () {
         readOnly_refs: [],
       }
       this.CollaboratorsHandler.promises.addUserIdToProject.resolves()
-      this._getInviteByToken = sinon.stub(
-        this.CollaboratorsInviteHandler.promises,
-        'getInviteByToken'
-      )
-      this._getInviteByToken.resolves(this.fakeInvite)
       this.CollaboratorsInviteHandler.promises._tryCancelInviteNotification =
         sinon.stub().resolves()
       this.ProjectInvite.deleteOne.returns({ exec: sinon.stub().resolves() })
       this.call = async () => {
         await this.CollaboratorsInviteHandler.promises.acceptInvite(
+          this.fakeInvite,
           this.projectId,
-          this.token,
           this.user
         )
       }
     })
 
-    afterEach(function () {
-      this._getInviteByToken.restore()
-    })
-
     describe('when all goes well', function () {
-      it('should have called getInviteByToken', async function () {
-        await this.call()
-        this._getInviteByToken.callCount.should.equal(1)
-        this._getInviteByToken
-          .calledWith(this.projectId, this.token)
-          .should.equal(true)
-      })
-
       it('should have called CollaboratorsHandler.addUserIdToProject', async function () {
         await this.call()
         this.CollaboratorsHandler.promises.addUserIdToProject.callCount.should.equal(
           1
         )
-        this.CollaboratorsHandler.promises.addUserIdToProject
-          .calledWith(
-            this.projectId,
-            this.sendingUserId,
-            this.userId,
-            this.fakeInvite.privileges
-          )
-          .should.equal(true)
+        this.CollaboratorsHandler.promises.addUserIdToProject.should.have.been.calledWith(
+          this.projectId,
+          this.sendingUserId,
+          this.userId,
+          this.fakeInvite.privileges
+        )
       })
 
       it('should have called ProjectInvite.deleteOne', async function () {
@@ -537,7 +523,6 @@ describe('CollaboratorsInviteHandler', function () {
     describe('when the invite is for readOnly access', function () {
       beforeEach(function () {
         this.fakeInvite.privileges = 'readOnly'
-        this._getInviteByToken.resolves(this.fakeInvite)
       })
 
       it('should have called CollaboratorsHandler.addUserIdToProject', async function () {
@@ -556,66 +541,6 @@ describe('CollaboratorsInviteHandler', function () {
       })
     })
 
-    describe('when getInviteByToken does not find an invite', function () {
-      beforeEach(function () {
-        this._getInviteByToken.resolves(null)
-      })
-
-      it('should produce an error', async function () {
-        await expect(this.call()).to.be.rejectedWith(Errors.NotFoundError)
-      })
-
-      it('should have called getInviteByToken', async function () {
-        await expect(this.call()).to.be.rejected
-        this._getInviteByToken.callCount.should.equal(1)
-        this._getInviteByToken
-          .calledWith(this.projectId, this.token)
-          .should.equal(true)
-      })
-
-      it('should not have called CollaboratorsHandler.addUserIdToProject', async function () {
-        await expect(this.call()).to.be.rejected
-        this.CollaboratorsHandler.promises.addUserIdToProject.callCount.should.equal(
-          0
-        )
-      })
-
-      it('should not have called ProjectInvite.deleteOne', async function () {
-        await expect(this.call()).to.be.rejected
-        this.ProjectInvite.deleteOne.callCount.should.equal(0)
-      })
-    })
-
-    describe('when getInviteByToken produces an error', function () {
-      beforeEach(function () {
-        this._getInviteByToken.rejects(new Error('woops'))
-      })
-
-      it('should produce an error', async function () {
-        await expect(this.call()).to.be.rejectedWith(Error)
-      })
-
-      it('should have called getInviteByToken', async function () {
-        await expect(this.call()).to.be.rejected
-        this._getInviteByToken.callCount.should.equal(1)
-        this._getInviteByToken
-          .calledWith(this.projectId, this.token)
-          .should.equal(true)
-      })
-
-      it('should not have called CollaboratorsHandler.addUserIdToProject', async function () {
-        await expect(this.call()).to.be.rejected
-        this.CollaboratorsHandler.promises.addUserIdToProject.callCount.should.equal(
-          0
-        )
-      })
-
-      it('should not have called ProjectInvite.deleteOne', async function () {
-        await expect(this.call()).to.be.rejected
-        this.ProjectInvite.deleteOne.callCount.should.equal(0)
-      })
-    })
-
     describe('when addUserIdToProject produces an error', function () {
       beforeEach(function () {
         this.CollaboratorsHandler.promises.addUserIdToProject.callsArgWith(
@@ -628,14 +553,6 @@ describe('CollaboratorsInviteHandler', function () {
         await expect(this.call()).to.be.rejectedWith(Error)
       })
 
-      it('should have called getInviteByToken', async function () {
-        await expect(this.call()).to.be.rejected
-        this._getInviteByToken.callCount.should.equal(1)
-        this._getInviteByToken
-          .calledWith(this.projectId, this.token)
-          .should.equal(true)
-      })
-
       it('should have called CollaboratorsHandler.addUserIdToProject', async function () {
         await expect(this.call()).to.be.rejected
         this.CollaboratorsHandler.promises.addUserIdToProject.callCount.should.equal(
@@ -668,27 +585,17 @@ describe('CollaboratorsInviteHandler', function () {
         await expect(this.call()).to.be.rejectedWith(Error)
       })
 
-      it('should have called getInviteByToken', async function () {
-        await expect(this.call()).to.be.rejected
-        this._getInviteByToken.callCount.should.equal(1)
-        this._getInviteByToken
-          .calledWith(this.projectId, this.token)
-          .should.equal(true)
-      })
-
       it('should have called CollaboratorsHandler.addUserIdToProject', async function () {
         await expect(this.call()).to.be.rejected
         this.CollaboratorsHandler.promises.addUserIdToProject.callCount.should.equal(
           1
         )
-        this.CollaboratorsHandler.promises.addUserIdToProject
-          .calledWith(
-            this.projectId,
-            this.sendingUserId,
-            this.userId,
-            this.fakeInvite.privileges
-          )
-          .should.equal(true)
+        this.CollaboratorsHandler.promises.addUserIdToProject.should.have.been.calledWith(
+          this.projectId,
+          this.sendingUserId,
+          this.userId,
+          this.fakeInvite.privileges
+        )
       })
 
       it('should have called ProjectInvite.deleteOne', async function () {

services/web/test/unit/src/Collaborators/OwnershipTransferHandlerTests.js

@@ -234,6 +234,7 @@ describe('OwnershipTransferHandler', function () {
         this.project._id,
         'transfer-ownership',
         sessionUserId,
+        '', // IP address
         {
           previousOwnerId: this.user._id,
           newOwnerId: this.collaborator._id,

services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -314,10 +314,16 @@ describe('ProjectController', function () {
       }
       this.res.sendStatus = code => {
         this.ProjectAuditLogHandler.addEntry
-          .calledWith(this.project_id, 'toggle-access-level', this.user._id, {
-            publicAccessLevel: 'readOnly',
-            status: 'OK',
-          })
+          .calledWith(
+            this.project_id,
+            'toggle-access-level',
+            this.user._id,
+            this.req.ip,
+            {
+              publicAccessLevel: 'readOnly',
+              status: 'OK',
+            }
+          )
           .should.equal(true)
         done()
       }