Re: [PR] [ISSUES-3623] Project management encrypts user warehouse passwords [incubator-streampark]

2024-03-27 Thread via GitHub 


wolfboys merged PR #3630:
URL: https://github.com/apache/incubator-streampark/pull/3630


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [ISSUES-3623] Project management encrypts user warehouse passwords [incubator-streampark]

2024-03-27 Thread via GitHub 


jianjun159 commented on PR #3630:
URL: 
https://github.com/apache/incubator-streampark/pull/3630#issuecomment-2022281480

   > Can you provide a testcase for encryption and decryption of passwords？
   The encryption and decryption method used in this change uses the 
corresponding method in EncryotUtils that has been encapsulated in the system, 
and the corresponding EncryptUtilsTest already exists in the test class, mainly 
for the security logic of the system itself


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [ISSUES-3623] Project management encrypts user warehouse passwords [incubator-streampark]

2024-03-27 Thread via GitHub 


wolfboys commented on PR #3630:
URL: 
https://github.com/apache/incubator-streampark/pull/3630#issuecomment-2022257706

   Can you provide a testcase for encryption and decryption of passwords？


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [ISSUES-3623] Project management encrypts user warehouse passwords [incubator-streampark]

2024-03-27 Thread via GitHub 


jianjun159 commented on PR #3630:
URL: 
https://github.com/apache/incubator-streampark/pull/3630#issuecomment-2022269493

   OK, I'll work on the encryption and decryption
   
   benjobs ***@***.***> 于2024年3月27日周三 17:04写道：
   
   > Can you provide a testcase for encryption and decryption of passwords？
   >
   > —
   > Reply to this email directly, view it on GitHub
   > 
,
   > or unsubscribe
   > 

   > .
   > You are receiving this because you authored the thread.Message ID:
   > ***@***.***>
   >
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [ISSUES-3623] Project management encrypts user warehouse passwords [incubator-streampark]

2024-03-27 Thread via GitHub 


jianjun159 commented on code in PR #3630:
URL: 
https://github.com/apache/incubator-streampark/pull/3630#discussion_r1540691011


