You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@streampark.apache.org by GitBox <gi...@apache.org> on 2022/10/14 02:23:07 UTC

[GitHub] [incubator-streampark] MonsterChenzhuo opened a new pull request, #1836: [Improve]Splitting ldap logic

MonsterChenzhuo opened a new pull request, #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836

   ## What problem does this PR solve?
   
   Issue Number: close #1735
   
   Problem Summary:
   
   ## What is changed and how it works?
   
   History Logic: in application.yaml
   ``` yaml
   #select the login mode: 1. PASSWORD 2.LDAP
   security:
     authentication:
       type: PASSWORD
   ```
   to choose whether to load the ldap or local user login implementation class.
   
   Now the logic: the two implementation classes correspond to two different interfaces, and the front-end option box is checked to determine whether to use ldap login or local user login
   ```java
       @PostMapping("signin")
       public RestResponse signin(
           @NotBlank(message = "{required}") String username,
           @NotBlank(message = "{required}") String password) throws Exception {
           if (StringUtils.isEmpty(username)) {
               return RestResponse.success().put("code", 0);
           }
           User user = authenticator.authenticate(username, password);
           Map<String, Object> userInfo = login(username, password, user);
           return new RestResponse().data(userInfo);
       }
   
       @PostMapping("ldapSignin")
       public RestResponse ldapSignin(
           @NotBlank(message = "{required}") String username,
           @NotBlank(message = "{required}") String password) throws Exception {
           if (StringUtils.isEmpty(username)) {
               return RestResponse.success().put("code", 0);
           }
           User user = authenticator.ldapAuthenticate(username, password);
           Map<String, Object> userInfo = login(username, password, user);
           return new RestResponse().data(userInfo);
       }
   
   ```
   
   ## Contribution Checklist
   
     - Make sure that the pull request corresponds to a [GITHUB issue](https://github.com/apache/streampark/issues).
   
     - Name the pull request in the form "[Feature] [component] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
   
     - Minor fixes should be named following this pattern: `[hotfix] [docs] Fix typo in README.md doc`.
   
   ## Purpose of this pull request
   
   Support the team management, it includes team management, team member management, all old projects and apps will be migrated to the default team.


-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] MonsterChenzhuo commented on a diff in pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
MonsterChenzhuo commented on code in PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#discussion_r997664527


