mike-jumper commented on code in PR #751:
URL: https://github.com/apache/guacamole-client/pull/751#discussion_r942672351
##########
guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js:
##########
@@ -56,6 +58,27 @@
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))
Review Comment:
Beware missing semicolon here.
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmDirectoryService.java:
##########
@@ -222,6 +231,36 @@ public void processAttributes(Attributes entity) throws
GuacamoleException {
}
+ @Override
+ 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
Review Comment:
Does it? I see interception of `add()` and `update()` overrides happening
below with `getUserDirectory()`, but not here.
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java:
##########
@@ -595,6 +596,38 @@ public KeeperRecord getRecordByDomain(String domain)
throws GuacamoleException {
* is invalid.
*/
public Future<String> 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
Review Comment:
secret*
##########
guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java:
##########
@@ -145,9 +150,51 @@ public UserHistoryResource getUserHistory()
@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<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.");
+ }
+ }
+ }
Review Comment:
This is already provided in principle via `filterAttributes()` on
`DirectoryObjectTranslator`:
https://github.com/apache/guacamole-client/blob/754e9649f1fa0ba225ee42b56ded64bc283d17df/guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryObjectTranslator.java#L119-L135
which is already invoked for every received user object for update:
https://github.com/apache/guacamole-client/blob/754e9649f1fa0ba225ee42b56ded64bc283d17df/guacamole/src/main/java/org/apache/guacamole/rest/user/UserObjectTranslator.java#L58-L66
To that end:
* I think we should leverage the existing `filterAttributes()`.
* Won't the existing `filterAttributes()` call filter out anything that
isn't declared as a user attribute here?
I wonder if perhaps the existing call to `filterAttributes()` should (1)
consider whether the user is attempting to modify themselves and (2) filter
based on user preference attributes instead of user attributes in that case.
##########
guacamole/src/main/frontend/src/translations/en.json:
##########
@@ -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.",
Review Comment:
For something like a user preference screen, the confirmatory message should
be more user-facing. As-written, this will only make sense to a Guacamole
developer who is aware of attributes, not to a Guacamole user who is just
trying to save their preferences.
##########
guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js:
##########
@@ -56,6 +58,27 @@
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))
Review Comment:
As `guacNotification.showStatus(false)` is a direct function call and not a
callback, this will be invoked immediately (rather than when the promise is
resolved).
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java:
##########
@@ -304,6 +325,72 @@ private String getConnectionGroupKsmConfig(
}
+ /**
+ * 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;
Review Comment:
It is available for balancing groups, but only the tokens that don't depend
on connection parameter values (ie: explicitly mapped tokens from the YAML).
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java:
##########
@@ -304,6 +325,72 @@ private String getConnectionGroupKsmConfig(
}
+ /**
+ * 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;
+
+ }
+
+ /**
+ * 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);
Review Comment:
The blank line separating the `return` from the controlling `if` makes this
difficult to read as being related.
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java:
##########
@@ -314,77 +401,85 @@ public Map<String, Future<String>> getTokens(UserContext
userContext, Connectabl
// 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);
-
- // 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));
+ // 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();
+
+ // 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));
}
- // 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));
- }
+ else {
- // 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
+ // 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));
Review Comment:
Given the increasing complexity of this function `getTokens()`, I think the
portion currently within a loop (the portion that adds all relevant tokens from
a particular `KsmClient`) would make more sense as its own function.
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/GuacamoleExceptionSupplier.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * 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<T> {
Review Comment:
With no concrete bits at all, this should be an `interface`.
##########
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.
Review Comment:
This is empirically correct, but will not make much sense to someone who
doesn't already know that this is required for attribute fields to appear
within the connection editor. I suggest noting that high-level purpose of this
class, not just the low-level implementation of that purpose.
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java:
##########
@@ -314,77 +401,85 @@ public Map<String, Future<String>> getTokens(UserContext
userContext, Connectabl
// 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);
-
- // 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));
+ // 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
Review Comment:
specific*
##########
guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js:
##########
@@ -78,6 +101,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<String, Object>
+ */
+ $scope.attributeMap = new Map();
Review Comment:
Does the build automatically polyfill this?
##########
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()
-
- ],
Review Comment:
Why is this being removed?
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java:
##########
@@ -36,28 +39,81 @@
@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<Form> KSM_CONNECTION_GROUP_ATTRIBUTES =
+ public static final Collection<Form> 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<Form> KSM_CONNECTION_ATTRIBUTES =
+
Collections.unmodifiableCollection(Arrays.asList(KSM_CONNECTION_FORM));
+
+ @Override
+ public Collection<Form> getConnectionAttributes() {
+ return KSM_CONNECTION_ATTRIBUTES;
+ }
+
@Override
public Collection<Form> getConnectionGroupAttributes() {
- return KSM_CONNECTION_GROUP_ATTRIBUTES;
+ return KSM_ATTRIBUTES;
+ }
+
+ @Override
+ public Collection<Form> getUserAttributes() {
+ return KSM_ATTRIBUTES;
+ }
+
+ @Override
+ public Collection<Form> getUserPreferenceAttributes() {
+
+ try {
+
+ // Expose the user attributes IFF user-level KSM configuration is
enabled
+ return configurationService.getAllowUserConfig() ? KSM_ATTRIBUTES
: Collections.emptyList();
+
+ } catch (GuacamoleException e) {
Review Comment:
Please don't cuddle the `catch`:
https://guacamole.apache.org/guac-style/#braces
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java:
##########
@@ -36,28 +39,81 @@
@Singleton
public class KsmAttributeService implements VaultAttributeService {
+
+ @Inject
+ private KsmConfigurationService configurationService;
Review Comment:
Please document.
##########
guacamole/src/main/java/org/apache/guacamole/rest/schema/SchemaResource.java:
##########
@@ -77,6 +77,26 @@ public Collection<Form> getUserAttributes() throws
GuacamoleException {
}
+ /**
+ * 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<Form> getUserAttrigetUserPreferenceAttributesbutes()
Review Comment:
Two function names got smooshed together here.
##########
guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js:
##########
@@ -197,7 +240,84 @@
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'
+ },
+
+ // Reload the user on successful save in case any
attributes changed
+ actions : [ ACKNOWLEDGE_ACTION_RELOAD ]
+ }),
+ 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) {
Review Comment:
This crosses wires a bit. We're editing and saving the user object for only
a single data source, but are aggregating the attributes from _all_ data
sources, which is technically incorrect since those data sources aren't
declaring those attributes.
If we are only editing a single user object from a single data source, then
we shouldn't be pulling attributes from all data sources; we should pull
attributes from that single data source. If we are editing the user object
across multiple data sources, then we would need to maintain each of those
objects separately, at least internally here, so that they can be independently
saved.
##########
guacamole/src/main/frontend/src/app/settings/directives/guacSettingsPreferences.js:
##########
@@ -197,7 +240,84 @@
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'
+ },
+
+ // Reload the user on successful save in case any
attributes changed
+ actions : [ ACKNOWLEDGE_ACTION_RELOAD ]
+ }),
+ 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) {
Review Comment:
We should verify that this gets polyfilled, as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
