From e4c65cba19fadc87ff61500d2065766472398ad2 Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Tue, 19 Jul 2022 18:31:40 +0000 Subject: [PATCH 1/8] GUACAMOLE-1656: Add per-user KSM vault functionality. --- .../vault/conf/VaultAttributeService.java | 30 +++ .../vault/user/VaultUserContext.java | 59 ++++-- .../ksm/KsmAuthenticationProviderModule.java | 1 + .../vault/ksm/conf/KsmAttributeService.java | 66 ++++++- .../ksm/conf/KsmConfigurationService.java | 26 +++ .../vault/ksm/secret/KsmSecretService.java | 177 +++++++++++------- .../vault/ksm/user/KsmConnection.java | 82 ++++++++ .../vault/ksm/user/KsmDirectoryService.java | 87 +++++++++ .../guacamole/vault/ksm/user/KsmUser.java | 82 ++++++++ .../src/main/resources/translations/en.json | 10 + .../net/auth/DelegatingUserContext.java | 5 + .../guacamole/net/auth/UserContext.java | 17 ++ .../controllers/manageUserController.js | 2 + .../src/app/rest/services/schemaService.js | 28 +++ .../directives/guacSettingsPreferences.js | 124 +++++++++++- .../templates/settingsPreferences.html | 10 + .../main/frontend/src/translations/en.json | 3 + guacamole/src/main/frontend/webpack.config.js | 12 -- .../guacamole/rest/schema/SchemaResource.java | 20 ++ .../guacamole/rest/user/UserResource.java | 53 +++++- 20 files changed, 785 insertions(+), 109 deletions(-) create mode 100644 extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java create mode 100644 extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java diff --git a/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/conf/VaultAttributeService.java b/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/conf/VaultAttributeService.java index 2bd08cde6..bccf08e03 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/conf/VaultAttributeService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/conf/VaultAttributeService.java @@ -30,6 +30,16 @@ import org.apache.guacamole.form.Form; */ public interface VaultAttributeService { + /** + * Return all custom connection attributes to be exposed through the + * admin UI for the current vault implementation. + * + * @return + * All custom connection attributes to be exposed through the + * admin UI for the current vault implementation. + */ + public Collection
getConnectionAttributes(); + /** * Return all custom connection group attributes to be exposed through the * admin UI for the current vault implementation. @@ -39,4 +49,24 @@ public interface VaultAttributeService { * admin UI for the current vault implementation. */ public Collection getConnectionGroupAttributes(); + + /** + * Return all custom user attributes to be exposed through the admin UI for + * the current vault implementation. + * + * @return + * All custom user attributes to be exposed through the admin UI for + * the current vault implementation. + */ + public Collection getUserAttributes(); + + /** + * Return all user preference attributes to be exposed through the user + * preferences UI for the current vault implementation. + * + * @return + * All user preference attributes to be exposed through the user + * preferences UI for the current vault implementation. + */ + public Collection getUserPreferenceAttributes(); } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/user/VaultUserContext.java b/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/user/VaultUserContext.java index 8e8f66853..748237dd2 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/user/VaultUserContext.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/user/VaultUserContext.java @@ -241,7 +241,7 @@ public class VaultUserContext extends TokenInjectingUserContext { * * @throws GuacamoleException * If the value for any applicable secret cannot be retrieved from the - * vault due to an error. + * vault due to an error.1 */ private Map> getTokens( Connectable connectable, Map tokenMapping, @@ -407,7 +407,6 @@ public class VaultUserContext extends TokenInjectingUserContext { TokenFilter filter = createFilter(); filter.setToken(CONNECTION_NAME_TOKEN, connection.getName()); filter.setToken(CONNECTION_IDENTIFIER_TOKEN, identifier); - // Add hostname and username tokens if available (implementations are // not required to expose connection configuration details) @@ -439,17 +438,6 @@ public class VaultUserContext extends TokenInjectingUserContext { } - @Override - public Collection getConnectionGroupAttributes() { - - // Add any custom attributes to any previously defined attributes - return Collections.unmodifiableCollection(Stream.concat( - super.getConnectionGroupAttributes().stream(), - attributeService.getConnectionGroupAttributes().stream() - ).collect(Collectors.toList())); - - } - @Override public Directory getUserDirectory() throws GuacamoleException { @@ -490,6 +478,51 @@ public class VaultUserContext extends TokenInjectingUserContext { // Defer to the vault-specific directory service return directoryService.getSharingProfileDirectory(super.getSharingProfileDirectory()); + + } + + @Override + public Collection getUserAttributes() { + + // Add any custom attributes to any previously defined attributes + return Collections.unmodifiableCollection(Stream.concat( + super.getUserAttributes().stream(), + attributeService.getUserAttributes().stream() + ).collect(Collectors.toList())); + + } + + @Override + public Collection getUserPreferenceAttributes() { + + // Add any custom preference attributes to any previously defined attributes + return Collections.unmodifiableCollection(Stream.concat( + super.getUserPreferenceAttributes().stream(), + attributeService.getUserPreferenceAttributes().stream() + ).collect(Collectors.toList())); + + } + + @Override + public Collection getConnectionAttributes() { + + // Add any custom attributes to any previously defined attributes + return Collections.unmodifiableCollection(Stream.concat( + super.getConnectionAttributes().stream(), + attributeService.getConnectionAttributes().stream() + ).collect(Collectors.toList())); + + } + + @Override + public Collection getConnectionGroupAttributes() { + + // Add any custom attributes to any previously defined attributes + return Collections.unmodifiableCollection(Stream.concat( + super.getConnectionGroupAttributes().stream(), + attributeService.getConnectionGroupAttributes().stream() + ).collect(Collectors.toList())); + } } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/KsmAuthenticationProviderModule.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/KsmAuthenticationProviderModule.java index 3c28553e0..faf22aa4e 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/KsmAuthenticationProviderModule.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/KsmAuthenticationProviderModule.java @@ -57,6 +57,7 @@ public class KsmAuthenticationProviderModule // Bind services specific to Keeper Secrets Manager bind(KsmRecordService.class); + bind(KsmAttributeService.class); bind(VaultAttributeService.class).to(KsmAttributeService.class); bind(VaultConfigurationService.class).to(KsmConfigurationService.class); bind(VaultSecretService.class).to(KsmSecretService.class); diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java index 83ab9c4a3..bdceb4c21 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java @@ -23,10 +23,13 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +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.vault.conf.VaultAttributeService; +import com.google.inject.Inject; import com.google.inject.Singleton; /** @@ -36,28 +39,81 @@ import com.google.inject.Singleton; @Singleton public class KsmAttributeService implements VaultAttributeService { + + @Inject + private KsmConfigurationService configurationService; + /** * The name of the attribute which can contain a KSM configuration blob - * associated with a connection group. + * associated with either a connection group or user. */ public static final String KSM_CONFIGURATION_ATTRIBUTE = "ksm-config"; /** * All attributes related to configuring the KSM vault on a - * per-connection-group basis. + * per-connection-group or per-user basis. */ public static final Form KSM_CONFIGURATION_FORM = new Form("ksm-config", Arrays.asList(new TextField(KSM_CONFIGURATION_ATTRIBUTE))); /** - * All KSM-specific connection group attributes, organized by form. + * All KSM-specific attributes for users or connection groups, organized by form. */ - public static final Collection KSM_CONNECTION_GROUP_ATTRIBUTES = + public static final Collection KSM_ATTRIBUTES = Collections.unmodifiableCollection(Arrays.asList(KSM_CONFIGURATION_FORM)); + /** + * The name of the attribute which can controls whether a KSM user configuration + * is enabled on a connection-by-connection basis. + */ + public static final String KSM_USER_CONFIG_ENABLED_ATTRIBUTE = "ksm-user-config-enabled"; + + /** + * The string value used by KSM attributes to represent the boolean value "true". + */ + public static final String TRUTH_VALUE = "true"; + + /** + * All attributes related to configuring the KSM vault on a per-connection basis. + */ + public static final Form KSM_CONNECTION_FORM = new Form("ksm-config", + Arrays.asList(new BooleanField(KSM_USER_CONFIG_ENABLED_ATTRIBUTE, TRUTH_VALUE))); + + /** + * All KSM-specific attributes for connections, organized by form. + */ + public static final Collection KSM_CONNECTION_ATTRIBUTES = + Collections.unmodifiableCollection(Arrays.asList(KSM_CONNECTION_FORM)); + + @Override + public Collection getConnectionAttributes() { + return KSM_CONNECTION_ATTRIBUTES; + } + @Override public Collection getConnectionGroupAttributes() { - return KSM_CONNECTION_GROUP_ATTRIBUTES; + return KSM_ATTRIBUTES; } + @Override + public Collection getUserAttributes() { + return KSM_ATTRIBUTES; + } + + @Override + public Collection getUserPreferenceAttributes() { + + try { + + // Expose the user attributes IFF user-level KSM configuration is enabled + return configurationService.getAllowUserConfig() ? KSM_ATTRIBUTES : Collections.emptyList(); + + } catch (GuacamoleException e) { + + // If the configuration can't be parsed, default to not exposing the attribute + return Collections.emptyList(); + } + } + + } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java index c9a9536ea..9d2e4565f 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java @@ -84,6 +84,17 @@ public class KsmConfigurationService extends VaultConfigurationService { } }; + /** + * Whether users should be able to supply their own KSM configurations. + */ + private static final BooleanGuacamoleProperty ALLOW_USER_CONFIG = new BooleanGuacamoleProperty() { + + @Override + public String getName() { + return "ksm-allow-user-config"; + } + }; + /** * Whether windows domains should be stripped off from usernames that are * read from the KSM vault. @@ -138,6 +149,21 @@ public class KsmConfigurationService extends VaultConfigurationService { return environment.getProperty(ALLOW_UNVERIFIED_CERT, false); } + /** + * Return whether users should be able to provide their own KSM configs. + * + * @return + * true if users should be able to provide their own KSM configs, + * false otherwise. + * + * @throws GuacamoleException + * If the value specified within guacamole.properties cannot be + * parsed. + */ + public boolean getAllowUserConfig() throws GuacamoleException { + return environment.getProperty(ALLOW_USER_CONFIG, false); + } + @Override public boolean getSplitWindowsUsernames() throws GuacamoleException { return environment.getProperty(STRIP_WINDOWS_DOMAINS, false); diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java index cb5868cfa..a3b772256 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java @@ -26,8 +26,11 @@ import com.keepersecurity.secretsManager.core.SecretsManagerOptions; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; +import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; @@ -304,6 +307,34 @@ public class KsmSecretService implements VaultSecretService { } + /** + * Returns true if user-level KSM configuration is enabled for the given + * Connectable, false otherwise. + * + * @param connectable + * The connectable to check for whether user-level KSM configs are + * enabled. + * + * @return + * True if user-level KSM configuration is enabled for the given + * Connectable, false otherwise. + */ + private boolean isKsmUserConfigEnabled(Connectable connectable) { + + // If it's a connection, user-level config is enabled IFF the appropriate + // attribute is set to true + if (connectable instanceof Connection) + return KsmAttributeService.TRUTH_VALUE.equals(((Connection) connectable).getAttributes().get( + KsmAttributeService.KSM_USER_CONFIG_ENABLED_ATTRIBUTE)); + + // KSM token replacement is not enabled for balancing groups, so for + // now, user-level KSM configs will be explicitly disabled. + // TODO: If token replacement is implemented for balancing groups, + // implement this functionality for them as well. + return false; + + } + @Override public Map> getTokens(UserContext userContext, Connectable connectable, GuacamoleConfiguration config, TokenFilter filter) throws GuacamoleException { @@ -314,78 +345,92 @@ public class KsmSecretService implements VaultSecretService { // Attempt to find a KSM config for this connection or group String ksmConfig = getConnectionGroupKsmConfig(userContext, connectable); - // Get a client instance for this KSM config - KsmClient ksm = getClient(ksmConfig); + // Create a list containing just the global / connection group config + List ksmClients = new ArrayList<>(2); + ksmClients.add(getClient(ksmConfig)); - // Retrieve and define server-specific tokens, if any - String hostname = parameters.get("hostname"); - if (hostname != null && !hostname.isEmpty()) - addRecordTokens(tokens, "KEEPER_SERVER_", - ksm.getRecordByHost(filter.filter(hostname))); + // Only use the user-specific KSM config if explicitly enabled in the global + // configuration, AND for the specific connectable being connected to + if (confService.getAllowUserConfig() && isKsmUserConfigEnabled(connectable)) { - // Tokens specific to RDP - if ("rdp".equals(config.getProtocol())) { - - // Retrieve and define gateway server-specific tokens, if any - String gatewayHostname = parameters.get("gateway-hostname"); - if (gatewayHostname != null && !gatewayHostname.isEmpty()) - addRecordTokens(tokens, "KEEPER_GATEWAY_", - ksm.getRecordByHost(filter.filter(gatewayHostname))); - - // Retrieve and define domain tokens, if any - String domain = parameters.get("domain"); - String filteredDomain = null; - if (domain != null && !domain.isEmpty()) { - filteredDomain = filter.filter(domain); - addRecordTokens(tokens, "KEEPER_DOMAIN_", - ksm.getRecordByDomain(filteredDomain)); - } - - // Retrieve and define gateway domain tokens, if any - String gatewayDomain = parameters.get("gateway-domain"); - String filteredGatewayDomain = null; - if (gatewayDomain != null && !gatewayDomain.isEmpty()) { - filteredGatewayDomain = filter.filter(gatewayDomain); - addRecordTokens(tokens, "KEEPER_GATEWAY_DOMAIN_", - ksm.getRecordByDomain(filteredGatewayDomain)); - } - - // If domain matching is disabled for user records, - // explicitly set the domains to null when storing - // user records to enable username-only matching - if (!confService.getMatchUserRecordsByDomain()) { - filteredDomain = null; - filteredGatewayDomain = null; - } - - // Retrieve and define user-specific tokens, if any - String username = parameters.get("username"); - if (username != null && !username.isEmpty()) - addRecordTokens(tokens, "KEEPER_USER_", - ksm.getRecordByLogin(filter.filter(username), - filteredDomain)); - - // Retrieve and define gateway user-specific tokens, if any - String gatewayUsername = parameters.get("gateway-username"); - if (gatewayUsername != null && !gatewayUsername.isEmpty()) - addRecordTokens(tokens, "KEEPER_GATEWAY_USER_", - ksm.getRecordByLogin( - filter.filter(gatewayUsername), - filteredGatewayDomain)); + // Find a user-specific KSM config, if one exists + String userKsmConfig = userContext.self().getAttributes().get( + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE); + // If a user-specific config exsts, process it first + if (userKsmConfig != null && !userKsmConfig.trim().isEmpty()) + ksmClients.add(0, getClient(userKsmConfig)); } - else { + // Iterate through the KSM clients, processing using the user-specific + // config first (if it exists), to ensure that any admin-defined values + // will override the user-speicifc values + Iterator ksmIterator = ksmClients.iterator(); + while (ksmIterator.hasNext()) { - // Retrieve and define user-specific tokens, if any - // NOTE that non-RDP connections do not have a domain - // field in the connection parameters, so the domain - // will always be null - String username = parameters.get("username"); - if (username != null && !username.isEmpty()) - addRecordTokens(tokens, "KEEPER_USER_", - ksm.getRecordByLogin(filter.filter(username), null)); - } + KsmClient ksm = ksmIterator.next(); + + // Retrieve and define server-specific tokens, if any + String hostname = parameters.get("hostname"); + if (hostname != null && !hostname.isEmpty()) + addRecordTokens(tokens, "KEEPER_SERVER_", + ksm.getRecordByHost(filter.filter(hostname))); + + // Tokens specific to RDP + if ("rdp".equals(config.getProtocol())) { + // Retrieve and define domain tokens, if any + String domain = parameters.get("domain"); + String filteredDomain = null; + if (domain != null && !domain.isEmpty()) { + filteredDomain = filter.filter(domain); + addRecordTokens(tokens, "KEEPER_DOMAIN_", + ksm.getRecordByDomain(filteredDomain)); + } + + // Retrieve and define gateway domain tokens, if any + String gatewayDomain = parameters.get("gateway-domain"); + String filteredGatewayDomain = null; + if (gatewayDomain != null && !gatewayDomain.isEmpty()) { + filteredGatewayDomain = filter.filter(gatewayDomain); + addRecordTokens(tokens, "KEEPER_GATEWAY_DOMAIN_", + ksm.getRecordByDomain(filteredGatewayDomain)); + } + + // If domain matching is disabled for user records, + // explicitly set the domains to null when storing + // user records to enable username-only matching + if (!confService.getMatchUserRecordsByDomain()) { + filteredDomain = null; + filteredGatewayDomain = null; + } + + // Retrieve and define user-specific tokens, if any + String username = parameters.get("username"); + if (username != null && !username.isEmpty()) + addRecordTokens(tokens, "KEEPER_USER_", + ksm.getRecordByLogin(filter.filter(username), + filteredDomain)); + + // Retrieve and define gateway user-specific tokens, if any + String gatewayUsername = parameters.get("gateway-username"); + if (gatewayUsername != null && !gatewayUsername.isEmpty()) + addRecordTokens(tokens, "KEEPER_GATEWAY_USER_", + ksm.getRecordByLogin( + filter.filter(gatewayUsername), + filteredGatewayDomain)); + } + + else { + + // Retrieve and define user-specific tokens, if any + // NOTE that non-RDP connections do not have a domain + // field in the connection parameters, so the domain + // will always be null + String username = parameters.get("username"); + if (username != null && !username.isEmpty()) + addRecordTokens(tokens, "KEEPER_USER_", + ksm.getRecordByLogin(filter.filter(username), null)); + } return tokens; diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java new file mode 100644 index 000000000..da2866202 --- /dev/null +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java @@ -0,0 +1,82 @@ +/* + * 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.List; +import java.util.Map; + +import org.apache.guacamole.net.auth.DelegatingConnection; +import org.apache.guacamole.net.auth.Connection; + +import com.google.common.collect.Maps; + +/** + * A Connection that explicitly adds a blank entry for any defined + * KSM connection attributes. + */ +public class KsmConnection extends DelegatingConnection { + + /** + * The names of all connection attributes defined for the vault. + */ + private List connectionAttributeNames; + + /** + * Create a new Vault connection wrapping the provided Connection record. Any + * attributes defined in the provided connection attribute forms will have empty + * values automatically populated when getAttributes() is called. + * + * @param connection + * The connection record to wrap. + * + * @param connectionAttributeNames + * The names of all connection attributes to automatically expose. + */ + KsmConnection(Connection connection, List connectionAttributeNames) { + + super(connection); + this.connectionAttributeNames = connectionAttributeNames; + + } + + /** + * Return the underlying wrapped connection record. + * + * @return + * The wrapped connection record. + */ + Connection getUnderlyingConnection() { + return getDelegateConnection(); + } + + @Override + public Map getAttributes() { + + // Make a copy of the existing map + Map attributeMap = Maps.newHashMap(super.getAttributes()); + + // Add every defined attribute + connectionAttributeNames.forEach( + attributeName -> attributeMap.putIfAbsent(attributeName, null)); + + return attributeMap; + } + +} diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java index b924bc524..3eff928fe 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java @@ -26,13 +26,16 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; 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.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; @@ -57,6 +60,12 @@ public class KsmDirectoryService extends VaultDirectoryService { @Inject private KsmConfigurationService configurationService; + /** + * Service for retrieving KSM-specific attributes. + */ + @Inject + private KsmAttributeService ksmAttributeService; + /** * A singleton ObjectMapper for converting a Map to a JSON string when * generating a base64-encoded JSON KSM config blob. @@ -222,6 +231,36 @@ public class KsmDirectoryService extends VaultDirectoryService { } + @Override + public Directory getConnectionDirectory( + Directory underlyingDirectory) throws GuacamoleException { + + // A Connection directory that will intercept add and update calls to + // validate KSM configurations, and translate one-time-tokens, if possible + return new DecoratingDirectory(underlyingDirectory) { + + @Override + protected Connection decorate(Connection connection) throws GuacamoleException { + + // Wrap in a KsmConnection class to ensure that all defined KSM fields will be + // present + return new KsmConnection( + connection, + ksmAttributeService.getConnectionAttributes().stream().flatMap( + form -> form.getFields().stream().map(field -> field.getName()) + ).collect(Collectors.toList())); + } + + @Override + protected Connection undecorate(Connection connection) throws GuacamoleException { + + // Unwrap the KsmUser + return ((KsmConnection) connection).getUnderlyingConnection(); + } + + }; + } + @Override public Directory getConnectionGroupDirectory( Directory underlyingDirectory) throws GuacamoleException { @@ -269,4 +308,52 @@ public class KsmDirectoryService extends VaultDirectoryService { }; } + + @Override + public Directory getUserDirectory( + Directory underlyingDirectory) throws GuacamoleException { + + // A User directory that will intercept add and update calls to + // validate KSM configurations, and translate one-time-tokens, if possible + return new DecoratingDirectory(underlyingDirectory) { + + @Override + public void add(User user) throws GuacamoleException { + + // Check for the KSM config attribute and translate the one-time token + // if possible before adding + processAttributes(user); + super.add(user); + } + + @Override + public void update(User user) throws GuacamoleException { + + // Check for the KSM config attribute and translate the one-time token + // if possible before updating + processAttributes(user); + super.update(user); + } + + @Override + protected User decorate(User user) throws GuacamoleException { + + // Wrap in a KsmUser class to ensure that all defined KSM fields will be + // present + return new KsmUser( + user, + ksmAttributeService.getUserAttributes().stream().flatMap( + form -> form.getFields().stream().map(field -> field.getName()) + ).collect(Collectors.toList())); + } + + @Override + protected User undecorate(User user) throws GuacamoleException { + + // Unwrap the KsmUser + return ((KsmUser) user).getUnderlyingUser(); + } + + }; + } } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java new file mode 100644 index 000000000..a846eb0d8 --- /dev/null +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java @@ -0,0 +1,82 @@ +/* + * 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.List; +import java.util.Map; + +import org.apache.guacamole.net.auth.DelegatingUser; +import org.apache.guacamole.net.auth.User; + +import com.google.common.collect.Maps; + +/** + * A User that explicitly adds a blank entry for any defined + * KSM user attributes. + */ +public class KsmUser extends DelegatingUser { + + /** + * The names of all user attributes defined for the vault. + */ + private List userAttributeNames; + + /** + * Create a new Vault user wrapping the provided User record. Any + * attributes defined in the provided user attribute forms will have empty + * values automatically populated when getAttributes() is called. + * + * @param user + * The user record to wrap. + * + * @param userAttributeNames + * The names of all user attributes to automatically expose. + */ + KsmUser(User user, List userAttributeNames) { + + super(user); + this.userAttributeNames = userAttributeNames; + + } + + /** + * Return the underlying wrapped user record. + * + * @return + * The wrapped user record. + */ + User getUnderlyingUser() { + return getDelegateUser(); + } + + @Override + public Map getAttributes() { + + // Make a copy of the existing map + Map attributeMap = Maps.newHashMap(super.getAttributes()); + + // Add every defined attribute + userAttributeNames.forEach( + attributeName -> attributeMap.putIfAbsent(attributeName, null)); + + return attributeMap; + } + +} diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/resources/translations/en.json b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/resources/translations/en.json index 4601a13f1..034f5f24a 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/resources/translations/en.json +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/resources/translations/en.json @@ -4,12 +4,22 @@ "NAME" : "Keeper Secrets Manager" }, + "CONNECTION_ATTRIBUTES" : { + "SECTION_HEADER_KSM_CONFIG" : "Keeper Secrets Manager", + "FIELD_HEADER_KSM_USER_CONFIG_ENABLED" : "Allow user-provided KSM configuration" + }, + "CONNECTION_GROUP_ATTRIBUTES" : { "SECTION_HEADER_KSM_CONFIG" : "Keeper Secrets Manager", "FIELD_HEADER_KSM_CONFIG" : "KSM Service Configuration ", "ERROR_INVALID_KSM_CONFIG_BLOB" : "The provided base64-encoded KSM configuration blob is not valid. Please ensure that you have copied the entire blob.", "ERROR_INVALID_KSM_ONE_TIME_TOKEN" : "The provided configuration is not a valid KSM one-time token or base64-encoded configuration blob. Please ensure that you have copied the entire token value." + }, + + "USER_ATTRIBUTES" : { + "SECTION_HEADER_KSM_CONFIG" : "Keeper Secrets Manager", + "FIELD_HEADER_KSM_CONFIG" : "KSM Service Configuration " } } diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingUserContext.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingUserContext.java index 85e025909..4b0343181 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingUserContext.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingUserContext.java @@ -127,6 +127,11 @@ public class DelegatingUserContext implements UserContext { return userContext.getUserAttributes(); } + @Override + public Collection getUserPreferenceAttributes() { + return userContext.getUserPreferenceAttributes(); + } + @Override public Collection getUserGroupAttributes() { return userContext.getUserGroupAttributes(); diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/UserContext.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/UserContext.java index ccdcaae09..3f75b899c 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/UserContext.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/UserContext.java @@ -20,6 +20,8 @@ package org.apache.guacamole.net.auth; import java.util.Collection; +import java.util.Collections; + import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.form.Form; @@ -211,6 +213,21 @@ public interface UserContext { */ Collection getUserAttributes(); + /** + * Retrieves a collection of user attributes, specific to the user preferences + * page in the UI. Unlike standard user attributes, these should be self-editable. + * + * @return + * A collection of form of user attributes, specific to the user preferences + * page in the UI. + */ + default Collection getUserPreferenceAttributes() { + + // By default, a user context does not expose any preference user attributes + return Collections.emptyList(); + + } + /** * Retrieves a collection of all attributes applicable to user groups. This * collection will contain only those attributes which the current user has diff --git a/guacamole/src/main/frontend/src/app/manage/controllers/manageUserController.js b/guacamole/src/main/frontend/src/app/manage/controllers/manageUserController.js index f7ead136a..94e3d8944 100644 --- a/guacamole/src/main/frontend/src/app/manage/controllers/manageUserController.js +++ b/guacamole/src/main/frontend/src/app/manage/controllers/manageUserController.js @@ -501,4 +501,6 @@ angular.module('manage').controller('manageUserController', ['$scope', '$injecto return userService.deleteUser($scope.dataSource, $scope.user); }; + console.log($scope); + }]); diff --git a/guacamole/src/main/frontend/src/app/rest/services/schemaService.js b/guacamole/src/main/frontend/src/app/rest/services/schemaService.js index dee10e1bd..d6afa2b75 100644 --- a/guacamole/src/main/frontend/src/app/rest/services/schemaService.js +++ b/guacamole/src/main/frontend/src/app/rest/services/schemaService.js @@ -58,6 +58,34 @@ angular.module('rest').factory('schemaService', ['$injector', }; + /** + * Makes a request to the REST API to get the list of available user preference + * attributes, returning a promise that provides an array of @link{Form} objects + * if successful. Each element of the array describes a logical grouping of + * possible user preference attributes. + * + * @param {String} dataSource + * The unique identifier of the data source containing the users whose + * available user preference attributes are to be retrieved. This + * identifier corresponds to an AuthenticationProvider within the + * Guacamole web application. + * + * @returns {Promise.} + * A promise which will resolve with an array of @link{Form} + * objects, where each @link{Form} describes a logical grouping of + * possible attributes. + */ + service.getUserPreferenceAttributes = function getUserPreferenceAttributes(dataSource) { + + // Retrieve available user attributes + return authenticationService.request({ + cache : cacheService.schema, + method : 'GET', + url : 'api/session/data/' + encodeURIComponent(dataSource) + '/schema/userPreferenceAttributes' + }); + + }; + /** * Makes a request to the REST API to get the list of available attributes * for user group objects, returning a promise that provides an array of diff --git a/guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js b/guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js index aad0a2e0e..5824eb194 100644 --- a/guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js +++ b/guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js @@ -21,7 +21,7 @@ * A directive for managing preferences local to the current user. */ angular.module('settings').directive('guacSettingsPreferences', [function guacSettingsPreferences() { - + return { // Element only restrict: 'E', @@ -33,16 +33,18 @@ angular.module('settings').directive('guacSettingsPreferences', [function guacSe controller: ['$scope', '$injector', function settingsPreferencesController($scope, $injector) { // Get required types - var PermissionSet = $injector.get('PermissionSet'); + const Form = $injector.get('Form'); + const PermissionSet = $injector.get('PermissionSet'); // Required services - var $translate = $injector.get('$translate'); - var authenticationService = $injector.get('authenticationService'); - var guacNotification = $injector.get('guacNotification'); - var permissionService = $injector.get('permissionService'); - var preferenceService = $injector.get('preferenceService'); - var requestService = $injector.get('requestService'); - var userService = $injector.get('userService'); + const $translate = $injector.get('$translate'); + const authenticationService = $injector.get('authenticationService'); + const guacNotification = $injector.get('guacNotification'); + const permissionService = $injector.get('permissionService'); + const preferenceService = $injector.get('preferenceService'); + const requestService = $injector.get('requestService'); + const schemaService = $injector.get('schemaService'); + const userService = $injector.get('userService'); /** * An action to be provided along with the object sent to @@ -56,6 +58,13 @@ angular.module('settings').directive('guacSettingsPreferences', [function guacSe } }; + /** + * The user being modified. + * + * @type User + */ + $scope.user = null; + /** * The username of the current user. * @@ -78,6 +87,26 @@ angular.module('settings').directive('guacSettingsPreferences', [function guacSe */ $scope.preferences = preferenceService.preferences; + /** + * All available user attributes, as a mapping of form name to form + * object. The form object contains a name, as well as a Map of fields. + * + * The Map type is used here to maintain form/name uniqueness, as well as + * insertion order, to ensure a consistent UI experience. + * + * @type Map + */ + $scope.attributeMap = new Map(); + + /** + * All available user attributes. This is only the set of attribute + * definitions, organized as logical groupings of attributes, not attribute + * values. + * + * @type Form[] + */ + $scope.attributes = null; + /** * The fields which should be displayed for choosing locale * preferences. Each field name must be a property on @@ -197,7 +226,82 @@ angular.module('settings').directive('guacSettingsPreferences', [function guacSe }; + + /** + * Saves the current user, displaying an acknowledgement message if + * saving was successful, or an error if the save failed. + */ + $scope.saveUser = function saveUser() { + return userService.saveUser(dataSource, $scope.user) + .then(() => guacNotification.showStatus({ + text : { + key : 'SETTINGS_PREFERENCES.INFO_PREFERENCE_ATTRIBUTES_CHANGED' + }, + actions : [ ACKNOWLEDGE_ACTION ] + }), + guacNotification.SHOW_REQUEST_ERROR); + }; + + // Fetch the user record + userService.getUser(dataSource, username).then(function saveUserData(user) { + $scope.user = user; + }) + + // Get all datasources that are available for this user + authenticationService.getAvailableDataSources().forEach(function loadAttributesForDataSource(dataSource) { + + // Fetch all user attribute forms defined for the datasource + schemaService.getUserPreferenceAttributes(dataSource).then(function saveAttributes(attributes) { + + // Iterate through all attribute forms + attributes.forEach(function addAttribute(attributeForm) { + + // If the form with the retrieved name already exists + if ($scope.attributeMap.has(attributeForm.name)) { + const existingFields = $scope.attributeMap.get(attributeForm.name).fields; + + // Add each field to the existing list for this form + attributeForm.fields.forEach(function addAllFieldsToExistingMap(field) { + existingFields.set(field.name, field); + }) + } + + else { + + // Create a new entry for the form + $scope.attributeMap.set(attributeForm.name, { + name: attributeForm.name, + + // With the field array from the API converted into a Map + fields: attributeForm.fields.reduce( + function addFieldToMap(currentFieldMap, field) { + currentFieldMap.set(field.name, field); + return currentFieldMap; + }, new Map() + ) + + }) + } + + }); + + // Re-generate the attributes array every time + $scope.attributes = Array.of(...$scope.attributeMap.values()).map(function convertFieldsToArray(formObject) { + + // Convert each temporary form object to a Form type + return new Form({ + name: formObject.name, + + // Convert the field map to a simple array of fields + fields: Array.of(...formObject.fields.values()) + }) + }); + + }); + + }); + }] }; - + }]); diff --git a/guacamole/src/main/frontend/src/app/settings/templates/settingsPreferences.html b/guacamole/src/main/frontend/src/app/settings/templates/settingsPreferences.html index cabd2ae9f..ac5cc4f59 100644 --- a/guacamole/src/main/frontend/src/app/settings/templates/settingsPreferences.html +++ b/guacamole/src/main/frontend/src/app/settings/templates/settingsPreferences.html @@ -89,4 +89,14 @@ + +

{{'SETTINGS_PREFERENCES.SECTION_HEADER_UPDATE_ATTRIBUTES' | translate}}

+
+ + + + +
+ diff --git a/guacamole/src/main/frontend/src/translations/en.json b/guacamole/src/main/frontend/src/translations/en.json index 05a9fa279..7f52fea11 100644 --- a/guacamole/src/main/frontend/src/translations/en.json +++ b/guacamole/src/main/frontend/src/translations/en.json @@ -916,6 +916,7 @@ "ACTION_ACKNOWLEDGE" : "@:APP.ACTION_ACKNOWLEDGE", "ACTION_CANCEL" : "@:APP.ACTION_CANCEL", + "ACTION_SAVE" : "@:APP.ACTION_SAVE", "ACTION_UPDATE_PASSWORD" : "@:APP.ACTION_UPDATE_PASSWORD", "DIALOG_HEADER_ERROR" : "@:APP.DIALOG_HEADER_ERROR", @@ -942,6 +943,7 @@ "HELP_UPDATE_PASSWORD" : "If you wish to change your password, enter your current password and the desired new password below, and click \"Update Password\". The change will take effect immediately.", "INFO_PASSWORD_CHANGED" : "Password changed.", + "INFO_PREFERENCE_ATTRIBUTES_CHANGED" : "User attributes saved.", "NAME_INPUT_METHOD_NONE" : "@:CLIENT.NAME_INPUT_METHOD_NONE", "NAME_INPUT_METHOD_OSK" : "@:CLIENT.NAME_INPUT_METHOD_OSK", @@ -949,6 +951,7 @@ "SECTION_HEADER_DEFAULT_INPUT_METHOD" : "Default Input Method", "SECTION_HEADER_DEFAULT_MOUSE_MODE" : "Default Mouse Emulation Mode", + "SECTION_HEADER_UPDATE_ATTRIBUTES" : "User Attributes", "SECTION_HEADER_UPDATE_PASSWORD" : "Change Password" }, diff --git a/guacamole/src/main/frontend/webpack.config.js b/guacamole/src/main/frontend/webpack.config.js index 29bb8ddd5..3f1574fa1 100644 --- a/guacamole/src/main/frontend/webpack.config.js +++ b/guacamole/src/main/frontend/webpack.config.js @@ -77,18 +77,6 @@ module.exports = { ] }, optimization: { - minimizer: [ - - // Minify using Google Closure Compiler - new ClosureWebpackPlugin({ mode: 'STANDARD' }, { - languageIn: 'ECMASCRIPT_2020', - languageOut: 'ECMASCRIPT5', - compilationLevel: 'SIMPLE' - }), - - new CssMinimizerPlugin() - - ], splitChunks: { cacheGroups: { diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/schema/SchemaResource.java b/guacamole/src/main/java/org/apache/guacamole/rest/schema/SchemaResource.java index 9086ac93e..edc125127 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/schema/SchemaResource.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/schema/SchemaResource.java @@ -77,6 +77,26 @@ public class SchemaResource { } + /** + * Retrieves the possible user preference attributes of a user object. + * + * @return + * A collection of forms which describe the possible preference attributes of a + * user object. + * + * @throws GuacamoleException + * If an error occurs while retrieving the possible attributes. + */ + @GET + @Path("userPreferenceAttributes") + public Collection getUserAttrigetUserPreferenceAttributesbutes() + throws GuacamoleException { + + // Retrieve all possible user preference attributes + return userContext.getUserPreferenceAttributes(); + + } + /** * Retrieves the possible attributes of a user group object. * diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java b/guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java index f31ce5dc8..5d4be4ffc 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java @@ -21,6 +21,11 @@ package org.apache.guacamole.rest.user; import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.AssistedInject; + +import java.util.Iterator; +import java.util.Set; +import java.util.stream.Collectors; + import javax.servlet.http.HttpServletRequest; import javax.ws.rs.Consumes; import javax.ws.rs.GET; @@ -145,9 +150,51 @@ public class UserResource @Override public void updateObject(APIUser modifiedObject) throws GuacamoleException { - // A user may not use this endpoint to modify himself - if (userContext.self().getIdentifier().equals(modifiedObject.getUsername())) - throw new GuacamoleSecurityException("Permission denied."); + User currentUser = userContext.self(); + + // A user may not use this endpoint to modify themself, except in the case + // that they are modifying one of the user attributes explicitly exposed + // in the user preferences form + if (currentUser.getIdentifier().equals(modifiedObject.getUsername())) { + + // A user may not use this endpoint to update their password + if (currentUser.getPassword() != null) + throw new GuacamoleSecurityException( + "Permission denied. The password update endpoint must" + + " be used to change the current user's password."); + + // All attributes exposed in the preferences forms + Set preferenceAttributes = ( + userContext.getUserPreferenceAttributes().stream() + .flatMap(form -> form.getFields().stream().map( + field -> field.getName()))) + .collect(Collectors.toSet()); + + // Go through every attribute value and check if it's changed + Iterator keyIterator = modifiedObject.getAttributes().keySet().iterator(); + while(keyIterator.hasNext()) { + + String key = keyIterator.next(); + String newValue = modifiedObject.getAttributes().get(key); + + // If it's not a preference attribute, editing is not allowed + if (!preferenceAttributes.contains(key)) { + + String currentValue = currentUser.getAttributes().get(key); + + // If the value of the attribute has been modified + if ( + !(currentValue == null && newValue == null) && ( + (currentValue == null && newValue != null) || + !currentValue.equals(newValue) + ) + ) + throw new GuacamoleSecurityException( + "Permission denied. Only user preference attributes" + + " can be modified for the current user."); + } + } + } super.updateObject(modifiedObject); From 87cd7fbe2281beff0f86e7f210a7b059ebf7d798 Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Thu, 4 Aug 2022 19:24:49 +0000 Subject: [PATCH 2/8] GUACAMOLE-1656: Remove user attributes header; it does not look good. --- .../java/org/apache/guacamole/vault/user/VaultUserContext.java | 2 +- .../src/app/settings/templates/settingsPreferences.html | 1 - guacamole/src/main/frontend/src/translations/en.json | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/user/VaultUserContext.java b/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/user/VaultUserContext.java index 748237dd2..eeddbe8d1 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/user/VaultUserContext.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/user/VaultUserContext.java @@ -241,7 +241,7 @@ public class VaultUserContext extends TokenInjectingUserContext { * * @throws GuacamoleException * If the value for any applicable secret cannot be retrieved from the - * vault due to an error.1 + * vault due to an error. */ private Map> getTokens( Connectable connectable, Map tokenMapping, diff --git a/guacamole/src/main/frontend/src/app/settings/templates/settingsPreferences.html b/guacamole/src/main/frontend/src/app/settings/templates/settingsPreferences.html index ac5cc4f59..6ecf6e0e7 100644 --- a/guacamole/src/main/frontend/src/app/settings/templates/settingsPreferences.html +++ b/guacamole/src/main/frontend/src/app/settings/templates/settingsPreferences.html @@ -90,7 +90,6 @@ -

{{'SETTINGS_PREFERENCES.SECTION_HEADER_UPDATE_ATTRIBUTES' | translate}}

diff --git a/guacamole/src/main/frontend/src/translations/en.json b/guacamole/src/main/frontend/src/translations/en.json index 7f52fea11..a1c0e0d92 100644 --- a/guacamole/src/main/frontend/src/translations/en.json +++ b/guacamole/src/main/frontend/src/translations/en.json @@ -951,7 +951,6 @@ "SECTION_HEADER_DEFAULT_INPUT_METHOD" : "Default Input Method", "SECTION_HEADER_DEFAULT_MOUSE_MODE" : "Default Mouse Emulation Mode", - "SECTION_HEADER_UPDATE_ATTRIBUTES" : "User Attributes", "SECTION_HEADER_UPDATE_PASSWORD" : "Change Password" }, From 33f2b499efeefa09b912393b09364bc010f1b87d Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Fri, 5 Aug 2022 00:01:12 +0000 Subject: [PATCH 3/8] GUACAMOLE-1656: Fall back to user KSM config for single value fetch. --- .../vault/ksm/GuacamoleExceptionSupplier.java | 42 +++++++++++ .../guacamole/vault/ksm/secret/KsmClient.java | 42 ++++++++++- .../vault/ksm/secret/KsmSecretService.java | 72 ++++++++++++++++--- 3 files changed, 143 insertions(+), 13 deletions(-) create mode 100644 extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/GuacamoleExceptionSupplier.java diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/GuacamoleExceptionSupplier.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/GuacamoleExceptionSupplier.java new file mode 100644 index 000000000..d966a7320 --- /dev/null +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/GuacamoleExceptionSupplier.java @@ -0,0 +1,42 @@ +/* + * 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; + +import org.apache.guacamole.GuacamoleException; + +/** + * A class that is basically equivalent to the standard Supplier class in + * Java, except that the get() function can throw GuacamoleException, which + * is impossible with any of the standard Java lambda type classes, since + * none of them can handle checked exceptions + */ +public abstract class GuacamoleExceptionSupplier { + + /** + * Returns a value of the declared type. + * + * @return + * A value of the declared type. + * + * @throws GuacamoleException + * If an error occurs while attemping to calculate the return value. + */ + public abstract T get() throws GuacamoleException; +} \ No newline at end of file diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java index 6e0bdf177..74a27e472 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java @@ -34,7 +34,6 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -45,10 +44,12 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.annotation.Nullable; + import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.net.auth.User; import org.apache.guacamole.vault.ksm.conf.KsmConfigurationService; import org.apache.guacamole.vault.secret.WindowsUsername; +import org.apache.guacamole.vault.ksm.GuacamoleExceptionSupplier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -595,6 +596,38 @@ public class KsmClient { * is invalid. */ public Future getSecret(String notation) throws GuacamoleException { + return getSecret(notation, null); + } + + /** + * 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 + * If a fallbackFunction is provided, it will be invoked to generate + * a return value in the case where no secrest is found with the given + * keeper notation. + * + * @param notation + * The Keeper notation of the secret to retrieve. + * + * @param fallbackFunction + * A function to invoke in order to produce a Future for return, + * if the requested secret is not found. If the provided Function + * is null, it will not be run. + * + * @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 getSecret( + String notation, + @Nullable GuacamoleExceptionSupplier> fallbackFunction) + throws GuacamoleException { validateCache(); cacheLock.readLock().lock(); try { @@ -614,6 +647,11 @@ public class KsmClient { catch (Error e) { logger.warn("Record \"{}\" does not exist.", notation); logger.debug("Retrieval of record by Keeper notation failed.", e); + + // If the secret is not found, invoke the fallback function + if (fallbackFunction != null) + return fallbackFunction.get(); + return CompletableFuture.completedFuture(null); } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java index a3b772256..6b8ba92a2 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java @@ -48,6 +48,7 @@ import org.apache.guacamole.net.auth.Directory; 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.secret.VaultSecretService; @@ -147,7 +148,24 @@ public class KsmSecretService implements VaultSecretService { // Attempt to find a KSM config for this connection or group String ksmConfig = getConnectionGroupKsmConfig(userContext, connectable); - return getClient(ksmConfig).getSecret(name); + return getClient(ksmConfig).getSecret(name, new GuacamoleExceptionSupplier>() { + + @Override + public Future get() throws GuacamoleException { + + // Get the user-supplied KSM config, if allowed by config and + // set by the user + String userKsmConfig = getUserKSMConfig(userContext, connectable); + + // If the user config happens to be the same as admin-defined one, + // don't bother trying again + if (userKsmConfig != ksmConfig) + return getClient(userKsmConfig).getSecret(name); + + return CompletableFuture.completedFuture(null); + } + + }); } @Override @@ -335,6 +353,44 @@ public class KsmSecretService implements VaultSecretService { } + /** + * Return the KSM config blob for the current user IFF user KSM configs + * are enabled globally, and are enabled for the given connectable. If no + * KSM config exists for the given user or KSM configs are not enabled, + * null will be returned. + * + * @param userContext + * The user context from which the current user should be fetched. + * + * @param connectable + * The connectable to which the connection is being established. This + * is the conneciton which will be checked to see if user KSM configs + * are enabled. + * + * @return + * The base64 encoded KSM config blob for the current user if one + * exists, and if user KSM configs are enabled globally and for the + * provided connectable. + * + * @throws GuacamoleException + * If an error occurs while attempting to fetch the KSM config. + */ + private String getUserKSMConfig( + UserContext userContext, Connectable connectable) throws GuacamoleException { + + // Check if user KSM configs are enabled globally, and for the given connectable + if (confService.getAllowUserConfig() && isKsmUserConfigEnabled(connectable)) + + // Return the user-specific KSM config, if one exists + return userContext.self().getAttributes().get( + KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE); + + + // If user-specific KSM config is disabled globally or for the given + // connectable, return null to indicate that no user config exists + return null; + } + @Override public Map> getTokens(UserContext userContext, Connectable connectable, GuacamoleConfiguration config, TokenFilter filter) throws GuacamoleException { @@ -351,16 +407,9 @@ public class KsmSecretService implements VaultSecretService { // Only use the user-specific KSM config if explicitly enabled in the global // configuration, AND for the specific connectable being connected to - if (confService.getAllowUserConfig() && isKsmUserConfigEnabled(connectable)) { - - // Find a user-specific KSM config, if one exists - String userKsmConfig = userContext.self().getAttributes().get( - KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE); - - // If a user-specific config exsts, process it first - if (userKsmConfig != null && !userKsmConfig.trim().isEmpty()) - ksmClients.add(0, getClient(userKsmConfig)); - } + String userKsmConfig = getUserKSMConfig(userContext, connectable); + if (userKsmConfig != null && !userKsmConfig.trim().isEmpty()) + ksmClients.add(0, getClient(userKsmConfig)); // Iterate through the KSM clients, processing using the user-specific // config first (if it exists), to ensure that any admin-defined values @@ -431,6 +480,7 @@ public class KsmSecretService implements VaultSecretService { addRecordTokens(tokens, "KEEPER_USER_", ksm.getRecordByLogin(filter.filter(username), null)); } + } return tokens; From e882a08486badd527a615ff38fce8bc3520b12de Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Fri, 5 Aug 2022 15:56:09 +0000 Subject: [PATCH 4/8] GUACAMOLE-1656: Ensure the preferences page refreshes on save in case a one-time-token was updated. --- .../directives/guacSettingsPreferences.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js b/guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js index 5824eb194..199b7ddfa 100644 --- a/guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js +++ b/guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js @@ -58,6 +58,20 @@ angular.module('settings').directive('guacSettingsPreferences', [function guacSe } }; + /** + * An action which closes the current dialog, and refreshes + * the user data on dialog close. + */ + const ACKNOWLEDGE_ACTION_RELOAD = { + name : 'SETTINGS_PREFERENCES.ACTION_ACKNOWLEDGE', + // Handle action + callback : function acknowledgeCallback() { + userService.getUser(dataSource, username) + .then(user => $scope.user = user) + .then(guacNotification.showStatus(false)) + } + }; + /** * The user being modified. * @@ -237,7 +251,9 @@ angular.module('settings').directive('guacSettingsPreferences', [function guacSe text : { key : 'SETTINGS_PREFERENCES.INFO_PREFERENCE_ATTRIBUTES_CHANGED' }, - actions : [ ACKNOWLEDGE_ACTION ] + + // Reload the user on successful save in case any attributes changed + actions : [ ACKNOWLEDGE_ACTION_RELOAD ] }), guacNotification.SHOW_REQUEST_ERROR); }; From 3790d76fc9485f5229632d4583976a593679abd3 Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Fri, 5 Aug 2022 18:30:22 +0000 Subject: [PATCH 5/8] GUACAMOLE-1656: Force refresh the user context on updateUserContext to ensure that any modified user attributes are picked up. --- .../JDBCAuthenticationProviderService.java | 45 ++++++++++++++++--- .../guacamole/auth/jdbc/user/UserService.java | 12 +++-- .../vault/ksm/GuacamoleExceptionSupplier.java | 1 + .../controllers/manageUserController.js | 2 - 4 files changed, 48 insertions(+), 12 deletions(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderService.java index d2576ec2b..e276b2766 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderService.java @@ -89,9 +89,31 @@ public class JDBCAuthenticationProviderService implements AuthenticationProvider } - @Override - public ModeledUserContext getUserContext(AuthenticationProvider authenticationProvider, - AuthenticatedUser authenticatedUser) throws GuacamoleException { + /** + * Gets a user context for the given authentication provider and user. If + * forceRefresh is set to true, the user record will be re-fetched even if + * it has already been loaded from the database. If not, the existing + * user will be used. + * + * @param authenticationProvider + * The authentication provider to use when loading or refreshing the user. + * + * @param authenticatedUser + * The user for which the user context is being fetched. + * + * @param forceRefresh + * A flag that, when set to true, will force the authenticated user to + * refreshed from the database. If false, an existing DB user will be + * reused. + * + * @return + * The fetched user context. + * + * @throws GuacamoleException + * If an error occurs while fetching or refreshing the user context. + */ + private ModeledUserContext getUserContext(AuthenticationProvider authenticationProvider, + AuthenticatedUser authenticatedUser, boolean forceRefresh) throws GuacamoleException { // Always allow but provide no data for users authenticated via our own // connection sharing links @@ -102,8 +124,9 @@ public class JDBCAuthenticationProviderService implements AuthenticationProvider boolean databaseCredentialsUsed = (authenticatedUser instanceof ModeledAuthenticatedUser); boolean databaseRestrictionsApplicable = (databaseCredentialsUsed || environment.isUserRequired()); - // Retrieve user account for already-authenticated user - ModeledUser user = userService.retrieveUser(authenticationProvider, authenticatedUser); + // Retrieve user account for already-authenticated user, forcing a refresh if requested + ModeledUser user = userService.retrieveUser( + authenticationProvider, authenticatedUser, forceRefresh); ModeledUserContext context = userContextProvider.get(); if (user != null && !user.isDisabled()) { @@ -159,13 +182,21 @@ public class JDBCAuthenticationProviderService implements AuthenticationProvider } + @Override + public ModeledUserContext getUserContext(AuthenticationProvider authenticationProvider, + AuthenticatedUser authenticatedUser) throws GuacamoleException { + + // Do not force refresh unless updateUserContext is explicitly called + return getUserContext(authenticationProvider, authenticatedUser, false); + } + @Override public UserContext updateUserContext(AuthenticationProvider authenticationProvider, UserContext context, AuthenticatedUser authenticatedUser, Credentials credentials) throws GuacamoleException { - // No need to update the context - return context; + // Force-refresh the user context + return getUserContext(authenticationProvider, authenticatedUser, true); } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java index c43aa1b01..3d23e64b0 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java @@ -404,6 +404,11 @@ public class UserService extends ModeledDirectoryObjectService { * If an error occurs while attemping to calculate the return value. */ public abstract T get() throws GuacamoleException; + } \ No newline at end of file diff --git a/guacamole/src/main/frontend/src/app/manage/controllers/manageUserController.js b/guacamole/src/main/frontend/src/app/manage/controllers/manageUserController.js index 94e3d8944..f7ead136a 100644 --- a/guacamole/src/main/frontend/src/app/manage/controllers/manageUserController.js +++ b/guacamole/src/main/frontend/src/app/manage/controllers/manageUserController.js @@ -501,6 +501,4 @@ angular.module('manage').controller('manageUserController', ['$scope', '$injecto return userService.deleteUser($scope.dataSource, $scope.user); }; - console.log($scope); - }]); From dfc7e6dd90bffacc6f0e52ed818da33bed63dcf2 Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Mon, 8 Aug 2022 21:38:45 +0000 Subject: [PATCH 6/8] GUACAMOLE-1656: Simplify auto-refresh behavior in JDBC auth provider. --- .../JDBCAuthenticationProviderService.java | 45 +++---------------- .../guacamole/auth/jdbc/user/UserService.java | 12 +---- 2 files changed, 8 insertions(+), 49 deletions(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderService.java index e276b2766..2b130b610 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderService.java @@ -89,31 +89,9 @@ public class JDBCAuthenticationProviderService implements AuthenticationProvider } - /** - * Gets a user context for the given authentication provider and user. If - * forceRefresh is set to true, the user record will be re-fetched even if - * it has already been loaded from the database. If not, the existing - * user will be used. - * - * @param authenticationProvider - * The authentication provider to use when loading or refreshing the user. - * - * @param authenticatedUser - * The user for which the user context is being fetched. - * - * @param forceRefresh - * A flag that, when set to true, will force the authenticated user to - * refreshed from the database. If false, an existing DB user will be - * reused. - * - * @return - * The fetched user context. - * - * @throws GuacamoleException - * If an error occurs while fetching or refreshing the user context. - */ - private ModeledUserContext getUserContext(AuthenticationProvider authenticationProvider, - AuthenticatedUser authenticatedUser, boolean forceRefresh) throws GuacamoleException { + @Override + public ModeledUserContext getUserContext(AuthenticationProvider authenticationProvider, + AuthenticatedUser authenticatedUser) throws GuacamoleException { // Always allow but provide no data for users authenticated via our own // connection sharing links @@ -124,9 +102,8 @@ public class JDBCAuthenticationProviderService implements AuthenticationProvider boolean databaseCredentialsUsed = (authenticatedUser instanceof ModeledAuthenticatedUser); boolean databaseRestrictionsApplicable = (databaseCredentialsUsed || environment.isUserRequired()); - // Retrieve user account for already-authenticated user, forcing a refresh if requested - ModeledUser user = userService.retrieveUser( - authenticationProvider, authenticatedUser, forceRefresh); + // Retrieve user account for already-authenticated user + ModeledUser user = userService.retrieveUser(authenticationProvider, authenticatedUser); ModeledUserContext context = userContextProvider.get(); if (user != null && !user.isDisabled()) { @@ -182,21 +159,13 @@ public class JDBCAuthenticationProviderService implements AuthenticationProvider } - @Override - public ModeledUserContext getUserContext(AuthenticationProvider authenticationProvider, - AuthenticatedUser authenticatedUser) throws GuacamoleException { - - // Do not force refresh unless updateUserContext is explicitly called - return getUserContext(authenticationProvider, authenticatedUser, false); - } - @Override public UserContext updateUserContext(AuthenticationProvider authenticationProvider, UserContext context, AuthenticatedUser authenticatedUser, Credentials credentials) throws GuacamoleException { - // Force-refresh the user context - return getUserContext(authenticationProvider, authenticatedUser, true); + // Refresh the user context + return getUserContext(authenticationProvider, authenticatedUser); } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java index 3d23e64b0..161976ce4 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java @@ -404,11 +404,6 @@ public class UserService extends ModeledDirectoryObjectService Date: Mon, 29 Aug 2022 22:08:31 +0000 Subject: [PATCH 7/8] GUACAMOLE-1656: Simplify, clean up, and improve documentation of KSM code. --- .../vault/ksm/GuacamoleExceptionSupplier.java | 8 +- .../vault/ksm/conf/KsmAttributeService.java | 22 +- .../guacamole/vault/ksm/secret/KsmClient.java | 2 +- .../vault/ksm/secret/KsmSecretService.java | 205 ++++++++++-------- .../vault/ksm/user/KsmConnection.java | 29 +-- .../vault/ksm/user/KsmConnectionGroup.java | 17 +- .../vault/ksm/user/KsmDirectoryService.java | 29 +-- .../guacamole/vault/ksm/user/KsmUser.java | 29 +-- .../directives/guacSettingsPreferences.js | 70 +----- .../main/frontend/src/translations/en.json | 2 +- guacamole/src/main/frontend/webpack.config.js | 12 + .../guacamole/rest/schema/SchemaResource.java | 2 +- .../rest/user/UserObjectTranslator.java | 19 +- .../guacamole/rest/user/UserResource.java | 54 +---- 14 files changed, 216 insertions(+), 284 deletions(-) diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/GuacamoleExceptionSupplier.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/GuacamoleExceptionSupplier.java index ed59f7b68..c99613740 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/GuacamoleExceptionSupplier.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/GuacamoleExceptionSupplier.java @@ -26,8 +26,12 @@ import org.apache.guacamole.GuacamoleException; * Java, except that the get() function can throw GuacamoleException, which * is impossible with any of the standard Java lambda type classes, since * none of them can handle checked exceptions + * + * @param + * The type of object which will be returned as a result of calling + * get(). */ -public abstract class GuacamoleExceptionSupplier { +public interface GuacamoleExceptionSupplier { /** * Returns a value of the declared type. @@ -38,6 +42,6 @@ public abstract class GuacamoleExceptionSupplier { * @throws GuacamoleException * If an error occurs while attemping to calculate the return value. */ - public abstract T get() throws GuacamoleException; + public T get() throws GuacamoleException; } \ No newline at end of file diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java index bdceb4c21..24958208d 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java @@ -28,6 +28,8 @@ import org.apache.guacamole.form.BooleanField; import org.apache.guacamole.form.Form; import org.apache.guacamole.form.TextField; import org.apache.guacamole.vault.conf.VaultAttributeService; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -39,7 +41,14 @@ import com.google.inject.Singleton; @Singleton public class KsmAttributeService implements VaultAttributeService { + /** + * Logger for this class. + */ + private static final Logger logger = LoggerFactory.getLogger(KsmAttributeService.class); + /** + * Service for retrieving KSM configuration details. + */ @Inject private KsmConfigurationService configurationService; @@ -57,7 +66,7 @@ public class KsmAttributeService implements VaultAttributeService { Arrays.asList(new TextField(KSM_CONFIGURATION_ATTRIBUTE))); /** - * All KSM-specific attributes for users or connection groups, organized by form. + * All KSM-specific attributes for users, connections, or connection groups, organized by form. */ public static final Collection KSM_ATTRIBUTES = Collections.unmodifiableCollection(Arrays.asList(KSM_CONFIGURATION_FORM)); @@ -108,7 +117,16 @@ public class KsmAttributeService implements VaultAttributeService { // Expose the user attributes IFF user-level KSM configuration is enabled return configurationService.getAllowUserConfig() ? KSM_ATTRIBUTES : Collections.emptyList(); - } catch (GuacamoleException e) { + } + + catch (GuacamoleException e) { + + logger.warn( + "Unable to determine if user preference attributes " + + "should be exposed due to config parsing error: {}.", e.getMessage()); + logger.debug( + "Config parsing error prevented checking user preference configuration", + e); // If the configuration can't be parsed, default to not exposing the attribute return Collections.emptyList(); diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java index 74a27e472..3aa436f69 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java @@ -605,7 +605,7 @@ public class KsmClient { * 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 * If a fallbackFunction is provided, it will be invoked to generate - * a return value in the case where no secrest is found with the given + * a return value in the case where no secret is found with the given * keeper notation. * * @param notation diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java index 6b8ba92a2..59c1450bf 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java @@ -19,6 +19,7 @@ package org.apache.guacamole.vault.ksm.secret; +import com.google.common.base.Objects; import com.google.inject.Inject; import com.google.inject.Singleton; import com.keepersecurity.secretsManager.core.KeeperRecord; @@ -41,6 +42,7 @@ import java.util.concurrent.Future; import javax.annotation.Nonnull; import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.net.auth.Attributes; import org.apache.guacamole.net.auth.Connectable; import org.apache.guacamole.net.auth.Connection; import org.apache.guacamole.net.auth.ConnectionGroup; @@ -159,7 +161,7 @@ public class KsmSecretService implements VaultSecretService { // If the user config happens to be the same as admin-defined one, // don't bother trying again - if (userKsmConfig != ksmConfig) + if (!Objects.equal(userKsmConfig, ksmConfig)) return getClient(userKsmConfig).getSecret(name); return CompletableFuture.completedFuture(null); @@ -339,16 +341,12 @@ public class KsmSecretService implements VaultSecretService { */ private boolean isKsmUserConfigEnabled(Connectable connectable) { - // If it's a connection, user-level config is enabled IFF the appropriate - // attribute is set to true - if (connectable instanceof Connection) - return KsmAttributeService.TRUTH_VALUE.equals(((Connection) connectable).getAttributes().get( + // User-level config is enabled IFF the appropriate attribute is set to true + if (connectable instanceof Attributes) + return KsmAttributeService.TRUTH_VALUE.equals(((Attributes) connectable).getAttributes().get( KsmAttributeService.KSM_USER_CONFIG_ENABLED_ATTRIBUTE)); - // KSM token replacement is not enabled for balancing groups, so for - // now, user-level KSM configs will be explicitly disabled. - // TODO: If token replacement is implemented for balancing groups, - // implement this functionality for them as well. + // If there's no attributes to check, the user config cannot be enabled return false; } @@ -378,10 +376,10 @@ public class KsmSecretService implements VaultSecretService { private String getUserKSMConfig( UserContext userContext, Connectable connectable) throws GuacamoleException { - // Check if user KSM configs are enabled globally, and for the given connectable + // 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)) - // Return the user-specific KSM config, if one exists return userContext.self().getAttributes().get( KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE); @@ -391,6 +389,106 @@ public class KsmSecretService implements VaultSecretService { return null; } + /** + * Use the provided KSM client to add parameter tokens tokens to the + * provided token map. The supplied filter will be used to replace + * existing tokens in the provided connection parameters before KSM + * record lookup. The supplied GuacamoleConfiguration instance will + * be used to check the protocol, in case RDP-specific behavior is + * needed. + + * @param config + * The GuacamoleConfiguration associated with the Connectable for which + * tokens are being added. + * + * @param ksm + * The KSM client to use when fetching records. + * + * @param tokens + * The tokens to which any fetched KSM record values should be added. + * + * @param parameters + * The connection parameters associated with the Connectable for which + * tokens are being added. + * + * @throws GuacamoleException + * If an error occurs while attempting to fetch KSM records or check + * configuration settings. + */ + private void addConnectableTokens( + GuacamoleConfiguration config, KsmClient ksm, Map> tokens, + Map parameters, TokenFilter filter) throws GuacamoleException { + + // Retrieve and define server-specific tokens, if any + String hostname = parameters.get("hostname"); + if (hostname != null && !hostname.isEmpty()) + addRecordTokens(tokens, "KEEPER_SERVER_", + ksm.getRecordByHost(filter.filter(hostname))); + + // Tokens specific to RDP + if ("rdp".equals(config.getProtocol())) { + + // Retrieve and define gateway server-specific tokens, if any + String gatewayHostname = parameters.get("gateway-hostname"); + if (gatewayHostname != null && !gatewayHostname.isEmpty()) + addRecordTokens(tokens, "KEEPER_GATEWAY_", + ksm.getRecordByHost(filter.filter(gatewayHostname))); + + // Retrieve and define domain tokens, if any + String domain = parameters.get("domain"); + String filteredDomain = null; + if (domain != null && !domain.isEmpty()) { + filteredDomain = filter.filter(domain); + addRecordTokens(tokens, "KEEPER_DOMAIN_", + ksm.getRecordByDomain(filteredDomain)); + } + + // Retrieve and define gateway domain tokens, if any + String gatewayDomain = parameters.get("gateway-domain"); + String filteredGatewayDomain = null; + if (gatewayDomain != null && !gatewayDomain.isEmpty()) { + filteredGatewayDomain = filter.filter(gatewayDomain); + addRecordTokens(tokens, "KEEPER_GATEWAY_DOMAIN_", + ksm.getRecordByDomain(filteredGatewayDomain)); + } + + // If domain matching is disabled for user records, + // explicitly set the domains to null when storing + // user records to enable username-only matching + if (!confService.getMatchUserRecordsByDomain()) { + filteredDomain = null; + filteredGatewayDomain = null; + } + + // Retrieve and define user-specific tokens, if any + String username = parameters.get("username"); + if (username != null && !username.isEmpty()) + addRecordTokens(tokens, "KEEPER_USER_", + ksm.getRecordByLogin(filter.filter(username), + filteredDomain)); + + // Retrieve and define gateway user-specific tokens, if any + String gatewayUsername = parameters.get("gateway-username"); + if (gatewayUsername != null && !gatewayUsername.isEmpty()) + addRecordTokens(tokens, "KEEPER_GATEWAY_USER_", + ksm.getRecordByLogin( + filter.filter(gatewayUsername), + filteredGatewayDomain)); + } + + else { + + // Retrieve and define user-specific tokens, if any + // NOTE that non-RDP connections do not have a domain + // field in the connection parameters, so the domain + // will always be null + String username = parameters.get("username"); + if (username != null && !username.isEmpty()) + addRecordTokens(tokens, "KEEPER_USER_", + ksm.getRecordByLogin(filter.filter(username), null)); + } + } + @Override public Map> getTokens(UserContext userContext, Connectable connectable, GuacamoleConfiguration config, TokenFilter filter) throws GuacamoleException { @@ -398,89 +496,18 @@ public class KsmSecretService implements VaultSecretService { Map> tokens = new HashMap<>(); Map parameters = config.getParameters(); - // Attempt to find a KSM config for this connection or group - String ksmConfig = getConnectionGroupKsmConfig(userContext, connectable); - - // Create a list containing just the global / connection group config - List ksmClients = new ArrayList<>(2); - ksmClients.add(getClient(ksmConfig)); - // Only use the user-specific KSM config if explicitly enabled in the global // configuration, AND for the specific connectable being connected to String userKsmConfig = getUserKSMConfig(userContext, connectable); if (userKsmConfig != null && !userKsmConfig.trim().isEmpty()) - ksmClients.add(0, getClient(userKsmConfig)); + addConnectableTokens( + config, getClient(userKsmConfig), tokens, parameters, filter); - // Iterate through the KSM clients, processing using the user-specific - // config first (if it exists), to ensure that any admin-defined values - // will override the user-speicifc values - Iterator ksmIterator = ksmClients.iterator(); - while (ksmIterator.hasNext()) { - - KsmClient ksm = ksmIterator.next(); - - // Retrieve and define server-specific tokens, if any - String hostname = parameters.get("hostname"); - if (hostname != null && !hostname.isEmpty()) - addRecordTokens(tokens, "KEEPER_SERVER_", - ksm.getRecordByHost(filter.filter(hostname))); - - // Tokens specific to RDP - if ("rdp".equals(config.getProtocol())) { - // Retrieve and define domain tokens, if any - String domain = parameters.get("domain"); - String filteredDomain = null; - if (domain != null && !domain.isEmpty()) { - filteredDomain = filter.filter(domain); - addRecordTokens(tokens, "KEEPER_DOMAIN_", - ksm.getRecordByDomain(filteredDomain)); - } - - // Retrieve and define gateway domain tokens, if any - String gatewayDomain = parameters.get("gateway-domain"); - String filteredGatewayDomain = null; - if (gatewayDomain != null && !gatewayDomain.isEmpty()) { - filteredGatewayDomain = filter.filter(gatewayDomain); - addRecordTokens(tokens, "KEEPER_GATEWAY_DOMAIN_", - ksm.getRecordByDomain(filteredGatewayDomain)); - } - - // If domain matching is disabled for user records, - // explicitly set the domains to null when storing - // user records to enable username-only matching - if (!confService.getMatchUserRecordsByDomain()) { - filteredDomain = null; - filteredGatewayDomain = null; - } - - // Retrieve and define user-specific tokens, if any - String username = parameters.get("username"); - if (username != null && !username.isEmpty()) - addRecordTokens(tokens, "KEEPER_USER_", - ksm.getRecordByLogin(filter.filter(username), - filteredDomain)); - - // Retrieve and define gateway user-specific tokens, if any - String gatewayUsername = parameters.get("gateway-username"); - if (gatewayUsername != null && !gatewayUsername.isEmpty()) - addRecordTokens(tokens, "KEEPER_GATEWAY_USER_", - ksm.getRecordByLogin( - filter.filter(gatewayUsername), - filteredGatewayDomain)); - } - - else { - - // Retrieve and define user-specific tokens, if any - // NOTE that non-RDP connections do not have a domain - // field in the connection parameters, so the domain - // will always be null - String username = parameters.get("username"); - if (username != null && !username.isEmpty()) - addRecordTokens(tokens, "KEEPER_USER_", - ksm.getRecordByLogin(filter.filter(username), null)); - } - } + // Add connection group or globally defined tokens after the user-specific + // ones to ensure that the user config will be overriden on collision + String ksmConfig = getConnectionGroupKsmConfig(userContext, connectable); + addConnectableTokens( + config, getClient(ksmConfig), tokens, parameters, filter); return tokens; diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java index da2866202..24d7dd062 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java @@ -19,25 +19,21 @@ package org.apache.guacamole.vault.ksm.user; -import java.util.List; import java.util.Map; import org.apache.guacamole.net.auth.DelegatingConnection; +import org.apache.guacamole.vault.ksm.conf.KsmAttributeService; import org.apache.guacamole.net.auth.Connection; import com.google.common.collect.Maps; /** - * A Connection that explicitly adds a blank entry for any defined - * KSM connection attributes. + * A Connection that explicitly adds a blank entry for any defined KSM + * connection attributes. This ensures that any such field will always + * be displayed to the user when editing a connection through the UI. */ public class KsmConnection extends DelegatingConnection { - /** - * The names of all connection attributes defined for the vault. - */ - private List connectionAttributeNames; - /** * Create a new Vault connection wrapping the provided Connection record. Any * attributes defined in the provided connection attribute forms will have empty @@ -45,15 +41,9 @@ public class KsmConnection extends DelegatingConnection { * * @param connection * The connection record to wrap. - * - * @param connectionAttributeNames - * The names of all connection attributes to automatically expose. */ - KsmConnection(Connection connection, List connectionAttributeNames) { - + KsmConnection(Connection connection) { super(connection); - this.connectionAttributeNames = connectionAttributeNames; - } /** @@ -70,13 +60,12 @@ public class KsmConnection extends DelegatingConnection { public Map getAttributes() { // Make a copy of the existing map - Map attributeMap = Maps.newHashMap(super.getAttributes()); + Map attributes = Maps.newHashMap(super.getAttributes()); - // Add every defined attribute - connectionAttributeNames.forEach( - attributeName -> attributeMap.putIfAbsent(attributeName, null)); + // Add the configuration attribute + attributes.putIfAbsent(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, null); + return attributes; - return attributeMap; } } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnectionGroup.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnectionGroup.java index 397c42f11..175e4dd5f 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnectionGroup.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnectionGroup.java @@ -19,13 +19,14 @@ package org.apache.guacamole.vault.ksm.user; -import java.util.HashMap; import java.util.Map; import org.apache.guacamole.net.auth.ConnectionGroup; import org.apache.guacamole.net.auth.DelegatingConnectionGroup; import org.apache.guacamole.vault.ksm.conf.KsmAttributeService; +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. @@ -50,17 +51,11 @@ import org.apache.guacamole.vault.ksm.conf.KsmAttributeService; @Override public Map getAttributes() { - // All attributes defined on the underlying connection group - Map attributes = super.getAttributes(); + // Make a copy of the existing map + Map attributes = Maps.newHashMap(super.getAttributes()); - // If the attribute is already present, there's no need to add it - return - // the existing attributes as they are - if (attributes.containsKey(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE)) - return attributes; - - // Make a copy of the existing attributes and add KSM_CONFIGURATION_ATTRIBUTE - attributes = new HashMap<>(attributes); - attributes.put(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, null); + // Add the configuration attribute + attributes.putIfAbsent(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, null); return attributes; } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java index 3eff928fe..6ce734601 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java @@ -26,7 +26,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.language.TranslatableGuacamoleClientException; @@ -60,12 +59,6 @@ public class KsmDirectoryService extends VaultDirectoryService { @Inject private KsmConfigurationService configurationService; - /** - * Service for retrieving KSM-specific attributes. - */ - @Inject - private KsmAttributeService ksmAttributeService; - /** * A singleton ObjectMapper for converting a Map to a JSON string when * generating a base64-encoded JSON KSM config blob. @@ -235,8 +228,9 @@ public class KsmDirectoryService extends VaultDirectoryService { public Directory getConnectionDirectory( Directory underlyingDirectory) throws GuacamoleException { - // A Connection directory that will intercept add and update calls to - // validate KSM configurations, and translate one-time-tokens, if possible + // 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(underlyingDirectory) { @Override @@ -244,17 +238,13 @@ public class KsmDirectoryService extends VaultDirectoryService { // Wrap in a KsmConnection class to ensure that all defined KSM fields will be // present - return new KsmConnection( - connection, - ksmAttributeService.getConnectionAttributes().stream().flatMap( - form -> form.getFields().stream().map(field -> field.getName()) - ).collect(Collectors.toList())); + return new KsmConnection(connection); } @Override protected Connection undecorate(Connection connection) throws GuacamoleException { - // Unwrap the KsmUser + // Unwrap the KsmConnection return ((KsmConnection) connection).getUnderlyingConnection(); } @@ -315,6 +305,9 @@ public class KsmDirectoryService extends VaultDirectoryService { // A User directory that will intercept add and update calls to // 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. return new DecoratingDirectory(underlyingDirectory) { @Override @@ -340,11 +333,7 @@ public class KsmDirectoryService extends VaultDirectoryService { // Wrap in a KsmUser class to ensure that all defined KSM fields will be // present - return new KsmUser( - user, - ksmAttributeService.getUserAttributes().stream().flatMap( - form -> form.getFields().stream().map(field -> field.getName()) - ).collect(Collectors.toList())); + return new KsmUser(user); } @Override diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java index a846eb0d8..dc2877564 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java @@ -19,11 +19,11 @@ package org.apache.guacamole.vault.ksm.user; -import java.util.List; import java.util.Map; import org.apache.guacamole.net.auth.DelegatingUser; import org.apache.guacamole.net.auth.User; +import org.apache.guacamole.vault.ksm.conf.KsmAttributeService; import com.google.common.collect.Maps; @@ -33,27 +33,16 @@ import com.google.common.collect.Maps; */ public class KsmUser extends DelegatingUser { - /** - * The names of all user attributes defined for the vault. - */ - private List userAttributeNames; - /** * Create a new Vault user wrapping the provided User record. Any - * attributes defined in the provided user attribute forms will have empty - * values automatically populated when getAttributes() is called. + * KSM-specific attribute forms will have empty values automatically + * populated when getAttributes() is called. * * @param user * The user record to wrap. - * - * @param userAttributeNames - * The names of all user attributes to automatically expose. */ - KsmUser(User user, List userAttributeNames) { - + KsmUser(User user) { super(user); - this.userAttributeNames = userAttributeNames; - } /** @@ -70,13 +59,11 @@ public class KsmUser extends DelegatingUser { public Map getAttributes() { // Make a copy of the existing map - Map attributeMap = Maps.newHashMap(super.getAttributes()); + Map attributes = Maps.newHashMap(super.getAttributes()); - // Add every defined attribute - userAttributeNames.forEach( - attributeName -> attributeMap.putIfAbsent(attributeName, null)); - - return attributeMap; + // Add the configuration attribute + attributes.putIfAbsent(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, null); + return attributes; } } diff --git a/guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js b/guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js index 199b7ddfa..a3f0fd2e5 100644 --- a/guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js +++ b/guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js @@ -68,7 +68,7 @@ angular.module('settings').directive('guacSettingsPreferences', [function guacSe callback : function acknowledgeCallback() { userService.getUser(dataSource, username) .then(user => $scope.user = user) - .then(guacNotification.showStatus(false)) + .then(() => guacNotification.showStatus(false)); } }; @@ -101,17 +101,6 @@ angular.module('settings').directive('guacSettingsPreferences', [function guacSe */ $scope.preferences = preferenceService.preferences; - /** - * All available user attributes, as a mapping of form name to form - * object. The form object contains a name, as well as a Map of fields. - * - * The Map type is used here to maintain form/name uniqueness, as well as - * insertion order, to ensure a consistent UI experience. - * - * @type Map - */ - $scope.attributeMap = new Map(); - /** * All available user attributes. This is only the set of attribute * definitions, organized as logical groupings of attributes, not attribute @@ -263,61 +252,10 @@ angular.module('settings').directive('guacSettingsPreferences', [function guacSe $scope.user = user; }) - // Get all datasources that are available for this user - authenticationService.getAvailableDataSources().forEach(function loadAttributesForDataSource(dataSource) { - - // Fetch all user attribute forms defined for the datasource - schemaService.getUserPreferenceAttributes(dataSource).then(function saveAttributes(attributes) { - - // Iterate through all attribute forms - attributes.forEach(function addAttribute(attributeForm) { - - // If the form with the retrieved name already exists - if ($scope.attributeMap.has(attributeForm.name)) { - const existingFields = $scope.attributeMap.get(attributeForm.name).fields; - - // Add each field to the existing list for this form - attributeForm.fields.forEach(function addAllFieldsToExistingMap(field) { - existingFields.set(field.name, field); - }) - } - - else { - - // Create a new entry for the form - $scope.attributeMap.set(attributeForm.name, { - name: attributeForm.name, - - // With the field array from the API converted into a Map - fields: attributeForm.fields.reduce( - function addFieldToMap(currentFieldMap, field) { - currentFieldMap.set(field.name, field); - return currentFieldMap; - }, new Map() - ) - - }) - } - - }); - - // Re-generate the attributes array every time - $scope.attributes = Array.of(...$scope.attributeMap.values()).map(function convertFieldsToArray(formObject) { - - // Convert each temporary form object to a Form type - return new Form({ - name: formObject.name, - - // Convert the field map to a simple array of fields - fields: Array.of(...formObject.fields.values()) - }) - }); - - }); - + // Fetch all user preference attribute forms defined + schemaService.getUserPreferenceAttributes(dataSource).then(function saveAttributes(attributes) { + $scope.attributes = attributes; }); - }] }; - }]); diff --git a/guacamole/src/main/frontend/src/translations/en.json b/guacamole/src/main/frontend/src/translations/en.json index a1c0e0d92..f2ba0b111 100644 --- a/guacamole/src/main/frontend/src/translations/en.json +++ b/guacamole/src/main/frontend/src/translations/en.json @@ -943,7 +943,7 @@ "HELP_UPDATE_PASSWORD" : "If you wish to change your password, enter your current password and the desired new password below, and click \"Update Password\". The change will take effect immediately.", "INFO_PASSWORD_CHANGED" : "Password changed.", - "INFO_PREFERENCE_ATTRIBUTES_CHANGED" : "User attributes saved.", + "INFO_PREFERENCE_ATTRIBUTES_CHANGED" : "User settings saved.", "NAME_INPUT_METHOD_NONE" : "@:CLIENT.NAME_INPUT_METHOD_NONE", "NAME_INPUT_METHOD_OSK" : "@:CLIENT.NAME_INPUT_METHOD_OSK", diff --git a/guacamole/src/main/frontend/webpack.config.js b/guacamole/src/main/frontend/webpack.config.js index 3f1574fa1..29bb8ddd5 100644 --- a/guacamole/src/main/frontend/webpack.config.js +++ b/guacamole/src/main/frontend/webpack.config.js @@ -77,6 +77,18 @@ module.exports = { ] }, optimization: { + minimizer: [ + + // Minify using Google Closure Compiler + new ClosureWebpackPlugin({ mode: 'STANDARD' }, { + languageIn: 'ECMASCRIPT_2020', + languageOut: 'ECMASCRIPT5', + compilationLevel: 'SIMPLE' + }), + + new CssMinimizerPlugin() + + ], splitChunks: { cacheGroups: { diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/schema/SchemaResource.java b/guacamole/src/main/java/org/apache/guacamole/rest/schema/SchemaResource.java index edc125127..a7ed08d60 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/schema/SchemaResource.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/schema/SchemaResource.java @@ -89,7 +89,7 @@ public class SchemaResource { */ @GET @Path("userPreferenceAttributes") - public Collection getUserAttrigetUserPreferenceAttributesbutes() + public Collection getUserPreferenceAttributes() throws GuacamoleException { // Retrieve all possible user preference attributes diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/user/UserObjectTranslator.java b/guacamole/src/main/java/org/apache/guacamole/rest/user/UserObjectTranslator.java index 8536b3507..8c63d7218 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/user/UserObjectTranslator.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/user/UserObjectTranslator.java @@ -59,9 +59,22 @@ public class UserObjectTranslator public void filterExternalObject(UserContext userContext, APIUser object) throws GuacamoleException { - // Filter object attributes by defined schema - object.setAttributes(filterAttributes(userContext.getUserAttributes(), - object.getAttributes())); + // If a user is editing themselves ... + if (object.getUsername().equals(userContext.self().getIdentifier())) { + + // ... they may only edit preference attributes + object.setAttributes(filterAttributes(userContext.getUserPreferenceAttributes(), + object.getAttributes())); + + } + + else { + + // In all other cases, filter object attributes by defined schema + object.setAttributes(filterAttributes(userContext.getUserAttributes(), + object.getAttributes())); + + } } diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java b/guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java index 5d4be4ffc..aa8a3ec7e 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java @@ -22,10 +22,6 @@ package org.apache.guacamole.rest.user; import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.AssistedInject; -import java.util.Iterator; -import java.util.Set; -import java.util.stream.Collectors; - import javax.servlet.http.HttpServletRequest; import javax.ws.rs.Consumes; import javax.ws.rs.GET; @@ -150,50 +146,14 @@ public class UserResource @Override public void updateObject(APIUser modifiedObject) throws GuacamoleException { + // A user may not use this endpoint to update their password User currentUser = userContext.self(); - - // A user may not use this endpoint to modify themself, except in the case - // that they are modifying one of the user attributes explicitly exposed - // in the user preferences form - if (currentUser.getIdentifier().equals(modifiedObject.getUsername())) { - - // A user may not use this endpoint to update their password - if (currentUser.getPassword() != null) - throw new GuacamoleSecurityException( - "Permission denied. The password update endpoint must" - + " be used to change the current user's password."); - - // All attributes exposed in the preferences forms - Set preferenceAttributes = ( - userContext.getUserPreferenceAttributes().stream() - .flatMap(form -> form.getFields().stream().map( - field -> field.getName()))) - .collect(Collectors.toSet()); - - // Go through every attribute value and check if it's changed - Iterator keyIterator = modifiedObject.getAttributes().keySet().iterator(); - while(keyIterator.hasNext()) { - - String key = keyIterator.next(); - String newValue = modifiedObject.getAttributes().get(key); - - // If it's not a preference attribute, editing is not allowed - if (!preferenceAttributes.contains(key)) { - - String currentValue = currentUser.getAttributes().get(key); - - // If the value of the attribute has been modified - if ( - !(currentValue == null && newValue == null) && ( - (currentValue == null && newValue != null) || - !currentValue.equals(newValue) - ) - ) - throw new GuacamoleSecurityException( - "Permission denied. Only user preference attributes" - + " can be modified for the current user."); - } - } + if ( + currentUser.getIdentifier().equals(modifiedObject.getUsername()) + && modifiedObject.getPassword() != null) { + throw new GuacamoleSecurityException( + "Permission denied. The password update endpoint must" + + " be used to change the current user's password."); } super.updateObject(modifiedObject); From 06d321fe5dde0cfebee17ef6d21f8267a078ddff Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Tue, 27 Sep 2022 17:09:25 +0000 Subject: [PATCH 8/8] GUCAMOLE-1656: Do not expose the KSM config blob through the REST API. --- .../ksm/KsmAuthenticationProviderModule.java | 8 + .../vault/ksm/conf/KsmAttributeService.java | 215 ++++++++++++++ .../vault/ksm/secret/KsmSecretService.java | 19 +- .../vault/ksm/user/KsmConnection.java | 5 +- .../vault/ksm/user/KsmConnectionGroup.java | 49 ++-- .../vault/ksm/user/KsmDirectory.java | 93 ++++++ .../vault/ksm/user/KsmDirectoryService.java | 269 ++++-------------- .../guacamole/vault/ksm/user/KsmUser.java | 74 ++++- .../vault/ksm/user/KsmUserFactory.java | 40 +++ 9 files changed, 519 insertions(+), 253 deletions(-) create mode 100644 extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectory.java create mode 100644 extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUserFactory.java diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/KsmAuthenticationProviderModule.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/KsmAuthenticationProviderModule.java index faf22aa4e..b9d38da93 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/KsmAuthenticationProviderModule.java +++ b/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)); } } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java index 24958208d..c1d3b95d5 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java +++ b/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 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 processAttributes( + Map 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 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; + + } + } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java index 59c1450bf..515c1fc3d 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java +++ b/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 observedIdentifiers = new HashSet<>(); observedIdentifiers.add(parentIdentifier); - Directory connectionGroupDirectory = userContext.getConnectionGroupDirectory(); + // Use the unwrapped connection group directory to avoid KSM config + // value sanitization + Directory connectionGroupDirectory = ( + (KsmDirectory) 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) 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 diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java index 24d7dd062..3981486fe 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnection.java +++ b/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 attributes = Maps.newHashMap(super.getAttributes()); - // Add the configuration attribute - attributes.putIfAbsent(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, null); + // Add the user-config-enabled configuration attribute + attributes.putIfAbsent(KsmAttributeService.KSM_USER_CONFIG_ENABLED_ATTRIBUTE, null); return attributes; - } } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnectionGroup.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnectionGroup.java index 175e4dd5f..68d445771 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmConnectionGroup.java +++ b/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 - * for connection groups that don't already have it set. + * The value of the attribute will be sanitized if non-empty. This ensures + * 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 - * ConnectionGroup. + * Create a new KsmConnectionGroup wrapping the provided ConnectionGroup record. * * @param connectionGroup - * The ConnectionGroup instance to wrap. + * The ConnectionGroup record to wrap. */ - public KsmConnectionGroup(ConnectionGroup connectionGroup) { - - // Wrap the provided connection group + KsmConnectionGroup(ConnectionGroup connectionGroup) { super(connectionGroup); } - @Override - public Map getAttributes() { - - // Make a copy of the existing map - Map attributes = Maps.newHashMap(super.getAttributes()); - - // Add the configuration attribute - attributes.putIfAbsent(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, null); - return attributes; - + /** + * Return the underlying wrapped connection group record. + * + * @return + * The wrapped connection group record. + */ + ConnectionGroup getUnderlyingConnectionGroup() { + return getDelegateConnectionGroup(); } /** @@ -70,4 +67,20 @@ import com.google.common.collect.Maps; return getDelegateConnectionGroup(); } + @Override + public Map getAttributes() { + + // Make a copy of the existing map + Map 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; + } + } \ No newline at end of file diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectory.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectory.java new file mode 100644 index 000000000..5cfcca947 --- /dev/null +++ b/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 + extends DelegatingDirectory { + + /** + * Create a new KsmDirectory, delegating to the provided directory. + * + * @param directory + * The directory to delegate to. + */ + public KsmDirectory(Directory directory) { + super(directory); + } + + /** + * Returns the underlying directory that this DecoratingDirectory is + * delegating to. + * + * @return + * The underlying directory. + */ + public Directory 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 getAll(Collection identifiers) + throws GuacamoleException { + + // Process and return each object from the wrapped directory + return super.getAll(identifiers).stream().map( + superObject -> wrap(superObject) + ).collect(Collectors.toList()); + + } + +} diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java index 6ce734601..5203a06dd 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java +++ b/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 - * generating a base64-encoded JSON KSM config blob. + * Service for retrieving any custom attributes defined for the + * current vault implementation and processing of said attributes. */ - private static final ObjectMapper objectMapper = new ObjectMapper(); - - /** - * All expected fields in the KSM configuration JSON blob. - */ - private static final List 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 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 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); - } - } - - } + @Inject + private KsmAttributeService attributeService; @Override public Directory 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(underlyingDirectory) { + // The value of the KSM_CONFIGURATION_ATTRIBUTE will be sanitized if set. + return new KsmDirectory(underlyingDirectory) { @Override public void add(ConnectionGroup connectionGroup) throws GuacamoleException { - // Check for the KSM config attribute and translate the one-time token - // if possible before adding - processAttributes(connectionGroup); + // Process attribute values before saving + connectionGroup.setAttributes( + 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 - // if possible before updating - processAttributes(connectionGroup); + // Unwrap the existing ConnectionGroup + if (connectionGroup instanceof KsmConnectionGroup) + 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 - return ((KsmConnectionGroup) connectionGroup).getUnderlyConnectionGroup(); + // Do not process the ConnectionGroup further if it does not exist + 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. - return new DecoratingDirectory(underlyingDirectory) { + // in the user attributes. The value of the KSM_CONFIGURATION_ATTRIBUTE + // will be sanitized if set. + return new KsmDirectory(underlyingDirectory) { @Override public void add(User user) throws GuacamoleException { - // Check for the KSM config attribute and translate the one-time token - // if possible before adding - processAttributes(user); + // Process attribute values before saving + user.setAttributes( + 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 - // if possible before updating - processAttributes(user); + // Unwrap the existing user + if (user instanceof KsmUser) + 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 - // present - return new KsmUser(user); - } + // Do not process the user further if it does not exist + if (object == null) + return null; - @Override - protected User undecorate(User user) throws GuacamoleException { + // Sanitize values when a user is fetched from the directory + return ksmUserFactory.create(object); - // Unwrap the KsmUser - return ((KsmUser) user).getUnderlyingUser(); } }; diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java index dc2877564..9c002eb4d 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java +++ b/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 - * KSM user attributes. - */ -public class KsmUser extends DelegatingUser { + /** + * A KSM-specific user implementation that exposes the + * KSM_CONFIGURATION_ATTRIBUTE attribute even if no value is set. but only + * if user-specific KSM configuration is enabled. The value of the attribute + * 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 - * KSM-specific attribute forms will have empty values automatically - * populated when getAttributes() is called. + * Logger for this class. + */ + 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 attributes = Maps.newHashMap(super.getAttributes()); - // Add the configuration attribute - attributes.putIfAbsent(KsmAttributeService.KSM_CONFIGURATION_ATTRIBUTE, null); + // Figure out if user-level KSM config is enabled + 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; } -} + } \ No newline at end of file diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUserFactory.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUserFactory.java new file mode 100644 index 000000000..9aafbb622 --- /dev/null +++ b/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); + +}