From 02233df1ac1db0cac1e9ed250ffbd962215c3ad8 Mon Sep 17 00:00:00 2001 From: Scott Schaab Date: Thu, 1 Nov 2018 17:01:45 -0700 Subject: [PATCH] Merging Dev to Master (#69) * Do not cache unversionned keys * Brihicks/fix gauva26.0 problems (#63) * Fixed issues resulting from incompatibilities with Guava 26.0 * Updated versioning * Update pom.xml (#66) * Versioning fix * Update pom.xml * Updated versions of all files 1.1.2 --- azure-keyvault-complete/pom.xml | 6 +- azure-keyvault-core/pom.xml | 4 +- azure-keyvault-cryptography/pom.xml | 4 +- azure-keyvault-extensions/pom.xml | 5 +- .../extensions/CachingKeyResolver.java | 29 ++++- .../keyvault/extensions/KeyVaultKey.java | 8 +- .../extensions/KeyVaultKeyResolver.java | 6 +- .../test/CachingKeyResolverTest.java | 100 ++++++++++++++---- azure-keyvault-webkey/pom.xml | 4 +- azure-keyvault/pom.xml | 4 +- pom.xml | 12 +-- 11 files changed, 131 insertions(+), 51 deletions(-) diff --git a/azure-keyvault-complete/pom.xml b/azure-keyvault-complete/pom.xml index 1099d7c..11fd130 100644 --- a/azure-keyvault-complete/pom.xml +++ b/azure-keyvault-complete/pom.xml @@ -7,13 +7,13 @@ the MIT License. See License.txt in the project root for license information. -- com.microsoft.azure azure-keyvault-parent - 1.1.1 + 1.1.2 ../pom.xml com.microsoft.azure azure-keyvault-complete - 1.1.1 + 1.1.2 pom @@ -69,4 +69,4 @@ the MIT License. See License.txt in the project root for license information. -- - \ No newline at end of file + diff --git a/azure-keyvault-core/pom.xml b/azure-keyvault-core/pom.xml index 8414241..cee8257 100644 --- a/azure-keyvault-core/pom.xml +++ b/azure-keyvault-core/pom.xml @@ -8,12 +8,12 @@ com.microsoft.azure azure-keyvault-parent - 1.1.1 + 1.1.2 ../pom.xml azure-keyvault-core - 1.1.1 + 1.1.2 jar Microsoft Azure SDK for Key Vault Core diff --git a/azure-keyvault-cryptography/pom.xml b/azure-keyvault-cryptography/pom.xml index d8763fe..edcde42 100644 --- a/azure-keyvault-cryptography/pom.xml +++ b/azure-keyvault-cryptography/pom.xml @@ -7,12 +7,12 @@ com.microsoft.azure azure-keyvault-parent - 1.1.1 + 1.1.2 ../pom.xml azure-keyvault-cryptography - 1.1.1 + 1.1.2 jar Microsoft Azure SDK for Key Vault Cryptography diff --git a/azure-keyvault-extensions/pom.xml b/azure-keyvault-extensions/pom.xml index 98e6957..6c3af7a 100644 --- a/azure-keyvault-extensions/pom.xml +++ b/azure-keyvault-extensions/pom.xml @@ -8,12 +8,12 @@ com.microsoft.azure azure-keyvault-parent - 1.1.1 + 1.1.2 ../pom.xml azure-keyvault-extensions - 1.1.1 + 1.1.2 jar Microsoft Azure SDK for Key Vault Extensions @@ -64,7 +64,6 @@ com.microsoft.azure azure-keyvault - 1.1.1 diff --git a/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/CachingKeyResolver.java b/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/CachingKeyResolver.java index cfefd73..62bca3f 100644 --- a/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/CachingKeyResolver.java +++ b/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/CachingKeyResolver.java @@ -10,6 +10,8 @@ import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; +import com.microsoft.azure.keyvault.KeyIdentifier; import com.microsoft.azure.keyvault.core.IKey; import com.microsoft.azure.keyvault.core.IKeyResolver; @@ -18,25 +20,46 @@ */ public class CachingKeyResolver implements IKeyResolver { + private final IKeyResolver keyResolver; private final LoadingCache> cache; - + /** * Constructor. * @param capacity the cache size * @param keyResolver the key resolver */ public CachingKeyResolver(int capacity, final IKeyResolver keyResolver) { + this.keyResolver = keyResolver; cache = CacheBuilder.newBuilder().maximumSize(capacity) .build(new CacheLoader>() { @Override public ListenableFuture load(String kid) { return keyResolver.resolveKeyAsync(kid); - } }); + } + }); } @Override public ListenableFuture resolveKeyAsync(String kid) { - return cache.getUnchecked(kid); + KeyIdentifier keyIdentifier = new KeyIdentifier(kid); + if (keyIdentifier.version() == null) { + final ListenableFuture key = keyResolver.resolveKeyAsync(kid); + key.addListener(new Runnable() { + @Override + public void run() { + try { + cache.put(key.get().getKid(), key); + } catch (Exception e) { + // Key caching will occur on first read + } + } + }, + MoreExecutors.directExecutor() + ); + return key; + } else { + return cache.getUnchecked(kid); + } } } diff --git a/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/KeyVaultKey.java b/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/KeyVaultKey.java index f903de9..bfb58a8 100644 --- a/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/KeyVaultKey.java +++ b/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/KeyVaultKey.java @@ -7,6 +7,8 @@ import java.io.IOException; import java.security.NoSuchAlgorithmException; + +import com.google.common.util.concurrent.MoreExecutors; import org.apache.commons.lang3.tuple.Pair; import org.apache.commons.lang3.tuple.Triple; @@ -160,7 +162,7 @@ public ListenableFuture decryptAsync(byte[] ciphertext, byte[] iv, byte[ new JsonWebKeyEncryptionAlgorithm(algorithm), ciphertext, null); - return Futures.transform(futureCall, new DecryptResultTransform()); + return Futures.transform(futureCall, new DecryptResultTransform(), MoreExecutors.directExecutor()); } @Override @@ -198,7 +200,7 @@ public ListenableFuture unwrapKeyAsync(byte[] ciphertext, String algorit new JsonWebKeyEncryptionAlgorithm(algorithm), ciphertext, null); - return Futures.transform(futureCall, new DecryptResultTransform()); + return Futures.transform(futureCall, new DecryptResultTransform(), MoreExecutors.directExecutor()); } @Override @@ -218,7 +220,7 @@ public ListenableFuture> signAsync(byte[] digest, String al new JsonWebKeySignatureAlgorithm(algorithm), digest, null); - return Futures.transform(futureCall, new SignResultTransform(algorithm)); + return Futures.transform(futureCall, new SignResultTransform(algorithm), MoreExecutors.directExecutor()); } @Override diff --git a/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/KeyVaultKeyResolver.java b/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/KeyVaultKeyResolver.java index a969082..3e8cc13 100644 --- a/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/KeyVaultKeyResolver.java +++ b/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/KeyVaultKeyResolver.java @@ -7,6 +7,8 @@ package com.microsoft.azure.keyvault.extensions; import java.security.Provider; + +import com.google.common.util.concurrent.MoreExecutors; import org.apache.commons.codec.binary.Base64; import com.google.common.base.Function; @@ -98,13 +100,13 @@ public KeyVaultKeyResolver(KeyVaultClient client, Provider provider) { private ListenableFuture resolveKeyFromSecretAsync(String kid) { ListenableFuture futureCall = client.getSecretAsync(kid, null); - return Futures.transform(futureCall, new FutureKeyFromSecret()); + return Futures.transform(futureCall, new FutureKeyFromSecret(), MoreExecutors.directExecutor()); } private ListenableFuture resolveKeyFromKeyAsync(String kid) { ListenableFuture futureCall = client.getKeyAsync(kid, null); - return Futures.transform(futureCall, new FutureKeyFromKey()); + return Futures.transform(futureCall, new FutureKeyFromKey(), MoreExecutors.directExecutor()); } @Override diff --git a/azure-keyvault-extensions/src/test/java/com/microsoft/azure/keyvault/extensions/test/CachingKeyResolverTest.java b/azure-keyvault-extensions/src/test/java/com/microsoft/azure/keyvault/extensions/test/CachingKeyResolverTest.java index a5263c8..39e15a5 100644 --- a/azure-keyvault-extensions/src/test/java/com/microsoft/azure/keyvault/extensions/test/CachingKeyResolverTest.java +++ b/azure-keyvault-extensions/src/test/java/com/microsoft/azure/keyvault/extensions/test/CachingKeyResolverTest.java @@ -18,27 +18,32 @@ package com.microsoft.azure.keyvault.extensions.test; -import static org.junit.Assert.*; - -import org.junit.Test; - import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.UncheckedExecutionException; import com.microsoft.azure.keyvault.core.IKey; import com.microsoft.azure.keyvault.core.IKeyResolver; import com.microsoft.azure.keyvault.extensions.CachingKeyResolver; +import org.junit.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import java.util.concurrent.Executor; + +import static org.junit.Assert.*; import static org.mockito.Mockito.*; public class CachingKeyResolverTest { - + @SuppressWarnings("unchecked") final ListenableFuture ikeyAsync = mock(ListenableFuture.class); - final static String keyId = "keyID"; - final static String keyId2 = "keyID2"; - final static String keyId3 = "keyID3"; - + final static String keyId = "https://test.vault.azure.net/keys/keyID/version"; + final static String keyId2 = "https://test.vault.azure.net/keys/keyID2/version"; + final static String keyId3 = "https://test.vault.azure.net/keys/keyID3/version"; + final static String newerKeyId3 = "https://test.vault.azure.net/keys/keyID3/version2"; + final static String unversionnedKeyId3 = "https://test.vault.azure.net/keys/keyID3"; + - /* + /* * Tests the capacity limit of CachingKeyResolver by adding more keys * than the cache limit and verifying that least recently used entity is evicted. */ @@ -47,51 +52,100 @@ public void KeyVault_CapacityLimitOfCachingKeyResolver() { IKeyResolver mockedKeyResolver = mock(IKeyResolver.class); CachingKeyResolver resolver = new CachingKeyResolver(2, mockedKeyResolver); - + when(mockedKeyResolver.resolveKeyAsync(keyId)).thenReturn(ikeyAsync); when(mockedKeyResolver.resolveKeyAsync(keyId2)).thenReturn(ikeyAsync); when(mockedKeyResolver.resolveKeyAsync(keyId3)).thenReturn(ikeyAsync); - + resolver.resolveKeyAsync(keyId); resolver.resolveKeyAsync(keyId2); resolver.resolveKeyAsync(keyId3); - + resolver.resolveKeyAsync(keyId2); resolver.resolveKeyAsync(keyId3); resolver.resolveKeyAsync(keyId); resolver.resolveKeyAsync(keyId3); - + verify(mockedKeyResolver, times(1)).resolveKeyAsync(keyId2); verify(mockedKeyResolver, times(1)).resolveKeyAsync(keyId3); verify(mockedKeyResolver, times(2)).resolveKeyAsync(keyId); } - - /* - * Tests the behavior of CachingKeyResolver when resolving key throws - * and validate that the failed entity is not added to the cache. + + /* + * Tests the behavior of CachingKeyResolver when resolving key throws + * and validate that the failed entity is not added to the cache. */ @Test public void KeyVault_CachingKeyResolverThrows() { IKeyResolver mockedKeyResolver = mock(IKeyResolver.class); CachingKeyResolver resolver = new CachingKeyResolver(10, mockedKeyResolver); - + // First throw exception and for the second call return a value when(mockedKeyResolver.resolveKeyAsync(keyId)) .thenThrow(new RuntimeException("test")) .thenReturn(ikeyAsync); - + try { resolver.resolveKeyAsync(keyId); - assertFalse("Should have thrown an exception.", true); + fail("Should have thrown an exception."); } catch (UncheckedExecutionException e) { assertTrue("RuntimeException is expected.", e.getCause() instanceof RuntimeException); } - + resolver.resolveKeyAsync(keyId); resolver.resolveKeyAsync(keyId); - + verify(mockedKeyResolver, times(2)).resolveKeyAsync(keyId); } + + /* + * Tests that CachingKeyResolver does not cache unversionned keys, + * but does cache the result versionned key + */ + @Test + public void KeyVault_CachingUnversionnedKey() throws Exception { + IKeyResolver mockedKeyResolver = mock(IKeyResolver.class); + CachingKeyResolver resolver = new CachingKeyResolver(2, mockedKeyResolver); + + IKey key = mock(IKey.class); + + when(mockedKeyResolver.resolveKeyAsync(unversionnedKeyId3)).thenReturn(ikeyAsync); + when(ikeyAsync.get()).thenReturn(key); + doAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocationOnMock) throws Throwable { + invocationOnMock.getArgumentAt(0, Runnable.class).run(); + return null; + } + }).when(ikeyAsync).addListener(any(Runnable.class), any(Executor.class)); + when(key.getKid()).thenReturn(keyId3); + + /* + * First resolve unversionned key + */ + ListenableFuture result = resolver.resolveKeyAsync(unversionnedKeyId3); + assertEquals(result.get().getKid(), keyId3); + verify(mockedKeyResolver, times(1)).resolveKeyAsync(unversionnedKeyId3); + verify(mockedKeyResolver, times(0)).resolveKeyAsync(keyId3); + + /* + * Second resolve unversionned key, but the result should be a newer key + */ + when(key.getKid()).thenReturn(newerKeyId3); + result = resolver.resolveKeyAsync(unversionnedKeyId3); + assertEquals(result.get().getKid(), newerKeyId3); + verify(mockedKeyResolver, times(2)).resolveKeyAsync(unversionnedKeyId3); + verify(mockedKeyResolver, times(0)).resolveKeyAsync(keyId3); + verify(mockedKeyResolver, times(0)).resolveKeyAsync(newerKeyId3); + + /* + * Check that versionned keys were added to the cache, and do not get resolved again + */ + resolver.resolveKeyAsync(keyId3); + resolver.resolveKeyAsync(newerKeyId3); + verify(mockedKeyResolver, times(0)).resolveKeyAsync(keyId3); + verify(mockedKeyResolver, times(0)).resolveKeyAsync(newerKeyId3); + } } diff --git a/azure-keyvault-webkey/pom.xml b/azure-keyvault-webkey/pom.xml index 258ec05..19c30ee 100644 --- a/azure-keyvault-webkey/pom.xml +++ b/azure-keyvault-webkey/pom.xml @@ -6,12 +6,12 @@ com.microsoft.azure azure-keyvault-parent - 1.1.1 + 1.1.2 ../pom.xml azure-keyvault-webkey - 1.1.1 + 1.1.2 jar Microsoft Azure SDK for Key Vault WebKey diff --git a/azure-keyvault/pom.xml b/azure-keyvault/pom.xml index 0e8da25..b6da877 100644 --- a/azure-keyvault/pom.xml +++ b/azure-keyvault/pom.xml @@ -6,12 +6,12 @@ the MIT License. See License.txt in the project root for license information. -- com.microsoft.azure azure-keyvault-parent - 1.1.1 + 1.1.2 ../pom.xml azure-keyvault - 1.1.1 + 1.1.2 jar Microsoft Azure SDK for Key Vault diff --git a/pom.xml b/pom.xml index ca2cd91..a87523e 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ 4.0.0 com.microsoft.azure - 1.1.1 + 1.1.2 azure-keyvault-parent pom @@ -90,27 +90,27 @@ com.microsoft.azure azure-keyvault-webkey - 1.1.1 + 1.1.2 com.microsoft.azure azure-keyvault-cryptography - 1.1.1 + 1.1.2 com.microsoft.azure azure-keyvault-core - 1.1.1 + 1.1.2 com.microsoft.azure azure-keyvault - 1.1.1 + 1.1.2 com.microsoft.azure azure-keyvault-extensions - 1.1.1 + 1.1.2