[GitHub] [guacamole-client] necouchman commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
necouchman commented on a change in pull request #353:
URL: https://github.com/apache/guacamole-client/pull/353#discussion_r693357038
##
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:
Okay, installed schema2ldif and used that to generate it - hopefully
that works better!
--
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]
[GitHub] [guacamole-client] necouchman commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
necouchman commented on a change in pull request #353: URL: https://github.com/apache/guacamole-client/pull/353#discussion_r642149729 ## File path: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java ## @@ -59,6 +62,42 @@ * 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 proxy configuration parameters + * for Guacamole. + */ +public static final String LDAP_ATTRIBUTE_PROXY = "guacConfigProxy"; + +/** + * The proxy parameter name that configures the hostname or IP address that + * will be used to connect to guacd. + */ +public static final String PROXY_PARAMETER_HOSTNAME = "proxy-hostname"; + +/** + * The proxy parameter name that configures the TCP port that will be used + * to connect to guacd. + */ +public static final String PROXY_PARAMETER_PORT = "proxy-port"; + +/** + * The proxy parameter name that configures the encryption method that will + * be used when connected to guacd. + */ +public static final String PROXY_PARAMETER_ENCRYPTION = "proxy-encryption"; Review comment: Yeah, that makes sense. I'll re-factor it for that. -- 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: [email protected]
[GitHub] [guacamole-client] necouchman commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
necouchman commented on a change in pull request #353: URL: https://github.com/apache/guacamole-client/pull/353#discussion_r642148001 ## File path: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java ## @@ -59,6 +62,42 @@ * 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 proxy configuration parameters + * for Guacamole. + */ +public static final String LDAP_ATTRIBUTE_PROXY = "guacConfigProxy"; + +/** + * The proxy parameter name that configures the hostname or IP address that + * will be used to connect to guacd. + */ +public static final String PROXY_PARAMETER_HOSTNAME = "proxy-hostname"; + +/** + * The proxy parameter name that configures the TCP port that will be used + * to connect to guacd. + */ +public static final String PROXY_PARAMETER_PORT = "proxy-port"; + +/** + * The proxy parameter name that configures the encryption method that will + * be used when connected to guacd. + */ +public static final String PROXY_PARAMETER_ENCRYPTION = "proxy-encryption"; Review comment: Okay - if you've another suggestion for parsing out guacConfigProxy, I'm certainly not opposed to a better way, but this is just the way I had gone about implementing it - with constants for each of the possible parameter names for `guacConfigProxy`. -- 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: [email protected]
[GitHub] [guacamole-client] necouchman commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
necouchman commented on a change in pull request #353: URL: https://github.com/apache/guacamole-client/pull/353#discussion_r642136913 ## File path: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java ## @@ -59,6 +62,42 @@ * 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 proxy configuration parameters + * for Guacamole. + */ +public static final String LDAP_ATTRIBUTE_PROXY = "guacConfigProxy"; + +/** + * The proxy parameter name that configures the hostname or IP address that + * will be used to connect to guacd. + */ +public static final String PROXY_PARAMETER_HOSTNAME = "proxy-hostname"; + +/** + * The proxy parameter name that configures the TCP port that will be used + * to connect to guacd. + */ +public static final String PROXY_PARAMETER_PORT = "proxy-port"; + +/** + * The proxy parameter name that configures the encryption method that will + * be used when connected to guacd. + */ +public static final String PROXY_PARAMETER_ENCRYPTION = "proxy-encryption"; Review comment: I'm not sure I understand? These were added as part of the `guacConfigProxy` attribute - what should I be replacing them with? -- 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: [email protected]
[GitHub] [guacamole-client] necouchman commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
necouchman commented on a change in pull request #353:
URL: https://github.com/apache/guacamole-client/pull/353#discussion_r642136002
##
File path: extensions/guacamole-auth-ldap/schema/guacConfigGroup.ldif
##
@@ -20,9 +20,14 @@
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 'guacConfigProxy'
+SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 )
Review comment:
Weird.
--
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:
[email protected]
[GitHub] [guacamole-client] necouchman commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
necouchman commented on a change in pull request #353: URL: https://github.com/apache/guacamole-client/pull/353#discussion_r642135965 ## File path: guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java ## @@ -157,6 +162,43 @@ public SimpleConnection(String name, String identifier, this.fullConfig = config; this.interpretTokens = interpretTokens; +} + +/** + * Creates a new SimpleConnection having the given identifier, Review comment: Ah, yes. -- 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: [email protected]
[GitHub] [guacamole-client] necouchman commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
necouchman commented on a change in pull request #353: URL: https://github.com/apache/guacamole-client/pull/353#discussion_r642135852 ## File path: guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java ## @@ -157,6 +162,43 @@ public SimpleConnection(String name, String identifier, this.fullConfig = config; this.interpretTokens = interpretTokens; +} + +/** + * Creates a new SimpleConnection having the given identifier, + * GuacamoleConfiguration, and GuacamoleProxyConfiguration. Parameter tokens + * will be interpreted if explicitly requested. + * + * @param name + * The name to associate with this connection. + * + * @param identifier + * The identifier to associate with this connection. + * + * @param config + * The configuration describing how to connect to this connection. + * + * @param interpretTokens + * Whether parameter tokens in the underlying GuacamoleConfiguration + * should be automatically applied upon connecting. If false, parameter + * tokens will not be interpreted at all. + * + * @param proxyConfig + * The Guacamole proxy configuration used to tell the client how to + * connect to guacd. Review comment: OKay, makes sense. -- 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: [email protected]
[GitHub] [guacamole-client] necouchman commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
necouchman commented on a change in pull request #353:
URL: https://github.com/apache/guacamole-client/pull/353#discussion_r642135827
##
File path:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java
##
@@ -171,9 +210,73 @@
logger.debug("LDAP exception when getting protocol
value.", e);
return null;
}
+
+// Get the default Proxy configuration
+GuacamoleProxyConfiguration proxyConfig;
+try {
+proxyConfig =
confService.getDefaultGuacamoleProxyConfiguration();
Review comment:
Makes sense.
##
File path:
guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java
##
@@ -157,6 +162,43 @@ public SimpleConnection(String name, String identifier,
this.fullConfig = config;
this.interpretTokens = interpretTokens;
+}
+
+/**
+ * Creates a new SimpleConnection having the given identifier,
+ * GuacamoleConfiguration, and GuacamoleProxyConfiguration. Parameter
tokens
+ * will be interpreted if explicitly requested.
+ *
+ * @param name
+ * The name to associate with this connection.
+ *
+ * @param identifier
+ * The identifier to associate with this connection.
+ *
+ * @param config
+ * The configuration describing how to connect to this connection.
+ *
+ * @param interpretTokens
+ * Whether parameter tokens in the underlying GuacamoleConfiguration
+ * should be automatically applied upon connecting. If false, parameter
+ * tokens will not be interpreted at all.
+ *
+ * @param proxyConfig
+ * The Guacamole proxy configuration used to tell the client how to
+ * connect to guacd.
Review comment:
Sure, sounds good.
--
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:
[email protected]
[GitHub] [guacamole-client] necouchman commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
necouchman commented on a change in pull request #353:
URL: https://github.com/apache/guacamole-client/pull/353#discussion_r642135777
##
File path:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/ConfigurationService.java
##
@@ -391,5 +392,21 @@ public MemberAttributeType getMemberAttributeType()
MemberAttributeType.DN
);
}
+
+/**
+ * Return the default configuration that determines how Guacamole Client
+ * connects to guacd in order to establish connections to remote servers.
+ *
+ * @return
+ * The default configuration used by Guacamole Client for connecting
+ * to guacd.
+ *
+ * @throws GuacamoleException
+ * If guacamole.properties cannot be parsed.
+ */
+public GuacamoleProxyConfiguration getDefaultGuacamoleProxyConfiguration()
+throws GuacamoleException {
+return environment.getDefaultGuacamoleProxyConfiguration();
+}
Review comment:
Removed.
##
File path:
guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java
##
@@ -53,6 +52,12 @@
* Backing configuration, containing all sensitive information.
*/
private GuacamoleConfiguration fullConfig;
+
+/**
+ * The proxy configuration used to tell the client how to connect to
Review comment:
Fixed.
--
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:
[email protected]
[GitHub] [guacamole-client] necouchman commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
necouchman commented on a change in pull request #353:
URL: https://github.com/apache/guacamole-client/pull/353#discussion_r642135699
##
File path:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java
##
@@ -152,11 +191,11 @@
}
// Get associated protocol
-Attribute protocol = entry.get("guacConfigProtocol");
+Attribute protocol = entry.get(LDAP_ATTRIBUTE_PROTOCOL);
if (protocol == null) {
logger.warn("guacConfigGroup \"{}\" is missing the "
- + "required \"guacConfigProtocol\" attribute.",
-cnName);
Review comment:
Hehe - I'll unwrap things throughout this file.
--
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:
[email protected]
[GitHub] [guacamole-client] necouchman commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
necouchman commented on a change in pull request #353:
URL: https://github.com/apache/guacamole-client/pull/353#discussion_r642135474
##
File path:
guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java
##
@@ -157,6 +162,43 @@ public SimpleConnection(String name, String identifier,
this.fullConfig = config;
this.interpretTokens = interpretTokens;
+}
+
+/**
+ * Creates a new SimpleConnection having the given identifier,
+ * GuacamoleConfiguration, and GuacamoleProxyConfiguration. Parameter
tokens
+ * will be interpreted if explicitly requested.
+ *
+ * @param name
+ * The name to associate with this connection.
+ *
+ * @param identifier
+ * The identifier to associate with this connection.
+ *
+ * @param config
+ * The configuration describing how to connect to this connection.
+ *
+ * @param interpretTokens
+ * Whether parameter tokens in the underlying GuacamoleConfiguration
+ * should be automatically applied upon connecting. If false, parameter
+ * tokens will not be interpreted at all.
+ *
+ * @param proxyConfig
+ * The Guacamole proxy configuration used to tell the client how to
+ * connect to guacd.
+ */
+public SimpleConnection(String name, String identifier,
+GuacamoleConfiguration config, boolean interpretTokens,
+GuacamoleProxyConfiguration proxyConfig) {
Review comment:
I'm good with that.
--
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:
[email protected]
[GitHub] [guacamole-client] necouchman commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
necouchman commented on a change in pull request #353:
URL: https://github.com/apache/guacamole-client/pull/353#discussion_r642134514
##
File path:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java
##
@@ -171,9 +210,73 @@
logger.debug("LDAP exception when getting protocol
value.", e);
return null;
}
+
+// Get the default Proxy configuration
+GuacamoleProxyConfiguration proxyConfig;
+try {
+proxyConfig =
confService.getDefaultGuacamoleProxyConfiguration();
+}
+catch (GuacamoleException e) {
+logger.warn("Could not retrieve Guacamole proxy
configuration.",
+e.getMessage());
+logger.debug("Error when trying to get Guacamole proxy
configuration.",
+e);
Review comment:
Under
stood,
makes perfect sense to
me.
--
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:
[email protected]
[GitHub] [guacamole-client] necouchman commented on a change in pull request #353: GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections
necouchman commented on a change in pull request #353:
URL: https://github.com/apache/guacamole-client/pull/353#discussion_r632564450
##
File path:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java
##
@@ -167,18 +201,34 @@
// Parse name
String name = parameter.substring(0, equals);
String value = parameter.substring(equals+1);
-
-config.setParameter(name, value);
+
+// Pull out and set proxy parameters, if present
+// Otherwise set the parameter.
+switch(name) {
+case PROXY_HOST_PARAMETER:
Review comment:
I think changing OIDs will be saved for a different JIRA issue...
--
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:
[email protected]
