You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/09/21 07:13:39 UTC

[GitHub] [dolphinscheduler] github-code-scanning[bot] commented on a diff in pull request #11255: [Feature] Add params check in createUser Function

github-code-scanning[bot] commented on code in PR #11255:
URL: https://github.com/apache/dolphinscheduler/pull/11255#discussion_r976127221


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -383,86 +380,37 @@
                                           String phone,
                                           String queue,
                                           int state,
-                                          String timeZone) throws IOException {
+                                          String timeZone) {
         Map<String, Object> result = new HashMap<>();
         result.put(Constants.STATUS, false);
 
         if (resourcePermissionCheckService.functionDisabled()) {
-            putMsg(result, Status.FUNCTION_DISABLED);
-            return result;
+            throw new ServiceException(Status.FUNCTION_DISABLED);
         }
         if (check(result, !canOperator(loginUser, userId), Status.USER_NO_OPERATION_PERM)) {
-            logger.warn("User does not have permission for this feature, userId:{}, userName:{}.", loginUser.getId(), loginUser.getUserName());
-            return result;
+            throw new ServiceException(Status.USER_NO_OPERATION_PERM);
         }
         User user = userMapper.selectById(userId);
         if (user == null) {
-            logger.error("User does not exist, userId:{}.", userId);
-            putMsg(result, Status.USER_NOT_EXIST, userId);
-            return result;
-        }
-        if (StringUtils.isNotEmpty(userName)) {
-
-            if (!CheckUtils.checkUserName(userName)) {
-                logger.warn("Parameter userName check failed.");
-                putMsg(result, Status.REQUEST_PARAMS_NOT_VALID_ERROR, userName);
-                return result;
-            }
-
-            User tempUser = userMapper.queryByUserNameAccurately(userName);
-            if (tempUser != null && tempUser.getId() != userId) {
-                logger.warn("User name already exists, userName:{}.", tempUser.getUserName());
-                putMsg(result, Status.USER_NAME_EXIST);
-                return result;
-            }
-            user.setUserName(userName);
-        }
-
-        if (StringUtils.isNotEmpty(userPassword)) {
-            if (!CheckUtils.checkPasswordLength(userPassword)) {
-                logger.warn("Parameter userPassword check failed.");
-                putMsg(result, Status.USER_PASSWORD_LENGTH_ERROR);
-                return result;
-            }
-            user.setUserPassword(EncryptionUtils.getMd5(userPassword));
-        }
-
-        if (StringUtils.isNotEmpty(email)) {
-            if (!CheckUtils.checkEmail(email)) {
-                logger.warn("Parameter email check failed.");
-                putMsg(result, Status.REQUEST_PARAMS_NOT_VALID_ERROR, email);
-                return result;
-            }
-            user.setEmail(email);
-        }
-
-        if (StringUtils.isNotEmpty(phone) && !CheckUtils.checkPhone(phone)) {
-            logger.warn("Parameter phone check failed.");
-            putMsg(result, Status.REQUEST_PARAMS_NOT_VALID_ERROR, phone);
-            return result;
-        }
-
-        if (state == 0 && user.getState() != state && Objects.equals(loginUser.getId(), user.getId())) {
-            logger.warn("Not allow to disable your own account, userId:{}, userName:{}.", user.getId(), user.getUserName());
-            putMsg(result, Status.NOT_ALLOW_TO_DISABLE_OWN_ACCOUNT);
-            return result;
+            logger.warn("User does not have permission for this feature, userId:{}, userName:{}.", loginUser.getId(), loginUser.getUserName());
+            throw new ServiceException(Status.USER_NOT_EXIST);
         }
 
+        checkUserParams(userPassword, email, phone);
         if (StringUtils.isNotEmpty(timeZone)) {
             if (!CheckUtils.checkTimeZone(timeZone)) {
-                logger.warn("Parameter time zone is illegal.");
-                putMsg(result, Status.TIME_ZONE_ILLEGAL, timeZone);
-                return result;
+                throw new ServiceException(Status.TIME_ZONE_ILLEGAL, timeZone);
             }
             user.setTimeZone(timeZone);
         }
 
-        user.setPhone(phone);
+        user = new User(userName, userPassword, email, phone, tenantId, state);
+        if (state == 0 && user.getState() != state && loginUser.getId() == user.getId()) {

Review Comment:
   ## Reference equality test of boxed types
   
   Suspicious reference comparison of boxed numerical values.
   
   [Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1510)



##########
dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/UsersServiceTest.java:
##########
@@ -741,13 +779,15 @@
         String userName = "userTest0001";
         String userPassword = "userTest";
         String email = "abc@x.com";
-        String phone = "123456789";
+        String phone = "17366666666";
         String tenantCode = "tenantCode";
+        Integer userId = 1;

Review Comment:
   ## Unread local variable
   
   Variable 'Integer userId' is never read.
   
   [Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1511)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org