Production and Tests should use the appropriate PasswordEncoder

- Production should use BCryptPasswordEncoder
- Tests should use FakePasswordEncoder

[#164967385]

server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioning.java

@@ -31,7 +31,6 @@
 import org.cloudfoundry.identity.uaa.scim.exception.ScimResourceNotFoundException;
 import org.cloudfoundry.identity.uaa.scim.util.ScimUtils;
 import org.cloudfoundry.identity.uaa.user.JdbcUaaUserDatabase;
-import org.cloudfoundry.identity.uaa.util.PasswordEncoderFactory;
 import org.cloudfoundry.identity.uaa.util.TimeService;
 import org.cloudfoundry.identity.uaa.util.TimeServiceImpl;
 import org.springframework.dao.DuplicateKeyException;
@@ -42,7 +41,6 @@
 import org.springframework.jdbc.core.PreparedStatementSetter;
 import org.springframework.jdbc.core.RowMapper;
 import org.springframework.security.authentication.BadCredentialsException;
-import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
 import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.util.Assert;
 
@@ -110,7 +108,7 @@ public Logger getLogger() {
 
     protected final JdbcTemplate jdbcTemplate;
 
-    private PasswordEncoder passwordEncoder = new PasswordEncoderFactory().get();
+    private final PasswordEncoder passwordEncoder;
 
     private boolean deactivateOnDelete = true;
 
@@ -120,11 +118,15 @@ public Logger getLogger() {
 
     private TimeService timeService = new TimeServiceImpl();
 
-    public JdbcScimUserProvisioning(JdbcTemplate jdbcTemplate, JdbcPagingListFactory pagingListFactory) {
+    public JdbcScimUserProvisioning(
+            JdbcTemplate jdbcTemplate,
+            JdbcPagingListFactory pagingListFactory,
+            final PasswordEncoder passwordEncoder) {
         super(jdbcTemplate, pagingListFactory, mapper);
         Assert.notNull(jdbcTemplate);
         this.jdbcTemplate = jdbcTemplate;
         setQueryConverter(new SimpleSearchQueryConverter());
+        this.passwordEncoder = passwordEncoder;
     }
 
     public void setTimeService(TimeService timeService) {
@@ -416,16 +418,6 @@ public void setDeactivateOnDelete(boolean deactivateOnDelete) {
         this.deactivateOnDelete = deactivateOnDelete;
     }
 
-    /**
-     * The encoder used to hash passwords before storing them in the database.
-     *
-     * Defaults to a {@link BCryptPasswordEncoder}.
-     */
-    public void setPasswordEncoder(PasswordEncoder passwordEncoder) {
-        Assert.notNull(passwordEncoder, "passwordEncoder cannot be null");
-        this.passwordEncoder = passwordEncoder;
-    }
-
     /**
      * Sets the regular expression which will be used to validate the username.
      */

server/src/main/java/org/cloudfoundry/identity/uaa/util/CachingPasswordEncoder.java

@@ -16,6 +16,8 @@
 
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.crypto.codec.Hex;
 import org.springframework.security.crypto.codec.Utf8;
@@ -61,28 +63,21 @@ public void setEnabled(boolean enabled) {
 
     private volatile Cache<CharSequence, Set<String>> cache = null;
 
-    private PasswordEncoder passwordEncoder;
+    private final PasswordEncoder passwordEncoder;
 
-    public CachingPasswordEncoder() throws NoSuchAlgorithmException {
+    public CachingPasswordEncoder(final PasswordEncoder passwordEncoder) throws NoSuchAlgorithmException {
+        this.passwordEncoder = passwordEncoder;
         messageDigest = MessageDigest.getInstance("SHA-256");
         this.secret = Utf8.encode(new RandomValueStringGenerator().generate());
         this.salt = KeyGenerators.secureRandom().generateKey();
         iterations = 25;
         buildCache();
     }
 
-    public PasswordEncoder getPasswordEncoder() {
-        return passwordEncoder;
-    }
-
-    public void setPasswordEncoder(PasswordEncoder passwordEncoder) {
-        this.passwordEncoder = passwordEncoder;
-    }
-
     @Override
     public String encode(CharSequence rawPassword) throws AuthenticationException {
         //we always use the Bcrypt mechanism, we never store repeated information
-        return getPasswordEncoder().encode(rawPassword);
+        return passwordEncoder.encode(rawPassword);
     }
 
     @Override
@@ -91,7 +86,7 @@ public boolean matches(CharSequence rawPassword, String encodedPassword) throws
             String cacheKey = cacheEncode(rawPassword);
             return internalMatches(cacheKey, rawPassword, encodedPassword);
         } else {
-            return getPasswordEncoder().matches(rawPassword, encodedPassword);
+            return passwordEncoder.matches(rawPassword, encodedPassword);
         }
     }
 
@@ -116,7 +111,7 @@ private boolean internalMatches(String cacheKey, CharSequence rawPassword, Strin
             }
         }
         if (!result) {
-            if (getPasswordEncoder().matches(rawPassword, encodedPassword)) {
+            if (passwordEncoder.matches(rawPassword, encodedPassword)) {
                 result = true;
                 cacheValue = getOrCreateHashList(cacheKey);
                 if (cacheValue!=null) {

server/src/main/java/org/cloudfoundry/identity/uaa/util/beans/PasswordEncoderConfig.java

@@ -0,0 +1,20 @@
+package org.cloudfoundry.identity.uaa.util.beans;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
+import org.springframework.security.crypto.password.PasswordEncoder;
+
+@Configuration
+public class PasswordEncoderConfig {
+
+    private static Logger logger = LoggerFactory.getLogger(PasswordEncoderConfig.class);
+
+    @Bean
+    public PasswordEncoder nonCachingPasswordEncoder() {
+        logger.info("Created instance of BCryptPasswordEncoder");
+        return new BCryptPasswordEncoder();
+    }
+}

server/src/main/java/org/cloudfoundry/identity/uaa/zone/MultitenantJdbcClientDetailsService.java

@@ -106,26 +106,20 @@ public class MultitenantJdbcClientDetailsService extends MultitenantClientServic
 
     private String selectClientDetailsSql = DEFAULT_SELECT_STATEMENT;
 
-    private PasswordEncoder passwordEncoder;
+    private final PasswordEncoder passwordEncoder;
 
     private final JdbcTemplate jdbcTemplate;
 
     private JdbcListFactory listFactory;
 
     public MultitenantJdbcClientDetailsService(
             final JdbcTemplate jdbcTemplate,
-            final IdentityZoneManager identityZoneManager) {
+            final IdentityZoneManager identityZoneManager,
+            final PasswordEncoder passwordEncoder) {
         super(identityZoneManager);
         Assert.notNull(jdbcTemplate, "JDbcTemplate required");
         this.jdbcTemplate = jdbcTemplate;
         this.listFactory = new DefaultJdbcListFactory(new NamedParameterJdbcTemplate(jdbcTemplate));
-    }
-
-    /**
-     * @param passwordEncoder
-     *            the password encoder to set
-     */
-    public void setPasswordEncoder(PasswordEncoder passwordEncoder) {
         this.passwordEncoder = passwordEncoder;
     }
 

server/src/test/java/org/cloudfoundry/identity/uaa/account/PasswordChangeEndpointTests.java

@@ -21,6 +21,7 @@
 import org.cloudfoundry.identity.uaa.scim.test.TestUtils;
 import org.cloudfoundry.identity.uaa.scim.validate.PasswordValidator;
 import org.cloudfoundry.identity.uaa.security.SecurityContextAccessor;
+import org.cloudfoundry.identity.uaa.util.FakePasswordEncoder;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
 import org.flywaydb.core.Flyway;
 import org.flywaydb.core.api.MigrationVersion;
@@ -33,7 +34,6 @@
 import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase;
 import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
 import org.springframework.security.authentication.BadCredentialsException;
-import org.springframework.security.crypto.password.NoOpPasswordEncoder;
 
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.mock;
@@ -67,9 +67,10 @@ public static void init() {
     @Before
     public void setup() {
         JdbcTemplate jdbcTemplate = new JdbcTemplate(database);
-        JdbcScimUserProvisioning dao = new JdbcScimUserProvisioning(jdbcTemplate,
-                        new JdbcPagingListFactory(jdbcTemplate, LimitSqlAdapterFactory.getLimitSqlAdapter()));
-        dao.setPasswordEncoder(NoOpPasswordEncoder.getInstance());
+        JdbcScimUserProvisioning dao = new JdbcScimUserProvisioning(
+                jdbcTemplate,
+                new JdbcPagingListFactory(jdbcTemplate, LimitSqlAdapterFactory.getLimitSqlAdapter()),
+                new FakePasswordEncoder());
 
         endpoints = new PasswordChangeEndpoint();
         endpoints.setScimUserProvisioning(dao);

server/src/test/java/org/cloudfoundry/identity/uaa/annotations/WithSpring.java

@@ -1,11 +1,8 @@
 package org.cloudfoundry.identity.uaa.annotations;
 
-import org.cloudfoundry.identity.uaa.FakePasswordEncoderConfig;
-import org.cloudfoundry.identity.uaa.security.PollutionPreventionExtension;
 import org.cloudfoundry.identity.uaa.test.HoneycombAuditEventTestListenerExtension;
 import org.cloudfoundry.identity.uaa.test.HoneycombJdbcInterceptorExtension;
 import org.cloudfoundry.identity.uaa.test.TestWebAppContext;
-import org.cloudfoundry.identity.uaa.util.FakePasswordEncoder;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.context.ContextConfiguration;
@@ -25,8 +22,7 @@
 @ActiveProfiles("default")
 @WebAppConfiguration
 @ContextConfiguration(classes = {
-        TestWebAppContext.class,
-        FakePasswordEncoderConfig.class
+        TestWebAppContext.class
 })
 public @interface WithSpring {
 }

server/src/test/java/org/cloudfoundry/identity/uaa/authentication/UaaClientAuthenticationProviderTest.java

@@ -49,8 +49,7 @@ public void setUpForClientTests() {
         IdentityZoneManager mockIdentityZoneManager = mock(IdentityZoneManager.class);
         when(mockIdentityZoneManager.getCurrentIdentityZoneId()).thenReturn(OriginKeys.UAA);
 
-        jdbcClientDetailsService = new MultitenantJdbcClientDetailsService(jdbcTemplate, mockIdentityZoneManager);
-        jdbcClientDetailsService.setPasswordEncoder(encoder);
+        jdbcClientDetailsService = new MultitenantJdbcClientDetailsService(jdbcTemplate, mockIdentityZoneManager, encoder);
         ClientDetailsUserDetailsService clientDetailsService = new ClientDetailsUserDetailsService(jdbcClientDetailsService);
         client = createClient();
         authenticationProvider = new ClientDetailsAuthenticationProvider(clientDetailsService, encoder);

server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrapTests.java

@@ -63,8 +63,7 @@ void setUpClientAdminTests() {
         IdentityZoneManager mockIdentityZoneManager = mock(IdentityZoneManager.class);
         when(mockIdentityZoneManager.getCurrentIdentityZoneId()).thenReturn(OriginKeys.UAA);
 
-        multitenantJdbcClientDetailsService = spy(new MultitenantJdbcClientDetailsService(jdbcTemplate, mockIdentityZoneManager));
-        multitenantJdbcClientDetailsService.setPasswordEncoder(fakePasswordEncoder);
+        multitenantJdbcClientDetailsService = spy(new MultitenantJdbcClientDetailsService(jdbcTemplate, mockIdentityZoneManager, fakePasswordEncoder));
 
         clientMetadataProvisioning = new JdbcClientMetadataProvisioning(multitenantJdbcClientDetailsService, jdbcTemplate);
 

server/src/test/java/org/cloudfoundry/identity/uaa/client/JdbcClientMetadataProvisioningTest.java

@@ -1,6 +1,7 @@
 package org.cloudfoundry.identity.uaa.client;
 
 import org.cloudfoundry.identity.uaa.annotations.WithDatabaseContext;
+import org.cloudfoundry.identity.uaa.util.FakePasswordEncoder;
 import org.cloudfoundry.identity.uaa.zone.MultitenantJdbcClientDetailsService;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -44,7 +45,7 @@ void createDatasource() {
         identityZoneId = "identityZoneId-" + randomValueStringGenerator.generate();
         clientId = "clientId-" + randomValueStringGenerator.generate();
 
-        MultitenantJdbcClientDetailsService clientService = new MultitenantJdbcClientDetailsService(jdbcTemplate, null);
+        MultitenantJdbcClientDetailsService clientService = new MultitenantJdbcClientDetailsService(jdbcTemplate, null, new FakePasswordEncoder());
         jdbcClientMetadataProvisioning = new JdbcClientMetadataProvisioning(clientService, jdbcTemplate);
     }
 

server/src/test/java/org/cloudfoundry/identity/uaa/oauth/JdbcQueryableClientDetailsServiceTests.java

@@ -16,6 +16,7 @@
 import org.cloudfoundry.identity.uaa.resources.jdbc.JdbcPagingListFactory;
 import org.cloudfoundry.identity.uaa.resources.jdbc.LimitSqlAdapter;
 import org.cloudfoundry.identity.uaa.test.JdbcTestBase;
+import org.cloudfoundry.identity.uaa.util.FakePasswordEncoder;
 import org.cloudfoundry.identity.uaa.zone.*;
 import org.junit.Before;
 import org.junit.Test;
@@ -35,7 +36,7 @@ public class JdbcQueryableClientDetailsServiceTests extends JdbcTestBase {
     @Before
     public void initJdbcScimClientDetailsServiceTests() {
         limitSqlAdapter = webApplicationContext.getBean(LimitSqlAdapter.class);
-        delegate = new MultitenantJdbcClientDetailsService(jdbcTemplate, null);
+        delegate = new MultitenantJdbcClientDetailsService(jdbcTemplate, null, new FakePasswordEncoder());
         service = new JdbcQueryableClientDetailsService(delegate, jdbcTemplate, new JdbcPagingListFactory(jdbcTemplate,
                 limitSqlAdapter));