Merge pull request #20036 from overleaf/tm-collab-limit-link-sharing

Enforce collaborator limit for link sharing

GitOrigin-RevId: b724dca0c616ef15e5bd6d07e9d898d34dd46acd

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

@@ -230,7 +230,8 @@ async function transferProjects(fromUserId, toUserId) {
 async function setCollaboratorPrivilegeLevel(
   projectId,
   userId,
-  privilegeLevel
+  privilegeLevel,
+  { pendingEditor } = {}
 ) {
   // Make sure we're only updating the project if the user is already a
   // collaborator
@@ -249,9 +250,14 @@ async function setCollaboratorPrivilegeLevel(
     }
     case PrivilegeLevels.READ_ONLY: {
       update = {
-        $pull: { collaberator_refs: userId, pendingEditor_refs: userId },
+        $pull: { collaberator_refs: userId },
         $addToSet: { readOnly_refs: userId },
       }
+      if (pendingEditor) {
+        update.$addToSet.pendingEditor_refs = userId
+      } else {
+        update.$pull.pendingEditor_refs = userId
+      }
       break
     }
     default: {

services/web/app/src/Features/TokenAccess/TokenAccessController.js

@@ -22,6 +22,7 @@ const {
 const { getSafeAdminDomainRedirect } = require('../Helpers/UrlHelper')
 const UserGetter = require('../User/UserGetter')
 const Settings = require('@overleaf/settings')
+const LimitationsManager = require('../Subscription/LimitationsManager')
 
 const orderedPrivilegeLevels = [
   PrivilegeLevels.NONE,
@@ -333,27 +334,43 @@ async function grantTokenAccessReadAndWrite(req, res, next) {
         })
       }
 
+      const linkSharingEnforcement =
+        await SplitTestHandler.promises.getAssignmentForUser(
+          project.owner_ref,
+          'link-sharing-enforcement'
+        )
+      const pendingEditor =
+        linkSharingEnforcement?.variant === 'active' &&
+        !(await LimitationsManager.promises.canAcceptEditCollaboratorInvite(
+          project._id
+        ))
       await ProjectAuditLogHandler.promises.addEntry(
         project._id,
         'accept-via-link-sharing',
         userId,
         req.ip,
-        { privileges: 'readAndWrite' }
+        {
+          privileges: pendingEditor ? 'readOnly' : 'readAndWrite',
+          ...(pendingEditor && { pendingEditor: true }),
+        }
       )
       AnalyticsManager.recordEventForUserInBackground(
         userId,
         'project-joined',
         {
-          mode: 'read-write',
+          mode: pendingEditor ? 'read-only' : 'read-write',
           projectId: project._id.toString(),
+          ...(pendingEditor && { pendingEditor: true }),
         }
       )
-      // Currently does not enforce the collaborator limit (warning phase)
       await CollaboratorsHandler.promises.addUserIdToProject(
         project._id,
         undefined,
         userId,
-        PrivilegeLevels.READ_AND_WRITE
+        pendingEditor
+          ? PrivilegeLevels.READ_ONLY
+          : PrivilegeLevels.READ_AND_WRITE,
+        { pendingEditor }
       )
       // Does not remove any pending invite or the invite notification
       // Should be a noop if the user is already a member,
@@ -536,20 +553,36 @@ async function sharingUpdatesConsent(req, res, next) {
 async function moveReadWriteToCollaborators(req, res, next) {
   const { Project_id: projectId } = req.params
   const userId = SessionManager.getLoggedInUserId(req.session)
+  const project = await ProjectGetter.promises.getProject(projectId, {
+    owner_ref: 1,
+  })
   const isInvitedMember =
     await CollaboratorsGetter.promises.isUserInvitedMemberOfProject(
       userId,
       projectId
     )
+  const linkSharingEnforcement =
+    await SplitTestHandler.promises.getAssignmentForUser(
+      project.owner_ref,
+      'link-sharing-enforcement'
+    )
+  const pendingEditor =
+    linkSharingEnforcement?.variant === 'active' &&
+    !(await LimitationsManager.promises.canAcceptEditCollaboratorInvite(
+      project._id
+    ))
   await ProjectAuditLogHandler.promises.addEntry(
     projectId,
     'accept-via-link-sharing',
     userId,
     req.ip,
     {
-      privileges: 'readAndWrite',
+      privileges: pendingEditor
+        ? PrivilegeLevels.READ_ONLY
+        : PrivilegeLevels.READ_AND_WRITE,
       tokenMember: true,
       invitedMember: isInvitedMember,
+      ...(pendingEditor && { pendingEditor: true }),
     }
   )
   if (isInvitedMember) {
@@ -561,7 +594,10 @@ async function moveReadWriteToCollaborators(req, res, next) {
     await CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel(
       projectId,
       userId,
-      PrivilegeLevels.READ_AND_WRITE
+      pendingEditor
+        ? PrivilegeLevels.READ_ONLY
+        : PrivilegeLevels.READ_AND_WRITE,
+      { pendingEditor }
     )
   } else {
     // Normal case, not invited, joining via link sharing
@@ -573,7 +609,10 @@ async function moveReadWriteToCollaborators(req, res, next) {
       projectId,
       undefined,
       userId,
-      PrivilegeLevels.READ_AND_WRITE
+      pendingEditor
+        ? PrivilegeLevels.READ_ONLY
+        : PrivilegeLevels.READ_AND_WRITE,
+      { pendingEditor }
     )
   }
   EditorRealTimeController.emitToRoom(projectId, 'project:membership:changed', {

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

@@ -592,6 +592,36 @@ describe('CollaboratorsHandler', function () {
       )
     })
 
+    it('sets a collaborator to read-only as a pendingEditor', async function () {
+      this.ProjectMock.expects('updateOne')
+        .withArgs(
+          {
+            _id: this.projectId,
+            $or: [
+              { collaberator_refs: this.userId },
+              { readOnly_refs: this.userId },
+            ],
+          },
+          {
+            $addToSet: {
+              readOnly_refs: this.userId,
+              pendingEditor_refs: this.userId,
+            },
+            $pull: {
+              collaberator_refs: this.userId,
+            },
+          }
+        )
+        .chain('exec')
+        .resolves({ matchedCount: 1 })
+      await this.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel(
+        this.projectId,
+        this.userId,
+        'readOnly',
+        { pendingEditor: true }
+      )
+    })
+
     it('throws a NotFoundError if the project or collaborator does not exist', async function () {
       this.ProjectMock.expects('updateOne')
         .chain('exec')

services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js

@@ -123,6 +123,12 @@ describe('TokenAccessController', function () {
       },
     }
 
+    this.LimitationsManager = {
+      promises: {
+        canAcceptEditCollaboratorInvite: sinon.stub().resolves(),
+      },
+    }
+
     this.TokenAccessController = SandboxedModule.require(MODULE_PATH, {
       requires: {
         '@overleaf/settings': this.Settings,
@@ -145,6 +151,7 @@ describe('TokenAccessController', function () {
         }),
         '../Analytics/AnalyticsManager': this.AnalyticsManager,
         '../User/UserGetter': this.UserGetter,
+        '../Subscription/LimitationsManager': this.LimitationsManager,
       },
     })
   })
@@ -195,9 +202,13 @@ describe('TokenAccessController', function () {
 
     describe('when project owner in link-sharing-warning split test', function () {
       beforeEach(function () {
-        this.SplitTestHandler.promises.getAssignmentForUser.resolves({
-          variant: 'active',
-        })
+        this.SplitTestHandler.promises.getAssignmentForUser.callsFake(
+          async (userId, test) => {
+            if (test === 'link-sharing-warning') {
+              return { variant: 'active' }
+            }
+          }
+        )
       })
 
       it('tells the ui to show the link-sharing-warning variant', async function () {
@@ -284,6 +295,168 @@ describe('TokenAccessController', function () {
           )
         })
       })
+
+      describe('when the project owner is in the link-sharing-enforcement split test', function () {
+        beforeEach(function () {
+          this.SplitTestHandler.promises.getAssignmentForUser.callsFake(
+            async (userId, test) => {
+              if (test === 'link-sharing-warning') {
+                return { variant: 'active' }
+              } else if (test === 'link-sharing-enforcement') {
+                return { variant: 'active' }
+              }
+            }
+          )
+        })
+
+        describe('normal case (edit slot available)', function () {
+          beforeEach(function (done) {
+            this.LimitationsManager.promises.canAcceptEditCollaboratorInvite.resolves(
+              true
+            )
+            this.req.params = { token: this.token }
+            this.req.body = {
+              confirmedByUser: true,
+              tokenHashPrefix: '#prefix',
+            }
+            this.res.callback = done
+            this.TokenAccessController.grantTokenAccessReadAndWrite(
+              this.req,
+              this.res,
+              done
+            )
+          })
+
+          it('adds the user as a read and write invited member', function () {
+            expect(
+              this.CollaboratorsHandler.promises.addUserIdToProject
+            ).to.have.been.calledWith(
+              this.project._id,
+              undefined,
+              this.user._id,
+              PrivilegeLevels.READ_AND_WRITE
+            )
+          })
+
+          it('writes a project audit log', function () {
+            expect(
+              this.ProjectAuditLogHandler.promises.addEntry
+            ).to.have.been.calledWith(
+              this.project._id,
+              'accept-via-link-sharing',
+              this.user._id,
+              this.req.ip,
+              { privileges: 'readAndWrite' }
+            )
+          })
+
+          it('records a project-joined event for the user', function () {
+            expect(
+              this.AnalyticsManager.recordEventForUserInBackground
+            ).to.have.been.calledWith(this.user._id, 'project-joined', {
+              mode: 'read-write',
+              projectId: this.project._id.toString(),
+            })
+          })
+
+          it('emits a project membership changed event', function () {
+            expect(
+              this.EditorRealTimeController.emitToRoom
+            ).to.have.been.calledWith(
+              this.project._id,
+              'project:membership:changed',
+              { members: true }
+            )
+          })
+
+          it('checks token hash', function () {
+            expect(
+              this.TokenAccessHandler.checkTokenHashPrefix
+            ).to.have.been.calledWith(
+              this.token,
+              '#prefix',
+              'readAndWrite',
+              this.user._id,
+              { projectId: this.project._id, action: 'continue' }
+            )
+          })
+        })
+
+        describe('when there are no edit collaborator slots available', function () {
+          beforeEach(function (done) {
+            this.LimitationsManager.promises.canAcceptEditCollaboratorInvite.resolves(
+              false
+            )
+            this.req.params = { token: this.token }
+            this.req.body = {
+              confirmedByUser: true,
+              tokenHashPrefix: '#prefix',
+            }
+            this.res.callback = done
+            this.TokenAccessController.grantTokenAccessReadAndWrite(
+              this.req,
+              this.res,
+              done
+            )
+          })
+
+          it('adds the user as a read only invited member instead (pendingEditor)', function () {
+            expect(
+              this.CollaboratorsHandler.promises.addUserIdToProject
+            ).to.have.been.calledWith(
+              this.project._id,
+              undefined,
+              this.user._id,
+              PrivilegeLevels.READ_ONLY,
+              { pendingEditor: true }
+            )
+          })
+
+          it('writes a project audit log', function () {
+            expect(
+              this.ProjectAuditLogHandler.promises.addEntry
+            ).to.have.been.calledWith(
+              this.project._id,
+              'accept-via-link-sharing',
+              this.user._id,
+              this.req.ip,
+              { privileges: 'readOnly', pendingEditor: true }
+            )
+          })
+
+          it('records a project-joined event for the user', function () {
+            expect(
+              this.AnalyticsManager.recordEventForUserInBackground
+            ).to.have.been.calledWith(this.user._id, 'project-joined', {
+              mode: 'read-only',
+              projectId: this.project._id.toString(),
+              pendingEditor: true,
+            })
+          })
+
+          it('emits a project membership changed event', function () {
+            expect(
+              this.EditorRealTimeController.emitToRoom
+            ).to.have.been.calledWith(
+              this.project._id,
+              'project:membership:changed',
+              { members: true }
+            )
+          })
+
+          it('checks token hash', function () {
+            expect(
+              this.TokenAccessHandler.checkTokenHashPrefix
+            ).to.have.been.calledWith(
+              this.token,
+              '#prefix',
+              'readAndWrite',
+              this.user._id,
+              { projectId: this.project._id, action: 'continue' }
+            )
+          })
+        })
+      })
     })
 
     describe('when the access was already granted', function () {
@@ -994,6 +1167,89 @@ describe('TokenAccessController', function () {
         expect(this.res.sendStatus).to.have.been.calledWith(204)
       })
     })
+
+    describe('when link-sharing-enforcement test is active', function () {
+      beforeEach(function () {
+        this.SplitTestHandler.promises.getAssignmentForUser.resolves({
+          variant: 'active',
+        })
+      })
+
+      describe('when there are collaborator slots available', function () {
+        beforeEach(function () {
+          this.LimitationsManager.promises.canAcceptEditCollaboratorInvite.resolves(
+            true
+          )
+        })
+
+        describe('previously joined token access user moving to named collaborator', function () {
+          beforeEach(function (done) {
+            this.CollaboratorsGetter.promises.isUserInvitedMemberOfProject.resolves(
+              false
+            )
+            this.res.callback = done
+            this.TokenAccessController.moveReadWriteToCollaborators(
+              this.req,
+              this.res,
+              done
+            )
+          })
+
+          it('sets the privilege level to read and write for the invited viewer', function () {
+            expect(
+              this.TokenAccessHandler.promises.removeReadAndWriteUserFromProject
+            ).to.have.been.calledWith(this.user._id, this.project._id)
+            expect(
+              this.CollaboratorsHandler.promises.addUserIdToProject
+            ).to.have.been.calledWith(
+              this.project._id,
+              undefined,
+              this.user._id,
+              PrivilegeLevels.READ_AND_WRITE
+            )
+            expect(this.res.sendStatus).to.have.been.calledWith(204)
+          })
+        })
+      })
+
+      describe('when there are no edit collaborator slots available', function () {
+        beforeEach(function () {
+          this.LimitationsManager.promises.canAcceptEditCollaboratorInvite.resolves(
+            false
+          )
+        })
+
+        describe('previously joined token access user moving to named collaborator', function () {
+          beforeEach(function (done) {
+            this.CollaboratorsGetter.promises.isUserInvitedMemberOfProject.resolves(
+              false
+            )
+            this.res.callback = done
+            this.TokenAccessController.moveReadWriteToCollaborators(
+              this.req,
+              this.res,
+              done
+            )
+          })
+
+          it('sets the privilege level to read only for the invited viewer (pendingEditor)', function () {
+            expect(
+              this.TokenAccessHandler.promises.removeReadAndWriteUserFromProject
+            ).to.have.been.calledWith(this.user._id, this.project._id)
+            expect(
+              this.CollaboratorsHandler.promises.addUserIdToProject
+            ).to.have.been.calledWith(
+              this.project._id,
+              undefined,
+              this.user._id,
+              PrivilegeLevels.READ_ONLY,
+              { pendingEditor: true }
+            )
+            expect(this.res.sendStatus).to.have.been.calledWith(204)
+          })
+        })
+      })
+    })
   })
 
   describe('moveReadWriteToReadOnly', function () {