GUCAMOLE-1656: Do not expose the KSM config blob through the REST API.

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

@@ -24,7 +24,10 @@ import org.apache.guacamole.vault.VaultAuthenticationProviderModule;
 import org.apache.guacamole.vault.ksm.conf.KsmAttributeService;
 import org.apache.guacamole.vault.ksm.conf.KsmConfigurationService;
 import org.apache.guacamole.vault.ksm.secret.KsmSecretService;
+import org.apache.guacamole.vault.ksm.user.KsmConnectionGroup;
 import org.apache.guacamole.vault.ksm.user.KsmDirectoryService;
+import org.apache.guacamole.vault.ksm.user.KsmUserFactory;
+import org.apache.guacamole.vault.ksm.user.KsmUser;
 import org.apache.guacamole.vault.conf.VaultAttributeService;
 import org.apache.guacamole.vault.conf.VaultConfigurationService;
 import org.apache.guacamole.vault.ksm.secret.KsmClient;
@@ -67,6 +70,11 @@ public class KsmAuthenticationProviderModule
         install(new FactoryModuleBuilder()
                 .implement(KsmClient.class, KsmClient.class)
                 .build(KsmClientFactory.class));
+
+        // Bind factory for creating KsmUsers
+        install(new FactoryModuleBuilder()
+                .implement(KsmUser.class, KsmUser.class)
+                .build(KsmUserFactory.class));
     }
 
 }

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

@@ -19,20 +19,30 @@
 
 package org.apache.guacamole.vault.ksm.conf;
 
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
+import java.util.Base64;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 
 import org.apache.guacamole.GuacamoleException;
 import org.apache.guacamole.form.BooleanField;
 import org.apache.guacamole.form.Form;
 import org.apache.guacamole.form.TextField;
+import org.apache.guacamole.language.TranslatableGuacamoleClientException;
 import org.apache.guacamole.vault.conf.VaultAttributeService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
+import com.keepersecurity.secretsManager.core.InMemoryStorage;
+import com.keepersecurity.secretsManager.core.SecretsManager;
+import com.keepersecurity.secretsManager.core.SecretsManagerOptions;
 
 /**
  * A service that exposes KSM-specific attributes, allowing setting KSM
@@ -52,12 +62,41 @@ public class KsmAttributeService implements VaultAttributeService {
     @Inject
     private KsmConfigurationService configurationService;
 
+    /**
+     * A singleton ObjectMapper for converting a Map to a JSON string when
+     * generating a base64-encoded JSON KSM config blob.
+     */
+    private static final ObjectMapper objectMapper = new ObjectMapper();
+
+    /**
+     * All expected fields in the KSM configuration JSON blob.
+     */
+    private static final List<String> EXPECTED_KSM_FIELDS = (
+            Collections.unmodifiableList(Arrays.asList(
+                    SecretsManager.KEY_HOSTNAME,
+                    SecretsManager.KEY_CLIENT_ID,
+                    SecretsManager.KEY_PRIVATE_KEY,
+                    SecretsManager.KEY_CLIENT_KEY,
+                    SecretsManager.KEY_APP_KEY,
+                    SecretsManager.KEY_OWNER_PUBLIC_KEY,
+                    SecretsManager.KEY_SERVER_PUBIC_KEY_ID
+    )));
+
     /**
      * The name of the attribute which can contain a KSM configuration blob
      * associated with either a connection group or user.
      */
     public static final String KSM_CONFIGURATION_ATTRIBUTE = "ksm-config";
 
