Просмотр исходного кода

Merge pull request #19301 from overleaf/jdt-collaborator-ai-usage

Prevent AI usage on projects where collaborators have a blocking policy

GitOrigin-RevId: 93bdd9c5accff51a14f0585249e13ed7f1fa4e53
Jimmy Domagala-Tang 2 лет назад
Родитель
Сommit
918c3e7e33

+ 2 - 2
services/web/app/src/Features/Authorization/PermissionsManager.js

@@ -422,8 +422,8 @@ async function checkUserPermissions(user, requiredCapabilities) {
  * checks if all collaborators of a given project have the specified capability, including the owner
  *
  * @async
- * @function checkCollaboratorsPermission
- * @param {string} userList - An array of all user to check permissions for
+ * @function checkUserListPermissions
+ * @param {Object[]} userList - An array of all user to check permissions for
  * @param {Array} capabilities - The list of the capabilities to check permission for.
  * @returns {Promise<boolean>} - A promise that resolves to `true` if all collaborators have the specified capability, otherwise `false`.
  */

+ 28 - 16
services/web/app/src/Features/Project/ProjectController.js

@@ -42,8 +42,7 @@ const PublicAccessLevels = require('../Authorization/PublicAccessLevels')
 const TagsHandler = require('../Tags/TagsHandler')
 const TutorialHandler = require('../Tutorial/TutorialHandler')
 const UserUpdater = require('../User/UserUpdater')
-const { checkUserPermissions } =
-  require('../Authorization/PermissionsManager').promises
+const Modules = require('../../infrastructure/Modules')
 const UserGetter = require('../User/UserGetter')
 
 /**
@@ -616,21 +615,34 @@ const _ProjectController = {
         !showPersonalAccessToken &&
         splitTestAssignments['personal-access-token'].variant === 'enabled' // `?personal-access-token=enabled`
 
-      // still allow users to access project if we cant get their permissions, but disable AI feature
-      let canUseAi
-      try {
-        canUseAi = await checkUserPermissions(user, ['use-ai'])
-      } catch (err) {
-        canUseAi = false
-      }
+      let showAiErrorAssistant = false
+      if (userId && Features.hasFeature('saas')) {
+        try {
+          // exit early if the user couldnt use ai anyways, since permissions checks are expensive
+          const canUseAiOnProject =
+            user.features?.aiErrorAssistant &&
+            (privilegeLevel === PrivilegeLevels.READ_AND_WRITE ||
+              privilegeLevel === PrivilegeLevels.OWNER)
+
+          if (canUseAiOnProject) {
+            // check permissions for user and project owner, to see if they allow AI on the project
+            const permissionsResults = await Modules.promises.hooks.fire(
+              'projectAllowsCapability',
+              project,
+              userId,
+              ['use-ai']
+            )
+            const aiAllowed = permissionsResults.every(
+              result => result === true
+            )
 
-      const showAiErrorAssistant =
-        userId &&
-        Features.hasFeature('saas') &&
-        user.features?.aiErrorAssistant &&
-        canUseAi &&
-        (privilegeLevel === PrivilegeLevels.READ_AND_WRITE ||
-          privilegeLevel === PrivilegeLevels.OWNER)
+            showAiErrorAssistant = aiAllowed
+          }
+        } catch (err) {
+          // still allow users to access project if we cant get their permissions, but disable AI feature
+          showAiErrorAssistant = false
+        }
+      }
 
       const template =
         detachRole === 'detached'

+ 64 - 0
services/web/test/unit/src/Authorization/PermissionsManagerTests.js

@@ -30,6 +30,38 @@ describe('PermissionsManager', function () {
     this.PermissionsManager.registerCapability('capability4', {
       default: false,
     })
+    this.PermissionsManager.registerPolicy('openPolicy', {
+      capability1: true,
+      capability2: true,
+    })
+    this.PermissionsManager.registerPolicy('restrictivePolicy', {
+      capability1: true,
+      capability2: false,
+    })
+    this.openPolicyResponseSet = [
+      [
+        {
+          managedUsersEnabled: true,
+          groupPolicy: { openPolicy: true },
+        },
+        {
+          managedUsersEnabled: true,
+          groupPolicy: { openPolicy: true },
+        },
+      ],
+    ]
+    this.restrictivePolicyResponseSet = [
+      [
+        {
+          managedUsersEnabled: true,
+          groupPolicy: { openPolicy: true },
+        },
+        {
+          managedUsersEnabled: true,
+          groupPolicy: { restrictivePolicy: true },
+        },
+      ],
+    ]
   })
 
   describe('hasPermission', function () {
@@ -717,4 +749,36 @@ describe('PermissionsManager', function () {
       })
     })
   })
+
+  describe('checkUserListPermissions', function () {
+    it('should return true when all users have permissions required', async function () {
+      const userList = ['user1', 'user2', 'user3']
+      const capabilities = ['capability1', 'capability2']
+      this.hooksFire.onCall(0).resolves(this.openPolicyResponseSet)
+      this.hooksFire.onCall(1).resolves(this.openPolicyResponseSet)
+      this.hooksFire.onCall(2).resolves(this.openPolicyResponseSet)
+
+      const usersHavePermission =
+        await this.PermissionsManager.promises.checkUserListPermissions(
+          userList,
+          capabilities
+        )
+      expect(usersHavePermission).to.equal(true)
+    })
+
+    it('should return false if any user does not have permission', async function () {
+      const userList = ['user1', 'user2', 'user3']
+      const capabilities = ['capability1', 'capability2']
+      this.hooksFire.onCall(0).resolves(this.openPolicyResponseSet)
+      this.hooksFire.onCall(1).resolves(this.restrictivePolicyResponseSet)
+      this.hooksFire.onCall(2).resolves(this.openPolicyResponseSet)
+
+      const usersHavePermission =
+        await this.PermissionsManager.promises.checkUserListPermissions(
+          userList,
+          capabilities
+        )
+      expect(usersHavePermission).to.equal(false)
+    })
+  })
 })