Quellcode durchsuchen

Merge pull request #13907 from overleaf/bg-managed-users-allow-cancelled-subscriptions

allow cancelled subscriptions for managed users

GitOrigin-RevId: 56262ce4bd4cc93d4e5ea92222c76a874d6cad1e
Brian Gough vor 3 Jahren
Ursprung
Commit
f3ac016566

+ 8 - 4
services/web/app/src/Features/Authorization/PermissionsController.js

@@ -27,8 +27,12 @@ function useCapabilities() {
     }
     try {
       // get the group policy applying to the user
-      const groupPolicy =
-        await ManagedUsersHandler.promises.getGroupPolicyForUser(req.user)
+      const { groupPolicy, managedBy, isManagedGroupAdmin } =
+        await ManagedUsersHandler.promises.getEnrollmentForUser(req.user)
+      // attach the subscription ID to the request object
+      req.managedBy = managedBy
+      // 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)) {
         req.capabilitySet.add(cap)
@@ -66,8 +70,8 @@ function requirePermission(...requiredCapabilities) {
     }
     try {
       // get the group policy applying to the user
-      const groupPolicy =
-        await ManagedUsersHandler.promises.getGroupPolicyForUser(req.user)
+      const { groupPolicy } =
+        await ManagedUsersHandler.promises.getEnrollmentForUser(req.user)
       // if there is no group policy, the user is not managed
       if (!groupPolicy) {
         return next()

+ 15 - 6
services/web/app/src/Features/Subscription/ManagedUsersHandler.js

@@ -42,9 +42,9 @@ async function enableManagedUsers(subscriptionId) {
  * @function
  * @param {Object} user - The user object to retrieve the group policy for.
  * @returns {Promise<Object>} - A Promise that resolves with the group policy
- *   object for the user's enrollment, or undefined if it does not exist.
+ *   and subscription objects for the user's enrollment, or null if it does not exist.
  */
-async function getGroupPolicyForUser(requestedUser) {
+async function getEnrollmentForUser(requestedUser) {
   // Don't rely on the user being populated, it may be a session user without
   // the enrollment property. Force the user to be loaded from mongo.
   const user = await User.findById(requestedUser._id, 'enrollment')
@@ -53,7 +53,7 @@ async function getGroupPolicyForUser(requestedUser) {
   }
   // Now we are sure the user exists and we have the full contents
   if (user.enrollment?.managedBy == null) {
-    return
+    return {}
   }
   // retrieve the subscription and the group policy (without the _id field)
   const subscription = await Subscription.findById(user.enrollment.managedBy)
@@ -64,11 +64,20 @@ async function getGroupPolicyForUser(requestedUser) {
       info: { subscriptionId: user.enrollment.managedBy, userId: user._id },
     })
   }
+
+  // check whether the user is an admin of the subscription
+  const isManagedGroupAdmin = user._id.equals(subscription.admin_id)
+
   // return the group policy as a plain object (without the __v field)
   const groupPolicy = subscription.groupPolicy.toObject({
     versionKey: false,
   })
-  return groupPolicy
+
+  return {
+    groupPolicy,
+    managedBy: user.enrollment.managedBy,
+    isManagedGroupAdmin,
+  }
 }
 
 async function enrollInSubscription(userId, subscription) {
@@ -109,10 +118,10 @@ async function enrollInSubscription(userId, subscription) {
 module.exports = {
   promises: {
     enableManagedUsers,
-    getGroupPolicyForUser,
+    getEnrollmentForUser,
     enrollInSubscription,
   },
   enableManagedUsers: callbackify(enableManagedUsers),
-  getGroupPolicyForUser: callbackify(getGroupPolicyForUser),
+  getEnrollmentForUser: callbackify(getEnrollmentForUser),
   enrollInSubscription: callbackify(enrollInSubscription),
 }

+ 44 - 6
services/web/app/src/Features/Subscription/ManagedUsersPolicy.js

@@ -4,6 +4,7 @@ const {
 } = require('../Authorization/PermissionsManager')
 const { getUsersSubscription, getGroupSubscriptionsMemberOf } =
   require('./SubscriptionLocator').promises
+const { subscriptionIsCanceledOrExpired } = require('./RecurlyClient')
 
 // This file defines the capabilities and policies that are used to
 // determine what managed users can and cannot do.
@@ -35,6 +36,9 @@ registerCapability('start-subscription', { default: true })
 // Register the capability for a user to join a subscription.
 registerCapability('join-subscription', { default: true })
 
+// Register the capability for a user to reactivate a subscription.
+registerCapability('reactivate-subscription', { default: true })
+
 // Register a policy to prevent a user deleting their own account.
 registerPolicy('userCannotDeleteOwnAccount', {
   'delete-own-account': false,
@@ -91,20 +95,54 @@ registerPolicy(
 // being a member of another group subscription.
 registerPolicy(
   'userCannotHaveSubscription',
-  { 'start-subscription': false, 'join-subscription': false },
+  {
+    'start-subscription': false,
+    'join-subscription': false,
+    'reactivate-subscription': false,
+  },
   {
     validator: async ({ user, subscription }) => {
       const usersSubscription = await getUsersSubscription(user)
-      const userHasSubscription = Boolean(usersSubscription)
+
+      // The user can be enrolled if:
+      // 1. they do not have a subscription and are not a member of another subscription (apart from the managed group subscription)
+      // 2. they have a subscription and it is canceled or expired
+      // 3. they have a subscription and it is the subscription they are trying to join as a managed user
+      // The last case is to allow the admin to join their own subscription as a managed user
+
+      const userHasSubscription =
+        Boolean(usersSubscription) &&
+        !subscriptionIsCanceledOrExpired(usersSubscription)
+
+      const userIsThisGroupAdmin =
+        Boolean(usersSubscription) &&
+        usersSubscription._id.toString() === subscription._id.toString()
+
       const userMemberOfSubscriptions = await getGroupSubscriptionsMemberOf(
         user
       )
-      // filter out the subscription of the managed group itself
-      // the user will always be a member of this subscription
+
       const isMemberOfOtherSubscriptions = userMemberOfSubscriptions.some(
-        sub => sub._id.toString() !== subscription._id.toString()
+        sub => {
+          // ignore the subscription of the managed group itself
+          if (sub._id.toString() === subscription._id.toString()) {
+            return false
+          }
+          // ignore the user's own subscription
+          if (
+            usersSubscription &&
+            sub._id.toString() === usersSubscription._id.toString()
+          ) {
+            return false
+          }
+          return true
+        }
+      )
+
+      return (
+        (!userHasSubscription || userIsThisGroupAdmin) &&
+        !isMemberOfOtherSubscriptions
       )
-      return !userHasSubscription && !isMemberOfOtherSubscriptions
     },
   }
 )

+ 6 - 0
services/web/app/src/Features/Subscription/RecurlyClient.js

@@ -90,6 +90,11 @@ async function cancelSubscriptionByUuid(subscriptionUuid) {
   }
 }
 
+function subscriptionIsCanceledOrExpired(subscription) {
+  const state = subscription?.recurlyStatus?.state
+  return state === 'canceled' || state === 'expired'
+}
+
 module.exports = {
   errors: recurly.errors,
 
@@ -102,6 +107,7 @@ module.exports = {
   removeSubscriptionChangeByUuid: callbackify(removeSubscriptionChangeByUuid),
   reactivateSubscriptionByUuid: callbackify(reactivateSubscriptionByUuid),
   cancelSubscriptionByUuid: callbackify(cancelSubscriptionByUuid),
+  subscriptionIsCanceledOrExpired,
 
   promises: {
     getSubscription,

+ 10 - 0
services/web/app/src/Features/Subscription/SubscriptionController.js

@@ -574,6 +574,16 @@ function updateAccountEmailAddress(req, res, next) {
 function reactivateSubscription(req, res, next) {
   const user = SessionManager.getSessionUser(req.session)
   logger.debug({ userId: user._id }, 'reactivating subscription')
+  try {
+    if (req.isManagedGroupAdmin) {
+      // allow admins to reactivate subscriptions
+    } else {
+      // otherwise require the user to have the reactivate-subscription permission
+      req.assertPermission('reactivate-subscription')
+    }
+  } catch (error) {
+    return next(error)
+  }
   SubscriptionHandler.reactivateSubscription(user, function (err) {
     if (err) {
       OError.tag(err, 'something went wrong reactivating subscription', {

+ 1 - 0
services/web/app/src/Features/Subscription/SubscriptionRouter.js

@@ -111,6 +111,7 @@ module.exports = {
     webRouter.post(
       '/user/subscription/reactivate',
       AuthenticationController.requireLogin(),
+      PermissionsController.useCapabilities(),
       SubscriptionController.reactivateSubscription
     )
 

+ 2 - 2
services/web/test/acceptance/src/helpers/Subscription.js

@@ -66,8 +66,8 @@ class Subscription {
     ManagedUsersHandler.enableManagedUsers(this._id, callback)
   }
 
-  getGroupPolicyForUser(user, callback) {
-    ManagedUsersHandler.getGroupPolicyForUser(user, callback)
+  getEnrollmentForUser(user, callback) {
+    ManagedUsersHandler.getEnrollmentForUser(user, callback)
   }
 
   getCapabilities(groupPolicy) {

+ 67 - 17
services/web/test/unit/src/Subscription/SubscriptionControllerTests.js

@@ -653,26 +653,76 @@ describe('SubscriptionController', function () {
   })
 
   describe('reactivateSubscription', function () {
-    beforeEach(function (done) {
-      this.res = {
-        redirect() {
-          done()
-        },
-      }
-      sinon.spy(this.res, 'redirect')
-      this.SubscriptionController.reactivateSubscription(this.req, this.res)
-    })
+    describe('when the user has permission', function () {
+      beforeEach(function (done) {
+        this.res = {
+          redirect() {
+            done()
+          },
+        }
+        this.req.assertPermission = sinon.stub()
+        this.next = sinon.stub().callsFake(error => {
+          done(error)
+        })
+        sinon.spy(this.res, 'redirect')
+        this.SubscriptionController.reactivateSubscription(
+          this.req,
+          this.res,
+          this.next
+        )
+      })
 
-    it('should tell the handler to reactivate this user', function (done) {
-      this.SubscriptionHandler.reactivateSubscription
-        .calledWith(this.user)
-        .should.equal(true)
-      done()
+      it('should assert the user has permission to reactivate their subscription', function (done) {
+        this.req.assertPermission
+          .calledWith('reactivate-subscription')
+          .should.equal(true)
+        done()
+      })
+
+      it('should tell the handler to reactivate this user', function (done) {
+        this.SubscriptionHandler.reactivateSubscription
+          .calledWith(this.user)
+          .should.equal(true)
+        done()
+      })
+
+      it('should redurect to the subscription page', function (done) {
+        this.res.redirect.calledWith('/user/subscription').should.equal(true)
+        done()
+      })
     })
 
-    it('should redurect to the subscription page', function (done) {
-      this.res.redirect.calledWith('/user/subscription').should.equal(true)
-      done()
+    describe('when the user does not have permission', function () {
+      beforeEach(function (done) {
+        this.res = {
+          redirect() {
+            done()
+          },
+        }
+        this.req.assertPermission = sinon.stub().throws()
+        this.next = sinon.stub().callsFake(() => {
+          done()
+        })
+        sinon.spy(this.res, 'redirect')
+        this.SubscriptionController.reactivateSubscription(
+          this.req,
+          this.res,
+          this.next
+        )
+      })
+
+      it('should not reactivate the user', function (done) {
+        this.req.assertPermission = sinon.stub().throws()
+        this.SubscriptionHandler.reactivateSubscription.called.should.equal(
+          false
+        )
+        done()
+      })
+
+      it('should call next with an error', function (done) {
+        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
+        done()
+      })
     })
   })