From 45ac06e0d038699f77208daecced62a62aed0ac7 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 9 Feb 2022 10:11:38 -0800 Subject: [PATCH] GUACAMOLE-462: Create connection history records for in-progress connections. Besides restoring historically-provided functionality, the ID generated by the database for connection history records is needed to generate a deterministic UUID that can be injected into connection configurations with ${HISTORY_UUID}. Having such a token allows session recordings to be given names that can be reliably matched with history records. --- .../auth/jdbc/base/ModeledActivityRecord.java | 14 +- .../connection/ConnectionRecordMapper.java | 14 ++ .../connection/ModeledConnectionRecord.java | 5 + .../AbstractGuacamoleTunnelService.java | 67 +++---- .../jdbc/tunnel/ActiveConnectionRecord.java | 186 +++++++++--------- .../connection/ConnectionRecordMapper.xml | 12 +- .../connection/ConnectionRecordMapper.xml | 12 +- .../connection/ConnectionRecordMapper.xml | 12 +- 8 files changed, 183 insertions(+), 139 deletions(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/ModeledActivityRecord.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/ModeledActivityRecord.java index 39bbcbf1c..927e78c3f 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/ModeledActivityRecord.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/ModeledActivityRecord.java @@ -63,6 +63,18 @@ public class ModeledActivityRecord implements ActivityRecord { this.namespace = namespace; } + /** + * Returns the backing model object. Changes to this record will affect the + * backing model object, and changes to the backing model object will + * affect this record. + * + * @return + * The backing model object. + */ + public ActivityRecordModel getModel() { + return model; + } + @Override public Date getStartDate() { return model.getStartDate(); @@ -103,5 +115,5 @@ public class ModeledActivityRecord implements ActivityRecord { .array()); } - + } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ConnectionRecordMapper.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ConnectionRecordMapper.java index 720ded317..bf329ff85 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ConnectionRecordMapper.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ConnectionRecordMapper.java @@ -56,6 +56,20 @@ public interface ConnectionRecordMapper { */ int insert(@Param("record") ConnectionRecordModel record); + /** + * Updates the given connection record in the database, assigning an end + * date. No column of the existing connection record is updated except for + * the end date. If the record does not actually exist, this operation has + * no effect. + * + * @param record + * The connection record to update. + * + * @return + * The number of rows updated. + */ + int updateEndDate(@Param("record") ConnectionRecordModel record); + /** * Searches for up to limit connection records that contain * the given terms, sorted by the given predicates, regardless of whether diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ModeledConnectionRecord.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ModeledConnectionRecord.java index b02ca14c1..9daa8f305 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ModeledConnectionRecord.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ModeledConnectionRecord.java @@ -67,4 +67,9 @@ public class ModeledConnectionRecord extends ModeledActivityRecord return model.getSharingProfileName(); } + @Override + public ConnectionRecordModel getModel() { + return model; + } + } 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 5e42688df..8c16363a6 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 @@ -30,6 +30,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.guacamole.auth.jdbc.user.ModeledAuthenticatedUser; @@ -56,6 +57,7 @@ import org.apache.guacamole.protocol.GuacamoleConfiguration; 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.SharedConnectionMap; import org.apache.guacamole.auth.jdbc.sharing.connection.SharedConnectionDefinition; import org.apache.guacamole.auth.jdbc.sharingprofile.ModeledSharingProfile; import org.apache.guacamole.auth.jdbc.sharingprofile.SharingProfileParameterMapper; @@ -110,10 +112,10 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS private ConnectionRecordMapper connectionRecordMapper; /** - * Provider for creating active connection records. + * Map of all currently-shared connections. */ @Inject - private Provider activeConnectionRecordProvider; + private SharedConnectionMap connectionMap; /** * All active connections through the tunnel having a given UUID. @@ -253,33 +255,6 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS } - /** - * Saves the given ActiveConnectionRecord to the database. The end date of - * the saved record will be populated with the current time. - * - * @param record - * The record to save. - */ - private void saveConnectionRecord(ActiveConnectionRecord record) { - - // Get associated models - ConnectionRecordModel recordModel = new ConnectionRecordModel(); - - // Copy user information and timestamps into new record - recordModel.setUsername(record.getUsername()); - recordModel.setConnectionIdentifier(record.getConnectionIdentifier()); - recordModel.setConnectionName(record.getConnectionName()); - recordModel.setRemoteHost(record.getRemoteHost()); - recordModel.setSharingProfileIdentifier(record.getSharingProfileIdentifier()); - recordModel.setSharingProfileName(record.getSharingProfileName()); - recordModel.setStartDate(record.getStartDate()); - recordModel.setEndDate(new Date()); - - // Insert connection record - connectionRecordMapper.insert(recordModel); - - } - /** * Returns an unconfigured GuacamoleSocket that is already connected to * guacd as specified in guacamole.properties, using SSL if necessary. @@ -370,7 +345,9 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS activeConnection.invalidate(); // Remove underlying tunnel from list of active tunnels - activeTunnels.remove(activeConnection.getUUID().toString()); + UUID uuid = activeConnection.getUUID(); // May be null if record not successfully inserted + if (uuid != null) + activeTunnels.remove(uuid.toString()); // Get original user RemoteAuthenticatedUser user = activeConnection.getUser(); @@ -393,9 +370,11 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS // Release any associated group if (activeConnection.hasBalancingGroup()) release(user, activeConnection.getBalancingGroup()); - - // Save history record to database - saveConnectionRecord(activeConnection); + + // Update history record with end date + ConnectionRecordModel recordModel = activeConnection.getModel(); + recordModel.setEndDate(new Date()); + connectionRecordMapper.updateEndDate(recordModel); } @@ -439,7 +418,16 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS // Record new active connection Runnable cleanupTask = new ConnectionCleanupTask(activeConnection); - activeTunnels.put(activeConnection.getUUID().toString(), activeConnection); + try { + connectionRecordMapper.insert(activeConnection.getModel()); // This MUST happen before getUUID() is invoked, to ensure the ID driving the UUID exists + activeTunnels.put(activeConnection.getUUID().toString(), activeConnection); + } + + // Execute cleanup if connection history could not be updated + catch (RuntimeException | Error e) { + cleanupTask.run(); + throw e; + } try { @@ -643,8 +631,7 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS acquire(user, Collections.singletonList(connection), true); // Connect only if the connection was successfully acquired - ActiveConnectionRecord connectionRecord = activeConnectionRecordProvider.get(); - connectionRecord.init(user, connection); + ActiveConnectionRecord connectionRecord = new ActiveConnectionRecord(connectionMap, user, connection); return assignGuacamoleTunnel(connectionRecord, info, tokens, false); } @@ -690,8 +677,7 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS try { // Connect to acquired child - ActiveConnectionRecord connectionRecord = activeConnectionRecordProvider.get(); - connectionRecord.init(user, connectionGroup, connection); + ActiveConnectionRecord connectionRecord = new ActiveConnectionRecord(connectionMap, user, connectionGroup, connection); GuacamoleTunnel tunnel = assignGuacamoleTunnel(connectionRecord, info, tokens, connections.size() > 1); @@ -746,9 +732,8 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS throws GuacamoleException { // Create a connection record which describes the shared connection - ActiveConnectionRecord connectionRecord = activeConnectionRecordProvider.get(); - connectionRecord.init(user, definition.getActiveConnection(), - definition.getSharingProfile()); + ActiveConnectionRecord connectionRecord = new ActiveConnectionRecord(connectionMap, + user, definition.getActiveConnection(), definition.getSharingProfile()); // Connect to shared connection described by the created record GuacamoleTunnel tunnel = assignGuacamoleTunnel(connectionRecord, info, tokens, false); 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 d1c4545cb..c3150ca97 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,11 @@ 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.ConnectionRecordModel; import org.apache.guacamole.auth.jdbc.connection.ModeledConnection; +import org.apache.guacamole.auth.jdbc.connection.ModeledConnectionRecord; import org.apache.guacamole.auth.jdbc.connectiongroup.ModeledConnectionGroup; import org.apache.guacamole.auth.jdbc.sharing.SharedConnectionMap; import org.apache.guacamole.auth.jdbc.sharing.SharedObjectManager; @@ -31,7 +32,6 @@ import org.apache.guacamole.auth.jdbc.user.RemoteAuthenticatedUser; import org.apache.guacamole.net.AbstractGuacamoleTunnel; import org.apache.guacamole.net.GuacamoleSocket; import org.apache.guacamole.net.GuacamoleTunnel; -import org.apache.guacamole.net.auth.ConnectionRecord; /** @@ -39,41 +39,31 @@ import org.apache.guacamole.net.auth.ConnectionRecord; * the associated connection has not yet ended, getEndDate() will always return * null. The associated start date will be the time of this objects creation. */ -public class ActiveConnectionRecord implements ConnectionRecord { +public class ActiveConnectionRecord extends ModeledConnectionRecord { /** * The user that connected to the connection associated with this connection * record. */ - private RemoteAuthenticatedUser user; + private final 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 ModeledConnectionGroup balancingGroup; + private final ModeledConnectionGroup balancingGroup; /** * The connection associated with this connection record. */ - private ModeledConnection connection; + private final 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 ModeledSharingProfile sharingProfile; - - /** - * The time this connection record was created. - */ - private final Date startDate = new Date(); - - /** - * The UUID that will be assigned to the underlying tunnel. - */ - private final UUID uuid = UUID.randomUUID(); + private final ModeledSharingProfile sharingProfile; /** * The connection ID of the connection as determined by guacd, not to be @@ -91,8 +81,7 @@ public class ActiveConnectionRecord implements ConnectionRecord { /** * Map of all currently-shared connections. */ - @Inject - private SharedConnectionMap connectionMap; + private final SharedConnectionMap connectionMap; /** * Manager which tracks all share keys associated with this connection @@ -111,7 +100,57 @@ public class ActiveConnectionRecord implements ConnectionRecord { }; /** - * Initializes this connection record, associating it with the given user, + * Creates a new connection record model object, associating it with the + * given user, 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. No end date will be assigned. + * + * @param user + * The user that connected to the connection associated with this + * connection record. + * + * @param connection + * The connection to associate with this connection record. + * + * @param sharingProfile + * The sharing profile that was used to share access to the given + * connection, or null if no sharing profile was used. + * + * @return + * A new connection record model object associated with the given user, + * connection, and sharing profile, and having the current date/time as + * its start date. + */ + private static ConnectionRecordModel createModel(RemoteAuthenticatedUser user, + ModeledConnection connection, + ModeledSharingProfile sharingProfile) { + + // Create model object representing an active connection that started + // at the current time ... + ConnectionRecordModel recordModel = new ConnectionRecordModel(); + recordModel.setStartDate(new Date()); + + // ... was established by the given user ... + recordModel.setUsername(user.getIdentifier()); + recordModel.setRemoteHost(user.getRemoteHost()); + + // ... to the given connection ... + recordModel.setConnectionIdentifier(connection.getIdentifier()); + recordModel.setConnectionName(connection.getName()); + + // ... using the given sharing profile (if any) + if (sharingProfile != null) { + recordModel.setSharingProfileIdentifier(sharingProfile.getIdentifier()); + recordModel.setSharingProfileName(sharingProfile.getName()); + } + + return recordModel; + + } + + /** + * Creates a new ActiveConnectionRecord associated 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 @@ -119,6 +158,10 @@ public class ActiveConnectionRecord implements ConnectionRecord { * The start date of this connection record will be the time of its * creation. * + * @param connectionMap + * The SharedConnectionMap instance tracking all active shared + * connections. + * * @param user * The user that connected to the connection associated with this * connection record. @@ -134,10 +177,13 @@ 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 void init(RemoteAuthenticatedUser user, + public ActiveConnectionRecord(SharedConnectionMap connectionMap, + RemoteAuthenticatedUser user, ModeledConnectionGroup balancingGroup, ModeledConnection connection, ModeledSharingProfile sharingProfile) { + super(createModel(user, connection, sharingProfile)); + this.connectionMap = connectionMap; this.user = user; this.balancingGroup = balancingGroup; this.connection = connection; @@ -145,12 +191,16 @@ public class ActiveConnectionRecord implements ConnectionRecord { } /** - * Initializes this connection record, associating it with the given user, + * Creates a new ActiveConnectionRecord associated 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 * the time of its creation. * + * @param connectionMap + * The SharedConnectionMap instance tracking all active shared + * connections. + * * @param user * The user that connected to the connection associated with this * connection record. @@ -161,17 +211,22 @@ public class ActiveConnectionRecord implements ConnectionRecord { * @param connection * The connection to associate with this connection record. */ - public void init(RemoteAuthenticatedUser user, + public ActiveConnectionRecord(SharedConnectionMap connectionMap, + RemoteAuthenticatedUser user, ModeledConnectionGroup balancingGroup, ModeledConnection connection) { - init(user, balancingGroup, connection, null); + this(connectionMap, user, balancingGroup, connection, null); } /** - * Initializes this connection record, associating it with the given user + * Creates a new ActiveConnectionRecord associated with the given user, * and connection. The start date of this connection record will be the time * of its creation. * + * @param connectionMap + * The SharedConnectionMap instance tracking all active shared + * connections. + * * @param user * The user that connected to the connection associated with this * connection record. @@ -179,18 +234,22 @@ public class ActiveConnectionRecord implements ConnectionRecord { * @param connection * The connection to associate with this connection record. */ - public void init(RemoteAuthenticatedUser user, - ModeledConnection connection) { - init(user, null, connection); + public ActiveConnectionRecord(SharedConnectionMap connectionMap, + RemoteAuthenticatedUser user, ModeledConnection connection) { + this(connectionMap, user, null, connection); } /** - * Initializes this connection record, associating it with the given user, + * Creates a new ActiveConnectionRecord 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. * + * @param connectionMap + * The SharedConnectionMap instance tracking all active shared + * connections. + * * @param user * The user that connected to the connection associated with this * connection record. @@ -204,10 +263,11 @@ public class ActiveConnectionRecord implements ConnectionRecord { * connection, or null if no sharing profile should be used (access to * the connection is unrestricted). */ - public void init(RemoteAuthenticatedUser user, + public ActiveConnectionRecord(SharedConnectionMap connectionMap, + RemoteAuthenticatedUser user, ActiveConnectionRecord activeConnection, ModeledSharingProfile sharingProfile) { - init(user, null, activeConnection.getConnection(), sharingProfile); + this(connectionMap, user, null, activeConnection.getConnection(), sharingProfile); this.connectionID = activeConnection.getConnectionID(); } @@ -286,63 +346,6 @@ public class ActiveConnectionRecord implements ConnectionRecord { return sharingProfile == null; } - @Override - public String getConnectionIdentifier() { - return connection.getIdentifier(); - } - - @Override - public String getConnectionName() { - return connection.getName(); - } - - @Override - public String getSharingProfileIdentifier() { - - // Return sharing profile identifier if known - if (sharingProfile != null) - return sharingProfile.getIdentifier(); - - // No associated sharing profile - return null; - - } - - @Override - public String getSharingProfileName() { - - // Return sharing profile name if known - if (sharingProfile != null) - return sharingProfile.getName(); - - // No associated sharing profile - return null; - - } - - @Override - public Date getStartDate() { - return startDate; - } - - @Override - public Date getEndDate() { - - // Active connections have not yet ended - return null; - - } - - @Override - public String getRemoteHost() { - return user.getRemoteHost(); - } - - @Override - public String getUsername() { - return user.getIdentifier(); - } - @Override public boolean isActive() { return tunnel != null && tunnel.isOpen(); @@ -387,7 +390,7 @@ public class ActiveConnectionRecord implements ConnectionRecord { @Override public UUID getUUID() { - return uuid; + return ActiveConnectionRecord.this.getUUID(); } }; @@ -401,11 +404,6 @@ public class ActiveConnectionRecord implements ConnectionRecord { } - @Override - public UUID getUUID() { - return uuid; - } - /** * Returns the connection ID of the in-progress connection as determined by * guacd, not to be confused with the connection identifier determined by diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/apache/guacamole/auth/jdbc/connection/ConnectionRecordMapper.xml b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/apache/guacamole/auth/jdbc/connection/ConnectionRecordMapper.xml index 6b1f2709f..39e302b29 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/apache/guacamole/auth/jdbc/connection/ConnectionRecordMapper.xml +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/apache/guacamole/auth/jdbc/connection/ConnectionRecordMapper.xml @@ -61,7 +61,8 @@ - + INSERT INTO guacamole_connection_history ( connection_id, @@ -92,6 +93,15 @@ + + + + UPDATE guacamole_connection_history + SET end_date = #{record.endDate,jdbcType=TIMESTAMP} + WHERE history_id = #{record.recordID,jdbcType=INTEGER} + + + - + INSERT INTO guacamole_connection_history ( connection_id, @@ -92,6 +93,15 @@ + + + + UPDATE guacamole_connection_history + SET end_date = #{record.endDate,jdbcType=TIMESTAMP} + WHERE history_id = #{record.recordID,jdbcType=INTEGER}::integer + + + + + + + UPDATE [guacamole_connection_history] + SET end_date = #{record.endDate,jdbcType=TIMESTAMP} + WHERE history_id = #{record.recordID,jdbcType=INTEGER} + + + - + INSERT INTO [guacamole_connection_history] ( connection_id,