GUACAMOLE-5: Automatically clean up share keys and any associated tunnels when the connection being shared is closed.
Project: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/commit/afb377d5 Tree: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/tree/afb377d5 Diff: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/diff/afb377d5 Branch: refs/heads/master Commit: afb377d5ed47b9d3ee143d3e4e8f01414b675304 Parents: e54d36c Author: Michael Jumper <[email protected]> Authored: Sun Jul 24 16:53:10 2016 -0700 Committer: Michael Jumper <[email protected]> Committed: Mon Jul 25 12:07:20 2016 -0700 ---------------------------------------------------------------------- .../jdbc/sharing/ConnectionSharingService.java | 4 + .../jdbc/sharing/HashSharedConnectionMap.java | 8 +- .../sharing/SharedConnectionDefinition.java | 58 ++++++++++++ .../tunnel/AbstractGuacamoleTunnelService.java | 37 ++++++-- .../jdbc/tunnel/ActiveConnectionRecord.java | 95 +++++++++++++++----- 5 files changed, 173 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/afb377d5/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/ConnectionSharingService.java ---------------------------------------------------------------------- diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/ConnectionSharingService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/ConnectionSharingService.java index 68dadb7..a3f8a23 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/ConnectionSharingService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/ConnectionSharingService.java @@ -117,6 +117,10 @@ public class ConnectionSharingService { connectionMap.add(new SharedConnectionDefinition(activeConnection, sharingProfile, key)); + // Ensure the share key is properly invalidated when the original + // connection is closed + activeConnection.registerShareKey(key); + // Return credentials defining a single expected parameter return new UserCredentials(SHARE_KEY, Collections.singletonMap(SHARE_KEY_NAME, key)); http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/afb377d5/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/HashSharedConnectionMap.java ---------------------------------------------------------------------- diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/HashSharedConnectionMap.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/HashSharedConnectionMap.java index ab898e4..ddd812b 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/HashSharedConnectionMap.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/HashSharedConnectionMap.java @@ -64,7 +64,13 @@ public class HashSharedConnectionMap implements SharedConnectionMap { return null; // Attempt to retrieve only if non-null - return connectionMap.remove(key); + SharedConnectionDefinition definition = connectionMap.remove(key); + if (definition == null) + return null; + + // Close all associated tunnels and disallow further sharing + definition.invalidate(); + return definition; } http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/afb377d5/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedConnectionDefinition.java ---------------------------------------------------------------------- diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedConnectionDefinition.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedConnectionDefinition.java index e971bff..d4a6b7e 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedConnectionDefinition.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedConnectionDefinition.java @@ -19,8 +19,12 @@ package org.apache.guacamole.auth.jdbc.sharing; +import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.auth.jdbc.sharingprofile.ModeledSharingProfile; import org.apache.guacamole.auth.jdbc.tunnel.ActiveConnectionRecord; +import org.apache.guacamole.net.GuacamoleTunnel; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Defines the semantics/restrictions of a shared connection by associating an @@ -33,6 +37,11 @@ import org.apache.guacamole.auth.jdbc.tunnel.ActiveConnectionRecord; public class SharedConnectionDefinition { /** + * Logger for this class. + */ + private final Logger logger = LoggerFactory.getLogger(SharedConnectionDefinition.class); + + /** * The active connection being shared. */ private final ActiveConnectionRecord activeConnection; @@ -49,6 +58,28 @@ public class SharedConnectionDefinition { private final String shareKey; /** + * Manager which tracks all tunnels associated with this shared connection + * definition. All tunnels registered with this manager will be + * automatically closed once the manager is invalidated. + */ + private final SharedObjectManager<GuacamoleTunnel> tunnels = + new SharedObjectManager<GuacamoleTunnel>() { + + @Override + protected void cleanup(GuacamoleTunnel tunnel) { + + try { + tunnel.close(); + } + catch (GuacamoleException e) { + logger.debug("Unable to close tunnel of shared connection.", e); + } + + } + + }; + + /** * Creates a new SharedConnectionDefinition which describes an active * connection that can be joined, including the restrictions dictated by a * given sharing profile. @@ -104,4 +135,31 @@ public class SharedConnectionDefinition { return shareKey; } + /** + * Registers the given tunnel with this SharedConnectionDefinition, such + * that the tunnel is automatically closed when this + * SharedConnectionDefinition is invalidated. For shared connections to be + * properly closed when the associated share key ceases being valid, the + * tunnels resulting from the use of the share key MUST be registered to the + * SharedConnectionDefinition associated with that share key. + * + * @param tunnel + * The tunnel which should automatically be closed when this + * SharedConnectionDefinition is invalidated. + */ + public void registerTunnel(GuacamoleTunnel tunnel) { + tunnels.register(tunnel); + } + + /** + * Invalidates this SharedConnectionDefinition and closes all registered + * tunnels. If any additional tunnels are registered after this function is + * invoked, those tunnels will be immediately closed. This function MUST be + * invoked when the share key associated with this + * SharedConnectionDefinition will no longer be used. + */ + public void invalidate() { + tunnels.invalidate(); + } + } http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/afb377d5/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java ---------------------------------------------------------------------- diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java index b600a9f..85206eb 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java @@ -109,6 +109,12 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS private ConnectionRecordMapper connectionRecordMapper; /** + * Provider for creating active connection records. + */ + @Inject + private Provider<ActiveConnectionRecord> activeConnectionRecordProvider; + + /** * The hostname to use when connecting to guacd if no hostname is provided * within guacamole.properties. */ @@ -385,6 +391,9 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS if (!hasRun.compareAndSet(false, true)) return; + // Connection can no longer be shared + activeConnection.invalidate(); + // Remove underlying tunnel from list of active tunnels activeTunnels.remove(activeConnection.getUUID().toString()); @@ -621,9 +630,13 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS final ModeledConnection connection, GuacamoleClientInformation info) throws GuacamoleException { - // Acquire and connect to single connection + // Acquire access to single connection acquire(user, Collections.singletonList(connection)); - return assignGuacamoleTunnel(new ActiveConnectionRecord(user, connection), info); + + // Connect only if the connection was successfully acquired + ActiveConnectionRecord connectionRecord = activeConnectionRecordProvider.get(); + connectionRecord.init(user, connection); + return assignGuacamoleTunnel(connectionRecord, info); } @@ -663,7 +676,9 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS user.preferConnection(connection.getIdentifier()); // Connect to acquired child - return assignGuacamoleTunnel(new ActiveConnectionRecord(user, connectionGroup, connection), info); + ActiveConnectionRecord connectionRecord = activeConnectionRecordProvider.get(); + connectionRecord.init(user, connectionGroup, connection); + return assignGuacamoleTunnel(connectionRecord, info); } @@ -685,10 +700,18 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS GuacamoleClientInformation info) throws GuacamoleException { - // Connect to shared connection - return assignGuacamoleTunnel( - new ActiveConnectionRecord(user, definition.getActiveConnection(), - definition.getSharingProfile()), info); + // Create a connection record which describes the shared connection + ActiveConnectionRecord connectionRecord = activeConnectionRecordProvider.get(); + connectionRecord.init(user, definition.getActiveConnection(), + definition.getSharingProfile()); + + // Connect to shared connection described by the created record + GuacamoleTunnel tunnel = assignGuacamoleTunnel(connectionRecord, info); + + // Register tunnel, such that it is closed when the + // SharedConnectionDefinition is invalidated + definition.registerTunnel(tunnel); + return tunnel; } http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/afb377d5/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/ActiveConnectionRecord.java ---------------------------------------------------------------------- diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/ActiveConnectionRecord.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/ActiveConnectionRecord.java index 2a3ea4f..3edd95c 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/ActiveConnectionRecord.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/ActiveConnectionRecord.java @@ -19,10 +19,13 @@ package org.apache.guacamole.auth.jdbc.tunnel; +import com.google.inject.Inject; import java.util.Date; import java.util.UUID; import org.apache.guacamole.auth.jdbc.connection.ModeledConnection; import org.apache.guacamole.auth.jdbc.connectiongroup.ModeledConnectionGroup; +import org.apache.guacamole.auth.jdbc.sharing.SharedConnectionMap; +import org.apache.guacamole.auth.jdbc.sharing.SharedObjectManager; import org.apache.guacamole.auth.jdbc.sharingprofile.ModeledSharingProfile; import org.apache.guacamole.auth.jdbc.user.RemoteAuthenticatedUser; import org.apache.guacamole.net.AbstractGuacamoleTunnel; @@ -45,25 +48,25 @@ public class ActiveConnectionRecord implements ConnectionRecord { * The user that connected to the connection associated with this connection * record. */ - private final RemoteAuthenticatedUser user; + private RemoteAuthenticatedUser user; /** * The balancing group from which the associated connection was chosen, if * any. If no balancing group was used, this will be null. */ - private final ModeledConnectionGroup balancingGroup; + private ModeledConnectionGroup balancingGroup; /** * The connection associated with this connection record. */ - private final ModeledConnection connection; + private ModeledConnection connection; /** * The sharing profile that was used to access the connection associated * with this connection record. If the connection was accessed directly * (without involving a sharing profile), this will be null. */ - private final ModeledSharingProfile sharingProfile; + private ModeledSharingProfile sharingProfile; /** * The time this connection record was created. @@ -89,7 +92,29 @@ public class ActiveConnectionRecord implements ConnectionRecord { private GuacamoleTunnel tunnel; /** - * Creates a new connection record associated with the given user, + * Map of all currently-shared connections. + */ + @Inject + private SharedConnectionMap connectionMap; + + /** + * Manager which tracks all share keys associated with this connection + * record. All share keys registered with this manager will automatically be + * removed from the common SharedConnectionMap once the manager is + * invalidated. + */ + private final SharedObjectManager<String> shareKeyManager = + new SharedObjectManager<String>() { + + @Override + protected void cleanup(String key) { + connectionMap.remove(key); + } + + }; + + /** + * Initializes this connection record, associating it with the given user, * connection, balancing connection group, and sharing profile. The given * balancing connection group MUST be the connection group from which the * given connection was chosen, and the given sharing profile MUST be the @@ -112,7 +137,7 @@ public class ActiveConnectionRecord implements ConnectionRecord { * The sharing profile that was used to share access to the given * connection, or null if no sharing profile was used. */ - private ActiveConnectionRecord(RemoteAuthenticatedUser user, + private void init(RemoteAuthenticatedUser user, ModeledConnectionGroup balancingGroup, ModeledConnection connection, ModeledSharingProfile sharingProfile) { @@ -123,7 +148,7 @@ public class ActiveConnectionRecord implements ConnectionRecord { } /** - * Creates a new connection record associated with the given user, + * Initializes this connection record, associating it with the given user, * connection, and balancing connection group. The given balancing * connection group MUST be the connection group from which the given * connection was chosen. The start date of this connection record will be @@ -139,16 +164,16 @@ public class ActiveConnectionRecord implements ConnectionRecord { * @param connection * The connection to associate with this connection record. */ - public ActiveConnectionRecord(RemoteAuthenticatedUser user, + public void init(RemoteAuthenticatedUser user, ModeledConnectionGroup balancingGroup, ModeledConnection connection) { - this(user, balancingGroup, connection, null); + init(user, balancingGroup, connection, null); } /** - * Creates a new connection record associated with the given user and - * connection. The start date of this connection record will be the time of - * its creation. + * Initializes this connection record, associating it with the given user + * and connection. The start date of this connection record will be the time + * of its creation. * * @param user * The user that connected to the connection associated with this @@ -157,17 +182,17 @@ public class ActiveConnectionRecord implements ConnectionRecord { * @param connection * The connection to associate with this connection record. */ - public ActiveConnectionRecord(RemoteAuthenticatedUser user, + public void init(RemoteAuthenticatedUser user, ModeledConnection connection) { - this(user, null, connection); + init(user, null, connection); } /** - * Creates a new connection record associated with the given user, active - * connection, and sharing profile. The given sharing profile MUST be the - * sharing profile that was used to share access to the given connection. - * The start date of this connection record will be the time of its - * creation. + * Initializes this connection record, associating it with the given user, + * active connection, and sharing profile. The given sharing profile MUST be + * the sharing profile that was used to share access to the given + * connection. The start date of this connection record will be the time of + * its creation. * * @param user * The user that connected to the connection associated with this @@ -182,10 +207,10 @@ public class ActiveConnectionRecord implements ConnectionRecord { * connection. As a record created in this way always refers to a * shared connection, this value may NOT be null. */ - public ActiveConnectionRecord(RemoteAuthenticatedUser user, + public void init(RemoteAuthenticatedUser user, ActiveConnectionRecord activeConnection, ModeledSharingProfile sharingProfile) { - this(user, null, activeConnection.getConnection(), sharingProfile); + init(user, null, activeConnection.getConnection(), sharingProfile); this.connectionID = activeConnection.getConnectionID(); } @@ -402,4 +427,32 @@ public class ActiveConnectionRecord implements ConnectionRecord { return connectionID; } + /** + * Registers the given share key with this ActiveConnectionRecord, such that + * the key is automatically removed from the common SharedConnectionMap when + * the connection represented by this ActiveConnectionRecord is closed. For + * share keys to be properly invalidated when the connection being shared is + * closed, all such share keys MUST be registered with the + * ActiveConnectionRecord of the connection being shared. + * + * @param key + * The share key which should automatically be removed from the common + * SharedConnectionMap when the connection represented by this + * ActiveConnectionRecord is closed. + */ + public void registerShareKey(String key) { + shareKeyManager.register(key); + } + + /** + * Invalidates this ActiveConnectionRecord and all registered share keys. If + * any additional share keys are registered after this function is invoked, + * those keys will be immediately invalidated. This function MUST be invoked + * when the connection represented by this ActiveConnectionRecord is + * closing. + */ + public void invalidate() { + shareKeyManager.invalidate(); + } + }
