fix: users + groups permissions assign logic + text hints

Author: NGPixel

client/components/admin/admin-groups-edit-permissions.vue

@@ -149,28 +149,28 @@ export default {
           items: [
             {
               permission: 'write:users',
-              hint: 'Can create or authorize new users, but not modify existing ones',
+              hint: 'Can create or authorize new users, but not modify existing ones. Can only assign to non-administrative groups',
               warning: false,
               restrictedForSystem: true,
               disabled: false
             },
             {
               permission: 'manage:users',
-              hint: 'Can manage all users (but not users with administrative permissions)',
+              hint: 'Can create, authorize and modify ANY users. Can only assign to non-administrative groups',
               warning: false,
               restrictedForSystem: true,
               disabled: false
             },
             {
               permission: 'write:groups',
-              hint: 'Can manage groups and assign CONTENT permissions / page rules',
+              hint: 'Can manage groups and set CONTENT permissions / page rules. Can only assign users to non-administrative groups',
               warning: false,
               restrictedForSystem: true,
               disabled: false
             },
             {
               permission: 'manage:groups',
-              hint: 'Can manage groups and assign ANY permissions (but not manage:system) / page rules',
+              hint: 'Can manage groups and set ANY permissions (but not manage:system) / page rules. Can assign users to ANY groups (except groups with the manage:system permission)',
               warning: true,
               restrictedForSystem: true,
               disabled: false
@@ -203,7 +203,7 @@ export default {
             },
             {
               permission: 'manage:system',
-              hint: 'Can manage and access everything. Root administrator.',
+              hint: 'Can manage and access everything. Root administrator',
               warning: true,
               restrictedForSystem: true,
               disabled: true

server/core/auth.js

@@ -318,41 +318,41 @@ module.exports = {
   },
 
   /**
-   * Check if user can perform assignment to a group with elevated permissions
+   * Check if user (requester) can perform user assignment to a group with elevated permissions
    *
-   * @param {User} user
-   * @param {Array<Number>} groupIds
+   * @param {User} requester The user attempting to perform the assignment
+   * @param {Array<Number>} groupIds List of group IDs to be assigned
    * @returns {Boolean}
    */
-  async checkAssignUserToGroupAccess(user, groupIds = []) {
+  async checkAssignUserToGroupAccess(requester, groupIds = []) {
     if (!groupIds || groupIds.length < 1) {
       return true
     }
 
-    const userPermissions = user.permissions ? user.permissions : user.getGlobalPermissions()
+    const requesterPermissions = requester.permissions ? requester.permissions : requester.getGlobalPermissions()
 
     // System Admin
-    if (userPermissions.includes('manage:system')) {
+    if (requesterPermissions.includes('manage:system')) {
       return true
     }
 
     // Ensure basic user management permission
-    if (!userPermissions.some(p => ['write:users', 'manage:users', 'write:groups', 'manage:groups'].includes(p))) {
+    if (!requesterPermissions.some(p => ['write:users', 'manage:users', 'write:groups', 'manage:groups'].includes(p))) {
       return false
     }
 
     const groups = await WIKI.models.groups.query().whereIn('id', groupIds)
-    return !groups.some(grp => {
-      // Check for manage:system permission
-      if (grp.permissions.includes('manage:system') && !userPermissions.includes('manage:groups')) {
+    return groups.every(grp => {
+      // Check group for manage:system permission
+      if (grp.permissions.includes('manage:system')) {
         return false
       }
 
-      // Check for elevated permissions
+      // Check group for administrative permissions
       if (grp.permissions.some(p => {
         const permType = _.last(p.split(':'))
         return ['users', 'groups', 'navigation', 'theme', 'api'].includes(permType)
-      }) && !(userPermissions.includes('write:groups') || userPermissions.includes('manage:groups'))) {
+      }) && !requesterPermissions.includes('manage:groups')) {
         return false
       }
 

server/graph/resolvers/group.js

@@ -45,15 +45,15 @@ module.exports = {
         throw new gql.GraphQLError('Invalid Group ID')
       }
 
-      // Check assigned permissions for write:groups
+      // Check assigned permissions for manage:users / write:groups
       if (
-        WIKI.auth.checkExclusiveAccess(req.user, ['write:groups'], ['manage:groups', 'manage:system']) &&
+        WIKI.auth.checkExclusiveAccess(req.user, ['manage:users', 'write:groups'], ['manage:groups', 'manage:system']) &&
         grp.permissions.some(p => {
           const resType = _.last(p.split(':'))
           return ['users', 'groups', 'navigation', 'theme', 'api', 'system'].includes(resType)
         })
       ) {
-        throw new gql.GraphQLError('You are not authorized to assign a user to this elevated group.')
+        throw new gql.GraphQLError('You are not authorized to assign a user to this administrative group.')
       }
 
       // Check assigned permissions for manage:groups
@@ -178,7 +178,7 @@ module.exports = {
           return ['users', 'groups', 'navigation', 'theme', 'api', 'system'].includes(resType)
         })
       ) {
-        throw new gql.GraphQLError('You are not authorized to manage this group or assign these permissions.')
+        throw new gql.GraphQLError('You are not authorized to manage this group or assign these administrative permissions.')
       }
 
       // Check assigned permissions for manage:groups

server/graph/resolvers/user.js

@@ -65,7 +65,7 @@ module.exports = {
     async create (obj, args, context) {
       try {
         if (!(await WIKI.auth.checkAssignUserToGroupAccess(context.req.user, args.groups))) {
-          throw new Error('You are not authorized to assign a user to a group with elevated permissions.')
+          throw new Error('You are not authorized to create a user with an assignment to an administrative group.')
         }
 
         await WIKI.models.users.createNewUser(args)
@@ -101,13 +101,13 @@ module.exports = {
     async update (obj, args, context) {
       try {
         if (!(await WIKI.auth.checkAssignUserToGroupAccess(context.req.user, args.groups))) {
-          throw new Error('You are not authorized to assign a user to a group with elevated permissions.')
+          throw new Error('You are not authorized to modify / assign a user from / to an administrative group.')
         }
 
         await WIKI.models.users.updateUser(args)
 
         return {
-          responseResult: graphHelper.generateSuccess('User created successfully')
+          responseResult: graphHelper.generateSuccess('User updated successfully')
         }
       } catch (err) {
         return graphHelper.generateError(err)

server/graph/schemas/group.graphql

@@ -18,7 +18,7 @@ type GroupQuery {
   list(
     filter: String
     orderBy: String
-  ): [GroupMinimal] @auth(requires: ["write:groups", "manage:groups", "manage:system"])
+  ): [GroupMinimal] @auth(requires: ["write:users", "manage:users", "write:groups", "manage:groups", "manage:system"])
 
   single(
     id: Int!
@@ -49,12 +49,12 @@ type GroupMutation {
   assignUser(
     groupId: Int!
     userId: Int!
-  ): DefaultResponse @auth(requires: ["write:groups", "manage:groups", "manage:system"])
+  ): DefaultResponse @auth(requires: ["manage:users", "write:groups", "manage:groups", "manage:system"])
 
   unassignUser(
     groupId: Int!
     userId: Int!
-  ): DefaultResponse @auth(requires: ["write:groups", "manage:groups", "manage:system"])
+  ): DefaultResponse @auth(requires: ["manage:users", "write:groups", "manage:groups", "manage:system"])
 }
 
 # -----------------------------------------------