You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@syncope.apache.org by GitBox <gi...@apache.org> on 2020/05/08 11:19:00 UTC

[GitHub] [syncope] DimaAy opened a new pull request #186: [SYNCOPE-1557] complete policies manipulating

DimaAy opened a new pull request #186:
URL: https://github.com/apache/syncope/pull/186


   remove
   
   ITCase attempt
   
   Cleanup + sample working test for ClientApp
   
   SyncopeServiceRegistry tests
   
   [SYNCOPE-1557] authModules  manipulating
   
   [SYNCOPE-1557] complete policies manipulating and adding more  tests


----------------------------------------------------------------
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] [syncope] mmoayyed commented on pull request #186: [SYNCOPE-1557] complete policies manipulating

Posted by GitBox <gi...@apache.org>.
mmoayyed commented on pull request #186:
URL: https://github.com/apache/syncope/pull/186#issuecomment-625821354


   Sure. I'll do a sanity check. 


----------------------------------------------------------------
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] [syncope] DimaAy commented on a change in pull request #186: [SYNCOPE-1557] complete policies manipulating

Posted by GitBox <gi...@apache.org>.
DimaAy commented on a change in pull request #186:
URL: https://github.com/apache/syncope/pull/186#discussion_r422100077



##########
File path: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/wa/WAClientAppBinderImpl.java
##########
@@ -35,15 +39,21 @@
     @Autowired
     private ClientAppDataBinder clientAppDataBinder;
 
