GUACAMOLE-5: Automatically clean up share keys and any associated tunnels when the connection being shared is closed.

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));

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;
 
     }
 

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<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
@@ -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();
+    }
+
 }

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<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
+        // Create a connection record which describes the shared connection
-        return assignGuacamoleTunnel(
+        ActiveConnectionRecord connectionRecord = activeConnectionRecordProvider.get();
-                new ActiveConnectionRecord(user, definition.getActiveConnection(),
+        connectionRecord.init(user, definition.getActiveConnection(),
-                        definition.getSharingProfile()), info);
+                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;
 
     }
 

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
+     * Initializes this connection record, associating it with the given user
-     * connection. The start date of this connection record will be the time of
+     * and connection. The start date of this connection record will be the time
-     * its creation.
+     * 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
+     * Initializes this connection record, associating it with the given user,
-     * connection, and sharing profile. The given sharing profile MUST be the
+     * active connection, and sharing profile. The given sharing profile MUST be
-     * sharing profile that was used to share access to the given connection.
+     * the sharing profile that was used to share access to the given
-     * The start date of this connection record will be the time of its
+     * connection. The start date of this connection record will be the time of
-     * creation.
+     * 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();
+    }
+
 }