GUACAMOLE-641: Retrieve tokens asynchronously and in parallel.

extensions/guacamole-auth-vault/modules/guacamole-auth-vault-azure/src/main/java/org/apache/guacamole/auth/vault/azure/secret/AzureKeyVaultSecretService.java

@@ -25,10 +25,11 @@ import com.google.inject.Singleton;
 import com.microsoft.azure.keyvault.KeyVaultClient;
 import com.microsoft.azure.keyvault.authentication.KeyVaultCredentials;
 import com.microsoft.azure.keyvault.models.SecretBundle;
+import com.microsoft.rest.ServiceCallback;
+import java.util.concurrent.CompletableFuture;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.apache.guacamole.GuacamoleException;
-import org.apache.guacamole.GuacamoleServerException;
 import org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultAuthenticationException;
 import org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultConfigurationService;
 import org.apache.guacamole.auth.vault.secret.CachedVaultSecretService;
@@ -77,20 +78,43 @@ public class AzureKeyVaultSecretService extends CachedVaultSecretService {
         int ttl = confService.getSecretTTL();
         String url = confService.getVaultURL();
 
-        try {
+        CompletableFuture<String> retrievedValue = new CompletableFuture<>();
 
-            // Retrieve requested secret from Azure Key Vault
+        // getSecretAsync() still blocks for around half a second, despite
-            KeyVaultClient client = new KeyVaultClient(credentialProvider.get());
+        // technically being asynchronous
-            SecretBundle secret = client.getSecret(url, name);
+        (new Thread() {
 
-            // Cache retrieved value
+            @Override
-            String value = (secret != null) ? secret.value() : null;
+            public void run() {
-            return new CachedSecret(value, ttl);
+                try {
 
-        }
+                    // Retrieve requested secret from Azure Key Vault
-        catch (AzureKeyVaultAuthenticationException e) {
+                    KeyVaultClient client = new KeyVaultClient(credentialProvider.get());
-            throw new GuacamoleServerException("Unable to authenticate with Azure.", e);
+                    client.getSecretAsync(url, name, new ServiceCallback<SecretBundle>() {
-        }
+
+                        @Override
+                        public void failure(Throwable t) {
+                            retrievedValue.completeExceptionally(t);
+                        }
+
+                        @Override
+                        public void success(SecretBundle secret) {
+                            String value = (secret != null) ? secret.value() : null;
+                            retrievedValue.complete(value);
+                        }
+
+                    });
+
+                }
+                catch (AzureKeyVaultAuthenticationException e) {
+                    retrievedValue.completeExceptionally(e);
+                }
+            }
+
+        }).start();
+
+        // Cache retrieved value
+        return new CachedSecret(retrievedValue, ttl);
 
     }
 

extensions/guacamole-auth-vault/modules/guacamole-auth-vault-base/src/main/java/org/apache/guacamole/auth/vault/secret/CachedVaultSecretService.java

