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/06/15 06:46:29 UTC

[GitHub] [dolphinscheduler] liqingwang opened a new pull request, #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

liqingwang opened a new pull request, #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451

   <!--Thanks very much for contributing to Apache DolphinScheduler. Please review https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html before opening a pull request.-->
   
   
   ## Purpose of the pull request
   
   When you log in with LDAP and are not a ds user, it will create a new user automatedly. 
   
   But in my option, LDAP login is the same as user-password log-in, if you are not a ds user, your login will be rejected.
   
   I think we can add a new config field, user can choose if they log in with LDAP and are not a ds user, which will create a new user or be rejected.
   
   <!--(For example: This pull request adds checkstyle plugin).-->
   
   ## Brief change log
   
   <!--*(for example:)*
     - *Add maven-checkstyle-plugin to root pom.xml*
   -->
   ## Verify this pull request
   
   <!--*(Please pick either of the following options)*-->
   
   This change added tests and can be verified as follows:
   
   <!--*(example:)*
     - *Added dolphinscheduler-dao tests for end-to-end.*
     - *Added CronUtilsTest to verify the change.*
     - *Manually verified the change by testing locally.* -->
   
     modified:   dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/security/SecurityConfigLDAPTest.java
     modified:   dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/security/impl/ldap/LdapAuthenticatorTest.java
   
   part of #10425 


-- 
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

[GitHub] [dolphinscheduler] liqingwang commented on a diff in pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
liqingwang commented on code in PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451#discussion_r897656610


##########
dolphinscheduler-api/src/main/resources/application.yaml:
##########
@@ -143,6 +143,8 @@ security:
       password: password
       user.identity.attribute: uid
       user.email.attribute: mail
+      # action when ldap user is not exist (supported types: CREATE,DENY)
+      user.not.exist.action: CREATE

Review Comment:
   Thanks for your reminder. I have reorganized the YAML configs. And the default policy is `create`, when a user does not exists will auto create a user when using LDAP log-in.



-- 
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

