stataru8 commented on code in PR #1863:
URL: https://github.com/apache/karaf/pull/1863#discussion_r1778852624


##########
jaas/modules/src/main/java/org/apache/karaf/jaas/modules/properties/PropertiesBackingEngine.java:
##########
@@ -52,14 +52,13 @@ public void addUser(String username, String password) {
         if (username.startsWith(GROUP_PREFIX))
             throw new IllegalArgumentException("Prefix not permitted: " + 
GROUP_PREFIX);
 
-        addUserInternal(username, password);
+        addUserInternal(username, encryptionSupport.encrypt(password));

Review Comment:
   I just moved this call from its original location in `addUserInternal`:
   
https://github.com/apache/karaf/blob/e9b9c973569596c8931cc4e8f7d62744d9c3ede5/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/properties/PropertiesBackingEngine.java#L62
   
   The call is only needed when adding a user and shouldn't be made when adding 
a group: 
https://github.com/apache/karaf/blob/c01d0bc4fe1a52859453b8fe69c2c306690769a3/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/properties/PropertiesBackingEngine.java#L262
 There is the risk of encrypting `""`, which with the defaults, results in 
`{CRYPT}ABC...{CRYPT}`. After `jaas:group-add karaf newGrup`, 
   `jaas:user-list` will return 
   ```
   User Name | Group   | Role
   
----------+---------+-------------------------------------------------------------------------------
   karaf     | newGrup | {CRYPT}ABC...{CRYPT}
   ```
   
   ~~Maybe at this point, we should create another `addUserInternal` with just 
the `username` as an argument: `private void addUserInternal(String username)`, 
or another function just for groups...~~



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