Addressed Review comments

strongbox · Oct 10, 2020 · 5f56f04 · 5f56f04
1 parent 33f3c68
commit 5f56f04
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 32 deletions.
diff --git a/...org/carlspring/strongbox/authentication/api/impl/ldap/LdapAuthenticationProviderTest.java b/...org/carlspring/strongbox/authentication/api/impl/ldap/LdapAuthenticationProviderTest.java
@@ -11,6 +11,7 @@
 
 import org.carlspring.strongbox.authentication.support.AuthenticationContextInitializer;
 import org.carlspring.strongbox.config.UsersConfig;
+import org.carlspring.strongbox.configuration.StrongboxDelegatingPasswordEncoder;
 import org.carlspring.strongbox.users.domain.SystemRole;
 
 import org.junit.jupiter.api.Test;
@@ -86,14 +87,16 @@ public void embeddedLdapServerRegistersExpectedAuthenticationProvider()
     @Test
     public void base64EncodedPasswordsShouldWork()
     {
+        StrongboxDelegatingPasswordEncoder passwordEncoder = new StrongboxDelegatingPasswordEncoder();
         UserDetails ldapUser = ldapUserDetailsService.loadUserByUsername("base64encoded-issue-1840");
 
         assertThat(ldapUser).isInstanceOf(LdapUserDetailsImpl.class);
         LdapUserDetails ldapUserDetails = (LdapUserDetailsImpl) ldapUser;
 
         assertThat(ldapUserDetails.getDn()).isEqualTo("uid=base64encoded-issue-1840,ou=Users,dc=carlspring,dc=com");
-        assertThat(ldapUserDetails.getPassword()).isEqualTo("{bcrypt}JDJhJDEwJGxwd2x4eWp2WEt6TjFjY0NydzJQQnVaeC5lVmVzV2JmbVRic3JDYm9NVS5nc05XVmNaV01p");
+        assertThat(ldapUserDetails.getPassword()).isEqualTo("{MD5}X03MO1qnZdYdgyfeuILPmQ==");
         assertThat(ldapUserDetails.getUsername()).isEqualTo("base64encoded-issue-1840");
+        assertThat(passwordEncoder.matches("password", ldapUserDetails.getPassword())).isEqualTo(true);
         assertThat(((List<SimpleGrantedAuthority>)ldapUser.getAuthorities()))
                 .contains(
                         new SimpleGrantedAuthority(SystemRole.REPOSITORY_MANAGER.name()),

diff --git a/...cation-providers/strongbox-ldap-authentication-provider/src/test/resources/strongbox.ldif b/...cation-providers/strongbox-ldap-authentication-provider/src/test/resources/strongbox.ldif
@@ -35,7 +35,7 @@ cn: base64encoded-issue-1840
 mail: mtodorov@carlspring.com
 givenName: Martin
 surname: Todorov
-userPassword: {bcrypt}JDJhJDEwJGxwd2x4eWp2WEt6TjFjY0NydzJQQnVaeC5lVmVzV2JmbVRic3JDYm9NVS5nc05XVmNaV01p
+userPassword: {MD5}X03MO1qnZdYdgyfeuILPmQ==
 
 dn: uid=stodorov,ou=Users,dc=carlspring,dc=com
 objectClass: inetOrgPerson

diff --git a/.../main/java/org/carlspring/strongbox/configuration/StrongboxDelegatingPasswordEncoder.java b/.../main/java/org/carlspring/strongbox/configuration/StrongboxDelegatingPasswordEncoder.java
@@ -1,11 +1,13 @@
 package org.carlspring.strongbox.configuration;
 
-import javax.annotation.PostConstruct;
+import java.util.Base64;
 
+import org.apache.commons.lang3.StringUtils;
+import org.springframework.security.crypto.codec.Hex;
+import org.springframework.security.crypto.codec.Utf8;
 import org.springframework.security.crypto.factory.PasswordEncoderFactories;
 import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.stereotype.Component;
-import org.springframework.util.Base64Utils;
 
 /**
  * This class encodes given raw String or Base64 encoded string.
@@ -19,10 +21,14 @@ public class StrongboxDelegatingPasswordEncoder
         implements PasswordEncoder
 {
 
+    private static final String PREFIX = "{";
+
+    private static final String SUFFIX = "}";
+
     private PasswordEncoder passwordEncoder;
 
-    @PostConstruct
-    private void init()
+
+    public StrongboxDelegatingPasswordEncoder()
     {
         passwordEncoder = PasswordEncoderFactories.createDelegatingPasswordEncoder();
     }
@@ -37,52 +43,85 @@ private void init()
     @Override
     public String encode(CharSequence rawCharSequence)
     {
-        return passwordEncoder.encode(getDecodedString(rawCharSequence));
+        return passwordEncoder.encode(rawCharSequence);
     }
 
     /**
      * Verify the encoded password obtained from storage matches the submitted raw password after it too is encoded.
      * Returns true if the passwords match, false if they do not. The stored password itself is never decoded.
      * In case given rawCharSequence is Base64 encoded, then it decode it first then verifies.
      *
-     * @param rawCharSequence Raw String or Base64 encoded String
-     * @param encodedString   Encoded String
+     * @param rawCharSequence Raw password
+     * @param encodedString   Encoded Hash String or hash containing Base64 encoded String
      * @return true if the passwords match
      * false if the password do not match
      */
     @Override
     public boolean matches(CharSequence rawCharSequence,
                            String encodedString)
     {
-        return passwordEncoder.matches(getDecodedString(rawCharSequence), encodedString);
+        boolean isMatches = passwordEncoder.matches(rawCharSequence, encodedString);
+
+        if (isMatches)
+        {
+            return true;
+        }
+
+        return passwordEncoder.matches(rawCharSequence, decodeBase64PasswordHash(encodedString));
     }
 
-    private String getDecodedString(CharSequence rawCharSequence)
+
+    private String decodeBase64PasswordHash(CharSequence prefixEncodedPassword)
     {
-        if (rawCharSequence == null)
+        if (prefixEncodedPassword == null)
         {
             return null;
         }
 
-        String rawString = rawCharSequence.toString();
+        String prefixEncodedPasswordString = prefixEncodedPassword.toString();
+        String algorithmUsed = extractId(prefixEncodedPasswordString);
 
         try
         {
-            //May throw IllegalArgumentException if raw string contains invalid Base64 characters
-            String base64DecodedString = new String(Base64Utils.decodeFromString(rawString));
-
-            String base64EncodedString = Base64Utils.encodeToString(base64DecodedString.getBytes());
+            String encodedPassword = prefixEncodedPasswordString;
 
-            if (rawString.equals(base64EncodedString))
+            if (StringUtils.isNotEmpty(algorithmUsed))
             {
-                return base64DecodedString;
+                encodedPassword = extractEncodedPassword(prefixEncodedPasswordString);
             }
+
+            return PREFIX + algorithmUsed + SUFFIX +
+                   new String(Hex.encode(Base64.getDecoder().decode(Utf8.encode(encodedPassword))));
+        }
+        catch (Exception ex)
+        {
+            return prefixEncodedPasswordString;
         }
-        catch (IllegalArgumentException e)
+    }
+
+    private String extractEncodedPassword(String prefixEncodedPassword)
+    {
+        int start = prefixEncodedPassword.indexOf(SUFFIX);
+
+        return prefixEncodedPassword.substring(start + 1);
+    }
+
+    private String extractId(String prefixEncodedPassword)
+    {
+        int start = prefixEncodedPassword.indexOf(PREFIX);
+
+        if (start != 0)
+        {
+            return StringUtils.EMPTY;
+        }
+
+        int end = prefixEncodedPassword.indexOf(SUFFIX, start);
+
+        if (end < 0)
         {
-            return rawString;
+            return StringUtils.EMPTY;
         }
 
-        return rawString;
+        return prefixEncodedPassword.substring(start + 1, end);
     }
 }
diff --git a/...t/java/org/carlspring/strongbox/configuration/StrongboxDelegatingPasswordEncoderTest.java b/...t/java/org/carlspring/strongbox/configuration/StrongboxDelegatingPasswordEncoderTest.java
@@ -9,8 +9,6 @@
 import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.context.ContextConfiguration;
-import org.springframework.util.Base64Utils;
-
 
 @SpringBootTest
 @ActiveProfiles(profiles = "test")
@@ -27,7 +25,6 @@ public static class SpringConfig
 
     }
 
