[GitHub] rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users current password when they are executing a password update

[GitBox <gi...@apache.org>]

rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users current password when they are executing a password update
URL: https://github.com/apache/cloudstack/pull/2574#discussion_r185206054
 
 

 ##########
 File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java
 ##########
 @@ -1153,157 +1152,246 @@ public UserVO createUser(String userName, String password, String firstName, Str
 
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = "creating User")
-    public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId,
-            String userUUID) {
+    public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, String userUUID) {
 
         return createUser(userName, password, firstName, lastName, email, timeZone, accountName, domainId, userUUID, User.Source.UNKNOWN);
     }
 
     @Override
-    @ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "updating User")
-    public UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey,
-            String timeZone) {
-        // Input validation
-        UserVO user = _userDao.getUser(userId);
+    @ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "Updating User")
+    public UserAccount updateUser(UpdateUserCmd updateUserCmd) {
+        UserVO user = retrieveAndValidateUser(updateUserCmd);
+        s_logger.debug("Updating user with Id: " + user.getUuid());
 
-        if (user == null) {
-            throw new InvalidParameterValueException("unable to find user by id");
-        }
+        validateAndUpdatApiAndSecretKeyIfNeeded(updateUserCmd, user);
+        Account account = retrieveAndValidateAccount(user);
 
-        if ((apiKey == null && secretKey != null) || (apiKey != null && secretKey == null)) {
-            throw new InvalidParameterValueException("Please provide an userApiKey/userSecretKey pair");
-        }
+        validateAndUpdateFirstNameIfNeeded(updateUserCmd, user);
+        validateAndUpdateLastNameIfNeeded(updateUserCmd, user);
+        validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account);
 
-        // If the account is an admin type, return an error. We do not allow this
-        Account account = _accountDao.findById(user.getAccountId());
-        if (account == null) {
-            throw new InvalidParameterValueException("unable to find user account " + user.getAccountId());
+        validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), user, updateUserCmd.getCurrentPassword());
+        String email = updateUserCmd.getEmail();
+        if (StringUtils.isNotBlank(email)) {
+            user.setEmail(email);
         }
-
-        // don't allow updating project account
-        if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
-            throw new InvalidParameterValueException("unable to find user by id");
+        String timezone = updateUserCmd.getTimezone();
+        if (StringUtils.isNotBlank(timezone)) {
+            user.setTimezone(timezone);
+        }
+        _userDao.update(user.getId(), user);
+        return _userAccountDao.findById(user.getId());
+    }
+
+    /**
+     * Updates the password in the user POJO if needed. If no password is provided, then the password is not updated.
+     * The following validations are executed if 'password' is not null. Admins (root admins or domain admins) can execute password updates without entering the current password.
+     * <ul>
+     *  <li> If 'password' is blank, we throw an {@link InvalidParameterValueException};
+     *  <li> If 'current password' is not provided and user is not an Admin, we throw an {@link InvalidParameterValueException};
+     *  <li> If a normal user is calling this method, we use {@link #validateCurrentPassword(UserVO, String)} to check if the provided old password matches the database one;
+     * </ul>
+     *
+     * If all checks pass, we encode the given password with the most preferable password mechanism given in {@link #_userPasswordEncoders}.
+     */
+    protected void validateUserPasswordAndUpdateIfNeeded(String newPassword, UserVO user, String currentPassword) {
+        if (newPassword == null) {
+            s_logger.trace("No new password to update for user: " + user.getUuid());
+            return;
         }
-
-        // don't allow updating system account
-        if (account.getId() == Account.ACCOUNT_ID_SYSTEM) {
-            throw new PermissionDeniedException("user id : " + userId + " is system account, update is not allowed");
+        if (StringUtils.isBlank(newPassword)) {
+            throw new InvalidParameterValueException("Password cannot be empty or blank.");
+        }
+        Account callingAccount = getCurrentCallingAccount();
+        boolean isRootAdminExecutingPasswordUpdate = callingAccount.getId() == Account.ACCOUNT_ID_SYSTEM || isRootAdmin(callingAccount.getId());
+        boolean isDomainAdmin = isDomainAdmin(callingAccount.getId());
+        boolean isAdmin = isDomainAdmin || isRootAdminExecutingPasswordUpdate;
+        if (isAdmin) {
+            s_logger.trace(String.format("Admin account [uuid=%s] executing password update for user [%s] ", callingAccount.getUuid(), user.getUuid()));
+        }
+        if (!isAdmin && StringUtils.isBlank(currentPassword)) {
+            throw new InvalidParameterValueException("To set a new password the current password must be provided.");
+        }
+        if (CollectionUtils.isEmpty(_userPasswordEncoders)) {
+            throw new CloudRuntimeException("No user authenticators configured!");
+        }
+        if (!isAdmin) {
+            validateCurrentPassword(user, currentPassword);
+        }
+        UserAuthenticator userAuthenticator = _userPasswordEncoders.get(0);
+        String newPasswordEncoded = userAuthenticator.encode(newPassword);
+        user.setPassword(newPasswordEncoded);
+    }
+
+    /**
+     * Iterates over all configured user authenticators and tries to authenticated the user using the current password.
 
 Review comment:
   Done. 
   
   Thanks for spotting this mistake.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services