jmuehlner commented on code in PR #751:
URL: https://github.com/apache/guacamole-client/pull/751#discussion_r982699668


##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectory.java:
##########
@@ -0,0 +1,76 @@
+/*
+ * 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 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();
+    }
+
+    @Override
+    public Collection<ObjectType> getAll(Collection<String> identifiers) 
throws GuacamoleException {
+
+        // Create a list of objects to return
+        List<ObjectType> objects = new 
ArrayList<ObjectType>(identifiers.size());
+
+        // Populate the list by looking up objects one by one
+        Iterator<String> identifierIterator = identifiers.iterator();
+        while(identifierIterator.hasNext()) {
+            objects.add(get(identifierIterator.next()));

Review Comment:
   Ah, good call. I shall do so.



##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java:
##########
@@ -52,218 +39,160 @@
 public class KsmDirectoryService extends VaultDirectoryService {
 
     /**
-     * Service for retrieving KSM configuration details.
+     * A factory for constructing new KsmUser instances.
      */
     @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();
+    private KsmUserFactory ksmUserFactory;
 
     /**
-     * All expected fields in the KSM configuration JSON blob.
+     * Service for retrieving any custom attributes defined for the
+     * current vault implementation and processing of said attributes.
      */
-    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);
+    @Inject
+    private KsmAttributeService attributeService;
 
-        // Check if the attribute is set to a non-empty value
-        if (ksmConfigValue != null && !ksmConfigValue.trim().isEmpty()) {
+    @Override
+    public Directory<Connection> getConnectionDirectory(
+            Directory<Connection> underlyingDirectory) throws 
GuacamoleException {
 
-            // If it's already base64-encoded, it's a KSM configuration blob,
-            // so validate it immediately
-            if (isBase64(ksmConfigValue)) {
+        // A Connection directory that will decorate all connections with a
+        // KsmConnection wrapper to ensure that all defined KSM fields will
+        // be exposed in the connection group attributes.
+        return new DecoratingDirectory<Connection>(underlyingDirectory) {
 
-                // Attempt to validate the config as a base64-econded KSM 
config blob
-                try {
-                    KsmConfig.parseKsmConfig(ksmConfigValue);
+            @Override
+            protected Connection decorate(Connection connection) throws 
GuacamoleException {
 
-                    // If it validates, the entity can be left alone - it's 
already valid
-                    return;
-                }
+                // Wrap in a KsmConnection class to ensure that all defined 
KSM fields will be
+                // present
+                return new KsmConnection(connection);
+            }
 
-                catch (GuacamoleException exception) {
+            @Override
+            protected Connection undecorate(Connection connection) throws 
GuacamoleException {
 
-                    // 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);
-                }
+                // Unwrap the KsmConnection
+                return ((KsmConnection) connection).getUnderlyingConnection();
             }
 
-            // 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();
+    @Override
+    public Directory<ConnectionGroup> getConnectionGroupDirectory(
+            Directory<ConnectionGroup> underlyingDirectory) throws 
GuacamoleException {
 
-                // Populate the in-memory storage using the one-time-token
-                SecretsManager.initializeStorage(storage, ksmConfigValue, 
null);
+        // A ConnectionGroup directory that will intercept add and update 
calls to
+        // 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.
+        // The value of the KSM_CONFIGURATION_ATTRIBUTE will be sanitized if 
set.
+        return new KsmDirectory<ConnectionGroup>(underlyingDirectory) {
 
-                // Create an options object using the values we extracted from 
the one-time token
-                SecretsManagerOptions options = new SecretsManagerOptions(
-                    storage, null,
-                    configurationService.getAllowUnverifiedCertificate());
+            @Override
+            public void add(ConnectionGroup connectionGroup) throws 
GuacamoleException {
 
-                // 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);
+                // Process attribute values before saving
+                connectionGroup.setAttributes(
+                    attributeService.processAttributes(
+                        connectionGroup.getAttributes()));
 
-                // Create a map to store the extracted values from the KSM 
storage
-                Map<String, String> configMap = new HashMap<>();
+                super.add(connectionGroup);
+            }
 
-                // Go through all the expected fields, extract from the KSM 
storage,
-                // and write to the newly created map
-                EXPECTED_KSM_FIELDS.forEach(configKey -> {
+            @Override
+            public void update(ConnectionGroup connectionGroup) throws 
GuacamoleException {
 
-                    // Only write the value into the new map if non-null
-                    String value = storage.getString(configKey);
-                    if (value != null)
-                        configMap.put(configKey, value);
+                // Unwrap the existing ConnectionGroup
+                if (connectionGroup instanceof KsmConnectionGroup)
+                    connectionGroup = ((KsmConnectionGroup) 
connectionGroup).getUnderlyingConnectionGroup();
 
-                });
+                // Process attribute values before saving
+                connectionGroup.setAttributes(
+                    attributeService.processAttributes(
+                        connectionGroup.getAttributes()));
 
-                // 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));
+                super.update(connectionGroup);
+
+            }
 
-                // Finally, try to parse the newly generated token as a KSM 
config. If this
-                // works, the config should be fully functional
-                KsmConfig.parseKsmConfig(base64EncodedJson);
+            @Override
+            public ConnectionGroup get(String identifier) throws 
GuacamoleException {
 
-                // 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);
+                // Fetch the ConnectionGroup
+                ConnectionGroup connectionGroup = super.get(identifier);
 
-                // Set the newly updated attributes back to the original object
-                entity.setAttributes(attributes);
+                // Do not process the ConnectionGroup further if it does not 
exist
+                if (connectionGroup == null)
+                    return null;
 
-            }
+                // Unwrap the existing ConnectionGroup before rewrapping
+                if (connectionGroup instanceof KsmConnectionGroup)
+                connectionGroup = ((KsmConnectionGroup) 
connectionGroup).getUnderlyingConnectionGroup();

Review Comment:
   Oops, will fix.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to