[web] Enforce collaborator limit (#19619)

* Enables collaborator limit enforcement

* Add pendingEditor_refs for editors downgraded during limit enforcement

* Add isPendingEditor to useEditorContext

---------

Co-authored-by: Thomas Mees <thomas.mees@overleaf.com>
GitOrigin-RevId: b622213f6282ccd8ee85a23ceb29b8c6f8ff6a96

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

@@ -40,6 +40,7 @@ module.exports = {
     getMemberIdPrivilegeLevel,
     getInvitedCollaboratorCount,
     getInvitedEditCollaboratorCount,
+    getInvitedPendingEditorCount,
     getProjectsUserIsMemberOf,
     dangerouslyGetAllProjectsUserIsMemberOf,
     isUserInvitedMemberOfProject,
@@ -59,6 +60,7 @@ async function getMemberIdsWithPrivilegeLevels(projectId) {
     tokenAccessReadOnly_refs: 1,
     tokenAccessReadAndWrite_refs: 1,
     publicAccesLevel: 1,
+    pendingEditor_refs: 1,
   })
   if (!project) {
     throw new Errors.NotFoundError(`no project found with id ${projectId}`)
@@ -69,7 +71,8 @@ async function getMemberIdsWithPrivilegeLevels(projectId) {
     project.readOnly_refs,
     project.tokenAccessReadAndWrite_refs,
     project.tokenAccessReadOnly_refs,
-    project.publicAccesLevel
+    project.publicAccesLevel,
+    project.pendingEditor_refs
   )
   return memberIds
 }
@@ -136,6 +139,17 @@ async function getInvitedEditCollaboratorCount(projectId) {
   ).length
 }
 
