ilgrosso commented on a change in pull request #319:
URL: https://github.com/apache/syncope/pull/319#discussion_r818370864



##########
File path: 
core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/entity/user/User.java
##########
@@ -54,8 +55,14 @@
     void setSecurityQuestion(SecurityQuestion securityQuestion);
 
     String getSecurityAnswer();
+    
+    String getClearSecurityAnswer();
 
-    void setSecurityAnswer(String securityAnswer);
+    void setEncodedSecurityAnswer(String securityAnswer);
+    
+    void setSecurityAnswer(String securityAnswer,  CipherAlgorithm 
cipherAlgoritm);

Review comment:
       This method would allow to set  a different `cipherAlgorithm` for the 
user than the one set for password.
   The risk is that `user#getCipherAlgorith` can return the wrong value when 
used to check password or security question values via `Encryptor#verify`.
   
   I think we should change `User` interface to:
   
   1. remove `cipherAlgorithm` param from `setPassword()` and 
`setSecurityAnswer()` methods
   1. add a separate `setCipherAlgorithm()` method, failing if 
`cipherAlgorithm` is already set, unless the provided parameter is `null`
   1. have `setPassword()` and `setSecurityAnswer()` to internally set 
`cipherAlgorithm` to the value from conf if none was provided before




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

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


Reply via email to