Merge pull request #18784 from overleaf/bg-allow-combined-group-policies

allow combined group policies

GitOrigin-RevId: b23fb0454f794e9094e8e15e732b4322a48ac1ee

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

@@ -2,6 +2,8 @@ const { ForbiddenError, UserNotFoundError } = require('../Errors/Errors')
 const {
   getUserCapabilities,
   getUserRestrictions,
+  combineGroupPolicies,
+  combineAllowedProperties,
 } = require('./PermissionsManager')
 const { checkUserPermissions } = require('./PermissionsManager').promises
 const Modules = require('../../infrastructure/Modules')
@@ -27,25 +29,29 @@ function useCapabilities() {
       return next()
     }
     try {
-      const result = (
+      let results = await Modules.promises.hooks.fire(
-        await Modules.promises.hooks.fire(
+        'getGroupPolicyForUser',
-          'getManagedUsersEnrollmentForUser',
+        req.user
-          req.user
+      )
-        )
+      // merge array of all results from all modules
-      )[0]
+      results = results.flat()
-      if (result) {
+
-        // get the group policy applying to the user
+      if (results.length > 0) {
-        const { groupPolicy, managedBy, isManagedGroupAdmin } = result
+        // get the combined group policy applying to the user
-        // attach the subscription ID to the request object
+        const groupPolicies = results.map(result => result.groupPolicy)
-        req.managedBy = managedBy
+        const combinedGroupPolicy = combineGroupPolicies(groupPolicies)
-        // attach the subscription admin status to the request object
-        req.isManagedGroupAdmin = isManagedGroupAdmin
         // attach the new capabilities to the request object
-        for (const cap of getUserCapabilities(groupPolicy)) {
+        for (const cap of getUserCapabilities(combinedGroupPolicy)) {
           req.capabilitySet.add(cap)
         }
         // also attach the user's restrictions (the capabilities they don't have)
-        req.userRestrictions = getUserRestrictions(groupPolicy)
+        req.userRestrictions = getUserRestrictions(combinedGroupPolicy)
+
+        // attach allowed properties to the request object
+        const allowedProperties = combineAllowedProperties(results)
+        for (const [prop, value] of Object.entries(allowedProperties)) {
+          req[prop] = value
+        }
       }
       next()
     } catch (error) {

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

@@ -48,6 +48,7 @@ const Modules = require('../../infrastructure/Modules')
 const POLICY_TO_CAPABILITY_MAP = new Map()
 const POLICY_TO_VALIDATOR_MAP = new Map()
 const DEFAULT_PERMISSIONS = new Map()
+const ALLOWED_PROPERTIES = new Set()
 
 /**
  * Throws an error if the given capability is not registered.
@@ -126,6 +127,24 @@ function registerPolicy(name, capabilities, options = {}) {
   }
 }
 
+/**
+ * Registers an allowed property that can be added to the request object.
+ *
+ * @param {string} name - The name of the property to register.
+ * @returns {void}
+ */
+function registerAllowedProperty(name) {
+  ALLOWED_PROPERTIES.add(name)
+}
+
+/**
+ * returns the set of allowed properties that have been registered
+ *
+ * @returns {Set} ALLOWED_PROPERTIES
+ */
+function getAllowedProperties() {
+  return ALLOWED_PROPERTIES
+}
 /**
  * Returns an array of policy names that are enforced based on the provided
  * group policy object.
@@ -240,6 +259,41 @@ function getUserCapabilities(groupPolicy) {
   return userCapabilities
 }
 
+/**
+ * Combines an array of group policies into a single policy object.
+ *
+ * @param {Array} groupPolicies - An array of group policies.
+ * @returns {Object} - The combined group policy object.
+ */
+function combineGroupPolicies(groupPolicies) {
+  const combinedGroupPolicy = {}
+  for (const groupPolicy of groupPolicies) {
+    const enforcedPolicyNames = getEnforcedPolicyNames(groupPolicy)
+    for (const enforcedPolicyName of enforcedPolicyNames) {
+      combinedGroupPolicy[enforcedPolicyName] = true
+    }
+  }
+  return combinedGroupPolicy
+}
+
+/**
+ * Combines the allowed properties from an array of property objects.
+ *
+ * @param {Array<Object>} propertyObjects - An array of property objects.
+ * @returns {Object} - An object containing the combined allowed properties.
+ */
+function combineAllowedProperties(propertyObjects) {
+  const userProperties = {}
+  for (const properties of propertyObjects) {
+    for (const [key, value] of Object.entries(properties)) {
+      if (ALLOWED_PROPERTIES.has(key)) {
+        userProperties[key] ??= value
+      }
+    }
+  }
+  return userProperties
+}
+
 /**
  * Returns a set of capabilities that a user does not have based on their group policy.
  *
@@ -317,6 +371,7 @@ async function getUserValidationStatus({ user, groupPolicy, subscription }) {
 
 /**
  * Checks if a user has permission for a given set of capabilities
+ *  as set out in both their current group subscription, and any institutions they are affiliated with
  *
  * @param {Object} user - The user object to retrieve the group policy for.
  *   Only the user's _id is required
@@ -325,21 +380,17 @@ async function getUserValidationStatus({ user, groupPolicy, subscription }) {
  * @throws {Error} If the user does not have permission
  */
 async function checkUserPermissions(user, requiredCapabilities) {
-  const result =
+  let results = await Modules.promises.hooks.fire('getGroupPolicyForUser', user)
-    (
+  results = results.flat()
-      await Modules.promises.hooks.fire(
+
-        'getManagedUsersEnrollmentForUser',
+  if (!results?.length) return
-        user
+
-      )
+  // get the combined group policy applying to the user
-    )[0] || {}
+  const groupPolicies = results.map(result => result.groupPolicy)
-  const { groupPolicy, managedUsersEnabled } = result
+  const combinedGroupPolicy = combineGroupPolicies(groupPolicies)
-  if (!managedUsersEnabled) {
-    return
-  }
-  // check that the user has all the required capabilities
   for (const requiredCapability of requiredCapabilities) {
     // if the user has the permission, continue
-    if (!hasPermission(groupPolicy, requiredCapability)) {
+    if (!hasPermission(combinedGroupPolicy, requiredCapability)) {
       throw new ForbiddenError(
         `user does not have permission for ${requiredCapability}`
       )
@@ -350,6 +401,10 @@ async function checkUserPermissions(user, requiredCapabilities) {
 module.exports = {
   registerCapability,
   registerPolicy,
+  registerAllowedProperty,
+  combineGroupPolicies,
+  combineAllowedProperties,
+  getAllowedProperties,
   hasPermission,
   getUserCapabilities,
   getUserRestrictions,

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

@@ -21,6 +21,9 @@ const GroupPolicySchema = new Schema(
 
     // User can't have other third-party SSO (e.g. ORCID/IEEE) active on their account, nor can they link it to their account
     userCannotHaveOtherThirdPartySSO: Boolean,
+
+    // User can't use any of our AI features, such as the compile-assistant
+    userCannotUseAIFeatures: Boolean,
   },
   { minimize: false }
 )

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

@@ -14,6 +14,7 @@ const InstitutionSchema = new Schema(
       optedOutUserIds: [{ type: ObjectId, ref: 'User' }],
       lastSent: { type: Date },
     },
+    groupPolicy: { type: ObjectId, ref: 'GroupPolicy' },
   },
   { minimize: false }
 )

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

@@ -156,6 +156,7 @@ const UserSchema = new Schema(
           zotero: { type: Boolean },
           referencesSearch: { type: Boolean },
           symbolPalette: { type: Boolean },
+          compileAssistant: { type: Boolean },
         },
       },
     ],

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

@@ -12,7 +12,7 @@ describe('PermissionsManager', function () {
         '../../infrastructure/Modules': (this.Modules = {
           promises: {
             hooks: {
-              fire: (this.hooksFire = sinon.stub().resolves([{}])),
+              fire: (this.hooksFire = sinon.stub().resolves([[]])),
             },
           },
         }),
@@ -399,7 +399,6 @@ describe('PermissionsManager', function () {
       )
     })
   })
-
   describe('checkUserPermissions', function () {
     describe('allowed', function () {
       it('should not error when managedUsersEnabled is not enabled for user', async function () {
@@ -416,10 +415,12 @@ describe('PermissionsManager', function () {
           default: true,
         })
         this.hooksFire.resolves([
-          {
+          [
-            managedUsersEnabled: true,
+            {
-            groupPolicy: {},
+              managedUsersEnabled: true,
-          },
+              groupPolicy: {},
+            },
+          ],
         ])
         const result =
           await this.PermissionsManager.promises.checkUserPermissions(
@@ -437,12 +438,14 @@ describe('PermissionsManager', function () {
           'some-policy-to-check': true,
         })
         this.hooksFire.resolves([
-          {
+          [
-            managedUsersEnabled: true,
+            {
-            groupPolicy: {
+              managedUsersEnabled: true,
-              userCanDoSomePolicy: true,
+              groupPolicy: {
+                userCanDoSomePolicy: true,
+              },
             },
-          },
+          ],
         ])
         const result =
           await this.PermissionsManager.promises.checkUserPermissions(
@@ -455,7 +458,7 @@ describe('PermissionsManager', function () {
 
     describe('not allowed', function () {
       it('should return error when managedUsersEnabled is enabled for user but there is no group policy', async function () {
-        this.hooksFire.resolves([{ managedUsersEnabled: true }])
+        this.hooksFire.resolves([[{ managedUsersEnabled: true }]])
         await expect(
           this.PermissionsManager.promises.checkUserPermissions(
             { _id: 'user123' },
@@ -469,10 +472,12 @@ describe('PermissionsManager', function () {
           default: false,
         })
         this.hooksFire.resolves([
-          {
+          [
-            managedUsersEnabled: true,
+            {
-            groupPolicy: {},
+              managedUsersEnabled: true,
-          },
+              groupPolicy: {},
+            },
+          ],
         ])
         await expect(
           this.PermissionsManager.promises.checkUserPermissions(
@@ -490,10 +495,12 @@ describe('PermissionsManager', function () {
           'some-policy-to-check': false,
         })
         this.hooksFire.resolves([
-          {
+          [
-            managedUsersEnabled: true,
+            {
-            groupPolicy: { userCannotDoSomePolicy: true },
+              managedUsersEnabled: true,
-          },
+              groupPolicy: { userCannotDoSomePolicy: true },
+            },
+          ],
         ])
         await expect(
           this.PermissionsManager.promises.checkUserPermissions(
@@ -504,4 +511,210 @@ describe('PermissionsManager', function () {
       })
     })
   })
+
+  describe('registerAllowedProperty', function () {
+    it('allows us to register a property', async function () {
+      this.PermissionsManager.registerAllowedProperty('metadata1')
+      const result = await this.PermissionsManager.getAllowedProperties()
+      expect(result).to.deep.equal(new Set(['metadata1']))
+    })
+
+    // used if multiple modules would require the same prop, since we dont know which will load first, both must register
+    it('should handle multiple registrations of the same property', async function () {
+      this.PermissionsManager.registerAllowedProperty('metadata1')
+      this.PermissionsManager.registerAllowedProperty('metadata1')
+      const result = await this.PermissionsManager.getAllowedProperties()
+      expect(result).to.deep.equal(new Set(['metadata1']))
+    })
+  })
+
+  describe('combineAllowedProperties', function () {
+    it('should handle multiple occurences of the same property, preserving the first occurence', async function () {
+      const policy1 = {
+        groupPolicy: {
+          policy: false,
+        },
+        prop1: 'some other value here',
+      }
+      const policy2 = {
+        groupPolicy: {
+          policy: false,
+        },
+        prop1: 'some value here',
+      }
+
+      const results = [policy1, policy2]
+      this.PermissionsManager.registerAllowedProperty('prop1')
+
+      const combinedProps =
+        this.PermissionsManager.combineAllowedProperties(results)
+
+      expect(combinedProps).to.deep.equal({
+        prop1: 'some other value here',
+      })
+    })
+
+    it('should add registered properties to the set', async function () {
+      const policy = {
+        groupPolicy: {
+          policy: false,
+        },
+        prop1: 'some value here',
+        propNotMeThough: 'dont copy please',
+      }
+
+      const policy2 = {
+        groupPolicy: {
+          policy: false,
+        },
+        prop2: 'some value here',
+      }
+
+      const results = [policy, policy2]
+      this.PermissionsManager.registerAllowedProperty('prop1')
+      this.PermissionsManager.registerAllowedProperty('prop2')
+
+      const combinedProps =
+        this.PermissionsManager.combineAllowedProperties(results)
+
+      expect(combinedProps).to.deep.equal({
+        prop1: 'some value here',
+        prop2: 'some value here',
+      })
+    })
+
+    it('should not add unregistered properties to the req object', async function () {
+      const policy = {
+        groupPolicy: {
+          policy: false,
+        },
+        prop1: 'some value here',
+      }
+
+      const policy2 = {
+        groupPolicy: {
+          policy: false,
+        },
+        prop2: 'some value here',
+      }
+      this.PermissionsManager.registerAllowedProperty('prop1')
+
+      const results = [policy, policy2]
+
+      const combinedProps =
+        this.PermissionsManager.combineAllowedProperties(results)
+
+      expect(combinedProps).to.deep.equal({ prop1: 'some value here' })
+    })
+
+    it('should handle an empty array', async function () {
+      const results = []
+
+      const combinedProps =
+        this.PermissionsManager.combineAllowedProperties(results)
+
+      expect(combinedProps).to.deep.equal({})
+    })
+  })
+
+  describe('combineGroupPolicies', function () {
+    it('should return an empty object when an empty array is passed', async function () {
+      const results = []
+
+      const combinedPolicy =
+        this.PermissionsManager.combineGroupPolicies(results)
+      expect(combinedPolicy).to.deep.equal({})
+    })
+
+    it('should combine multiple group policies into a single policy object', async function () {
+      const groupPolicy = {
+        policy1: true,
+      }
+
+      const groupPolicy2 = {
+        policy2: false,
+        policy3: true,
+      }
+      this.PermissionsManager.registerAllowedProperty('prop1')
+
+      const results = [groupPolicy, groupPolicy2]
+
+      const combinedPolicy =
+        this.PermissionsManager.combineGroupPolicies(results)
+
+      expect(combinedPolicy).to.deep.equal({
+        policy1: true,
+        policy3: true,
+      })
+    })
+
+    it('should handle duplicate enforced policies across different group policies', async function () {
+      const groupPolicy = {
+        policy1: false,
+        policy2: true,
+      }
+
+      const groupPolicy2 = {
+        policy2: true,
+        policy3: true,
+      }
+      this.PermissionsManager.registerAllowedProperty('prop1')
+
+      const results = [groupPolicy, groupPolicy2]
+
+      const combinedPolicy =
+        this.PermissionsManager.combineGroupPolicies(results)
+
+      expect(combinedPolicy).to.deep.equal({
+        policy2: true,
+        policy3: true,
+      })
+    })
+
+    it('should handle group policies with no enforced policies', async function () {
+      const groupPolicy = {
+        policy1: false,
+        policy2: false,
+      }
+
+      const groupPolicy2 = {
+        policy2: false,
+        policy3: true,
+      }
+      this.PermissionsManager.registerAllowedProperty('prop1')
+
+      const results = [groupPolicy, groupPolicy2]
+
+      const combinedPolicy =
+        this.PermissionsManager.combineGroupPolicies(results)
+
+      expect(combinedPolicy).to.deep.equal({ policy3: true })
+    })
+
+    it('should choose the stricter option between two policy values', async function () {
+      const groupPolicy = {
+        policy1: false,
+        policy2: true,
+        policy4: true,
+      }
+
+      const groupPolicy2 = {
+        policy2: false,
+        policy3: true,
+        policy4: false,
+      }
+      this.PermissionsManager.registerAllowedProperty('prop1')
+
+      const results = [groupPolicy, groupPolicy2]
+
+      const combinedPolicy =
+        this.PermissionsManager.combineGroupPolicies(results)
+
+      expect(combinedPolicy).to.deep.equal({
+        policy2: true,
+        policy3: true,
+        policy4: true,
+      })
+    })
+  })
 })