+    /**
+     * The KSM configuration attribute contains sensitive information, so it
+     * should not be exposed through the directory. Instead, if a value is
+     * set on the attributes of an object, the following value will be exposed
+     * in its place, and correspondingly the underlying value will not be
+     * changed if this value is provided to an update call.
+     */
+    public static final String KSM_ATTRIBUTE_PLACEHOLDER_VALUE = "**********";
+
     /**
      * All attributes related to configuring the KSM vault on a
      * per-connection-group or per-user basis.
@@ -133,5 +172,181 @@ public class KsmAttributeService implements VaultAttributeService {
         }
     }
 
+    /**
+     * Sanitize the value of the provided KSM config attribute. If the provided
+     * config value is non-empty, it will be replaced with the placeholder
+     * value to avoid leaking sensitive information. If the value is empty, it
+     * will be replaced by `null`.
+     *
+     * @param ksmAttributeValue
+     *    The KSM configuration attribute value to sanitize.
+     *
+     * @return
+     *    The sanitized KSM configuration attribute value, stripped of any
+     *    sensitive information.
+     */
+    public static String sanitizeKsmAttributeValue(String ksmAttributeValue) {
+
+        // Any non-empty values may contain sensitive information, and should
+        // be replaced by the safe placeholder value
+        if (ksmAttributeValue != null && !ksmAttributeValue.trim().isEmpty())
+            return KSM_ATTRIBUTE_PLACEHOLDER_VALUE;
+
+        // If the configuration value is empty, expose a null value
+        else
+            return null;
+
+    }
+
+    /**
+     * Return true if the provided input is a valid base64-encoded string,
+     * false otherwise.
+     *
+     * @param input
+     *     The string to check if base-64 encoded.
+     *
+     * @return
+     *     true if the provided input is a valid base64-encoded string,
+     *     false otherwise.
+     */
+    private static boolean isBase64(String input) {
+
+        try {
+            Base64.getDecoder().decode(input);
+            return true;
+        } catch (IllegalArgumentException e) {
+            return false;
+        }
+    }
+
+    /**
+     * Given a map of attribute values, check for the presence of the
+     * KSM_CONFIGURATION_ATTRIBUTE attribute. If it's set, check if it's a valid
+     * KSM one-time token. If so, attempt to translate it to a base-64-encoded
+     * json KSM config blob. If it's already a KSM config blob, validate it as
+     * config blob. If either validation fails, a GuacamoleException will be thrown.
+     * The processed attribute values will be returned.
+     *
+     * @param attributes
+     *     The attributes for which the KSM configuration attribute
+     *     parsing/validation should be performed.
+     *
+     * @throws GuacamoleException
+     *     If the KSM_CONFIGURATION_ATTRIBUTE is set, but fails to validate as
+     *     either a KSM one-time-token, or a KSM base64-encoded JSON config blob.
+     */
+    public Map<String, String> processAttributes(
+            Map<String, String> attributes) throws GuacamoleException {
+
+        // Get the value of the KSM config attribute in the provided map
+        String ksmConfigValue = attributes.get(
+                KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE);
+
+        // If the placeholder value was provided, do not update the attribute
+        if (KsmAttributeService.KSM_ATTRIBUTE_PLACEHOLDER_VALUE.equals(ksmConfigValue)) {
+
+            // Remove the attribute from the map so it won't be updated
+            attributes = new HashMap<>(attributes);
+            attributes.remove(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE);
+
+        }
+
+        // Check if the attribute is set to a non-empty value
+        else if (ksmConfigValue != null && !ksmConfigValue.trim().isEmpty()) {
+
+            // If it's already base64-encoded, it's a KSM configuration blob,
+            // so validate it immediately
+            if (isBase64(ksmConfigValue)) {
+
+                // Attempt to validate the config as a base64-econded KSM config blob
+                try {
+                    KsmConfig.parseKsmConfig(ksmConfigValue);
+
+                    // If it validates, the entity can be left alone - it's already valid
+                    return attributes;
+                }
+
+                catch (GuacamoleException exception) {
+
+                    // If the parsing attempt fails, throw a translatable error for display
+                    // on the frontend
+                    throw new TranslatableGuacamoleClientException(
+                            "Invalid base64-encoded JSON KSM config provided for "
+                            + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE + " attribute",
+                            "CONNECTION_GROUP_ATTRIBUTES.ERROR_INVALID_KSM_CONFIG_BLOB",
+                            exception);
+                }
+            }
+
+            // It wasn't a valid base64-encoded string, it should be a one-time token, so
+            // attempt to validat it as such, and if valid, update the attribute to the
+            // base64 config blob generated by the token
+            try {
+
+                // Create an initially empty storage to be populated using the one-time token
+                InMemoryStorage storage = new InMemoryStorage();
+
+                // Populate the in-memory storage using the one-time-token
+                SecretsManager.initializeStorage(storage, ksmConfigValue, null);
+
+                // Create an options object using the values we extracted from the one-time token
+                SecretsManagerOptions options = new SecretsManagerOptions(
+                    storage, null,
+                    configurationService.getAllowUnverifiedCertificate());
+
+                // Attempt to fetch secrets using the options we created. This will both validate
+                // that the configuration works, and potentially populate missing fields that the
+                // initializeStorage() call did not set.
+                SecretsManager.getSecrets(options);
+
+                // Create a map to store the extracted values from the KSM storage
+                Map<String, String> configMap = new HashMap<>();
+
+                // Go through all the expected fields, extract from the KSM storage,
+                // and write to the newly created map
+                EXPECTED_KSM_FIELDS.forEach(configKey -> {
+
+                    // Only write the value into the new map if non-null
+                    String value = storage.getString(configKey);
+                    if (value != null)
+                        configMap.put(configKey, value);
+
+                });
+
+                // JSON-encode the value, and then base64 encode that to get the format
+                // that KSM would expect
+                String jsonString = objectMapper.writeValueAsString(configMap);
+                String base64EncodedJson = Base64.getEncoder().encodeToString(
+                        jsonString.getBytes(StandardCharsets.UTF_8));
+
+                // Finally, try to parse the newly generated token as a KSM config. If this
+                // works, the config should be fully functional
+                KsmConfig.parseKsmConfig(base64EncodedJson);
+
+                // Make a copy of the existing attributes, modifying just the value for
+                // KSM_CONFIGURATION_ATTRIBUTE
+                attributes = new HashMap<>(attributes);
+                attributes.put(
+                        KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, base64EncodedJson);
+
+            }
+
+            // The KSM SDK only throws raw Exceptions, so we can't be more specific
+            catch (Exception exception) {
+
+                // If the parsing attempt fails, throw a translatable error for display
+                // on the frontend
+                throw new TranslatableGuacamoleClientException(
+                        "Invalid one-time KSM token provided for "
+                        + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE + " attribute",
+                        "CONNECTION_GROUP_ATTRIBUTES.ERROR_INVALID_KSM_ONE_TIME_TOKEN",
+                        exception);
+            }
+        }
+
+        return attributes;
+
+    }
+
 
 }

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

