GUACAMOLE-1629: Implement multiple-vault support for KSM codebase.

extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/KsmAuthenticationProviderModule.java

@@ -26,10 +26,14 @@ import org.apache.guacamole.vault.ksm.conf.KsmConfigurationService;
 import org.apache.guacamole.vault.ksm.secret.KsmSecretService;
 import org.apache.guacamole.vault.conf.VaultAttributeService;
 import org.apache.guacamole.vault.conf.VaultConfigurationService;
+import org.apache.guacamole.vault.ksm.secret.KsmCache;
+import org.apache.guacamole.vault.ksm.secret.KsmCacheFactory;
 import org.apache.guacamole.vault.ksm.secret.KsmClient;
 import org.apache.guacamole.vault.ksm.secret.KsmRecordService;
 import org.apache.guacamole.vault.secret.VaultSecretService;
 
+import com.google.inject.assistedinject.FactoryModuleBuilder;
+
 /**
  * Guice module which configures injections specific to Keeper Secrets
  * Manager support.
@@ -56,6 +60,11 @@ public class KsmAuthenticationProviderModule
         bind(VaultAttributeService.class).to(KsmAttributeService.class);
         bind(VaultConfigurationService.class).to(KsmConfigurationService.class);
         bind(VaultSecretService.class).to(KsmSecretService.class);
+
+        // Bind factory for creating KSM Caches
+        install(new FactoryModuleBuilder()
+                .implement(KsmCache.class, KsmCache.class)
+                .build(KsmCacheFactory.class));
     }
 
 }

extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfig.java

@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.vault.ksm.conf;
+
+import com.keepersecurity.secretsManager.core.InMemoryStorage;
+import com.keepersecurity.secretsManager.core.KeyValueStorage;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+
+/**
+ * A utility for parsing base64-encoded JSON, as output by the Keeper Commander
+ * CLI tool via the "sm client add" command into a Keeper Secrets Manager
+ * {@link KeyValueStorage} object.
+ */
+public class KsmConfig {
+
+    /**
+     * Given a base64-encoded JSON KSM configuration, parse and return a
+     * KeyValueStorage object.
+     *
+     * @param value
+     *     The base64-encoded JSON KSM configuration to parse.
+     *
+     * @return
+     *     The KeyValueStorage that is a result of the parsing operation
+     *
+     * @throws GuacamoleException
+     *     If the provided value is not valid base-64 encoded JSON KSM configuration.
+     */
+    public static KeyValueStorage parseKsmConfig(String value) throws GuacamoleException {
+
+        // Parse base64 value as KSM config storage
+        try {
+            return new InMemoryStorage(value);
+        }
+        catch (IllegalArgumentException e) {
+            throw new GuacamoleServerException("Invalid base64 configuration "
+                    + "for Keeper Secrets Manager.", e);
+        }
+
+    }
+
+}

extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigProperty.java

@@ -19,10 +19,8 @@
 
 package org.apache.guacamole.vault.ksm.conf;
 
-import com.keepersecurity.secretsManager.core.InMemoryStorage;
 import com.keepersecurity.secretsManager.core.KeyValueStorage;
 import org.apache.guacamole.GuacamoleException;
-import org.apache.guacamole.GuacamoleServerException;
 import org.apache.guacamole.properties.GuacamoleProperty;
 
 /**
@@ -39,15 +37,8 @@ public abstract class KsmConfigProperty implements GuacamoleProperty<KeyValueSto
         if (value == null)
             return null;
 
-        // Parse base64 value as KSM config storage
+        // Parse the base-64 encoded JSON into a KeyValueStorage object
-        try {
+        return KsmConfig.parseKsmConfig(value);
-            return new InMemoryStorage(value);
-        }
-        catch (IllegalArgumentException e) {
-            throw new GuacamoleServerException("Invalid base64 configuration "
-                    + "for Keeper Secrets Manager.", e);
-        }
-
     }
 
 }

extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java

@@ -21,10 +21,15 @@ package org.apache.guacamole.vault.ksm.conf;
 
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
+
+import javax.annotation.Nullable;
+
 import org.apache.guacamole.GuacamoleException;
 import org.apache.guacamole.environment.Environment;
 import org.apache.guacamole.properties.BooleanGuacamoleProperty;
 import org.apache.guacamole.vault.conf.VaultConfigurationService;
+
+import com.keepersecurity.secretsManager.core.KeyValueStorage;
 import com.keepersecurity.secretsManager.core.SecretsManagerOptions;
 
 /**
@@ -126,18 +131,31 @@ public class KsmConfigurationService extends VaultConfigurationService {
      * Returns the options required to authenticate with Keeper Secrets Manager
      * when retrieving secrets. These options are read from the contents of
      * base64-encoded JSON configuration data generated by the Keeper Commander
-     * CLI tool.
+     * CLI tool. This configuration data may be passed directly as an argument.
+     * If not provided as an argument, it must be present in the config file.
+     *
+     * @param ksmConfig
+     *     Optional KSM config data. If provided, it will be used instead of any
+     *     KSM config data present in the config file. If not provided, the value
+     *     in the config file will be used.
      *
      * @return
      *     The options that should be used when connecting to Keeper Secrets
      *     Manager when retrieving secrets.
      *
      * @throws GuacamoleException
-     *     If required properties are not specified within
+     *     If an invalid ksmConfig parameter is provided, or required properties
-     *     guacamole.properties or cannot be parsed.
+     *     are not specified within guacamole.properties or cannot be parsed, or
+     *     the KSM configuration cannot be parsed.
      */
