GUACAMOLE-1656: Simplify, clean up, and improve documentation of KSM code.

This commit is contained in:
James Muehlner
2022-08-29 22:08:31 +00:00
parent dfc7e6dd90
commit 8a7bde8e9c
14 changed files with 216 additions and 284 deletions

View File

@@ -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 <T>
* The type of object which will be returned as a result of calling
* get().
*/
public abstract class GuacamoleExceptionSupplier<T> {
public interface GuacamoleExceptionSupplier<T> {
/**
* Returns a value of the declared type.
@@ -38,6 +42,6 @@ public abstract class GuacamoleExceptionSupplier<T> {
* @throws GuacamoleException
* If an error occurs while attemping to calculate the return value.
*/
public abstract T get() throws GuacamoleException;
public T get() throws GuacamoleException;
}

View File

@@ -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<Form> 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();

View File

@@ -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

View File

@@ -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,33 +389,35 @@ public class KsmSecretService implements VaultSecretService {
return null;
}
@Override
public Map<String, Future<String>> getTokens(UserContext userContext, Connectable connectable,
GuacamoleConfiguration config, TokenFilter filter) throws GuacamoleException {
/**
* 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.
Map<String, Future<String>> tokens = new HashMap<>();
Map<String, String> 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<KsmClient> 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));
// 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<KsmClient> ksmIterator = ksmClients.iterator();
while (ksmIterator.hasNext()) {
KsmClient ksm = ksmIterator.next();
* @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<String, Future<String>> tokens,
Map<String, String> parameters, TokenFilter filter) throws GuacamoleException {
// Retrieve and define server-specific tokens, if any
String hostname = parameters.get("hostname");
@@ -427,6 +427,13 @@ public class KsmSecretService implements VaultSecretService {
// 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;
@@ -482,6 +489,26 @@ public class KsmSecretService implements VaultSecretService {
}
}
@Override
public Map<String, Future<String>> getTokens(UserContext userContext, Connectable connectable,
GuacamoleConfiguration config, TokenFilter filter) throws GuacamoleException {
Map<String, Future<String>> tokens = new HashMap<>();
Map<String, String> parameters = config.getParameters();
// 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())
addConnectableTokens(
config, getClient(userKsmConfig), tokens, parameters, filter);
// 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;
}

View File

@@ -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<String> 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<String> connectionAttributeNames) {
KsmConnection(Connection connection) {
super(connection);
this.connectionAttributeNames = connectionAttributeNames;
}
/**
@@ -70,13 +60,12 @@ public class KsmConnection extends DelegatingConnection {
public Map<String, String> getAttributes() {
// Make a copy of the existing map
Map<String, String> attributeMap = Maps.newHashMap(super.getAttributes());
Map<String, String> 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;
}
}

View File

@@ -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<String, String> getAttributes() {
// All attributes defined on the underlying connection group
Map<String, String> attributes = super.getAttributes();
// Make a copy of the existing map
Map<String, String> 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;
}

View File

@@ -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<Connection> getConnectionDirectory(
Directory<Connection> 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<Connection>(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<User>(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

View File

@@ -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<String> 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<String> userAttributeNames) {
KsmUser(User user) {
super(user);
this.userAttributeNames = userAttributeNames;
}
/**
@@ -70,13 +59,11 @@ public class KsmUser extends DelegatingUser {
public Map<String, String> getAttributes() {
// Make a copy of the existing map
Map<String, String> attributeMap = Maps.newHashMap(super.getAttributes());
Map<String, String> 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;
}
}

View File

@@ -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<String, Object>
*/
$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
// Fetch all user preference attribute forms defined
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()
)
})
}
$scope.attributes = attributes;
});
// 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())
})
});
});
});
}]
};
}]);

View File

@@ -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",

View File

@@ -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: {

View File

@@ -89,7 +89,7 @@ public class SchemaResource {
*/
@GET
@Path("userPreferenceAttributes")
public Collection<Form> getUserAttrigetUserPreferenceAttributesbutes()
public Collection<Form> getUserPreferenceAttributes()
throws GuacamoleException {
// Retrieve all possible user preference attributes

View File

@@ -59,10 +59,23 @@ public class UserObjectTranslator
public void filterExternalObject(UserContext userContext, APIUser object)
throws GuacamoleException {
// Filter object attributes by defined schema
// 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()));
}
}
}

View File

@@ -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 {
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)
User currentUser = userContext.self();
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.");
// All attributes exposed in the preferences forms
Set<String> 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<String> 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);