From cdab051703dc07c54d8be9d09efb7219aa6702fb Mon Sep 17 00:00:00 2001 From: Gian Miguel Del Mundo Date: Thu, 28 Sep 2023 13:37:05 +0800 Subject: [PATCH 1/3] Added null cache invalidation in refresh --- .../uid2/shared/auth/AuthorizableStore.java | 11 ++++++ .../shared/auth/AuthorizableStoreTest.java | 37 +++++++++++++++---- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/uid2/shared/auth/AuthorizableStore.java b/src/main/java/com/uid2/shared/auth/AuthorizableStore.java index 99b7e727..3cf0bfac 100644 --- a/src/main/java/com/uid2/shared/auth/AuthorizableStore.java +++ b/src/main/java/com/uid2/shared/auth/AuthorizableStore.java @@ -43,6 +43,7 @@ public AuthorizableStore(Class cls) { public void refresh(Collection authorizablesToRefresh) { authorizables.set(new AuthorizableStoreSnapshot(authorizablesToRefresh)); + invalidateInvalidKeys(); } public T getAuthorizableByKey(String key) { @@ -100,6 +101,16 @@ private Cache createCache() { .build(); } + private void invalidateInvalidKeys() { + List invalidKeys = keyToHashCache.asMap() + .entrySet() + .stream() + .filter(entry -> entry.getValue().isBlank()) + .map(Map.Entry::getKey) + .collect(Collectors.toList()); + invalidKeys.forEach(keyToHashCache::invalidate); + } + private ByteBuffer wrapHashToByteBuffer(String hash) { byte[] hashBytes = convertBase64StringToBytes(hash); return hashBytes == null ? null : ByteBuffer.wrap(hashBytes); diff --git a/src/test/java/com/uid2/shared/auth/AuthorizableStoreTest.java b/src/test/java/com/uid2/shared/auth/AuthorizableStoreTest.java index 14997058..1513ce03 100644 --- a/src/test/java/com/uid2/shared/auth/AuthorizableStoreTest.java +++ b/src/test/java/com/uid2/shared/auth/AuthorizableStoreTest.java @@ -6,6 +6,7 @@ import org.junit.jupiter.api.Test; import java.time.Instant; +import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.Set; @@ -22,16 +23,16 @@ public class AuthorizableStoreTest { private static final String SITE_13_CLIENT_KEY_LEGACY = "abcdef.abcdefabcdefabcdefabcdefabcdefabcdefab"; private AuthorizableStore clientKeyStore; + private List clients; @BeforeEach public void setup() { - List clients = List.of( - createClientKey(KEY_HASHER.hashKey(SITE_11_CLIENT_KEY), "client11", 11), - createClientKey(KEY_HASHER.hashKey(SITE_12_CLIENT_KEY_1), "client12_1", 12), - createClientKey(KEY_HASHER.hashKey(SITE_12_CLIENT_KEY_2), "client12_2", 12), - createClientKey(KEY_HASHER.hashKey(SITE_13_CLIENT_KEY), "client13", 13), - createClientKey(KEY_HASHER.hashKey(SITE_13_CLIENT_KEY_LEGACY), "client13_legacy", 13) - ); + clients = new ArrayList<>(); + clients.add(createClientKey(KEY_HASHER.hashKey(SITE_11_CLIENT_KEY), "client11", 11)); + clients.add(createClientKey(KEY_HASHER.hashKey(SITE_12_CLIENT_KEY_1), "client12_1", 12)); + clients.add(createClientKey(KEY_HASHER.hashKey(SITE_12_CLIENT_KEY_2), "client12_2", 12)); + clients.add(createClientKey(KEY_HASHER.hashKey(SITE_13_CLIENT_KEY), "client13", 13)); + clients.add(createClientKey(KEY_HASHER.hashKey(SITE_13_CLIENT_KEY_LEGACY), "client13_legacy", 13)); this.clientKeyStore = new AuthorizableStore<>(ClientKey.class); this.clientKeyStore.refresh(clients); @@ -135,6 +136,28 @@ public void refresh_returnsNewClients_afterRefresh() { ); } + @Test + public void refresh_returnsPreviouslyInvalidClients_afterRefresh() { + String key = "UID2-C-L-14-abcdef.abcdefabcdefabcdefabcdefabcdefabcdefab"; + ClientKey invalidClientKey = clientKeyStore.getAuthorizableByKey(key); + + ClientKey client = createClientKey(KEY_HASHER.hashKey(key), "client14", 14); + clients.add(client); + clientKeyStore.refresh(clients); + ClientKey validClientKey = clientKeyStore.getAuthorizableByKey(key); + + assertAll( + "refresh returns previously invalid clients after refresh", + () -> assertNull(invalidClientKey), + () -> assertEquals("client14", validClientKey.getName()), + () -> assertEquals("client11", clientKeyStore.getAuthorizableByKey(SITE_11_CLIENT_KEY).getName()), + () -> assertEquals("client12_1", clientKeyStore.getAuthorizableByKey(SITE_12_CLIENT_KEY_1).getName()), + () -> assertEquals("client12_2", clientKeyStore.getAuthorizableByKey(SITE_12_CLIENT_KEY_2).getName()), + () -> assertEquals("client13", clientKeyStore.getAuthorizableByKey(SITE_13_CLIENT_KEY).getName()), + () -> assertEquals("client13_legacy", clientKeyStore.getAuthorizableByKey(SITE_13_CLIENT_KEY_LEGACY).getName()) + ); + } + private ClientKey createClientKey(KeyHashResult khr, String name, int siteId) { return new ClientKey(khr.getHash(), khr.getSalt(), "", name, NOW, Set.of(), siteId); } From 1f7f5738fd2633fa2bbfc8e7cf83b9b3f402884f Mon Sep 17 00:00:00 2001 From: Gian Miguel Del Mundo Date: Thu, 28 Sep 2023 13:57:12 +0800 Subject: [PATCH 2/3] Updated new cache invalidation test to use reflection --- .../shared/auth/AuthorizableStoreTest.java | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/test/java/com/uid2/shared/auth/AuthorizableStoreTest.java b/src/test/java/com/uid2/shared/auth/AuthorizableStoreTest.java index 1513ce03..998bff9d 100644 --- a/src/test/java/com/uid2/shared/auth/AuthorizableStoreTest.java +++ b/src/test/java/com/uid2/shared/auth/AuthorizableStoreTest.java @@ -1,10 +1,12 @@ package com.uid2.shared.auth; +import com.github.benmanes.caffeine.cache.Cache; import com.uid2.shared.secret.KeyHashResult; import com.uid2.shared.secret.KeyHasher; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.lang.reflect.Field; import java.time.Instant; import java.util.ArrayList; import java.util.List; @@ -137,24 +139,28 @@ public void refresh_returnsNewClients_afterRefresh() { } @Test - public void refresh_returnsPreviouslyInvalidClients_afterRefresh() { + public void refresh_returnsPreviouslyInvalidClients_afterRefresh() throws Exception { + Field cacheField = clientKeyStore.getClass().getDeclaredField("keyToHashCache"); + cacheField.setAccessible(true); + Cache cache = (Cache) cacheField.get(clientKeyStore); + String key = "UID2-C-L-14-abcdef.abcdefabcdefabcdefabcdefabcdefabcdefab"; ClientKey invalidClientKey = clientKeyStore.getAuthorizableByKey(key); + String invalidCacheValue = cache.getIfPresent(key); - ClientKey client = createClientKey(KEY_HASHER.hashKey(key), "client14", 14); + KeyHashResult khr = KEY_HASHER.hashKey(key); + ClientKey client = createClientKey(khr, "client14", 14); clients.add(client); clientKeyStore.refresh(clients); ClientKey validClientKey = clientKeyStore.getAuthorizableByKey(key); + String validCacheValue = cache.getIfPresent(key); assertAll( "refresh returns previously invalid clients after refresh", + () -> assertEquals("", invalidCacheValue), () -> assertNull(invalidClientKey), - () -> assertEquals("client14", validClientKey.getName()), - () -> assertEquals("client11", clientKeyStore.getAuthorizableByKey(SITE_11_CLIENT_KEY).getName()), - () -> assertEquals("client12_1", clientKeyStore.getAuthorizableByKey(SITE_12_CLIENT_KEY_1).getName()), - () -> assertEquals("client12_2", clientKeyStore.getAuthorizableByKey(SITE_12_CLIENT_KEY_2).getName()), - () -> assertEquals("client13", clientKeyStore.getAuthorizableByKey(SITE_13_CLIENT_KEY).getName()), - () -> assertEquals("client13_legacy", clientKeyStore.getAuthorizableByKey(SITE_13_CLIENT_KEY_LEGACY).getName()) + () -> assertEquals(khr.getHash(), validCacheValue), + () -> assertEquals("client14", validClientKey.getName()) ); } From 263dc6b69b64088c311cef268aa36ac4709a4de3 Mon Sep 17 00:00:00 2001 From: Gian Miguel Del Mundo Date: Thu, 28 Sep 2023 14:12:20 +0800 Subject: [PATCH 3/3] Enhanced cache invalidation test assertions --- .../shared/auth/AuthorizableStoreTest.java | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/test/java/com/uid2/shared/auth/AuthorizableStoreTest.java b/src/test/java/com/uid2/shared/auth/AuthorizableStoreTest.java index 998bff9d..e78e8250 100644 --- a/src/test/java/com/uid2/shared/auth/AuthorizableStoreTest.java +++ b/src/test/java/com/uid2/shared/auth/AuthorizableStoreTest.java @@ -144,6 +144,8 @@ public void refresh_returnsPreviouslyInvalidClients_afterRefresh() throws Except cacheField.setAccessible(true); Cache cache = (Cache) cacheField.get(clientKeyStore); + clientKeyStore.getAuthorizableByKey(SITE_11_CLIENT_KEY); + String key = "UID2-C-L-14-abcdef.abcdefabcdefabcdefabcdefabcdefabcdefab"; ClientKey invalidClientKey = clientKeyStore.getAuthorizableByKey(key); String invalidCacheValue = cache.getIfPresent(key); @@ -152,15 +154,28 @@ public void refresh_returnsPreviouslyInvalidClients_afterRefresh() throws Except ClientKey client = createClientKey(khr, "client14", 14); clients.add(client); clientKeyStore.refresh(clients); + + String existingCacheValue = cache.getIfPresent(SITE_11_CLIENT_KEY); + ClientKey validClientKey = clientKeyStore.getAuthorizableByKey(key); String validCacheValue = cache.getIfPresent(key); assertAll( "refresh returns previously invalid clients after refresh", - () -> assertEquals("", invalidCacheValue), - () -> assertNull(invalidClientKey), - () -> assertEquals(khr.getHash(), validCacheValue), - () -> assertEquals("client14", validClientKey.getName()) + () -> assertAll( + "refresh returns previously invalid clients after refresh - invalid values were previously invalid", + () -> assertNull(invalidClientKey), + () -> assertEquals("", invalidCacheValue) + ), + () -> assertAll( + "refresh returns previously invalid clients after refresh - invalid values are now valid", + () -> assertEquals("client14", validClientKey.getName()), + () -> assertEquals(khr.getHash(), validCacheValue) + ), + () -> assertAll( + "refresh returns previously invalid clients after refresh - valid values are still valid", + () -> assertEquals(clients.get(0).getKeyHash(), existingCacheValue) + ) ); }