##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/PassportController.java:
##########
@@ -67,39 +67,22 @@ public class PassportController {
     public RestResponse signin(
         @NotBlank(message = "{required}") String username,
         @NotBlank(message = "{required}") String password) throws Exception {
-
         if (StringUtils.isEmpty(username)) {
             return RestResponse.success().put("code", 0);
         }
-
         User user = authenticator.authenticate(username, password);
+        return login(username, password, user);
+    }
 
-        if (user == null) {
+    @PostMapping("ldapSignin")
+    public RestResponse ldapSignin(

Review Comment:
   The current pr also needs to be matched with a front-end changes, specific changes to the style and logic have been described in detail at https://github.com/apache/incubator-streampark/issues/1735



-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] MonsterChenzhuo commented on pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
MonsterChenzhuo commented on PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#issuecomment-1281739760

   The current pr also needs to be matched with a front-end changes, specific changes to the style and logic have been described in detail at `https://github.com/apache/incubator-streampark/issues/1735`


-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#discussion_r997667848


##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/PassportController.java:
##########
@@ -67,39 +67,22 @@ public class PassportController {
     public RestResponse signin(
         @NotBlank(message = "{required}") String username,
         @NotBlank(message = "{required}") String password) throws Exception {
-
         if (StringUtils.isEmpty(username)) {
             return RestResponse.success().put("code", 0);
         }
-
         User user = authenticator.authenticate(username, password);
+        return login(username, password, user);
+    }
 
-        if (user == null) {
+    @PostMapping("ldapSignin")
+    public RestResponse ldapSignin(

Review Comment:
   > The current pr also needs to be matched with a front-end changes, specific changes to the style and logic have been described in detail at #1735
   
   good job. BTW,  ldap related documents also need to be provided



-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] MonsterChenzhuo commented on pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
MonsterChenzhuo commented on PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#issuecomment-1281708003

   @1996fanrui @wolfboys Thanks for the huge review work, I have revised all your suggestions.
   I performed a test of the logic and everything was fine
   <img width="464" alt="图片" src="https://user-images.githubusercontent.com/60029759/196316792-d7e37abc-5bc0-4814-8ecc-c5f36cb14f7c.png">
   <img width="555" alt="图片" src="https://user-images.githubusercontent.com/60029759/196317218-dd5c2006-4390-44ec-9449-0b49569c7171.png">
   


-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#discussion_r995772420


##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/security/impl/AuthenticatorImpl.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.streampark.console.system.security.impl;
+
+import org.apache.streampark.console.base.util.ShaHashUtils;
+import org.apache.streampark.console.core.enums.UserType;
+import org.apache.streampark.console.system.entity.User;
+import org.apache.streampark.console.system.security.Authenticator;
+import org.apache.streampark.console.system.security.impl.ldap.LdapService;
+import org.apache.streampark.console.system.service.UserService;
+
+import org.apache.commons.lang3.StringUtils;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import java.util.Date;
+
+@Component
+public class AuthenticatorImpl implements Authenticator {
+    @Autowired
+    private UserService usersService;
+    @Autowired
+    private LdapService ldapService;
+
+    @Override
+    public User authenticate(String username, String password) {
+        User user = usersService.findByName(username);
+        if (user == null) {
+            return null;
+        }
+        String salt = user.getSalt();
+        password = ShaHashUtils.encrypt(salt, password);
+        if (!StringUtils.equals(user.getPassword(), password)) {
+            return null;
+        }
+        return user;
+    }
+
+    @Override
+    public User ldapAuthenticate(String username, String password) throws Exception {
+        User user = null;
+        String ldapEmail = ldapService.ldapLogin(username, password);
+        if (ldapEmail != null) {
+            //check if user exist
+            user = usersService.findByName(username);
+            if (user == null && ldapService.createIfUserNotExists()) {
+                User newUser = new User();
+                newUser.setCreateTime(new Date());
+                newUser.setUsername(username);
+                newUser.setNickName(username);
+                newUser.setUserType(UserType.USER);
+                newUser.setStatus("1");
+                newUser.setSex("1");
+
+                String salt = ShaHashUtils.getRandomSalt();
+                String saltPass = ShaHashUtils.encrypt(salt, password);
+                newUser.setSalt(salt);
+                newUser.setPassword(saltPass);
+                usersService.createUser(newUser);
+                return newUser;
+            }
+        }
+        return user;

Review Comment:
   ```suggestion
           String ldapEmail = ldapService.ldapLogin(username, password);
           if (ldapEmail == null) {
               return null;
           }
   
           //check if user exist
           User user = usersService.findByName(username);
           if (user != null || !ldapService.createIfUserNotExists()) {
               return user;
           }
           User newUser = new User();
           newUser.setCreateTime(new Date());
           newUser.setUsername(username);
           newUser.setNickName(username);
           newUser.setUserType(UserType.USER);
           newUser.setStatus("1");
           newUser.setSex("1");
   
           String salt = ShaHashUtils.getRandomSalt();
           String saltPass = ShaHashUtils.encrypt(salt, password);
           newUser.setSalt(salt);
           newUser.setPassword(saltPass);
           usersService.createUser(newUser);
           return newUser;
   ```
   
   Minor comment, it can reduce this tab.



-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] 1996fanrui commented on pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
1996fanrui commented on PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#issuecomment-1279025332

   > I recommend a pr to do only one thing, for example, the current pr, to solve LDAP-related issue, if you want to improve the response status code, you can open a new pr
   
   Totally agree!


-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] wolfboys merged pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
wolfboys merged PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836


-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#discussion_r995336596


##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/enums/Status.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.streampark.console.core.enums;
+
+import java.util.Optional;
+
+/**
+ * status enum
+ */
+public enum Status {
+
+    SUCCESS(0, "success"),
+
+    INTERNAL_SERVER_ERROR_ARGS(10000, "Internal Server Error: {0}"),
+    USER_CURRENTLY_LOCKED(10002, "user is currently locked"),
+    USER_NAME_EXIST(10003, "user name already exists"),
+    USER_NAME_NULL(10004, "user name is null"),
+    USER_NOT_EXIST(10005, "user {0} not exists"),
+    NOT_SUPPORTED_REQUEST_METHOD(10006, "not supported request method exception: {0}"),
+    API_FAIL(10007, "API Invoke exception:{0}"),
+    REQUEST_PARAMS_NOT_VALID_ERROR(10007, "request parameter {0} is not valid"),
+    READ_BUILD_LOG_EXECPTION(10008, "read build log exception: {0}"),

Review Comment:
   unify the definition of several of the most common exceptions, very good, but there are many purely business exceptions, the definition out of the unreasonable. Too much information about business exceptions, can not be predetermined in advance



##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/base/handler/GlobalExceptionHandler.java:
##########
@@ -51,21 +54,21 @@ public class GlobalExceptionHandler {
     @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
     public RestResponse handleException(Exception e) {
         log.info("Internal server error:", e);
-        return RestResponse.fail("internal server error: " + e.getMessage(), ResponseCode.CODE_FAIL);
+        return RestResponse.fail(INTERNAL_SERVER_ERROR_ARGS, e.getMessage());

Review Comment:
   `RestResponse.fail(...)` In many cases, a very specific business exception is returned, in short, the exception information of this error cannot be extracted uniformly and predetermined in advance.



-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] wolfboys commented on pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
wolfboys commented on PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#issuecomment-1278481641

   I recommend a pr to do only one thing, for example, the current pr, to solve LDAP-related issue, if you want to improve the response status code, you can open a new pr


-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] MonsterChenzhuo commented on pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
MonsterChenzhuo commented on PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#issuecomment-1280535472

   > I recommend a pr to do only one thing, for example, the current pr, to solve LDAP-related issue, if you want to improve the response status code, you can open a new pr
   
   ok,no problem


-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] MonsterChenzhuo commented on a diff in pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
MonsterChenzhuo commented on code in PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#discussion_r997675635


##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/PassportController.java:
##########
@@ -67,39 +67,22 @@ public class PassportController {
     public RestResponse signin(
         @NotBlank(message = "{required}") String username,
         @NotBlank(message = "{required}") String password) throws Exception {
-
         if (StringUtils.isEmpty(username)) {
             return RestResponse.success().put("code", 0);
         }
-
         User user = authenticator.authenticate(username, password);
+        return login(username, password, user);
+    }
 
-        if (user == null) {
+    @PostMapping("ldapSignin")
+    public RestResponse ldapSignin(

Review Comment:
   The documentation is already available



-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] MonsterChenzhuo commented on a diff in pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
MonsterChenzhuo commented on code in PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#discussion_r997676135


##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/PassportController.java:
##########
@@ -67,39 +67,22 @@ public class PassportController {
     public RestResponse signin(
         @NotBlank(message = "{required}") String username,
         @NotBlank(message = "{required}") String password) throws Exception {
-
         if (StringUtils.isEmpty(username)) {
             return RestResponse.success().put("code", 0);
         }
-
         User user = authenticator.authenticate(username, password);
+        return login(username, password, user);
+    }
 
-        if (user == null) {
+    @PostMapping("ldapSignin")
+    public RestResponse ldapSignin(

Review Comment:
   <img width="977" alt="图片" src="https://user-images.githubusercontent.com/60029759/196326538-ef1cb77a-d436-471d-8500-36d2107e45f1.png">
   



-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#discussion_r997661128


##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/PassportController.java:
##########
@@ -67,39 +67,22 @@ public class PassportController {
     public RestResponse signin(
         @NotBlank(message = "{required}") String username,
         @NotBlank(message = "{required}") String password) throws Exception {
-
         if (StringUtils.isEmpty(username)) {
             return RestResponse.success().put("code", 0);
         }
-
         User user = authenticator.authenticate(username, password);
+        return login(username, password, user);
+    }
 
-        if (user == null) {
+    @PostMapping("ldapSignin")
+    public RestResponse ldapSignin(

Review Comment:
   the front-end will leave an entry for ldap login, I did not find which interface on the front-end will request this method



-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] wolfboys commented on pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
wolfboys commented on PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#issuecomment-1278480140

   hi MonsterChenzhuo:
   
   unify the definition of several of the most common exceptions, very good, but some business exceptions, the definition out of the unreasonable. Too much information about business exceptions, can not be predetermined in advance.


-- 
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: issues-unsubscribe@streampark.apache.org

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


[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1836: [Improve]Splitting ldap logic

Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1836:
URL: https://github.com/apache/incubator-streampark/pull/1836#discussion_r995772420


##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/security/impl/AuthenticatorImpl.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.streampark.console.system.security.impl;
+
+import org.apache.streampark.console.base.util.ShaHashUtils;
+import org.apache.streampark.console.core.enums.UserType;
+import org.apache.streampark.console.system.entity.User;
+import org.apache.streampark.console.system.security.Authenticator;
+import org.apache.streampark.console.system.security.impl.ldap.LdapService;
+import org.apache.streampark.console.system.service.UserService;
+
+import org.apache.commons.lang3.StringUtils;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import java.util.Date;
+
+@Component
+public class AuthenticatorImpl implements Authenticator {
+    @Autowired
+    private UserService usersService;
+    @Autowired
+    private LdapService ldapService;
+
+    @Override
+    public User authenticate(String username, String password) {
+        User user = usersService.findByName(username);
+        if (user == null) {
+            return null;
+        }
+        String salt = user.getSalt();
+        password = ShaHashUtils.encrypt(salt, password);
+        if (!StringUtils.equals(user.getPassword(), password)) {
+            return null;
+        }
+        return user;
+    }
+
+    @Override
+    public User ldapAuthenticate(String username, String password) throws Exception {
+        User user = null;
+        String ldapEmail = ldapService.ldapLogin(username, password);
+        if (ldapEmail != null) {
+            //check if user exist
+            user = usersService.findByName(username);
+            if (user == null && ldapService.createIfUserNotExists()) {
+                User newUser = new User();
+                newUser.setCreateTime(new Date());
+                newUser.setUsername(username);
+                newUser.setNickName(username);
+                newUser.setUserType(UserType.USER);
+                newUser.setStatus("1");
+                newUser.setSex("1");
+
+                String salt = ShaHashUtils.getRandomSalt();
+                String saltPass = ShaHashUtils.encrypt(salt, password);
+                newUser.setSalt(salt);
+                newUser.setPassword(saltPass);
+                usersService.createUser(newUser);
+                return newUser;
+            }
+        }
+        return user;

Review Comment:
   ```suggestion
           String ldapEmail = ldapService.ldapLogin(username, password);
           if (ldapEmail == null) {
               return null;
           }
   
           //check if user exist
           User user = usersService.findByName(username);
           if (user != null || !ldapService.createIfUserNotExists()) {
               return user;
           }
           User newUser = new User();
           newUser.setCreateTime(new Date());
           newUser.setUsername(username);
           newUser.setNickName(username);
           newUser.setUserType(UserType.USER);
           newUser.setStatus("1");
           newUser.setSex("1");
   
           String salt = ShaHashUtils.getRandomSalt();
           String saltPass = ShaHashUtils.encrypt(salt, password);
           newUser.setSalt(salt);
           newUser.setPassword(saltPass);
           usersService.createUser(newUser);
           return newUser;
   ```
   
   Hi @MonsterChenzhuo , thanks for your great contribution. I have left a minor comment, it can reduce the tab.



-- 
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: issues-unsubscribe@streampark.apache.org

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