Merge pull request #13694 from overleaf/revert-13584-bg-managed-users-block-delete-own-account

Revert "block account deletion by managed users"

GitOrigin-RevId: ece8024b2fac16066abd36af9a9670ba483b3628

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

@@ -1,38 +1,7 @@
 const { ForbiddenError } = require('../Errors/Errors')
-const { hasPermission, getUserCapabilities } = require('./PermissionsManager')
+const { hasPermission } = 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)
-      // if there is no group policy, the user is not managed
-      if (!groupPolicy) {
-        return next()
-      }
-      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.
@@ -76,5 +45,4 @@ 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 `userCannotHaveSecondaryEmail` policy would check whether the user has
+ * the `userCannotAddSecondaryEmail` 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
- *       "userCannotHaveSecondaryEmail": false // not enforced
+ *       "userCannotAddSecondaryEmail": false // not enforced
  *     }
  */
 

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

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

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

@@ -1,5 +1,4 @@
 const AuthenticationController = require('../Authentication/AuthenticationController')
-const PermissionsController = require('../Authorization/PermissionsController')
 const SubscriptionController = require('./SubscriptionController')
 const SubscriptionGroupController = require('./SubscriptionGroupController')
 const TeamInvitesController = require('./TeamInvitesController')
@@ -59,7 +58,6 @@ module.exports = {
     webRouter.delete(
       '/subscription/group/user',
       AuthenticationController.requireLogin(),
-      PermissionsController.requirePermission('leave-group-subscription'),
       SubscriptionGroupController.removeSelfFromGroup
     )
 
@@ -72,7 +70,6 @@ module.exports = {
       '/subscription/invites/:token/',
       AuthenticationController.requireLogin(),
       RateLimiterMiddleware.rateLimit(teamInviteRateLimiter),
-      PermissionsController.requirePermission('join-subscription'),
       TeamInvitesController.acceptInvite
     )
 
@@ -90,7 +87,6 @@ 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,11 +194,6 @@ async function ensureAffiliationMiddleware(req, res, next) {
   } catch (error) {
     return new Errors.UserNotFoundError({ info: { userId } })
   }
-  try {
-    req.assertPermission('add-affiliation')
-  } catch (error) {
-    return next(error)
-  }
   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
-    userCannotHaveSecondaryEmail: Boolean,
+    userCannotAddSecondaryEmail: 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,7 +12,6 @@ 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')
@@ -301,7 +300,6 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
   webRouter.get(
     '/user/emails',
     AuthenticationController.requireLogin(),
-    PermissionsController.useCapabilities(),
     UserController.promises.ensureAffiliationMiddleware,
     UserEmailsController.list
   )
@@ -334,7 +332,6 @@ 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
@@ -353,7 +350,6 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
     webRouter.post(
       '/user/emails/endorse',
       AuthenticationController.requireLogin(),
-      PermissionsController.requirePermission('endorse-email'),
       RateLimiterMiddleware.rateLimit(rateLimiters.endorseEmail),
       UserEmailsController.endorse
     )
@@ -399,7 +395,6 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
     '/user/delete',
     RateLimiterMiddleware.rateLimit(rateLimiters.deleteUser),
     AuthenticationController.requireLogin(),
-    PermissionsController.requirePermission('delete-own-account'),
     UserController.tryDeleteUser
   )
 

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

@@ -928,18 +928,12 @@ 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
@@ -966,18 +960,12 @@ 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
@@ -1004,18 +992,12 @@ 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))
       })