[GitHub] [dolphinscheduler] liqingwang commented on a diff in pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
liqingwang commented on code in PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451#discussion_r899721325


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/security/impl/ldap/LdapAuthenticator.java:
##########
@@ -37,7 +38,10 @@ public User login(String userId, String password, String extra) {
             //check if user exist
             user = usersService.getUserByUserName(userId);
             if (user == null) {
-                user = usersService.createUser(ldapService.getUserType(userId), userId, ldapEmail);
+                LdapUserNotExistActionType type = ldapService.getLdapUserNotExistAction();
+                if(type == LdapUserNotExistActionType.CREATE){
+                    user = usersService.createUser(ldapService.getUserType(userId), userId, ldapEmail);
+                }

Review Comment:
   Yeah, It will be more elegant, Done.



-- 
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

[GitHub] [dolphinscheduler] EricGao888 commented on a diff in pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
EricGao888 commented on code in PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451#discussion_r897607181


##########
docs/docs/en/architecture/configuration.md:
##########
@@ -226,6 +226,7 @@ security.authentication.ldap.username|cn=read-only-admin,dc=example,dc=com|LDAP
 security.authentication.ldap.password|password|LDAP password
 security.authentication.ldap.user.identity.attribute|uid|LDAP user identity attribute 
 security.authentication.ldap.user.email.attribute|mail|LDAP user email attribute
+security.authentication.ldap.user.not.exist.action|CREATION|action when LDAP user is not exist

Review Comment:
   What about using `CREATE` instead of `CREATION` to maintain consistency with `DENY`? Since `DENY` is used instead of `DENIAL`. BTW, will it be better to add a line to explain the `DENY` option in docs?
   



-- 
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

[GitHub] [dolphinscheduler] liqingwang commented on a diff in pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
liqingwang commented on code in PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451#discussion_r897625370


##########
docs/docs/en/architecture/configuration.md:
##########
@@ -226,6 +226,7 @@ security.authentication.ldap.username|cn=read-only-admin,dc=example,dc=com|LDAP
 security.authentication.ldap.password|password|LDAP password
 security.authentication.ldap.user.identity.attribute|uid|LDAP user identity attribute 
 security.authentication.ldap.user.email.attribute|mail|LDAP user email attribute
+security.authentication.ldap.user.not.exist.action|CREATION|action when LDAP user is not exist

Review Comment:
   I agree with you. Done.



-- 
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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451#issuecomment-1158523661

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10451)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL)
   
   [![81.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '81.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_coverage&view=list) [81.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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

[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451#issuecomment-1156065276

   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10451?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#10451](https://codecov.io/gh/apache/dolphinscheduler/pull/10451?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ffc68d3) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/50846760e5ab5f68cb708edc72725b235e933a34?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5084676) will **increase** coverage by `0.32%`.
   > The diff coverage is `78.57%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev   #10451      +/-   ##
   ============================================
   + Coverage     40.59%   40.91%   +0.32%     
   - Complexity     4778     4855      +77     
   ============================================
     Files           878      884       +6     
     Lines         35747    35983     +236     
     Branches       3970     3992      +22     
   ============================================
   + Hits          14512    14723     +211     
   - Misses        19789    19804      +15     
   - Partials       1446     1456      +10     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/10451?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...nscheduler/api/security/impl/ldap/LdapService.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlY3VyaXR5L2ltcGwvbGRhcC9MZGFwU2VydmljZS5qYXZh) | `7.31% <25.00%> (+1.91%)` | :arrow_up: |
   | [...duler/api/security/LdapUserNotExistActionType.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlY3VyaXR5L0xkYXBVc2VyTm90RXhpc3RBY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (ø)` | |
   | [...uler/api/security/impl/ldap/LdapAuthenticator.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlY3VyaXR5L2ltcGwvbGRhcC9MZGFwQXV0aGVudGljYXRvci5qYXZh) | `100.00% <100.00%> (ø)` | |
   | [.../org/apache/dolphinscheduler/api/enums/Status.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2VudW1zL1N0YXR1cy5qYXZh) | `100.00% <0.00%> (ø)` | |
   | [...r/plugin/task/sqoop/parameter/SqoopParameters.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stc3Fvb3Avc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svc3Fvb3AvcGFyYW1ldGVyL1Nxb29wUGFyYW1ldGVycy5qYXZh) | `53.33% <0.00%> (ø)` | |
   | [...rg/apache/dolphinscheduler/api/dto/ClusterDto.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2R0by9DbHVzdGVyRHRvLmphdmE=) | `92.85% <0.00%> (ø)` | |
   | [...olphinscheduler/common/utils/ClusterConfUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL0NsdXN0ZXJDb25mVXRpbHMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...scheduler/api/service/impl/ClusterServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9DbHVzdGVyU2VydmljZUltcGwuamF2YQ==) | `88.28% <0.00%> (ø)` | |
   | [...hinscheduler/api/controller/ClusterController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvQ2x1c3RlckNvbnRyb2xsZXIuamF2YQ==) | `89.47% <0.00%> (ø)` | |
   | [...rg/apache/dolphinscheduler/dao/entity/Cluster.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYW8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvZGFvL2VudGl0eS9DbHVzdGVyLmphdmE=) | `100.00% <0.00%> (ø)` | |
   | ... and [1 more](https://codecov.io/gh/apache/dolphinscheduler/pull/10451/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10451?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10451?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5084676...ffc68d3](https://codecov.io/gh/apache/dolphinscheduler/pull/10451?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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

[GitHub] [dolphinscheduler] kezhenxu94 commented on a diff in pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451#discussion_r897630215


##########
dolphinscheduler-api/src/main/resources/application.yaml:
##########
@@ -143,6 +143,8 @@ security:
       password: password
       user.identity.attribute: uid
       user.email.attribute: mail
+      # action when ldap user is not exist (supported types: CREATE,DENY)
+      user.not.exist.action: CREATE

Review Comment:
   1. Please use YAML hierarchy to re-organize the configs (including `user.email.attribute`, `user.identity.attribute`, `user.admin`), and you should rename the config a little bit (not using `.` to split as in YAML that will become a new section), an example below
   
   ```suggestion
         user:
           admin: ""
           email-attribute: ""
           identity-attribute: ""
           absent-policy: CREATE
   ```
   
   2. When adding a new config, make sure its default value is the same as the one before you add this config. Previously the policy is `DENY` so you should  set the default value of the new config as `DENY`, so that users don't face different behaviors after upgrading.



-- 
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

[GitHub] [dolphinscheduler] liqingwang commented on pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
liqingwang commented on PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451#issuecomment-1157143259

   PTAL @zhongjiajie  @caishunfeng 


-- 
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

[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451#discussion_r899712941


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/security/impl/ldap/LdapAuthenticator.java:
##########
@@ -37,7 +38,10 @@ public User login(String userId, String password, String extra) {
             //check if user exist
             user = usersService.getUserByUserName(userId);
             if (user == null) {
-                user = usersService.createUser(ldapService.getUserType(userId), userId, ldapEmail);
+                LdapUserNotExistActionType type = ldapService.getLdapUserNotExistAction();
+                if(type == LdapUserNotExistActionType.CREATE){
+                    user = usersService.createUser(ldapService.getUserType(userId), userId, ldapEmail);
+                }

Review Comment:
   NIT, Since we only check whether ldap action is created now, maybe we can add a function named `createIfUserNotExists` in `ladService`  and we here can use
   
   ```suggestion
                   if(ldapService.createIfUserNotExists()){
                       user = usersService.createUser(ldapService.getUserType(userId), userId, ldapEmail);
                   }
   ```



-- 
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

[GitHub] [dolphinscheduler] zhongjiajie merged pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
zhongjiajie merged PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451


-- 
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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451#issuecomment-1156070291

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10451)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL)
   
   [![82.4%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '82.4%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_coverage&view=list) [82.4% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451#issuecomment-1158476575

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10451)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL)
   
   [![78.9%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '78.9%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_coverage&view=list) [78.9% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451#issuecomment-1156115577

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10451)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL)
   
   [![82.4%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '82.4%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_coverage&view=list) [82.4% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10451: [Improvement][LDAP]Add LDAP User Not Exist Action Config

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10451:
URL: https://github.com/apache/dolphinscheduler/pull/10451#issuecomment-1156146141

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10451)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10451&resolved=false&types=CODE_SMELL)
   
   [![82.4%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '82.4%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_coverage&view=list) [82.4% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10451&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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