@@ -47,12 +47,14 @@ import org.apache.guacamole.net.auth.Connectable;
 import org.apache.guacamole.net.auth.Connection;
 import org.apache.guacamole.net.auth.ConnectionGroup;
 import org.apache.guacamole.net.auth.Directory;
+import org.apache.guacamole.net.auth.User;
 import org.apache.guacamole.net.auth.UserContext;
 import org.apache.guacamole.protocol.GuacamoleConfiguration;
 import org.apache.guacamole.token.TokenFilter;
 import org.apache.guacamole.vault.ksm.GuacamoleExceptionSupplier;
 import org.apache.guacamole.vault.ksm.conf.KsmAttributeService;
 import org.apache.guacamole.vault.ksm.conf.KsmConfigurationService;
+import org.apache.guacamole.vault.ksm.user.KsmDirectory;
 import org.apache.guacamole.vault.secret.VaultSecretService;
 import org.apache.guacamole.vault.secret.WindowsUsername;
 import org.slf4j.Logger;
@@ -299,7 +301,12 @@ public class KsmSecretService implements VaultSecretService {
         Set<String> observedIdentifiers = new HashSet<>();
         observedIdentifiers.add(parentIdentifier);
 
-        Directory<ConnectionGroup> connectionGroupDirectory = userContext.getConnectionGroupDirectory();
+        // Use the unwrapped connection group directory to avoid KSM config
+        // value sanitization
+        Directory<ConnectionGroup> connectionGroupDirectory = (
+                (KsmDirectory<ConnectionGroup>) userContext.getConnectionGroupDirectory()
+                ).getUnderlyingDirectory();
+
         while (true) {
 
             // Fetch the parent group, if one exists
@@ -378,10 +385,16 @@ public class KsmSecretService implements VaultSecretService {
 
         // If user KSM configs are enabled globally, and for the given connectable,
         // return the user-specific KSM config, if one exists
-        if (confService.getAllowUserConfig() && isKsmUserConfigEnabled(connectable))
+        if (confService.getAllowUserConfig() && isKsmUserConfigEnabled(connectable)) {
 
-            return userContext.self().getAttributes().get(
+            // Get the underlying user, to avoid the KSM config sanitization
+            User self = (
+                    ((KsmDirectory<User>) userContext.getUserDirectory())
+                    .getUnderlyingDirectory().get(userContext.self().getIdentifier()));
+
+            return self.getAttributes().get(
                     KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE);
+        }
 
 
         // If user-specific KSM config is disabled globally or for the given

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

@@ -62,10 +62,9 @@ public class KsmConnection extends DelegatingConnection {
         // Make a copy of the existing map
         Map<String, String> attributes = Maps.newHashMap(super.getAttributes());
 
-        // Add the configuration attribute
+        // Add the user-config-enabled configuration attribute
-        attributes.putIfAbsent(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, null);
+        attributes.putIfAbsent(KsmAttributeService.KSM_USER_CONFIG_ENABLED_ATTRIBUTE, null);
         return attributes;
-
     }
 
 }

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

@@ -30,34 +30,31 @@ import com.google.common.collect.Maps;
  /**
   * A KSM-specific connection group implementation that always exposes
   * the KSM_CONFIGURATION_ATTRIBUTE attribute, even when no value is set.
-  * This ensures that the attribute will always show up in the UI, even
+  * The value of the attribute will be sanitized if non-empty. This ensures
-  * for connection groups that don't already have it set.
+  * that the attribute will always show up in the UI, even for connection
+  * groups that don't already have it set, and that any sensitive information
+  * in the attribute value will not be exposed.
   */
  public class KsmConnectionGroup extends DelegatingConnectionGroup {
 
     /**
-     * Create a new KsmConnectionGroup instance, wrapping the provided
+     * Create a new KsmConnectionGroup wrapping the provided ConnectionGroup record.
-     * ConnectionGroup.
      *
      * @param connectionGroup
-     *     The ConnectionGroup instance to wrap.
+     *     The ConnectionGroup record to wrap.
      */
-    public KsmConnectionGroup(ConnectionGroup connectionGroup) {
+    KsmConnectionGroup(ConnectionGroup connectionGroup) {
-
-        // Wrap the provided connection group
         super(connectionGroup);
     }
 
-    @Override
+    /**
-    public Map<String, String> getAttributes() {
+     * Return the underlying wrapped connection group record.
-
+     *
-        // Make a copy of the existing map
+     * @return
-        Map<String, String> attributes = Maps.newHashMap(super.getAttributes());
+     *     The wrapped connection group record.
-
+     */
-        // Add the configuration attribute
+    ConnectionGroup getUnderlyingConnectionGroup() {
-        attributes.putIfAbsent(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, null);
+        return getDelegateConnectionGroup();
-        return attributes;
-
     }
 
     /**
@@ -70,4 +67,20 @@ import com.google.common.collect.Maps;
         return getDelegateConnectionGroup();
     }
 
+    @Override
+    public Map<String, String> getAttributes() {
+
+        // Make a copy of the existing map
+        Map<String, String> attributes = Maps.newHashMap(super.getAttributes());
+
+        // Sanitize the KSM configuration attribute, and ensure the attribute
+        // is always present
+        attributes.put(
+                KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE,
+                KsmAttributeService.sanitizeKsmAttributeValue(
+                    attributes.get(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE)));
+
+        return attributes;
+    }
+
  }

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

@@ -0,0 +1,93 @@
+/*
+ * 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.user;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.net.auth.DelegatingDirectory;
+import org.apache.guacamole.net.auth.Directory;
+import org.apache.guacamole.net.auth.Identifiable;
+
+/**
+ * A KSM-specific version of DecoratingDirectory that exposes the underlying
+ * directory for when it's needed.
+ */
+public abstract class KsmDirectory<ObjectType extends Identifiable>
+        extends DelegatingDirectory<ObjectType> {
+
+    /**
+     * Create a new KsmDirectory, delegating to the provided directory.
+     *
+     * @param directory
+     *    The directory to delegate to.
+     */
+    public KsmDirectory(Directory<ObjectType> directory) {
+        super(directory);
+    }
+
+    /**
+     * Returns the underlying directory that this DecoratingDirectory is
+     * delegating to.
+     *
+     * @return
+     *    The underlying directory.
+     */
+    public Directory<ObjectType> getUnderlyingDirectory() {
+        return getDelegateDirectory();
+    }
+
+    /**
+     * Process and return a potentially-modified version of the object
+     * with the same identifier in the wrapped directory.
+     *
+     * @param object
+     *     The object from the underlying directory.
+     *
+     * @return
+     *     A potentially-modified version of the object with the same
+     *     identifier in the wrapped directory.
+     */
+    protected abstract ObjectType wrap(ObjectType object);
+
+    @Override
+    public ObjectType get(String identifier) throws GuacamoleException {
+
+        // Process and return the object from the wrapped directory
+        return wrap(super.get(identifier));
+
+    }
+
+    @Override
+    public Collection<ObjectType> getAll(Collection<String> identifiers)
+            throws GuacamoleException {
+
+        // Process and return each object from the wrapped directory
+        return super.getAll(identifiers).stream().map(
+                superObject -> wrap(superObject)
+                ).collect(Collectors.toList());
+
+    }
+
+}

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

@@ -19,32 +19,17 @@
 
 package org.apache.guacamole.vault.ksm.user;
 
-import java.nio.charset.StandardCharsets;
-import java.util.Arrays;
-import java.util.Base64;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
 import org.apache.guacamole.GuacamoleException;
-import org.apache.guacamole.language.TranslatableGuacamoleClientException;
-import org.apache.guacamole.net.auth.Attributes;
 import org.apache.guacamole.net.auth.Connection;
 import org.apache.guacamole.net.auth.ConnectionGroup;
 import org.apache.guacamole.net.auth.DecoratingDirectory;
+import org.apache.guacamole.net.auth.DelegatingDirectory;
 import org.apache.guacamole.net.auth.Directory;
 import org.apache.guacamole.net.auth.User;
 import org.apache.guacamole.vault.ksm.conf.KsmAttributeService;
-import org.apache.guacamole.vault.ksm.conf.KsmConfig;
-import org.apache.guacamole.vault.ksm.conf.KsmConfigurationService;
 import org.apache.guacamole.vault.user.VaultDirectoryService;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.inject.Inject;
-import com.keepersecurity.secretsManager.core.InMemoryStorage;
-import com.keepersecurity.secretsManager.core.SecretsManager;
-import com.keepersecurity.secretsManager.core.SecretsManagerOptions;
 
 /**
  * A KSM-specific vault directory service that wraps the connection group directory
@@ -54,175 +39,17 @@ import com.keepersecurity.secretsManager.core.SecretsManagerOptions;
 public class KsmDirectoryService extends VaultDirectoryService {
 
     /**
-     * Service for retrieving KSM configuration details.
+     * A factory for constructing new KsmUser instances.
      */
     @Inject
-    private KsmConfigurationService configurationService;
+    private KsmUserFactory ksmUserFactory;
 
     /**
-     * A singleton ObjectMapper for converting a Map to a JSON string when
+     * Service for retrieving any custom attributes defined for the
-     * generating a base64-encoded JSON KSM config blob.
+     * current vault implementation and processing of said attributes.
      */
-    private static final ObjectMapper objectMapper = new ObjectMapper();
+    @Inject
-
+    private KsmAttributeService attributeService;
-    /**
-     * All expected fields in the KSM configuration JSON blob.
-     */
-    private static final List<String> EXPECTED_KSM_FIELDS = (
-            Collections.unmodifiableList(Arrays.asList(
-                    SecretsManager.KEY_HOSTNAME,
-                    SecretsManager.KEY_CLIENT_ID,
-                    SecretsManager.KEY_PRIVATE_KEY,
-                    SecretsManager.KEY_CLIENT_KEY,
-                    SecretsManager.KEY_APP_KEY,
-                    SecretsManager.KEY_OWNER_PUBLIC_KEY,
-                    SecretsManager.KEY_SERVER_PUBIC_KEY_ID
-    )));
-
-    /**
-     * Return true if the provided input is a valid base64-encoded string,
-     * false otherwise.
-     *
-     * @param input
-     *     The string to check if base-64 encoded.
-     *
-     * @return
-     *     true if the provided input is a valid base64-encoded string,
-     *     false otherwise.
-     */
-    private static boolean isBase64(String input) {
-
-        try {
-            Base64.getDecoder().decode(input);
-            return true;
-        } catch (IllegalArgumentException e) {
-            return false;
-        }
-    }
-
-    /**
-     * Given an attributes-enabled entity, check for the presence of the
-     * KSM_CONFIGURATION_ATTRIBUTE attribute. If it's set, check if it's a valid
-     * KSM one-time token. If so, attempt to translate it to a base-64-encoded
-     * json KSM config blob, and set it back to the provided entity.
-     * If it's already a KSM config blob, validate it as config blob. If either
-     * validation fails, a GuacamoleException will be thrown.
-     *
-     * @param entity
-     *     The attributes-enabled entity for which the KSM configuration
-     *     attribute parsing/validation should be performed.
-     *
-     * @throws GuacamoleException
-     *     If the KSM_CONFIGURATION_ATTRIBUTE is set, but fails to validate as
-     *     either a KSM one-time-token, or a KSM base64-encoded JSON config blob.
-     */
-    public void processAttributes(Attributes entity) throws GuacamoleException {
-
-        // By default, if the KSM config attribute isn't being set, pass the
-        // provided attributes through without any changes
-        Map<String, String> attributes = entity.getAttributes();
-
-        // Get the value of the KSM config attribute in the provided map
-        String ksmConfigValue = attributes.get(
-                KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE);
-
-        // Check if the attribute is set to a non-empty value
-        if (ksmConfigValue != null && !ksmConfigValue.trim().isEmpty()) {
-
-            // If it's already base64-encoded, it's a KSM configuration blob,
-            // so validate it immediately
-            if (isBase64(ksmConfigValue)) {
-
-                // Attempt to validate the config as a base64-econded KSM config blob
-                try {
-                    KsmConfig.parseKsmConfig(ksmConfigValue);
-
-                    // If it validates, the entity can be left alone - it's already valid
-                    return;
-                }
-
-                catch (GuacamoleException exception) {
-
-                    // If the parsing attempt fails, throw a translatable error for display
-                    // on the frontend
-                    throw new TranslatableGuacamoleClientException(
-                            "Invalid base64-encoded JSON KSM config provided for "
-                            + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE + " attribute",
-                            "CONNECTION_GROUP_ATTRIBUTES.ERROR_INVALID_KSM_CONFIG_BLOB",
-                            exception);
-                }
-            }
-
-            // It wasn't a valid base64-encoded string, it should be a one-time token, so
-            // attempt to validat it as such, and if valid, update the attribute to the
-            // base64 config blob generated by the token
-            try {
-
-                // Create an initially empty storage to be populated using the one-time token
-                InMemoryStorage storage = new InMemoryStorage();
-
-                // Populate the in-memory storage using the one-time-token
-                SecretsManager.initializeStorage(storage, ksmConfigValue, null);
-
-                // Create an options object using the values we extracted from the one-time token
-                SecretsManagerOptions options = new SecretsManagerOptions(
-                    storage, null,
-                    configurationService.getAllowUnverifiedCertificate());
-
-                // Attempt to fetch secrets using the options we created. This will both validate
-                // that the configuration works, and potentially populate missing fields that the
-                // initializeStorage() call did not set.
-                SecretsManager.getSecrets(options);
-
-                // Create a map to store the extracted values from the KSM storage
-                Map<String, String> configMap = new HashMap<>();
-
-                // Go through all the expected fields, extract from the KSM storage,
-                // and write to the newly created map
-                EXPECTED_KSM_FIELDS.forEach(configKey -> {
-
-                    // Only write the value into the new map if non-null
-                    String value = storage.getString(configKey);
-                    if (value != null)
-                        configMap.put(configKey, value);
-
-                });
-
-                // JSON-encode the value, and then base64 encode that to get the format
-                // that KSM would expect
-                String jsonString = objectMapper.writeValueAsString(configMap);
-                String base64EncodedJson = Base64.getEncoder().encodeToString(
-                        jsonString.getBytes(StandardCharsets.UTF_8));
-
-                // Finally, try to parse the newly generated token as a KSM config. If this
-                // works, the config should be fully functional
-                KsmConfig.parseKsmConfig(base64EncodedJson);
-
-                // Make a copy of the existing attributes, modifying just the value for
-                // KSM_CONFIGURATION_ATTRIBUTE
-                attributes = new HashMap<>(attributes);
-                attributes.put(
-                        KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, base64EncodedJson);
-
-                // Set the newly updated attributes back to the original object
-                entity.setAttributes(attributes);
-
-            }
-
-            // The KSM SDK only throws raw Exceptions, so we can't be more specific
-            catch (Exception exception) {
-
-                // If the parsing attempt fails, throw a translatable error for display
-                // on the frontend
-                throw new TranslatableGuacamoleClientException(
-                        "Invalid one-time KSM token provided for "
-                        + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE + " attribute",
-                        "CONNECTION_GROUP_ATTRIBUTES.ERROR_INVALID_KSM_ONE_TIME_TOKEN",
-                        exception);
-            }
-        }
-
-    }
 
     @Override
     public Directory<Connection> getConnectionDirectory(
@@ -259,40 +86,45 @@ public class KsmDirectoryService extends VaultDirectoryService {
         // validate KSM configurations, and translate one-time-tokens, if possible,
         // as well as ensuring that all ConnectionGroups returned include the
         // KSM_CONFIGURATION_ATTRIBUTE attribute, so it will be available in the UI.
-        return new DecoratingDirectory<ConnectionGroup>(underlyingDirectory) {
+        // The value of the KSM_CONFIGURATION_ATTRIBUTE will be sanitized if set.
+        return new KsmDirectory<ConnectionGroup>(underlyingDirectory) {
 
             @Override
             public void add(ConnectionGroup connectionGroup) throws GuacamoleException {
 
-                // Check for the KSM config attribute and translate the one-time token
+                // Process attribute values before saving
-                // if possible before adding
+                connectionGroup.setAttributes(
-                processAttributes(connectionGroup);
+                    attributeService.processAttributes(
+                        connectionGroup.getAttributes()));
+
                 super.add(connectionGroup);
             }
 
             @Override
             public void update(ConnectionGroup connectionGroup) throws GuacamoleException {
 
-                // Check for the KSM config attribute and translate the one-time token
+                // Unwrap the existing ConnectionGroup
-                // if possible before updating
+                if (connectionGroup instanceof KsmConnectionGroup)
-                processAttributes(connectionGroup);
+                    connectionGroup = ((KsmConnectionGroup) connectionGroup).getUnderlyingConnectionGroup();
+
+                // Process attribute values before saving
+                connectionGroup.setAttributes(
+                    attributeService.processAttributes(
+                        connectionGroup.getAttributes()));
+
                 super.update(connectionGroup);
-            }
-
-            @Override
-            protected ConnectionGroup decorate(ConnectionGroup connectionGroup) throws GuacamoleException {
-
-                // Wrap the existing connection group in a KsmConnection to ensure the presence of the
-                // KSM_CONFIGURATION_ATTRIBUTE attribute
-                return new KsmConnectionGroup(connectionGroup);
 
             }
 
             @Override
-            protected ConnectionGroup undecorate(ConnectionGroup connectionGroup) throws GuacamoleException {
+            protected ConnectionGroup wrap(ConnectionGroup object) {
 
-                // Return the underlying connection group that the KsmConnectionGroup wraps
+                // Do not process the ConnectionGroup further if it does not exist
-                return ((KsmConnectionGroup) connectionGroup).getUnderlyConnectionGroup();
+                if (object == null)
+                    return null;
+
+                // Sanitize values when a ConnectionGroup is fetched from the directory
+                return new KsmConnectionGroup(object);
 
             }
 
@@ -307,40 +139,47 @@ public class KsmDirectoryService extends VaultDirectoryService {
         // validate KSM configurations, and translate one-time-tokens, if possible
         // Additionally, this directory will will decorate all users with a
         // KsmUser wrapper to ensure that all defined KSM fields will be exposed
-        // in the user attributes.
+        // in the user attributes.  The value of the KSM_CONFIGURATION_ATTRIBUTE
-        return new DecoratingDirectory<User>(underlyingDirectory) {
+        // will be sanitized if set.
+        return new KsmDirectory<User>(underlyingDirectory) {
 
             @Override
             public void add(User user) throws GuacamoleException {
 
-                // Check for the KSM config attribute and translate the one-time token
+                // Process attribute values before saving
-                // if possible before adding
+                user.setAttributes(
-                processAttributes(user);
+                    attributeService.processAttributes(
+                        user.getAttributes()));
+
                 super.add(user);
             }
 
             @Override
             public void update(User user) throws GuacamoleException {
 
-                // Check for the KSM config attribute and translate the one-time token
+                // Unwrap the existing user
-                // if possible before updating
+                if (user instanceof KsmUser)
-                processAttributes(user);
+                    user = ((KsmUser) user).getUnderlyingUser();
+
+                // Process attribute values before saving
+                user.setAttributes(
+                    attributeService.processAttributes(
+                        user.getAttributes()));
+
                 super.update(user);
+
             }
 
             @Override
-            protected User decorate(User user) throws GuacamoleException {
+            protected User wrap(User object) {
 
-                // Wrap in a KsmUser class to ensure that all defined KSM fields will be
+                // Do not process the user further if it does not exist
-                // present
+                if (object == null)
-                return new KsmUser(user);
+                    return null;
-            }
 
-            @Override
+                // Sanitize values when a user is fetched from the directory
-            protected User undecorate(User user) throws GuacamoleException {
+                return ksmUserFactory.create(object);
 
-                // Unwrap the KsmUser
-                return ((KsmUser) user).getUnderlyingUser();
             }
 
         };

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

@@ -21,27 +21,49 @@ package org.apache.guacamole.vault.ksm.user;
 
 import java.util.Map;
 
-import org.apache.guacamole.net.auth.DelegatingUser;
+import org.apache.guacamole.GuacamoleException;
 import org.apache.guacamole.net.auth.User;
+import org.apache.guacamole.net.auth.DelegatingUser;
 import org.apache.guacamole.vault.ksm.conf.KsmAttributeService;
+import org.apache.guacamole.vault.ksm.conf.KsmConfigurationService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.google.common.collect.Maps;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
 
-/**
+ /**
- * A User that explicitly adds a blank entry for any defined
+  * A KSM-specific user implementation that exposes the
- * KSM user attributes.
+  * KSM_CONFIGURATION_ATTRIBUTE attribute even if no value is set. but only
- */
+  * if user-specific KSM configuration is enabled. The value of the attribute
-public class KsmUser extends DelegatingUser {
+  * will be sanitized if non-empty. This ensures that the attribute will always
+  * show up in the UI when the feature is enabled, even for users that don't
+  * already have it set, and that any sensitive information in the attribute
+  * value will not be exposed.
+  */
+ public class KsmUser extends DelegatingUser {
 
     /**
-     * Create a new Vault user wrapping the provided User record. Any
+     * Logger for this class.
-     * KSM-specific attribute forms will have empty values automatically
+     */
-     * populated when getAttributes() is called.
+    private static final Logger logger = LoggerFactory.getLogger(KsmUser.class);
+
+    /**
+     * Service for retrieving KSM configuration details.
+     */
+    @Inject
+    private KsmConfigurationService configurationService;
+
+    /**
+     * Create a new Ksmuser wrapping the provided User record.
      *
      * @param user
-     *     The user record to wrap.
+     *     The User record to wrap.
      */
-    KsmUser(User user) {
+    @AssistedInject
+    KsmUser(@Assisted User user) {
         super(user);
     }
 
@@ -61,9 +83,33 @@ public class KsmUser extends DelegatingUser {
         // Make a copy of the existing map
         Map<String, String> attributes = Maps.newHashMap(super.getAttributes());
 
-        // Add the configuration attribute
+        // Figure out if user-level KSM config is enabled
-        attributes.putIfAbsent(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, null);
+        boolean userKsmConfigEnabled = false;
+        try {
+            userKsmConfigEnabled = configurationService.getAllowUserConfig();
+        } catch (GuacamoleException e) {
+
+            logger.warn(
+                    "Disabling user KSM config due to exception: {}"
+                    , e.getMessage());
+            logger.debug("Error looking up if user KSM config is enabled.", e);
+
+        }
+
+        // If user-specific KSM configuration is not enabled, do not expose the
+        // attribute at all
+        if (!userKsmConfigEnabled)
+            attributes.remove(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE);
+
+        else
+            // Sanitize the KSM configuration attribute, and ensure the attribute
+            // is always present
+            attributes.put(
+                    KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE,
+                    KsmAttributeService.sanitizeKsmAttributeValue(
+                        attributes.get(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE)));
+
         return attributes;
     }
 
-}
+ }

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

@@ -0,0 +1,40 @@
+/*
+ * 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.user;
+
+import org.apache.guacamole.net.auth.User;
+
+/**
+ * Factory for creating KSM-specific users, which wrap an underlying User.
+ */
+public interface KsmUserFactory {
+
+    /**
+     * Returns a new instance of a KsmUser, wrapping the provided underlying User.
+     *
+     * @param user
+     *     The underlying User that should be wrapped.
+     *
+     * @return
+     *     A new instance of a KsmUser, wrapping the provided underlying User.
+     */
+    KsmUser create(User user);
+
+}