From d334aa97d68d7e0bf1c0d9facd4c2ccc149c33e3 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 22 Jul 2016 16:49:05 -0700 Subject: [PATCH 1/5] GUACAMOLE-5: Store share key within SharedConnectionDefinition. --- .../sharing/ConnectionSharingService.java | 4 ++-- .../jdbc/sharing/HashSharedConnectionMap.java | 8 +++++-- .../sharing/SharedConnectionDefinition.java | 22 ++++++++++++++++++- .../jdbc/sharing/SharedConnectionMap.java | 10 +++------ 4 files changed, 32 insertions(+), 12 deletions(-) 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 7b97f5701..e05417a18 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 @@ -114,8 +114,8 @@ public class ConnectionSharingService { // Generate a share key for the requested connection String key = keyGenerator.getShareKey(); - connectionMap.put(key, new SharedConnectionDefinition(activeConnection, - sharingProfile)); + connectionMap.add(new SharedConnectionDefinition(activeConnection, + sharingProfile, key)); // Return credentials defining a single expected parameter return new UserCredentials(SHARE_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 e3dff02a7..ab898e414 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 @@ -48,8 +48,12 @@ public class HashSharedConnectionMap implements SharedConnectionMap { } @Override - public void put(String key, SharedConnectionDefinition definition) { - connectionMap.put(key, definition); + public void add(SharedConnectionDefinition definition) { + + // Store definition by share key + String shareKey = definition.getShareKey(); + connectionMap.put(shareKey, definition); + } @Override 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 78ed62cdd..7e7566bee 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 @@ -43,6 +43,11 @@ public class SharedConnectionDefinition { */ private final ModeledSharingProfile sharingProfile; + /** + * The unique key with which a user may access the shared connection. + */ + private final String shareKey; + /** * Creates a new SharedConnectionDefinition which describes an active * connection that can be joined, including the restrictions dictated by a @@ -54,11 +59,15 @@ public class SharedConnectionDefinition { * @param sharingProfile * A sharing profile whose associated parameters dictate the level of * access provided to the shared connection. + * + * @param shareKey + * The unique key with which a user may access the shared connection. */ public SharedConnectionDefinition(TrackedActiveConnection activeConnection, - ModeledSharingProfile sharingProfile) { + ModeledSharingProfile sharingProfile, String shareKey) { this.activeConnection = activeConnection; this.sharingProfile = sharingProfile; + this.shareKey = shareKey; } /** @@ -84,4 +93,15 @@ public class SharedConnectionDefinition { return sharingProfile; } + /** + * Returns the unique key with which a user may access the shared + * connection. + * + * @return + * The unique key with which a user may access the shared connection. + */ + public String getShareKey() { + return shareKey; + } + } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedConnectionMap.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedConnectionMap.java index 2df4b2d76..29bce5050 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedConnectionMap.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedConnectionMap.java @@ -28,19 +28,15 @@ package org.apache.guacamole.auth.jdbc.sharing; public interface SharedConnectionMap { /** - * Associates the given share key with a SharedConnectionDefinition, + * Stores the given SharedConnectionDefinition by its associated share key, * allowing the connection it describes to be accessed by users having the * share key. * - * @param key - * The share key to use to share the connection described by the given - * SharedConnectionDefinition. - * * @param definition * The SharedConnectionDefinition describing the connection being - * shared via the given share key. + * shared. */ - public void put(String key, SharedConnectionDefinition definition); + public void add(SharedConnectionDefinition definition); /** * Retrieves the connection definition associated with the given share key. From 16fce2931f173a5ff1efc759f9e31c0769bba82e Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 22 Jul 2016 16:53:40 -0700 Subject: [PATCH 2/5] GUACAMOLE-5: Store SharedConnectionDefinition directly, rather than passing around its contents. --- .../auth/jdbc/sharing/SharedConnection.java | 29 +++++++++---------- .../AbstractGuacamoleTunnelService.java | 11 +++---- .../jdbc/tunnel/GuacamoleTunnelService.java | 15 ++++------ 3 files changed, 24 insertions(+), 31 deletions(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedConnection.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedConnection.java index 70b894429..18862dc19 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedConnection.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedConnection.java @@ -26,9 +26,7 @@ import java.util.Map; import java.util.Set; import java.util.UUID; import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.auth.jdbc.activeconnection.TrackedActiveConnection; import org.apache.guacamole.auth.jdbc.connectiongroup.RootConnectionGroup; -import org.apache.guacamole.auth.jdbc.sharingprofile.ModeledSharingProfile; import org.apache.guacamole.auth.jdbc.tunnel.GuacamoleTunnelService; import org.apache.guacamole.net.GuacamoleTunnel; import org.apache.guacamole.net.auth.Connection; @@ -64,15 +62,10 @@ public class SharedConnection implements Connection { private SharedConnectionUser user; /** - * The active connection being shared. + * The SharedConnectionDefinition dictating the connection being shared and + * any associated restrictions. */ - private TrackedActiveConnection activeConnection; - - /** - * The sharing profile which dictates the level of access provided to a user - * of the shared connection. - */ - private ModeledSharingProfile sharingProfile; + private SharedConnectionDefinition definition; /** * Creates a new SharedConnection which can be used to join the connection @@ -88,8 +81,7 @@ public class SharedConnection implements Connection { */ public void init(SharedConnectionUser user, SharedConnectionDefinition definition) { this.user = user; - this.activeConnection = definition.getActiveConnection(); - this.sharingProfile = definition.getSharingProfile(); + this.definition = definition; } @Override @@ -104,7 +96,7 @@ public class SharedConnection implements Connection { @Override public String getName() { - return sharingProfile.getName(); + return definition.getSharingProfile().getName(); } @Override @@ -124,9 +116,15 @@ public class SharedConnection implements Connection { @Override public GuacamoleConfiguration getConfiguration() { + + // Pull the connection being shared + Connection primaryConnection = definition.getActiveConnection().getConnection(); + + // Construct a skeletal configuration that exposes only the protocol in use GuacamoleConfiguration config = new GuacamoleConfiguration(); - config.setProtocol(activeConnection.getConnection().getConfiguration().getProtocol()); + config.setProtocol(primaryConnection.getConfiguration().getProtocol()); return config; + } @Override @@ -137,8 +135,7 @@ public class SharedConnection implements Connection { @Override public GuacamoleTunnel connect(GuacamoleClientInformation info) throws GuacamoleException { - return tunnelService.getGuacamoleTunnel(user, activeConnection, - sharingProfile, info); + return tunnelService.getGuacamoleTunnel(user, definition, info); } @Override 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 d82563195..b600a9f30 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 @@ -42,7 +42,6 @@ import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleResourceNotFoundException; import org.apache.guacamole.GuacamoleSecurityException; import org.apache.guacamole.auth.jdbc.JDBCEnvironment; -import org.apache.guacamole.auth.jdbc.activeconnection.TrackedActiveConnection; import org.apache.guacamole.auth.jdbc.connection.ConnectionMapper; import org.apache.guacamole.environment.Environment; import org.apache.guacamole.net.GuacamoleSocket; @@ -56,6 +55,7 @@ import org.apache.guacamole.token.StandardTokens; import org.apache.guacamole.token.TokenFilter; import org.mybatis.guice.transactional.Transactional; import org.apache.guacamole.auth.jdbc.connection.ConnectionParameterMapper; +import org.apache.guacamole.auth.jdbc.sharing.SharedConnectionDefinition; import org.apache.guacamole.auth.jdbc.sharing.SharedConnectionUser; import org.apache.guacamole.auth.jdbc.sharingprofile.ModeledSharingProfile; import org.apache.guacamole.auth.jdbc.sharingprofile.SharingProfileParameterMapper; @@ -467,7 +467,7 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS // Verify that the connection ID is known String connectionID = activeConnection.getConnectionID(); - if (!activeConnection.isActive() || connectionID == null) + if (connectionID == null) throw new GuacamoleResourceNotFoundException("No existing connection to be joined."); // Build configuration from the sharing profile and the ID of @@ -681,13 +681,14 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS @Override @Transactional public GuacamoleTunnel getGuacamoleTunnel(SharedConnectionUser user, - TrackedActiveConnection activeConnection, - ModeledSharingProfile sharingProfile, + SharedConnectionDefinition definition, GuacamoleClientInformation info) throws GuacamoleException { // Connect to shared connection - return assignGuacamoleTunnel(new ActiveConnectionRecord(user, activeConnection, sharingProfile), info); + return assignGuacamoleTunnel( + new ActiveConnectionRecord(user, definition.getActiveConnection(), + definition.getSharingProfile()), info); } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/GuacamoleTunnelService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/GuacamoleTunnelService.java index 6a00b2e2c..34965a7b9 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/GuacamoleTunnelService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/GuacamoleTunnelService.java @@ -24,9 +24,8 @@ import org.apache.guacamole.auth.jdbc.user.AuthenticatedUser; import org.apache.guacamole.auth.jdbc.connection.ModeledConnection; import org.apache.guacamole.auth.jdbc.connectiongroup.ModeledConnectionGroup; import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.auth.jdbc.activeconnection.TrackedActiveConnection; +import org.apache.guacamole.auth.jdbc.sharing.SharedConnectionDefinition; import org.apache.guacamole.auth.jdbc.sharing.SharedConnectionUser; -import org.apache.guacamole.auth.jdbc.sharingprofile.ModeledSharingProfile; import org.apache.guacamole.net.GuacamoleTunnel; import org.apache.guacamole.net.auth.Connection; import org.apache.guacamole.net.auth.ConnectionGroup; @@ -158,12 +157,9 @@ public interface GuacamoleTunnelService { * @param user * The user for whom the connection is being established. * - * @param activeConnection - * The active connection the user is joining. - * - * @param sharingProfile - * The sharing profile whose associated parameters dictate the level - * of access granted to the user joining the connection. + * @param definition + * The SharedConnectionDefinition dictating the connection being shared + * and any associated restrictions. * * @param info * Information describing the Guacamole client connecting to the given @@ -178,8 +174,7 @@ public interface GuacamoleTunnelService { * rules. */ GuacamoleTunnel getGuacamoleTunnel(SharedConnectionUser user, - TrackedActiveConnection activeConnection, - ModeledSharingProfile sharingProfile, + SharedConnectionDefinition definition, GuacamoleClientInformation info) throws GuacamoleException; From b201eac61707cff57dda4cb5992401757ac0887d Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 24 Jul 2016 14:34:22 -0700 Subject: [PATCH 3/5] GUACAMOLE-5: Use ActiveConnectionRecord as the basis for sharing. TrackedActiveConnection is really only meant for interchange via the ActiveConnection Directory. --- .../TrackedActiveConnection.java | 30 +++++-------------- .../sharing/ConnectionSharingService.java | 4 +-- .../sharing/SharedConnectionDefinition.java | 12 ++++---- .../jdbc/tunnel/ActiveConnectionRecord.java | 3 +- 4 files changed, 17 insertions(+), 32 deletions(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/activeconnection/TrackedActiveConnection.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/activeconnection/TrackedActiveConnection.java index 6c2e4d5a8..29243f4d1 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/activeconnection/TrackedActiveConnection.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/activeconnection/TrackedActiveConnection.java @@ -50,6 +50,12 @@ public class TrackedActiveConnection extends RestrictedObject implements ActiveC */ private String identifier; + /** + * The actual connection record from which this ActiveConnection derives its + * data. + */ + private ActiveConnectionRecord connectionRecord; + /** * The connection being actively used or shared. */ @@ -75,13 +81,6 @@ public class TrackedActiveConnection extends RestrictedObject implements ActiveC */ private String username; - /** - * The connection ID of the connection as determined by guacd, not to be - * confused with the connection identifier determined by the database. This - * is the ID that must be supplied to guacd if joining this connection. - */ - private String connectionID; - /** * The underlying GuacamoleTunnel. */ @@ -111,10 +110,10 @@ public class TrackedActiveConnection extends RestrictedObject implements ActiveC boolean includeSensitiveInformation) { super.init(currentUser); + this.connectionRecord = activeConnectionRecord; // Copy all non-sensitive data from given record this.connection = activeConnectionRecord.getConnection(); - this.connectionID = activeConnectionRecord.getConnectionID(); this.sharingProfileIdentifier = activeConnectionRecord.getSharingProfileIdentifier(); this.identifier = activeConnectionRecord.getUUID().toString(); this.startDate = activeConnectionRecord.getStartDate(); @@ -150,19 +149,6 @@ public class TrackedActiveConnection extends RestrictedObject implements ActiveC return connection; } - /** - * Returns the connection ID of the in-progress connection as determined by - * guacd, not to be confused with the connection identifier determined by - * the database. This is the ID that must be supplied to guacd if joining - * this connection. - * - * @return - * The ID of the in-progress connection, as determined by guacd. - */ - public String getConnectionID() { - return connectionID; - } - @Override public String getConnectionIdentifier() { return connection.getIdentifier(); @@ -189,7 +175,7 @@ public class TrackedActiveConnection extends RestrictedObject implements ActiveC public UserCredentials getSharingCredentials(String identifier) throws GuacamoleException { return sharingService.generateTemporaryCredentials(getCurrentUser(), - this, identifier); + connectionRecord, identifier); } @Override 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 e05417a18..68dadb753 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 @@ -25,9 +25,9 @@ import javax.servlet.http.HttpServletRequest; import org.apache.guacamole.auth.jdbc.user.AuthenticatedUser; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleSecurityException; -import org.apache.guacamole.auth.jdbc.activeconnection.TrackedActiveConnection; import org.apache.guacamole.auth.jdbc.sharingprofile.ModeledSharingProfile; import org.apache.guacamole.auth.jdbc.sharingprofile.SharingProfileService; +import org.apache.guacamole.auth.jdbc.tunnel.ActiveConnectionRecord; import org.apache.guacamole.form.Field; import org.apache.guacamole.net.auth.AuthenticationProvider; import org.apache.guacamole.net.auth.Credentials; @@ -98,7 +98,7 @@ public class ConnectionSharingService { * If permission to share the given connection is denied. */ public UserCredentials generateTemporaryCredentials(AuthenticatedUser user, - TrackedActiveConnection activeConnection, + ActiveConnectionRecord activeConnection, String sharingProfileIdentifier) throws GuacamoleException { // Pull sharing profile (verifying access) 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 7e7566bee..e971bffde 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,8 @@ package org.apache.guacamole.auth.jdbc.sharing; -import org.apache.guacamole.auth.jdbc.activeconnection.TrackedActiveConnection; import org.apache.guacamole.auth.jdbc.sharingprofile.ModeledSharingProfile; +import org.apache.guacamole.auth.jdbc.tunnel.ActiveConnectionRecord; /** * Defines the semantics/restrictions of a shared connection by associating an @@ -35,7 +35,7 @@ public class SharedConnectionDefinition { /** * The active connection being shared. */ - private final TrackedActiveConnection activeConnection; + private final ActiveConnectionRecord activeConnection; /** * The sharing profile which dictates the level of access provided to a user @@ -63,7 +63,7 @@ public class SharedConnectionDefinition { * @param shareKey * The unique key with which a user may access the shared connection. */ - public SharedConnectionDefinition(TrackedActiveConnection activeConnection, + public SharedConnectionDefinition(ActiveConnectionRecord activeConnection, ModeledSharingProfile sharingProfile, String shareKey) { this.activeConnection = activeConnection; this.sharingProfile = sharingProfile; @@ -71,13 +71,13 @@ public class SharedConnectionDefinition { } /** - * Returns the TrackedActiveConnection of the actual in-progress connection + * Returns the ActiveConnectionRecord of the actual in-progress connection * being shared. * * @return - * The TrackedActiveConnection being shared. + * The ActiveConnectionRecord being shared. */ - public TrackedActiveConnection getActiveConnection() { + public ActiveConnectionRecord getActiveConnection() { return activeConnection; } 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 16da6899b..2a3ea4f5b 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 @@ -21,7 +21,6 @@ package org.apache.guacamole.auth.jdbc.tunnel; import java.util.Date; import java.util.UUID; -import org.apache.guacamole.auth.jdbc.activeconnection.TrackedActiveConnection; import org.apache.guacamole.auth.jdbc.connection.ModeledConnection; import org.apache.guacamole.auth.jdbc.connectiongroup.ModeledConnectionGroup; import org.apache.guacamole.auth.jdbc.sharingprofile.ModeledSharingProfile; @@ -184,7 +183,7 @@ public class ActiveConnectionRecord implements ConnectionRecord { * shared connection, this value may NOT be null. */ public ActiveConnectionRecord(RemoteAuthenticatedUser user, - TrackedActiveConnection activeConnection, + ActiveConnectionRecord activeConnection, ModeledSharingProfile sharingProfile) { this(user, null, activeConnection.getConnection(), sharingProfile); this.connectionID = activeConnection.getConnectionID(); From e54d36cae56c22f1c23e6afe1e1aedb6aa0a1d4e Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 24 Jul 2016 16:02:12 -0700 Subject: [PATCH 4/5] GUACAMOLE-5: Implement thread-safe automatic cleanup of a group of shared objects. --- .../jdbc/sharing/SharedObjectManager.java | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedObjectManager.java diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedObjectManager.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedObjectManager.java new file mode 100644 index 000000000..c04148510 --- /dev/null +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharing/SharedObjectManager.java @@ -0,0 +1,124 @@ +/* + * 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.sharing; + +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * Provides thread-safe registration and cleanup of a growing set of objects. + * Each SharedObjectManager can track arbitrarily-many objects, each registered + * with the register() function. A SharedObjectManager tracks objects until it + * is invalidated, after which all registered objects are cleaned up. Attempts + * to register new objects after the SharedObjectManager has been invalidated + * will cause the provided object to be immediately cleaned up. + * + * @author Michael Jumper + * @param + * The type of object managed by this SharedObjectManager. + */ +public abstract class SharedObjectManager { + + /** + * Whether this SharedObjectManager has been invalidated. + */ + private final AtomicBoolean invalidated = new AtomicBoolean(false); + + /** + * The collection of all objects being tracked by this SharedObjectManager. + */ + private final Queue objects = new ConcurrentLinkedQueue(); + + /** + * Cleans up the given object. This function is invoked exactly once on all + * tracked objects after invalidate() is called, and exactly once for any + * call to register() which occurs after invalidate() was called. + * + * @param object + * The object to cleanup. + */ + protected abstract void cleanup(T object); + + /** + * Invokes the cleanup() function on all tracked objects, removing each + * object from the underlying collection. It is guaranteed that cleanup() + * will be invoked only once for each object, even if multiple calls to + * cleanupAll() are running concurrently, and that the underlying collection + * will be empty after all calls to cleanupAll() complete. + */ + private void cleanupAll() { + + T current; + + // Remove all objects from underlying collection, cleaning up each + // object individually + while ((current = objects.poll()) != null) + cleanup(current); + + } + + /** + * Registers the given object with this SharedObjectManager such that it is + * cleaned up once the SharedObjectManager is invalidated. If the + * SharedObjectManager has already been invalidated, the object will be + * cleaned up immediately. + * + * @param object + * The object to register with this SharedObjectManager. + */ + public void register(T object) { + + // If already invalidated (or invalidation is in progress), avoid adding + // the object unnecessarily - just cleanup now + if (invalidated.get()) { + cleanup(object); + return; + } + + // Store provided object + objects.add(object); + + // If collection was invalidated while object was being added, recheck + // the underlying collection and cleanup + if (invalidated.get()) + cleanupAll(); + + } + + /** + * Invalidates this SharedObjectManager, cleaning up any registered objects + * and preventing future registration of objects. If attempts to register + * objects are made after this function is invoked, those objects will be + * immediately cleaned up. + */ + public void invalidate() { + + // Mark collection as invalidated, but do not bother cleaning up if + // already invalidated + if (!invalidated.compareAndSet(false, true)) + return; + + // Clean up all stored objects + cleanupAll(); + + } + +} From afb377d5ed47b9d3ee143d3e4e8f01414b675304 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 24 Jul 2016 16:53:10 -0700 Subject: [PATCH 5/5] GUACAMOLE-5: Automatically clean up share keys and any associated tunnels when the connection being shared is closed. --- .../sharing/ConnectionSharingService.java | 4 + .../jdbc/sharing/HashSharedConnectionMap.java | 8 +- .../sharing/SharedConnectionDefinition.java | 58 +++++++++++ .../AbstractGuacamoleTunnelService.java | 37 ++++++-- .../jdbc/tunnel/ActiveConnectionRecord.java | 95 +++++++++++++++---- 5 files changed, 173 insertions(+), 29 deletions(-) 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(); + } + }