+    @Autowired
+    private AuthModuleDAO authModuleDAO;
+
     @Override
     public WAClientApp getWAClientApp(final ClientApp clientApp) {
         WAClientApp waClientApp = new WAClientApp();
         waClientApp.setClientAppTO(clientAppDataBinder.getClientAppTO(clientApp));
 
         try {
+            AuthPolicyConf authPolicyConf = null;
             if (clientApp.getAuthPolicy() != null) {
+                authPolicyConf = clientApp.getAuthPolicy().getConf();
                 waClientApp.setAuthPolicyConf((clientApp.getAuthPolicy()).getConf());
             } else if (clientApp.getRealm().getAuthPolicy() != null) {
+                authPolicyConf = clientApp.getAuthPolicy().getConf();
                 waClientApp.setAuthPolicyConf((clientApp.getRealm().getAuthPolicy()).getConf());

Review comment:
       ah, my bad this line has to be:
   ` authPolicyConf=(clientApp.getRealm().getAuthPolicy()).getConf(); `
   as a consequence, authPoliceConf is needed in both condition 




----------------------------------------------------------------
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] [syncope] ilgrosso commented on a change in pull request #186: [SYNCOPE-1557] complete policies manipulating

Posted by GitBox <gi...@apache.org>.
ilgrosso commented on a change in pull request #186:
URL: https://github.com/apache/syncope/pull/186#discussion_r422092397



##########
File path: common/am/lib/src/main/java/org/apache/syncope/common/lib/wa/WAClientApp.java
##########
@@ -36,6 +38,8 @@
 
     private AttrReleasePolicyConf attrReleasePolicyConf;
 
+    private final Map<String, Object> releaseAttributes = new HashMap<>();

Review comment:
       please rename as `releaseAttrs` to follow the conventions

##########
File path: common/am/lib/src/main/java/org/apache/syncope/common/lib/wa/WAClientApp.java
##########
@@ -67,4 +71,8 @@ public AttrReleasePolicyConf getAttrReleasePolicyConf() {
     public void setAttrReleasePolicyConf(final AttrReleasePolicyConf attrReleasePolicyConf) {
         this.attrReleasePolicyConf = attrReleasePolicyConf;
     }
+
+    public Map<String, Object> getReleaseAttributes() {

Review comment:
       Same as above

##########
File path: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/wa/WAClientAppBinderImpl.java
##########
@@ -35,15 +39,21 @@
     @Autowired
     private ClientAppDataBinder clientAppDataBinder;
 
+    @Autowired
+    private AuthModuleDAO authModuleDAO;
+
     @Override
     public WAClientApp getWAClientApp(final ClientApp clientApp) {
         WAClientApp waClientApp = new WAClientApp();
         waClientApp.setClientAppTO(clientAppDataBinder.getClientAppTO(clientApp));
 
         try {
+            AuthPolicyConf authPolicyConf = null;
             if (clientApp.getAuthPolicy() != null) {
+                authPolicyConf = clientApp.getAuthPolicy().getConf();
                 waClientApp.setAuthPolicyConf((clientApp.getAuthPolicy()).getConf());

Review comment:
       this line must be removed

##########
File path: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/wa/WAClientAppBinderImpl.java
##########
@@ -35,15 +39,21 @@
     @Autowired
     private ClientAppDataBinder clientAppDataBinder;
 
+    @Autowired
+    private AuthModuleDAO authModuleDAO;
+
     @Override
     public WAClientApp getWAClientApp(final ClientApp clientApp) {
         WAClientApp waClientApp = new WAClientApp();
         waClientApp.setClientAppTO(clientAppDataBinder.getClientAppTO(clientApp));
 
         try {
+            AuthPolicyConf authPolicyConf = null;
             if (clientApp.getAuthPolicy() != null) {
+                authPolicyConf = clientApp.getAuthPolicy().getConf();
                 waClientApp.setAuthPolicyConf((clientApp.getAuthPolicy()).getConf());
             } else if (clientApp.getRealm().getAuthPolicy() != null) {
+                authPolicyConf = clientApp.getAuthPolicy().getConf();
                 waClientApp.setAuthPolicyConf((clientApp.getRealm().getAuthPolicy()).getConf());

Review comment:
       this line must be moved right after the subsequent `}`

##########
File path: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/wa/WAClientAppBinderImpl.java
##########
@@ -58,6 +68,26 @@ public WAClientApp getWAClientApp(final ClientApp clientApp) {
             } else if (clientApp.getRealm().getAttrReleasePolicy() != null) {
                 waClientApp.setAttrReleasePolicyConf((clientApp.getRealm().getAttrReleasePolicy()).getConf());
             }
+
+            if (authPolicyConf != null && authPolicyConf instanceof DefaultAuthPolicyConf

Review comment:
       Please remove `authPolicyConf != null &&` - `instanceof` will do the job

##########
File path: docker/wa/src/main/resources/application.properties
##########
@@ -26,8 +26,17 @@ spring.http.encoding.force=true
 
 server.servlet.contextPath=/syncope-wa
 
-spring.resources.static-locations=classpath:/syncope/static,classpath:/static
-spring.thymeleaf.templateResolverOrder=1 
+spring.resources.static-locations=classpath:/thymeleaf/static,classpath:/static

Review comment:
       Can you please check that
   * `docker/wa/src/main/resources/application.properties`
   * `docker/wa/src/main/resources/wa.properties`
   
   are aligned with
   
   * `wa/starter/src/main/resources/application.properties`
   * `wa/starter/src/main/resources/wa.properties`
   
   ?




----------------------------------------------------------------
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] [syncope] ilgrosso commented on a change in pull request #186: [SYNCOPE-1557] complete policies manipulating

Posted by GitBox <gi...@apache.org>.
ilgrosso commented on a change in pull request #186:
URL: https://github.com/apache/syncope/pull/186#discussion_r422119688



##########
File path: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/wa/WAClientAppBinderImpl.java
##########
@@ -53,7 +53,7 @@ public WAClientApp getWAClientApp(final ClientApp clientApp) {
                 authPolicyConf = clientApp.getAuthPolicy().getConf();
                 waClientApp.setAuthPolicyConf((clientApp.getAuthPolicy()).getConf());
             } else if (clientApp.getRealm().getAuthPolicy() != null) {
-                authPolicyConf = clientApp.getAuthPolicy().getConf();
+                authPolicyConf = (clientApp.getRealm().getAuthPolicy()).getConf();

Review comment:
       please remove the unneeded additional parenthesis




----------------------------------------------------------------
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] [syncope] DimaAy commented on a change in pull request #186: [SYNCOPE-1557] complete policies manipulating

Posted by GitBox <gi...@apache.org>.
DimaAy commented on a change in pull request #186:
URL: https://github.com/apache/syncope/pull/186#discussion_r422100077



##########
File path: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/wa/WAClientAppBinderImpl.java
##########
@@ -35,15 +39,21 @@
     @Autowired
     private ClientAppDataBinder clientAppDataBinder;
 
+    @Autowired
+    private AuthModuleDAO authModuleDAO;
+
     @Override
     public WAClientApp getWAClientApp(final ClientApp clientApp) {
         WAClientApp waClientApp = new WAClientApp();
         waClientApp.setClientAppTO(clientAppDataBinder.getClientAppTO(clientApp));
 
         try {
+            AuthPolicyConf authPolicyConf = null;
             if (clientApp.getAuthPolicy() != null) {
+                authPolicyConf = clientApp.getAuthPolicy().getConf();
                 waClientApp.setAuthPolicyConf((clientApp.getAuthPolicy()).getConf());
             } else if (clientApp.getRealm().getAuthPolicy() != null) {
+                authPolicyConf = clientApp.getAuthPolicy().getConf();
                 waClientApp.setAuthPolicyConf((clientApp.getRealm().getAuthPolicy()).getConf());

Review comment:
       ah, my bad this line has to be:
   ` authPolicyConf=(clientApp.getRealm().getAuthPolicy()).getConf(); `
   as a sequence, authPoliceConf is needed in both condition 




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