@@ -48,9 +48,10 @@ public abstract class CachedVaultSecretService implements VaultSecretService {
     protected class CachedSecret {
 
         /**
-         * The value of the secret at the time it was last retrieved.
+         * A Future which contains or will contain the value of the secret at
+         * the time it was last retrieved.
          */
-        private final String value;
+        private final Future<String> value;
 
         /**
          * The time the value should be considered out-of-date, in milliseconds
@@ -64,13 +65,15 @@ public abstract class CachedVaultSecretService implements VaultSecretService {
          * which it should be considered out-of-date.
          *
          * @param value
-         *     The current value of the secret.
+         *     A Future which contains or will contain the current value of the
+         *     secret. If no such secret exists, the given Future should
+         *     complete with null.
          *
          * @param ttl
          *     The maximum number of milliseconds that this value should be
          *     cached.
          */
-        public CachedSecret(String value, int ttl) {
+        public CachedSecret(Future<String> value, int ttl) {
             this.value = value;
             this.expires = System.currentTimeMillis() + ttl;
         }
@@ -80,9 +83,14 @@ public abstract class CachedVaultSecretService implements VaultSecretService {
          * The actual value of the secret may have changed.
          *
          * @return
-         *     The value of the secret at the time it was last retrieved.
+         *     A Future which will eventually complete with the value of the
+         *     secret at the time it was last retrieved. If no such secret
+         *     exists, the Future will be completed with null. If an error
+         *     occurs which prevents retrieval of the secret, that error will
+         *     be exposed through an ExecutionException when an attempt is made
+         *     to retrieve the value from the Future.
          */
-        public String getValue() {
+        public Future<String> getValue() {
             return value;
         }
 
@@ -129,7 +137,7 @@ public abstract class CachedVaultSecretService implements VaultSecretService {
             throws GuacamoleException;
 
     @Override
-    public String getValue(String name) throws GuacamoleException {
+    public Future<String> getValue(String name) throws GuacamoleException {
 
         CompletableFuture<CachedSecret> refreshEntry;
 
@@ -175,7 +183,7 @@ public abstract class CachedVaultSecretService implements VaultSecretService {
         try {
             CachedSecret secret = refreshCachedSecret(name);
             refreshEntry.complete(secret);
-            logger.debug("Cached secret for \"{}\" has been refreshed.", name);
+            logger.debug("Cached secret for \"{}\" will be refreshed.", name);
             return secret.getValue();
         }
 

extensions/guacamole-auth-vault/modules/guacamole-auth-vault-base/src/main/java/org/apache/guacamole/auth/vault/secret/VaultSecretService.java

@@ -19,6 +19,7 @@
 
 package org.apache.guacamole.auth.vault.secret;
 
+import java.util.concurrent.Future;
 import org.apache.guacamole.GuacamoleException;
 
 /**
@@ -49,19 +50,23 @@ public interface VaultSecretService {
     String canonicalize(String name);
 
     /**
-     * Returns the value of the secret having the given name. If no such
+     * Returns a Future which eventually completes with the value of the secret
-     * secret exists, null is returned.
+     * having the given name. If no such secret exists, the Future will be
+     * completed with null.
      *
      * @param name
      *     The name of the secret to retrieve.
      *
      * @return
-     *     The value of the secret having the given name, or null if no such
+     *     A Future which completes with value of the secret having the given
-     *     secret exists.
+     *     name. If no such secret exists, the Future will be completed with
+     *     null. If an error occurs asynchronously which prevents retrieval of
+     *     the secret, that error will be exposed through an ExecutionException
+     *     when an attempt is made to retrieve the value from the Future.
      *
      * @throws GuacamoleException
      *     If the secret cannot be retrieved due to an error.
      */
-    String getValue(String name) throws GuacamoleException;
+    Future<String> getValue(String name) throws GuacamoleException;
 
 }

extensions/guacamole-auth-vault/modules/guacamole-auth-vault-base/src/main/java/org/apache/guacamole/auth/vault/user/VaultUserContext.java

@@ -24,7 +24,10 @@ import com.google.inject.assistedinject.Assisted;
 import com.google.inject.assistedinject.AssistedInject;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
 import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
 import org.apache.guacamole.auth.vault.conf.VaultConfigurationService;
 import org.apache.guacamole.net.auth.Connection;
 import org.apache.guacamole.net.auth.ConnectionGroup;
@@ -149,9 +152,9 @@ public class VaultUserContext extends TokenInjectingUserContext {
     }
 
     /**
-     * Retrieve all applicable tokens and corresponding values from the vault,
+     * Initiates asynchronous retrieval of all applicable tokens and
-     * using the given TokenFilter to filter tokens within the secret names
+     * corresponding values from the vault, using the given TokenFilter to
-     * prior to retrieving those secrets.
+     * filter tokens within the secret names prior to retrieving those secrets.
      *
      * @param tokenMapping
      *     The mapping dictating the name of the secret which maps to each
@@ -165,20 +168,20 @@ public class VaultUserContext extends TokenInjectingUserContext {
      *     secrets to be retrieved from the vault.
      *
      * @return
-     *     The tokens which should be added to the in-progress call to
+     *     A Map of token name to Future, where each Future represents the
-     *     connect().
+     *     pending retrieval operation which will ultimately be completed with
+     *     the value of all secrets mapped to that token.
      *
      * @throws GuacamoleException
      *     If the value for any applicable secret cannot be retrieved from the
      *     vault due to an error.
      */
-    private Map<String, String> getTokens(Map<String, String> tokenMapping,
+    private Map<String, Future<String>> getTokens(Map<String, String> tokenMapping,
             TokenFilter filter) throws GuacamoleException {
 
-        Map<String, String> tokens = new HashMap<>();
+        // Populate map with pending secret retrieval operations corresponding
-
+        // to each mapped token
-        // Populate map with tokens containing the values of all secrets
+        Map<String, Future<String>> pendingTokens = new HashMap<>(tokenMapping.size());
-        // indicated in the token mapping
         for (Map.Entry<String, String> entry : tokenMapping.entrySet()) {
 
             // Translate secret pattern into secret name, ignoring any
@@ -195,19 +198,60 @@ public class VaultUserContext extends TokenInjectingUserContext {
                 continue;
             }
 
+            // Initiate asynchronous retrieval of the token value
+            String tokenName = entry.getKey();
+            Future<String> secret = secretService.getValue(secretName);
+            pendingTokens.put(tokenName, secret);
+
+        }
+
+        return pendingTokens;
+
+    }
+
+    /**
+     * Waits for all pending secret retrieval operations to complete,
+     * transforming each Future within the given Map into its contained String
+     * value.
+     *
+     * @param pendingTokens
+     *     A Map of token name to Future, where each Future represents the
+     *     pending retrieval operation which will ultimately be completed with
+     *     the value of all secrets mapped to that token.
+     *
+     * @throws GuacamoleException
+     *     If the value for any applicable secret cannot be retrieved from the
+     *     vault due to an error.
+     */
+    private Map<String, String> resolve(Map<String,
+            Future<String>> pendingTokens) throws GuacamoleException {
+
+        // Populate map with tokens containing the values of their
+        // corresponding secrets
+        Map<String, String> tokens = new HashMap<>(pendingTokens.size());
+        for (Map.Entry<String, Future<String>> entry : pendingTokens.entrySet()) {
+
+            // Complete secret retrieval operation, blocking if necessary
+            String secretValue;
+            try {
+                secretValue = entry.getValue().get();
+            }
+            catch (InterruptedException | ExecutionException e) {
+                throw new GuacamoleServerException("Retrieval of secret value "
+                        + "failed.", e);
+            }
+
             // If a value is defined for the secret in question, store that
             // value under the mapped token
             String tokenName = entry.getKey();
-            String secretValue = secretService.getValue(secretName);
             if (secretValue != null) {
                 tokens.put(tokenName, secretValue);
                 logger.debug("Token \"{}\" populated with value from "
-                        + "secret \"{}\".", tokenName, secretName);
+                        + "secret.", tokenName);
             }
             else
                 logger.debug("Token \"{}\" not populated. Mapped "
-                        + "secret \"{}\" has no value.",
+                        + "secret has no value.", tokenName);
-                        tokenName, secretName);
 
         }
 
@@ -231,7 +275,7 @@ public class VaultUserContext extends TokenInjectingUserContext {
 
         // Substitute tokens producing secret names, retrieving and storing
         // those secrets as parameter tokens
-        return getTokens(confService.getTokenMapping(), filter);
+        return resolve(getTokens(confService.getTokenMapping(), filter));
 
     }
 
@@ -274,7 +318,7 @@ public class VaultUserContext extends TokenInjectingUserContext {
 
         // Substitute tokens producing secret names, retrieving and storing
         // those secrets as parameter tokens
-        return getTokens(confService.getTokenMapping(), filter);
+        return resolve(getTokens(confService.getTokenMapping(), filter));
 
     }