##
streampark-console/streampark-console-service/src/main/assembly/script/schema/mysql-schema.sql:
##
@@ -189,8 +189,9 @@ create table `t_flink_project` (
   `url` varchar(255) collate utf8mb4_general_ci default null,
   `branches` varchar(64) collate utf8mb4_general_ci default null,
   `user_name` varchar(64) collate utf8mb4_general_ci default null,
-  `password` varchar(64) collate utf8mb4_general_ci default null,
+  `password` varchar(512) collate utf8mb4_general_ci default null,

Review Comment:
   This situation is in response to very long passwords, and it cannot be ruled 
out that there are users whose passwords are very long



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [ISSUES-3623] Project management encrypts user warehouse passwords [incubator-streampark]

2024-03-26 Thread via GitHub 


wolfboys commented on code in PR #3630:
URL: 
https://github.com/apache/incubator-streampark/pull/3630#discussion_r1540428887


##
streampark-console/streampark-console-service/src/main/assembly/script/schema/mysql-schema.sql:
##
@@ -189,8 +189,9 @@ create table `t_flink_project` (
   `url` varchar(255) collate utf8mb4_general_ci default null,
   `branches` varchar(64) collate utf8mb4_general_ci default null,
   `user_name` varchar(64) collate utf8mb4_general_ci default null,
-  `password` varchar(64) collate utf8mb4_general_ci default null,
+  `password` varchar(512) collate utf8mb4_general_ci default null,

Review Comment:
   512 😳, I think this length is too long? Can you confirm it?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [ISSUES-3623] Project management encrypts user warehouse passwords [incubator-streampark]

2024-03-26 Thread via GitHub 


jianjun159 commented on code in PR #3630:
URL: 
https://github.com/apache/incubator-streampark/pull/3630#discussion_r1540373252


##
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/service/impl/ProjectServiceImpl.java:
##
@@ -116,6 +132,38 @@ public boolean update(Project projectParam) {
 ApiAlertException.throwIfFalse(
 !project.getBuildState().equals(BuildStateEnum.BUILDING.get()),
 "The project is being built, update project failed.");
+project.setName(projectParam.getName());
+project.setUrl(projectParam.getUrl());
+project.setBranches(projectParam.getBranches());
+project.setPrvkeyPath(projectParam.getPrvkeyPath());
+project.setUserName(projectParam.getUserName());
+if (StringUtils.isNotBlank(projectParam.getPassword())) {
+  if (StringUtils.isBlank(project.getPassword())) {
+String salt = ShaHashUtils.getRandomSalt();
+try {
+  String encrypt = EncryptUtils.encrypt(projectParam.getPassword(), 
salt);
+  project.setSalt(salt);
+  project.setPassword(encrypt);
+} catch (Exception e) {
+  log.error("project password encrypt failed");
+  throw new ApiAlertException(e);
+}
+  } else {
+// Check whether the encrypted password is the same as the current 
password
+if (!Objects.equals(project.getPassword(), 
projectParam.getPassword())) {
+  try {
+String encrypt = EncryptUtils.encrypt(projectParam.getPassword(), 
project.getSalt());
+project.setPassword(encrypt);
+  } catch (Exception e) {
+log.error("project password encrypt failed");
+throw new ApiAlertException(e);
+  }
+}
+  }
+}
+project.setPom(projectParam.getPom());
+project.setDescription(projectParam.getDescription());
+project.setBuildArgs(projectParam.getBuildArgs());
 updateInternal(projectParam, project);

Review Comment:
   The code has been optimized in a more concise direction



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [ISSUES-3623] Project management encrypts user warehouse passwords [incubator-streampark]

2024-03-26 Thread via GitHub 


jianjun159 commented on code in PR #3630:
URL: 
https://github.com/apache/incubator-streampark/pull/3630#discussion_r1540361785


##
streampark-console/streampark-console-service/src/main/assembly/script/schema/mysql-schema.sql:
##
@@ -191,6 +191,7 @@ create table `t_flink_project` (
   `user_name` varchar(64) collate utf8mb4_general_ci default null,
   `password` varchar(64) collate utf8mb4_general_ci default null,
   `prvkey_path` varchar(128) collate utf8mb4_general_ci default null,
+  `salt` varchar(26) collate utf8mb4_general_ci default null,

Review Comment:
   The salt length is used in the ShaHashUtils salt generation method, the 
default length is 26, but the password will be longer after encryption, 
resulting in database writing problems



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [ISSUES-3623] Project management encrypts user warehouse passwords [incubator-streampark]

2024-03-26 Thread via GitHub 


wolfboys commented on PR #3630:
URL: 
https://github.com/apache/incubator-streampark/pull/3630#issuecomment-2020478730

   You can execute the command to fix the checkstyle issue:
   mvn spotless:apply


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [ISSUES-3623] Project management encrypts user warehouse passwords [incubator-streampark]

2024-03-26 Thread via GitHub 


wolfboys commented on code in PR #3630:
URL: 
https://github.com/apache/incubator-streampark/pull/3630#discussion_r1539244566


##
streampark-console/streampark-console-service/src/main/assembly/script/schema/mysql-schema.sql:
##
@@ -191,6 +191,7 @@ create table `t_flink_project` (
   `user_name` varchar(64) collate utf8mb4_general_ci default null,
   `password` varchar(64) collate utf8mb4_general_ci default null,
   `prvkey_path` varchar(128) collate utf8mb4_general_ci default null,
+  `salt` varchar(26) collate utf8mb4_general_ci default null,

Review Comment:
   We need to add this field in the [upgrade 
script](https://github.com/apache/incubator-streampark/blob/dev/streampark-console/streampark-console-service/src/main/assembly/script/upgrade/mysql/2.2.0.sql)



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [ISSUES-3623] Project management encrypts user warehouse passwords [incubator-streampark]

2024-03-26 Thread via GitHub 


wolfboys commented on code in PR #3630:
URL: 
https://github.com/apache/incubator-streampark/pull/3630#discussion_r1539238080


##
streampark-console/streampark-console-service/src/main/assembly/script/schema/mysql-schema.sql:
##
@@ -191,6 +191,7 @@ create table `t_flink_project` (
   `user_name` varchar(64) collate utf8mb4_general_ci default null,
   `password` varchar(64) collate utf8mb4_general_ci default null,
   `prvkey_path` varchar(128) collate utf8mb4_general_ci default null,
+  `salt` varchar(26) collate utf8mb4_general_ci default null,

Review Comment:
   Why is the length 26? I don’t know if it is too short, can you confirm it?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [ISSUES-3623] Project management encrypts user warehouse passwords [incubator-streampark]

2024-03-26 Thread via GitHub 


wolfboys commented on code in PR #3630:
URL: 
https://github.com/apache/incubator-streampark/pull/3630#discussion_r1539234115


##
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/service/impl/ProjectServiceImpl.java:
##
@@ -94,7 +100,17 @@ public RestResponse create(Project project) {
 RestResponse response = RestResponse.success();
 
 ApiAlertException.throwIfTrue(count > 0, "project name already exists, add 
project failed");
-
+if (StringUtils.isNotBlank(project.getPassword())) {

Review Comment:
   We can make the code more cleaner:
   
   ```
   @Override
 public boolean update(Project projectParam) {
   Project project = getById(projectParam.getId());
   AssertUtils.notNull(project);
   ApiAlertException.throwIfFalse(
   project.getTeamId().equals(projectParam.getTeamId()),
   "TeamId can't be changed, update project failed.");
   ApiAlertException.throwIfFalse(
   !project.getBuildState().equals(BuildStateEnum.BUILDING.get()),
   "The project is being built, update project failed.");
   
   updateInternal(projectParam, project);
   
   if (project.isHttpRepositoryUrl()) {
 if (StringUtils.isBlank(projectParam.getUserName())) {
   project.setUserName(null);
   project.setPassword(null);
   project.setSalt(null);
 } else {
   project.setUserName(projectParam.getUserName());
   if (!Objects.equals(projectParam.getPassword(), 
project.getPassword())) {
 try {
   String salt = ShaHashUtils.getRandomSalt();
   String encrypt = 
EncryptUtils.encrypt(projectParam.getPassword(), salt);
   project.setPassword(encrypt);
   project.setSalt(salt);
 } catch (Exception e) {
   log.error("project password encrypt failed");
   throw new ApiAlertException(e);
 }
   }
 }
   }
   
   if (projectParam.getBuildState() != null) {
 project.setBuildState(projectParam.getBuildState());
 if (BuildStateEnum.NEED_REBUILD == 
BuildStateEnum.of(projectParam.getBuildState())) {
   List applications = listApps(project);
   // Update deployment status
   applications.forEach(
   (app) -> {
 log.info(
 "update deploy by project: {}, appName:{}", 
project.getName(), app.getJobName());
 app.setRelease(ReleaseStateEnum.NEED_CHECK.get());
 applicationManageService.updateRelease(app);
   });
 }
   }
   baseMapper.updateById(project);
   return true;
 }
   
 private static void updateInternal(Project projectParam, Project project) {
   project.setName(projectParam.getName());
   project.setUrl(projectParam.getUrl());
   project.setBranches(projectParam.getBranches());
   project.setPrvkeyPath(projectParam.getPrvkeyPath());
   project.setPom(projectParam.getPom());
   project.setDescription(projectParam.getDescription());
   project.setBuildArgs(projectParam.getBuildArgs());
 }
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [ISSUES-3623] Project management encrypts user warehouse passwords [incubator-streampark]

2024-03-26 Thread via GitHub 


wolfboys commented on code in PR #3630:
URL: 
https://github.com/apache/incubator-streampark/pull/3630#discussion_r1539225381


##
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/service/impl/ProjectServiceImpl.java:
##
@@ -116,6 +132,38 @@ public boolean update(Project projectParam) {
 ApiAlertException.throwIfFalse(
 !project.getBuildState().equals(BuildStateEnum.BUILDING.get()),
 "The project is being built, update project failed.");
+project.setName(projectParam.getName());
+project.setUrl(projectParam.getUrl());
+project.setBranches(projectParam.getBranches());
+project.setPrvkeyPath(projectParam.getPrvkeyPath());
+project.setUserName(projectParam.getUserName());
+if (StringUtils.isNotBlank(projectParam.getPassword())) {
+  if (StringUtils.isBlank(project.getPassword())) {
+String salt = ShaHashUtils.getRandomSalt();
+try {
+  String encrypt = EncryptUtils.encrypt(projectParam.getPassword(), 
salt);
+  project.setSalt(salt);
+  project.setPassword(encrypt);
+} catch (Exception e) {
+  log.error("project password encrypt failed");
+  throw new ApiAlertException(e);
+}
+  } else {
+// Check whether the encrypted password is the same as the current 
password
+if (!Objects.equals(project.getPassword(), 
projectParam.getPassword())) {
+  try {
+String encrypt = EncryptUtils.encrypt(projectParam.getPassword(), 
project.getSalt());
+project.setPassword(encrypt);
+  } catch (Exception e) {
+log.error("project password encrypt failed");
+throw new ApiAlertException(e);
+  }
+}
+  }
+}
+project.setPom(projectParam.getPom());
+project.setDescription(projectParam.getDescription());
+project.setBuildArgs(projectParam.getBuildArgs());
 updateInternal(projectParam, project);

Review Comment:
   The code in  135 ~ 139,164 ~ 166 is the same as the logic in the  
updateInternal method, we need delete lines 135 ~ 139,164 ~ 166 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]