Skip to content

Commit 4d036dd

Browse files
committed
Migrate CachingPasswordEncoder to Caffeine Cache.
Remove Guava Library Signed-off-by: Duane May <duane.may@broadcom.com>
1 parent 6beb383 commit 4d036dd

File tree

4 files changed

+34
-42
lines changed

4 files changed

+34
-42
lines changed

dependencies.gradle

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ versions.springBootVersion = "2.7.18"
1414
versions.springFrameworkVersion = "5.3.39"
1515
versions.springSecurityVersion = "5.8.16"
1616
versions.tomcatCargoVersion = "9.0.97"
17-
versions.guavaVersion = "33.3.1-jre"
1817
versions.seleniumVersion = "4.27.0"
1918
versions.braveVersion = "6.0.3"
2019
versions.jacksonVersion = "2.18.2"
@@ -58,8 +57,6 @@ libraries.dumbster = "dumbster:dumbster:1.6"
5857
libraries.eclipseJgit = "org.eclipse.jgit:org.eclipse.jgit:7.0.0.202409031743-r"
5958
libraries.flywayCore = "org.flywaydb:flyway-core"
6059
libraries.greenmail = "com.icegreen:greenmail:1.6.15"
61-
libraries.guava = "com.google.guava:guava:${versions.guavaVersion}"
62-
libraries.guavaTestLib = "com.google.guava:guava-testlib:${versions.guavaVersion}"
6360
libraries.hamcrest = "org.hamcrest:hamcrest:${versions.hamcrestVersion}"
6461
libraries.hibernateValidator = "org.hibernate.validator:hibernate-validator"
6562
libraries.hsqldb = "org.hsqldb:hsqldb"

