Pārlūkot izejas kodu

Add serverside checks for changing the user access level after link sharing changes (#19168)

* Add getEditInviteCount to count only edit collaborators

* Add getInvitedEditCollaboratorCount to count joined editors

* Add canAddXEditCollaborators to determine if owner can add more editors

* Update setCollaboratorInfo to check if editor slots are available

GitOrigin-RevId: a88707f102dfbde39322f5a7bbc79d47b6e810d5
Thomas 2 gadi atpakaļ
vecāks
revīzija
a047388b08

+ 26 - 0
services/web/app/src/Features/Collaborators/CollaboratorsController.js

@@ -13,6 +13,10 @@ const { expressify } = require('@overleaf/promise-utils')
 const { hasAdminAccess } = require('../Helpers/AdminAuthorizationHelper')
 const TokenAccessHandler = require('../TokenAccess/TokenAccessHandler')
 const ProjectAuditLogHandler = require('../Project/ProjectAuditLogHandler')
+const ProjectGetter = require('../Project/ProjectGetter')
+const SplitTestHandler = require('../SplitTests/SplitTestHandler')
+const LimitationsManager = require('../Subscription/LimitationsManager')
+const PrivilegeLevels = require('../Authorization/PrivilegeLevels')
 
 module.exports = {
   removeUserFromProject: expressify(removeUserFromProject),
@@ -75,6 +79,28 @@ async function setCollaboratorInfo(req, res, next) {
     const projectId = req.params.Project_id
     const userId = req.params.user_id
     const { privilegeLevel } = req.body
+
+    if (privilegeLevel !== PrivilegeLevels.READ_ONLY) {
+      const project = await ProjectGetter.promises.getProject(projectId, {
+        owner_ref: 1,
+      })
+      const linkSharingChanges =
+        await SplitTestHandler.promises.getAssignmentForUser(
+          project.owner_ref,
+          'link-sharing-warning'
+        )
+      const allowed =
+        await LimitationsManager.promises.canAddXEditCollaborators(projectId, 1)
+      if (linkSharingChanges?.variant === 'active') {
+        if (!allowed) {
+          return HttpErrorHandler.forbidden(
+            req,
+            res,
+            'edit collaborator limit reached'
+          )
+        }
+      }
+    }
     await CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel(
       projectId,
       userId,

+ 11 - 0
services/web/app/src/Features/Collaborators/CollaboratorsGetter.js

@@ -39,6 +39,7 @@ module.exports = {
     getInvitedMembersWithPrivilegeLevelsFromFields,
     getMemberIdPrivilegeLevel,
     getInvitedCollaboratorCount,
+    getInvitedEditCollaboratorCount,
     getProjectsUserIsMemberOf,
     dangerouslyGetAllProjectsUserIsMemberOf,
     isUserInvitedMemberOfProject,
@@ -125,6 +126,16 @@ async function getInvitedCollaboratorCount(projectId) {
   return count - 1 // Don't count project owner
 }
 
+async function getInvitedEditCollaboratorCount(projectId) {
+  // Only counts invited members with readAndWrite privilege
+  const members = await getMemberIdsWithPrivilegeLevels(projectId)
+  return members.filter(
+    m =>
+      m.source === Sources.INVITE &&
+      m.privilegeLevel === PrivilegeLevels.READ_AND_WRITE
+  ).length
+}
+
 async function isUserInvitedMemberOfProject(userId, projectId) {
   if (!userId) {
     return false

+ 10 - 0
services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.js

@@ -8,6 +8,7 @@ const UserGetter = require('../User/UserGetter')
 const ProjectGetter = require('../Project/ProjectGetter')
 const Crypto = require('crypto')
 const NotificationsBuilder = require('../Notifications/NotificationsBuilder')
+const PrivilegeLevels = require('../Authorization/PrivilegeLevels')
 
 const randomBytes = promisify(Crypto.randomBytes)
 
@@ -28,6 +29,15 @@ const CollaboratorsInviteHandler = {
     return count
   },
 
+  async getEditInviteCount(projectId) {
+    logger.debug({ projectId }, 'counting edit invites for project')
+    const count = await ProjectInvite.countDocuments({
+      projectId,
+      privileges: { $ne: PrivilegeLevels.READ_ONLY },
+    }).exec()
+    return count
+  },
+
   async _trySendInviteNotification(projectId, sendingUser, invite) {
     const { email } = invite
     const existingUser = await UserGetter.promises.getUserByAnyEmail(email, {

+ 20 - 1
services/web/app/src/Features/Subscription/LimitationsManager.js

@@ -36,7 +36,24 @@ async function canAddXCollaborators(projectId, numberOfNewCollaborators) {
     await CollaboratorsInvitesHandler.promises.getInviteCount(projectId)
   return (
     currentNumber + inviteCount + numberOfNewCollaborators <= allowedNumber ||
-    allowedNumber < 0
+    allowedNumber < 0 // -1 means unlimited
+  )
+}
+
+async function canAddXEditCollaborators(
+  projectId,
+  numberOfNewEditCollaborators
+) {
+  const allowedNumber = await allowedNumberOfCollaboratorsInProject(projectId)
+  const currentEditors =
+    await CollaboratorsGetter.promises.getInvitedEditCollaboratorCount(
+      projectId
+    )
+  const editInviteCount =
+    await CollaboratorsInvitesHandler.promises.getEditInviteCount(projectId)
+  return (
+    currentEditors + editInviteCount + numberOfNewEditCollaborators <=
+      allowedNumber || allowedNumber < 0 // -1 means unlimited
   )
 }
 
@@ -130,6 +147,7 @@ const LimitationsManager = {
     allowedNumberOfCollaboratorsForUser
   ),
   canAddXCollaborators: callbackify(canAddXCollaborators),
+  canAddXEditCollaborators: callbackify(canAddXEditCollaborators),
   hasPaidSubscription: callbackifyMultiResult(hasPaidSubscription, [
     'hasPaidSubscription',
     'subscription',
@@ -159,6 +177,7 @@ const LimitationsManager = {
     allowedNumberOfCollaboratorsInProject,
     allowedNumberOfCollaboratorsForUser,
     canAddXCollaborators,
+    canAddXEditCollaborators,
     hasPaidSubscription,
     userHasSubscriptionOrIsGroupMember,
     userHasV2Subscription,

+ 108 - 0
services/web/test/unit/src/Collaborators/CollaboratorsControllerTests.js

@@ -59,6 +59,24 @@ describe('CollaboratorsController', function () {
       addEntryInBackground: sinon.stub(),
     }
 
+    this.ProjectGetter = {
+      promises: {
+        getProject: sinon.stub().resolves({ owner_ref: this.user._id }),
+      },
+    }
+
+    this.SplitTestHandler = {
+      promises: {
+        getAssignmentForUser: sinon.stub().resolves({ variant: 'default' }),
+      },
+    }
+
+    this.LimitationsManager = {
+      promises: {
+        canAddXEditCollaborators: sinon.stub().resolves(),
+      },
+    }
+
     this.CollaboratorsController = SandboxedModule.require(MODULE_PATH, {
       requires: {
         mongodb: { ObjectId },
@@ -71,6 +89,9 @@ describe('CollaboratorsController', function () {
         '../Authentication/SessionManager': this.SessionManager,
         '../TokenAccess/TokenAccessHandler': this.TokenAccessHandler,
         '../Project/ProjectAuditLogHandler': this.ProjectAuditLogHandler,
+        '../Project/ProjectGetter': this.ProjectGetter,
+        '../SplitTests/SplitTestHandler': this.SplitTestHandler,
+        '../Subscription/LimitationsManager': this.LimitationsManager,
       },
     })
   })
@@ -271,6 +292,93 @@ describe('CollaboratorsController', function () {
         this.next
       )
     })
+
+    describe('when link-sharing-warning test active', function () {
+      beforeEach(function () {
+        this.SplitTestHandler.promises.getAssignmentForUser.resolves({
+          variant: 'active',
+        })
+      })
+
+      describe('when setting privilege level to readAndWrite', function () {
+        beforeEach(function () {
+          this.req.body = { privilegeLevel: 'readAndWrite' }
+        })
+
+        describe('when owner can add new edit collaborators', function () {
+          beforeEach(function () {
+            this.LimitationsManager.promises.canAddXEditCollaborators.resolves(
+              true
+            )
+          })
+
+          it('should set privilege level after checking collaborators can be added', function (done) {
+            this.res.sendStatus = status => {
+              expect(status).to.equal(204)
+              expect(
+                this.LimitationsManager.promises.canAddXEditCollaborators
+              ).to.have.been.calledWith(this.projectId, 1)
+              done()
+            }
+            this.CollaboratorsController.setCollaboratorInfo(this.req, this.res)
+          })
+        })
+
+        describe('when owner cannot add edit collaborators', function () {
+          beforeEach(function () {
+            this.LimitationsManager.promises.canAddXEditCollaborators.resolves(
+              false
+            )
+          })
+
+          it('should return a 403 if trying to set a new edit collaborator', function (done) {
+            this.HttpErrorHandler.forbidden = sinon.spy((req, res) => {
+              expect(req).to.equal(this.req)
+              expect(res).to.equal(this.res)
+              expect(
+                this.LimitationsManager.promises.canAddXEditCollaborators
+              ).to.have.been.calledWith(this.projectId, 1)
+              expect(
+                this.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel
+              ).to.not.have.been.called
+              done()
+            })
+            this.CollaboratorsController.setCollaboratorInfo(this.req, this.res)
+          })
+        })
+      })
+
+      describe('when setting privilege level to readOnly', function () {
+        beforeEach(function () {
+          this.req.body = { privilegeLevel: 'readOnly' }
+        })
+
+        describe('when owner cannot add edit collaborators', function () {
+          beforeEach(function () {
+            this.LimitationsManager.promises.canAddXEditCollaborators.resolves(
+              false
+            )
+          })
+
+          it('should always allow setting a collaborator to viewer even if user cant add edit collaborators', function (done) {
+            this.res.sendStatus = status => {
+              expect(status).to.equal(204)
+              expect(this.LimitationsManager.promises.canAddXEditCollaborators)
+                .to.not.have.been.called
+              expect(
+                this.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel
+              ).to.have.been.calledWith(
+                this.projectId,
+                this.user._id,
+                'readOnly'
+              )
+              done()
+            }
+            this.CollaboratorsController.setCollaboratorInfo(this.req, this.res)
+          })
+        })
+      })
+    })
   })
 
   describe('transferOwnership', function () {

+ 10 - 0
services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js

@@ -474,4 +474,14 @@ describe('CollaboratorsGetter', function () {
       })
     })
   })
+
+  describe('getInvitedEditCollaboratorCount', function () {
+    it('should return the count of invited edit collaborators (token, readAndWrite)', async function () {
+      const count =
+        await this.CollaboratorsGetter.promises.getInvitedEditCollaboratorCount(
+          this.project._id
+        )
+      expect(count).to.equal(2)
+    })
+  })
 })

+ 34 - 0
services/web/test/unit/src/Collaborators/CollaboratorsInviteHandlerTests.js

@@ -117,6 +117,40 @@ describe('CollaboratorsInviteHandler', function () {
     })
   })
 
+  describe('getEditInviteCount', function () {
+    beforeEach(function () {
+      this.ProjectInvite.countDocuments.returns({
+        exec: sinon.stub().resolves(2),
+      })
+      this.call = async () => {
+        return await this.CollaboratorsInviteHandler.promises.getEditInviteCount(
+          this.projectId
+        )
+      }
+    })
+
+    it('should produce the count of documents', async function () {
+      const count = await this.call()
+      expect(this.ProjectInvite.countDocuments).to.be.calledWith({
+        projectId: this.projectId,
+        privileges: { $ne: 'readOnly' },
+      })
+      expect(count).to.equal(2)
+    })
+
+    describe('when model.countDocuments produces an error', function () {
+      beforeEach(function () {
+        this.ProjectInvite.countDocuments.returns({
+          exec: sinon.stub().rejects(new Error('woops')),
+        })
+      })
+
+      it('should produce an error', async function () {
+        await expect(this.call()).to.be.rejectedWith(Error)
+      })
+    })
+  })
+
   describe('getAllInvites', function () {
     beforeEach(function () {
       this.fakeInvites = [

+ 170 - 2
services/web/test/unit/src/Subscription/LimitationsManagerTests.js

@@ -54,11 +54,17 @@ describe('LimitationsManager', function () {
     }
 
     this.CollaboratorsGetter = {
-      promises: { getInvitedCollaboratorCount: sinon.stub().resolves() },
+      promises: {
+        getInvitedCollaboratorCount: sinon.stub().resolves(),
+        getInvitedEditCollaboratorCount: sinon.stub().resolves(),
+      },
     }
 
     this.CollaboratorsInviteHandler = {
-      promises: { getInviteCount: sinon.stub().resolves() },
+      promises: {
+        getInviteCount: sinon.stub().resolves(),
+        getEditInviteCount: sinon.stub().resolves(),
+      },
     }
 
     this.LimitationsManager = SandboxedModule.require(modulePath, {
@@ -320,6 +326,168 @@ describe('LimitationsManager', function () {
       })
     })
   })
+  describe('canAddXEditCollaborators', function () {
+    describe('when the project has fewer collaborators than allowed', function () {
+      beforeEach(function (done) {
+        this.current_number = 1
+        this.user.features.collaborators = 2
+        this.invite_count = 0
+        this.CollaboratorsGetter.promises.getInvitedEditCollaboratorCount =
+          sinon.stub().resolves(this.current_number)
+        this.CollaboratorsInviteHandler.promises.getEditInviteCount = sinon
+          .stub()
+          .resolves(this.invite_count)
+        this.callback = sinon.stub().callsFake(() => done())
+        this.LimitationsManager.canAddXEditCollaborators(
+          this.projectId,
+          1,
+          this.callback
+        )
+      })
+
+      it('should return true', function () {
+        this.callback.calledWith(null, true).should.equal(true)
+      })
+    })
+
+    describe('when the project has fewer collaborators and invites than allowed', function () {
+      beforeEach(function (done) {
+        this.current_number = 1
+        this.user.features.collaborators = 4
+        this.invite_count = 1
+        this.CollaboratorsGetter.promises.getInvitedEditCollaboratorCount =
+          sinon.stub().resolves(this.current_number)
+        this.CollaboratorsInviteHandler.promises.getEditInviteCount = sinon
+          .stub()
+          .resolves(this.invite_count)
+        this.callback = sinon.stub().callsFake(() => done())
+        this.LimitationsManager.canAddXEditCollaborators(
+          this.projectId,
+          1,
+          this.callback
+        )
+      })
+
+      it('should return true', function () {
+        this.callback.calledWith(null, true).should.equal(true)
+      })
+    })
+
+    describe('when the project has fewer collaborators than allowed but I want to add more than allowed', function () {
+      beforeEach(function (done) {
+        this.current_number = 1
+        this.user.features.collaborators = 2
+        this.invite_count = 0
+        this.CollaboratorsGetter.promises.getInvitedEditCollaboratorCount =
+          sinon.stub().resolves(this.current_number)
+        this.CollaboratorsInviteHandler.promises.getEditInviteCount = sinon
+          .stub()
+          .resolves(this.invite_count)
+        this.callback = sinon.stub().callsFake(() => done())
+        this.LimitationsManager.canAddXEditCollaborators(
+          this.projectId,
+          2,
+          this.callback
+        )
+      })
+
+      it('should return false', function () {
+        this.callback.calledWith(null, false).should.equal(true)
+      })
+    })
+
+    describe('when the project has more collaborators than allowed', function () {
+      beforeEach(function (done) {
+        this.current_number = 3
+        this.user.features.collaborators = 2
+        this.invite_count = 0
+        this.CollaboratorsGetter.promises.getInvitedEditCollaboratorCount =
+          sinon.stub().resolves(this.current_number)
+        this.CollaboratorsInviteHandler.promises.getEditInviteCount = sinon
+          .stub()
+          .resolves(this.invite_count)
+        this.callback = sinon.stub().callsFake(() => done())
+        this.LimitationsManager.canAddXEditCollaborators(
+          this.projectId,
+          1,
+          this.callback
+        )
+      })
+
+      it('should return false', function () {
+        this.callback.calledWith(null, false).should.equal(true)
+      })
+    })
+
+    describe('when the project has infinite collaborators', function () {
+      beforeEach(function (done) {
+        this.current_number = 100
+        this.user.features.collaborators = -1
+        this.invite_count = 0
+        this.CollaboratorsGetter.promises.getInvitedEditCollaboratorCount =
+          sinon.stub().resolves(this.current_number)
+        this.CollaboratorsInviteHandler.promises.getEditInviteCount = sinon
+          .stub()
+          .resolves(this.invite_count)
+        this.callback = sinon.stub().callsFake(() => done())
+        this.LimitationsManager.canAddXEditCollaborators(
+          this.projectId,
+          1,
+          this.callback
+        )
+      })
+
+      it('should return true', function () {
+        this.callback.calledWith(null, true).should.equal(true)
+      })
+    })
+
+    describe('when the project has more invites than allowed', function () {
+      beforeEach(function (done) {
+        this.current_number = 0
+        this.user.features.collaborators = 2
+        this.invite_count = 2
+        this.CollaboratorsGetter.promises.getInvitedEditCollaboratorCount =
+          sinon.stub().resolves(this.current_number)
+        this.CollaboratorsInviteHandler.promises.getEditInviteCount = sinon
+          .stub()
+          .resolves(this.invite_count)
+        this.callback = sinon.stub().callsFake(() => done())
+        this.LimitationsManager.canAddXEditCollaborators(
+          this.projectId,
+          1,
+          this.callback
+        )
+      })
+
+      it('should return false', function () {
+        this.callback.calledWith(null, false).should.equal(true)
+      })
+    })
+
+    describe('when the project has more invites and collaborators than allowed', function () {
+      beforeEach(function (done) {
+        this.current_number = 1
+        this.user.features.collaborators = 2
+        this.invite_count = 1
+        this.CollaboratorsGetter.promises.getInvitedEditCollaboratorCount =
+          sinon.stub().resolves(this.current_number)
+        this.CollaboratorsInviteHandler.promises.getEditInviteCount = sinon
+          .stub()
+          .resolves(this.invite_count)
+        this.callback = sinon.stub().callsFake(() => done())
+        this.LimitationsManager.canAddXEditCollaborators(
+          this.projectId,
+          1,
+          this.callback
+        )
+      })
+
+      it('should return false', function () {
+        this.callback.calledWith(null, false).should.equal(true)
+      })
+    })
+  })
 
   describe('userHasV2Subscription', function () {
     beforeEach(function () {