[web] fetch project once for joinProject (#25667)

* [web] fetch project once for joinProject

* [web] await all the nested helpers for getting privilege levels

Co-authored-by: Mathias Jakobsen <mathias.jakobsen@overleaf.com>

---------

Co-authored-by: Mathias Jakobsen <mathias.jakobsen@overleaf.com>
GitOrigin-RevId: f0280c36ef995b417ccdab15014f05954e18c5f0

services/web/app/src/Features/Authorization/AuthorizationManager.js

@@ -88,9 +88,54 @@ async function getPrivilegeLevelForProject(
   opts = {}
 ) {
   if (userId) {
-    return getPrivilegeLevelForProjectWithUser(userId, projectId, opts)
+    return await getPrivilegeLevelForProjectWithUser(
+      userId,
+      projectId,
+      null,
+      opts
+    )
   } else {
-    return getPrivilegeLevelForProjectWithoutUser(projectId, token, opts)
+    return await getPrivilegeLevelForProjectWithoutUser(projectId, token, opts)
+  }
+}
+
+/**
+ * Get the privilege level that the user has for the project.
+ *
+ * @param userId - The id of the user that wants to access the project.
+ * @param projectId - The id of the project to be accessed.
+ * @param {string} token
+ * @param {ProjectAccess} projectAccess
+ * @param {Object} opts
+ * @param {boolean} opts.ignoreSiteAdmin - Do not consider whether the user is
+ *     a site admin.
+ * @param {boolean} opts.ignorePublicAccess - Do not consider the project is
+ *     publicly accessible.
+ *
+ * @returns {string|boolean} The privilege level. One of "owner",
+ *     "readAndWrite", "readOnly" or false.
+ */
+async function getPrivilegeLevelForProjectWithProjectAccess(
+  userId,
+  projectId,
+  token,
+  projectAccess,
+  opts = {}
+) {
+  if (userId) {
+    return await getPrivilegeLevelForProjectWithUser(
+      userId,
+      projectId,
+      projectAccess,
+      opts
+    )
+  } else {
+    return await _getPrivilegeLevelForProjectWithoutUserWithPublicAccessLevel(
+      projectId,
+      token,
+      projectAccess.publicAccessLevel(),
+      opts
+    )
   }
 }
 
@@ -98,6 +143,7 @@ async function getPrivilegeLevelForProject(
 async function getPrivilegeLevelForProjectWithUser(
   userId,
   projectId,
+  projectAccess,
   opts = {}
 ) {
   if (!opts.ignoreSiteAdmin) {
@@ -106,11 +152,11 @@ async function getPrivilegeLevelForProjectWithUser(
     }
   }
 
-  const privilegeLevel =
-    await CollaboratorsGetter.promises.getMemberIdPrivilegeLevel(
-      userId,
-      projectId
-    )
+  projectAccess =
+    projectAccess ||
+    (await CollaboratorsGetter.promises.getProjectAccess(projectId))
+
+  const privilegeLevel = projectAccess.privilegeLevelForUser(userId)
   if (privilegeLevel && privilegeLevel !== PrivilegeLevels.NONE) {
     // The user has direct access
     return privilegeLevel
@@ -119,7 +165,7 @@ async function getPrivilegeLevelForProjectWithUser(
   if (!opts.ignorePublicAccess) {
     // Legacy public-access system
     // User is present (not anonymous), but does not have direct access
-    const publicAccessLevel = await getPublicAccessLevel(projectId)
+    const publicAccessLevel = projectAccess.publicAccessLevel()
     if (publicAccessLevel === PublicAccessLevels.READ_ONLY) {
       return PrivilegeLevels.READ_ONLY
     }
@@ -137,7 +183,21 @@ async function getPrivilegeLevelForProjectWithoutUser(
   token,
   opts = {}
 ) {
-  const publicAccessLevel = await getPublicAccessLevel(projectId)
+  return await _getPrivilegeLevelForProjectWithoutUserWithPublicAccessLevel(
+    projectId,
+    token,
+    await getPublicAccessLevel(projectId),
+    opts
+  )
+}
+
+// User is Anonymous, Try Token-based access
+async function _getPrivilegeLevelForProjectWithoutUserWithPublicAccessLevel(
+  projectId,
+  token,
+  publicAccessLevel,
+  opts = {}
+) {
   if (!opts.ignorePublicAccess) {
     if (publicAccessLevel === PublicAccessLevels.READ_ONLY) {
       // Legacy public read-only access for anonymous user
@@ -149,7 +209,7 @@ async function getPrivilegeLevelForProjectWithoutUser(
     }
   }
   if (publicAccessLevel === PublicAccessLevels.TOKEN_BASED) {
-    return getPrivilegeLevelForProjectWithToken(projectId, token)
+    return await getPrivilegeLevelForProjectWithToken(projectId, token)
   }
 
   // Deny anonymous user access
@@ -309,6 +369,7 @@ module.exports = {
     canUserRenameProject,
     canUserAdminProject,
     getPrivilegeLevelForProject,
+    getPrivilegeLevelForProjectWithProjectAccess,
     isRestrictedUserForProject,
     isUserSiteAdmin,
   },

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

@@ -32,6 +32,7 @@ module.exports = {
   userIsTokenMember: callbackify(userIsTokenMember),
   getAllInvitedMembers: callbackify(getAllInvitedMembers),
   promises: {
+    getProjectAccess,
     getMemberIdsWithPrivilegeLevels,
     getMemberIds,
     getInvitedMemberIds,
@@ -134,6 +135,7 @@ class ProjectAccess {
    * @return {typeof PrivilegeLevels[keyof PrivilegeLevels]}
    */
   privilegeLevelForUser(userId) {
+    if (!userId) return PrivilegeLevels.NONE
     for (const member of this.#members) {
       if (member.id === userId.toString()) {
         return member.privilegeLevel
@@ -142,11 +144,26 @@ class ProjectAccess {
     return PrivilegeLevels.NONE
   }
 
+  /**
+   * @param {string | ObjectId} userId
+   * @return {boolean}
+   */
+  isUserTokenMember(userId) {
+    if (!userId) return false
+    for (const member of this.#members) {
+      if (member.id === userId.toString() && member.source === Sources.TOKEN) {
+        return true
+      }
+    }
+    return false
+  }
+
   /**
    * @param {string | ObjectId} userId
    * @return {boolean}
    */
   isUserInvitedMember(userId) {
+    if (!userId) return false
     for (const member of this.#members) {
       if (member.id === userId.toString() && member.source !== Sources.TOKEN) {
         return true
@@ -199,6 +216,8 @@ class ProjectAccess {
   }
 }
 
+module.exports.ProjectAccess = ProjectAccess
+
 async function getProjectAccess(projectId) {
   const project = await ProjectGetter.promises.getProject(projectId, {
     owner_ref: 1,

services/web/app/src/Features/Editor/EditorHttpController.js

@@ -4,14 +4,13 @@ const ProjectGetter = require('../Project/ProjectGetter')
 const AuthorizationManager = require('../Authorization/AuthorizationManager')
 const ProjectEditorHandler = require('../Project/ProjectEditorHandler')
 const Metrics = require('@overleaf/metrics')
-const CollaboratorsGetter = require('../Collaborators/CollaboratorsGetter')
 const CollaboratorsInviteGetter = require('../Collaborators/CollaboratorsInviteGetter')
-const CollaboratorsHandler = require('../Collaborators/CollaboratorsHandler')
 const PrivilegeLevels = require('../Authorization/PrivilegeLevels')
 const SessionManager = require('../Authentication/SessionManager')
 const Errors = require('../Errors/Errors')
 const { expressify } = require('@overleaf/promise-utils')
 const Settings = require('@overleaf/settings')
+const { ProjectAccess } = require('../Collaborators/CollaboratorsGetter')
 
 module.exports = {
   joinProject: expressify(joinProject),
@@ -75,31 +74,23 @@ async function _buildJoinProjectView(req, projectId, userId) {
   if (project == null) {
     throw new Errors.NotFoundError('project not found')
   }
-  const members =
-    await CollaboratorsGetter.promises.getInvitedMembersWithPrivilegeLevels(
-      projectId
-    )
+  const projectAccess = new ProjectAccess(project)
+  const members = await projectAccess.loadInvitedMembers()
   const token = req.body.anonymousAccessToken
   const privilegeLevel =
-    await AuthorizationManager.promises.getPrivilegeLevelForProject(
+    await AuthorizationManager.promises.getPrivilegeLevelForProjectWithProjectAccess(
       userId,
       projectId,
-      token
+      token,
+      projectAccess
     )
   if (privilegeLevel == null || privilegeLevel === PrivilegeLevels.NONE) {
     return { project: null, privilegeLevel: null, isRestrictedUser: false }
   }
   const invites =
     await CollaboratorsInviteGetter.promises.getAllInvites(projectId)
-  const isTokenMember = await CollaboratorsHandler.promises.userIsTokenMember(
-    userId,
-    projectId
-  )
-  const isInvitedMember =
-    await CollaboratorsGetter.promises.isUserInvitedMemberOfProject(
-      userId,
-      projectId
-    )
+  const isTokenMember = projectAccess.isUserTokenMember(userId)
+  const isInvitedMember = projectAccess.isUserInvitedMember(userId)
   const isRestrictedUser = AuthorizationManager.isRestrictedUser(
     userId,
     privilegeLevel,

services/web/test/unit/src/Authorization/AuthorizationManagerTests.js

@@ -27,7 +27,10 @@ describe('AuthorizationManager', function () {
 
     this.CollaboratorsGetter = {
       promises: {
-        getMemberIdPrivilegeLevel: sinon.stub().resolves(PrivilegeLevels.NONE),
+        getProjectAccess: sinon.stub().resolves({
+          publicAccessLevel: sinon.stub().returns(PublicAccessLevels.PRIVATE),
+          privilegeLevelForUser: sinon.stub().returns(PrivilegeLevels.NONE),
+        }),
       },
     }
 
@@ -113,9 +116,17 @@ describe('AuthorizationManager', function () {
 
       describe('with a user id with a privilege level', function () {
         beforeEach(async function () {
-          this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel
-            .withArgs(this.user._id, this.project._id)
-            .resolves(PrivilegeLevels.READ_ONLY)
+          this.CollaboratorsGetter.promises.getProjectAccess
+            .withArgs(this.project._id)
+            .resolves({
+              publicAccessLevel: sinon
+                .stub()
+                .returns(PublicAccessLevels.PRIVATE),
+              privilegeLevelForUser: sinon
+                .stub()
+                .withArgs(this.user._id)
+                .returns(PrivilegeLevels.READ_ONLY),
+            })
           this.result =
             await this.AuthorizationManager.promises.getPrivilegeLevelForProject(
               this.user._id,
@@ -171,8 +182,8 @@ describe('AuthorizationManager', function () {
               )
           })
 
-          it('should not call CollaboratorsGetter.getMemberIdPrivilegeLevel', function () {
-            this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel.called.should.equal(
+          it('should not call CollaboratorsGetter.getProjectAccess', function () {
+            this.CollaboratorsGetter.promises.getProjectAccess.called.should.equal(
               false
             )
           })
@@ -204,8 +215,8 @@ describe('AuthorizationManager', function () {
               )
           })
 
-          it('should not call CollaboratorsGetter.getMemberIdPrivilegeLevel', function () {
-            this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel.called.should.equal(
+          it('should not call CollaboratorsGetter.getProjectAccess', function () {
+            this.CollaboratorsGetter.promises.getProjectAccess.called.should.equal(
               false
             )
           })
@@ -237,8 +248,8 @@ describe('AuthorizationManager', function () {
               )
           })
 
-          it('should not call CollaboratorsGetter.getMemberIdPrivilegeLevel', function () {
-            this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel.called.should.equal(
+          it('should not call CollaboratorsGetter.getProjectAccess', function () {
+            this.CollaboratorsGetter.promises.getProjectAccess.called.should.equal(
               false
             )
           })
@@ -264,9 +275,17 @@ describe('AuthorizationManager', function () {
 
       describe('with a user id with a privilege level', function () {
         beforeEach(async function () {
-          this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel
-            .withArgs(this.user._id, this.project._id)
-            .resolves(PrivilegeLevels.READ_ONLY)
+          this.CollaboratorsGetter.promises.getProjectAccess
+            .withArgs(this.project._id)
+            .resolves({
+              publicAccessLevel: sinon
+                .stub()
+                .returns(PublicAccessLevels.PRIVATE),
+              privilegeLevelForUser: sinon
+                .stub()
+                .withArgs(this.user._id)
+                .returns(PrivilegeLevels.READ_ONLY),
+            })
           this.result =
             await this.AuthorizationManager.promises.getPrivilegeLevelForProject(
               this.user._id,
@@ -321,8 +340,8 @@ describe('AuthorizationManager', function () {
             )
         })
 
-        it('should not call CollaboratorsGetter.getMemberIdPrivilegeLevel', function () {
-          this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel.called.should.equal(
+        it('should not call CollaboratorsGetter.getProjectAccess', function () {
+          this.CollaboratorsGetter.promises.getProjectAccess.called.should.equal(
             false
           )
         })
@@ -336,13 +355,32 @@ describe('AuthorizationManager', function () {
     describe('with a public project', function () {
       beforeEach(function () {
         this.project.publicAccesLevel = 'readAndWrite'
+        this.CollaboratorsGetter.promises.getProjectAccess
+          .withArgs(this.project._id)
+          .resolves({
+            publicAccessLevel: sinon
+              .stub()
+              .returns(this.project.publicAccesLevel),
+            privilegeLevelForUser: sinon
+              .stub()
+              .withArgs(this.user._id)
+              .returns(PrivilegeLevels.NONE),
+          })
       })
 
       describe('with a user id with a privilege level', function () {
         beforeEach(async function () {
-          this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel
-            .withArgs(this.user._id, this.project._id)
-            .resolves(PrivilegeLevels.READ_ONLY)
+          this.CollaboratorsGetter.promises.getProjectAccess
+            .withArgs(this.project._id)
+            .resolves({
+              publicAccessLevel: sinon
+                .stub()
+                .returns(this.project.publicAccesLevel),
+              privilegeLevelForUser: sinon
+                .stub()
+                .withArgs(this.user._id)
+                .returns(PrivilegeLevels.READ_ONLY),
+            })
           this.result =
             await this.AuthorizationManager.promises.getPrivilegeLevelForProject(
               this.user._id,
@@ -397,8 +435,8 @@ describe('AuthorizationManager', function () {
             )
         })
 
-        it('should not call CollaboratorsGetter.getMemberIdPrivilegeLevel', function () {
-          this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel.called.should.equal(
+        it('should not call CollaboratorsGetter.getProjectAccess', function () {
+          this.CollaboratorsGetter.promises.getProjectAccess.called.should.equal(
             false
           )
         })
@@ -410,6 +448,11 @@ describe('AuthorizationManager', function () {
     })
 
     describe("when the project doesn't exist", function () {
+      beforeEach(function () {
+        this.CollaboratorsGetter.promises.getProjectAccess.rejects(
+          new Errors.NotFoundError()
+        )
+      })
       it('should return a NotFoundError', async function () {
         const someOtherId = new ObjectId()
         await expect(
@@ -424,9 +467,15 @@ describe('AuthorizationManager', function () {
 
     describe('when the project id is not valid', function () {
       beforeEach(function () {
-        this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel
-          .withArgs(this.user._id, this.project._id)
-          .resolves(PrivilegeLevels.READ_ONLY)
+        this.CollaboratorsGetter.promises.getProjectAccess
+          .withArgs(this.project._id)
+          .resolves({
+            publicAccessLevel: sinon.stub().returns(PublicAccessLevels.PRIVATE),
+            privilegeLevelForUser: sinon
+              .stub()
+              .withArgs(this.user._id)
+              .returns(PrivilegeLevels.READ_ONLY),
+          })
       })
 
       it('should return a error', async function () {
@@ -529,9 +578,15 @@ describe('AuthorizationManager', function () {
 
   describe('canUserDeleteOrResolveThread', function () {
     it('should return true when user has write permissions', async function () {
-      this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel
-        .withArgs(this.user._id, this.project._id)
-        .resolves(PrivilegeLevels.READ_AND_WRITE)
+      this.CollaboratorsGetter.promises.getProjectAccess
+        .withArgs(this.project._id)
+        .resolves({
+          publicAccessLevel: sinon.stub().returns(PublicAccessLevels.PRIVATE),
+          privilegeLevelForUser: sinon
+            .stub()
+            .withArgs(this.user._id)
+            .returns(PrivilegeLevels.READ_AND_WRITE),
+        })
 
       const canResolve =
         await this.AuthorizationManager.promises.canUserDeleteOrResolveThread(
@@ -546,9 +601,15 @@ describe('AuthorizationManager', function () {
     })
 
     it('should return false when user has read permission', async function () {
-      this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel
-        .withArgs(this.user._id, this.project._id)
-        .resolves(PrivilegeLevels.READ_ONLY)
+      this.CollaboratorsGetter.promises.getProjectAccess
+        .withArgs(this.project._id)
+        .resolves({
+          publicAccessLevel: sinon.stub().returns(PublicAccessLevels.PRIVATE),
+          privilegeLevelForUser: sinon
+            .stub()
+            .withArgs(this.user._id)
+            .returns(PrivilegeLevels.READ_ONLY),
+        })
 
       const canResolve =
         await this.AuthorizationManager.promises.canUserDeleteOrResolveThread(
@@ -564,9 +625,15 @@ describe('AuthorizationManager', function () {
 
     describe('when user has review permission', function () {
       beforeEach(function () {
-        this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel
-          .withArgs(this.user._id, this.project._id)
-          .resolves(PrivilegeLevels.REVIEW)
+        this.CollaboratorsGetter.promises.getProjectAccess
+          .withArgs(this.project._id)
+          .resolves({
+            publicAccessLevel: sinon.stub().returns(PublicAccessLevels.PRIVATE),
+            privilegeLevelForUser: sinon
+              .stub()
+              .withArgs(this.user._id)
+              .returns(PrivilegeLevels.REVIEW),
+          })
       })
 
       it('should return false when user is not the comment author', async function () {
@@ -691,15 +758,27 @@ function testPermission(permission, privilegeLevels) {
 
 function setupUserPrivilegeLevel(privilegeLevel) {
   beforeEach(`set user privilege level to ${privilegeLevel}`, function () {
-    this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel
-      .withArgs(this.user._id, this.project._id)
-      .resolves(privilegeLevel)
+    this.CollaboratorsGetter.promises.getProjectAccess
+      .withArgs(this.project._id)
+      .resolves({
+        publicAccessLevel: sinon.stub().returns(PublicAccessLevels.PRIVATE),
+        privilegeLevelForUser: sinon
+          .stub()
+          .withArgs(this.user._id)
+          .returns(privilegeLevel),
+      })
   })
 }
 
 function setupPublicAccessLevel(level) {
   beforeEach(`set public access level to ${level}`, function () {
     this.project.publicAccesLevel = level
+    this.CollaboratorsGetter.promises.getProjectAccess
+      .withArgs(this.project._id)
+      .resolves({
+        publicAccessLevel: sinon.stub().returns(this.project.publicAccesLevel),
+        privilegeLevelForUser: sinon.stub().returns(PrivilegeLevels.NONE),
+      })
   })
 }
 

services/web/test/unit/src/Editor/EditorHttpControllerTests.js

@@ -51,10 +51,25 @@ describe('EditorHttpController', function () {
     this.AuthorizationManager = {
       isRestrictedUser: sinon.stub().returns(false),
       promises: {
-        getPrivilegeLevelForProject: sinon.stub().resolves('owner'),
+        getPrivilegeLevelForProjectWithProjectAccess: sinon
+          .stub()
+          .resolves('owner'),
       },
     }
     this.CollaboratorsGetter = {
+      ProjectAccess: class {
+        loadInvitedMembers() {
+          return []
+        }
+
+        isUserTokenMember() {
+          return false
+        }
+
+        isUserInvitedMember() {
+          return false
+        }
+      },
       promises: {
         getInvitedMembersWithPrivilegeLevels: sinon
           .stub()
@@ -170,9 +185,12 @@ describe('EditorHttpController', function () {
 
     describe('successfully', function () {
       beforeEach(function (done) {
-        this.CollaboratorsGetter.promises.isUserInvitedMemberOfProject.resolves(
-          true
-        )
+        sinon
+          .stub(
+            this.CollaboratorsGetter.ProjectAccess.prototype,
+            'isUserInvitedMember'
+          )
+          .returns(true)
         this.res.callback = done
         this.EditorHttpController.joinProject(this.req, this.res)
       })
@@ -214,7 +232,7 @@ describe('EditorHttpController', function () {
     describe('with a restricted user', function () {
       beforeEach(function (done) {
         this.AuthorizationManager.isRestrictedUser.returns(true)
-        this.AuthorizationManager.promises.getPrivilegeLevelForProject.resolves(
+        this.AuthorizationManager.promises.getPrivilegeLevelForProjectWithProjectAccess.resolves(
           'readOnly'
         )
         this.res.callback = done
@@ -234,7 +252,7 @@ describe('EditorHttpController', function () {
 
     describe('when not authorized', function () {
       beforeEach(function (done) {
-        this.AuthorizationManager.promises.getPrivilegeLevelForProject.resolves(
+        this.AuthorizationManager.promises.getPrivilegeLevelForProjectWithProjectAccess.resolves(
           null
         )
         this.res.callback = done
@@ -258,7 +276,7 @@ describe('EditorHttpController', function () {
         this.AuthorizationManager.isRestrictedUser
           .withArgs(null, 'readOnly', false, false)
           .returns(true)
-        this.AuthorizationManager.promises.getPrivilegeLevelForProject
+        this.AuthorizationManager.promises.getPrivilegeLevelForProjectWithProjectAccess
           .withArgs(null, this.project._id, this.token)
           .resolves('readOnly')
         this.EditorHttpController.joinProject(this.req, this.res)
@@ -277,11 +295,19 @@ describe('EditorHttpController', function () {
 
     describe('with a token access user', function () {
       beforeEach(function (done) {
-        this.CollaboratorsGetter.promises.isUserInvitedMemberOfProject.resolves(
-          false
-        )
-        this.CollaboratorsHandler.promises.userIsTokenMember.resolves(true)
-        this.AuthorizationManager.promises.getPrivilegeLevelForProject.resolves(
+        sinon
+          .stub(
+            this.CollaboratorsGetter.ProjectAccess.prototype,
+            'isUserInvitedMember'
+          )
+          .returns(false)
+        sinon
+          .stub(
+            this.CollaboratorsGetter.ProjectAccess.prototype,
+            'isUserTokenMember'
+          )
+          .returns(true)
+        this.AuthorizationManager.promises.getPrivilegeLevelForProjectWithProjectAccess.resolves(
           'readAndWrite'
         )
         this.res.callback = done