From d2d2386441c5802a0d7ed6a153a6ec8a903d6598 Mon Sep 17 00:00:00 2001 From: Alexandre Bourdin Date: Thu, 3 Aug 2023 12:06:04 +0200 Subject: [PATCH] Merge pull request #14106 from overleaf/ab-ab-group-settings-admin-only [web] Restrict group settings page and managed users activation to group admin GitOrigin-RevId: 97235d3e78d97d9c367ce7de70072607f15d98f0 --- .../Subscription/SubscriptionController.js | 26 +++++----- .../UserMembershipAuthorization.js | 19 +++---- .../UserMembershipEntityConfigs.js | 49 +++++-------------- .../UserMembershipMiddleware.js | 14 +++++- 4 files changed, 46 insertions(+), 62 deletions(-) diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.js b/services/web/app/src/Features/Subscription/SubscriptionController.js index 591c94f198..44a8f13893 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionController.js +++ b/services/web/app/src/Features/Subscription/SubscriptionController.js @@ -138,8 +138,8 @@ async function plansPage(req, res) { } /** - * @param {import("express").Request} req - * @param {import("express").Response} res + * @param {import('express').Request} req + * @param {import('express').Response} res * @returns {Promise} */ async function paymentPage(req, res) { @@ -208,8 +208,8 @@ function formatGroupPlansDataForDash() { } /** - * @param {import("express").Request} req - * @param {import("express").Response} res + * @param {import('express').Request} req + * @param {import('express').Response} res * @returns {Promise} */ async function userSubscriptionPage(req, res) { @@ -246,9 +246,13 @@ async function userSubscriptionPage(req, res) { const groupPlansDataForDash = formatGroupPlansDataForDash() + // display the Group Settings button only to admins of group subscriptions with the Managed Users feature available const groupSettingsEnabledFor = (managedGroupSubscriptions || []) - .filter(subscription => - ManagedUsersManager.hasManagedUsersFeature(subscription) + .filter( + subscription => + ManagedUsersManager.hasManagedUsersFeature(subscription) && + (subscription.admin_id._id || subscription.admin_id).toString() === + user._id.toString() ) .map(subscription => subscription._id.toString()) @@ -399,8 +403,8 @@ async function createSubscription(req, res) { } /** - * @param {import("express").Request} req - * @param {import("express").Response} res + * @param {import('express').Request} req + * @param {import('express').Response} res * @returns {Promise} */ async function successfulSubscription(req, res) { @@ -440,9 +444,9 @@ function cancelSubscription(req, res, next) { } /** - * @param {import("express").Request} req - * @param {import("express").Response} res - * @param {import("express").NextFunction} next + * @param {import('express').Request} req + * @param {import('express').Response} res + * @param {import('express').NextFunction} next * @returns {Promise} */ function canceledSubscription(req, res, next) { diff --git a/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js b/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js index e3c29d32d6..a7f25c10a5 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js +++ b/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js @@ -17,19 +17,12 @@ const UserMembershipAuthorization = { if (!req.entity) { return false } - return req.entity[req.entityConfig.fields.access].some(accessUserId => - accessUserId.equals(req.user._id) - ) - } - }, - - isEntityMember() { - return req => { - if (!req.entity) { - return false - } - return req.entity[req.entityConfig.fields.membership].some(accessUserId => - accessUserId.equals(req.user._id) + const fieldAccess = req.entity[req.entityConfig.fields.access] + const fieldAccessArray = Array.isArray(fieldAccess) + ? fieldAccess + : [fieldAccess.toString()] + return fieldAccessArray.some( + accessUserId => accessUserId.toString() === req.user._id.toString() ) } }, diff --git a/services/web/app/src/Features/UserMembership/UserMembershipEntityConfigs.js b/services/web/app/src/Features/UserMembership/UserMembershipEntityConfigs.js index 94bc01f7f2..781036db97 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipEntityConfigs.js +++ b/services/web/app/src/Features/UserMembership/UserMembershipEntityConfigs.js @@ -14,19 +14,6 @@ module.exports = { baseQuery: { groupPlan: true, }, - translations: { - title: 'group_subscription', - subtitle: 'members_management', - remove: 'remove_from_group', - }, - pathsFor(id) { - return { - addMember: `/manage/groups/${id}/invites`, - removeMember: `/manage/groups/${id}/user`, - removeInvite: `/manage/groups/${id}/invites`, - exportMembers: `/manage/groups/${id}/members/export`, - } - }, }, team: { @@ -54,16 +41,20 @@ module.exports = { baseQuery: { groupPlan: true, }, - translations: { - title: 'group_subscription', - subtitle: 'managers_management', - remove: 'remove_manager', + }, + + groupAdmin: { + modelName: 'Subscription', + fields: { + primaryKey: '_id', + read: ['admin_id'], + write: 'admin_id', + access: 'admin_id', + membership: 'admin_id', + name: 'teamName', }, - pathsFor(id) { - return { - addMember: `/manage/groups/${id}/managers`, - removeMember: `/manage/groups/${id}/managers`, - } + baseQuery: { + groupPlan: true, }, }, @@ -77,16 +68,9 @@ module.exports = { membership: 'member_ids', name: 'name', }, - translations: { - title: 'institution_account', - subtitle: 'managers_management', - remove: 'remove_manager', - }, pathsFor(id) { return { index: `/manage/institutions/${id}/managers`, - addMember: `/manage/institutions/${id}/managers`, - removeMember: `/manage/institutions/${id}/managers`, } }, }, @@ -101,16 +85,9 @@ module.exports = { membership: 'member_ids', name: 'name', }, - translations: { - title: 'publisher_account', - subtitle: 'managers_management', - remove: 'remove_manager', - }, pathsFor(id) { return { index: `/manage/publishers/${id}/managers`, - addMember: `/manage/publishers/${id}/managers`, - removeMember: `/manage/publishers/${id}/managers`, } }, }, diff --git a/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.js b/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.js index fc443cf67a..920bd9fbac 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.js +++ b/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.js @@ -64,6 +64,17 @@ const UserMembershipMiddleware = { ]), ], + requireGroupAdminAccess: [ + AuthenticationController.requireLogin(), + fetchEntityConfig('groupAdmin'), + fetchEntity(), + requireEntity(), + allowAccessIfAny([ + UserMembershipAuthorization.hasEntityAccess(), + UserMembershipAuthorization.hasStaffAccess('groupManagement'), + ]), + ], + requireInstitutionMetricsAccess: [ AuthenticationController.requireLogin(), fetchEntityConfig('institution'), @@ -222,12 +233,11 @@ function fetchEntityConfig(entityName) { // fetch the entity with id and config, and set it in the request function fetchEntity() { return expressify(async (req, res, next) => { - const entity = + req.entity = await UserMembershipHandler.promises.getEntityWithoutAuthorizationCheck( req.params.id, req.entityConfig ) - req.entity = entity next() }) }