necouchman commented on code in PR #1163:
URL: https://github.com/apache/guacamole-client/pull/1163#discussion_r2795790765
##########
extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/user/VaultUserContext.java:
##########
@@ -357,6 +358,72 @@ protected void addTokens(ConnectionGroup connectionGroup,
connectionGroup, confService.getTokenMapping(), filter,
null, new TokenFilter(tokens))));
+ // For BALANCING groups, the JDBC layer selects and connects a child
+ // connection internally, bypassing the vault's addTokens(Connection).
+ // Pre-resolve vault tokens for child connections here so they are
+ // available when the JDBC layer applies tokens to the child's config.
+ if (connectionGroup.getType() == ConnectionGroup.Type.BALANCING) {
+
+ Set<String> childIds;
+ try {
+ childIds = connectionGroup.getConnectionIdentifiers();
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Unable to retrieve child connection identifiers "
+ + "for BALANCING group \"{}\": {}", identifier,
+ e.getMessage());
+ return;
+ }
+
+ for (String childId : childIds) {
+ try {
+
+ Connection child = getPrivileged()
+ .getConnectionDirectory().get(childId);
+ if (child == null)
+ continue;
+
+ GuacamoleConfiguration childConfig =
+ child.getConfiguration();
+ if (childConfig == null)
+ continue;
+
+ logger.debug("Resolving vault tokens for BALANCING "
+ + "child connection \"{}\" (\"{}\").",
+ child.getIdentifier(), child.getName());
+
+ TokenFilter childFilter = createFilter();
+ childFilter.setToken(CONNECTION_NAME_TOKEN,
+ child.getName());
+ childFilter.setToken(CONNECTION_IDENTIFIER_TOKEN,
+ child.getIdentifier());
+
+ Map<String, String> parameters =
+ childConfig.getParameters();
+
+ String hostname = parameters.get("hostname");
+ if (hostname != null && !hostname.isEmpty())
+ childFilter.setToken(CONNECTION_HOSTNAME_TOKEN,
+ hostname);
+
+ String username = parameters.get("username");
+ if (username != null && !username.isEmpty())
+ childFilter.setToken(CONNECTION_USERNAME_TOKEN,
+ username);
+
+ tokens.putAll(resolve(getTokens(child,
+ confService.getTokenMapping(), childFilter,
+ childConfig, new TokenFilter(tokens))));
+
+ }
Review Comment:
I see what you're trying to do, but I'm not sure this is the best way to go
about it. It looks to me like the code, here, basically duplicates code
elsewhere, processing a handful of standard tokens (connection name/identifier,
username, etc.). This creates two problems:
* What if other extensions provide other tokens? For example, the LDAP
extension provides the ability to use LDAP attributes as tokens in the
connection data, as do the various SSO providers. I think the implementation,
here, would not capture those correctly?
* What happens if other tokens are added in the future? Whether processing
these standard ones or all of the ones from all of the extensions, creating a
location, here, where we have to maintain all of the tokens we want available
and process them seems cumbersome and destined to get out-of-sync with the rest
of the code.
There has to be a better way...
--
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]
