diff --git a/services/web/app/src/Features/Collaborators/OwnershipTransferHandler.js b/services/web/app/src/Features/Collaborators/OwnershipTransferHandler.js index e4064db88f..b656bc6ce6 100644 --- a/services/web/app/src/Features/Collaborators/OwnershipTransferHandler.js +++ b/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 + } } -async function _transferOwnership(projectId, previousOwnerId, newOwnerId) { +function _userIsCollaborator(user, project) { + return Boolean(_getUserPermissions(user, project)) +} + +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 ) } diff --git a/services/web/test/unit/src/Collaborators/OwnershipTransferHandlerTests.js b/services/web/test/unit/src/Collaborators/OwnershipTransferHandlerTests.js index e61805565a..46dc7eba67 100644 --- a/services/web/test/unit/src/Collaborators/OwnershipTransferHandlerTests.js +++ b/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,