Merge pull request #13717 from overleaf/bg-managed-users-block-affiliations-fix

fix for block affiliations for managed users

GitOrigin-RevId: cac54288592323ea3f1cd7655d4e2b89ee301002

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

@@ -1,7 +1,34 @@
 const { ForbiddenError } = require('../Errors/Errors')
-const { hasPermission } = require('./PermissionsManager')
+const { hasPermission, getUserCapabilities } = require('./PermissionsManager')
 const ManagedUsersHandler = require('../Subscription/ManagedUsersHandler')
 
+/**
+ * Function that returns middleware to add an `assertPermission` function to the request object to check if the user has a specific capability.
+ * @returns {Function} The middleware function that adds the `assertPermission` function to the request object.
+ */
+function useCapabilities() {
+  return async function (req, res, next) {
+    if (!req.user) {
+      return next()
+    }
+    try {
+      // get the group policy applying to the user
+      const groupPolicy =
+        await ManagedUsersHandler.promises.getGroupPolicyForUser(req.user)
+      const capabilitySet = getUserCapabilities(groupPolicy)
+      req.assertPermission = capability => {
+        if (!capabilitySet.has(capability)) {
+          throw new ForbiddenError(
+            `user does not have permission for ${capability}`
+          )
+        }
+      }
+      next()
+    } catch (error) {
+      next(error)
+    }
+  }
+}
 /**
  * Function that returns middleware to check if the user has permission to access a resource.
  * @param {[string]} requiredCapabilities - the capabilities required to access the resource.
@@ -45,4 +72,5 @@ function requirePermission(...requiredCapabilities) {
 
 module.exports = {
   requirePermission,
+  useCapabilities,
 }

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

@@ -26,7 +26,7 @@
  *
  * Validator: a function that takes a user and returns a boolean indicating
  * whether the user satisfies the policy or not. For example, a validator for
- * the `userCannotAddSecondaryEmail` policy would check whether the user has
+ * the `userCannotHaveSecondaryEmail` policy would check whether the user has
  * more than one email address.
  *
  * Group Policies: a collection of policies with a setting indicating whether
@@ -37,7 +37,7 @@
  *
  *     {
  *       "userCannotDeleteOwnAccount": true, // enforced
- *       "userCannotAddSecondaryEmail": false // not enforced
+ *       "userCannotHaveSecondaryEmail": false // not enforced
  *     }
  */
 

services/web/app/src/Features/Subscription/ManagedUsersPolicy.js

@@ -13,6 +13,12 @@ registerCapability('delete-own-account', { default: true })
 // Register the capability for a user to add a secondary email to their account.
 registerCapability('add-secondary-email', { default: true })
 
+// Register the capability for a user to add an affiliation to their account.
+registerCapability('add-affiliation', { default: true })
+
+// Register the capability for a user to endorse an email address.
+registerCapability('endorse-email', { default: true })
+
 // Register the capability for a user to sign in with Google to their account
 registerCapability('link-google-sso', { default: true })
 
@@ -20,11 +26,14 @@ registerCapability('link-google-sso', { default: true })
 registerCapability('link-other-third-party-sso', { default: true })
 
 // Register the capability for a user to leave a managed group subscription.
-registerCapability('leave-managing-group-subscription', { default: true })
+registerCapability('leave-group-subscription', { default: true })
 
 // Register the capability for a user to start a subscription.
 registerCapability('start-subscription', { default: true })
 
