Merge pull request #2255 from overleaf/em-audit-log

Project audit logs

GitOrigin-RevId: 439add2959be140c4f56ce9b41b9f59d432c494d

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

@@ -1,7 +1,9 @@
 const OError = require('@overleaf/o-error')
 const HttpErrors = require('@overleaf/o-error/http')
+const { ObjectId } = require('mongodb')
 const CollaboratorsHandler = require('./CollaboratorsHandler')
 const CollaboratorsGetter = require('./CollaboratorsGetter')
+const OwnershipTransferHandler = require('./OwnershipTransferHandler')
 const AuthenticationController = require('../Authentication/AuthenticationController')
 const EditorRealTimeController = require('../Editor/EditorRealTimeController')
 const TagsHandler = require('../Tags/TagsHandler')
@@ -13,7 +15,8 @@ module.exports = {
   removeUserFromProject: expressify(removeUserFromProject),
   removeSelfFromProject: expressify(removeSelfFromProject),
   getAllMembers: expressify(getAllMembers),
-  setCollaboratorInfo: expressify(setCollaboratorInfo)
+  setCollaboratorInfo: expressify(setCollaboratorInfo),
+  transferOwnership: expressify(transferOwnership)
 }
 
 async function removeUserFromProject(req, res, next) {
@@ -73,6 +76,43 @@ async function setCollaboratorInfo(req, res, next) {
   }
 }
 