server/build.gradle

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@ dependencies {
3333
implementation(libraries.bouncyCastleTlsFips)
3434
implementation(libraries.bouncyCastlePkixFips)
3535

36-
implementation(libraries.guava)
37-
3836
implementation(libraries.aspectJRt)
3937
implementation(libraries.aspectJWeaver)
4038

@@ -105,7 +103,6 @@ dependencies {
105103
testImplementation(libraries.tomcatJdbc)
106104

107105
testImplementation(libraries.jsonPathAssert)
108-
testImplementation(libraries.guavaTestLib)
109106
testImplementation(libraries.xmlUnit)
110107

111108
implementation(libraries.commonsIo)

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

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
package org.cloudfoundry.identity.uaa.util;
22

3-
import com.google.common.cache.Cache;
4-
import com.google.common.cache.CacheBuilder;
3+
import com.github.benmanes.caffeine.cache.Cache;
4+
import com.github.benmanes.caffeine.cache.Caffeine;
5+
import com.google.common.annotations.VisibleForTesting;
6+
import org.cloudfoundry.identity.uaa.oauth.common.util.RandomValueStringGenerator;
57
import org.springframework.security.core.AuthenticationException;
68
import org.springframework.security.crypto.codec.Hex;
79
import org.springframework.security.crypto.codec.Utf8;
810
import org.springframework.security.crypto.keygen.KeyGenerators;
911
import org.springframework.security.crypto.password.PasswordEncoder;
10-
import org.cloudfoundry.identity.uaa.oauth.common.util.RandomValueStringGenerator;
1112

1213
import java.security.MessageDigest;
1314
import java.security.NoSuchAlgorithmException;
@@ -18,7 +19,6 @@
1819
import java.util.List;
1920
import java.util.Set;
2021
import java.util.concurrent.ConcurrentMap;
21-
import java.util.concurrent.TimeUnit;
2222

2323
import static org.springframework.security.crypto.util.EncodingUtils.concatenate;
2424

@@ -28,24 +28,30 @@
2828
*/
2929
public class CachingPasswordEncoder implements PasswordEncoder {
3030

31+
private static final int ITERATIONS = 25;
32+
private static final int MAX_KEYS = 1000;
33+
private static final int MAX_ENCODED_PASSWORDS = 5;
34+
35+
@VisibleForTesting
36+
static Duration DEFAULT_CACHE_TTL = Duration.ofMinutes(5L);
37+
3138
private final MessageDigest messageDigest;
3239
private final byte[] secret;
3340
private final byte[] salt;
3441

35-
private static final int ITERATIONS = 25;
36-
private static final int MAX_KEYS = 1000;
37-
private static final int MAX_ENCODED_PASSWORDS = 5;
38-
private final Duration CACHE_TTL = Duration.ofMinutes(5L);
39-
private volatile Cache<CharSequence, Set<String>> cache;
42+
private final Cache<CharSequence, Set<String>> cache;
4043

4144
private final PasswordEncoder passwordEncoder;
4245

4346
CachingPasswordEncoder(final PasswordEncoder passwordEncoder) throws NoSuchAlgorithmException {
4447
this.passwordEncoder = passwordEncoder;
45-
this.messageDigest = MessageDigest.getInstance("SHA-256");
46-
this.secret = Utf8.encode(new RandomValueStringGenerator().generate());
47-
this.salt = KeyGenerators.secureRandom().generateKey();
48-
buildCache();
48+
messageDigest = MessageDigest.getInstance("SHA-256");
49+
secret = Utf8.encode(new RandomValueStringGenerator().generate());
50+
salt = KeyGenerators.secureRandom().generateKey();
51+
cache = Caffeine.newBuilder()
52+
.expireAfterWrite(DEFAULT_CACHE_TTL)
53+
.maximumSize(MAX_KEYS)
54+
.build();
4955
}
5056

5157
@Override
@@ -65,7 +71,7 @@ public boolean matches(CharSequence rawPassword, String encodedPassword) throws
6571
Set<String> getOrCreateHashList(String cacheKey) {
6672
Set<String> result = cache.getIfPresent(cacheKey);
6773
if (result == null) {
68-
if (cache.size() >= MAX_KEYS) {
74+
if (cache.estimatedSize() >= MAX_KEYS) {
6975
cache.invalidateAll();
7076
}
7177
cache.put(cacheKey, Collections.synchronizedSet(new LinkedHashSet<>()));
@@ -86,8 +92,8 @@ private boolean internalMatches(String cacheKey, CharSequence rawPassword, Strin
8692
result = true;
8793
cacheValue = getOrCreateHashList(cacheKey);
8894
if (cacheValue != null) {
89-
//this list should never grow very long.
90-
//Only if you store multiple versions of the same password more than once
95+
// This list should never grow very long.
96+
// Only if you store multiple versions of the same password more than once
9197
if (cacheValue.size() >= MAX_ENCODED_PASSWORDS) {
9298
cacheValue.clear();
9399
}
@@ -140,16 +146,10 @@ int getMaxEncodedPasswords() {
140146
}
141147

142148
long getNumberOfKeys() {
143-
return cache.size();
149+
return cache.estimatedSize();
144150
}
145151

146152
ConcurrentMap<CharSequence, Set<String>> asMap() {
147153
return cache.asMap();
148154
}
149-
150-
void buildCache() {
151-
cache = CacheBuilder.newBuilder()
152-
.expireAfterWrite(CACHE_TTL.getSeconds(), TimeUnit.SECONDS)
153-
.build();
154-
}
155155
}

server/src/test/java/org/cloudfoundry/identity/uaa/util/CachingPasswordEncoderTest.java

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
package org.cloudfoundry.identity.uaa.util;
22

3+
import org.cloudfoundry.identity.uaa.oauth.common.util.RandomValueStringGenerator;
34
import org.junit.jupiter.api.BeforeEach;
45
import org.junit.jupiter.api.Test;
56
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
67
import org.springframework.security.crypto.password.PasswordEncoder;
7-
import org.cloudfoundry.identity.uaa.oauth.common.util.RandomValueStringGenerator;
8-
import org.springframework.test.util.ReflectionTestUtils;
98

109
import java.security.NoSuchAlgorithmException;
1110
import java.time.Duration;
@@ -60,19 +59,19 @@ void matches() {
6059
@Test
6160
void matchesButExpires() throws Exception {
6261
Duration shortTTL = Duration.ofSeconds(1);
63-
ReflectionTestUtils.setField(cachingPasswordEncoder, "CACHE_TTL", shortTTL);
64-
cachingPasswordEncoder.buildCache();
62+
synchronized (CachingPasswordEncoder.class) {
63+
CachingPasswordEncoder.DEFAULT_CACHE_TTL = shortTTL;
64+
cachingPasswordEncoder = new CachingPasswordEncoder(passwordEncoder);
65+
CachingPasswordEncoder.DEFAULT_CACHE_TTL = Duration.ofMinutes(5);
66+
}
6567
String encoded = cachingPasswordEncoder.encode(password);
6668
String cacheKey = cachingPasswordEncoder.cacheEncode(password);
6769

6870
assertTrue(passwordEncoder.matches(password, encoded));
6971
assertTrue(cachingPasswordEncoder.matches(password, encoded));
70-
71-
assertTrue(!cachingPasswordEncoder.getOrCreateHashList(cacheKey).isEmpty(),
72-
"Password is no longer cached when we expected it to be cached");
72+
assertFalse(cachingPasswordEncoder.getOrCreateHashList(cacheKey).isEmpty(), "Password is no longer cached when we expected it to be cached");
7373

7474
Thread.sleep(shortTTL.toMillis() + 100);
75-
7675
assertEquals(0, cachingPasswordEncoder.getOrCreateHashList(cacheKey).size(), "Password is still cached when we expected it to be expired");
7776
}
7877

@@ -95,7 +94,7 @@ void cacheIs10XFasterThanNonCached() throws NoSuchAlgorithmException {
9594

9695
int iterations = 10;
9796

98-
String password = new RandomValueStringGenerator().generate();
97+
password = new RandomValueStringGenerator().generate();
9998
String encodedBCrypt = cachingPasswordEncoder.encode(password);
10099
PasswordEncoder nonCachingPasswordEncoder = passwordEncoder;
101100

@@ -122,24 +121,23 @@ void cacheIs10XFasterThanNonCached() throws NoSuchAlgorithmException {
122121
}
123122

124123
@Test
125-
// TODO: This test takes a long time to run :(
124+
// TODO: This test takes a long time to run :(
126125
void ensureNoMemoryLeak() {
127126
assertEquals(0, cachingPasswordEncoder.getNumberOfKeys());
128127
for (int i = 0; i < cachingPasswordEncoder.getMaxKeys(); i++) {
129-
String password = new RandomValueStringGenerator().generate();
128+
password = new RandomValueStringGenerator().generate();
130129
for (int j = 0; j < cachingPasswordEncoder.getMaxEncodedPasswords(); j++) {
131130
String encoded = cachingPasswordEncoder.encode(password);
132131
assertTrue(cachingPasswordEncoder.matches(password, encoded));
133132
}
134133
}
135134
assertEquals(cachingPasswordEncoder.getMaxKeys(), cachingPasswordEncoder.getNumberOfKeys());
136-
String password = new RandomValueStringGenerator().generate();
135+
password = new RandomValueStringGenerator().generate();
137136
String encoded = cachingPasswordEncoder.encode(password);
138137
assertTrue(cachingPasswordEncoder.matches(password, encoded));
139138
//overflow happened
140139
assertEquals(1, cachingPasswordEncoder.getNumberOfKeys());
141140

142-
143141
for (int j = 1; j < cachingPasswordEncoder.getMaxEncodedPasswords(); j++) {
144142
encoded = cachingPasswordEncoder.encode(password);
145143
assertTrue(cachingPasswordEncoder.matches(password, encoded));

0 commit comments

Comments
 (0)