From 35906b4018fb31f9e89f4bc80ab0b419818f8263 Mon Sep 17 00:00:00 2001
From: Copilot <198982749+Copilot@users.noreply.github.com>
Date: Thu, 5 Feb 2026 09:28:52 +0100
Subject: [PATCH] Deduplicate users in checkUserListPermissions to avoid
 redundant permission checks (#29461)

* Fix duplicate permission checks for same user

Deduplicate user list in checkUserListPermissions before running expensive checks.
Handles ObjectId vs string comparison by converting to string.
Adds tests to verify deduplication works correctly.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thomas- <2176518+thomas-@users.noreply.github.com>
GitOrigin-RevId: 96eede1cbeb18b807deaca7d4c370aef5c48c4bc
---
 .../Authorization/PermissionsManager.mjs      |  7 +++-
 .../Authorization/PermissionsManager.test.mjs | 38 ++++++++++++++++++-
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/services/web/app/src/Features/Authorization/PermissionsManager.mjs b/services/web/app/src/Features/Authorization/PermissionsManager.mjs
index f35d8c4469..85384810ed 100644
--- a/services/web/app/src/Features/Authorization/PermissionsManager.mjs
+++ b/services/web/app/src/Features/Authorization/PermissionsManager.mjs
@@ -42,6 +42,7 @@
  */
 
 import { callbackify } from 'node:util'
+import _ from 'lodash'
 
 import Errors from '../Errors/Errors.js'
 import Modules from '../../infrastructure/Modules.mjs'
@@ -459,7 +460,11 @@ async function checkUserPermissions(user, requiredCapabilities) {
  * @returns {Promise<boolean>} - A promise that resolves to `true` if all collaborators have the specified capability, otherwise `false`.
  */
 async function checkUserListPermissions(userList, capabilities) {
-  for (const user of userList) {
+  // Deduplicate the user list to avoid checking the same user multiple times
+  // Convert _id to string for comparison since it can be ObjectId or string
+  const uniqueUsers = _.uniqBy(userList, user => String(user._id))
+
+  for (const user of uniqueUsers) {
     // mimic a user object with only id, since we need it to fetch permissions
     const allowed = await checkUserPermissions(user, capabilities)
     if (!allowed) {
diff --git a/services/web/test/unit/src/Authorization/PermissionsManager.test.mjs b/services/web/test/unit/src/Authorization/PermissionsManager.test.mjs
index b14506c99c..125120263b 100644
--- a/services/web/test/unit/src/Authorization/PermissionsManager.test.mjs
+++ b/services/web/test/unit/src/Authorization/PermissionsManager.test.mjs
@@ -797,7 +797,7 @@ describe('PermissionsManager', function () {
 
   describe('checkUserListPermissions', function () {
     it('should return true when all users have permissions required', async function (ctx) {
-      const userList = ['user1', 'user2', 'user3']
+      const userList = [{ _id: 'user1' }, { _id: 'user2' }, { _id: 'user3' }]
       const capabilities = ['capability1', 'capability2']
       ctx.hooksFire.onCall(0).resolves(ctx.openPolicyResponseSet)
       ctx.hooksFire.onCall(1).resolves(ctx.openPolicyResponseSet)
@@ -812,7 +812,7 @@ describe('PermissionsManager', function () {
     })
 
     it('should return false if any user does not have permission', async function (ctx) {
-      const userList = ['user1', 'user2', 'user3']
+      const userList = [{ _id: 'user1' }, { _id: 'user2' }, { _id: 'user3' }]
       const capabilities = ['capability1', 'capability2']
       ctx.hooksFire.onCall(0).resolves(ctx.openPolicyResponseSet)
       ctx.hooksFire.onCall(1).resolves(ctx.restrictivePolicyResponseSet)
@@ -825,5 +825,39 @@ describe('PermissionsManager', function () {
         )
       expect(usersHavePermission).to.equal(false)
     })
+
+    it('should deduplicate users with duplicate IDs', async function (ctx) {
+      const userList = [{ _id: 'user1' }, { _id: 'user1' }, { _id: 'user2' }]
+      const capabilities = ['capability1', 'capability2']
+      ctx.hooksFire.onCall(0).resolves(ctx.openPolicyResponseSet)
+      ctx.hooksFire.onCall(1).resolves(ctx.openPolicyResponseSet)
+
+      const usersHavePermission =
+        await ctx.PermissionsManager.promises.checkUserListPermissions(
+          userList,
+          capabilities
+        )
+      expect(usersHavePermission).to.equal(true)
+      // Should only call hooksFire twice (once per unique user), not three times
+      expect(ctx.hooksFire.callCount).to.equal(2)
+    })
+
+    it('should deduplicate users when _id is ObjectId vs string', async function (ctx) {
+      const mongodb = await import('mongodb-legacy')
+      const ObjectId = mongodb.default.ObjectId
+      const userId = new ObjectId()
+      const userList = [{ _id: userId }, { _id: userId.toString() }]
+      const capabilities = ['capability1', 'capability2']
+      ctx.hooksFire.onCall(0).resolves(ctx.openPolicyResponseSet)
+
+      const usersHavePermission =
+        await ctx.PermissionsManager.promises.checkUserListPermissions(
+          userList,
+          capabilities
+        )
+      expect(usersHavePermission).to.equal(true)
+      // Should only call hooksFire once since both users are the same
+      expect(ctx.hooksFire.callCount).to.equal(1)
+    })
   })
 })