Enhancement / Edit user from users list (#1834)

* added users list (all) view * code cleanup * users list: show surveys for user * fixed DeepScan issue * code cleanup * user edit: hide Role in Survey dropdown when editing from all users list * users list: do not allow editing users * fixed DeepScan issue * users list: update user from list * confirm user will be system admin * fixed DeepScan issue; code cleanup * code cleanup * code cleanup * code cleanup Co-authored-by: Stefano Ricci <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
openforis · Oct 11, 2021 · 08211cd · 08211cd
1 parent 9321afe
commit 08211cd
Show file tree

Hide file tree

Showing 29 changed files with 371 additions and 182 deletions.
diff --git a/common/analysis/chain.js b/common/analysis/chain.js
@@ -12,15 +12,15 @@ export const keys = {
   statusExec: 'status_exec',
   uuid: ObjectUtils.keys.uuid,
   temporary: ObjectUtils.keys.temporary,
-  validation: ObjectUtils.keys.validation,
+  validation: Validation.keys.validation,
   scriptCommon: 'script_common',
 }
 
 export const keysProps = {
   labels: ObjectUtils.keysProps.labels,
   descriptions: ObjectUtils.keysProps.descriptions,
   cycles: ObjectUtils.keysProps.cycles,
-  analysisNodeDefs: 'analysisNodeDefs'
+  analysisNodeDefs: 'analysisNodeDefs',
 }
 
 export const statusExec = {

diff --git a/core/auth/authorizer.js b/core/auth/authorizer.js
@@ -132,7 +132,12 @@ export const canViewUsersAccessRequests = (user) => User.isSystemAdmin(user)
 export const canEditUsersAccessRequests = (user) => User.isSystemAdmin(user)
 
 // INVITE
-export const getUserGroupsCanAssign = ({ user, surveyInfo = null, editingLoggedUser = false }) => {
+export const getUserGroupsCanAssign = ({
+  user,
+  surveyInfo = null,
+  editingLoggedUser = false,
+  showOnlySurveyGroups = false,
+}) => {
   let surveyGroups
   if (editingLoggedUser && !surveyInfo) {
     // This can happen for system administrators when they don't have an active survey
@@ -144,7 +149,7 @@ export const getUserGroupsCanAssign = ({ user, surveyInfo = null, editingLoggedU
   }
 
   const groups = []
-  if (User.isSystemAdmin(user) || User.isSurveyManager(user)) {
+  if (!showOnlySurveyGroups && (User.isSystemAdmin(user) || User.isSurveyManager(user))) {
     // Add SystemAdmin or SurveyManager group if current user is a SystemAdmin or SurveyManager himself
     groups.push(...User.getAuthGroups(user))
   }

diff --git a/core/i18n/resources/en.js b/core/i18n/resources/en.js
@@ -482,7 +482,9 @@ Thank you and enjoy **$t(common.appNameFull)**!`,
     userSurveys: 'User Surveys',
     surveyName: 'Survey name',
     roleInSurvey: 'Role in survey',
+    roleInCurrentSurvey: 'Role in current survey',
     userNotInvitedToAnySurvey: `User not invited to any survey`,
+    confirmUserWillBeSystemAdmin: 'User will be a system administrator. Continue?',
   },
 
   usersAccessRequestView: {

diff --git a/core/user/_user/userKeys.js b/core/user/_user/userKeys.js
@@ -1,4 +1,5 @@
 import * as ObjectUtils from '@core/objectUtils'
+import * as Validation from '@core/validation/validation'
 
 export const keys = {
   uuid: ObjectUtils.keys.uuid,
@@ -12,9 +13,11 @@ export const keys = {
   profilePicture: 'profilePicture',
   props: ObjectUtils.keys.props,
   status: 'status',
-  validation: ObjectUtils.keys.validation,
-  groupUuid: 'groupUuid', // Used only when editing user survey group
+  validation: Validation.keys.validation,
   invitationExpired: 'invitationExpired',
   invitedBy: 'invitedBy',
   invitedDate: 'invitedDate',
+
+  // Used only when editing user auth groups
+  authGroupsUuids: 'authGroupsUuids'
 }
diff --git a/core/user/user.js b/core/user/user.js
@@ -25,7 +25,6 @@ export const getEmail = R.prop(keys.email)
 export const getInvitedBy = R.prop(keys.invitedBy)
 export const getInvitedDate = R.prop(keys.invitedDate)
 export const getPassword = R.prop(keys.password)
-export const getGroupUuid = R.prop(keys.groupUuid)
 export const getLang = R.propOr('en', keys.lang)
 export const { getAuthGroups } = ObjectUtils
 export const getPrefs = R.propOr({}, keys.prefs)
@@ -34,12 +33,12 @@ export const getProfilePicture = R.prop(keys.profilePicture)
 export const hasProfilePicture = R.propEq(keys.hasProfilePicture, true)
 export const getStatus = R.prop(keys.status)
 export const { getValidation } = Validation
+export const getAuthGroupsUuids = R.propOr([], keys.authGroupsUuids)
 
 // ====== UPDATE
 export const assocProp = R.assoc
 export const assocEmail = R.assoc(keys.email)
 export const { assocValidation } = Validation
-export const assocGroupUuid = R.assoc(keys.groupUuid)
 export const assocName = R.assoc(keys.name)
 export const assocInvitationExpired = R.assoc(keys.invitationExpired)
 export const assocProfilePicture = R.assoc(keys.profilePicture)
@@ -67,7 +66,16 @@ export const getAuthGroupByName = (groupName) => (user) => {
   return authGroups.find((group) => AuthGroup.getName(group) === groupName)
 }
 
-export const assocAuthGroups = R.assoc(keys.authGroups)
+export const getAuthGroupsNonSurvey = () => (user) => {
+  const authGroups = getAuthGroups(user)
+  return authGroups.filter((group) => !AuthGroup.getSurveyId(group))
+}
+
+export const getSystemAdminGroup = (user) => user && getAuthGroups(user).find(AuthGroup.isSystemAdminGroup)
+export const getSurveyManagerGroup = (user) => user && getAuthGroups(user).find(AuthGroup.isSurveyManagerGroup)
+
+export const assocAuthGroups = (authGroups) =>
+  R.pipe(R.assoc(keys.authGroups, authGroups), R.assoc(keys.authGroupsUuids, authGroups.map(ObjectUtils.getUuid)))
 
 const _updateAuthGroups = (updateFn) => (user) =>
   R.pipe(getAuthGroups, updateFn, (authGroups) => assocAuthGroups(authGroups)(user))(user)

diff --git a/core/user/userValidator.js b/core/user/userValidator.js
@@ -12,7 +12,7 @@ export const validateUser = async (user) =>
     ],
     [User.keys.name]: [Validator.validateRequired(Validation.messageKeys.nameRequired)],
     [User.keys.email]: [Validator.validateRequired(Validation.messageKeys.user.emailRequired), validateEmail],
-    [User.keys.groupUuid]: [Validator.validateRequired(Validation.messageKeys.user.groupRequired)],
+    [User.keys.authGroupsUuids]: [Validator.validateRequired(Validation.messageKeys.user.groupRequired)],
   })
 
 export const validateInvitation = async (userInvite) =>

diff --git a/server/modules/auth/manager/authManager.js b/server/modules/auth/manager/authManager.js
@@ -1,6 +1,7 @@
 export {
   fetchGroupByName,
   fetchGroupByUuid,
+  fetchGroupsByUuids,
   updateUserGroup,
   fetchSurveyGroups,
 } from '../repository/authGroupRepository'
diff --git a/server/modules/auth/repository/authGroupRepository.js b/server/modules/auth/repository/authGroupRepository.js
@@ -60,6 +60,18 @@ export const fetchGroupByUuid = async (groupUuid, client = db) =>
     dbTransformCallback
   )
 
+export const fetchGroupsByUuids = async (groupUuids, client = db) =>
+  client.map(
+    `
+    SELECT g.*, s.id as survey_id
+    FROM auth_group g
+    LEFT OUTER JOIN survey s 
+      ON g.survey_uuid = s.uuid
+    WHERE g.uuid IN ($1:csv)`,
+    [groupUuids],
+    dbTransformCallback
+  )
+
 export const fetchSurveyGroups = async (surveyId, client = db) =>
   client.map(
     `
@@ -134,7 +146,7 @@ export const deleteAllUserGroups = async (userUuid, client = db) =>
 
 // ==== DELETE
 
-export const deleteUserGroup = async (surveyId, userUuid, client = db) =>
+export const deleteUserGroupBySurveyAndUser = async (surveyId, userUuid, client = db) =>
   client.query(
     `
     DELETE
@@ -161,3 +173,16 @@ export const deleteUserGroup = async (surveyId, userUuid, client = db) =>
   `,
     [userUuid, surveyId]
   )
+
+export const deleteUserGroupByUserAndGroupUuid = async ({ userUuid, groupUuid }, client = db) =>
+  client.query(
+    `
+      DELETE
+      FROM
+        auth_group_user
+      WHERE
+        user_uuid = $1
+      AND group_uuid = $2
+    `,
+    [userUuid, groupUuid]
+  )
diff --git a/server/modules/user/api/userApi.js b/server/modules/user/api/userApi.js
@@ -9,20 +9,11 @@ import * as Validation from '@core/validation/validation'
 import * as ProcessUtils from '@core/processUtils'
 
 import SystemError from '@core/systemError'
-import UnauthorizedError from '@server/utils/unauthorizedError'
 import * as SurveyManager from '@server/modules/survey/manager/surveyManager'
 
 import * as UserService from '../service/userService'
 import * as AuthMiddleware from '../../auth/authApiMiddleware'
 
-const _checkSelf = (req) => {
-  const { userUuid } = Request.getParams(req)
-  const user = Request.getUser(req)
-  if (userUuid !== User.getUuid(user)) {
-    throw new UnauthorizedError(user && User.getName(user))
-  }
-}
-
 export const init = (app) => {
   // ==== CREATE
 
@@ -113,9 +104,8 @@ export const init = (app) => {
     }
   })
 
-  app.get('/user/:userUuid', async (req, res, next) => {
+  app.get('/user/:userUuid', AuthMiddleware.requireUserViewPermission, async (req, res, next) => {
     try {
-      _checkSelf(req)
       await _getUser(req, res)
     } catch (error) {
       next(error)
@@ -263,9 +253,8 @@ export const init = (app) => {
     }
   })
 
-  app.put('/user/:userUuid', async (req, res, next) => {
+  app.put('/user/:userUuid', AuthMiddleware.requireUserEditPermission, async (req, res, next) => {
     try {
-      _checkSelf(req)
       await _updateUser(req, res)
     } catch (error) {
       next(error)

diff --git a/server/modules/user/manager/userManager.js b/server/modules/user/manager/userManager.js
@@ -28,11 +28,8 @@ export const {
   importNewUser,
 } = UserRepository
 
-export const {
-  findUserUuidByUuid: findResetPasswordUserUuidByUuid,
-  deleteUserResetPasswordByUuid,
-  deleteUserResetPasswordExpired,
-} = UserResetPasswordRepository
+export const { findResetPasswordUserUuidByUuid, deleteUserResetPasswordByUuid, deleteUserResetPasswordExpired } =
+  UserResetPasswordRepository
 
 // ==== CREATE
 
@@ -154,6 +151,7 @@ const _userFetcher =
 export const fetchUserByEmail = _userFetcher(UserRepository.fetchUserByEmail)
 
 export const fetchUserByUuid = _userFetcher(UserRepository.fetchUserByUuid)
+
 export const fetchUserByUuidWithPassword = _userFetcher(UserRepository.fetchUserByUuidWithPassword)
 
 export const fetchUsers = async ({ offset, limit }, client = db) =>
@@ -196,25 +194,68 @@ export {
 
 // ==== UPDATE
 
+const _insertOrDeleteUserGroup = async ({ userUuid, authGroupsNew, userToUpdateOld, groupName }, client = db) => {
+  const group = User.getAuthGroupByName(groupName)(userToUpdateOld)
+  if (group) {
+    // user previously associated to the group: remove user from group
+    await AuthGroupRepository.deleteUserGroupByUserAndGroupUuid(
+      { userUuid, groupUuid: AuthGroup.getUuid(group) },
+      client
+    )
+  } else {
+    // add user to the new group
+    const groupNew = authGroupsNew.find((authGroup) => AuthGroup.getName(authGroup) === groupName)
+    await AuthGroupRepository.insertUserGroup(AuthGroup.getUuid(groupNew), userUuid, client)
+  }
+}
+
 const _updateUser = async (user, surveyId, userToUpdate, profilePicture, client = db) =>
   client.tx(async (t) => {
     const userUuid = User.getUuid(userToUpdate)
-    const groupUuid = User.getGroupUuid(userToUpdate)
-    const newGroup = await AuthGroupRepository.fetchGroupByUuid(groupUuid)
-
-    if (AuthGroup.isSystemAdminGroup(newGroup)) {
-      // If new group is SystemAdmin, delete all user groups and set his new group to SystemAdmin
-      await AuthGroupRepository.deleteAllUserGroups(userUuid, t)
-      await AuthGroupRepository.insertUserGroup(groupUuid, userUuid, t)
-    } else if (surveyId) {
-      await AuthGroupRepository.updateUserGroup(surveyId, userUuid, groupUuid, t)
-      // Log user update activity only for non system admin users
-      await ActivityLogRepository.insert(user, surveyId, ActivityLog.type.userUpdate, userToUpdate, false, t)
+    const userToUpdateOld = await fetchUserByUuid(userUuid, t)
+    const authGroupsUuidsNew = User.getAuthGroupsUuids(userToUpdate)
+
+    const authGroupsNew = await AuthGroupRepository.fetchGroupsByUuids(authGroupsUuidsNew, t)
+
+    // if user admin role changed, insert or delete the user auth group
+    const userToUpdateWillBeSystemAdmin = authGroupsNew.some(AuthGroup.isSystemAdminGroup)
+    if (userToUpdateWillBeSystemAdmin !== User.isSystemAdmin(userToUpdateOld)) {
+      if (userToUpdateWillBeSystemAdmin) {
+        // user will be system admin: delete all user groups and set his new group to SystemAdmin
+        await AuthGroupRepository.deleteAllUserGroups(userUuid, t)
+      }
+      const groupName = AuthGroup.groupNames.systemAdmin
+      await _insertOrDeleteUserGroup({ userUuid, authGroupsNew, userToUpdateOld, groupName }, t)
+    }
+    // if user survey manager role changed, insert or delete the user auth group
+    const userToUpdateWillBeSurveyManager = authGroupsNew.some(AuthGroup.isSurveyManagerGroup)
+    if (userToUpdateWillBeSurveyManager !== User.isSurveyManager(userToUpdateOld)) {
+      const groupName = AuthGroup.groupNames.surveyManager
+      await _insertOrDeleteUserGroup({ userUuid, authGroupsNew, userToUpdateOld, groupName }, t)
     }
 
+    // insert or update user survey auth group relation
+    if (surveyId && !userToUpdateWillBeSystemAdmin) {
+      const surveyAuthGroupNew = authGroupsNew.find((authGroup) => AuthGroup.getSurveyId(authGroup) === surveyId)
+      const surveyUuid = AuthGroup.getSurveyUuid(surveyAuthGroupNew)
+      const groupUuid = AuthGroup.getUuid(surveyAuthGroupNew)
+      const surveyAuthGroupOld = User.getAuthGroupBySurveyUuid(surveyUuid, false)(userToUpdateOld)
+      if (!AuthGroup.isEqual(surveyAuthGroupNew)(surveyAuthGroupOld)) {
+        if (surveyAuthGroupOld) {
+          await AuthGroupRepository.updateUserGroup(surveyId, userUuid, groupUuid, t)
+        } else {
+          await AuthGroupRepository.insertUserGroup(groupUuid, userUuid, t)
+        }
+        // Log user update activity only for non system admin users
+        await ActivityLogRepository.insert(user, surveyId, ActivityLog.type.userUpdate, userToUpdate, false, t)
+      }
+    }
+
+    // update user props and picture
     const name = User.getName(userToUpdate)
     const email = User.getEmail(userToUpdate)
     const props = User.getProps(userToUpdate)
+
     return UserRepository.updateUser(
       {
         userUuid,
@@ -240,7 +281,7 @@ export const deleteUser = async ({ user, userUuidToRemove, survey }, client = db
   client.tx(async (t) => {
     const surveyId = Survey.getId(survey)
     return Promise.all([
-      AuthGroupRepository.deleteUserGroup(surveyId, userUuidToRemove, t),
+      AuthGroupRepository.deleteUserGroupBySurveyAndUser(surveyId, userUuidToRemove, t),
       ActivityLogRepository.insert(
         user,
         surveyId,

diff --git a/server/modules/user/repository/userRepository.js b/server/modules/user/repository/userRepository.js
@@ -70,7 +70,7 @@ export const fetchUsers = async ({ offset = 0, limit = null }, client = db) =>
     SELECT 
         ${selectFieldsCommaSep}
     FROM "user" u
-    ORDER BY u.name, u.email
+    ORDER BY u.email
     LIMIT ${limit || 'ALL'}
     OFFSET ${offset}`,
     [],

diff --git a/server/modules/user/repository/userResetPasswordRepository.js b/server/modules/user/repository/userResetPasswordRepository.js
@@ -15,7 +15,7 @@ export const insertOrUpdateResetPassword = async (userUuid, client = db) =>
     R.prop('uuid')
   )
 
-export const findUserUuidByUuid = async (uuid, client = db) =>
+export const findResetPasswordUserUuidByUuid = async (uuid, client = db) =>
   await client.oneOrNone(
     `SELECT user_uuid
      FROM user_reset_password