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 68dadb753..a3f8a23bc 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)); 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 ab898e414..ddd812b3b 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; } 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 e971bffde..d4a6b7e8f 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 @@ -32,6 +36,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. */ @@ -48,6 +57,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 tunnels = + new SharedObjectManager() { + + @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 @@ -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(); + } + } 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 b600a9f30..85206eb5b 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 @@ -108,6 +108,12 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS @Inject private ConnectionRecordMapper connectionRecordMapper; + /** + * Provider for creating active connection records. + */ + @Inject + private Provider 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; } 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 2a3ea4f5b..3edd95cc7 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 shareKeyManager = + new SharedObjectManager() { + + @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(); + } + }