-    public SecretsManagerOptions getSecretsManagerOptions() throws GuacamoleException {
+    public SecretsManagerOptions getSecretsManagerOptions(@Nullable String ksmConfig) throws GuacamoleException {
-        return new SecretsManagerOptions(environment.getRequiredProperty(KSM_CONFIG), null,
+
-                getAllowUnverifiedCertificate());
+        // Attempt to parse the KSM config provided as an argument if available.
+        // If not provided as an argument, it must be present in the config file.
+        KeyValueStorage parsedKsmConfig = ksmConfig != null
+                ? KsmConfig.parseKsmConfig(ksmConfig)
+                : environment.getRequiredProperty(KSM_CONFIG);
+
+        return new SecretsManagerOptions(parsedKsmConfig, null, getAllowUnverifiedCertificate());
     }
 }

extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmCache.java

@@ -0,0 +1,484 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.vault.ksm.secret;
+
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
+import com.keepersecurity.secretsManager.core.Hosts;
+import com.keepersecurity.secretsManager.core.KeeperRecord;
+import com.keepersecurity.secretsManager.core.KeeperSecrets;
+import com.keepersecurity.secretsManager.core.Login;
+import com.keepersecurity.secretsManager.core.Notation;
+import com.keepersecurity.secretsManager.core.SecretsManager;
+import com.keepersecurity.secretsManager.core.SecretsManagerOptions;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Future;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.guacamole.GuacamoleException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A cache of Keeper Secrets Manager records, specific to a single KSM account. This class
+ * includes internal functionality necessary to fetch records from KSM as needed.
+ */
+public class KsmCache {
+    /**
+     * Logger for this class.
+     */
+    private static final Logger logger = LoggerFactory.getLogger(KsmCache.class);
+
+    /**
+     * Service for retrieving data from records.
+     */
+    @Inject
+    private KsmRecordService recordService;
+
+    /**
+     * The publicly-accessible URL for Keeper's documentation covering Keeper
+     * notation.
+     */
+    private static final String KEEPER_NOTATION_DOC_URL =
+            "https://docs.keeper.io/secrets-manager/secrets-manager/about/keeper-notation";
+
+    /**
+     * The regular expression that Keeper notation must match to be related to
+     * file retrieval. As the Keeper SDK provides mutually-exclusive for
+     * retrieving secret values and files via notation, the notation must first
+     * be tested to determine whether it refers to a file.
+     */
+    private static final Pattern KEEPER_FILE_NOTATION = Pattern.compile("^(keeper://)?[^/]*/file/.+");
+
+    /**
+     * The maximum amount of time that an entry will be stored in the cache
+     * before being refreshed, in milliseconds.
+     */
+    private static final long CACHE_INTERVAL = 5000;
+
+    /**
+     * The KSM configuration associated with this cache instance.
+     */
+    private final SecretsManagerOptions ksmConfig;
+
+    /**
+     * Create a new KSM cache instance based around the provided KSM configuration.
+     *
+     * @param ksmConfig
+     *     The KSM configuration to use when retrieving properties from KSM.
+     */
+    @AssistedInject
+    public KsmCache(@Assisted SecretsManagerOptions ksmConfig) {
+        this.ksmConfig = ksmConfig;
+    }
+
+    /**
+     * Read/write lock which guards access to all cached data, including the
+     * timestamp recording the last time the cache was refreshed. Readers of
+     * the cache must first acquire (and eventually release) the read lock, and
+     * writers of the cache must first acquire (and eventually release) the
+     * write lock.
+     */
+    private final ReadWriteLock cacheLock = new ReentrantReadWriteLock();
+
+    /**
+     * The timestamp that the cache was last refreshed, in milliseconds, as
+     * returned by System.currentTimeMillis(). This value is automatically
+     * updated if {@link #validateCache()} refreshes the cache. This value must
+     * not be accessed without {@link #cacheLock} acquired appropriately.
+     */
+    private volatile long cacheTimestamp = 0;
+
+    /**
+     * The full cached set of secrets last retrieved from Keeper Secrets
+     * Manager. This value is automatically updated if {@link #validateCache()}
+     * refreshes the cache. This value must not be accessed without
+     * {@link #cacheLock} acquired appropriately.
+     */
+    private KeeperSecrets cachedSecrets = null;
+
+    /**
+     * All records retrieved from Keeper Secrets Manager, where each key is the
+     * UID of the corresponding record. The contents of this Map are
+     * automatically updated if {@link #validateCache()} refreshes the cache.
+     * This Map must not be accessed without {@link #cacheLock} acquired
+     * appropriately.
+     */
+    private final Map<String, KeeperRecord> cachedRecordsByUid = new HashMap<>();
+
+    /**
+     * All records retrieved from Keeper Secrets Manager, where each key is the
+     * hostname or IP address of the corresponding record. The hostname or IP
+     * address of a record is determined by {@link Hosts} fields, thus a record
+     * may be associated with multiple hosts. If a record is associated with
+     * multiple hosts, there will be multiple references to that record within
+     * this Map. The contents of this Map are automatically updated if
+     * {@link #validateCache()} refreshes the cache. This Map must not be
+     * accessed without {@link #cacheLock} acquired appropriately. Before using
+     * a value from this Map, {@link #cachedAmbiguousHosts} must first be
+     * checked to verify that there is indeed only one record associated with
+     * that host.
+     */
+    private final Map<String, KeeperRecord> cachedRecordsByHost = new HashMap<>();
+
+    /**
+     * The set of all hostnames or IP addresses that are associated with
+     * multiple records, and thus cannot uniquely identify a record. The
+     * contents of this Set are automatically updated if
+     * {@link #validateCache()} refreshes the cache. This Set must not be
+     * accessed without {@link #cacheLock} acquired appropriately.This Set
+     * must be checked before using a value retrieved from
+     * {@link #cachedRecordsByHost}.
+     */
+    private final Set<String> cachedAmbiguousHosts = new HashSet<>();
+
+    /**
+     * All records retrieved from Keeper Secrets Manager, where each key is the
+     * username of the corresponding record. The username of a record is
+     * determined by {@link Login} fields, thus a record may be associated with
+     * multiple users. If a record is associated with multiple users, there
+     * will be multiple references to that record within this Map. The contents
+     * of this Map are automatically updated if {@link #validateCache()}
+     * refreshes the cache. This Map must not be accessed without
+     * {@link #cacheLock} acquired appropriately. Before using a value from
+     * this Map, {@link #cachedAmbiguousUsernames} must first be checked to
+     * verify that there is indeed only one record associated with that user.
+     */
+    private final Map<String, KeeperRecord> cachedRecordsByUsername = new HashMap<>();
+
+    /**
+     * The set of all usernames that are associated with multiple records, and
+     * thus cannot uniquely identify a record. The contents of this Set are
+     * automatically updated if {@link #validateCache()} refreshes the cache.
+     * This Set must not be accessed without {@link #cacheLock} acquired
+     * appropriately.This Set must be checked before using a value retrieved
+     * from {@link #cachedRecordsByUsername}.
+     */
+    private final Set<String> cachedAmbiguousUsernames = new HashSet<>();
+
+    /**
+     * Validates that all cached data is current with respect to
+     * {@link #CACHE_INTERVAL}, refreshing data from the server as needed.
+     *
+     * @throws GuacamoleException
+     *     If an error occurs preventing the cached data from being refreshed.
+     */
+    private void validateCache() throws GuacamoleException {
+
+        long currentTime = System.currentTimeMillis();
+
+        // Perform a read-only check that the cache has actually expired before
+        // continuing
+        cacheLock.readLock().lock();
+        try {
+            if (currentTime - cacheTimestamp < CACHE_INTERVAL)
+                return;
+        }
+        finally {
+            cacheLock.readLock().unlock();
+        }
+
+        cacheLock.writeLock().lock();
+        try {
+
+            // Cache may have been updated since the read-only check. Re-verify
+            // that the cache has expired before continuing with a full refresh
+            if (currentTime - cacheTimestamp < CACHE_INTERVAL)
+                return;
+
+            // Attempt to pull all records first, allowing that operation to
+            // succeed/fail BEFORE we clear out the last cached success
+            KeeperSecrets secrets = SecretsManager.getSecrets(ksmConfig);
+            List<KeeperRecord> records = secrets.getRecords();
+
+            // Store all secrets within cache
+            cachedSecrets = secrets;
+
+            // Clear unambiguous cache of all records by UID
+            cachedRecordsByUid.clear();
+
+            // Clear cache of host-based records
+            cachedAmbiguousHosts.clear();
+            cachedRecordsByHost.clear();
+
+            // Clear cache of login-based records
+            cachedAmbiguousUsernames.clear();
+            cachedRecordsByUsername.clear();
+
+            // Store all records, sorting each into host-based and login-based
+            // buckets
+            records.forEach(record -> {
+
+                // Store based on UID ...
+                cachedRecordsByUid.put(record.getRecordUid(), record);
+
+                // ... and hostname/address
+                String hostname = recordService.getHostname(record);
+                addRecordForHost(record, hostname);
+
+                // Store based on username ONLY if no hostname (will otherwise
+                // result in ambiguous entries for servers tied to identical
+                // accounts)
+                if (hostname == null)
+                    addRecordForLogin(record, recordService.getUsername(record));
+
+            });
+
+            // Cache has been refreshed
+            this.cacheTimestamp = System.currentTimeMillis();
+
+        }
+        finally {
+            cacheLock.writeLock().unlock();
+        }
+
+    }
+
+    /**
+     * Associates the given record with the given hostname. The hostname may be
+     * null. Both {@link #cachedRecordsByHost} and {@link #cachedAmbiguousHosts}
+     * are updated appropriately. The write lock of {@link #cacheLock} must
+     * already be acquired before invoking this function.
+     *
+     * @param record
+     *     The record to associate with the hosts in the given field.
+     *
+     * @param hostname
+     *     The hostname/address that the given record should be associated
+     *     with. This may be null.
+     */
+    private void addRecordForHost(KeeperRecord record, String hostname) {
+
+        if (hostname == null)
+            return;
+
+        KeeperRecord existing = cachedRecordsByHost.putIfAbsent(hostname, record);
+        if (existing != null && record != existing)
+            cachedAmbiguousHosts.add(hostname);
+
+    }
+
+    /**
+     * Associates the given record with the given username. The given username
+     * may be null. Both {@link #cachedRecordsByUsername} and
+     * {@link #cachedAmbiguousUsernames} are updated appropriately. The write
+     * lock of {@link #cacheLock} must already be acquired before invoking this
+     * function.
+     *
+     * @param record
+     *     The record to associate with the given username.
+     *
+     * @param username
+     *     The username that the given record should be associated with. This
+     *     may be null.
+     */
+    private void addRecordForLogin(KeeperRecord record, String username) {
+
+        if (username == null)
+            return;
+
+        KeeperRecord existing = cachedRecordsByUsername.putIfAbsent(username, record);
+        if (existing != null && record != existing)
+            cachedAmbiguousUsernames.add(username);
+
+    }
+
+    /**
+     * Returns all records accessible via Keeper Secrets Manager. The records
+     * returned are arbitrarily ordered.
+     *
+     * @return
+     *     An unmodifiable Collection of all records accessible via Keeper
+     *     Secrets Manager,  in no particular order.
+     *
+     * @throws GuacamoleException
+     *     If an error occurs that prevents records from being retrieved.
+     */
+    public Collection<KeeperRecord> getRecords() throws GuacamoleException {
+        validateCache();
+        cacheLock.readLock().lock();
+        try {
+            return Collections.unmodifiableCollection(cachedRecordsByUid.values());
+        }
+        finally {
+            cacheLock.readLock().unlock();
+        }
+    }
+
+    /**
+     * Returns the record having the given UID. If no such record exists, null
+     * is returned.
+     *
+     * @param uid
+     *     The UID of the record to return.
+     *
+     * @return
+     *     The record having the given UID, or null if there is no such record.
+     *
+     * @throws GuacamoleException
+     *     If an error occurs that prevents the record from being retrieved.
+     */
+    public KeeperRecord getRecord(String uid) throws GuacamoleException {
+        validateCache();
+        cacheLock.readLock().lock();
+        try {
+            return cachedRecordsByUid.get(uid);
+        }
+        finally {
+            cacheLock.readLock().unlock();
+        }
+    }
+
+    /**
+     * Returns the record associated with the given hostname or IP address. If
+     * no such record exists, or there are multiple such records, null is
+     * returned.
+     *
+     * @param hostname
+     *     The hostname of the record to return.
+     *
+     * @return
+     *     The record associated with the given hostname, or null if there is
+     *     no such record or multiple such records.
+     *
+     * @throws GuacamoleException
+     *     If an error occurs that prevents the record from being retrieved.
+     */
+    public KeeperRecord getRecordByHost(String hostname) throws GuacamoleException {
+        validateCache();
+        cacheLock.readLock().lock();
+        try {
+
+            if (cachedAmbiguousHosts.contains(hostname)) {
+                logger.debug("The hostname/address \"{}\" is referenced by "
+                        + "multiple Keeper records and cannot be used to "
+                        + "locate individual secrets.", hostname);
+                return null;
+            }
+
+            return cachedRecordsByHost.get(hostname);
+
+        }
+        finally {
+            cacheLock.readLock().unlock();
+        }
+    }
+
+    /**
+     * Returns the record associated with the given username. If no such record
+     * exists, or there are multiple such records, null is returned.
+     *
+     * @param username
+     *     The username of the record to return.
+     *
+     * @return
+     *     The record associated with the given username, or null if there is
+     *     no such record or multiple such records.
+     *
+     * @throws GuacamoleException
+     *     If an error occurs that prevents the record from being retrieved.
+     */
+    public KeeperRecord getRecordByLogin(String username) throws GuacamoleException {
+        validateCache();
+        cacheLock.readLock().lock();
+        try {
+
+            if (cachedAmbiguousUsernames.contains(username)) {
+                logger.debug("The username \"{}\" is referenced by multiple "
+                        + "Keeper records and cannot be used to locate "
+                        + "individual secrets.", username);
+                return null;
+            }
+
+            return cachedRecordsByUsername.get(username);
+
+        }
+        finally {
+            cacheLock.readLock().unlock();
+        }
+    }
+
+    /**
+     * Returns the value of the secret stored within Keeper Secrets Manager and
+     * represented by the given Keeper notation. Keeper notation locates the
+     * value of a specific field, custom field, or file associated with a
+     * specific record. See: https://docs.keeper.io/secrets-manager/secrets-manager/about/keeper-notation
+     *
+     * @param notation
+     *     The Keeper notation of the secret to retrieve.
+     *
+     * @return
+     *     A Future which completes with the value of the secret represented by
+     *     the given Keeper notation, or null if there is no such secret.
+     *
+     * @throws GuacamoleException
+     *     If the requested secret cannot be retrieved or the Keeper notation
+     *     is invalid.
+     */
+    public Future<String> getSecret(String notation) throws GuacamoleException {
+        validateCache();
+        cacheLock.readLock().lock();
+        try {
+
+            // Retrieve any relevant file asynchronously
+            Matcher fileNotationMatcher = KEEPER_FILE_NOTATION.matcher(notation);
+            if (fileNotationMatcher.matches())
+                return recordService.download(Notation.getFile(cachedSecrets, notation));
+
+            // Retrieve string values synchronously
+            return CompletableFuture.completedFuture(Notation.getValue(cachedSecrets, notation));
+
+        }
+
+        // Unfortunately, the notation parser within the Keeper SDK throws
+        // plain Errors for retrieval failures ...
+        catch (Error e) {
+            logger.warn("Record \"{}\" does not exist.", notation);
+            logger.debug("Retrieval of record by Keeper notation failed.", e);
+            return CompletableFuture.completedFuture(null);
+        }
+
+        // ... and plain Exceptions for parse failures (no subclasses)
+        catch (Exception e) {
+            logger.warn("\"{}\" is not valid Keeper notation. Please check "
+                    + "the documentation at {} for valid formatting.",
+                    notation, KEEPER_NOTATION_DOC_URL);
+            logger.debug("Provided Keeper notation could not be parsed.", e);
+            return CompletableFuture.completedFuture(null);
+        }
+        finally {
+            cacheLock.readLock().unlock();
+        }
+
+    }
+
+}

extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmCacheFactory.java

@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.vault.ksm.secret;
+
+import javax.annotation.Nonnull;
+
+import com.keepersecurity.secretsManager.core.SecretsManagerOptions;
+
+/**
+ * Factory for creating KsmCache instances.
+ */
+public interface KsmCacheFactory {
+
+    /**
+     * Returns a new instance of a KsmCache instance associated with
+     * the provided KSM configuration options.
+     *
+     * @param ksmConfigOptions
+     *     The KSM config options to use when constructing the KsmCache
+     *     object.
+     *
+     * @return
+     *     A new KsmCache instance associated with the provided KSM config
+     *     options.
+     */
+    KsmCache create(@Nonnull SecretsManagerOptions ksmConfigOptions);
+
+}

extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java

@@ -21,29 +21,18 @@ package org.apache.guacamole.vault.ksm.secret;
 
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
-import com.keepersecurity.secretsManager.core.Hosts;
 import com.keepersecurity.secretsManager.core.KeeperRecord;
-import com.keepersecurity.secretsManager.core.KeeperSecrets;
+import com.keepersecurity.secretsManager.core.SecretsManagerOptions;
-import com.keepersecurity.secretsManager.core.Login;
+
-import com.keepersecurity.secretsManager.core.Notation;
-import com.keepersecurity.secretsManager.core.SecretsManager;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Future;
-import java.util.concurrent.locks.ReadWriteLock;
+
-import java.util.concurrent.locks.ReentrantReadWriteLock;
+import javax.annotation.Nullable;
-import java.util.regex.Matcher;
+
-import java.util.regex.Pattern;
 import org.apache.guacamole.GuacamoleException;
 import org.apache.guacamole.vault.ksm.conf.KsmConfigurationService;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Client which retrieves records from Keeper Secrets Manager, allowing