+// Register the capability for a user to join a subscription.
+registerCapability('join-subscription', { default: true })
+
 // Register a policy to prevent a user deleting their own account.
 registerPolicy('userCannotDeleteOwnAccount', {
   'delete-own-account': false,
@@ -32,9 +41,11 @@ registerPolicy('userCannotDeleteOwnAccount', {
 
 // Register a policy to prevent a user having secondary email addresses on their account.
 registerPolicy(
-  'userCannotAddSecondaryEmail',
+  'userCannotHaveSecondaryEmail',
   {
     'add-secondary-email': false,
+    'add-affiliation': false,
+    'endorse-email': false,
   },
   {
     validator: async user => {
@@ -46,7 +57,7 @@ registerPolicy(
 
 // Register a policy to prevent a user leaving the group subscription they are managed by.
 registerPolicy('userCannotLeaveManagingGroupSubscription', {
-  'leave-managing-group-subscription': false,
+  'leave-group-subscription': false,
 })
 
 // Register a policy to prevent a user having third-party SSO linked to their account.
@@ -78,7 +89,7 @@ registerPolicy(
 // Register a policy to prevent a user having an active personal subscription.
 registerPolicy(
   'userCannotHaveSubscription',
-  { 'start-subscription': false },
+  { 'start-subscription': false, 'join-subscription': false },
   {
     validator: async user => {
       return !(await SubscriptionLocator.promises.getUserIndividualSubscription(
@@ -101,7 +112,7 @@ registerPolicy(
 function getDefaultPolicy() {
   return {
     userCannotDeleteOwnAccount: true,
-    userCannotAddSecondaryEmail: true,
+    userCannotHaveSecondaryEmail: true,
     userCannotHaveSubscription: true,
     userCannotLeaveManagingGroupSubscription: true,
     userCannotHaveGoogleSSO: false, // we want to allow google SSO by default

services/web/app/src/Features/Subscription/SubscriptionRouter.js

@@ -1,4 +1,5 @@
 const AuthenticationController = require('../Authentication/AuthenticationController')
+const PermissionsController = require('../Authorization/PermissionsController')
 const SubscriptionController = require('./SubscriptionController')
 const SubscriptionGroupController = require('./SubscriptionGroupController')
 const TeamInvitesController = require('./TeamInvitesController')
@@ -58,6 +59,7 @@ module.exports = {
     webRouter.delete(
       '/subscription/group/user',
       AuthenticationController.requireLogin(),
+      PermissionsController.requirePermission('leave-group-subscription'),
       SubscriptionGroupController.removeSelfFromGroup
     )
 
@@ -70,6 +72,7 @@ module.exports = {
       '/subscription/invites/:token/',
       AuthenticationController.requireLogin(),
       RateLimiterMiddleware.rateLimit(teamInviteRateLimiter),
+      PermissionsController.requirePermission('join-subscription'),
       TeamInvitesController.acceptInvite
     )
 
@@ -87,6 +90,7 @@ module.exports = {
     webRouter.post(
       '/user/subscription/create',
       AuthenticationController.requireLogin(),
+      PermissionsController.requirePermission('start-subscription'),
       SubscriptionController.createSubscription
     )
     webRouter.post(

services/web/app/src/Features/User/UserController.js

@@ -194,6 +194,14 @@ async function ensureAffiliationMiddleware(req, res, next) {
   } catch (error) {
     return new Errors.UserNotFoundError({ info: { userId } })
   }
+  // if the user does not have permission to add an affiliation, we skip this middleware
+  try {
+    req.assertPermission('add-affiliation')
+  } catch (error) {
+    if (error instanceof Errors.ForbiddenError) {
+      return next()
+    }
+  }
   try {
     await ensureAffiliation(user)
   } catch (error) {

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

@@ -8,7 +8,7 @@ const GroupPolicySchema = new Schema(
     userCannotDeleteOwnAccount: Boolean,
 
     // User can't add a secondary email address, or affiliation
-    userCannotAddSecondaryEmail: Boolean,
+    userCannotHaveSecondaryEmail: Boolean,
 
     // User can't have an active  (currently auto-renewing) personal subscription, nor can they start one
     userCannotHaveSubscription: Boolean,

services/web/app/src/router.js

@@ -12,6 +12,7 @@ const UploadsRouter = require('./Features/Uploads/UploadsRouter')
 const metrics = require('@overleaf/metrics')
 const ReferalController = require('./Features/Referal/ReferalController')
 const AuthenticationController = require('./Features/Authentication/AuthenticationController')
+const PermissionsController = require('./Features/Authorization/PermissionsController')
 const SessionManager = require('./Features/Authentication/SessionManager')
 const TagsController = require('./Features/Tags/TagsController')
 const NotificationsController = require('./Features/Notifications/NotificationsController')
@@ -300,6 +301,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
   webRouter.get(
     '/user/emails',
     AuthenticationController.requireLogin(),
+    PermissionsController.useCapabilities(),
     UserController.promises.ensureAffiliationMiddleware,
     UserEmailsController.list
   )
@@ -332,6 +334,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
     webRouter.post(
       '/user/emails',
       AuthenticationController.requireLogin(),
+      PermissionsController.requirePermission('add-secondary-email'),
       RateLimiterMiddleware.rateLimit(rateLimiters.addEmail),
       CaptchaMiddleware.validateCaptcha('addEmail'),
       UserEmailsController.add
@@ -350,6 +353,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
     webRouter.post(
       '/user/emails/endorse',
       AuthenticationController.requireLogin(),
+      PermissionsController.requirePermission('endorse-email'),
       RateLimiterMiddleware.rateLimit(rateLimiters.endorseEmail),
       UserEmailsController.endorse
     )
@@ -395,6 +399,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
     '/user/delete',
     RateLimiterMiddleware.rateLimit(rateLimiters.deleteUser),
     AuthenticationController.requireLogin(),
+    PermissionsController.requirePermission('delete-own-account'),
     UserController.tryDeleteUser
   )
 

services/web/test/acceptance/src/helpers/User.js

@@ -205,6 +205,10 @@ class User {
     )
   }
 
+  setEmails(emails, callback) {
+    UserModel.updateOne({ _id: this.id }, { emails }, callback)
+  }
+
   logout(callback) {
     this.getCsrfToken(error => {
       if (error != null) {

services/web/test/unit/src/User/UserControllerTests.js

@@ -928,12 +928,18 @@ describe('UserController', function () {
         ]
         this.Features.hasFeature.withArgs('affiliations').returns(true)
         this.req.query.ensureAffiliation = true
+        this.req.assertPermission = sinon.stub()
         await this.UserController.promises.ensureAffiliationMiddleware(
           this.req,
           this.res,
           this.next
         )
       })
+      it('should check the user has permission', function () {
+        expect(this.req.assertPermission).to.have.been.calledWith(
+          'add-affiliation'
+        )
+      })
       it('should unflag the emails but not confirm', function () {
         expect(
           this.UserUpdater.promises.addAffiliationForNewUser
@@ -960,12 +966,18 @@ describe('UserController', function () {
         ]
         this.Features.hasFeature.withArgs('affiliations').returns(true)
         this.req.query.ensureAffiliation = true
+        this.req.assertPermission = sinon.stub()
         await this.UserController.promises.ensureAffiliationMiddleware(
           this.req,
           this.res,
           this.next
         )
       })
+      it('should check the user has permission', function () {
+        expect(this.req.assertPermission).to.have.been.calledWith(
+          'add-affiliation'
+        )
+      })
       it('should add affiliation to v1, unflag and confirm on v2', function () {
         expect(this.UserUpdater.promises.addAffiliationForNewUser).to.have.not
           .been.called
@@ -992,12 +1004,18 @@ describe('UserController', function () {
         ]
         this.Features.hasFeature.withArgs('affiliations').returns(true)
         this.req.query.ensureAffiliation = true
+        this.req.assertPermission = sinon.stub()
         await this.UserController.promises.ensureAffiliationMiddleware(
           this.req,
           this.res,
           this.next
         )
       })
+      it('should check the user has permission', function () {
+        expect(this.req.assertPermission).to.have.been.calledWith(
+          'add-affiliation'
+        )
+      })
       it('should return the error', function () {
         expect(this.next).to.be.calledWith(sinon.match.instanceOf(Error))
       })