[GitHub] guacamole-client pull request #354: GUACAMOLE-688: added LDAP_USER_SEARCH_FI...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/354#discussion_r246567460 --- Diff: guacamole-docker/bin/start.sh --- @@ -317,6 +317,10 @@ END "ldap-search-bind-password" \ "$LDAP_SEARCH_BIND_PASSWORD" +set_optional_property \ +"ldap-user-search-filter" \ --- End diff -- Ever so minor nit-pick, but, in keeping with the style on the rest of the document, please line up the `\` at the end of the lines, here. ---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/353#discussion_r245858704
--- Diff:
guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java
---
@@ -68,21 +72,34 @@ public SimpleConnection() {
* @param identifier The identifier to associate with this connection.
* @param config The configuration describing how to connect to this
* connection.
+ *
+ * @throws GuacamoleException
+ * If the default proxy configuration cannot be retrieved.
*/
public SimpleConnection(String name, String identifier,
-GuacamoleConfiguration config) {
+GuacamoleConfiguration config) throws GuacamoleException {
// Set name
-setName(name);
+super.setName(name);
// Set identifier
-setIdentifier(identifier);
+super.setIdentifier(identifier);
// Set config
-setConfiguration(config);
+super.setConfiguration(config);
this.config = config;
+this.proxyConfig = new
LocalEnvironment().getDefaultGuacamoleProxyConfiguration();
}
+
+public SimpleConnection(String name, String identifier,
--- End diff --
Documented.
---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/353#discussion_r245858747 --- Diff: guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connection.java --- @@ -125,5 +125,17 @@ * connection. */ public Set getSharingProfileIdentifiers() throws GuacamoleException; + +/** + * Get the GuacamoleProxyConfiguration for this connection. + * + * @return + * The GuacamoleProxyConfiguration for this connection. + * + * @throws GuacamoleException + * If configuration information cannot be retrieved or parsed. --- End diff -- Pulled this out. ---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/353#discussion_r245858656
--- Diff:
guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java
---
@@ -68,21 +72,34 @@ public SimpleConnection() {
* @param identifier The identifier to associate with this connection.
* @param config The configuration describing how to connect to this
* connection.
+ *
+ * @throws GuacamoleException
+ * If the default proxy configuration cannot be retrieved.
*/
public SimpleConnection(String name, String identifier,
-GuacamoleConfiguration config) {
+GuacamoleConfiguration config) throws GuacamoleException {
--- End diff --
Backed this change out and just kept things in the `connect()` method.
---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/353#discussion_r245858609 --- Diff: guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connection.java --- @@ -125,5 +125,17 @@ * connection. */ public Set getSharingProfileIdentifiers() throws GuacamoleException; + +/** + * Get the GuacamoleProxyConfiguration for this connection. --- End diff -- Removed. ---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/353#discussion_r245858540 --- Diff: guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Connection.java --- @@ -125,5 +125,17 @@ * connection. */ public Set getSharingProfileIdentifiers() throws GuacamoleException; + +/** + * Get the GuacamoleProxyConfiguration for this connection. + * + * @return + * The GuacamoleProxyConfiguration for this connection. + * + * @throws GuacamoleException + * If configuration information cannot be retrieved or parsed. + */ +public GuacamoleProxyConfiguration getGuacamoleProxyConfiguration() --- End diff -- Okay, I've removed this entirely from the interfaces and abstracts. ---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/353#discussion_r245858363
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java
---
@@ -148,12 +169,25 @@
// Set protocol
GuacamoleConfiguration config = new
GuacamoleConfiguration();
+
+GuacamoleProxyConfiguration proxyConfig;
+try {
+proxyConfig = new
LocalEnvironment().getDefaultGuacamoleProxyConfiguration();
+}
+catch (GuacamoleException e) {
+return null;
--- End diff --
Logged.
---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/353#discussion_r245858320
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java
---
@@ -148,12 +169,25 @@
// Set protocol
GuacamoleConfiguration config = new
GuacamoleConfiguration();
+
+GuacamoleProxyConfiguration proxyConfig;
+try {
+proxyConfig = new
LocalEnvironment().getDefaultGuacamoleProxyConfiguration();
--- End diff --
Okay, switched back to this method.
---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/353#discussion_r245858199
--- Diff:
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:
--- End diff --
Okay, created a new LDAP attribute for the proxy configuration, under the
existing OID string.
---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/353#discussion_r245818609
--- Diff:
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:
--- End diff --
Cool. I'm thinking the right way to do this is to continue with the
current OID in this PR, and then open a separate JIRA issue (and PR) for
converting from the old OID to the ASF OID?
---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/353#discussion_r245789239
--- Diff:
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:
--- End diff --
Cool. Do we need to open an INFRA ticket to get that OID assigned under
ASF? Or is there some documentation somewhere on the ASF site that documents
this?
---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/353#discussion_r245760171
--- Diff:
guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleConnection.java
---
@@ -68,21 +72,34 @@ public SimpleConnection() {
* @param identifier The identifier to associate with this connection.
* @param config The configuration describing how to connect to this
* connection.
+ *
+ * @throws GuacamoleException
+ * If the default proxy configuration cannot be retrieved.
*/
public SimpleConnection(String name, String identifier,
-GuacamoleConfiguration config) {
+GuacamoleConfiguration config) throws GuacamoleException {
--- End diff --
Okay, will refactor.
---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/353#discussion_r245757432
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java
---
@@ -148,12 +169,25 @@
// Set protocol
GuacamoleConfiguration config = new
GuacamoleConfiguration();
+
+GuacamoleProxyConfiguration proxyConfig;
+try {
+proxyConfig = new
LocalEnvironment().getDefaultGuacamoleProxyConfiguration();
--- End diff --
I originally went this route, but thought it might be a little overkill
:-). I'll go back to this method...
---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/353#discussion_r245757512
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java
---
@@ -148,12 +169,25 @@
// Set protocol
GuacamoleConfiguration config = new
GuacamoleConfiguration();
+
+GuacamoleProxyConfiguration proxyConfig;
+try {
+proxyConfig = new
LocalEnvironment().getDefaultGuacamoleProxyConfiguration();
+}
+catch (GuacamoleException e) {
+return null;
--- End diff --
Got it.
---
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/353#discussion_r245756503
--- Diff:
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:
--- End diff --
Sounds good. I hesitated to modify the LDAP schema, but I'm happy to go
that route if that's the best way to go. Do you think separate LDAP attributes
specifically for proxying, or just a generic Guacamole Attribute that is
handled similar to the parameters?
For the OID, should we use the Guacamole one, or does ASF have one we
should go with?
---
[GitHub] guacamole-website pull request #64: Add draft release notes for first RC of ...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-website/pull/64#discussion_r245422999 --- Diff: _releases/1.0.0.md --- @@ -0,0 +1,805 @@ +--- + +released: false +title: 1.0.0 +date: 2018-12-20 22:00:00 -0800 +summary: > +User groups, improved clipboard integration, TOTP (Google Authenticator), +RADIUS, dead keys. + +artifact-root: "https://dist.apache.org/repos/dist/dev/; +checksum-root: "https://dist.apache.org/repos/dist/dev/; +download-path: "guacamole/1.0.0-RC1/" + +source-dist: +- "source/guacamole-client-1.0.0.tar.gz" +- "source/guacamole-server-1.0.0.tar.gz" + +binary-dist: +- "binary/guacamole-1.0.0.war" +- "binary/guacamole-auth-cas-1.0.0.tar.gz" +- "binary/guacamole-auth-duo-1.0.0.tar.gz" +- "binary/guacamole-auth-header-1.0.0.tar.gz" +- "binary/guacamole-auth-jdbc-1.0.0.tar.gz" +- "binary/guacamole-auth-ldap-1.0.0.tar.gz" +- "binary/guacamole-auth-openid-1.0.0.tar.gz" +- "binary/guacamole-auth-quickconnect-1.0.0.tar.gz" +- "binary/guacamole-auth-totp-1.0.0.tar.gz" + +documentation: +"Manual" : "/doc/1.0.0/gug" +"guacamole-common": "/doc/1.0.0/guacamole-common" +"guacamole-common-js" : "/doc/1.0.0/guacamole-common-js" +"guacamole-ext" : "/doc/1.0.0/guacamole-ext" +"libguac" : "/doc/1.0.0/libguac" + +--- + +The 1.0.0 release features support for user groups, improved clipboard +integration leveraging the Asynchronous Clipboard API, as well as support for +TOTP (Google Authenticator), RADIUS, and dead keys. + +**This release contains changes which break compatibility with past releases.** +Please see the [deprecation / compatibility +notes](#deprecation--compatibility-notes) section for more information. + + +New features and improvements +- + +### Support for user groups + +Guacamole now supports [granting permissions based on group +membership](/doc/1.0.0/gug/administration.html#user-group-management). While +this has been supported to a degree for some time via LDAP and the `seeAlso` +attribute, groups can now be defined and used within a database, with LDAP and +a database combined, or within other extensions using Guacamole's extension +API. + + * [GUACAMOLE-220](https://issues.apache.org/jira/browse/GUACAMOLE-220) - Implement user groups + +### Clipboard integration with the Asynchronous Clipboard API + +For browsers which implement the [Asynchronous Clipboard +API](https://www.w3.org/TR/clipboard-apis/#async-clipboard-api), Guacamole will +now automatically synchronize the local and remote clipboards. Users will be +prompted to grant clipboard access upon opening Guacamole, and Guacamole will +synchronize the clipboard if access is granted. + +This API [has been implemented in Google Chrome since version +66](https://developers.google.com/web/updates/2018/03/clipboardapi), and other +browsers will likely follow suit. The legacy synchronous clipboard API will +continue to be used as a fallback for browsers that support clipboard access +but lack support for the newer API (Internet Explorer). + + * [GUACAMOLE-559](https://issues.apache.org/jira/browse/GUACAMOLE-559) - Add support for the new Asynchronous Clipboard API + +### Multi-factor authentication with Google Authenticator / TOTP + +Guacamole now has support for TOTP as an additional authentication factor. +TOTP ([Time-based One-Time Password](https://en.wikipedia.org/wiki/Time-based_One-time_Password_algorithm)) +is a [standardized algorithm](https://tools.ietf.org/html/rfc6238) used for +multi-factor authentication. With this new support, Guacamole may be used with +any application or authentication device which supports the TOTP standard, +including the popular Google Authenticator. + + * [GUACAMOLE-96](https://issues.apache.org/jira/browse/GUACAMOLE-96) - Two factor authentication with Google Authenticator + +### Support for RADIUS authentication + +RADIUS support has been added, allowing Guacamole to delegate authentication to +a RADIUS service like FreeRADIUS for validating credentials, enforcing multiple +authentication factors, etc. + +Because the RADIUS library used by this support is licensed under the LGPL, a +convenience binary for this extension is not provided. If you wish to use the +RADIUS support
[GitHub] guacamole-website pull request #64: Add draft release notes for first RC of ...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-website/pull/64#discussion_r245422397 --- Diff: _releases/1.0.0.md --- @@ -0,0 +1,805 @@ +--- + +released: false +title: 1.0.0 +date: 2018-12-20 22:00:00 -0800 +summary: > +User groups, improved clipboard integration, TOTP (Google Authenticator), +RADIUS, dead keys. + +artifact-root: "https://dist.apache.org/repos/dist/dev/; +checksum-root: "https://dist.apache.org/repos/dist/dev/; +download-path: "guacamole/1.0.0-RC1/" + +source-dist: +- "source/guacamole-client-1.0.0.tar.gz" +- "source/guacamole-server-1.0.0.tar.gz" + +binary-dist: +- "binary/guacamole-1.0.0.war" +- "binary/guacamole-auth-cas-1.0.0.tar.gz" +- "binary/guacamole-auth-duo-1.0.0.tar.gz" +- "binary/guacamole-auth-header-1.0.0.tar.gz" +- "binary/guacamole-auth-jdbc-1.0.0.tar.gz" +- "binary/guacamole-auth-ldap-1.0.0.tar.gz" +- "binary/guacamole-auth-openid-1.0.0.tar.gz" +- "binary/guacamole-auth-quickconnect-1.0.0.tar.gz" +- "binary/guacamole-auth-totp-1.0.0.tar.gz" + +documentation: +"Manual" : "/doc/1.0.0/gug" +"guacamole-common": "/doc/1.0.0/guacamole-common" +"guacamole-common-js" : "/doc/1.0.0/guacamole-common-js" +"guacamole-ext" : "/doc/1.0.0/guacamole-ext" +"libguac" : "/doc/1.0.0/libguac" + +--- + +The 1.0.0 release features support for user groups, improved clipboard +integration leveraging the Asynchronous Clipboard API, as well as support for +TOTP (Google Authenticator), RADIUS, and dead keys. + +**This release contains changes which break compatibility with past releases.** +Please see the [deprecation / compatibility +notes](#deprecation--compatibility-notes) section for more information. + + +New features and improvements +- + +### Support for user groups + +Guacamole now supports [granting permissions based on group +membership](/doc/1.0.0/gug/administration.html#user-group-management). While +this has been supported to a degree for some time via LDAP and the `seeAlso` +attribute, groups can now be defined and used within a database, with LDAP and +a database combined, or within other extensions using Guacamole's extension +API. + + * [GUACAMOLE-220](https://issues.apache.org/jira/browse/GUACAMOLE-220) - Implement user groups + +### Clipboard integration with the Asynchronous Clipboard API + +For browsers which implement the [Asynchronous Clipboard +API](https://www.w3.org/TR/clipboard-apis/#async-clipboard-api), Guacamole will +now automatically synchronize the local and remote clipboards. Users will be +prompted to grant clipboard access upon opening Guacamole, and Guacamole will +synchronize the clipboard if access is granted. + +This API [has been implemented in Google Chrome since version +66](https://developers.google.com/web/updates/2018/03/clipboardapi), and other +browsers will likely follow suit. The legacy synchronous clipboard API will +continue to be used as a fallback for browsers that support clipboard access +but lack support for the newer API (Internet Explorer). + + * [GUACAMOLE-559](https://issues.apache.org/jira/browse/GUACAMOLE-559) - Add support for the new Asynchronous Clipboard API + +### Multi-factor authentication with Google Authenticator / TOTP + +Guacamole now has support for TOTP as an additional authentication factor. +TOTP ([Time-based One-Time Password](https://en.wikipedia.org/wiki/Time-based_One-time_Password_algorithm)) +is a [standardized algorithm](https://tools.ietf.org/html/rfc6238) used for +multi-factor authentication. With this new support, Guacamole may be used with +any application or authentication device which supports the TOTP standard, +including the popular Google Authenticator. + + * [GUACAMOLE-96](https://issues.apache.org/jira/browse/GUACAMOLE-96) - Two factor authentication with Google Authenticator + +### Support for RADIUS authentication + +RADIUS support has been added, allowing Guacamole to delegate authentication to +a RADIUS service like FreeRADIUS for validating credentials, enforcing multiple +authentication factors, etc. + +Because the RADIUS library used by this support is licensed under the LGPL, a +convenience binary for this extension is not provided. If you wish to use the +RADIUS support
[GitHub] guacamole-client pull request #353: GUACAMOLE-577: Support for configuring t...
GitHub user necouchman opened a pull request: https://github.com/apache/guacamole-client/pull/353 GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections This pull request implements the changes necessary to support the `GuacamoleProxyConfiguration` in LDAP connections (and any future extensions that make use of the `SimpleConnection` class. Not entirely sure this is the right/best way to go, but took a stab at it. You can merge this pull request into a Git repository by running: $ git pull https://github.com/necouchman/guacamole-client jira/577 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/guacamole-client/pull/353.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #353 commit 60189b1870ffe368893e24cef5c0ed830b108329 Author: Nick Couchman Date: 2019-01-02T16:33:57Z GUACAMOLE-577: Support for configuring the Guacamole Proxy settings in LDAP connections. commit 366804344f1d83ec68243a566ffa3fe3265dcb79 Author: Nick Couchman Date: 2019-01-02T16:39:25Z GUACAMOLE-577: Fix up comments, remove unnecessary code. commit f3a7e7f241ebdb237a1f53eeeb4b58f135f308bc Author: Nick Couchman Date: 2019-01-02T16:41:22Z GUACAMOLE-577: Remove unused code. ---
[GitHub] guacamole-client pull request #352: GUACAMOLE-684: Insufficient credentials ...
GitHub user necouchman opened a pull request: https://github.com/apache/guacamole-client/pull/352 GUACAMOLE-684: Insufficient credentials should take precedence over other failures This resolves an issue where, depending on the order of stacked authentication modules, `GuacamoleInsufficientCredentialsException`s are overriden by `GuacamoleInvalidCredentialsException`s that occur prior to the insufficient exceptions. This particular issue has come up with the RADIUS module when it gets a challenge/response and is stacked on top of the JDBC module, where the JDBC authentication occurs (and fails) prior to the RADIUS authentication attempt, and the additional credentials are never requested. You can merge this pull request into a Git repository by running: $ git pull https://github.com/necouchman/guacamole-client jira/684 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/guacamole-client/pull/352.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #352 commit a710558854092a162161d8e66d1634b5831e1824 Author: Nick Couchman Date: 2018-12-31T17:37:03Z GUACAMOLE-684: Insufficient credentials should take precedence over other credentials errors. ---
[GitHub] guacamole-client pull request #351: GUACAMOLE-683: Add OpenID support in Doc...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/351#discussion_r244603234
--- Diff: guacamole-docker/bin/start.sh ---
@@ -404,6 +404,37 @@ END
ln -s /opt/guacamole/radius/guacamole-auth-*.jar "$GUACAMOLE_EXT"
}
+## Adds properties to guacamole.properties which select the OPENID
+## authentication provider, and configure it to connect to the specified
OPENID
+## provider.
+##
+associate_openid() {
+
+# Verify required parameters are present
+if [ -z "$OPENID_AUTHORIZATION_ENDPOINT" -o -z "$OPENID_CLIENT_ID" ];
then
--- End diff --
According to the OpenID documentation
(http://guacamole.apache.org/doc/gug/openid-auth.html), the following
parameters are required:
- openid-authorization-endpoint
- openid-jwks-endpoint
- openid-issuer
- openid-client-id
- openid-redirect-uri
Seems like we should probably check for all of these as required, here, and
not just the authorization endpoint and client ID?
---
[GitHub] guacamole-client pull request #351: GUACAMOLE-683
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/351#discussion_r244544987 --- Diff: guacamole-docker/bin/build-guacamole.sh --- @@ -126,3 +126,15 @@ if [ -f extensions/guacamole-auth-radius/target/guacamole-auth-radius*.jar ]; th mkdir -p "$DESTINATION/radius" cp extensions/guacamole-auth-radius/target/guacamole-auth-radius*.jar "$DESTINATION/radius" fi + +# Copy OPENID auth extension and schema modifications +# + +mkdir -p "$DESTINATION/openid" +tar -xzf extensions/guacamole-auth-openid/target/*.tar.gz \ --- End diff -- A couple of items, here: - Why not just use the jar file? In the case of LDAP above, it is extracted because the LDIF file is also there. For this module, though, just having the JAR should be sufficient. - All of the slashes at the end of the line should line up. ---
[GitHub] guacamole-server pull request #205: GUACAMOLE-667: Enable loading configurat...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/205#discussion_r244046919
--- Diff: src/guacd/conf-file.c ---
@@ -169,48 +192,40 @@ int guacd_conf_parse_file(guacd_config* conf, int fd)
{
}
-guacd_config* guacd_conf_load() {
-
-guacd_config* conf = malloc(sizeof(guacd_config));
-if (conf == NULL)
-return NULL;
-
-/* Load defaults */
-conf->bind_host = NULL;
-conf->bind_port = strdup("4822");
-conf->pidfile = NULL;
-conf->foreground = 0;
-conf->print_version = 0;
-conf->max_log_level = GUAC_LOG_INFO;
-
-#ifdef ENABLE_SSL
-conf->cert_file = NULL;
-conf->key_file = NULL;
-#endif
+int guacd_conf_load(guacd_config* conf, const char* conf_file_path) {
/* Read configuration from file */
-int fd = open(GUACD_CONF_FILE, O_RDONLY);
+int fd = open(conf_file_path, O_RDONLY);
if (fd > 0) {
int retval = guacd_conf_parse_file(conf, fd);
close(fd);
if (retval != 0) {
-fprintf(stderr, "Unable to parse \"" GUACD_CONF_FILE "\".\n");
-free(conf);
-return NULL;
+fprintf(stderr, "Unable to parse \"%s\".\n", conf_file_path);
+return 1;
}
}
/* Notify of errors preventing reading */
else if (errno != ENOENT) {
-fprintf(stderr, "Unable to open \"" GUACD_CONF_FILE "\": %s\n",
strerror(errno));
-free(conf);
-return NULL;
+fprintf(stderr, "Unable to open \"%s\": %s\n", conf_file_path,
strerror(errno));
+return 1;
}
-return conf;
+/* Load successfully */
+return 0;
}
+int guacd_conf_load_default(guacd_config* conf) {
+
+/* Determine path to the default configuration file and load it */
+char *path = getenv("GUACD_CONF_FILE");
--- End diff --
This may be a bit of a nit-pick, but is loading the location of the config
file from an environment variable really loading a default configuration file?
---
[GitHub] guacamole-server pull request #205: GUACAMOLE-667: Enable loading configurat...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/205#discussion_r244047068 --- Diff: src/guacd/conf-file.h --- @@ -24,6 +24,11 @@ #include "conf.h" +/** + * Creates a new configuration structure, filled with default settings. + */ +guacd_config* guacd_conf_create(); + --- End diff -- Even though it's independent of the parsing, it still seems like it's related to pulling values from the files, so I'm fine with it staying where it is. ---
[GitHub] guacamole-server pull request #205: GUACAMOLE-667: Enable loading configurat...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/205#discussion_r244047256 --- Diff: src/guacd/conf-file.h --- @@ -32,10 +37,17 @@ int guacd_conf_parse_file(guacd_config* conf, int fd); --- End diff -- My only comment here would be to stay consistent with how this is done elsewhere in the code. Are other functions hidden using static, or just not stubbed in header files? Go with the route that provides the most consistency, unless there's a really good reason to do otherwise. ---
[GitHub] guacamole-client pull request #349: GUACAMOLE-678: Implement new UriGuacamol...
GitHub user necouchman opened a pull request: https://github.com/apache/guacamole-client/pull/349 GUACAMOLE-678: Implement new UriGuacamoleProperty This PR implements a new property type, the `UriGuacamoleProperty`, which parses a string into a URI. Several of the extensions were converted to use the new property, and I also added properties to the JDBC modules that allow for configuring the connection with a single URI property rather than the individual components. Providing this type of property allows for validation of URI/URLs in the configuration file, catching errors when the configuration is loaded. The other advantage of using URI is that, from what I have read, it takes care of all of the encoding of the various URL parts internal, which avoids the need to explicitly use `URLEncoder`. For the JDBC modules, I configured it such that, if the URI is specified, the other module parameters are completely ignored. This could go the other direction where the URI is the base and the others override it, if desired - it would be a pretty easy change. You can merge this pull request into a Git repository by running: $ git pull https://github.com/necouchman/guacamole-client jira/678 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/guacamole-client/pull/349.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #349 commit 1366b6e78743c35a8511cd949cffe8f8f296a443 Author: Nick Couchman Date: 2018-12-25T03:08:57Z GUACAMOLE-678: Implement new UriGuacamoleProperty. commit adf09f5edebea522c68bfd3aa71ecd5411771d3e Author: Nick Couchman Date: 2018-12-25T20:24:55Z GUACAMOLE-678: Use new UriGuacamoleProperty for extension configuration. ---
[GitHub] guacamole-client pull request #254: GUACAMOLE-103: Implement SAML Authentica...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/254#discussion_r243860177
--- Diff:
extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/AuthenticationProviderService.java
---
@@ -0,0 +1,202 @@
+/*
+ * 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.auth.saml;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.onelogin.saml2.authn.AuthnRequest;
+import com.onelogin.saml2.authn.SamlResponse;
+import com.onelogin.saml2.exception.SettingsException;
+import com.onelogin.saml2.exception.ValidationError;
+import com.onelogin.saml2.http.HttpRequest;
+import com.onelogin.saml2.servlet.ServletUtils;
+import com.onelogin.saml2.settings.Saml2Settings;
+import com.onelogin.saml2.util.Util;
+import java.io.IOException;
+import java.util.Arrays;
+import javax.servlet.http.HttpServletRequest;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.xpath.XPathExpressionException;
+import org.apache.guacamole.auth.saml.conf.ConfigurationService;
+import org.apache.guacamole.auth.saml.form.SAMLRedirectField;
+import org.apache.guacamole.auth.saml.user.AuthenticatedUser;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.form.Field;
+import org.apache.guacamole.net.auth.Credentials;
+import org.apache.guacamole.net.auth.credentials.CredentialsInfo;
+import
org.apache.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException;
+import
org.apache.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.xml.sax.SAXException;
+
+/**
+ * Class that provides services for use by the SAML Authentication
Provider class.
+ */
+public class AuthenticationProviderService {
+
+/**
+ * Logger for this class.
+ */
+private final Logger logger =
LoggerFactory.getLogger(AuthenticationProviderService.class);
+
+/**
+ * Service for retrieving SAML configuration information.
+ */
+@Inject
+private ConfigurationService confService;
+
+/**
+ * Provider for AuthenticatedUser objects.
+ */
+@Inject
+private Provider authenticatedUserProvider;
+
+/**
+ * Returns an AuthenticatedUser representing the user authenticated by
the
+ * given credentials.
+ *
+ * @param credentials
+ * The credentials to use for authentication.
+ *
+ * @return
+ * An AuthenticatedUser representing the user authenticated by the
+ * given credentials.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while authenticating the user, or if access
is
+ * denied.
+ */
+public AuthenticatedUser authenticateUser(Credentials credentials)
+throws GuacamoleException {
+
+HttpServletRequest request = credentials.getRequest();
+
+// Initialize and configure SAML client.
+Saml2Settings samlSettings = confService.getSamlSettings();
+
+if (request != null) {
+
+// Look for the SAML Response parameter.
+String samlResponseParam =
request.getParameter("SAMLResponse");
+
+if (samlResponseParam != null) {
+
+// Convert the SAML response into the version needed for
the client.
+HttpRequest httpRequest =
ServletUtils.makeHttpRequest(request);
+try {
+
+// Generate the response object
+SamlResponse samlResponse = new
SamlResponse(samlSettings, httpRequest);
+
+if (!samlResponse.validateNumAssertions()) {
+
[GitHub] guacamole-client pull request #348: GUACAMOLE-422: Forward Timezone to RDP a...
GitHub user necouchman opened a pull request: https://github.com/apache/guacamole-client/pull/348 GUACAMOLE-422: Forward Timezone to RDP and SSH Connections These are the client-side changes to allow both detection of (via JSTZ) and overriding of the client timezone for remote connections (SSH and RDP). You can merge this pull request into a Git repository by running: $ git pull https://github.com/necouchman/guacamole-client jira/422 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/guacamole-client/pull/348.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #348 commit 632475adab8179147dfbb9798be4decd646db47d Author: Nick Couchman Date: 2018-06-02T12:47:41Z GUACAMOLE-422: Add timezone selection for RDP connections. commit 0af7ea02d86c4e55a595c864508e955fe3a28493 Author: Nick Couchman Date: 2018-06-03T01:04:01Z GUACAMOLE-422: Implement client-side timezone detection. commit 0c931e0cd126c1c6938818723267453a7d4aa193 Author: Nick Couchman Date: 2018-06-03T12:41:35Z GUACAMOLE-422: Add tunnel parameter for sending the timezone. commit eddfb10ef0ec05603a8480490af38098266f7178 Author: Nick Couchman Date: 2018-06-03T12:48:46Z GUACAMOLE-422: Add field for timezone for SSH connections. commit 2c4c1b056fd7dd2835e5e785755c24eadfa109c3 Author: Nick Couchman Date: 2018-06-03T18:59:47Z GUACAMOLE-422: Add timezone to tunnel connections. commit f18fe3ede1f25fdb8a2380678b7ad12207a6901f Author: Nick Couchman Date: 2018-06-03T19:02:03Z GUACAMOLE-422: Correct javadoc tag. commit f295ddf67fef1c5c60477f3cac33b6b9e5e219f5 Author: Nick Couchman Date: 2018-06-04T10:37:31Z GUACAMOLE-422: Revert weird addition of timezone field in JDBC module. commit bcdd429d0f53c6da78b52690bd2629abd86f3d3c Author: Nick Couchman Date: 2018-06-04T10:45:10Z GUACAMOLE-422: Remove extra space in pom.xml ---
[GitHub] guacamole-client pull request #347: GUACAMOLE-682: docker build with optiona...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/347#discussion_r243608567 --- Diff: guacamole-docker/bin/build-guacamole.sh --- @@ -53,7 +54,12 @@ mkdir -p "$DESTINATION" # cd "$BUILD_DIR" -mvn package + +if [ -z "$BUILD_PROFILE" ]; then + mvn package +else + mvn -P "$BUILD_PROFILE" package +fi --- End diff -- Please stick with accepted style of four-space indentations. ---
[GitHub] guacamole-client pull request #347: GUACAMOLE-682: docker build with optiona...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/347#discussion_r243608670 --- Diff: guacamole-docker/bin/build-guacamole.sh --- @@ -107,3 +113,11 @@ tar -xzf extensions/guacamole-auth-ldap/target/*.tar.gz \ "*.jar" \ "*.ldif" +# +# Copy Radius auth extension if it was build +# + +if [ -f extensions/guacamole-auth-radius/target/guacamole-auth-radius*.jar ]; then + mkdir -p "$DESTINATION/radius" + cp extensions/guacamole-auth-radius/target/guacamole-auth-radius*.jar "$DESTINATION/radius" --- End diff -- As with above, four-space indentations, please ---
[GitHub] guacamole-client pull request #346: GUACAMOLE-680: Rearrange logout handling
GitHub user necouchman opened a pull request: https://github.com/apache/guacamole-client/pull/346 GUACAMOLE-680: Rearrange logout handling Per discussion on the mailing list, I've taken a stab at renaming the pre-logout broadcast and implenting a post-logout broadcast in order to set up the possibility that events could be fired by the web application after the Guacamole logout has completed. No idea if this is what was intended, but took a shot... [1] - http://mail-archives.apache.org/mod_mbox/guacamole-dev/201812.mbox/%3CCALKeL-O1XQBAGFE17_jgB_OC5%2Bf-uCCEgfOiHbn39XyGRm7s6Q%40mail.gmail.com%3E You can merge this pull request into a Git repository by running: $ git pull https://github.com/necouchman/guacamole-client jira/680 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/guacamole-client/pull/346.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #346 commit 341335ab7a9a4430728926afa74cfdafe0482aef Author: Nick Couchman Date: 2018-12-16T01:03:56Z GUACAMOLE-680: Change guacLogout to beforeGuacLogout. commit c0a0c869fbc634ea746355c15b5c7965a17a36b1 Author: Nick Couchman Date: 2018-12-16T02:58:17Z GUACAMOLE-680: Add a broadcast after the logout has been completed. ---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241958169
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java
---
@@ -240,17 +244,24 @@ public LDAPAuthenticatedUser
authenticateUser(Credentials credentials)
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
+Dn authDn = new Dn(ldapConnectionConfig.getName());
--- End diff --
This does seem to work (now that I cleared up the other things that were
wrong), but let me know if it's a reasonable approach or if I should try
something else.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241933797
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java
---
@@ -188,46 +183,50 @@ public String generateQuery(String filter,
* information required to execute the query cannot be read from
* guacamole.properties.
*/
-public List search(LDAPConnection ldapConnection,
-String baseDN, String query) throws GuacamoleException {
+public List search(LdapConnection ldapConnection,
+Dn baseDN, ExprNode query) throws GuacamoleException {
logger.debug("Searching \"{}\" for objects matching \"{}\".",
baseDN, query);
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
+
// Search within subtree of given base DN
-LDAPSearchResults results = ldapConnection.search(baseDN,
-LDAPConnection.SCOPE_SUB, query, null, false,
-confService.getLDAPSearchConstraints());
+SearchRequest request = ldapService.getSearchRequest(baseDN,
+query);
+
+SearchCursor results = ldapConnection.search(request);
// Produce list of all entries in the search result,
automatically
// following referrals if configured to do so
-List entries = new ArrayList<>(results.getCount());
-while (results.hasMore()) {
+List entries = new ArrayList<>();
+while (results.next()) {
-try {
-entries.add(results.next());
+Response response = results.get();
+if (response instanceof SearchResultEntry) {
+entries.add(((SearchResultEntry) response).getEntry());
}
-
-// Warn if referrals cannot be followed
-catch (LDAPReferralException e) {
-if (confService.getFollowReferrals()) {
-logger.error("Could not follow referral: {}",
e.getFailedReferral());
-logger.debug("Error encountered trying to follow
referral.", e);
-throw new GuacamoleServerException("Could not
follow LDAP referral.", e);
-}
-else {
-logger.warn("Given a referral, but referrals are
disabled. Error was: {}", e.getMessage());
-logger.debug("Got a referral, but configured to
not follow them.", e);
+else if (response instanceof SearchResultReference &&
+request.isFollowReferrals()) {
+
+Referral referral = ((SearchResultReference)
response).getReferral();
+int referralHop = 0;
+for (String url : referral.getLdapUrls()) {
+LdapConnection referralConnection =
ldapService.referralConnection(
+new LdapUrl(url), ldapConnectionConfig,
referralHop++);
+entries.addAll(search(referralConnection, baseDN,
query));
--- End diff --
Took a stab at this, as well - not entirely sure this is the best/cleanest
way, but let me know what you think. As usual, I'm open to suggestions for how
to approach it.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241928473
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/LdapDnGuacamoleProperty.java
---
@@ -0,0 +1,50 @@
+/*
+ * 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.auth.ldap.conf;
+
+import
org.apache.directory.api.ldap.model.exception.LdapInvalidDnException;
+import org.apache.directory.api.ldap.model.name.Dn;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.properties.GuacamoleProperty;
+
+/**
+ * A GuacamoleProperty that converts a string to a Dn that can be used
+ * in LDAP connections. An exception is thrown if the provided DN is
invalid
+ * and cannot be parsed.
+ */
+public abstract class LdapDnGuacamoleProperty implements
GuacamoleProperty {
+
+@Override
+public Dn parseValue(String value) throws GuacamoleException {
+
+if (value == null)
+return null;
+
+try {
+return new Dn(value);
+}
+catch (LdapInvalidDnException e) {
+throw new GuacamoleServerException("Invalid DN specified in
configuration.", e);
--- End diff --
Reworded.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241928458
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/LdapFilterGuacamoleProperty.java
---
@@ -0,0 +1,53 @@
+/*
+ * 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.auth.ldap.conf;
+
+import java.text.ParseException;
+import org.apache.directory.api.ldap.model.filter.ExprNode;
+import org.apache.directory.api.ldap.model.filter.FilterParser;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.properties.GuacamoleProperty;
+
+/**
+ * A GuacamoleProperty with a value of an ExprNode query filter. The
string
+ * provided is passed through the FilterParser returning the ExprNode
object,
+ * or an exception is thrown if the filter is invalid and cannot be
correctly
+ * parsed.
+ */
+public abstract class LdapFilterGuacamoleProperty implements
GuacamoleProperty {
+
+@Override
+public ExprNode parseValue(String value) throws GuacamoleException {
+
+// No value provided, so return null.
+if (value == null)
+return null;
+
+try {
+return FilterParser.parse(value);
+}
+catch (ParseException e) {
+throw new GuacamoleServerException("Error parsing filter", e);
--- End diff --
Reworded.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241928420
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java
---
@@ -156,38 +146,84 @@ public LDAPConnection bindAs(String userDN, String
password)
// Bind using provided credentials
try {
-byte[] passwordBytes;
-try {
-
-// Convert password into corresponding byte array
-if (password != null)
-passwordBytes = password.getBytes("UTF-8");
-else
-passwordBytes = null;
-
-}
-catch (UnsupportedEncodingException e) {
-logger.error("Unexpected lack of support for UTF-8: {}",
e.getMessage());
-logger.debug("Support for UTF-8 (as required by Java spec)
not found.", e);
-disconnect(ldapConnection);
-return null;
-}
-
-// Bind as user
-ldapConnection.bind(LDAPConnection.LDAP_V3, userDN,
passwordBytes);
+BindRequest bindRequest = new BindRequestImpl();
+bindRequest.setDn(userDN);
+bindRequest.setCredentials(password);
+ldapConnection.bind(bindRequest);
}
// Disconnect if an error occurs during bind
-catch (LDAPException e) {
-logger.debug("LDAP bind failed.", e);
+catch (LdapException e) {
+logger.debug("Unable to bind to LDAP server.", e);
disconnect(ldapConnection);
return null;
}
return ldapConnection;
}
+
+/**
+ * Generate a new LdapConnection object for following a referral
+ * with the given LdapUrl, and copy the username and password
+ * from the original connection.
+ *
+ * @param referralUrl
+ * The LDAP URL to follow.
+ *
+ * @param ldapConfig
+ * The connection configuration to use to retrieve username and
+ * password.
+ *
+ * @param hop
+ * The current hop number of this referral - once the configured
+ * limit is reached, this method will throw an exception.
+ *
+ * @return
+ * A LdapConnection object that points at the location
+ * specified in the referralUrl.
+ *
+ * @throws GuacamoleException
+ * If an error occurs parsing out the LdapUrl object or the
+ * maximum number of referral hops is reached.
+ */
+public LdapConnection referralConnection(LdapUrl referralUrl,
+LdapConnectionConfig ldapConfig, Integer hop)
--- End diff --
Fixed.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241928440
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java
---
@@ -188,46 +183,50 @@ public String generateQuery(String filter,
* information required to execute the query cannot be read from
* guacamole.properties.
*/
-public List search(LDAPConnection ldapConnection,
-String baseDN, String query) throws GuacamoleException {
+public List search(LdapConnection ldapConnection,
+Dn baseDN, ExprNode query) throws GuacamoleException {
logger.debug("Searching \"{}\" for objects matching \"{}\".",
baseDN, query);
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
+
// Search within subtree of given base DN
-LDAPSearchResults results = ldapConnection.search(baseDN,
-LDAPConnection.SCOPE_SUB, query, null, false,
-confService.getLDAPSearchConstraints());
+SearchRequest request = ldapService.getSearchRequest(baseDN,
+query);
+
+SearchCursor results = ldapConnection.search(request);
// Produce list of all entries in the search result,
automatically
// following referrals if configured to do so
-List entries = new ArrayList<>(results.getCount());
-while (results.hasMore()) {
+List entries = new ArrayList<>();
+while (results.next()) {
-try {
-entries.add(results.next());
+Response response = results.get();
+if (response instanceof SearchResultEntry) {
--- End diff --
Reworked to avoid this using the calls you mentioned.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241919019
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java
---
@@ -240,17 +244,24 @@ public LDAPAuthenticatedUser
authenticateUser(Credentials credentials)
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
--- End diff --
Unfortunately a different LdapConnection implementation does get used -
LdapReferralConnection. I might be able to clean up some of it, but I think
some of the typecasting will have to stay. We'll see.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241916742
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/LdapDnGuacamoleProperty.java
---
@@ -0,0 +1,50 @@
+/*
+ * 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.auth.ldap.conf;
+
+import
org.apache.directory.api.ldap.model.exception.LdapInvalidDnException;
+import org.apache.directory.api.ldap.model.name.Dn;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.properties.GuacamoleProperty;
+
+/**
+ * A GuacamoleProperty that converts a string to a Dn that can be used
+ * in LDAP connections. An exception is thrown if the provided DN is
invalid
+ * and cannot be parsed.
+ */
+public abstract class LdapDnGuacamoleProperty implements
GuacamoleProperty {
+
+@Override
+public Dn parseValue(String value) throws GuacamoleException {
+
+if (value == null)
+return null;
+
+try {
+return new Dn(value);
+}
+catch (LdapInvalidDnException e) {
+throw new GuacamoleServerException("Invalid DN specified in
configuration.", e);
--- End diff --
Sounds good, I'll rework this error a bit.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241916665
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java
---
@@ -188,46 +183,50 @@ public String generateQuery(String filter,
* information required to execute the query cannot be read from
* guacamole.properties.
*/
-public List search(LDAPConnection ldapConnection,
-String baseDN, String query) throws GuacamoleException {
+public List search(LdapConnection ldapConnection,
+Dn baseDN, ExprNode query) throws GuacamoleException {
logger.debug("Searching \"{}\" for objects matching \"{}\".",
baseDN, query);
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
+
// Search within subtree of given base DN
-LDAPSearchResults results = ldapConnection.search(baseDN,
-LDAPConnection.SCOPE_SUB, query, null, false,
-confService.getLDAPSearchConstraints());
+SearchRequest request = ldapService.getSearchRequest(baseDN,
+query);
+
+SearchCursor results = ldapConnection.search(request);
// Produce list of all entries in the search result,
automatically
// following referrals if configured to do so
-List entries = new ArrayList<>(results.getCount());
-while (results.hasMore()) {
+List entries = new ArrayList<>();
+while (results.next()) {
-try {
-entries.add(results.next());
+Response response = results.get();
+if (response instanceof SearchResultEntry) {
+entries.add(((SearchResultEntry) response).getEntry());
}
-
-// Warn if referrals cannot be followed
-catch (LDAPReferralException e) {
-if (confService.getFollowReferrals()) {
-logger.error("Could not follow referral: {}",
e.getFailedReferral());
-logger.debug("Error encountered trying to follow
referral.", e);
-throw new GuacamoleServerException("Could not
follow LDAP referral.", e);
-}
-else {
-logger.warn("Given a referral, but referrals are
disabled. Error was: {}", e.getMessage());
-logger.debug("Got a referral, but configured to
not follow them.", e);
+else if (response instanceof SearchResultReference &&
+request.isFollowReferrals()) {
+
+Referral referral = ((SearchResultReference)
response).getReferral();
+int referralHop = 0;
+for (String url : referral.getLdapUrls()) {
+LdapConnection referralConnection =
ldapService.referralConnection(
+new LdapUrl(url), ldapConnectionConfig,
referralHop++);
+entries.addAll(search(referralConnection, baseDN,
query));
--- End diff --
Yeah, you're probably right. The entire recursion of this function was
kind of a wild stab, so I'm not surprised I made a few mistakes along the way.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241916587
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java
---
@@ -188,46 +183,50 @@ public String generateQuery(String filter,
* information required to execute the query cannot be read from
* guacamole.properties.
*/
-public List search(LDAPConnection ldapConnection,
-String baseDN, String query) throws GuacamoleException {
+public List search(LdapConnection ldapConnection,
+Dn baseDN, ExprNode query) throws GuacamoleException {
logger.debug("Searching \"{}\" for objects matching \"{}\".",
baseDN, query);
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
+
// Search within subtree of given base DN
-LDAPSearchResults results = ldapConnection.search(baseDN,
-LDAPConnection.SCOPE_SUB, query, null, false,
-confService.getLDAPSearchConstraints());
+SearchRequest request = ldapService.getSearchRequest(baseDN,
+query);
+
+SearchCursor results = ldapConnection.search(request);
// Produce list of all entries in the search result,
automatically
// following referrals if configured to do so
-List entries = new ArrayList<>(results.getCount());
-while (results.hasMore()) {
+List entries = new ArrayList<>();
+while (results.next()) {
-try {
-entries.add(results.next());
+Response response = results.get();
+if (response instanceof SearchResultEntry) {
--- End diff --
I'll take a closer look.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241916353
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java
---
@@ -240,17 +244,24 @@ public LDAPAuthenticatedUser
authenticateUser(Credentials credentials)
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
--- End diff --
I'll see what makes the most sense - maybe just converting everything to
LdapNetworkConnection is the most straight-forward, since that's really the
only implementation that ever gets used.
---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/336#discussion_r238684733 --- Diff: extensions/guacamole-auth-vault/.gitignore --- @@ -0,0 +1,2 @@ +target/ --- End diff -- Looks like these .gitignore files are not present in most of the other modules, at least at the base. Even the jdbc module doesn't have it at the root? ---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r240048911
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-base/src/main/java/org/apache/guacamole/auth/vault/user/VaultUserContext.java
---
@@ -0,0 +1,325 @@
+/*
+ * 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.auth.vault.user;
+
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.auth.vault.conf.VaultConfigurationService;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.ConnectionGroup;
+import org.apache.guacamole.net.auth.TokenInjectingUserContext;
+import org.apache.guacamole.net.auth.UserContext;
+import org.apache.guacamole.auth.vault.secret.VaultSecretService;
+import org.apache.guacamole.protocol.GuacamoleConfiguration;
+import org.apache.guacamole.token.GuacamoleTokenUndefinedException;
+import org.apache.guacamole.token.TokenFilter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * UserContext implementation which automatically injects tokens
containing the
+ * values of secrets retrieved from a vault.
+ */
+public class VaultUserContext extends TokenInjectingUserContext {
+
+/**
+ * Logger for this class.
+ */
+private final Logger logger =
LoggerFactory.getLogger(VaultUserContext.class);
+
+/**
+ * The name of the token which will be replaced with the username of
the
+ * current user if specified within the name of a secret. This token
+ * applies to both connections and connection groups.
+ */
+private static final String USERNAME_TOKEN = "GUAC_USERNAME";
+
+/**
+ * The name of the token which will be replaced with the name of the
+ * current connection group if specified within the name of a secret.
This
+ * token only applies only to connection groups.
+ */
+private static final String CONNECTION_GROUP_NAME_TOKEN =
"CONNECTION_GROUP_NAME";
+
+/**
+ * The name of the token which will be replaced with the identifier of
the
+ * current connection group if specified within the name of a secret.
This
+ * token only applies only to connection groups.
+ */
+private static final String CONNECTION_GROUP_IDENTIFIER_TOKEN =
"CONNECTION_GROUP_ID";
+
+/**
+ * The name of the token which will be replaced with the \"hostname\"
+ * connection parameter of the current connection if specified within
the
+ * name of a secret. This token only applies only to connections.
+ */
+private static final String CONNECTION_HOSTNAME_TOKEN =
"CONNECTION_HOSTNAME";
+
+/**
+ * The name of the token which will be replaced with the \"username\"
+ * connection parameter of the current connection if specified within
the
+ * name of a secret. This token only applies only to connections.
+ */
+private static final String CONNECTION_USERNAME_TOKEN =
"CONNECTION_USERNAME";
+
+/**
+ * The name of the token which will be replaced with the name of the
+ * current connection if specified within the name of a secret. This
token
+ * only applies only to connections.
+ */
+private static final String CONNECTION_NAME_TOKEN = "CONNECTION_NAME";
+
+/**
+ * The name of the token which will be replaced with the ide
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r238686408
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-azure/src/main/java/org/apache/guacamole/auth/vault/azure/AzureKeyVaultAuthenticationProviderModule.java
---
@@ -0,0 +1,61 @@
+/*
+ * 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.auth.vault.azure;
+
+import com.microsoft.azure.keyvault.authentication.KeyVaultCredentials;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.vault.VaultAuthenticationProviderModule;
+import
org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultConfigurationService;
+import org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultCredentials;
+import
org.apache.guacamole.auth.vault.azure.secret.AzureKeyVaultSecretService;
+import org.apache.guacamole.auth.vault.conf.VaultConfigurationService;
+import org.apache.guacamole.auth.vault.secret.VaultSecretService;
+
+/**
+ * Guice module which configures injections specific to Azure Key Vault
+ * support.
+ */
+public class AzureKeyVaultAuthenticationProviderModule
+extends VaultAuthenticationProviderModule {
+
+/**
+ * Creates a new AzureKeyVaultAuthenticationiProviderModule which
--- End diff --
AzureKeyVaultAuthenticationiProviderModule ->
AzureKeyVaultAuthenticationProviderModule
(extra i)
---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r240043908
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-azure/src/main/java/org/apache/guacamole/auth/vault/azure/secret/AzureKeyVaultSecretService.java
---
@@ -0,0 +1,121 @@
+/*
+ * 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.auth.vault.azure.secret;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.microsoft.azure.keyvault.KeyVaultClient;
+import com.microsoft.azure.keyvault.authentication.KeyVaultCredentials;
+import com.microsoft.azure.keyvault.models.SecretBundle;
+import com.microsoft.rest.ServiceCallback;
+import java.util.concurrent.CompletableFuture;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import org.apache.guacamole.GuacamoleException;
+import
org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultAuthenticationException;
+import
org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultConfigurationService;
+import org.apache.guacamole.auth.vault.secret.CachedVaultSecretService;
+
+/**
+ * Service which retrieves secrets from Azure Key Vault.
+ */
+@Singleton
+public class AzureKeyVaultSecretService extends CachedVaultSecretService {
+
+/**
+ * Pattern which matches contiguous groups of characters which are not
+ * allowed within Azure Key Vault secret names.
+ */
+private static final Pattern DISALLOWED_CHARACTERS =
Pattern.compile("[^a-zA-Z0-9-]+");
+
+/**
+ * Service for retrieving configuration information.
+ */
+@Inject
+private AzureKeyVaultConfigurationService confService;
+
+/**
+ * Provider for Azure Key Vault credentials.
+ */
+@Inject
+private Provider credentialProvider;
+
+/**
+ * {@inheritDoc}
+ *
+ * Azure Key Vault allows strictly a-z, A-Z, 0-9, and "-". This
--- End diff --
Is this `` here because of the `{@inheritDoc}` tag above? I know we've
discussed this before, but it's been a while...
---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r240044367
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-base/src/main/java/org/apache/guacamole/auth/vault/user/VaultUserContext.java
---
@@ -0,0 +1,325 @@
+/*
+ * 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.auth.vault.user;
+
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.auth.vault.conf.VaultConfigurationService;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.ConnectionGroup;
+import org.apache.guacamole.net.auth.TokenInjectingUserContext;
+import org.apache.guacamole.net.auth.UserContext;
+import org.apache.guacamole.auth.vault.secret.VaultSecretService;
+import org.apache.guacamole.protocol.GuacamoleConfiguration;
+import org.apache.guacamole.token.GuacamoleTokenUndefinedException;
+import org.apache.guacamole.token.TokenFilter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * UserContext implementation which automatically injects tokens
containing the
+ * values of secrets retrieved from a vault.
+ */
+public class VaultUserContext extends TokenInjectingUserContext {
+
+/**
+ * Logger for this class.
+ */
+private final Logger logger =
LoggerFactory.getLogger(VaultUserContext.class);
--- End diff --
Just curious, in the past I think the `logger` object has generally been
declared `private static final` - is there a reason why it's better to/not to
use `static` for those? I think a lot of the recent changes have not included
the `static` keyword, so not sure if there's a reason things have trended in
that direction. It'd be good to be consistent about it.
---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r240043831
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-azure/src/main/java/org/apache/guacamole/auth/vault/azure/conf/AzureKeyVaultCredentials.java
---
@@ -0,0 +1,115 @@
+/*
+ * 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.auth.vault.azure.conf;
+
+import com.google.inject.Inject;
+import com.microsoft.aad.adal4j.AuthenticationContext;
+import com.microsoft.aad.adal4j.AuthenticationResult;
+import com.microsoft.aad.adal4j.ClientCredential;
+import com.microsoft.azure.keyvault.authentication.KeyVaultCredentials;
+import java.net.MalformedURLException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import org.apache.guacamole.GuacamoleException;
+
+/**
+ * KeyVaultCredentials implementation which retrieves the required client
ID
+ * and key from guacamole.properties. Note that KeyVaultCredentials as
+ * implemented in the Azure Java SDK is NOT THREADSAFE; it leverages a
+ * non-concurrent HashMap for authentication result caching and does not
+ * perform any synchronization.
+ */
+public class AzureKeyVaultCredentials extends KeyVaultCredentials {
+
+/**
+ * Service for retrieving configuration information.
+ */
+@Inject
+private AzureKeyVaultConfigurationService confService;
+
+/**
+ * {@inheritDoc}
+ *
+ * @throws AzureKeyVaultAuthenticationException
+ * If an error occurs preventing successful authentication. Note
that
+ * this exception is unchecked. Uses of this class which need to be
+ * aware of errors in the authentication process must manually
catch
+ * this exception.
+ */
+@Override
+public String doAuthenticate(String authorization, String resource,
+String scope) throws AzureKeyVaultAuthenticationException {
+
+// Read Azure credentials from guacamole.properties
+ClientCredential credentials;
+try {
+credentials = confService.getClientCredentials();
+}
+catch (GuacamoleException e) {
--- End diff --
What's the rationale for not passing this through as a `GuacamoleException`?
---
[GitHub] guacamole-client pull request #204: GUACAMOLE-234: Migration from JLDAP to A...
Github user necouchman closed the pull request at: https://github.com/apache/guacamole-client/pull/204 ---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
GitHub user necouchman opened a pull request: https://github.com/apache/guacamole-client/pull/345 GUACAMOLE-234: Migrate to Apache Directory API for LDAP Extension This change request takes a new stab at migrating from the legacy Novell JLDAP API to the Apache Directory API for LDAP access. I've tried to make sure we're using as much of the API as possible for parsing and verification and eliminated overlapping functionality. I'm sure there are some more optimizations that could be done, but this pretty well takes care of things for now, I think. I have tested it against ActiveDirectory and it seems to work fine, but it'd be nice if others could test it against other directories. You can merge this pull request into a Git repository by running: $ git pull https://github.com/necouchman/guacamole-client jira/234 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/guacamole-client/pull/345.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #345 commit 6643923a61fbd97866d889eea78194e090c0a374 Author: Nick Couchman Date: 2018-12-09T13:32:16Z GUACAMOLE-234: Convert LDAP extension to use Apache Directory LDAP API. commit 37022bb552e60631033f3630bc0a0545f9c6f1d7 Author: Nick Couchman Date: 2018-12-09T14:48:01Z GUACAMOLE-234: Clean up some LDAP implementation details. commit 2abd46b0e130548fe1219de7d6369b724c80fa55 Author: Nick Couchman Date: 2018-12-09T15:42:12Z GUACAMOLE-234: Clean up comments. commit 197f5fe7997cc1f8a636e9c114e2dd967b8ae4a1 Author: Nick Couchman Date: 2018-12-09T15:55:39Z GUACAMOLE-234: Exclude slf4j from Apache Directory dependency. ---
[GitHub] guacamole-client pull request #342: GUACAMOLE-598: Ignore if current user ha...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/342#discussion_r239272644
--- Diff:
guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js ---
@@ -95,13 +95,10 @@ angular.module('navigation').directive('guacUserMenu',
[function guacUserMenu()
*/
$scope.role = null;
-// Pull user data
+// Display user profile attributes if available
userService.getUser(authenticationService.getDataSource(),
$scope.username)
.then(function userRetrieved(user) {
-// Store retrieved user object
-$scope.user = user;
--- End diff --
Ping @mike-jumper - just wanted to verify this and then I think I can push
forward with merging this PR.
---
[GitHub] guacamole-client pull request #343: GUACAMOLE-526: Correct redirect issue wi...
GitHub user necouchman opened a pull request: https://github.com/apache/guacamole-client/pull/343 GUACAMOLE-526: Correct redirect issue with CAS module These changes clean up code and correct a minor typo that caused the CAS redirect to fail. You can merge this pull request into a Git repository by running: $ git pull https://github.com/necouchman/guacamole-client jira/671 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/guacamole-client/pull/343.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #343 commit df8c07c18755d0742df20d2e66c5e6d5881129d8 Author: Nick Couchman Date: 2018-12-02T21:34:16Z GUACAMOLE-526: Fix loading of CAS ticket field and redirection. commit 29982e3a855fcec2fae4c0d37e2e0ad71b1ca726 Author: Nick Couchman Date: 2018-12-05T00:16:57Z GUACAMOLE-526: Clean up unused code and minor errors. ---
[GitHub] guacamole-client pull request #342: GUACAMOLE-598: Ignore if current user ha...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/342#discussion_r238886562
--- Diff:
guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js ---
@@ -95,13 +95,10 @@ angular.module('navigation').directive('guacUserMenu',
[function guacUserMenu()
*/
$scope.role = null;
-// Pull user data
+// Display user profile attributes if available
userService.getUser(authenticationService.getDataSource(),
$scope.username)
.then(function userRetrieved(user) {
-// Store retrieved user object
-$scope.user = user;
--- End diff --
Just want to make sure this doesn't have any impact? Was this $scope.user
not used at all?
---
[GitHub] guacamole-server pull request #207: GUACAMOLE-673: Create automatically need...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/207#discussion_r238667965
--- Diff: src/protocols/rdp/guac_rdpdr/rdpdr_fs_service.c ---
@@ -153,5 +153,42 @@ void guac_rdpdr_register_fs(guac_rdpdrPlugin* rdpdr,
char* drive_name) {
/* Init data */
device->data = rdp_client->filesystem;
+ /* Init directory for file transfer */
--- End diff --
Just off the bat, please follow the style used throughout the rest of the
code - each tab should be 4 spaces.
See: http://guacamole.apache.org/guac-style/
---
[GitHub] guacamole-server pull request #207: GUACAMOLE-673: Create automatically need...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/207#discussion_r238668883
--- Diff: src/protocols/rdp/guac_rdpdr/rdpdr_fs_service.c ---
@@ -153,5 +153,42 @@ void guac_rdpdr_register_fs(guac_rdpdrPlugin* rdpdr,
char* drive_name) {
/* Init data */
device->data = rdp_client->filesystem;
+ /* Init directory for file transfer */
+ int create_fs = 0;
+
+ if (rdp_client) {
--- End diff --
Not sure this check is necessary, here - rdp_client is used previously in
here under the assumption that it is not null, so this is probably not
necessary.
---
[GitHub] guacamole-server pull request #207: GUACAMOLE-673: Create automatically need...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/207#discussion_r238670994
--- Diff: src/protocols/rdp/guac_rdpdr/rdpdr_fs_service.c ---
@@ -153,5 +153,42 @@ void guac_rdpdr_register_fs(guac_rdpdrPlugin* rdpdr,
char* drive_name) {
/* Init data */
device->data = rdp_client->filesystem;
+ /* Init directory for file transfer */
+ int create_fs = 0;
+
+ if (rdp_client) {
+ guac_rdp_settings* settings = rdp_client->settings;
+ if (settings) {
+ create_fs = settings->create_drive_path;
+ }
+ else {
+ guac_client_log(device->rdpdr->client, GUAC_LOG_ERROR,
"No settings.");
+ }
+ }
+ else {
+ guac_client_log(device->rdpdr->client, GUAC_LOG_ERROR, "No rdp
client.");
+ }
+
+ if (create_fs) {
+
+ /* Get filesystem, return error if no filesystem */
+ guac_rdp_fs* fs = rdp_client->filesystem;
+ if (fs == NULL) {
+ guac_client_log(device->rdpdr->client, GUAC_LOG_ERROR,
"No filesystem.");
+ return;
--- End diff --
A couple of issues, here. First, the `rdp_client->filesystem` is already
assigned to `device->data` above - not sure there's any reason to re-assign it,
here? Second, I don't know that the null check is required, here, either. I'd
need to go back and look at the code that calls this `guac_rdpdr_register_fs`
function, but I think it might be previously checked?
---
[GitHub] guacamole-server pull request #207: GUACAMOLE-673: Create automatically need...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/207#discussion_r238669729
--- Diff: src/protocols/rdp/guac_rdpdr/rdpdr_fs_service.c ---
@@ -153,5 +153,42 @@ void guac_rdpdr_register_fs(guac_rdpdrPlugin* rdpdr,
char* drive_name) {
/* Init data */
device->data = rdp_client->filesystem;
+ /* Init directory for file transfer */
+ int create_fs = 0;
+
+ if (rdp_client) {
+ guac_rdp_settings* settings = rdp_client->settings;
+ if (settings) {
+ create_fs = settings->create_drive_path;
+ }
+ else {
+ guac_client_log(device->rdpdr->client, GUAC_LOG_ERROR,
"No settings.");
+ }
+ }
+ else {
+ guac_client_log(device->rdpdr->client, GUAC_LOG_ERROR, "No rdp
client.");
+ }
+
+ if (create_fs) {
--- End diff --
My guess is that this and a bunch of the code above could be simplified
into something like this:
if (rdp_client->settings->create_drive_path)
Since both `rdp_client` and `rdp_client->settings` should be initialized
and defined by this point in the code.
---
[GitHub] guacamole-client pull request #341: GUACAMOLE-670: Add slf4j logging to CAS ...
GitHub user necouchman opened a pull request: https://github.com/apache/guacamole-client/pull/341 GUACAMOLE-670: Add slf4j logging to CAS and RADIUS modules Looks like there are some differing versions of slf4j getting pulled into some of the extension modules by other dependencies, and causing errors to occur when loading the modules. This adds the explicit dependency to the pom.xml files for the modules exhibiting these issues. You can merge this pull request into a Git repository by running: $ git pull https://github.com/necouchman/guacamole-client jira/670 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/guacamole-client/pull/341.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #341 commit 73744d22f955b7b1522a2152ccb2cd01cb86837e Author: Nick Couchman Date: 2018-12-02T20:43:12Z GUACAMOLE-670: Add slf4j logging to modules to avoid pulling in conflicting versions. ---
[GitHub] guacamole-server pull request #205: GUACAMOLE-667: Enable loading configurat...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/205#discussion_r238082585
--- Diff: src/guacd/conf-file.c ---
@@ -188,15 +188,21 @@ guacd_config* guacd_conf_load() {
conf->key_file = NULL;
#endif
+/* Determine path of configuration file */
+if(conf_file_path == NULL) {
+conf_file_path = getenv("GUACD_CONF_FILE");
--- End diff --
Ah, yes, seems I overlooked that part of the original message.
---
[GitHub] guacamole-server pull request #205: GUACAMOLE-667: Enable loading configurat...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/205#discussion_r236387468
--- Diff: src/guacd/conf-args.c ---
@@ -122,3 +128,15 @@ int guacd_conf_parse_args(guacd_config* config, int
argc, char** argv) {
}
+const char* guacd_conf_get_path(int argc, char** argv) {
+
+/* Find -c option */
+int i;
+for(i=1; i
[GitHub] guacamole-server pull request #205: GUACAMOLE-667: Enable loading configurat...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/205#discussion_r236386735
--- Diff: src/guacd/conf-file.c ---
@@ -188,15 +188,21 @@ guacd_config* guacd_conf_load() {
conf->key_file = NULL;
#endif
+/* Determine path of configuration file */
+if(conf_file_path == NULL) {
+conf_file_path = getenv("GUACD_CONF_FILE");
--- End diff --
Why the change from the constant `GUACD_CONF_FILE` to
`getenv("GUACD_CONF_FILE")`? I believe this changes from using the constant to
assuming that it is defined within the environment??
---
[GitHub] guacamole-server pull request #205: GUACAMOLE-667: Enable loading configurat...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/205#discussion_r236385213
--- Diff: src/guacd/conf-file.c ---
@@ -188,15 +188,21 @@ guacd_config* guacd_conf_load() {
conf->key_file = NULL;
#endif
+/* Determine path of configuration file */
+if(conf_file_path == NULL) {
--- End diff --
Two issues, here:
* If is not a function...
`if (conf_file_path == NULL) {`
* Why do this particular check, here? Why not always make sure this
function is fed a path, and then upstream feed either the provided location or
the constant `GUACD_CONF_FILE`?
---
[GitHub] guacamole-server pull request #169: GUACAMOLE-422: Add support for timezone ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/169#discussion_r232741345
--- Diff: src/protocols/ssh/ssh.c ---
@@ -256,6 +256,17 @@ void* ssh_client_thread(void* data) {
return NULL;
}
+/* Set the client timezone */
+if (settings->timezone != NULL) {
+if (libssh2_channel_setenv(ssh_client->term_channel, "TZ",
+settings->timezone)) {
+guac_client_log(client, GUAC_LOG_WARNING,
+"Unable to set the timzeone: SSH server "
--- End diff --
Speling corected.
---
[GitHub] guacamole-server pull request #169: GUACAMOLE-422: Add support for timezone ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/169#discussion_r232737279
--- Diff: src/protocols/rdp/rdp_settings.c ---
@@ -1264,6 +1281,14 @@ void guac_rdp_push_settings(guac_rdp_settings*
guac_settings, freerdp* rdp) {
#endif
#endif
+/* Timezone redirection */
+if (guac_settings->timezone) {
+if(setenv("TZ", guac_settings->timezone, 1)) {
--- End diff --
Wow, and I thought I was past that.
---
[GitHub] guacamole-server pull request #169: GUACAMOLE-422: Add support for timezone ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/169#discussion_r232737390
--- Diff: src/protocols/rdp/rdp_settings.c ---
@@ -1264,6 +1281,14 @@ void guac_rdp_push_settings(guac_rdp_settings*
guac_settings, freerdp* rdp) {
#endif
#endif
+/* Timezone redirection */
+if (guac_settings->timezone) {
+if(setenv("TZ", guac_settings->timezone, 1)) {
+guac_client_log(client, GUAC_LOG_WARNING,
+"Unable to set TZ variable, error %i", errno);
--- End diff --
Used your suggestion.
---
[GitHub] guacamole-server pull request #169: GUACAMOLE-422: Add support for timezone ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/169#discussion_r232507335
--- Diff: src/protocols/rdp/rdp_settings.c ---
@@ -1265,6 +1281,9 @@ void guac_rdp_push_settings(guac_rdp_settings*
guac_settings, freerdp* rdp) {
#endif
/* Device redirection */
--- End diff --
Fixed.
---
[GitHub] guacamole-server pull request #169: GUACAMOLE-422: Add support for timezone ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/169#discussion_r232507326
--- Diff: src/protocols/rdp/rdp_settings.c ---
@@ -1265,6 +1277,9 @@ void guac_rdp_push_settings(guac_rdp_settings*
guac_settings, freerdp* rdp) {
#endif
/* Device redirection */
+if (guac_settings->timezone)
+setenv("TZ", guac_settings->timezone, 1);
--- End diff --
Okay, should be good, now.
---
[GitHub] guacamole-server pull request #201: GUACAMOLE-547: Add support for SSH NONE ...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/201#discussion_r232504541 --- Diff: src/common-ssh/ssh.c --- @@ -321,6 +321,10 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session) guac_client_log(client, GUAC_LOG_DEBUG, "Supported authentication methods: %s", user_authlist); +/* If auth list is NULL, then authentication has succeeded with NONE */ +if (user_authlist == NULL) --- End diff -- Relocated and logged the NONE success. ---
[GitHub] guacamole-server pull request #201: GUACAMOLE-547: Add support for SSH NONE ...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/201#discussion_r232504438 --- Diff: src/common-ssh/ssh.c --- @@ -321,6 +321,10 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session) guac_client_log(client, GUAC_LOG_DEBUG, "Supported authentication methods: %s", user_authlist); +/* If auth list is NULL, then authentication has succeeded with NONE */ +if (user_authlist == NULL) --- End diff -- Good point. Perhaps that NULL check should be located elsewhere... ---
[GitHub] guacamole-server pull request #169: GUACAMOLE-422: Add support for timezone ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/169#discussion_r232504386
--- Diff: src/protocols/rdp/rdp_settings.c ---
@@ -356,6 +357,11 @@ enum RDP_ARGS_IDX {
*/
IDX_PRECONNECTION_BLOB,
+/**
+ * The timezone to pass through to the RDP connection.
--- End diff --
Added reference to Wikipedia page.
---
[GitHub] guacamole-server pull request #169: GUACAMOLE-422: Add support for timezone ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/169#discussion_r232504348
--- Diff: src/protocols/rdp/rdp_settings.c ---
@@ -1265,6 +1277,9 @@ void guac_rdp_push_settings(guac_rdp_settings*
guac_settings, freerdp* rdp) {
#endif
/* Device redirection */
+if (guac_settings->timezone)
+setenv("TZ", guac_settings->timezone, 1);
--- End diff --
It appears that, in order to do this, I would need to pass the entire
`client` object through to this `guac_rdp_push_settings()` function?
---
[GitHub] guacamole-server pull request #169: GUACAMOLE-422: Add support for timezone ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/169#discussion_r232504354
--- Diff: src/protocols/ssh/ssh.c ---
@@ -256,6 +256,10 @@ void* ssh_client_thread(void* data) {
return NULL;
}
+/* Set the client timezone */
+if (settings->timezone != NULL)
+libssh2_channel_setenv(ssh_client->term_channel, "TZ",
settings->timezone);
--- End diff --
Done.
---
[GitHub] guacamole-server pull request #169: GUACAMOLE-422: Add support for timezone ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/169#discussion_r232504364
--- Diff: src/protocols/ssh/settings.h ---
@@ -253,6 +253,10 @@ typedef struct guac_ssh_settings {
* environment variable.
*/
char* locale;
+/**
--- End diff --
Added.
---
[GitHub] guacamole-server pull request #201: GUACAMOLE-547: Add support for SSH NONE ...
GitHub user necouchman opened a pull request: https://github.com/apache/guacamole-server/pull/201 GUACAMOLE-547: Add support for SSH NONE authentication method I'm not 100% certain that this is all that's needed, because I don't have a SSH server to test this against, but, according to the libssh2 documentation, this may be all that's required to allow the session to continue when there's no authentication required. https://libssh2.org/libssh2_userauth_list.html You can merge this pull request into a Git repository by running: $ git pull https://github.com/necouchman/guacamole-server jira/547 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/guacamole-server/pull/201.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #201 commit 469defd983df7a64a2825a961f004ae8122cdf56 Author: Nick Couchman Date: 2018-11-11T20:16:22Z GUACAMOLE-547: Add support for SSH NONE authentication method. ---
[GitHub] guacamole-server pull request #200: GUACAMOLE-630: Allow font parameters of ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/200#discussion_r232485885
--- Diff: src/protocols/ssh/argv.c ---
@@ -81,16 +110,51 @@ static int guac_ssh_argv_blob_handler(guac_user* user,
static int guac_ssh_argv_end_handler(guac_user* user,
guac_stream* stream) {
+int size;
+
guac_client* client = user->client;
-guac_ssh_client* telnet_client = (guac_ssh_client*) client->data;
-guac_terminal* terminal = telnet_client->term;
--- End diff --
Wow, not sure how I missed that last time around!
---
[GitHub] guacamole-server pull request #198: GUACAMOLE-630: Allow color scheme of act...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/198#discussion_r232439189
--- Diff: src/protocols/ssh/argv.c ---
@@ -0,0 +1,128 @@
+/*
+ * 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.
+ */
+
+#include "config.h"
+#include "argv.h"
+#include "ssh.h"
+#include "terminal/terminal.h"
+
+#include
+#include
+#include
+
+#include
+#include
+
+/**
+ * The value or current status of a connection parameter received over an
+ * "argv" stream.
+ */
+typedef struct guac_ssh_argv {
+
+/**
+ * Buffer space for containing the received argument value.
+ */
+char buffer[16384];
--- End diff --
Should this number be a constant somewhere instead of being called like
this, especially given other efforts to define those magic numbers?
---
[GitHub] guacamole-server pull request #198: GUACAMOLE-630: Allow color scheme of act...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/198#discussion_r232438968
--- Diff: src/protocols/ssh/argv.c ---
@@ -0,0 +1,128 @@
+/*
+ * 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.
+ */
+
+#include "config.h"
+#include "argv.h"
+#include "ssh.h"
+#include "terminal/terminal.h"
+
+#include
+#include
+#include
+
+#include
+#include
+
+/**
+ * The value or current status of a connection parameter received over an
+ * "argv" stream.
+ */
+typedef struct guac_ssh_argv {
+
+/**
+ * Buffer space for containing the received argument value.
+ */
+char buffer[16384];
+
+/**
+ * The number of bytes received so far.
+ */
+int length;
+
+} guac_ssh_argv;
+
+/**
+ * Handler for "blob" instructions which appends the data from received
blobs
+ * to the end of the in-progress argument value buffer.
+ *
+ * @see guac_user_blob_handler
+ */
+static int guac_ssh_argv_blob_handler(guac_user* user,
+guac_stream* stream, void* data, int length) {
+
+guac_ssh_argv* argv = (guac_ssh_argv*) stream->data;
+
+/* Calculate buffer size remaining, including space for null
terminator,
+ * adjusting received length accordingly */
+int remaining = sizeof(argv->buffer) - argv->length - 1;
+if (length > remaining)
+length = remaining;
+
+/* Append received data to end of buffer */
+memcpy(argv->buffer + argv->length, data, length);
+argv->length += length;
+
+return 0;
+
+}
+
+/**
+ * Handler for "end" instructions which applies the changes specified by
the
+ * argument value buffer associated with the stream.
+ *
+ * @see guac_user_end_handler
+ */
+static int guac_ssh_argv_end_handler(guac_user* user,
+guac_stream* stream) {
+
+guac_client* client = user->client;
+guac_ssh_client* telnet_client = (guac_ssh_client*) client->data;
+guac_terminal* terminal = telnet_client->term;
+
+/* Append null terminator to value */
+guac_ssh_argv* argv = (guac_ssh_argv*) stream->data;
+argv->buffer[argv->length] = '\0';
--- End diff --
Is there any scenario under which this could overrun? In looking through
the other code I couldn't be certain there was a place this was checked?
---
[GitHub] guacamole-server pull request #199: GUACAMOLE-649: Add support for setting L...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/199#discussion_r231131516
--- Diff: src/protocols/ssh/ssh.c ---
@@ -320,6 +320,17 @@ void* ssh_client_thread(void* data) {
return NULL;
}
+/* Forward specified locale */
+if (settings->locale != NULL) {
+if (libssh2_channel_setenv(ssh_client->term_channel, "LANG",
+settings->locale)) {
+guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR,
--- End diff --
Should this really be a fatal error in terms of the overall connection? I
ask because in general this is not fatal in terms of other SSH clients - for
example, if you use the "ssh" command on pretty much any platform and ask to
forward a variable that the server does not allow, it is simply silently
ignored and does not abort the connection.
---
[GitHub] guacamole-server pull request #196: GUACAMOLE-527: Correct order of SFTP par...
GitHub user necouchman opened a pull request: https://github.com/apache/guacamole-server/pull/196 GUACAMOLE-527: Correct order of SFTP parameters in VNC. Corrects the order of parameters in the VNC settings that I believe was causing issues with VNC known host checking (and probably other items). You can merge this pull request into a Git repository by running: $ git pull https://github.com/necouchman/guacamole-server jira/527 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/guacamole-server/pull/196.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #196 commit 34c02b2678a353a3ea101227fce9db42babf Author: Nick Couchman Date: 2018-10-16T13:27:45Z GUACAMOLE-527: Correct issue with order of VNC SFTP settings. ---
[GitHub] guacamole-client pull request #328: GUACAMOLE-232: Address regressions in ha...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/328#discussion_r222318256
--- Diff: guacamole-common-js/src/main/webapp/modules/Keyboard.js ---
@@ -953,37 +1006,58 @@ Guacamole.Keyboard = function Keyboard(element) {
0xFFE9, // Left alt
0xFFEA, // Right alt
0xFE03 // AltGr
-]);
+], keyEvent);
// Resync state of shift
updateModifierState(guac_keyboard.modifiers.shift, state.shift, [
0xFFE1, // Left shift
0xFFE2 // Right shift
-]);
+], keyEvent);
// Resync state of ctrl
updateModifierState(guac_keyboard.modifiers.ctrl, state.ctrl, [
0xFFE3, // Left ctrl
0xFFE4 // Right ctrl
-]);
+], keyEvent);
// Resync state of meta
updateModifierState(guac_keyboard.modifiers.meta, state.meta, [
0xFFE7, // Left meta
0xFFE8 // Right meta
-]);
+], keyEvent);
// Resync state of hyper
updateModifierState(guac_keyboard.modifiers.hyper, state.hyper, [
0xFFEB, // Left hyper
0xFFEC // Right hyper
-]);
+], keyEvent);
// Update state
guac_keyboard.modifiers = state;
};
+/**
+ * Returns whether all currently pressed keys were implicitly pressed.
A
+ * key is implicitly pressed if its status was inferred indirectly from
+ * inspection of other key events.
+ *
+ * @private
+ * @returns {Boolean}
+ * true of all currently pressed keys were implicitly pressed,
false
--- End diff --
of -> if
---
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/319#discussion_r221599026
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/permission/ModeledObjectPermissionService.java
---
@@ -47,14 +49,14 @@ protected ObjectPermission
getPermissionInstance(ObjectPermissionModel model) {
}
@Override
-protected ObjectPermissionModel getModelInstance(ModeledUser
targetUser,
+protected ObjectPermissionModel getModelInstance(
+ModeledPermissions targetEntity,
ObjectPermission permission) {
ObjectPermissionModel model = new ObjectPermissionModel();
--- End diff --
I don't know that it needs to hold up this particular PR, and I don't know
how valuable it will be. Seems like I ran across several instances in various
places that had this sort of empty constructor plus several `set` methods, so
probably worth a separate cleanup effort.
---
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/319#discussion_r221436490
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/RelatedObjectSet.java
---
@@ -0,0 +1,211 @@
+/*
+ * 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.auth.jdbc.base;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import org.apache.guacamole.auth.jdbc.user.ModeledAuthenticatedUser;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleSecurityException;
+import org.apache.guacamole.net.auth.permission.ObjectPermission;
+import org.apache.guacamole.net.auth.permission.ObjectPermissionSet;
+
+/**
+ * A database implementation of RelatedObjectSet which provides access to a
+ * parent object and corresponding set of objects related to the parent,
subject
+ * to object-level permissions. Though the parent and child objects have
+ * specific types, only the parent object's type is enforced through type
+ * parameters, as child objects are represented by identifiers only.
+ *
+ * @param
+ * The type of object that represents the parent side of the relation.
+ *
+ * @param
+ * The underlying database model of the parent object.
+ */
+public abstract class RelatedObjectSet, ParentModelType extends ObjectModel>
+extends RestrictedObject implements
org.apache.guacamole.net.auth.RelatedObjectSet {
+
+/**
+ * The parent object which shares some arbitrary relation with the
objects
+ * within this set.
+ */
+private ParentObjectType parent;
+
+/**
+ * Creates a new RelatedObjectSet. The resulting object set must still
be
+ * initialized by a call to init().
+ */
+public RelatedObjectSet() {
+}
+
+/**
+ * Initializes this RelatedObjectSet with the current user and the
single
+ * object on the parent side of the one-to-many relation represented
by the
+ * set.
+ *
+ * @param currentUser
+ * The user who queried this RelatedObjectSet, and whose
permissions
+ * dictate the access level of all operations performed on this
set.
+ *
+ * @param parent
+ * The parent object which shares some arbitrary relation with the
+ * objects within this set.
+ */
+public void init(ModeledAuthenticatedUser currentUser,
ParentObjectType parent) {
+super.init(currentUser);
+this.parent = parent;
+}
+
+/**
+ * Returns the mapper which provides low-level access to the the
database
+ * models which drive the relation represented by this
RelatedObjectSet.
+ *
+ * @return
+ * The mapper which provides low-level access to the the database
+ * models which drive the relation represented by this
+ * RelatedObjectSet.
+ */
+protected abstract ObjectRelationMapper
getObjectRelationMapper();
+
+/**
+ * Returns the permission set which exposes the effective permissions
+ * available to the current user regarding the objects on the parent
side
+ * of the one-to-many relationship represented by this
RelatedObjectSet.
+ * Permission inheritance through user groups is taken into account.
+ *
+ * @return
+ * The permission set which exposes the effective permissions
+ * available to the current user regarding the objects on the
parent
+ * side of the one-to-many relationship represented by this
+ * RelatedObjectSet.
+ *
+ * @throws GuacamoleException
+ * If permission to query permission status is denied.
+ */
+protected abstr
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/319#discussion_r221438180
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/permission/ModeledObjectPermissionService.java
---
@@ -47,14 +49,14 @@ protected ObjectPermission
getPermissionInstance(ObjectPermissionModel model) {
}
@Override
-protected ObjectPermissionModel getModelInstance(ModeledUser
targetUser,
+protected ObjectPermissionModel getModelInstance(
+ModeledPermissions targetEntity,
ObjectPermission permission) {
ObjectPermissionModel model = new ObjectPermissionModel();
--- End diff --
Out of curiosity, is there a particular rationale or driver here for having
the empty constructor, and then calling each of the `set...()` methods below?
---
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/319#discussion_r221280997 --- Diff: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/pom.xml --- @@ -109,33 +109,33 @@ org.mybatis mybatis -3.2.8 +3.4.6 --- End diff -- Sounds good to me - it does seem like a good time to sneak this in. By "everything" I assume you mean everything except the base guacamole-common stuff? ---
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/319#discussion_r221253121
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/UserGroupService.java
---
@@ -0,0 +1,189 @@
+/*
+ * 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.auth.jdbc.usergroup;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.util.Collection;
+import java.util.Collections;
+import org.apache.guacamole.auth.jdbc.base.ModeledDirectoryObjectMapper;
+import org.apache.guacamole.auth.jdbc.base.ModeledDirectoryObjectService;
+import org.apache.guacamole.GuacamoleClientException;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.base.EntityMapper;
+import org.apache.guacamole.auth.jdbc.permission.ObjectPermissionMapper;
+import org.apache.guacamole.auth.jdbc.permission.UserGroupPermissionMapper;
+import org.apache.guacamole.auth.jdbc.user.ModeledAuthenticatedUser;
+import org.apache.guacamole.net.auth.UserGroup;
+import org.apache.guacamole.net.auth.permission.ObjectPermission;
+import org.apache.guacamole.net.auth.permission.ObjectPermissionSet;
+import org.apache.guacamole.net.auth.permission.SystemPermission;
+import org.apache.guacamole.net.auth.permission.SystemPermissionSet;
+
+/**
+ * Service which provides convenience methods for creating, retrieving, and
+ * manipulating user groups.
+ */
+public class UserGroupService extends
ModeledDirectoryObjectService {
+
+/**
+ * Mapper for creating/deleting entities.
+ */
+@Inject
+private EntityMapper entityMapper;
+
+/**
+ * Mapper for accessing user groups.
+ */
+@Inject
+private UserGroupMapper userGroupMapper;
+
+/**
+ * Mapper for manipulating user group permissions.
+ */
+@Inject
+private UserGroupPermissionMapper userGroupPermissionMapper;
+
+/**
+ * Provider for creating user groups.
+ */
+@Inject
+private Provider userGroupProvider;
+
+@Override
+protected ModeledDirectoryObjectMapper
getObjectMapper() {
+return userGroupMapper;
+}
+
+@Override
+protected ObjectPermissionMapper getPermissionMapper() {
+return userGroupPermissionMapper;
+}
+
+@Override
+protected ModeledUserGroup getObjectInstance(ModeledAuthenticatedUser
currentUser,
+UserGroupModel model) throws GuacamoleException {
+
+boolean exposeRestrictedAttributes;
+
+// Expose restricted attributes if the user group does not yet
exist
+if (model.getObjectID() == null)
+exposeRestrictedAttributes = true;
+
+// Otherwise, expose restricted attributes only if the user has
+// ADMINISTER permission
+else
+exposeRestrictedAttributes = hasObjectPermission(currentUser,
+model.getIdentifier(),
ObjectPermission.Type.ADMINISTER);
+
+// Produce ModeledUserGroup exposing only those attributes for
which the
+// current user has permission
+ModeledUserGroup group = userGroupProvider.get();
+group.init(currentUser, model, exposeRestrictedAttributes);
+return group;
+
+}
+
+@Override
+protected UserGroupModel getModelInstance(ModeledAuthenticatedUser
currentUser,
+final UserGroup object) throws GuacamoleException {
+
+// Create new ModeledUserGroup backed by blank model
+UserGroupModel model = new UserGroupModel();
+ModeledUserGroup group = getObjectInstance(currentUser, model);
+
+// Set model contents through ModeledUser, copying
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/319#discussion_r220513791 --- Diff: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/pom.xml --- @@ -109,33 +109,33 @@ org.mybatis mybatis -3.2.8 +3.4.6 --- End diff -- Looking at the POM file it looks like there are certain features of this version of MyBatis that aren't usable with older versions of Java. There's a block toward the bottom that excludes certain code file from the compile when the version is 1.6 and a block that excludes for 1.7. It's possible I was using a feature that isn't available in 1.6, even if the overall project is compatible with it. It also looks like the test suite requires at least 1.7, so maybe that is what I was seeing before. I definitely am more inclined toward moving on to 1.7 or 1.8 given the age of 1.6 (and even 1.7), but I also understand maintaining compatibility as much as possible. However, it does seem like several aspects of the web app are using some features that require 1.7 or 1.8, and there are also some Java syntax features that we would probably benefit from by moving on to the higher versions. Two that come to mind are "try with resources" and the ability to handle multiple exception types in a single `catch` block. ---
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/319#discussion_r220506258
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/apache/guacamole/auth/jdbc/connection/ConnectionRecordMapper.xml
---
@@ -79,7 +79,10 @@
#{record.sharingProfileIdentifier,jdbcType=VARCHAR},
#{record.sharingProfileName,jdbcType=VARCHAR},
(SELECT user_id FROM guacamole_user
- WHERE username = #{record.username,jdbcType=VARCHAR}),
+ JOIN guacamole_entity ON guacamole_user.entity_id =
guacamole_entity.entity_id
--- End diff --
Oops - I think I was confusing that block of SQL code for the `INSERT`
statement with a query that would `SELECT` rows from the table. My bad.
---
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/319#discussion_r220396450
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/schema/upgrade/upgrade-pre-1.0.0.sql
---
@@ -17,6 +17,319 @@
-- under the License.
--
+--
+-- Add new system-level permission
+--
+
+ALTER TABLE `guacamole_system_permission`
+MODIFY `permission` enum('CREATE_CONNECTION',
+ 'CREATE_CONNECTION_GROUP',
+ 'CREATE_SHARING_PROFILE',
+ 'CREATE_USER',
+ 'CREATE_USER_GROUP',
+ 'ADMINISTER') NOT NULL;
+
+--
+-- Table of base entities which may each be either a user or user group.
Other
+-- tables which represent qualities shared by both users and groups will
point
+-- to guacamole_entity, while tables which represent qualities specific to
+-- users or groups will point to guacamole_user or guacamole_user_group.
+--
+
+CREATE TABLE `guacamole_entity` (
+
+ `entity_id` int(11)NOT NULL AUTO_INCREMENT,
+ `name` varchar(128) NOT NULL,
+ `type` enum('USER',
+ 'USER_GROUP') NOT NULL,
+
+ PRIMARY KEY (`entity_id`),
+ UNIQUE KEY `guacamole_entity_name_scope` (`type`, `name`)
+
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+--
+-- Table of user groups. Each user group may have an arbitrary set of
member
+-- users and member groups, with those members inheriting the permissions
+-- granted to that group.
+--
+
+CREATE TABLE `guacamole_user_group` (
+
+ `user_group_id` int(11) NOT NULL AUTO_INCREMENT,
+ `entity_id` int(11) NOT NULL,
+
+ -- Group disabled status
+ `disabled` boolean NOT NULL DEFAULT 0,
+
+ PRIMARY KEY (`user_group_id`),
+
+ UNIQUE KEY `guacamole_user_group_single_entity` (`entity_id`),
+
+ CONSTRAINT `guacamole_user_group_entity`
+FOREIGN KEY (`entity_id`)
+REFERENCES `guacamole_entity` (`entity_id`)
+ON DELETE CASCADE
+
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+--
+-- Table of users which are members of given user groups.
+--
+
+CREATE TABLE `guacamole_user_group_member` (
+
+ `user_group_id`int(11) NOT NULL,
+ `member_entity_id` int(11) NOT NULL,
+
+ PRIMARY KEY (`user_group_id`, `member_entity_id`),
+
+ -- Parent must be a user group
+ CONSTRAINT `guacamole_user_group_member_parent_id`
+FOREIGN KEY (`user_group_id`)
+REFERENCES `guacamole_user_group` (`user_group_id`) ON DELETE CASCADE,
+
+ -- Member may be either a user or a user group (any entity)
+ CONSTRAINT `guacamole_user_group_member_entity_id`
+FOREIGN KEY (`member_entity_id`)
+REFERENCES `guacamole_entity` (`entity_id`) ON DELETE CASCADE
+
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+--
+-- Table of user group permissions. Each user group permission grants a
user
+-- or user group access to a another user group (the "affected" user
group) for
+-- a specific type of operation.
+--
+
+CREATE TABLE `guacamole_user_group_permission` (
+
+ `entity_id` int(11) NOT NULL,
+ `affected_user_group_id` int(11) NOT NULL,
+ `permission` enum('READ',
+'UPDATE',
+'DELETE',
+'ADMINISTER') NOT NULL,
+
+ PRIMARY KEY (`entity_id`, `affected_user_group_id`, `permission`),
+
+ CONSTRAINT `guacamole_user_group_permission_affected_user_group`
+FOREIGN KEY (`affected_user_group_id`)
+REFERENCES `guacamole_user_group` (`user_group_id`) ON DELETE CASCADE,
+
+ CONSTRAINT `guacamole_user_group_permission_entity`
+FOREIGN KEY (`entity_id`)
+REFERENCES `guacamole_entity` (`entity_id`) ON DELETE CASCADE
+
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+--
+-- Modify guacamole_user table to use guacamole_entity as a base
+--
+
+-- Add new entity_id column
+ALTER TABLE guacamole_user ADD COLUMN entity_id int(11);
+
+-- Create user entities for each guacamole_user entry
+INSERT INTO guacamole_entity (name, type)
+SELECT username, 'USER' FROM guacamole_user;
+
+-- Update guacamole_user to point to corresponding guacamole_entity
+UPDATE guacamole_user SET entity_id = (
+SELECT entity_id FROM guacamole_entity
+WHERE
+username = guacamole_entity.name
+AND type = 'USER'
+);
+
+-- The entity_id column should now be safely non-NULL
+A
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/319#discussion_r220395193
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/UserGroupService.java
---
@@ -0,0 +1,189 @@
+/*
+ * 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.auth.jdbc.usergroup;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.util.Collection;
+import java.util.Collections;
+import org.apache.guacamole.auth.jdbc.base.ModeledDirectoryObjectMapper;
+import org.apache.guacamole.auth.jdbc.base.ModeledDirectoryObjectService;
+import org.apache.guacamole.GuacamoleClientException;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.base.EntityMapper;
+import org.apache.guacamole.auth.jdbc.permission.ObjectPermissionMapper;
+import org.apache.guacamole.auth.jdbc.permission.UserGroupPermissionMapper;
+import org.apache.guacamole.auth.jdbc.user.ModeledAuthenticatedUser;
+import org.apache.guacamole.net.auth.UserGroup;
+import org.apache.guacamole.net.auth.permission.ObjectPermission;
+import org.apache.guacamole.net.auth.permission.ObjectPermissionSet;
+import org.apache.guacamole.net.auth.permission.SystemPermission;
+import org.apache.guacamole.net.auth.permission.SystemPermissionSet;
+
+/**
+ * Service which provides convenience methods for creating, retrieving, and
+ * manipulating user groups.
+ */
+public class UserGroupService extends
ModeledDirectoryObjectService {
+
+/**
+ * Mapper for creating/deleting entities.
+ */
+@Inject
+private EntityMapper entityMapper;
+
+/**
+ * Mapper for accessing user groups.
+ */
+@Inject
+private UserGroupMapper userGroupMapper;
+
+/**
+ * Mapper for manipulating user group permissions.
+ */
+@Inject
+private UserGroupPermissionMapper userGroupPermissionMapper;
+
+/**
+ * Provider for creating user groups.
+ */
+@Inject
+private Provider userGroupProvider;
+
+@Override
+protected ModeledDirectoryObjectMapper
getObjectMapper() {
+return userGroupMapper;
+}
+
+@Override
+protected ObjectPermissionMapper getPermissionMapper() {
+return userGroupPermissionMapper;
+}
+
+@Override
+protected ModeledUserGroup getObjectInstance(ModeledAuthenticatedUser
currentUser,
+UserGroupModel model) throws GuacamoleException {
+
+boolean exposeRestrictedAttributes;
+
+// Expose restricted attributes if the user group does not yet
exist
+if (model.getObjectID() == null)
+exposeRestrictedAttributes = true;
+
+// Otherwise, expose restricted attributes only if the user has
+// ADMINISTER permission
+else
+exposeRestrictedAttributes = hasObjectPermission(currentUser,
+model.getIdentifier(),
ObjectPermission.Type.ADMINISTER);
+
+// Produce ModeledUserGroup exposing only those attributes for
which the
+// current user has permission
+ModeledUserGroup group = userGroupProvider.get();
+group.init(currentUser, model, exposeRestrictedAttributes);
+return group;
+
+}
+
+@Override
+protected UserGroupModel getModelInstance(ModeledAuthenticatedUser
currentUser,
+final UserGroup object) throws GuacamoleException {
+
+// Create new ModeledUserGroup backed by blank model
+UserGroupModel model = new UserGroupModel();
+ModeledUserGroup group = getObjectInstance(currentUser, model);
+
+// Set model contents through ModeledUser, copying
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/319#discussion_r220397742
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/apache/guacamole/auth/jdbc/connection/ConnectionRecordMapper.xml
---
@@ -79,7 +79,10 @@
#{record.sharingProfileIdentifier,jdbcType=VARCHAR},
#{record.sharingProfileName,jdbcType=VARCHAR},
(SELECT user_id FROM guacamole_user
- WHERE username = #{record.username,jdbcType=VARCHAR}),
+ JOIN guacamole_entity ON guacamole_user.entity_id =
guacamole_entity.entity_id
--- End diff --
Seems like this code here expects that, when users are deleted from the
database, their entries will remain present in the `guacamole_entity` table?
Just want to confirm...
---
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/319#discussion_r220395046
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/UserGroupService.java
---
@@ -0,0 +1,189 @@
+/*
+ * 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.auth.jdbc.usergroup;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.util.Collection;
+import java.util.Collections;
+import org.apache.guacamole.auth.jdbc.base.ModeledDirectoryObjectMapper;
+import org.apache.guacamole.auth.jdbc.base.ModeledDirectoryObjectService;
+import org.apache.guacamole.GuacamoleClientException;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.base.EntityMapper;
+import org.apache.guacamole.auth.jdbc.permission.ObjectPermissionMapper;
+import org.apache.guacamole.auth.jdbc.permission.UserGroupPermissionMapper;
+import org.apache.guacamole.auth.jdbc.user.ModeledAuthenticatedUser;
+import org.apache.guacamole.net.auth.UserGroup;
+import org.apache.guacamole.net.auth.permission.ObjectPermission;
+import org.apache.guacamole.net.auth.permission.ObjectPermissionSet;
+import org.apache.guacamole.net.auth.permission.SystemPermission;
+import org.apache.guacamole.net.auth.permission.SystemPermissionSet;
+
+/**
+ * Service which provides convenience methods for creating, retrieving, and
+ * manipulating user groups.
+ */
+public class UserGroupService extends
ModeledDirectoryObjectService {
+
+/**
+ * Mapper for creating/deleting entities.
+ */
+@Inject
+private EntityMapper entityMapper;
+
+/**
+ * Mapper for accessing user groups.
+ */
+@Inject
+private UserGroupMapper userGroupMapper;
+
+/**
+ * Mapper for manipulating user group permissions.
+ */
+@Inject
+private UserGroupPermissionMapper userGroupPermissionMapper;
+
+/**
+ * Provider for creating user groups.
+ */
+@Inject
+private Provider userGroupProvider;
+
+@Override
+protected ModeledDirectoryObjectMapper
getObjectMapper() {
+return userGroupMapper;
+}
+
+@Override
+protected ObjectPermissionMapper getPermissionMapper() {
+return userGroupPermissionMapper;
+}
+
+@Override
+protected ModeledUserGroup getObjectInstance(ModeledAuthenticatedUser
currentUser,
+UserGroupModel model) throws GuacamoleException {
+
+boolean exposeRestrictedAttributes;
+
+// Expose restricted attributes if the user group does not yet
exist
+if (model.getObjectID() == null)
+exposeRestrictedAttributes = true;
+
+// Otherwise, expose restricted attributes only if the user has
+// ADMINISTER permission
+else
+exposeRestrictedAttributes = hasObjectPermission(currentUser,
+model.getIdentifier(),
ObjectPermission.Type.ADMINISTER);
+
+// Produce ModeledUserGroup exposing only those attributes for
which the
+// current user has permission
+ModeledUserGroup group = userGroupProvider.get();
+group.init(currentUser, model, exposeRestrictedAttributes);
+return group;
+
+}
+
+@Override
+protected UserGroupModel getModelInstance(ModeledAuthenticatedUser
currentUser,
+final UserGroup object) throws GuacamoleException {
+
+// Create new ModeledUserGroup backed by blank model
+UserGroupModel model = new UserGroupModel();
+ModeledUserGroup group = getObjectInstance(currentUser, model);
+
+// Set model contents through ModeledUser, copying
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/319#discussion_r220390959
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/permission/SystemPermissionService.java
---
@@ -124,39 +129,40 @@ public void
deletePermissions(ModeledAuthenticatedUser user, ModeledUser targetU
}
/**
- * Retrieves the permission of the given type associated with the given
- * user, if it exists. If no such permission exists, null is returned.
+ * Retrieves whether the permission of the given type has been granted
to
+ * the given entity. Permission inheritance through group membership is
+ * taken into account.
*
* @param user
* The user retrieving the permission.
*
- * @param targetUser
- * The user associated with the permission to be retrieved.
+ * @param targetEntity
+ * The entity associated with the permission to be retrieved.
*
* @param type
* The type of permission to retrieve.
*
+ * @param effectiveGroups
+ * The identifiers of all groups that should be taken into account
+ * when determining the permissions effectively granted to the
user. If
+ * no groups are given, only permissions directly granted to the
user
+ * will be used.
+ *
* @return
- * The permission of the given type associated with the given
user, or
- * null if no such permission exists.
+ * true if permission of the given type has been granted to the
given
+ * user, false otherwise.
*
* @throws GuacamoleException
* If an error occurs while retrieving the requested permission.
*/
-public SystemPermission retrievePermission(ModeledAuthenticatedUser
user,
-ModeledUser targetUser, SystemPermission.Type type) throws
GuacamoleException {
+public boolean hasPermission(ModeledAuthenticatedUser user,
+ModeledPermissions targetEntity,
+SystemPermission.Type type, Set effectiveGroups)
+throws GuacamoleException {
// Retrieve permissions only if allowed
-if (canReadPermissions(user, targetUser)) {
-
-// Read permission from database, return null if not found
-SystemPermissionModel model =
getPermissionMapper().selectOne(targetUser.getModel(), type);
-if (model == null)
-return null;
-
-return getPermissionInstance(model);
-
-}
+if (canReadPermissions(user, targetEntity))
+return
getPermissionMapper().selectOne(targetEntity.getModel(), type, effectiveGroups)
!= null;
// User cannot read this user's permissions
--- End diff --
user's -> entity's
---
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/319#discussion_r220394415
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/UserGroupService.java
---
@@ -0,0 +1,189 @@
+/*
+ * 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.auth.jdbc.usergroup;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.util.Collection;
+import java.util.Collections;
+import org.apache.guacamole.auth.jdbc.base.ModeledDirectoryObjectMapper;
+import org.apache.guacamole.auth.jdbc.base.ModeledDirectoryObjectService;
+import org.apache.guacamole.GuacamoleClientException;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.base.EntityMapper;
+import org.apache.guacamole.auth.jdbc.permission.ObjectPermissionMapper;
+import org.apache.guacamole.auth.jdbc.permission.UserGroupPermissionMapper;
+import org.apache.guacamole.auth.jdbc.user.ModeledAuthenticatedUser;
+import org.apache.guacamole.net.auth.UserGroup;
+import org.apache.guacamole.net.auth.permission.ObjectPermission;
+import org.apache.guacamole.net.auth.permission.ObjectPermissionSet;
+import org.apache.guacamole.net.auth.permission.SystemPermission;
+import org.apache.guacamole.net.auth.permission.SystemPermissionSet;
+
+/**
+ * Service which provides convenience methods for creating, retrieving, and
+ * manipulating user groups.
+ */
+public class UserGroupService extends
ModeledDirectoryObjectService {
+
+/**
+ * Mapper for creating/deleting entities.
+ */
+@Inject
+private EntityMapper entityMapper;
+
+/**
+ * Mapper for accessing user groups.
+ */
+@Inject
+private UserGroupMapper userGroupMapper;
+
+/**
+ * Mapper for manipulating user group permissions.
+ */
+@Inject
+private UserGroupPermissionMapper userGroupPermissionMapper;
+
+/**
+ * Provider for creating user groups.
+ */
+@Inject
+private Provider userGroupProvider;
+
+@Override
+protected ModeledDirectoryObjectMapper
getObjectMapper() {
+return userGroupMapper;
+}
+
+@Override
+protected ObjectPermissionMapper getPermissionMapper() {
+return userGroupPermissionMapper;
+}
+
+@Override
+protected ModeledUserGroup getObjectInstance(ModeledAuthenticatedUser
currentUser,
+UserGroupModel model) throws GuacamoleException {
+
+boolean exposeRestrictedAttributes;
+
+// Expose restricted attributes if the user group does not yet
exist
+if (model.getObjectID() == null)
+exposeRestrictedAttributes = true;
+
+// Otherwise, expose restricted attributes only if the user has
+// ADMINISTER permission
+else
+exposeRestrictedAttributes = hasObjectPermission(currentUser,
+model.getIdentifier(),
ObjectPermission.Type.ADMINISTER);
+
+// Produce ModeledUserGroup exposing only those attributes for
which the
+// current user has permission
+ModeledUserGroup group = userGroupProvider.get();
+group.init(currentUser, model, exposeRestrictedAttributes);
+return group;
+
+}
+
+@Override
+protected UserGroupModel getModelInstance(ModeledAuthenticatedUser
currentUser,
+final UserGroup object) throws GuacamoleException {
+
+// Create new ModeledUserGroup backed by blank model
+UserGroupModel model = new UserGroupModel();
+ModeledUserGroup group = getObjectInstance(currentUser, model);
+
+// Set model contents through ModeledUser, copying
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/319#discussion_r220390391
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/permission/SystemPermissionService.java
---
@@ -76,23 +78,25 @@ protected SystemPermissionModel getModelInstance(final
ModeledUser targetUser,
@Override
public SystemPermissionSet getPermissionSet(ModeledAuthenticatedUser
user,
-ModeledUser targetUser) throws GuacamoleException {
+ModeledPermissions targetEntity,
+Set effectiveGroups) throws GuacamoleException {
// Create permission set for requested user
--- End diff --
user -> entity
?
---
[GitHub] guacamole-client pull request #319: GUACAMOLE-220: Add database support for ...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/319#discussion_r219843129
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/ObjectRelationMapper.java
---
@@ -0,0 +1,126 @@
+/*
+ * 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.auth.jdbc.base;
+
+import java.util.Collection;
+import java.util.Set;
+import org.apache.guacamole.auth.jdbc.user.UserModel;
+import org.apache.ibatis.annotations.Param;
+
+/**
+ * Mapper for the relations represented by a particular RelatedObjectSet
+ * implementation.
+ *
+ * @param
+ * The underlying database model of the object on the parent side of
the
+ * one-to-many relationship represented by the RelatedObjectSet mapped
by
+ * this ObjectRelationMapper.
+ */
+public interface ObjectRelationMapper
{
+
+/**
+ * Inserts rows as necessary to establish the one-to-many relationship
+ * represented by the RelatedObjectSet between the given parent and
+ * children. If the relation for any parent/child pair is already
present,
+ * no attempt is made to insert a new row for that relation.
+ *
+ * @param parent
+ * The model of the object on the parent side of the one-to-many
+ * relationship represented by the RelatedObjectSet.
+ *
+ * @param children
+ * The identifiers of the objects on the child side of the
one-to-many
+ * relationship represented by the RelatedObjectSet.
+ *
+ * @return
+ * The number of rows inserted.
+ */
+int insert(@Param("parent") ParentModelType parent,
+@Param("children") Collection children);
+
+/**
+ * Deletes rows as necessary to establish the one-to-many relationship
--- End diff --
Is "to establish" the correct language, here? Or is this to modify/remove
one-to-many relationships?
---
[GitHub] guacamole-client pull request #320: Guacamole-626 - Add support for Docker s...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/320#discussion_r219650390 --- Diff: guacamole-docker/README.md --- @@ -28,14 +28,25 @@ Once the Guacamole image is running, Guacamole will be accessible at `-p 8080:8080` option to expose this port at the level of the machine hosting Docker, as well. +Docker Secrets +== +The string `_FILE` may be appended to some of the environment variables listed below if you are using MySQL or PostgreSQL authentication. This will cause the startup script to load the values for those variables from files within in the container. This is useful for specifying sensitive info, ie. passwords for the database, in secured files instead of plaintext environment variables, and is generally used for loading values from [Docker secrets](https://docs.docker.com/engine/swarm/secrets/#read-more-about-docker-secret-commands), which are stored in `/run/secrets/` within the container. + --- End diff -- A couple of issues, here: - In the rest of the README.md file, here, lines are formatted with a roughly similar maximum column size. It looks like you've got a single very long line, here - please reformat these lines (and the others you've added below) to match the style used throughout the rest of the file. - There is a mistake in the line "for those variables from files within in the" should be "for those variables from files within the". ---
[GitHub] guacamole-client pull request #320: Guacamole-626 - Add support for Docker s...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/320#discussion_r219652878 --- Diff: guacamole-docker/README.md --- @@ -90,6 +107,9 @@ the image will stop: 1. `MYSQL_DATABASE` - The name of the database to use for Guacamole authentication. 2. `MYSQL_USER` - The user that Guacamole will use to connect to MySQL. 3. `MYSQL_PASSWORD` - The password that Guacamole will provide when connecting to MySQL as `MYSQL_USER`. +4. `MYSQL_DATABASE_FILE` - The path of the docker secret containing the name of database to use for Guacamole authentication. +5. `MYSQL_USER` - The path of the docker secret containing the name of the user that Guacamole will use to connect to MySQL. +6. `MYSQL_PASSWORD` - The path of the docker secret containing the password that Guacamole will provide when connecting to MySQL as `MYSQL_USER`. --- End diff -- `MYSQL_PASSWORD` -> `MYSQL_PASSWORD_FILE` ---
[GitHub] guacamole-client pull request #320: Guacamole-626 - Add support for Docker s...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/320#discussion_r219652862 --- Diff: guacamole-docker/README.md --- @@ -90,6 +107,9 @@ the image will stop: 1. `MYSQL_DATABASE` - The name of the database to use for Guacamole authentication. 2. `MYSQL_USER` - The user that Guacamole will use to connect to MySQL. 3. `MYSQL_PASSWORD` - The password that Guacamole will provide when connecting to MySQL as `MYSQL_USER`. +4. `MYSQL_DATABASE_FILE` - The path of the docker secret containing the name of database to use for Guacamole authentication. +5. `MYSQL_USER` - The path of the docker secret containing the name of the user that Guacamole will use to connect to MySQL. --- End diff -- `MYSQL_USER` -> `MYSQL_USER_FILE` ---
[GitHub] guacamole-client pull request #320: Guacamole-626 - Add support for Docker s...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/320#discussion_r219652808 --- Diff: guacamole-docker/README.md --- @@ -45,6 +56,9 @@ the image will stop: 1. `POSTGRES_DATABASE` - The name of the database to use for Guacamole authentication. 2. `POSTGRES_USER` - The user that Guacamole will use to connect to PostgreSQL. 3. `POSTGRES_PASSWORD` - The password that Guacamole will provide when connecting to PostgreSQL as `POSTGRES_USER`. +4. `POSTGRES_DATABASE_FILE` - The path of the docker secret containing the name of database to use for Guacamole authentication. +5. `POSTGRES_USER` - The path of the docker secret containing the name of the user that Guacamole will use to connect to PostgreSQL. --- End diff -- This should probably be `POSTGRES_USER_FILE` if it's the "path of the docker secret," no? ---
[GitHub] guacamole-client pull request #320: Guacamole-626 - Add support for Docker s...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/320#discussion_r219652842 --- Diff: guacamole-docker/README.md --- @@ -45,6 +56,9 @@ the image will stop: 1. `POSTGRES_DATABASE` - The name of the database to use for Guacamole authentication. 2. `POSTGRES_USER` - The user that Guacamole will use to connect to PostgreSQL. 3. `POSTGRES_PASSWORD` - The password that Guacamole will provide when connecting to PostgreSQL as `POSTGRES_USER`. +4. `POSTGRES_DATABASE_FILE` - The path of the docker secret containing the name of database to use for Guacamole authentication. +5. `POSTGRES_USER` - The path of the docker secret containing the name of the user that Guacamole will use to connect to PostgreSQL. +6. `POSTGRES_PASSWORD` - The path of the docker secret containing the password that Guacamole will provide when connecting to PostgreSQL as `POSTGRES_USER`. --- End diff -- This should probably be `POSTGRES_PASSWORD_FILE` if it's the "path of the docker secret," no? ---
[GitHub] guacamole-server pull request #187: GUACAMOLE-622: Withhold first SSH/telnet...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/187#discussion_r217197430
--- Diff: src/protocols/telnet/settings.h ---
@@ -253,6 +267,16 @@ typedef struct guac_telnet_settings {
guac_telnet_settings* guac_telnet_parse_args(guac_user* user,
int argc, const char** argv);
+/**
+ * Frees the regex pointed to by the given pointer, assigning the value
NULL to
+ * that pointer once the regex is freed. If the pointer already contains
NULL,
+ * this functino has no effect.
--- End diff --
*function
---
