You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2021/06/03 02:24:54 UTC
[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
mike-jumper commented on a change in pull request #353:
URL: https://github.com/apache/guacamole-client/pull/353#discussion_r644433980
##########
File path: extensions/guacamole-auth-ldap/schema/guacConfigGroup.ldif
##########
@@ -20,9 +20,24 @@
dn: cn=guacConfigGroup,cn=schema,cn=config
objectClass: olcSchemaConfig
cn: guacConfigGroup
-olcAttributeTypes: {0}( 1.3.6.1.4.1.38971.1.1.1 NAME 'guacConfigProtocol' SYNTAX 1.3.6.1.4.1.1466
- .115.121.1.15 )
-olcAttributeTypes: {1}( 1.3.6.1.4.1.38971.1.1.2 NAME 'guacConfigParameter' SYNTAX 1.3.6.1.4.1.146
- 6.115.121.1.15 )
-olcObjectClasses: {0}( 1.3.6.1.4.1.38971.1.2.1 NAME 'guacConfigGroup' DESC 'Guacamole config
- uration group' SUP groupOfNames MUST guacConfigProtocol MAY guacConfigParameter )
+olcAttributeTypes: {0}( 1.3.6.1.4.1.38971.1.1.1 NAME 'guacConfigProtocol'
+ SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 )
+olcAttributeTypes: {1}( 1.3.6.1.4.1.38971.1.1.2 NAME 'guacConfigParameter'
+ SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 )
+olcAttributeTypes: {2}( 1.3.6.1.4.1.38971.1.1.3 NAME 'guacConfigProxyHostname'
+ SINGLE-VALUE
+ SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 )
+olcAttributeTypes: {2}( 1.3.6.1.4.1.38971.1.1.4 NAME 'guacConfigProxyPort'
+ SINGLE-VALUE
+ SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 )
+olcAttributeTypes: {2}( 1.3.6.1.4.1.38971.1.1.5 NAME 'guacConfigProxyEncryption'
+ SINGLE-VALUE
+ SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 )
+olcObjectClasses: {0}( 1.3.6.1.4.1.38971.1.2.1 NAME 'guacConfigGroup'
+ DESC 'Guacamole configuration group'
+ SUP groupOfNames
+ MUST guacConfigProtocol
+ MAY ( guacConfigParameter $
+ guacConfigProxyHostname $
+ guacConfigProxyPort $
+ guacConfigProxyEncryption ) )
Review comment:
When I run this against OpenLDAP with `ldapadd`, I get:
```
adding new entry "cn=guacConfigGroup,cn=schema,cn=config"
ldap_add: Other (e.g., implementation specific) error (80)
additional info: olcAttributeTypes: Unexpected token before 1.3.6.1.4.1.1466.115.121.1.15 )
```
I think this is because of how the LDIF format handles indentation, with a single space being a continuation of the previous line. Something like:
```
FOO
BAR
```
is equivalent to "FOOBAR", whereas:
```
FOO
BAR
```
is equivalent to "FOO BAR".
##########
File path: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java
##########
@@ -166,14 +194,52 @@
config.setProtocol(protocol.getString());
}
catch (LdapInvalidAttributeValueException e) {
- logger.error("Invalid value of the protocol entry: {}",
- e.getMessage());
+ logger.error("Invalid value of the protocol entry: {}", e.getMessage());
logger.debug("LDAP exception when getting protocol value.", e);
return null;
}
+
+ // Get proxy configuration, if any
+ GuacamoleProxyConfiguration proxyConfig;
+ try {
+
+ // Get default proxy configuration values
+ proxyConfig = LocalEnvironment.getInstance().getDefaultGuacamoleProxyConfiguration();
+ String proxyHostname = proxyConfig.getHostname();
+ int proxyPort = proxyConfig.getPort();
+ EncryptionMethod proxyEncryption = proxyConfig.getEncryptionMethod();
+
+ // Get the proxy hostname
+ Attribute proxyHostAttr = entry.get(LDAP_ATTRIBUTE_PROXY_HOSTNAME);
+ if (proxyHostAttr != null && proxyHostAttr.size() > 0)
+ proxyHostname = proxyHostAttr.getString();
+
+ // Get the proxy port
+ Attribute proxyPortAttr = entry.get(LDAP_ATTRIBUTE_PROXY_PORT);
+ if (proxyPortAttr != null && proxyPortAttr.size() > 0)
+ proxyPort = Integer.parseInt(proxyPortAttr.getString());
+
+ // Get the proxy encryption method
+ Attribute proxyEncryptionAttr = entry.get(LDAP_ATTRIBUTE_PROXY_ENCRYPTION);
+ if (proxyEncryptionAttr != null && proxyEncryptionAttr.size() > 0)
+ proxyEncryption = EncryptionMethod.valueOf(proxyEncryptionAttr.getString());
Review comment:
To avoid making this existing function more complex, I recommend separating out this logic into its own function. Something that returns the `GuacamoleProxyConfiguration` for an arbitrary `Entry` (regardless of whether all or part of the ultimate configuration come from the environment) could abstract this nicely.
##########
File path: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java
##########
@@ -59,6 +62,33 @@
* Logger for this class.
*/
private static final Logger logger = LoggerFactory.getLogger(ConnectionService.class);
+
+ /**
+ * The name of the LDAP attribute that stores connection configuration
+ * parameters for Guacamole.
+ */
+ public static final String LDAP_ATTRIBUTE_PARAMETER = "guacConfigParameter";
+
+ /**
+ * The name of the LDAP attribute that stores the protocol for a Guacamole
+ * connection.
+ */
+ public static final String LDAP_ATTRIBUTE_PROTOCOL = "guacConfigProtocol";
+
+ /**
+ * The name of the LDAP attribute that stores guacd proxy hostname.
+ */
+ public static final String LDAP_ATTRIBUTE_PROXY_HOSTNAME = "guacConfigProxyHostname";
+
+ /**
+ * The name of the LDAP attribute that stores guacd proxy port..
Review comment:
You have two "." here.
##########
File path: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java
##########
@@ -166,14 +194,52 @@
config.setProtocol(protocol.getString());
}
catch (LdapInvalidAttributeValueException e) {
- logger.error("Invalid value of the protocol entry: {}",
- e.getMessage());
+ logger.error("Invalid value of the protocol entry: {}", e.getMessage());
logger.debug("LDAP exception when getting protocol value.", e);
return null;
}
+
+ // Get proxy configuration, if any
+ GuacamoleProxyConfiguration proxyConfig;
+ try {
+
+ // Get default proxy configuration values
+ proxyConfig = LocalEnvironment.getInstance().getDefaultGuacamoleProxyConfiguration();
+ String proxyHostname = proxyConfig.getHostname();
+ int proxyPort = proxyConfig.getPort();
+ EncryptionMethod proxyEncryption = proxyConfig.getEncryptionMethod();
+
+ // Get the proxy hostname
+ Attribute proxyHostAttr = entry.get(LDAP_ATTRIBUTE_PROXY_HOSTNAME);
+ if (proxyHostAttr != null && proxyHostAttr.size() > 0)
+ proxyHostname = proxyHostAttr.getString();
+
+ // Get the proxy port
+ Attribute proxyPortAttr = entry.get(LDAP_ATTRIBUTE_PROXY_PORT);
+ if (proxyPortAttr != null && proxyPortAttr.size() > 0)
+ proxyPort = Integer.parseInt(proxyPortAttr.getString());
+
+ // Get the proxy encryption method
+ Attribute proxyEncryptionAttr = entry.get(LDAP_ATTRIBUTE_PROXY_ENCRYPTION);
+ if (proxyEncryptionAttr != null && proxyEncryptionAttr.size() > 0)
+ proxyEncryption = EncryptionMethod.valueOf(proxyEncryptionAttr.getString());
Review comment:
This will throw an `IllegalArgumentException` if the provided value does not identically match one of the internal names of the enum values (currently `NONE` and `SSL`).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org