+async function transferOwnership(req, res, next) {
+  const sessionUser = AuthenticationController.getSessionUser(req)
+  const projectId = req.params.Project_id
+  const toUserId = req.body.user_id
+  try {
+    await OwnershipTransferHandler.promises.transferOwnership(
+      projectId,
+      toUserId,
+      {
+        allowTransferToNonCollaborators: sessionUser.isAdmin,
+        sessionUserId: ObjectId(sessionUser._id)
+      }
+    )
+    res.sendStatus(204)
+  } catch (err) {
+    if (err instanceof Errors.ProjectNotFoundError) {
+      throw new HttpErrors.NotFoundError({
+        info: { public: { message: `project not found: ${projectId}` } }
+      })
+    } else if (err instanceof Errors.UserNotFoundError) {
+      throw new HttpErrors.NotFoundError({
+        info: { public: { message: `user not found: ${toUserId}` } }
+      })
+    } else if (err instanceof Errors.UserNotCollaboratorError) {
+      throw new HttpErrors.ForbiddenError({
+        info: {
+          public: {
+            message: `user ${toUserId} should be a collaborator in project ${projectId} prior to ownership transfer`
+          }
+        }
+      })
+    } else {
+      throw new HttpErrors.InternalServerError({}).withCause(err)
+    }
+  }
+}
+
 async function _removeUserIdFromProject(projectId, userId) {
   await CollaboratorsHandler.promises.removeUserFromProject(projectId, userId)
   EditorRealTimeController.emitToRoom(

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

@@ -47,6 +47,21 @@ module.exports = {
       CollaboratorsController.getAllMembers
     )
 
+    webRouter.post(
+      '/project/:Project_id/transfer-ownership',
+      AuthenticationController.requireLogin(),
+      validate({
+        params: Joi.object({
+          Project_id: Joi.objectId()
+        }),
+        body: Joi.object({
+          user_id: Joi.objectId()
+        })
+      }),
+      AuthorizationMiddleware.ensureUserCanAdminProject,
+      CollaboratorsController.transferOwnership
+    )
+
     // invites
     webRouter.post(
       '/project/:Project_id/invite',

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

@@ -0,0 +1,126 @@
+const logger = require('logger-sharelatex')
+const { Project } = require('../../models/Project')
+const ProjectGetter = require('../Project/ProjectGetter')
+const UserGetter = require('../User/UserGetter')
+const CollaboratorsHandler = require('./CollaboratorsHandler')
+const EmailHandler = require('../Email/EmailHandler')
+const Errors = require('../Errors/Errors')
+const PrivilegeLevels = require('../Authorization/PrivilegeLevels')
+const ProjectEntityHandler = require('../Project/ProjectEntityHandler')
+const ProjectAuditLogHandler = require('../Project/ProjectAuditLogHandler')
+
+module.exports = {
+  promises: { transferOwnership }
+}
+
+async function transferOwnership(projectId, newOwnerId, options = {}) {
+  const { allowTransferToNonCollaborators, sessionUserId } = options
+
+  // Fetch project and user
+  const [project, newOwner] = await Promise.all([
+    _getProject(projectId),
+    _getUser(newOwnerId)
+  ])
+
+  // Exit early if the transferee is already the project owner
+  const previousOwnerId = project.owner_ref
+  if (previousOwnerId.equals(newOwnerId)) {
+    return
+  }
+
+  // Check that user is already a collaborator
+  if (
+    !allowTransferToNonCollaborators &&
+    !_userIsCollaborator(newOwner, project)
+  ) {
+    throw new Errors.UserNotCollaboratorError({ info: { userId: newOwnerId } })
+  }
+
+  // Transfer ownership
+  await ProjectAuditLogHandler.promises.addEntry(
+    projectId,
+    'transfer-ownership',
+    sessionUserId,
+    { previousOwnerId, newOwnerId }
+  )
+  await _transferOwnership(projectId, previousOwnerId, newOwnerId)
+
+  // Flush project to TPDS
+  await ProjectEntityHandler.promises.flushProjectToThirdPartyDataStore(
+    projectId
+  )
+
+  // Send confirmation emails
+  const previousOwner = await UserGetter.promises.getUser(previousOwnerId)
+  await _sendEmails(project, previousOwner, newOwner)
+}
+
+async function _getProject(projectId) {
+  const project = await ProjectGetter.promises.getProject(projectId, {
+    owner_ref: 1,
+    collaberator_refs: 1,
+    name: 1
+  })
+  if (project == null) {
+    throw new Errors.ProjectNotFoundError({ info: { projectId } })
+  }
+  return project
+}
+
+async function _getUser(userId) {
+  const user = await UserGetter.promises.getUser(userId)
+  if (user == null) {
+    throw new Errors.UserNotFoundError({ info: { userId } })
+  }
+  return user
+}
+
+function _userIsCollaborator(user, project) {
+  const collaboratorIds = project.collaberator_refs || []
+  return collaboratorIds.some(collaboratorId => collaboratorId.equals(user._id))
+}
+
+async function _transferOwnership(projectId, previousOwnerId, newOwnerId) {
+  await CollaboratorsHandler.promises.removeUserFromProject(
+    projectId,
+    newOwnerId
+  )
+  await Project.update(
+    { _id: projectId },
+    { $set: { owner_ref: newOwnerId } }
+  ).exec()
+  await CollaboratorsHandler.promises.addUserIdToProject(
+    projectId,
+    newOwnerId,
+    previousOwnerId,
+    PrivilegeLevels.READ_AND_WRITE
+  )
+}
+
+async function _sendEmails(project, previousOwner, newOwner) {
+  if (previousOwner == null) {
+    // The previous owner didn't exist. This is not supposed to happen, but
+    // since we're changing the owner anyway, we'll just warn
+    logger.warn(
+      { projectId: project._id, ownerId: previousOwner._id },
+      'Project owner did not exist before ownership transfer'
+    )
+  } else {
+    // Send confirmation emails
+    await Promise.all([
+      EmailHandler.promises.sendEmail(
+        'ownershipTransferConfirmationPreviousOwner',
+        {
+          to: previousOwner.email,
+          project,
+          newOwner
+        }
+      ),
+      EmailHandler.promises.sendEmail('ownershipTransferConfirmationNewOwner', {
+        to: newOwner.email,
+        project,
+        previousOwner
+      })
+    ])
+  }
+}

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

@@ -0,0 +1,43 @@
+const OError = require('@overleaf/o-error')
+const { Project } = require('../../models/Project')
+
+const MAX_AUDIT_LOG_ENTRIES = 200
+
+module.exports = {
+  promises: {
+    addEntry
+  }
+}
+
+/**
+ * Add an audit log entry
+ *
+ * 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
+ */
+async function addEntry(projectId, operation, initiatorId, info = {}) {
+  const timestamp = new Date()
+  const entry = {
+    operation,
+    initiatorId,
+    timestamp,
+    info
+  }
+  const result = await Project.updateOne(
+    { _id: projectId },
+    {
+      $push: {
+        auditLog: { $each: [entry], $slice: -MAX_AUDIT_LOG_ENTRIES }
+      }
+    }
+  ).exec()
+  if (result.nModified === 0) {
+    throw new OError({
+      message: 'project not found',
+      info: { projectId }
+    })
+  }
+}

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

@@ -3,13 +3,9 @@ const fs = require('fs')
 const crypto = require('crypto')
 const async = require('async')
 const logger = require('logger-sharelatex')
-const HttpErrors = require('@overleaf/o-error/http')
-const { ObjectId } = require('mongodb')
-const Errors = require('../Errors/Errors')
 const ProjectDeleter = require('./ProjectDeleter')
 const ProjectDuplicator = require('./ProjectDuplicator')
 const ProjectCreationHandler = require('./ProjectCreationHandler')
-const ProjectDetailsHandler = require('./ProjectDetailsHandler')
 const EditorController = require('../Editor/EditorController')
 const ProjectHelper = require('./ProjectHelper')
 const metrics = require('metrics-sharelatex')
@@ -813,75 +809,6 @@ const ProjectController = {
     )
   },
 
-  transferOwnership(req, res, next) {
-    const sessionUser = AuthenticationController.getSessionUser(req)
-    if (req.body.user_id == null) {
-      return next(
-        new HttpErrors.BadRequestError({
-          info: { public: { message: 'Missing parameter: user_id' } }
-        })
-      )
-    }
-    let projectId
-    try {
-      projectId = ObjectId(req.params.Project_id)
-    } catch (err) {
-      return next(
-        new HttpErrors.BadRequestError({
-          info: {
-            public: { message: `Invalid project id: ${req.params.Project_id}` }
-          }
-        })
-      )
-    }
-    let toUserId
-    try {
-      toUserId = ObjectId(req.body.user_id)
-    } catch (err) {
-      return next(
-        new HttpErrors.BadRequestError({
-          info: { public: { message: `Invalid user id: ${req.body.user_id}` } }
-        })
-      )
-    }
-    ProjectDetailsHandler.transferOwnership(
-      projectId,
-      toUserId,
-      { allowTransferToNonCollaborators: sessionUser.isAdmin },
-      err => {
-        if (err != null) {
-          if (err instanceof Errors.ProjectNotFoundError) {
-            next(
-              new HttpErrors.NotFoundError({
-                info: { public: { message: `project not found: ${projectId}` } }
-              })
-            )
-          } else if (err instanceof Errors.UserNotFoundError) {
-            next(
-              new HttpErrors.NotFoundError({
-                info: { public: { message: `user not found: ${toUserId}` } }
-              })
-            )
-          } else if (err instanceof Errors.UserNotCollaboratorError) {
-            next(
-              new HttpErrors.ForbiddenError({
-                info: {
-                  public: {
-                    message: `user ${toUserId} should be a collaborator in project ${projectId} prior to ownership transfer`
-                  }
-                }
-              })
-            )
-          } else {
-            next(err)
-          }
-        } else {
-          res.sendStatus(204)
-        }
-      }
-    )
-  },
-
   _buildProjectList(allProjects, userId, v1Projects) {
     let project
     if (v1Projects == null) {

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

@@ -7,11 +7,7 @@ const TpdsUpdateSender = require('../ThirdPartyDataStore/TpdsUpdateSender')
 const PublicAccessLevels = require('../Authorization/PublicAccessLevels')
 const Errors = require('../Errors/Errors')
 const ProjectTokenGenerator = require('./ProjectTokenGenerator')
-const ProjectEntityHandler = require('./ProjectEntityHandler')
 const ProjectHelper = require('./ProjectHelper')
-const CollaboratorsHandler = require('../Collaborators/CollaboratorsHandler')
-const PrivilegeLevels = require('../Authorization/PrivilegeLevels')
-const EmailHandler = require('../Email/EmailHandler')
 const settings = require('settings-sharelatex')
 const { callbackify } = require('util')
 
@@ -22,7 +18,6 @@ module.exports = {
   getDetails: callbackify(getDetails),
   getProjectDescription: callbackify(getProjectDescription),
   setProjectDescription: callbackify(setProjectDescription),
-  transferOwnership: callbackify(transferOwnership),
   renameProject: callbackify(renameProject),
   validateProjectName: callbackify(validateProjectName),
   generateUniqueName: callbackify(generateUniqueName),
@@ -34,7 +29,6 @@ module.exports = {
     getDetails,
     getProjectDescription,
     setProjectDescription,
-    transferOwnership,
     renameProject,
     validateProjectName,
     generateUniqueName,
@@ -103,87 +97,6 @@ async function setProjectDescription(projectId, description) {
     throw err
   }
 }
-
-async function transferOwnership(projectId, toUserId, options = {}) {
-  // Fetch project and user
-  const [project, toUser] = await Promise.all([
-    ProjectGetter.promises.getProject(projectId, {
-      owner_ref: 1,
-      collaberator_refs: 1,
-      name: 1
-    }),
-    UserGetter.promises.getUser(toUserId)
-  ])
-  if (project == null) {
-    throw new Errors.ProjectNotFoundError({ info: { projectId: projectId } })
-  }
-  if (toUser == null) {
-    throw new Errors.UserNotFoundError({ info: { userId: toUserId } })
-  }
-
-  // Exit early if the transferee is already the project owner
-  const fromUserId = project.owner_ref
-  if (fromUserId.equals(toUserId)) {
-    return
-  }
-
-  // Check permissions
-  const collaboratorIds = project.collaberator_refs || []
-  if (
-    !options.allowTransferToNonCollaborators &&
-    !collaboratorIds.some(collaboratorId => collaboratorId.equals(toUser._id))
-  ) {
-    throw new Errors.UserNotCollaboratorError({ info: { userId: toUserId } })
-  }
-
-  // Fetch the current project owner
-  const fromUser = await UserGetter.promises.getUser(fromUserId)
-
-  // Transfer ownership
-  await CollaboratorsHandler.promises.removeUserFromProject(projectId, toUserId)
-  await Project.update(
-    { _id: projectId },
-    { $set: { owner_ref: toUserId } }
-  ).exec()
-  await CollaboratorsHandler.promises.addUserIdToProject(
-    projectId,
-    toUserId,
-    fromUserId,
-    PrivilegeLevels.READ_AND_WRITE
-  )
-
-  // Flush project to TPDS
-  await ProjectEntityHandler.promises.flushProjectToThirdPartyDataStore(
-    projectId
-  )
-
-  if (fromUser == null) {
-    // The previous owner didn't exist. This is not supposed to happen, but
-    // since we're changing the owner anyway, we'll just warn
-    logger.warn(
-      { projectId, ownerId: fromUserId },
-      'Project owner did not exist before ownership transfer'
-    )
-  } else {
-    // Send confirmation emails
-    await Promise.all([
-      EmailHandler.promises.sendEmail(
-        'ownershipTransferConfirmationPreviousOwner',
-        {
-          to: fromUser.email,
-          project,
-          newOwner: toUser
-        }
-      ),
-      EmailHandler.promises.sendEmail('ownershipTransferConfirmationNewOwner', {
-        to: toUser.email,
-        project,
-        previousOwner: fromUser
-      })
-    ])
-  }
-}
-
 async function renameProject(projectId, newName) {
   await validateProjectName(newName)
   logger.log({ projectId, newName }, 'renaming project')

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

@@ -39,6 +39,14 @@ const DeletedFileSchema = new Schema({
   deletedAt: { type: Date }
 })
 
+const AuditLogEntrySchema = new Schema({
+  _id: false,
+  operation: { type: String },
+  initiatorId: { type: Schema.Types.ObjectId },
+  timestamp: { type: Date },
+  info: { type: Object }
+})
+
 const ProjectSchema = new Schema({
   name: { type: String, default: 'new project' },
   lastUpdated: {
@@ -120,7 +128,8 @@ const ProjectSchema = new Schema({
         }
       }
     }
-  ]
+  ],
+  auditLog: [AuditLogEntrySchema]
 })
 
 ProjectSchema.statics.getProject = function(project_or_id, fields, callback) {

services/web/app/src/router.js

@@ -496,11 +496,6 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
     AuthorizationMiddleware.ensureUserCanAdminProject,
     ProjectController.renameProject
   )
-  webRouter.post(
-    '/project/:Project_id/transfer-ownership',
-    AuthorizationMiddleware.ensureUserCanAdminProject,
-    ProjectController.transferOwnership
-  )
   webRouter.get(
     '/project/:Project_id/updates',
     AuthorizationMiddleware.ensureUserCanReadProject,

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

@@ -15,7 +15,7 @@ describe('CollaboratorsController', function() {
     this.res = new MockResponse()
     this.req = new MockRequest()
 
-    this.userId = ObjectId()
+    this.user = { _id: ObjectId() }
     this.projectId = ObjectId()
     this.callback = sinon.stub()
 
@@ -39,7 +39,13 @@ describe('CollaboratorsController', function() {
       }
     }
     this.AuthenticationController = {
-      getLoggedInUserId: sinon.stub().returns(this.userId)
+      getSessionUser: sinon.stub().returns(this.user),
+      getLoggedInUserId: sinon.stub().returns(this.user._id)
+    }
+    this.OwnershipTransferHandler = {
+      promises: {
+        transferOwnership: sinon.stub().resolves()
+      }
     }
     this.logger = {
       err: sinon.stub(),
@@ -54,6 +60,7 @@ describe('CollaboratorsController', function() {
       requires: {
         './CollaboratorsHandler': this.CollaboratorsHandler,
         './CollaboratorsGetter': this.CollaboratorsGetter,
+        './OwnershipTransferHandler': this.OwnershipTransferHandler,
         '../Editor/EditorRealTimeController': this.EditorRealTimeController,
         '../Tags/TagsHandler': this.TagsHandler,
         '../Authentication/AuthenticationController': this
@@ -69,7 +76,7 @@ describe('CollaboratorsController', function() {
     beforeEach(function(done) {
       this.req.params = {
         Project_id: this.projectId,
-        user_id: this.userId
+        user_id: this.user._id
       }
       this.res.sendStatus = sinon.spy(() => {
         done()
@@ -80,14 +87,14 @@ describe('CollaboratorsController', function() {
     it('should from the user from the project', function() {
       expect(
         this.CollaboratorsHandler.promises.removeUserFromProject
-      ).to.have.been.calledWith(this.projectId, this.userId)
+      ).to.have.been.calledWith(this.projectId, this.user._id)
     })
 
     it('should emit a userRemovedFromProject event to the proejct', function() {
       expect(this.EditorRealTimeController.emitToRoom).to.have.been.calledWith(
         this.projectId,
         'userRemovedFromProject',
-        this.userId
+        this.user._id
       )
     })
 
@@ -115,21 +122,21 @@ describe('CollaboratorsController', function() {
     it('should remove the logged in user from the project', function() {
       expect(
         this.CollaboratorsHandler.promises.removeUserFromProject
-      ).to.have.been.calledWith(this.projectId, this.userId)
+      ).to.have.been.calledWith(this.projectId, this.user._id)
     })
 
     it('should emit a userRemovedFromProject event to the proejct', function() {
       expect(this.EditorRealTimeController.emitToRoom).to.have.been.calledWith(
         this.projectId,
         'userRemovedFromProject',
-        this.userId
+        this.user._id
       )
     })
 
     it('should remove the project from all tags', function() {
       expect(
         this.TagsHandler.promises.removeProjectFromAllTags
-      ).to.have.been.calledWith(this.userId, this.projectId)
+      ).to.have.been.calledWith(this.user._id, this.projectId)
     })
 
     it('should return a success code', function() {
@@ -198,7 +205,7 @@ describe('CollaboratorsController', function() {
     beforeEach(function() {
       this.req.params = {
         Project_id: this.projectId,
-        user_id: this.userId
+        user_id: this.user._id
       }
       this.req.body = { privilegeLevel: 'readOnly' }
     })
@@ -208,7 +215,7 @@ describe('CollaboratorsController', function() {
         expect(status).to.equal(204)
         expect(
           this.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel
-        ).to.have.been.calledWith(this.projectId, this.userId, 'readOnly')
+        ).to.have.been.calledWith(this.projectId, this.user._id, 'readOnly')
         done()
       }
       this.CollaboratorsController.setCollaboratorInfo(this.req, this.res)
@@ -228,4 +235,46 @@ describe('CollaboratorsController', function() {
       )
     })
   })
+
+  describe('transferOwnership', function() {
+    beforeEach(function() {
+      this.req.body = { user_id: this.user._id.toString() }
+    })
+
+    it('returns 204 on success', function(done) {
+      this.res.sendStatus = status => {
+        expect(status).to.equal(204)
+        done()
+      }
+      this.CollaboratorsController.transferOwnership(this.req, this.res)
+    })
+
+    it('returns 404 if the project does not exist', function(done) {
+      this.OwnershipTransferHandler.promises.transferOwnership.rejects(
+        new Errors.ProjectNotFoundError()
+      )
+      this.CollaboratorsController.transferOwnership(
+        this.req,
+        this.res,
+        err => {
+          expect(err).to.be.instanceof(HttpErrors.NotFoundError)
+          done()
+        }
+      )
+    })
+
+    it('returns 404 if the user does not exist', function(done) {
+      this.OwnershipTransferHandler.promises.transferOwnership.rejects(
+        new Errors.UserNotFoundError()
+      )
+      this.CollaboratorsController.transferOwnership(
+        this.req,
+        this.res,
+        err => {
+          expect(err).to.be.instanceof(HttpErrors.NotFoundError)
+          done()
+        }
+      )
+    })
+  })
 })

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

@@ -0,0 +1,234 @@
+const SandboxedModule = require('sandboxed-module')
+const sinon = require('sinon')
+const { expect } = require('chai')
+const PrivilegeLevels = require('../../../../app/src/Features/Authorization/PrivilegeLevels')
+const Errors = require('../../../../app/src/Features/Errors/Errors')
+const { ObjectId } = require('mongodb')
+
+const MODULE_PATH =
+  '../../../../app/src/Features/Collaborators/OwnershipTransferHandler'
+
+describe('OwnershipTransferHandler', function() {
+  beforeEach(function() {
+    this.user = { _id: ObjectId(), email: 'owner@example.com' }
+    this.collaborator = { _id: ObjectId(), email: 'collaborator@example.com' }
+    this.project = {
+      _id: ObjectId(),
+      name: 'project',
+      owner_ref: this.user._id,
+      collaberator_refs: [this.collaborator._id]
+    }
+    this.ProjectGetter = {
+      promises: {
+        getProject: sinon.stub().resolves(this.project)
+      }
+    }
+    this.ProjectModel = {
+      update: sinon.stub().returns({
+        exec: sinon.stub().resolves()
+      })
+    }
+    this.UserGetter = {
+      promises: {
+        getUser: sinon.stub().resolves(this.user)
+      }
+    }
+    this.TpdsUpdateSender = {
+      promises: {
+        moveEntity: sinon.stub().resolves()
+      }
+    }
+    this.ProjectEntityHandler = {
+      promises: {
+        flushProjectToThirdPartyDataStore: sinon.stub().resolves()
+      }
+    }
+    this.CollaboratorsHandler = {
+      promises: {
+        removeUserFromProject: sinon.stub().resolves(),
+        addUserIdToProject: sinon.stub().resolves()
+      }
+    }
+    this.EmailHandler = {
+      promises: {
+        sendEmail: sinon.stub().resolves()
+      }
+    }
+    this.ProjectAuditLogHandler = {
+      promises: {
+        addEntry: sinon.stub().resolves()
+      }
+    }
+    this.handler = SandboxedModule.require(MODULE_PATH, {
+      globals: {
+        console: console
+      },
+      requires: {
+        '../Project/ProjectGetter': this.ProjectGetter,
+        '../../models/Project': {
+          Project: this.ProjectModel
+        },
+        '../User/UserGetter': this.UserGetter,
+        '../Project/ProjectEntityHandler': this.ProjectEntityHandler,
+        '../Project/ProjectAuditLogHandler': this.ProjectAuditLogHandler,
+        '../Email/EmailHandler': this.EmailHandler,
+        './CollaboratorsHandler': this.CollaboratorsHandler,
+        'logger-sharelatex': {
+          log() {},
+          warn() {},
+          err() {}
+        },
+        '../Errors/Errors': Errors
+      }
+    })
+  })
+
+  describe('transferOwnership', function() {
+    beforeEach(function() {
+      this.UserGetter.promises.getUser
+        .withArgs(this.user._id)
+        .resolves(this.user)
+      this.UserGetter.promises.getUser
+        .withArgs(this.collaborator._id)
+        .resolves(this.collaborator)
+    })
+
+    it("should return a not found error if the project can't be found", async function() {
+      this.ProjectGetter.promises.getProject.resolves(null)
+      await expect(
+        this.handler.promises.transferOwnership('abc', this.collaborator._id)
+      ).to.be.rejectedWith(Errors.ProjectNotFoundError)
+    })
+
+    it("should return a not found error if the user can't be found", async function() {
+      this.UserGetter.promises.getUser
+        .withArgs(this.collaborator._id)
+        .resolves(null)
+      await expect(
+        this.handler.promises.transferOwnership(
+          this.project._id,
+          this.collaborator._id
+        )
+      ).to.be.rejectedWith(Errors.UserNotFoundError)
+    })
+
+    it('should return an error if user cannot be removed as collaborator ', async function() {
+      this.CollaboratorsHandler.promises.removeUserFromProject.rejects(
+        new Error('user-cannot-be-removed')
+      )
+      await expect(
+        this.handler.promises.transferOwnership(
+          this.project._id,
+          this.collaborator._id
+        )
+      ).to.be.rejected
+    })
+
+    it('should transfer ownership of the project', async function() {
+      await this.handler.promises.transferOwnership(
+        this.project._id,
+        this.collaborator._id
+      )
+      expect(this.ProjectModel.update).to.have.been.calledWith(
+        { _id: this.project._id },
+        sinon.match({ $set: { owner_ref: this.collaborator._id } })
+      )
+    })
+
+    it('should do nothing if transferring back to the owner', async function() {
+      await this.handler.promises.transferOwnership(
+        this.project._id,
+        this.user._id
+      )
+      expect(this.ProjectModel.update).not.to.have.been.called
+    })
+
+    it("should remove the user from the project's collaborators", async function() {
+      await this.handler.promises.transferOwnership(
+        this.project._id,
+        this.collaborator._id
+      )
+      expect(
+        this.CollaboratorsHandler.promises.removeUserFromProject
+      ).to.have.been.calledWith(this.project._id, this.collaborator._id)
+    })
+
+    it('should add the former project owner as a read/write collaborator', async function() {
+      await this.handler.promises.transferOwnership(
+        this.project._id,
+        this.collaborator._id
+      )
+      expect(
+        this.CollaboratorsHandler.promises.addUserIdToProject
+      ).to.have.been.calledWith(
+        this.project._id,
+        this.collaborator._id,
+        this.user._id,
+        PrivilegeLevels.READ_AND_WRITE
+      )
+    })
+
+    it('should flush the project to tpds', async function() {
+      await this.handler.promises.transferOwnership(
+        this.project._id,
+        this.collaborator._id
+      )
+      expect(
+        this.ProjectEntityHandler.promises.flushProjectToThirdPartyDataStore
+      ).to.have.been.calledWith(this.project._id)
+    })
+
+    it('should send an email notification', async function() {
+      await this.handler.promises.transferOwnership(
+        this.project._id,
+        this.collaborator._id
+      )
+      expect(this.EmailHandler.promises.sendEmail).to.have.been.calledWith(
+        'ownershipTransferConfirmationPreviousOwner',
+        {
+          to: this.user.email,
+          project: this.project,
+          newOwner: this.collaborator
+        }
+      )
+      expect(this.EmailHandler.promises.sendEmail).to.have.been.calledWith(
+        'ownershipTransferConfirmationNewOwner',
+        {
+          to: this.collaborator.email,
+          project: this.project,
+          previousOwner: this.user
+        }
+      )
+    })
+
+    it('should write an entry in the audit log', async function() {
+      const sessionUserId = ObjectId()
+      await this.handler.promises.transferOwnership(
+        this.project._id,
+        this.collaborator._id,
+        { sessionUserId }
+      )
+      expect(
+        this.ProjectAuditLogHandler.promises.addEntry
+      ).to.have.been.calledWith(
+        this.project._id,
+        'transfer-ownership',
+        sessionUserId,
+        {
+          previousOwnerId: this.user._id,
+          newOwnerId: this.collaborator._id
+        }
+      )
+    })
+
+    it('should decline to transfer ownership to a non-collaborator', async function() {
+      this.project.collaberator_refs = []
+      await expect(
+        this.handler.promises.transferOwnership(
+          this.project._id,
+          this.collaborator._id
+        )
+      ).to.be.rejectedWith(Errors.UserNotCollaboratorError)
+    })
+  })
+})

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

@@ -0,0 +1,80 @@
+const sinon = require('sinon')
+const { expect } = require('chai')
+const { ObjectId } = require('mongodb')
+const SandboxedModule = require('sandboxed-module')
+const { Project } = require('../helpers/models/Project')
+
+const MODULE_PATH =
+  '../../../../app/src/Features/Project/ProjectAuditLogHandler'
+
+describe('ProjectAuditLogHandler', function() {
+  beforeEach(function() {
+    this.projectId = ObjectId()
+    this.userId = ObjectId()
+    this.ProjectMock = sinon.mock(Project)
+    this.ProjectAuditLogHandler = SandboxedModule.require(MODULE_PATH, {
+      requires: {
+        '../../models/Project': { Project }
+      }
+    })
+  })
+
+  afterEach(function() {
+    this.ProjectMock.restore()
+  })
+
+  describe('addEntry', function() {
+    describe('success', function() {
+      beforeEach(async function() {
+        this.dbUpdate = this.ProjectMock.expects('updateOne').withArgs(
+          { _id: this.projectId },
+          {
+            $push: {
+              auditLog: {
+                $each: [
+                  {
+                    operation: 'translate',
+                    initiatorId: this.userId,
+                    info: { destinationLanguage: 'tagalog' },
+                    timestamp: sinon.match.typeOf('date')
+                  }
+                ],
+                $slice: -200
+              }
+            }
+          }
+        )
+        this.dbUpdate.chain('exec').resolves({ nModified: 1 })
+        this.operationId = await this.ProjectAuditLogHandler.promises.addEntry(
+          this.projectId,
+          'translate',
+          this.userId,
+          { destinationLanguage: 'tagalog' }
+        )
+      })
+
+      it('writes a log', async function() {
+        this.ProjectMock.verify()
+      })
+    })
+
+    describe('when the project does not exist', function() {
+      beforeEach(function() {
+        this.ProjectMock.expects('updateOne')
+          .chain('exec')
+          .resolves({ nModified: 0 })
+      })
+
+      it('throws an error', async function() {
+        await expect(
+          this.ProjectAuditLogHandler.promises.addEntry(
+            this.projectId,
+            'translate',
+            this.userId,
+            { destinationLanguage: 'tagalog' }
+          )
+        ).to.be.rejected
+      })
+    })
+  })
+})

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

@@ -73,9 +73,6 @@ describe('ProjectController', function() {
       findAllUsersProjects: sinon.stub(),
       getProject: sinon.stub()
     }
-    this.ProjectDetailsHandler = {
-      transferOwnership: sinon.stub().yields()
-    }
     this.ProjectHelper = {
       isArchived: sinon.stub(),
       isTrashed: sinon.stub(),
@@ -1268,46 +1265,4 @@ describe('ProjectController', function() {
       ])
     })
   })
-
-  describe('transferOwnership', function() {
-    beforeEach(function() {
-      this.req.body = { user_id: this.user._id.toString() }
-    })
-
-    it('validates the request body', function(done) {
-      this.req.body = {}
-      this.ProjectController.transferOwnership(this.req, this.res, err => {
-        expect(err).to.be.instanceof(HttpErrors.BadRequestError)
-        done()
-      })
-    })
-
-    it('returns 204 on success', function(done) {
-      this.res.sendStatus = status => {
-        expect(status).to.equal(204)
-        done()
-      }
-      this.ProjectController.transferOwnership(this.req, this.res)
-    })
-
-    it('returns 404 if the project does not exist', function(done) {
-      this.ProjectDetailsHandler.transferOwnership.yields(
-        new Errors.ProjectNotFoundError()
-      )
-      this.ProjectController.transferOwnership(this.req, this.res, err => {
-        expect(err).to.be.instanceof(HttpErrors.NotFoundError)
-        done()
-      })
-    })
-
-    it('returns 404 if the user does not exist', function(done) {
-      this.ProjectDetailsHandler.transferOwnership.yields(
-        new Errors.UserNotFoundError()
-      )
-      this.ProjectController.transferOwnership(this.req, this.res, err => {
-        expect(err).to.be.instanceof(HttpErrors.NotFoundError)
-        done()
-      })
-    })
-  })
 })

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

@@ -3,23 +3,22 @@ const sinon = require('sinon')
 const { expect } = require('chai')
 const { ObjectId } = require('mongodb')
 const Errors = require('../../../../app/src/Features/Errors/Errors')
-const PrivilegeLevels = require('../../../../app/src/Features/Authorization/PrivilegeLevels')
 
 const MODULE_PATH = '../../../../app/src/Features/Project/ProjectDetailsHandler'
 
 describe('ProjectDetailsHandler', function() {
   beforeEach(function() {
     this.user = {
-      _id: ObjectId('abcdefabcdefabcdefabcdef'),
+      _id: ObjectId(),
       email: 'user@example.com',
       features: 'mock-features'
     }
     this.collaborator = {
-      _id: ObjectId('123456123456123456123456'),
+      _id: ObjectId(),
       email: 'collaborator@example.com'
     }
     this.project = {
-      _id: ObjectId('5d5dabdbb351de090cdff0b2'),
+      _id: ObjectId(),
       name: 'project',
       description: 'this is a great project',
       something: 'should not exist',
@@ -55,22 +54,6 @@ describe('ProjectDetailsHandler', function() {
         moveEntity: sinon.stub().resolves()
       }
     }
-    this.ProjectEntityHandler = {
-      promises: {
-        flushProjectToThirdPartyDataStore: sinon.stub().resolves()
-      }
-    }
-    this.CollaboratorsHandler = {
-      promises: {
-        removeUserFromProject: sinon.stub().resolves(),
-        addUserIdToProject: sinon.stub().resolves()
-      }
-    }
-    this.EmailHandler = {
-      promises: {
-        sendEmail: sinon.stub().resolves()
-      }
-    }
     this.ProjectTokenGenerator = {
       readAndWriteToken: sinon.stub(),
       promises: {
@@ -91,9 +74,6 @@ describe('ProjectDetailsHandler', function() {
         },
         '../User/UserGetter': this.UserGetter,
         '../ThirdPartyDataStore/TpdsUpdateSender': this.TpdsUpdateSender,
-        './ProjectEntityHandler': this.ProjectEntityHandler,
-        '../Collaborators/CollaboratorsHandler': this.CollaboratorsHandler,
-        '../Email/EmailHandler': this.EmailHandler,
         'logger-sharelatex': {
           log() {},
           warn() {},
@@ -143,135 +123,6 @@ describe('ProjectDetailsHandler', function() {
     })
   })
 
-  describe('transferOwnership', function() {
-    beforeEach(function() {
-      this.UserGetter.promises.getUser
-        .withArgs(this.user._id)
-        .resolves(this.user)
-      this.UserGetter.promises.getUser
-        .withArgs(this.collaborator._id)
-        .resolves(this.collaborator)
-    })
-
-    it("should return a not found error if the project can't be found", async function() {
-      this.ProjectGetter.promises.getProject.resolves(null)
-      await expect(
-        this.handler.promises.transferOwnership('abc', this.collaborator._id)
-      ).to.be.rejectedWith(Errors.ProjectNotFoundError)
-    })
-
-    it("should return a not found error if the user can't be found", async function() {
-      this.UserGetter.promises.getUser
-        .withArgs(this.collaborator._id)
-        .resolves(null)
-      await expect(
-        this.handler.promises.transferOwnership(
-          this.project._id,
-          this.collaborator._id
-        )
-      ).to.be.rejectedWith(Errors.UserNotFoundError)
-    })
-
-    it('should return an error if user cannot be removed as collaborator ', async function() {
-      this.CollaboratorsHandler.promises.removeUserFromProject.rejects(
-        new Error('user-cannot-be-removed')
-      )
-      await expect(
-        this.handler.promises.transferOwnership(
-          this.project._id,
-          this.collaborator._id
-        )
-      ).to.be.rejected
-    })
-
-    it('should transfer ownership of the project', async function() {
-      await this.handler.promises.transferOwnership(
-        this.project._id,
-        this.collaborator._id
-      )
-      expect(this.ProjectModel.update).to.have.been.calledWith(
-        { _id: this.project._id },
-        sinon.match({ $set: { owner_ref: this.collaborator._id } })
-      )
-    })
-
-    it('should do nothing if transferring back to the owner', async function() {
-      await this.handler.promises.transferOwnership(
-        this.project._id,
-        this.user._id
-      )
-      expect(this.ProjectModel.update).not.to.have.been.called
-    })
-
-    it("should remove the user from the project's collaborators", async function() {
-      await this.handler.promises.transferOwnership(
-        this.project._id,
-        this.collaborator._id
-      )
-      expect(
-        this.CollaboratorsHandler.promises.removeUserFromProject
-      ).to.have.been.calledWith(this.project._id, this.collaborator._id)
-    })
-
-    it('should add the former project owner as a read/write collaborator', async function() {
-      await this.handler.promises.transferOwnership(
-        this.project._id,
-        this.collaborator._id
-      )
-      expect(
-        this.CollaboratorsHandler.promises.addUserIdToProject
-      ).to.have.been.calledWith(
-        this.project._id,
-        this.collaborator._id,
-        this.user._id,
-        PrivilegeLevels.READ_AND_WRITE
-      )
-    })
-
-    it('should flush the project to tpds', async function() {
-      await this.handler.promises.transferOwnership(
-        this.project._id,
-        this.collaborator._id
-      )
-      expect(
-        this.ProjectEntityHandler.promises.flushProjectToThirdPartyDataStore
-      ).to.have.been.calledWith(this.project._id)
-    })
-
-    it('should send an email notification', async function() {
-      await this.handler.promises.transferOwnership(
-        this.project._id,
-        this.collaborator._id
-      )
-      expect(this.EmailHandler.promises.sendEmail).to.have.been.calledWith(
-        'ownershipTransferConfirmationPreviousOwner',
-        {
-          to: this.user.email,
-          project: this.project,
-          newOwner: this.collaborator
-        }
-      )
-      expect(this.EmailHandler.promises.sendEmail).to.have.been.calledWith(
-        'ownershipTransferConfirmationNewOwner',
-        {
-          to: this.collaborator.email,
-          project: this.project,
-          previousOwner: this.user
-        }
-      )
-    })
-
-    it('should decline to transfer ownership to a non-collaborator', async function() {
-      this.project.collaberator_refs = []
-      await expect(
-        this.handler.promises.transferOwnership(
-          this.project._id,
-          this.collaborator._id
-        )
-      ).to.be.rejectedWith(Errors.UserNotCollaboratorError)
-    })
-  })
-
   describe('getProjectDescription', function() {
     it('should make a call to mongo just for the description', async function() {
       this.ProjectGetter.promises.getProject.resolves()