Repository: SAP/cloud-security-xsuaa-integration
Pull request #269 (2020-04-23 14:21:23)

Improved cache tests
Changed caching strategy in `TokenKeyServiceWithCache`

nenaraab: @mwdb this belongs to the mail from last week. Please have a final look at it.
nenaraab: @mwdb can you please have a look at it, the details were provided to you via mail. 
Thanks!
nenaraab: @hassler-d 

please adapt the configuration:
- reduce time to 10 minutes

and we have to clarify with Patrick , whether is a delay given, until the new token key is used for token creation...
hassler-d: > @hassler-d
> 
> please adapt the configuration:
> 
> * reduce time to 10 minutes

Was already at 10 minutes. However, the comment explaining the value was not correct. I changed that.

Patch:
diff --git a/api/README.md b/api/README.md
new file mode 100644
index 00000000..21354448
--- /dev/null
+++ b/api/README.md
@@ -0,0 +1,10 @@
+## Configuration
+
+### Maven Dependencies
+```xml
+<dependency>
+    <groupId>com.sap.cloud.security.xsuaa</groupId>
+    <artifactId>api</artifactId>
+    <version>2.7.5</version>
+</dependency>
+```
diff --git a/api/src/main/java/com/sap/xsa/security/container/XSPrincipal.java b/api/src/main/java/com/sap/xsa/security/container/XSPrincipal.java
index 213d0578..666cf1a0 100644
--- a/api/src/main/java/com/sap/xsa/security/container/XSPrincipal.java
+++ b/api/src/main/java/com/sap/xsa/security/container/XSPrincipal.java
@@ -9,7 +9,7 @@ package com.sap.xsa.security.container;
 import java.security.Principal;
 
 /**
- * deprecated with version 2.4.0 in favor of the new SAP Java Container library.
+ * deprecated with version 2.4.0 in favor of the new SAP Java Client library.
  */
 public interface XSPrincipal extends Principal {
 
diff --git a/api/src/main/java/com/sap/xsa/security/container/XSUserInfo.java b/api/src/main/java/com/sap/xsa/security/container/XSUserInfo.java
index 9dca93be..3ff7f43d 100644
--- a/api/src/main/java/com/sap/xsa/security/container/XSUserInfo.java
+++ b/api/src/main/java/com/sap/xsa/security/container/XSUserInfo.java
@@ -10,7 +10,7 @@ package com.sap.xsa.security.container;
  * API for OAuth resource servers to extract authentication and authorization
  * information from the OAuth token.
  *
- * deprecated with version 2.4.0 in favor of the new SAP Java Container library.
+ * deprecated with version 2.4.0 in favor of the new SAP Java Client library.
  */
 public interface XSUserInfo {
 
diff --git a/api/src/main/java/com/sap/xsa/security/container/XSUserInfoException.java b/api/src/main/java/com/sap/xsa/security/container/XSUserInfoException.java
index 72358393..d7ea74fd 100644
--- a/api/src/main/java/com/sap/xsa/security/container/XSUserInfoException.java
+++ b/api/src/main/java/com/sap/xsa/security/container/XSUserInfoException.java
@@ -7,7 +7,7 @@
 package com.sap.xsa.security.container;
 
 /**
- * deprecated with version 2.4.0 in favor of the new SAP Java Container library.
+ * deprecated with version 2.4.0 in favor of the new SAP Java Client library.
  */
 public class XSUserInfoException extends RuntimeException {
 
diff --git a/java-api/README.md b/java-api/README.md
new file mode 100644
index 00000000..e89d9149
--- /dev/null
+++ b/java-api/README.md
@@ -0,0 +1,10 @@
+## Configuration
+
+### Maven Dependencies
+```xml
+<dependency>
+    <groupId>com.sap.cloud.security</groupId>
+    <artifactId>java-api</artifactId>
+    <version>2.7.5</version>
+</dependency>
+```
diff --git a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCache.java b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCache.java
index dc6ec64d..ca25b30d 100644
--- a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCache.java
+++ b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCache.java
@@ -17,6 +17,7 @@ import com.github.benmanes.caffeine.cache.Cache;
 import com.github.benmanes.caffeine.cache.Caffeine;
 import com.sap.cloud.security.config.CacheConfiguration;
 import com.sap.cloud.security.xsuaa.Assertions;
+import com.github.benmanes.caffeine.cache.Ticker;
 import com.sap.cloud.security.xsuaa.client.DefaultOAuth2TokenKeyService;
 import com.sap.cloud.security.xsuaa.client.OAuth2ServiceException;
 import com.sap.cloud.security.xsuaa.client.OAuth2TokenKeyService;
@@ -34,6 +35,7 @@ class OAuth2TokenKeyServiceWithCache implements Cacheable {
 	private OAuth2TokenKeyService tokenKeyService; // access via getter
 	private Cache<String, PublicKey> cache; // access via getter
 	private CacheConfiguration cacheConfiguration = TokenKeyCacheConfiguration.defaultConfiguration();
+	private Ticker cacheTicker;
 
 	private OAuth2TokenKeyServiceWithCache() {
 		// use getInstance factory method
@@ -46,6 +48,21 @@ class OAuth2TokenKeyServiceWithCache implements Cacheable {
 	 */
 	public static OAuth2TokenKeyServiceWithCache getInstance() {
 		OAuth2TokenKeyServiceWithCache instance = new OAuth2TokenKeyServiceWithCache();
+		instance.cacheTicker = Ticker.systemTicker();
+		return instance;
+	}
+
+	/**
+	 * Creates a new instance and sets the cache ticker. This is used for testing.
+	 *
+	 * @param cacheTicker
+	 * 			ticker the cache uses to determine time
+	 *
+	 * @return the new instance.
+	 */
+	static OAuth2TokenKeyServiceWithCache getInstance(Ticker cacheTicker) {
+		OAuth2TokenKeyServiceWithCache instance = new OAuth2TokenKeyServiceWithCache();
+		instance.cacheTicker = cacheTicker;
 		return instance;
 	}
 
@@ -182,6 +199,7 @@ class OAuth2TokenKeyServiceWithCache implements Cacheable {
 	private Cache<String, PublicKey> getCache() {
 		if (cache == null) {
 			cache = Caffeine.newBuilder()
+					.ticker(cacheTicker)
 					.expireAfterWrite(cacheConfiguration.getCacheDuration())
 					.maximumSize(cacheConfiguration.getCacheSize())
 					.build();
diff --git a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/OidcConfigurationServiceWithCache.java b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/OidcConfigurationServiceWithCache.java
index d373e3ed..3160cc68 100644
--- a/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/OidcConfigurationServiceWithCache.java
+++ b/java-security/src/main/java/com/sap/cloud/security/token/validation/validators/OidcConfigurationServiceWithCache.java
@@ -21,7 +21,7 @@ import com.sap.cloud.security.xsuaa.client.OidcConfigurationService;
 public class OidcConfigurationServiceWithCache {
 	private OidcConfigurationService oidcConfigurationService; // access via getter
 	private Cache<String, OAuth2ServiceEndpointsProvider> cache;
-	private long cacheValidityInSeconds = 600; // Revert after 15 Minutes
+	private long cacheValidityInSeconds = 600; // old keys should expire after 10 minutes
 	private long cacheSize = 1000;
 
 	private OidcConfigurationServiceWithCache() {
@@ -111,7 +111,8 @@ public class OidcConfigurationServiceWithCache {
 
 	private Cache<String, OAuth2ServiceEndpointsProvider> getCache() {
 		if (cache == null) {
-			cache = Caffeine.newBuilder().expireAfterWrite(cacheValidityInSeconds, TimeUnit.SECONDS)
+			cache = Caffeine.newBuilder()
+					.expireAfterWrite(cacheValidityInSeconds, TimeUnit.SECONDS)
 					.maximumSize(cacheSize)
 					.build();
 		}


Test patch:
diff --git a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCacheTest.java b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCacheTest.java
index cffbef60..a23fd48f 100644
--- a/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCacheTest.java
+++ b/java-security/src/test/java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCacheTest.java
@@ -3,6 +3,7 @@ package com.sap.cloud.security.token.validation.validators;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.*;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.when;
 
@@ -14,25 +15,34 @@ import java.security.PublicKey;
 import java.security.spec.InvalidKeySpecException;
 import java.time.Duration;
 
+import com.github.benmanes.caffeine.cache.Ticker;
 import com.sap.cloud.security.xsuaa.client.OAuth2ServiceException;
 import com.sap.cloud.security.xsuaa.client.OAuth2TokenKeyService;
+import com.sap.cloud.security.xsuaa.tokenflows.TokenCacheConfiguration;
 import org.apache.commons.io.IOUtils;
 import org.junit.Before;
 import org.junit.Test;
-import org.mockito.Mockito;
 
 public class OAuth2TokenKeyServiceWithCacheTest {
+
+	public static final TokenCacheConfiguration CACHE_CONFIGURATION = TokenCacheConfiguration.defaultConfiguration();
+
 	OAuth2TokenKeyServiceWithCache cut;
 	OAuth2TokenKeyService tokenKeyServiceMock;
 	URI TOKEN_KEYS_URI = URI.create("https://myauth.com/jwks_uri");
+	private TestCacheTicker testCacheTicker;
 
 	@Before
 	public void setup() throws IOException {
-		tokenKeyServiceMock = Mockito.mock(OAuth2TokenKeyService.class);
+		tokenKeyServiceMock = mock(OAuth2TokenKeyService.class);
 		when(tokenKeyServiceMock.retrieveTokenKeys(TOKEN_KEYS_URI))
 				.thenReturn(IOUtils.resourceToString("/jsonWebTokenKeys.json", StandardCharsets.UTF_8));
 
-		cut = OAuth2TokenKeyServiceWithCache.getInstance().withTokenKeyService(tokenKeyServiceMock);
+		testCacheTicker = new TestCacheTicker();
+		cut = OAuth2TokenKeyServiceWithCache
+				.getInstance(testCacheTicker)
+				.withTokenKeyService(tokenKeyServiceMock)
+				.withCacheConfiguration(CACHE_CONFIGURATION);
 	}
 
 	@Test
@@ -63,29 +73,27 @@ public class OAuth2TokenKeyServiceWithCacheTest {
 	@Test
 	public void retrieveTokenKeys() throws OAuth2ServiceException, InvalidKeySpecException, NoSuchAlgorithmException {
 		PublicKey key = cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI);
-		assertThat(String.valueOf(key.getAlgorithm())).isEqualTo("RSA");
 
-		Mockito.verify(tokenKeyServiceMock, times(1))
-				.retrieveTokenKeys(TOKEN_KEYS_URI);
+		assertThat(String.valueOf(key.getAlgorithm())).isEqualTo("RSA");
+		verify(tokenKeyServiceMock, times(1)).retrieveTokenKeys(TOKEN_KEYS_URI);
 	}
 
 	@Test
 	public void getCachedTokenKeys() throws OAuth2ServiceException, InvalidKeySpecException, NoSuchAlgorithmException {
 		PublicKey key = cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI);
-
 		PublicKey cachedKey = cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI);
+
 		assertThat(cachedKey).isNotNull();
 		assertThat(cachedKey).isSameAs(key);
-
-		Mockito.verify(tokenKeyServiceMock, times(1))
-				.retrieveTokenKeys(TOKEN_KEYS_URI);
+		verify(tokenKeyServiceMock, times(1)).retrieveTokenKeys(TOKEN_KEYS_URI);
 	}
 
 	@Test
 	public void retrieveNoTokenKeys_returnsNull()
 			throws OAuth2ServiceException, InvalidKeySpecException, NoSuchAlgorithmException {
-		cut.withTokenKeyService(Mockito.mock(OAuth2TokenKeyService.class));
+		cut.withTokenKeyService(mock(OAuth2TokenKeyService.class));
 		PublicKey key = cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI);
+
 		assertThat(key).isNull();
 	}
 
@@ -103,24 +111,52 @@ public class OAuth2TokenKeyServiceWithCacheTest {
 	public void retrieveTokenKeys_afterCacheWasCleared()
 			throws OAuth2ServiceException, InvalidKeySpecException, NoSuchAlgorithmException {
 		cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI);
-
 		cut.clearCache();
-
 		PublicKey cachedKey = cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI);
+
 		assertThat(cachedKey).isNotNull();
+		verify(tokenKeyServiceMock, times(2)).retrieveTokenKeys(TOKEN_KEYS_URI);
+	}
+
+	@Test
+	public void retrieveTokenKeys_doesRequestKeysAgainAfterCacheExpired()
+			throws OAuth2ServiceException, InvalidKeySpecException, NoSuchAlgorithmException {
+		cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI);
+		testCacheTicker.advance(CACHE_CONFIGURATION.getCacheDuration()); // just expired
+		cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI);
+
+		verify(tokenKeyServiceMock, times(2)).retrieveTokenKeys(any());
+	}
+
+	@Test
+	public void retrieveTokenKeysForNewKeyId()
+			throws OAuth2ServiceException, InvalidKeySpecException, NoSuchAlgorithmException {
+		cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI);
+		cut.getPublicKey(JwtSignatureAlgorithm.RS256, "not-seen-yet", TOKEN_KEYS_URI);
 
-		Mockito.verify(tokenKeyServiceMock, times(2))
-				.retrieveTokenKeys(TOKEN_KEYS_URI);
+		verify(tokenKeyServiceMock, times(2)).retrieveTokenKeys(any());
 	}
 
+
 	@Test
-	public void retrieveTokenKeysFromAnotherEndpoint()
+	public void retrieveTokenKeysForNewEndpoint()
 			throws OAuth2ServiceException, InvalidKeySpecException, NoSuchAlgorithmException {
 		cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", TOKEN_KEYS_URI);
-		cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-1", TOKEN_KEYS_URI);
 		cut.getPublicKey(JwtSignatureAlgorithm.RS256, "key-id-0", URI.create("http://another/url"));
 
-		Mockito.verify(tokenKeyServiceMock, times(2))
-				.retrieveTokenKeys(any());
+		verify(tokenKeyServiceMock, times(2)).retrieveTokenKeys(any());
+	}
+
+	private class TestCacheTicker implements  Ticker {
+		long elapsed = 0;
+
+		@Override
+		public long read() {
+			return elapsed;
+		}
+
+		public void advance(Duration duration) {
+			this.elapsed = elapsed + duration.toNanos();
+		}
 	}
 }