Merge pull request #20306 from overleaf/rh-change-owner-viewer

[web] Allow project ownership to be transferred to Viewers

GitOrigin-RevId: 061d6e7b10c5c15212419e0736d0bfac39b65e96

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

@@ -52,7 +52,18 @@ async function transferOwnership(projectId, newOwnerId, options = {}) {
     '', // IP address
     { previousOwnerId, newOwnerId }
   )
-  await _transferOwnership(projectId, previousOwnerId, newOwnerId)
+
+  // Determine which permissions to give old owner based on
+  // new owner's existing permissions
+  const newPermissions =
+    _getUserPermissions(newOwner, project) || PrivilegeLevels.READ_ONLY
+
+  await _transferOwnership(
+    projectId,
+    previousOwnerId,
+    newOwnerId,
+    newPermissions
+  )
 
   // Flush project to TPDS
   await TpdsProjectFlusher.promises.flushProjectToTpds(projectId)
@@ -68,6 +79,7 @@ async function _getProject(projectId) {
   const project = await ProjectGetter.promises.getProject(projectId, {
     owner_ref: 1,
     collaberator_refs: 1,
+    readOnly_refs: 1,
     name: 1,
   })
   if (project == null) {
@@ -84,12 +96,28 @@ async function _getUser(userId) {
   return user
 }
 
-function _userIsCollaborator(user, project) {
+function _getUserPermissions(user, project) {
   const collaboratorIds = project.collaberator_refs || []
-  return collaboratorIds.some(collaboratorId => collaboratorId.equals(user._id))
+  const readOnlyIds = project.readOnly_refs || []
+  if (collaboratorIds.some(collaboratorId => collaboratorId.equals(user._id))) {
+    return PrivilegeLevels.READ_AND_WRITE
+  } else if (
+    readOnlyIds.some(collaboratorId => collaboratorId.equals(user._id))
+  ) {
+    return PrivilegeLevels.READ_ONLY
+  }
+}
+
+function _userIsCollaborator(user, project) {
+  return Boolean(_getUserPermissions(user, project))
 }
 
-async function _transferOwnership(projectId, previousOwnerId, newOwnerId) {
+async function _transferOwnership(
+  projectId,
+  previousOwnerId,
+  newOwnerId,
+  newPermissions
+) {
   await CollaboratorsHandler.promises.removeUserFromProject(
     projectId,
     newOwnerId
@@ -102,7 +130,7 @@ async function _transferOwnership(projectId, previousOwnerId, newOwnerId) {
     projectId,
     newOwnerId,
     previousOwnerId,
-    PrivilegeLevels.READ_AND_WRITE
+    newPermissions
   )
 }
 

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

@@ -15,11 +15,16 @@ describe('OwnershipTransferHandler', function () {
       _id: new ObjectId(),
       email: 'collaborator@example.com',
     }
+    this.readOnlyCollaborator = {
+      _id: new ObjectId(),
+      email: 'readonly@example.com',
+    }
     this.project = {
       _id: new ObjectId(),
       name: 'project',
       owner_ref: this.user._id,
       collaberator_refs: [this.collaborator._id],
+      readOnly_refs: [this.readOnlyCollaborator._id],
     }
     this.ProjectGetter = {
       promises: {
@@ -89,6 +94,9 @@ describe('OwnershipTransferHandler', function () {
       this.UserGetter.promises.getUser
         .withArgs(this.collaborator._id)
         .resolves(this.collaborator)
+      this.UserGetter.promises.getUser
+        .withArgs(this.readOnlyCollaborator._id)
+        .resolves(this.readOnlyCollaborator)
     })
 
     it("should return a not found error if the project can't be found", async function () {
@@ -133,6 +141,32 @@ describe('OwnershipTransferHandler', function () {
       )
     })
 
+    it('should transfer ownership of the project to a read-only collaborator', async function () {
+      await this.handler.promises.transferOwnership(
+        this.project._id,
+        this.readOnlyCollaborator._id
+      )
+      expect(this.ProjectModel.updateOne).to.have.been.calledWith(
+        { _id: this.project._id },
+        sinon.match({ $set: { owner_ref: this.readOnlyCollaborator._id } })
+      )
+    })
+
+    it('gives old owner read-only permissions if new owner was previously a viewer', async function () {
+      await this.handler.promises.transferOwnership(
+        this.project._id,
+        this.readOnlyCollaborator._id
+      )
+      expect(
+        this.CollaboratorsHandler.promises.addUserIdToProject
+      ).to.have.been.calledWith(
+        this.project._id,
+        this.readOnlyCollaborator._id,
+        this.user._id,
+        PrivilegeLevels.READ_ONLY
+      )
+    })
+
     it('should do nothing if transferring back to the owner', async function () {
       await this.handler.promises.transferOwnership(
         this.project._id,
@@ -248,6 +282,7 @@ describe('OwnershipTransferHandler', function () {
 
     it('should decline to transfer ownership to a non-collaborator', async function () {
       this.project.collaberator_refs = []
+      this.project.readOnly_refs = []
       await expect(
         this.handler.promises.transferOwnership(
           this.project._id,