-
     @Inject
     private PasswordEncoder passwordEncoder;
 
@@ -39,15 +36,25 @@ void testNullEncodeAndMatch()
     }
 
     @Test
-    void encodeAndMatch()
+    void testHashContainsBase64EncodePassword()
     {
-        String text = "password12";
-        String base64EncodedString = Base64Utils.encodeToString(text.getBytes());
+        String base64HashedString = "{MD5}X03MO1qnZdYdgyfeuILPmQ==";
+        Assertions.assertTrue(passwordEncoder.matches("password", base64HashedString));
+    }
 
-        String normalEncode = passwordEncoder.encode(text);
-        String base64Encode = passwordEncoder.encode(base64EncodedString);
+    @Test
+    void testHashWithoutBase64EncodePassword()
+    {
+        String hashedString = "{bcrypt}$2a$10$lpwlxyjvXKzN1ccCrw2PBuZx.eVesWbfmTbsrCboMU.gsNWVcZWMi";
+        Assertions.assertTrue(passwordEncoder.matches("password", hashedString));
+    }
 
-        Assertions.assertTrue(passwordEncoder.matches(text, normalEncode));
-        Assertions.assertTrue(passwordEncoder.matches(text, base64Encode));
+    @Test
+    void testEncodeDecodeTest()
+    {
+        String text = "password-12";
+        String hashedString = passwordEncoder.encode(text);
+        Assertions.assertTrue(passwordEncoder.matches(text, hashedString));
     }
 }
+