@@ -56,11 +45,6 @@ import org.slf4j.LoggerFactory;
 @Singleton
 public class KsmClient {
 
-    /**
-     * Logger for this class.
-     */
-    private static final Logger logger = LoggerFactory.getLogger(KsmClient.class);
-
     /**
      * Service for retrieving configuration information.
      */
@@ -68,268 +52,78 @@ public class KsmClient {
     private KsmConfigurationService confService;
 
     /**
-     * Service for retrieving data from records.
+     * Factory for creating KSM cache instances for particular KSM configs.
      */
     @Inject
-    private KsmRecordService recordService;
+    private KsmCacheFactory ksmCacheFactory;
 
     /**
-     * The publicly-accessible URL for Keeper's documentation covering Keeper
+     * A map of base-64 encoded JSON KSM config blobs to associated KSM cache instances.
-     * notation.
+     * The `null` entry in this Map is associated with the KSM configuration parsed
+     * from the guacamole.properties config file.
      */
-    private static final String KEEPER_NOTATION_DOC_URL =
+    private final Map<String, KsmCache> ksmCacheMap = new HashMap<>();
-            "https://docs.keeper.io/secrets-manager/secrets-manager/about/keeper-notation";
 
     /**
-     * The regular expression that Keeper notation must match to be related to
+     * Create and return a KSM cache for the provided KSM config if not already
-     * file retrieval. As the Keeper SDK provides mutually-exclusive for
+     * present in the cache map, the existing cache entry.
-     * retrieving secret values and files via notation, the notation must first
+     *
-     * be tested to determine whether it refers to a file.
+     * @param ksmConfig
-     */
+     *     The base-64 encoded JSON KSM config blob associated with the cache entry.
-    private static final Pattern KEEPER_FILE_NOTATION = Pattern.compile("^(keeper://)?[^/]*/file/.+");
+     *     If an associated entry does not already exist, it will be created using
-
+     *     this configuration.
-    /**
+     *
-     * The maximum amount of time that an entry will be stored in the cache
+     * @return
-     * before being refreshed, in milliseconds.
+     *     A KSM cache for the provided KSM config if not already present in the
-     */
+     *     cache map, otherwise the existing cache entry.
-    private static final long CACHE_INTERVAL = 5000;
-
-    /**
-     * Read/write lock which guards access to all cached data, including the
-     * timestamp recording the last time the cache was refreshed. Readers of
-     * the cache must first acquire (and eventually release) the read lock, and
-     * writers of the cache must first acquire (and eventually release) the
-     * write lock.
-     */
-    private final ReadWriteLock cacheLock = new ReentrantReadWriteLock();
-
-    /**
-     * The timestamp that the cache was last refreshed, in milliseconds, as
-     * returned by System.currentTimeMillis(). This value is automatically
-     * updated if {@link #validateCache()} refreshes the cache. This value must
-     * not be accessed without {@link #cacheLock} acquired appropriately.
-     */
-    private volatile long cacheTimestamp = 0;
-
-    /**
-     * The full cached set of secrets last retrieved from Keeper Secrets
-     * Manager. This value is automatically updated if {@link #validateCache()}
-     * refreshes the cache. This value must not be accessed without
-     * {@link #cacheLock} acquired appropriately.
-     */
-    private KeeperSecrets cachedSecrets = null;
-
-    /**
-     * All records retrieved from Keeper Secrets Manager, where each key is the
-     * UID of the corresponding record. The contents of this Map are
-     * automatically updated if {@link #validateCache()} refreshes the cache.
-     * This Map must not be accessed without {@link #cacheLock} acquired
-     * appropriately.
-     */
-    private final Map<String, KeeperRecord> cachedRecordsByUid = new HashMap<>();
-
-    /**
-     * All records retrieved from Keeper Secrets Manager, where each key is the
-     * hostname or IP address of the corresponding record. The hostname or IP
-     * address of a record is determined by {@link Hosts} fields, thus a record
-     * may be associated with multiple hosts. If a record is associated with
-     * multiple hosts, there will be multiple references to that record within
-     * this Map. The contents of this Map are automatically updated if
-     * {@link #validateCache()} refreshes the cache. This Map must not be
-     * accessed without {@link #cacheLock} acquired appropriately. Before using
-     * a value from this Map, {@link #cachedAmbiguousHosts} must first be
-     * checked to verify that there is indeed only one record associated with
-     * that host.
-     */
-    private final Map<String, KeeperRecord> cachedRecordsByHost = new HashMap<>();
-
-    /**
-     * The set of all hostnames or IP addresses that are associated with
-     * multiple records, and thus cannot uniquely identify a record. The
-     * contents of this Set are automatically updated if
-     * {@link #validateCache()} refreshes the cache. This Set must not be
-     * accessed without {@link #cacheLock} acquired appropriately.This Set
-     * must be checked before using a value retrieved from
-     * {@link #cachedRecordsByHost}.
-     */
-    private final Set<String> cachedAmbiguousHosts = new HashSet<>();
-
-    /**
-     * All records retrieved from Keeper Secrets Manager, where each key is the
-     * username of the corresponding record. The username of a record is
-     * determined by {@link Login} fields, thus a record may be associated with
-     * multiple users. If a record is associated with multiple users, there
-     * will be multiple references to that record within this Map. The contents
-     * of this Map are automatically updated if {@link #validateCache()}
-     * refreshes the cache. This Map must not be accessed without
-     * {@link #cacheLock} acquired appropriately. Before using a value from
-     * this Map, {@link #cachedAmbiguousUsernames} must first be checked to
-     * verify that there is indeed only one record associated with that user.
-     */
-    private final Map<String, KeeperRecord> cachedRecordsByUsername = new HashMap<>();
-
-    /**
-     * The set of all usernames that are associated with multiple records, and
-     * thus cannot uniquely identify a record. The contents of this Set are
-     * automatically updated if {@link #validateCache()} refreshes the cache.
-     * This Set must not be accessed without {@link #cacheLock} acquired
-     * appropriately.This Set must be checked before using a value retrieved
-     * from {@link #cachedRecordsByUsername}.
-     */
-    private final Set<String> cachedAmbiguousUsernames = new HashSet<>();
-
-    /**
-     * Validates that all cached data is current with respect to
-     * {@link #CACHE_INTERVAL}, refreshing data from the server as needed.
      *
      * @throws GuacamoleException
-     *     If an error occurs preventing the cached data from being refreshed.
+     *     If an error occurs while creating the KSM cache.
      */
-    private void validateCache() throws GuacamoleException {
+    private KsmCache createCacheIfNeeded(@Nullable String ksmConfig)
+            throws GuacamoleException {
 
-        long currentTime = System.currentTimeMillis();
+        // If a cache already exists for the provided config, use it
-
+        KsmCache ksmCache = ksmCacheMap.get(ksmConfig);
-        // Perform a read-only check that the cache has actually expired before
+        if (ksmCache != null)
-        // continuing
+            return ksmCache;
-        cacheLock.readLock().lock();
-        try {
-            if (currentTime - cacheTimestamp < CACHE_INTERVAL)
-                return;
-        }
-        finally {
-            cacheLock.readLock().unlock();
-        }
-
-        cacheLock.writeLock().lock();
-        try {
-
-            // Cache may have been updated since the read-only check. Re-verify
-            // that the cache has expired before continuing with a full refresh
-            if (currentTime - cacheTimestamp < CACHE_INTERVAL)
-                return;
-
-            // Attempt to pull all records first, allowing that operation to
-            // succeed/fail BEFORE we clear out the last cached success
-            KeeperSecrets secrets = SecretsManager.getSecrets(confService.getSecretsManagerOptions());
-            List<KeeperRecord> records = secrets.getRecords();
-
-            // Store all secrets within cache
-            cachedSecrets = secrets;
-
-            // Clear unambiguous cache of all records by UID
-            cachedRecordsByUid.clear();
-
-            // Clear cache of host-based records
-            cachedAmbiguousHosts.clear();
-            cachedRecordsByHost.clear();
-
-            // Clear cache of login-based records
-            cachedAmbiguousUsernames.clear();
-            cachedRecordsByUsername.clear();
-
-            // Store all records, sorting each into host-based and login-based
-            // buckets
-            records.forEach(record -> {
-
-                // Store based on UID ...
-                cachedRecordsByUid.put(record.getRecordUid(), record);
-
-                // ... and hostname/address
-                String hostname = recordService.getHostname(record);
-                addRecordForHost(record, hostname);
-
-                // Store based on username ONLY if no hostname (will otherwise
-                // result in ambiguous entries for servers tied to identical
-                // accounts)
-                if (hostname == null)
-                    addRecordForLogin(record, recordService.getUsername(record));
-
-            });
-
-            // Cache has been refreshed
-            this.cacheTimestamp = System.currentTimeMillis();
-
-        }
-        finally {
-            cacheLock.writeLock().unlock();
-        }
 
+        // Create and store a new KSM cache instance for the provided KSM config blob
+        SecretsManagerOptions options = confService.getSecretsManagerOptions(ksmConfig);
+        return ksmCacheMap.put(ksmConfig, ksmCacheFactory.create(options));
     }
 
     /**
-     * Associates the given record with the given hostname. The hostname may be
+     * Returns all records accessible via Keeper Secrets Manager, associated
-     * null. Both {@link #cachedRecordsByHost} and {@link #cachedAmbiguousHosts}
+     * with the provided KSM config. If no KSM config is provided, records
-     * are updated appropriately. The write lock of {@link #cacheLock} must
+     * associated with the default configuration as retrieved from the config
-     * already be acquired before invoking this function.
+     * file will be used. The records returned are arbitrarily ordered.
      *
-     * @param record
+     * @param ksmConfig
-     *     The record to associate with the hosts in the given field.
+     *     The base-64 encoded JSON KSM config blob associated associated with
-     *
+     *     the KSM vault that should be used to fetch the records.
-     * @param hostname
-     *     The hostname/address that the given record should be associated
-     *     with. This may be null.
-     */
-    private void addRecordForHost(KeeperRecord record, String hostname) {
-
-        if (hostname == null)
-            return;
-
-        KeeperRecord existing = cachedRecordsByHost.putIfAbsent(hostname, record);
-        if (existing != null && record != existing)
-            cachedAmbiguousHosts.add(hostname);
-
-    }
-
-    /**
-     * Associates the given record with the given username. The given username
-     * may be null. Both {@link #cachedRecordsByUsername} and
-     * {@link #cachedAmbiguousUsernames} are updated appropriately. The write
-     * lock of {@link #cacheLock} must already be acquired before invoking this
-     * function.
-     *
-     * @param record
-     *     The record to associate with the given username.
-     *
-     * @param username
-     *     The username that the given record should be associated with. This
-     *     may be null.
-     */
-    private void addRecordForLogin(KeeperRecord record, String username) {
-
-        if (username == null)
-            return;
-
-        KeeperRecord existing = cachedRecordsByUsername.putIfAbsent(username, record);
-        if (existing != null && record != existing)
-            cachedAmbiguousUsernames.add(username);
-
-    }
-
-    /**
-     * Returns all records accessible via Keeper Secrets Manager. The records
-     * returned are arbitrarily ordered.
      *
      * @return
      *     An unmodifiable Collection of all records accessible via Keeper
-     *     Secrets Manager,  in no particular order.
+     *     Secrets Manager, in no particular order.
      *
      * @throws GuacamoleException
      *     If an error occurs that prevents records from being retrieved.
      */
-    public Collection<KeeperRecord> getRecords() throws GuacamoleException {
+    public Collection<KeeperRecord> getRecords(
-        validateCache();
+                @Nullable String ksmConfig) throws GuacamoleException {
-        cacheLock.readLock().lock();
+
-        try {
+        // Call through to the associated KSM cache instance
-            return Collections.unmodifiableCollection(cachedRecordsByUid.values());
+        return createCacheIfNeeded(ksmConfig).getRecords();
-        }
-        finally {
-            cacheLock.readLock().unlock();
-        }
     }
 
     /**
-     * Returns the record having the given UID. If no such record exists, null
+     * Returns the record having the given KSM config, and the given UID.
-     * is returned.
+     * If no such record exists, null is returned.
+     *
+     * @param ksmConfig
+     *     The base-64 encoded JSON KSM config blob associated associated with
+     *     the KSM vault that should be used to fetch the record.
      *
      * @param uid
      *     The UID of the record to return.
@@ -340,21 +134,21 @@ public class KsmClient {
      * @throws GuacamoleException
      *     If an error occurs that prevents the record from being retrieved.
      */
-    public KeeperRecord getRecord(String uid) throws GuacamoleException {
+    public KeeperRecord getRecord(
-        validateCache();
+                @Nullable String ksmConfig, String uid) throws GuacamoleException {
-        cacheLock.readLock().lock();
+
-        try {
+        // Call through to the associated KSM cache instance
-            return cachedRecordsByUid.get(uid);
+        return createCacheIfNeeded(ksmConfig).getRecord(uid);
-        }
-        finally {
-            cacheLock.readLock().unlock();
-        }
     }
 
     /**
-     * Returns the record associated with the given hostname or IP address. If
+     * Returns the record associated with the given hostname or IP address
-     * no such record exists, or there are multiple such records, null is
+     * and the given KSM config. If no such record exists, or there are multiple
-     * returned.
+     * such records, null is returned.
+     *
+     * @param ksmConfig
+     *     The base-64 encoded JSON KSM config blob associated associated with
+     *     the KSM vault that should be used to fetch the record.
      *
      * @param hostname
      *     The hostname of the record to return.
@@ -366,30 +160,22 @@ public class KsmClient {
      * @throws GuacamoleException
      *     If an error occurs that prevents the record from being retrieved.
      */
-    public KeeperRecord getRecordByHost(String hostname) throws GuacamoleException {
+    public KeeperRecord getRecordByHost(
-        validateCache();
+                @Nullable String ksmConfig, String hostname) throws GuacamoleException {
-        cacheLock.readLock().lock();
-        try {
 
-            if (cachedAmbiguousHosts.contains(hostname)) {
+        // Call through to the associated KSM cache instance
-                logger.debug("The hostname/address \"{}\" is referenced by "
+        return createCacheIfNeeded(ksmConfig).getRecordByHost(hostname);
-                        + "multiple Keeper records and cannot be used to "
-                        + "locate individual secrets.", hostname);
-                return null;
-            }
 
-            return cachedRecordsByHost.get(hostname);
-
-        }
-        finally {
-            cacheLock.readLock().unlock();
-        }
     }
 
     /**
      * Returns the record associated with the given username. If no such record
      * exists, or there are multiple such records, null is returned.
      *
+     * @param ksmConfig
+     *     The base-64 encoded JSON KSM config blob associated associated with
+     *     the KSM vault that should be used to fetch the record.
+     *
      * @param username
      *     The username of the record to return.
      *
@@ -400,31 +186,24 @@ public class KsmClient {
      * @throws GuacamoleException
      *     If an error occurs that prevents the record from being retrieved.
      */
-    public KeeperRecord getRecordByLogin(String username) throws GuacamoleException {
+    public KeeperRecord getRecordByLogin(
-        validateCache();
+                @Nullable String ksmConfig, String username) throws GuacamoleException {
-        cacheLock.readLock().lock();
-        try {
 
-            if (cachedAmbiguousUsernames.contains(username)) {
+        // Call through to the associated KSM cache instance
-                logger.debug("The username \"{}\" is referenced by multiple "
+        return createCacheIfNeeded(ksmConfig).getRecordByLogin(username);
-                        + "Keeper records and cannot be used to locate "
-                        + "individual secrets.", username);
-                return null;
-            }
 
-            return cachedRecordsByUsername.get(username);
-
-        }
-        finally {
-            cacheLock.readLock().unlock();
-        }
     }
 
     /**
-     * Returns the value of the secret stored within Keeper Secrets Manager and
+     * Returns the value of the secret stored within the Keeper Secrets Manager vault
-     * represented by the given Keeper notation. Keeper notation locates the
+     * associated with the provided KSM config, and represented by the given Keeper
-     * value of a specific field, custom field, or file associated with a
+     * notation. Keeper notation locates the value of a specific field, custom field,
-     * specific record. See: https://docs.keeper.io/secrets-manager/secrets-manager/about/keeper-notation
+     * or file associated with a specific record.
+     * See: https://docs.keeper.io/secrets-manager/secrets-manager/about/keeper-notation
+     *
+     * @param ksmConfig
+     *     The base-64 encoded JSON KSM config blob associated associated with
+     *     the KSM vault that should be used to fetch the record.
      *
      * @param notation
      *     The Keeper notation of the secret to retrieve.
@@ -437,40 +216,11 @@ public class KsmClient {
      *     If the requested secret cannot be retrieved or the Keeper notation
      *     is invalid.
      */
-    public Future<String> getSecret(String notation) throws GuacamoleException {
+    public Future<String> getSecret(
-        validateCache();
+        @Nullable String ksmConfig, String notation) throws GuacamoleException {
-        cacheLock.readLock().lock();
-        try {
 
-            // Retrieve any relevant file asynchronously
+        // Call through to the associated KSM cache instance
-            Matcher fileNotationMatcher = KEEPER_FILE_NOTATION.matcher(notation);
+        return createCacheIfNeeded(ksmConfig).getSecret(notation);
-            if (fileNotationMatcher.matches())
-                return recordService.download(Notation.getFile(cachedSecrets, notation));
-
-            // Retrieve string values synchronously
-            return CompletableFuture.completedFuture(Notation.getValue(cachedSecrets, notation));
-
-        }
-
-        // Unfortunately, the notation parser within the Keeper SDK throws
-        // plain Errors for retrieval failures ...
-        catch (Error e) {
-            logger.warn("Record \"{}\" does not exist.", notation);
-            logger.debug("Retrieval of record by Keeper notation failed.", e);
-            return CompletableFuture.completedFuture(null);
-        }
-
-        // ... and plain Exceptions for parse failures (no subclasses)
-        catch (Exception e) {
-            logger.warn("\"{}\" is not valid Keeper notation. Please check "
-                    + "the documentation at {} for valid formatting.",
-                    notation, KEEPER_NOTATION_DOC_URL);
-            logger.debug("Provided Keeper notation could not be parsed.", e);
-            return CompletableFuture.completedFuture(null);
-        }
-        finally {
-            cacheLock.readLock().unlock();
-        }
 
     }