+async function getInvitedPendingEditorCount(projectId) {
+  // Only counts invited members that are readonly pending editors
+  const members = await getMemberIdsWithPrivilegeLevels(projectId)
+  return members.filter(
+    m =>
+      m.source === Sources.INVITE &&
+      m.privilegeLevel === PrivilegeLevels.READ_ONLY &&
+      m.pendingEditor === true
+  ).length
+}
+
 async function isUserInvitedMemberOfProject(userId, projectId) {
   if (!userId) {
     return false
@@ -309,7 +323,8 @@ function _getMemberIdsWithPrivilegeLevelsFromFields(
   readOnlyIds,
   tokenAccessIds,
   tokenAccessReadOnlyIds,
-  publicAccessLevel
+  publicAccessLevel,
+  pendingEditorIds
 ) {
   const members = []
   members.push({
@@ -329,6 +344,9 @@ function _getMemberIdsWithPrivilegeLevelsFromFields(
       id: memberId.toString(),
       privilegeLevel: PrivilegeLevels.READ_ONLY,
       source: Sources.INVITE,
+      ...(pendingEditorIds?.some(pe => memberId.equals(pe)) && {
+        pendingEditor: true,
+      }),
     })
   }
   if (publicAccessLevel === PublicAccessLevels.TOKEN_BASED) {
@@ -364,7 +382,11 @@ async function _loadMembers(members) {
           signUpDate: 1,
         })
         if (user != null) {
-          return { user, privilegeLevel: member.privilegeLevel }
+          return {
+            user,
+            privilegeLevel: member.privilegeLevel,
+            ...(member.pendingEditor && { pendingEditor: true }),
+          }
         } else {
           return null
         }

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

@@ -49,6 +49,7 @@ async function removeUserFromProject(projectId, userId) {
           $pull: {
             collaberator_refs: userId,
             readOnly_refs: userId,
+            pendingEditor_refs: userId,
             tokenAccessReadOnly_refs: userId,
             tokenAccessReadAndWrite_refs: userId,
             trashed: userId,
@@ -62,6 +63,7 @@ async function removeUserFromProject(projectId, userId) {
           $pull: {
             collaberator_refs: userId,
             readOnly_refs: userId,
+            pendingEditor_refs: userId,
             tokenAccessReadOnly_refs: userId,
             tokenAccessReadAndWrite_refs: userId,
             archived: userId,
@@ -196,6 +198,19 @@ async function transferProjects(fromUserId, toUserId) {
     }
   ).exec()
 
+  await Project.updateMany(
+    { pendingEditor_refs: fromUserId },
+    {
+      $addToSet: { pendingEditor_refs: toUserId },
+    }
+  ).exec()
+  await Project.updateMany(
+    { pendingEditor_refs: fromUserId },
+    {
+      $pull: { pendingEditor_refs: fromUserId },
+    }
+  ).exec()
+
   // Flush in background, no need to block on this
   _flushProjects(projectIds).catch(err => {
     logger.err(
@@ -220,14 +235,14 @@ async function setCollaboratorPrivilegeLevel(
   switch (privilegeLevel) {
     case PrivilegeLevels.READ_AND_WRITE: {
       update = {
-        $pull: { readOnly_refs: userId },
+        $pull: { readOnly_refs: userId, pendingEditor_refs: userId },
         $addToSet: { collaberator_refs: userId },
       }
       break
     }
     case PrivilegeLevels.READ_ONLY: {
       update = {
-        $pull: { collaberator_refs: userId },
+        $pull: { collaberator_refs: userId, pendingEditor_refs: userId },
         $addToSet: { readOnly_refs: userId },
       }
       break

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

@@ -415,6 +415,7 @@ const _ProjectController = {
           tokens: 1,
           tokenAccessReadAndWrite_refs: 1, // used for link sharing analytics
           collaberator_refs: 1, // used for link sharing analytics
+          pendingEditor_refs: 1, // used for link sharing analytics
         }),
         userIsMemberOfGroupSubscription: sessionUser
           ? (async () =>
@@ -488,12 +489,24 @@ const _ProjectController = {
           anonRequestToken
         )
 
-      const linkSharingChanges =
-        await SplitTestHandler.promises.getAssignmentForUser(
+      const [linkSharingChanges, linkSharingEnforcement] = await Promise.all([
+        SplitTestHandler.promises.getAssignmentForUser(
           project.owner_ref,
           'link-sharing-warning'
-        )
+        ),
+        SplitTestHandler.promises.getAssignmentForUser(
+          project.owner_ref,
+          'link-sharing-enforcement'
+        ),
+      ])
+
       if (linkSharingChanges?.variant === 'active') {
+        if (linkSharingEnforcement?.variant === 'active') {
+          await Modules.promises.hooks.fire(
+            'enforceCollaboratorLimit',
+            projectId
+          )
+        }
         if (isTokenMember) {
           // Check explicitly that the user is in read write token refs, while this could be inferred
           // from the privilege level, the privilege level of token members might later be restricted
@@ -570,12 +583,14 @@ const _ProjectController = {
         )
         const planLimit = ownerFeatures?.collaborators || 0
         const namedEditors = project.collaberator_refs?.length || 0
+        const pendingEditors = project.pendingEditor_refs?.length || 0
         const exceedAtLimit = planLimit > -1 && namedEditors >= planLimit
         const projectOpenedSegmentation = {
           projectId: project._id,
           // temporary link sharing segmentation:
           linkSharingWarning: linkSharingChanges?.variant,
           namedEditors,
+          pendingEditors,
           tokenEditors: project.tokenAccessReadAndWrite_refs?.length || 0,
           planLimit,
           exceedAtLimit,
@@ -1034,7 +1049,6 @@ const ProjectController = {
   ),
   updateProjectSettings: expressify(_ProjectController.updateProjectSettings),
   userProjectsJson: expressify(_ProjectController.userProjectsJson),
-
   _buildProjectList: _ProjectController._buildProjectList,
   _buildProjectViewModel: _ProjectController._buildProjectViewModel,
   _injectProjectUsers: _ProjectController._injectProjectUsers,

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

@@ -83,11 +83,9 @@ module.exports = ProjectEditorHandler = {
     for (const member of members || []) {
       if (member.privilegeLevel === 'owner') {
         ownerFeatures = member.user.features
-        owner = this.buildUserModelView(member.user, 'owner')
+        owner = this.buildUserModelView(member)
       } else {
-        filteredMembers.push(
-          this.buildUserModelView(member.user, member.privilegeLevel)
-        )
+        filteredMembers.push(this.buildUserModelView(member))
       }
     }
     return {
@@ -97,14 +95,16 @@ module.exports = ProjectEditorHandler = {
     }
   },
 
-  buildUserModelView(user, privileges) {
+  buildUserModelView(member) {
+    const user = member.user
     return {
       _id: user._id,
       first_name: user.first_name,
       last_name: user.last_name,
       email: user.email,
-      privileges,
+      privileges: member.privilegeLevel,
       signUpDate: user.signUpDate,
+      pendingEditor: member.pendingEditor,
     }
   },
 

services/web/app/src/Features/Subscription/LimitationsManager.js

@@ -30,14 +30,14 @@ async function allowedNumberOfCollaboratorsForUser(userId) {
 
 async function canAddXCollaborators(projectId, numberOfNewCollaborators) {
   const allowedNumber = await allowedNumberOfCollaboratorsInProject(projectId)
+  if (allowedNumber < 0) {
+    return true // -1 means unlimited
+  }
   const currentNumber =
     await CollaboratorsGetter.promises.getInvitedCollaboratorCount(projectId)
   const inviteCount =
     await CollaboratorsInvitesHandler.promises.getInviteCount(projectId)
-  return (
-    currentNumber + inviteCount + numberOfNewCollaborators <= allowedNumber ||
-    allowedNumber < 0 // -1 means unlimited
-  )
+  return currentNumber + inviteCount + numberOfNewCollaborators <= allowedNumber
 }
 
 async function canAddXEditCollaborators(
@@ -45,6 +45,9 @@ async function canAddXEditCollaborators(
   numberOfNewEditCollaborators
 ) {
   const allowedNumber = await allowedNumberOfCollaboratorsInProject(projectId)
+  if (allowedNumber < 0) {
+    return true // -1 means unlimited
+  }
   const currentEditors =
     await CollaboratorsGetter.promises.getInvitedEditCollaboratorCount(
       projectId
@@ -53,7 +56,7 @@ async function canAddXEditCollaborators(
     await CollaboratorsInvitesHandler.promises.getEditInviteCount(projectId)
   return (
     currentEditors + editInviteCount + numberOfNewEditCollaborators <=
-      allowedNumber || allowedNumber < 0 // -1 means unlimited
+    allowedNumber
   )
 }
 

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

@@ -39,6 +39,7 @@ const ProjectSchema = new Schema(
     owner_ref: { type: ObjectId, ref: 'User' },
     collaberator_refs: [{ type: ObjectId, ref: 'User' }],
     readOnly_refs: [{ type: ObjectId, ref: 'User' }],
+    pendingEditor_refs: [{ type: ObjectId, ref: 'User' }],
     rootDoc_id: { type: ObjectId },
     rootFolder: [FolderSchema],
     version: { type: Number }, // incremented for every change in the project structure (folders and filenames)

services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/edit-member.tsx

@@ -124,9 +124,17 @@ export default function EditMember({
       <FormGroup className="project-member">
         <Col xs={7}>
           <div className="project-member-email-icon">
-            <Icon type={shouldWarnMember() ? 'warning' : 'user'} fw />
+            <Icon
+              type={
+                shouldWarnMember() || member.pendingEditor ? 'warning' : 'user'
+              }
+              fw
+            />
             <div className="email-warning">
               {member.email}
+              {member.pendingEditor && (
+                <div className="subtitle">Pending editor</div>
+              )}
               {shouldWarnMember() && (
                 <div className="subtitle">
                   {t('will_lose_edit_access_on_date', {

services/web/frontend/js/shared/context/editor-context.tsx

@@ -44,6 +44,7 @@ export const EditorContext = createContext<
       insertSymbol?: (symbol: string) => void
       isProjectOwner: boolean
       isRestrictedTokenMember?: boolean
+      isPendingEditor: boolean
       permissionsLevel: 'readOnly' | 'readAndWrite' | 'owner'
       deactivateTutorial: (tutorial: string) => void
       inactiveTutorials: string[]
@@ -62,7 +63,7 @@ export const EditorProvider: FC = ({ children }) => {
   const { role } = useDetachContext()
   const { showGenericMessageModal } = useModalsContext()
 
-  const { owner, features, _id: projectId } = useProjectContext()
+  const { owner, features, _id: projectId, members } = useProjectContext()
 
   const cobranding = useMemo(() => {
     const brandVariation = getMeta('ol-brandVariation')
@@ -98,6 +99,17 @@ export const EditorProvider: FC = ({ children }) => {
 
   const [currentPopup, setCurrentPopup] = useState<string | null>(null)
 
+  const isPendingEditor = useMemo(
+    () =>
+      members?.some(
+        member =>
+          member._id === userId &&
+          member.pendingEditor &&
+          member.privileges === 'readAndWrite'
+      ),
+    [members, userId]
+  )
+
   const deactivateTutorial = useCallback(
     tutorialKey => {
       setInactiveTutorials([...inactiveTutorials, tutorialKey])
@@ -174,6 +186,7 @@ export const EditorProvider: FC = ({ children }) => {
       setPermissionsLevel,
       isProjectOwner: owner?._id === userId,
       isRestrictedTokenMember: getMeta('ol-isRestrictedTokenMember'),
+      isPendingEditor,
       showSymbolPalette,
       toggleSymbolPalette,
       insertSymbol,
@@ -194,6 +207,7 @@ export const EditorProvider: FC = ({ children }) => {
       renameProject,
       permissionsLevel,
       setPermissionsLevel,
+      isPendingEditor,
       showSymbolPalette,
       toggleSymbolPalette,
       insertSymbol,

services/web/frontend/js/shared/context/types/project-context.tsx

@@ -7,6 +7,7 @@ export type ProjectContextMember = {
   email: string
   first_name: string
   last_name: string
+  pendingEditor?: boolean
 }
 
 export type ProjectContextValue = {

services/web/test/acceptance/src/helpers/User.js

@@ -946,6 +946,10 @@ class User {
       updateOp = { $addToSet: { collaberator_refs: user._id } }
     } else if (privileges === 'readOnly') {
       updateOp = { $addToSet: { readOnly_refs: user._id } }
+    } else if (privileges === 'pendingEditor') {
+      updateOp = {
+        $addToSet: { readOnly_refs: user._id, pendingEditor_refs: user._id },
+      }
     }
     db.projects.updateOne({ _id: new ObjectId(projectId) }, updateOp, callback)
   }

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

@@ -17,6 +17,7 @@ describe('CollaboratorsGetter', function () {
     this.ownerRef = new ObjectId()
     this.readOnlyRef1 = new ObjectId()
     this.readOnlyRef2 = new ObjectId()
+    this.pendingEditorRef = new ObjectId()
     this.readWriteRef1 = new ObjectId()
     this.readWriteRef2 = new ObjectId()
     this.readOnlyTokenRef = new ObjectId()
@@ -25,7 +26,12 @@ describe('CollaboratorsGetter', function () {
     this.project = {
       _id: new ObjectId(),
       owner_ref: [this.ownerRef],
-      readOnly_refs: [this.readOnlyRef1, this.readOnlyRef2],
+      readOnly_refs: [
+        this.readOnlyRef1,
+        this.readOnlyRef2,
+        this.pendingEditorRef,
+      ],
+      pendingEditor_refs: [this.pendingEditorRef],
       collaberator_refs: [this.readWriteRef1, this.readWriteRef2],
       tokenAccessReadAndWrite_refs: [this.readWriteTokenRef],
       tokenAccessReadOnly_refs: [this.readOnlyTokenRef],
@@ -99,6 +105,12 @@ describe('CollaboratorsGetter', function () {
             privilegeLevel: 'readOnly',
             source: 'invite',
           },
+          {
+            id: this.pendingEditorRef.toString(),
+            privilegeLevel: 'readOnly',
+            source: 'invite',
+            pendingEditor: true,
+          },
           {
             id: this.readOnlyTokenRef.toString(),
             privilegeLevel: 'readOnly',
@@ -139,6 +151,7 @@ describe('CollaboratorsGetter', function () {
         this.readOnlyRef2.toString(),
         this.readWriteRef1.toString(),
         this.readWriteRef2.toString(),
+        this.pendingEditorRef.toString(),
         this.readWriteTokenRef.toString(),
         this.readOnlyTokenRef.toString(),
       ])
@@ -157,6 +170,7 @@ describe('CollaboratorsGetter', function () {
         this.readOnlyRef2.toString(),
         this.readWriteRef1.toString(),
         this.readWriteRef2.toString(),
+        this.pendingEditorRef.toString(),
       ])
     })
   })
@@ -484,4 +498,14 @@ describe('CollaboratorsGetter', function () {
       expect(count).to.equal(2)
     })
   })
+
+  describe('getInvitedPendingEditorCount', function () {
+    it('should return the count of pending editors', async function () {
+      const count =
+        await this.CollaboratorsGetter.promises.getInvitedPendingEditorCount(
+          this.project._id
+        )
+      expect(count).to.equal(1)
+    })
+  })
 })

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

@@ -106,6 +106,7 @@ describe('CollaboratorsHandler', function () {
               $pull: {
                 collaberator_refs: this.userId,
                 readOnly_refs: this.userId,
+                pendingEditor_refs: this.userId,
                 tokenAccessReadOnly_refs: this.userId,
                 tokenAccessReadAndWrite_refs: this.userId,
                 archived: this.userId,
@@ -148,6 +149,7 @@ describe('CollaboratorsHandler', function () {
               $pull: {
                 collaberator_refs: this.userId,
                 readOnly_refs: this.userId,
+                pendingEditor_refs: this.userId,
                 tokenAccessReadOnly_refs: this.userId,
                 tokenAccessReadAndWrite_refs: this.userId,
                 trashed: this.userId,
@@ -182,6 +184,7 @@ describe('CollaboratorsHandler', function () {
               $pull: {
                 collaberator_refs: this.userId,
                 readOnly_refs: this.userId,
+                pendingEditor_refs: this.userId,
                 tokenAccessReadOnly_refs: this.userId,
                 tokenAccessReadAndWrite_refs: this.userId,
                 archived: this.userId,
@@ -377,6 +380,7 @@ describe('CollaboratorsHandler', function () {
               $pull: {
                 collaberator_refs: this.userId,
                 readOnly_refs: this.userId,
+                pendingEditor_refs: this.userId,
                 tokenAccessReadOnly_refs: this.userId,
                 tokenAccessReadAndWrite_refs: this.userId,
                 archived: this.userId,
@@ -457,6 +461,24 @@ describe('CollaboratorsHandler', function () {
         )
         .chain('exec')
         .resolves()
+      this.ProjectMock.expects('updateMany')
+        .withArgs(
+          { pendingEditor_refs: this.fromUserId },
+          {
+            $addToSet: { pendingEditor_refs: this.toUserId },
+          }
+        )
+        .chain('exec')
+        .resolves()
+      this.ProjectMock.expects('updateMany')
+        .withArgs(
+          { pendingEditor_refs: this.fromUserId },
+          {
+            $pull: { pendingEditor_refs: this.fromUserId },
+          }
+        )
+        .chain('exec')
+        .resolves()
     })
 
     describe('successfully', function () {
@@ -501,7 +523,10 @@ describe('CollaboratorsHandler', function () {
             ],
           },
           {
-            $pull: { collaberator_refs: this.userId },
+            $pull: {
+              collaberator_refs: this.userId,
+              pendingEditor_refs: this.userId,
+            },
             $addToSet: { readOnly_refs: this.userId },
           }
         )
@@ -526,7 +551,10 @@ describe('CollaboratorsHandler', function () {
           },
           {
             $addToSet: { collaberator_refs: this.userId },
-            $pull: { readOnly_refs: this.userId },
+            $pull: {
+              readOnly_refs: this.userId,
+              pendingEditor_refs: this.userId,
+            },
           }
         )
         .chain('exec')

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

@@ -131,6 +131,11 @@ describe('ProjectController', function () {
         isUserInvitedReadWriteMemberOfProject: sinon.stub().resolves(true),
       },
     }
+    this.CollaboratorsHandler = {
+      promises: {
+        setCollaboratorPrivilegeLevel: sinon.stub().resolves(),
+      },
+    }
     this.ProjectEntityHandler = {}
     this.UserGetter = {
       getUserFullEmails: sinon.stub().yields(null, []),
@@ -200,12 +205,16 @@ describe('ProjectController', function () {
     this.OnboardingDataCollectionManager = {
       getOnboardingDataValue: sinon.stub().resolves(null),
     }
+    this.Modules = {
+      promises: { hooks: { fire: sinon.stub().resolves() } },
+    }
 
     this.ProjectController = SandboxedModule.require(MODULE_PATH, {
       requires: {
         'mongodb-legacy': { ObjectId },
         '@overleaf/settings': this.settings,
         '@overleaf/metrics': this.Metrics,
+        '../Collaborators/CollaboratorsHandler': this.CollaboratorsHandler,
         '../SplitTests/SplitTestHandler': this.SplitTestHandler,
         '../SplitTests/SplitTestSessionHandler': this.SplitTestSessionHandler,
         './ProjectDeleter': this.ProjectDeleter,
@@ -255,6 +264,7 @@ describe('ProjectController', function () {
             updateUser: sinon.stub().resolves(),
           },
         },
+        '../../infrastructure/Modules': this.Modules,
       },
     })
 
@@ -1015,9 +1025,13 @@ describe('ProjectController', function () {
 
     describe('link sharing changes active', 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' }
+            }
+          }
+        )
       })
 
       describe('when user is a read write token member (and not already a named editor)', function () {
@@ -1059,6 +1073,57 @@ describe('ProjectController', function () {
         })
       })
     })
+
+    describe('link sharing enforcement', function () {
+      describe('when not active (default)', 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: 'default' }
+              }
+            }
+          )
+        })
+
+        it('should not call the collaborator limit enforcement check', function (done) {
+          this.res.render = (pageName, opts) => {
+            this.Modules.promises.hooks.fire.should.not.have.been.calledWith(
+              'enforceCollaboratorLimit'
+            )
+            done()
+          }
+          this.ProjectController.loadEditor(this.req, this.res)
+        })
+      })
+
+      describe('when active', 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' }
+              }
+            }
+          )
+        })
+
+        it('should call the collaborator limit enforcement check', function (done) {
+          this.res.render = (pageName, opts) => {
+            this.Modules.promises.hooks.fire.should.have.been.calledWith(
+              'enforceCollaboratorLimit',
+              this.project_id
+            )
+            done()
+          }
+          this.ProjectController.loadEditor(this.req, this.res)
+        })
+      })
+    })
   })
 
   describe('userProjectsJson', function () {