You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2021/01/28 13:30:01 UTC

[GitHub] [sling-org-apache-sling-serviceusermapper] karlpauls opened a new pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

karlpauls opened a new pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2


   


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

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] commented on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769058127


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='58.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [58.3% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] karlpauls commented on a change in pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
karlpauls commented on a change in pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#discussion_r566223492



##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -85,7 +87,13 @@
                      "default mapping is applied which uses the service user \"serviceuser--\" + bundleId + [\"--\" + subServiceName]")
         boolean user_enable_default_mapping() default true;
 
+        @AttributeDefinition(name = "Require Validation",
+                description = "Do validators have to be present and validate the service users?")

Review comment:
       yup
   




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

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] removed a comment on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769078058


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='60.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [60.7% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] karlpauls commented on a change in pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
karlpauls commented on a change in pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#discussion_r566223624



##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -487,18 +487,24 @@ private boolean isValidUser(final String userId, final String serviceName, final
             return false;
         }
         List<ServiceUserValidator> validators = getUserValidators();
-        if (!validators.isEmpty()) {
+        if (validators.isEmpty()) {
+            if (require) {
+                log.debug("isValidUser: No active validators for userId [{}] and require -> invalid", userId);
+                return false;
+            } else {
+                log.debug("isValidUser: No active validators for userId [{}] -> valid", userId);

Review comment:
       done

##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -487,18 +487,24 @@ private boolean isValidUser(final String userId, final String serviceName, final
             return false;
         }
         List<ServiceUserValidator> validators = getUserValidators();
-        if (!validators.isEmpty()) {
+        if (validators.isEmpty()) {
+            if (require) {
+                log.debug("isValidUser: No active validators for userId [{}] and require -> invalid", userId);

Review comment:
       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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] commented on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769054061


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='58.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [58.3% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] removed a comment on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769054975


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='58.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [58.3% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] karlpauls commented on a change in pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
karlpauls commented on a change in pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#discussion_r566224618



##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -85,7 +87,13 @@
                      "default mapping is applied which uses the service user \"serviceuser--\" + bundleId + [\"--\" + subServiceName]")
         boolean user_enable_default_mapping() default true;
 
+        @AttributeDefinition(name = "Require Validation",
+                description = "Do validators have to be present and validate the service users?")
         boolean require_validation() default false;
+
+        @AttributeDefinition(name = "Required Validators",
+                description = "A list of required validators ids. If not present and require validation is on not user will be valid")

Review comment:
       I reworded 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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] commented on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769054975


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='58.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [58.3% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] anchela commented on a change in pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
anchela commented on a change in pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#discussion_r566112312



##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -455,38 +486,54 @@ private boolean isValidUser(final String userId, final String serviceName, final
             log.debug("isValidUser: userId is null -> invalid");
             return false;
         }
-        if ( !userValidators.isEmpty()  || require) {
-            for (final ServiceUserValidator validator : userValidators) {
-                if ( validator.isValid(userId, serviceName, subServiceName) ) {
-                    log.debug("isValidUser: Validator {} accepts userId [{}] -> valid", validator, userId);
-                    return true;
+        List<ServiceUserValidator> validators = getUserValidators();
+        if (!validators.isEmpty()) {
+            for (final ServiceUserValidator validator : validators) {
+                if (!validator.isValid(userId, serviceName, subServiceName)) {
+                    log.debug("isValidUser: Validator {} doesn't accept userId [{}] -> invalid", validator, userId);
+                    return false;
                 }
             }
-            log.debug("isValidUser: No validator accepted userId [{}] -> invalid", userId);
-            return false;
+            log.debug("isValidUser: All validators accepted userId [{}] -> valid", userId);
         } else {
             log.debug("isValidUser: No active validators for userId [{}] -> valid", userId);
-            return true;
         }
+        return !require || !validators.isEmpty();
     }
 
     private boolean areValidPrincipals(final Iterable<String> principalNames, final String serviceName, final String subServiceName, boolean require) {
         if (principalNames == null) {
             log.debug("areValidPrincipals: principalNames are null -> invalid");
             return false;
         }
-        if ( !principalsValidators.isEmpty() || require ) {
-            for (final ServicePrincipalsValidator validator : principalsValidators) {
-                if ( validator.isValid(principalNames, serviceName, subServiceName) ) {
-                    log.debug("areValidPrincipals: Validator {} accepts principal names [{}] -> valid", validator, principalNames);
-                    return true;
+        List<ServicePrincipalsValidator> validators = getPrincipalsValidators();
+        if (!validators.isEmpty() || require) {
+            for (final ServicePrincipalsValidator validator : validators) {
+                if (!validator.isValid(principalNames, serviceName, subServiceName)) {
+                    log.debug("areValidPrincipals: Validator {} doesn't accept principal names [{}] -> invalid", validator, principalNames);
+                    return false;
                 }
             }
-            log.debug("areValidPrincipals: No validator accepted principal names [{}] -> invalid", principalNames);
-            return false;
+            log.debug("areValidPrincipals: All validators accepted principal names [{}] -> valid", principalNames);
         } else {
             log.debug("areValidPrincipals: No active validators for principal names [{}] -> valid", principalNames);
-            return true;
+        }
+        return !require || !validators.isEmpty();
+    }
+
+    private List<ServiceUserValidator> getUserValidators() {

Review comment:
       maybe add NotNull annotation

##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -455,38 +486,54 @@ private boolean isValidUser(final String userId, final String serviceName, final
             log.debug("isValidUser: userId is null -> invalid");
             return false;
         }
-        if ( !userValidators.isEmpty()  || require) {
-            for (final ServiceUserValidator validator : userValidators) {
-                if ( validator.isValid(userId, serviceName, subServiceName) ) {
-                    log.debug("isValidUser: Validator {} accepts userId [{}] -> valid", validator, userId);
-                    return true;
+        List<ServiceUserValidator> validators = getUserValidators();
+        if (!validators.isEmpty()) {
+            for (final ServiceUserValidator validator : validators) {
+                if (!validator.isValid(userId, serviceName, subServiceName)) {
+                    log.debug("isValidUser: Validator {} doesn't accept userId [{}] -> invalid", validator, userId);
+                    return false;
                 }
             }
-            log.debug("isValidUser: No validator accepted userId [{}] -> invalid", userId);
-            return false;
+            log.debug("isValidUser: All validators accepted userId [{}] -> valid", userId);
         } else {
             log.debug("isValidUser: No active validators for userId [{}] -> valid", userId);
-            return true;
         }
+        return !require || !validators.isEmpty();
     }
 
     private boolean areValidPrincipals(final Iterable<String> principalNames, final String serviceName, final String subServiceName, boolean require) {
         if (principalNames == null) {
             log.debug("areValidPrincipals: principalNames are null -> invalid");
             return false;
         }
-        if ( !principalsValidators.isEmpty() || require ) {
-            for (final ServicePrincipalsValidator validator : principalsValidators) {
-                if ( validator.isValid(principalNames, serviceName, subServiceName) ) {
-                    log.debug("areValidPrincipals: Validator {} accepts principal names [{}] -> valid", validator, principalNames);
-                    return true;
+        List<ServicePrincipalsValidator> validators = getPrincipalsValidators();
+        if (!validators.isEmpty() || require) {
+            for (final ServicePrincipalsValidator validator : validators) {
+                if (!validator.isValid(principalNames, serviceName, subServiceName)) {
+                    log.debug("areValidPrincipals: Validator {} doesn't accept principal names [{}] -> invalid", validator, principalNames);
+                    return false;
                 }
             }
-            log.debug("areValidPrincipals: No validator accepted principal names [{}] -> invalid", principalNames);
-            return false;
+            log.debug("areValidPrincipals: All validators accepted principal names [{}] -> valid", principalNames);
         } else {
             log.debug("areValidPrincipals: No active validators for principal names [{}] -> valid", principalNames);
-            return true;
+        }
+        return !require || !validators.isEmpty();
+    }
+
+    private List<ServiceUserValidator> getUserValidators() {
+        if (presentValidators.containsAll(requiredValidators)) {
+            return userValidators;
+        } else {
+            return Collections.emptyList();
+        }
+    }
+
+    private List<ServicePrincipalsValidator> getPrincipalsValidators() {

Review comment:
       maybe add NotNull annotation
   also that's almost a duplication of getUserValidators.

##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -455,38 +486,54 @@ private boolean isValidUser(final String userId, final String serviceName, final
             log.debug("isValidUser: userId is null -> invalid");
             return false;
         }
-        if ( !userValidators.isEmpty()  || require) {
-            for (final ServiceUserValidator validator : userValidators) {
-                if ( validator.isValid(userId, serviceName, subServiceName) ) {
-                    log.debug("isValidUser: Validator {} accepts userId [{}] -> valid", validator, userId);
-                    return true;
+        List<ServiceUserValidator> validators = getUserValidators();
+        if (!validators.isEmpty()) {
+            for (final ServiceUserValidator validator : validators) {
+                if (!validator.isValid(userId, serviceName, subServiceName)) {
+                    log.debug("isValidUser: Validator {} doesn't accept userId [{}] -> invalid", validator, userId);
+                    return false;
                 }
             }
-            log.debug("isValidUser: No validator accepted userId [{}] -> invalid", userId);
-            return false;
+            log.debug("isValidUser: All validators accepted userId [{}] -> valid", userId);
         } else {
             log.debug("isValidUser: No active validators for userId [{}] -> valid", userId);
-            return true;
         }
+        return !require || !validators.isEmpty();

Review comment:
       same as below

##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -85,7 +87,13 @@
                      "default mapping is applied which uses the service user \"serviceuser--\" + bundleId + [\"--\" + subServiceName]")
         boolean user_enable_default_mapping() default true;
 
+        @AttributeDefinition(name = "Require Validation",
+                description = "Do validators have to be present and validate the service users?")
         boolean require_validation() default false;
+
+        @AttributeDefinition(name = "Required Validators",
+                description = "A list of required validators ids. If not present and require validation is on not user will be valid")

Review comment:
       that's a bit hard to read.... i would either put on in quotes 'on' or replace _on_ by _enabled_.
   then: 'not user will be valid'. shouldn't that read 'no userid and no principal name will be valid'?

##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -455,38 +486,54 @@ private boolean isValidUser(final String userId, final String serviceName, final
             log.debug("isValidUser: userId is null -> invalid");
             return false;
         }
-        if ( !userValidators.isEmpty()  || require) {
-            for (final ServiceUserValidator validator : userValidators) {
-                if ( validator.isValid(userId, serviceName, subServiceName) ) {
-                    log.debug("isValidUser: Validator {} accepts userId [{}] -> valid", validator, userId);
-                    return true;
+        List<ServiceUserValidator> validators = getUserValidators();
+        if (!validators.isEmpty()) {
+            for (final ServiceUserValidator validator : validators) {
+                if (!validator.isValid(userId, serviceName, subServiceName)) {
+                    log.debug("isValidUser: Validator {} doesn't accept userId [{}] -> invalid", validator, userId);
+                    return false;
                 }
             }
-            log.debug("isValidUser: No validator accepted userId [{}] -> invalid", userId);
-            return false;
+            log.debug("isValidUser: All validators accepted userId [{}] -> valid", userId);
         } else {
             log.debug("isValidUser: No active validators for userId [{}] -> valid", userId);
-            return true;
         }
+        return !require || !validators.isEmpty();
     }
 
     private boolean areValidPrincipals(final Iterable<String> principalNames, final String serviceName, final String subServiceName, boolean require) {
         if (principalNames == null) {
             log.debug("areValidPrincipals: principalNames are null -> invalid");
             return false;
         }
-        if ( !principalsValidators.isEmpty() || require ) {
-            for (final ServicePrincipalsValidator validator : principalsValidators) {
-                if ( validator.isValid(principalNames, serviceName, subServiceName) ) {
-                    log.debug("areValidPrincipals: Validator {} accepts principal names [{}] -> valid", validator, principalNames);
-                    return true;
+        List<ServicePrincipalsValidator> validators = getPrincipalsValidators();
+        if (!validators.isEmpty() || require) {
+            for (final ServicePrincipalsValidator validator : validators) {
+                if (!validator.isValid(principalNames, serviceName, subServiceName)) {
+                    log.debug("areValidPrincipals: Validator {} doesn't accept principal names [{}] -> invalid", validator, principalNames);
+                    return false;
                 }
             }
-            log.debug("areValidPrincipals: No validator accepted principal names [{}] -> invalid", principalNames);
-            return false;
+            log.debug("areValidPrincipals: All validators accepted principal names [{}] -> valid", principalNames);
         } else {
             log.debug("areValidPrincipals: No active validators for principal names [{}] -> valid", principalNames);
-            return true;
+        }
+        return !require || !validators.isEmpty();

Review comment:
       that's a bit hard to read.... i think it would be easier if 'validators' would be renamed to reflect the fact that this are only the required-and-satisfied-validators..... in other words.... some hint that allows to easily understand that if 'validators' are empty not all required ones are present.




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

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



[GitHub] [sling-org-apache-sling-serviceusermapper] karlpauls commented on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
karlpauls commented on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769216323


   @cziegeler - PTAL 


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

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] commented on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769262249


   Kudos, SonarCloud Quality Gate passed!
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='86.6%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [86.6% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] karlpauls commented on a change in pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
karlpauls commented on a change in pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#discussion_r566224484



##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -455,38 +486,54 @@ private boolean isValidUser(final String userId, final String serviceName, final
             log.debug("isValidUser: userId is null -> invalid");
             return false;
         }
-        if ( !userValidators.isEmpty()  || require) {
-            for (final ServiceUserValidator validator : userValidators) {
-                if ( validator.isValid(userId, serviceName, subServiceName) ) {
-                    log.debug("isValidUser: Validator {} accepts userId [{}] -> valid", validator, userId);
-                    return true;
+        List<ServiceUserValidator> validators = getUserValidators();
+        if (!validators.isEmpty()) {
+            for (final ServiceUserValidator validator : validators) {
+                if (!validator.isValid(userId, serviceName, subServiceName)) {
+                    log.debug("isValidUser: Validator {} doesn't accept userId [{}] -> invalid", validator, userId);
+                    return false;
                 }
             }
-            log.debug("isValidUser: No validator accepted userId [{}] -> invalid", userId);
-            return false;
+            log.debug("isValidUser: All validators accepted userId [{}] -> valid", userId);
         } else {
             log.debug("isValidUser: No active validators for userId [{}] -> valid", userId);
-            return true;
         }
+        return !require || !validators.isEmpty();
     }
 
     private boolean areValidPrincipals(final Iterable<String> principalNames, final String serviceName, final String subServiceName, boolean require) {
         if (principalNames == null) {
             log.debug("areValidPrincipals: principalNames are null -> invalid");
             return false;
         }
-        if ( !principalsValidators.isEmpty() || require ) {
-            for (final ServicePrincipalsValidator validator : principalsValidators) {
-                if ( validator.isValid(principalNames, serviceName, subServiceName) ) {
-                    log.debug("areValidPrincipals: Validator {} accepts principal names [{}] -> valid", validator, principalNames);
-                    return true;
+        List<ServicePrincipalsValidator> validators = getPrincipalsValidators();
+        if (!validators.isEmpty() || require) {
+            for (final ServicePrincipalsValidator validator : validators) {
+                if (!validator.isValid(principalNames, serviceName, subServiceName)) {
+                    log.debug("areValidPrincipals: Validator {} doesn't accept principal names [{}] -> invalid", validator, principalNames);
+                    return false;
                 }
             }
-            log.debug("areValidPrincipals: No validator accepted principal names [{}] -> invalid", principalNames);
-            return false;
+            log.debug("areValidPrincipals: All validators accepted principal names [{}] -> valid", principalNames);
         } else {
             log.debug("areValidPrincipals: No active validators for principal names [{}] -> valid", principalNames);
-            return true;
+        }
+        return !require || !validators.isEmpty();
+    }
+
+    private List<ServiceUserValidator> getUserValidators() {
+        if (presentValidators.containsAll(requiredValidators)) {
+            return userValidators;
+        } else {
+            return Collections.emptyList();
+        }
+    }
+
+    private List<ServicePrincipalsValidator> getPrincipalsValidators() {

Review comment:
       I changed it to reuse a common base methods - we don't have annotations in here yet.




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

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



[GitHub] [sling-org-apache-sling-serviceusermapper] bdelacretaz commented on a change in pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
bdelacretaz commented on a change in pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#discussion_r566140137



##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -85,7 +87,13 @@
                      "default mapping is applied which uses the service user \"serviceuser--\" + bundleId + [\"--\" + subServiceName]")
         boolean user_enable_default_mapping() default true;
 
+        @AttributeDefinition(name = "Require Validation",
+                description = "Do validators have to be present and validate the service users?")

Review comment:
       To be consistent with the suggested description of "Required Validators" this might be
   
   > If true, a service user is only valid if all the validators listed in the 'required validators' accept 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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] commented on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769083568


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='64.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [64.3% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] commented on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769349040


   Kudos, SonarCloud Quality Gate passed!
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='86.6%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [86.6% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] removed a comment on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769058127


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='58.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [58.3% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] removed a comment on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769083568


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='64.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [64.3% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] commented on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769078058


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='60.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [60.7% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] removed a comment on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769239973


   Kudos, SonarCloud Quality Gate passed!
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='86.6%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [86.6% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] karlpauls merged pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
karlpauls merged pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2


   


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

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



[GitHub] [sling-org-apache-sling-serviceusermapper] bdelacretaz commented on a change in pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
bdelacretaz commented on a change in pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#discussion_r566137928



##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -85,7 +87,13 @@
                      "default mapping is applied which uses the service user \"serviceuser--\" + bundleId + [\"--\" + subServiceName]")
         boolean user_enable_default_mapping() default true;
 
+        @AttributeDefinition(name = "Require Validation",
+                description = "Do validators have to be present and validate the service users?")
         boolean require_validation() default false;
+
+        @AttributeDefinition(name = "Required Validators",
+                description = "A list of required validators ids. If not present and require validation is on not user will be valid")

Review comment:
       If would also add quotes on 'require validation', so maybe
   
   > A list of required validators IDs. If this is empty and 'require validation' is enabled, validation will fail for all users and principals




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

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] commented on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769096584


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='64.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [64.3% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] anchela commented on a change in pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
anchela commented on a change in pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#discussion_r566129215



##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -487,18 +487,24 @@ private boolean isValidUser(final String userId, final String serviceName, final
             return false;
         }
         List<ServiceUserValidator> validators = getUserValidators();
-        if (!validators.isEmpty()) {
+        if (validators.isEmpty()) {
+            if (require) {
+                log.debug("isValidUser: No active validators for userId [{}] and require -> invalid", userId);
+                return false;
+            } else {
+                log.debug("isValidUser: No active validators for userId [{}] -> valid", userId);

Review comment:
       same here:
   `"isValidUser: No active validators for userId '{}' -> valid", userId`
   instead of [] around the id.

##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -487,18 +487,24 @@ private boolean isValidUser(final String userId, final String serviceName, final
             return false;
         }
         List<ServiceUserValidator> validators = getUserValidators();
-        if (!validators.isEmpty()) {
+        if (validators.isEmpty()) {
+            if (require) {
+                log.debug("isValidUser: No active validators for userId [{}] and require -> invalid", userId);

Review comment:
       i would put the userid in single quotes '{}' instead of using the [] brackets.




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

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] removed a comment on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769054061


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='58.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [58.3% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] removed a comment on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769262249


   Kudos, SonarCloud Quality Gate passed!
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='86.6%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [86.6% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] removed a comment on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769096584


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='64.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [64.3% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] sonarcloud[bot] commented on pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#issuecomment-769239973


   Kudos, SonarCloud Quality Gate passed!
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='86.6%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list) [86.6% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-serviceusermapper&pullRequest=2&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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] bdelacretaz commented on a change in pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
bdelacretaz commented on a change in pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#discussion_r566140137



##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -85,7 +87,13 @@
                      "default mapping is applied which uses the service user \"serviceuser--\" + bundleId + [\"--\" + subServiceName]")
         boolean user_enable_default_mapping() default true;
 
+        @AttributeDefinition(name = "Require Validation",
+                description = "Do validators have to be present and validate the service users?")

Review comment:
       To be consistent with the suggested description of "Required Validators" this might be
   
   > If true, a service user is only valid if all the validators listed in the 'required validators' parameter accept 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.

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



[GitHub] [sling-org-apache-sling-serviceusermapper] bdelacretaz commented on a change in pull request #2: SLING-10099 : Validators should all agree for a service user to be valid and should be configurable

Posted by GitBox <gi...@apache.org>.
bdelacretaz commented on a change in pull request #2:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/2#discussion_r566137928



##########
File path: src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
##########
@@ -85,7 +87,13 @@
                      "default mapping is applied which uses the service user \"serviceuser--\" + bundleId + [\"--\" + subServiceName]")
         boolean user_enable_default_mapping() default true;
 
+        @AttributeDefinition(name = "Require Validation",
+                description = "Do validators have to be present and validate the service users?")
         boolean require_validation() default false;
+
+        @AttributeDefinition(name = "Required Validators",
+                description = "A list of required validators ids. If not present and require validation is on not user will be valid")

Review comment:
       If would also add quotes on 'require validation', so maybe
   
   > A list of required validators IDs. If this is empty and 'require validation' is enabled, validation fails for all service users




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

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