You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2022/05/27 21:54:54 UTC

[GitHub] [guacamole-client] jmuehlner opened a new pull request, #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

jmuehlner opened a new pull request, #730:
URL: https://github.com/apache/guacamole-client/pull/730

   As discussed in https://issues.apache.org/jira/browse/GUACAMOLE-1616, this allows JDBC extensions to track connection history for connections that are sourced from outside the extension. 
   
   The properties `mysql-track-external-connection-history`, `postgres-track-external-connection-history`, and `sqlserver-track-external-connection-history` control this behavior. 
   
   Each property defaults to `true` unless otherwise specified. since the only reason you'd probably ever want to disable this feature is if you have multiple JDBC auth providers installed at the same time, which is not going to be very common.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] mike-jumper commented on pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#issuecomment-1142727680

   LGTM. What say you, @necouchman?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r884018930


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingConnectionDirectory.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingDirectory;
+import org.apache.guacamole.net.auth.Directory;
+import org.apache.guacamole.net.auth.User;
+
+/**
+ * A connection directory that returns HistoryTrackingConnection-wrapped connections
+ * when queried.
+ */
+public class HistoryTrackingConnectionDirectory extends DelegatingDirectory<Connection> {

Review Comment:
   Done.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r884025041


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingUserContext.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingUserContext;
+import org.apache.guacamole.net.auth.Directory;
+import org.apache.guacamole.net.auth.UserContext;
+
+/**
+ * DelegatingUserContext implementation which writes connection history records
+ * when connections are established and closed.
+ */
+public class HistoryTrackingUserContext extends DelegatingUserContext {
+
+    /**
+     * The remote host that the user associated with the user context
+     * connected from.
+     */
+    private final String remoteHost;
+
+    /**
+     * The connection record mapper to use when writing history entries for
+     * established connections.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * Creates a new HistoryTrackingUserContext which wraps the given
+     * UserContext, allowing for tracking of connection history external to
+     * this authentication provider.
+     *
+     * @param userContext
+     *     The UserContext to wrap.
+     *
+     * @param string
+     *
+     * @param connectionRecordMapper
+     *     The mapper to use when writing connection history entries to the DB.
+     */
+    public HistoryTrackingUserContext(UserContext userContext, String remoteHost, ConnectionRecordMapper connectionRecordMapper) {
+        super(userContext);
+
+        this.remoteHost = remoteHost;
+        this.connectionRecordMapper = connectionRecordMapper;
+    }
+
+    @Override
+    public Directory<Connection> getConnectionDirectory() throws GuacamoleException {
+        return new HistoryTrackingConnectionDirectory(
+                super.getConnectionDirectory(), this.self(),

Review Comment:
   Ah, right right right ...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r886058754


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingConnection.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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;
+
+import java.util.Date;
+import java.util.Map;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordModel;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingConnection;
+import org.apache.guacamole.net.auth.User;
+import org.apache.guacamole.protocol.GuacamoleClientInformation;
+
+/**
+ * Connection implementation that creates a history record when the connection
+ * is established, and returns a HistoryTrackingTunnel to automatically set the
+ * end date when the connection is closed.
+ */
+public class HistoryTrackingConnection extends DelegatingConnection {
+
+    /**
+     * The current Guacamole user.
+     */
+    private final User currentUser;
+
+    /**
+     * The remote host that the user connected from.
+     */
+    private final String remoteHost;
+
+    /**
+     * The connection record mapper to use when writing history entries for
+     * established connections.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * Creates a new HistoryConnection that wraps the given connection,
+     * automatically creating a history record when the connection is
+     * established, and returning a HistoryTrackingTunnel to set the end
+     * date on the history entry when the connection is closed.
+     *
+     * @param currentUser
+     *     The current Guacamole user.
+     *
+     * @param remoteHost
+     *     The remote host that the user connected from.
+     *
+     * @param connection
+     *     The connection to wrap.
+     *
+     * @param connectionRecordMapper
+     *     The connection record mapper that will be used to write the connection history records.
+     */
+    public HistoryTrackingConnection(User currentUser, String remoteHost, Connection connection, ConnectionRecordMapper connectionRecordMapper) {
+        super(connection);
+
+        this.currentUser = currentUser;
+        this.remoteHost = remoteHost;
+        this.connectionRecordMapper = connectionRecordMapper;
+    }
+
+    @Override
+    public GuacamoleTunnel connect(GuacamoleClientInformation info,
+            Map<String, String> tokens) throws GuacamoleException {
+
+        // Connect to the tunnel before writing the history entry, in case it fails
+        GuacamoleTunnel tunnel = super.connect(info, tokens);
+
+        // Create a connection record model, starting at the current date/time
+        ConnectionRecordModel connectionRecordModel = new ConnectionRecordModel();
+        connectionRecordModel.setStartDate(new Date());
+
+        // Set the user information
+        connectionRecordModel.setUsername(this.currentUser.getIdentifier());
+        connectionRecordModel.setRemoteHost(this.remoteHost);
+
+        // Set the connection information
+        connectionRecordModel.setConnectionName(this.getDelegateConnection().getName());
+
+        // Insert the connection history record to mark the start of this connection
+        connectionRecordMapper.insert(connectionRecordModel);

Review Comment:
   Oops yeah, lemme do that.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r884945931


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingTunnel.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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;
+
+import java.util.Date;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordModel;
+import org.apache.guacamole.net.DelegatingGuacamoleTunnel;
+import org.apache.guacamole.net.GuacamoleTunnel;
+
+/**
+ * Tunnel implementation which automatically writes an end date for the
+ * provided connection history record model using the provided connection
+ * history mapper, when the tunnel is closed.
+ */
+public class HistoryTrackingTunnel extends DelegatingGuacamoleTunnel {
+
+    /**
+     * The connection for which this tunnel was established.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * The user for which this tunnel was established.
+     */
+    private final ConnectionRecordModel connectionRecordModel;
+
+    /**
+     * Creates a new HistoryTrackingTunnel that wraps the given tunnel,
+     * automatically setting the end date for the provided connection history records,
+     * using the provided connection history record mapper.
+     */

Review Comment:
   Oops, added!



##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingUserContext.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingUserContext;
+import org.apache.guacamole.net.auth.Directory;
+import org.apache.guacamole.net.auth.UserContext;
+
+/**
+ * DelegatingUserContext implementation which writes connection history records
+ * when connections are established and closed.
+ */
+public class HistoryTrackingUserContext extends DelegatingUserContext {
+
+    /**
+     * The remote host that the user associated with the user context
+     * connected from.
+     */
+    private final String remoteHost;
+
+    /**
+     * The connection record mapper to use when writing history entries for
+     * established connections.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * Creates a new HistoryTrackingUserContext which wraps the given
+     * UserContext, allowing for tracking of connection history external to
+     * this authentication provider.
+     *
+     * @param userContext
+     *     The UserContext to wrap.
+     *
+     * @param string

Review Comment:
   Yep, my bad. Fixed now.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r884017622


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingUserContext.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingUserContext;
+import org.apache.guacamole.net.auth.Directory;
+import org.apache.guacamole.net.auth.UserContext;
+
+/**
+ * DelegatingUserContext implementation which writes connection history records
+ * when connections are established and closed.
+ */
+public class HistoryTrackingUserContext extends DelegatingUserContext {
+
+    /**
+     * The remote host that the user associated with the user context
+     * connected from.
+     */
+    private final String remoteHost;
+
+    /**
+     * The connection record mapper to use when writing history entries for
+     * established connections.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * Creates a new HistoryTrackingUserContext which wraps the given
+     * UserContext, allowing for tracking of connection history external to
+     * this authentication provider.
+     *
+     * @param userContext
+     *     The UserContext to wrap.
+     *
+     * @param string
+     *
+     * @param connectionRecordMapper
+     *     The mapper to use when writing connection history entries to the DB.
+     */
+    public HistoryTrackingUserContext(UserContext userContext, String remoteHost, ConnectionRecordMapper connectionRecordMapper) {
+        super(userContext);
+
+        this.remoteHost = remoteHost;
+        this.connectionRecordMapper = connectionRecordMapper;
+    }
+
+    @Override
+    public Directory<Connection> getConnectionDirectory() throws GuacamoleException {
+        return new HistoryTrackingConnectionDirectory(
+                super.getConnectionDirectory(), this.self(),

Review Comment:
   Well, I mean, I still need `super` or my stack will overflow.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r884008284


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingUserContext.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingUserContext;
+import org.apache.guacamole.net.auth.Directory;
+import org.apache.guacamole.net.auth.UserContext;
+
+/**
+ * DelegatingUserContext implementation which writes connection history records
+ * when connections are established and closed.
+ */
+public class HistoryTrackingUserContext extends DelegatingUserContext {
+
+    /**
+     * The remote host that the user associated with the user context
+     * connected from.
+     */
+    private final String remoteHost;
+
+    /**
+     * The connection record mapper to use when writing history entries for
+     * established connections.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * Creates a new HistoryTrackingUserContext which wraps the given
+     * UserContext, allowing for tracking of connection history external to
+     * this authentication provider.
+     *
+     * @param userContext
+     *     The UserContext to wrap.
+     *
+     * @param string
+     *
+     * @param connectionRecordMapper
+     *     The mapper to use when writing connection history entries to the DB.
+     */
+    public HistoryTrackingUserContext(UserContext userContext, String remoteHost, ConnectionRecordMapper connectionRecordMapper) {
+        super(userContext);
+
+        this.remoteHost = remoteHost;
+        this.connectionRecordMapper = connectionRecordMapper;
+    }
+
+    @Override
+    public Directory<Connection> getConnectionDirectory() throws GuacamoleException {
+        return new HistoryTrackingConnectionDirectory(
+                super.getConnectionDirectory(), this.self(),

Review Comment:
   Why `super` for `getConnectionDirectory()` but `this` for `self()`? Why not just `getConnectionDirectory()` and `self()`?



##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingConnectionDirectory.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingDirectory;
+import org.apache.guacamole.net.auth.Directory;
+import org.apache.guacamole.net.auth.User;
+
+/**
+ * A connection directory that returns HistoryTrackingConnection-wrapped connections
+ * when queried.
+ */
+public class HistoryTrackingConnectionDirectory extends DelegatingDirectory<Connection> {

Review Comment:
   This should instead extend `DecoratingDirectory`.
   
   The `update()` function of `Directory` needs to receive the same object returned by `get()` of that `Directory` implementation. If a decorated `Directory` returns a different object from `get()`, it needs to unwrap that object and return the original within `update()`. The `DecoratingDirectory` convenience object does that for you.



##########
guacamole-ext/src/main/java/org/apache/guacamole/net/auth/UserContext.java:
##########
@@ -103,7 +103,7 @@ public interface UserContext {
      * connections and their configurations, but only as allowed by the
      * permissions given to the user.
      *
-     * @return A Directory whose operations are bound by the permissions of 
+     * @return A Directory whose operations are bound by the permissions of

Review Comment:
   Why is this file modified?



##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java:
##########
@@ -202,79 +202,92 @@ private PostgreSQLGuacamoleProperties() {}
         public String getName() { return "postgresql-default-max-group-connections-per-user"; }
 
     };
-    
+
     /**
      * The SSL mode that should be used by the JDBC driver when making
      * connections to the remote server.  By default SSL will be attempted but
      * plain-text will be allowed if SSL fails.
      */
     public static final EnumGuacamoleProperty<PostgreSQLSSLMode> POSTGRESQL_SSL_MODE =
             new EnumGuacamoleProperty<PostgreSQLSSLMode>(PostgreSQLSSLMode.class) {
-        
+
         @Override
         public String getName() { return "postgresql-ssl-mode"; }
-        
+
     };
-    
+
     /**
      * The client SSL certificate file used by the JDBC driver to make the
      * SSL connection.
      */
     public static final FileGuacamoleProperty POSTGRESQL_SSL_CERT_FILE =
             new FileGuacamoleProperty() {
-             
+
         @Override
         public String getName() { return "postgresql-ssl-cert-file"; }
-                
+
     };
-    
+
     /**
      * The client SSL private key file used by the JDBC driver to make the
      * SSL connection.
      */
     public static final FileGuacamoleProperty POSTGRESQL_SSL_KEY_FILE =
             new FileGuacamoleProperty() {
-    
+
         @Override
         public String getName() { return "postgresql-ssl-key-file"; }
-        
+
     };
-    
+
     /**
      * The client SSL root certificate file used by the JDBC driver to validate
      * certificates when making the SSL connection.
      */
     public static final FileGuacamoleProperty POSTGRESQL_SSL_ROOT_CERT_FILE =
             new FileGuacamoleProperty() {
-        
+
         @Override
         public String getName() { return "postgresql-ssl-root-cert-file"; }
-        
+
     };
-    
+
     /**
      * The password of the SSL private key used by the JDBC driver to make
      * the SSL connection to the PostgreSQL server.
      */
     public static final StringGuacamoleProperty POSTGRESQL_SSL_KEY_PASSWORD =
             new StringGuacamoleProperty() {
-        
+
         @Override
         public String getName() { return "postgresql-ssl-key-password"; }
-        
+
     };
-    
+
     /**
      * Whether or not to automatically create accounts in the PostgreSQL
      * database for users who successfully authenticate through another
      * extension. By default users will not be automatically created.
      */
     public static final BooleanGuacamoleProperty POSTGRESQL_AUTO_CREATE_ACCOUNTS =
             new BooleanGuacamoleProperty() {
-                
+
         @Override
         public String getName() { return "postgresql-auto-create-accounts"; }
-                
+
     };
-    
+
+    /**
+     * Whether or not to track connection history for connections that do not originate
+     * from within the Postgres database. By default, external connection history will be
+     * tracked.
+     */
+    public static final BooleanGuacamoleProperty POSTGRESQL_TRACK_EXTERNAL_CONNECTION_HISTORY =
+            new BooleanGuacamoleProperty() {
+
+        @Override
+        public String getName() { return "postgres-track-external-connection-history"; }

Review Comment:
   This should be `postgresql-*` for consistency.



##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ModeledConnection.java:
##########
@@ -505,4 +506,13 @@ public boolean isFailoverOnly() {
         return getModel().isFailoverOnly();
     }
 
+    /**
+     * Returns the JDBC environment associated with this connection.
+     *
+     * @return the JDBC environment associated with this connection.
+     */
+    public JDBCEnvironment gEnvironment() {
+        return this.environment;
+    }

Review Comment:
   Is this called anywhere? If not, this should be removed. If it is, then:
   
   1. Please format the JavaDoc to match the formatting used elsewhere (new line after `@return`, sentence case for the info that follows, indented).
   2. This should be `getEnvironment()`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r886023557


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingConnection.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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;
+
+import java.util.Date;
+import java.util.Map;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordModel;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingConnection;
+import org.apache.guacamole.net.auth.User;
+import org.apache.guacamole.protocol.GuacamoleClientInformation;
+
+/**
+ * Connection implementation that creates a history record when the connection
+ * is established, and returns a HistoryTrackingTunnel to automatically set the
+ * end date when the connection is closed.
+ */
+public class HistoryTrackingConnection extends DelegatingConnection {
+
+    /**
+     * The current Guacamole user.
+     */
+    private final User currentUser;
+
+    /**
+     * The remote host that the user connected from.
+     */
+    private final String remoteHost;
+
+    /**
+     * The connection record mapper to use when writing history entries for
+     * established connections.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * Creates a new HistoryConnection that wraps the given connection,
+     * automatically creating a history record when the connection is
+     * established, and returning a HistoryTrackingTunnel to set the end
+     * date on the history entry when the connection is closed.
+     *
+     * @param currentUser
+     *     The current Guacamole user.
+     *
+     * @param remoteHost
+     *     The remote host that the user connected from.
+     *
+     * @param connection
+     *     The connection to wrap.
+     *
+     * @param connectionRecordMapper
+     *     The connection record mapper that will be used to write the connection history records.
+     */
+    public HistoryTrackingConnection(User currentUser, String remoteHost, Connection connection, ConnectionRecordMapper connectionRecordMapper) {
+        super(connection);
+
+        this.currentUser = currentUser;
+        this.remoteHost = remoteHost;
+        this.connectionRecordMapper = connectionRecordMapper;
+    }
+
+    @Override
+    public GuacamoleTunnel connect(GuacamoleClientInformation info,
+            Map<String, String> tokens) throws GuacamoleException {
+
+        // Connect to the tunnel before writing the history entry, in case it fails
+        GuacamoleTunnel tunnel = super.connect(info, tokens);
+
+        // Create a connection record model, starting at the current date/time
+        ConnectionRecordModel connectionRecordModel = new ConnectionRecordModel();
+        connectionRecordModel.setStartDate(new Date());
+
+        // Set the user information
+        connectionRecordModel.setUsername(this.currentUser.getIdentifier());
+        connectionRecordModel.setRemoteHost(this.remoteHost);
+
+        // Set the connection information
+        connectionRecordModel.setConnectionName(this.getDelegateConnection().getName());
+
+        // Insert the connection history record to mark the start of this connection
+        connectionRecordMapper.insert(connectionRecordModel);

Review Comment:
   Looks good, but I think you'll still need to manually inject `HISTORY_UUID` here via `tokens`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r884013364


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingUserContext.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingUserContext;
+import org.apache.guacamole.net.auth.Directory;
+import org.apache.guacamole.net.auth.UserContext;
+
+/**
+ * DelegatingUserContext implementation which writes connection history records
+ * when connections are established and closed.
+ */
+public class HistoryTrackingUserContext extends DelegatingUserContext {
+
+    /**
+     * The remote host that the user associated with the user context
+     * connected from.
+     */
+    private final String remoteHost;
+
+    /**
+     * The connection record mapper to use when writing history entries for
+     * established connections.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * Creates a new HistoryTrackingUserContext which wraps the given
+     * UserContext, allowing for tracking of connection history external to
+     * this authentication provider.
+     *
+     * @param userContext
+     *     The UserContext to wrap.
+     *
+     * @param string
+     *
+     * @param connectionRecordMapper
+     *     The mapper to use when writing connection history entries to the DB.
+     */
+    public HistoryTrackingUserContext(UserContext userContext, String remoteHost, ConnectionRecordMapper connectionRecordMapper) {
+        super(userContext);
+
+        this.remoteHost = remoteHost;
+        this.connectionRecordMapper = connectionRecordMapper;
+    }
+
+    @Override
+    public Directory<Connection> getConnectionDirectory() throws GuacamoleException {
+        return new HistoryTrackingConnectionDirectory(
+                super.getConnectionDirectory(), this.self(),

Review Comment:
   Sure



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r885916450


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingConnection.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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;
+
+import java.util.Date;
+import java.util.Map;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordModel;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingConnection;
+import org.apache.guacamole.net.auth.User;
+import org.apache.guacamole.protocol.GuacamoleClientInformation;
+
+/**
+ * Connection implementation that creates a history record when the connection
+ * is established, and returns a HistoryTrackingTunnel to automatically set the
+ * end date when the connection is closed.
+ */
+public class HistoryTrackingConnection extends DelegatingConnection {
+
+    /**
+     * The current Guacamole user.
+     */
+    private final User currentUser;
+
+    /**
+     * The remote host that the user connected from.
+     */
+    private final String remoteHost;
+
+    /**
+     * The connection record mapper to use when writing history entries for
+     * established connections.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * Creates a new HistoryConnection that wraps the given connection,
+     * automatically creating a history record when the connection is
+     * established, and returning a HistoryTrackingTunnel to set the end
+     * date on the history entry when the connection is closed.
+     *
+     * @param currentUser
+     *     The current Guacamole user.
+     *
+     * @param remoteHost
+     *     The remote host that the user connected from.
+     *
+     * @param connection
+     *     The connection to wrap.
+     *
+     * @param connectionRecordMapper
+     *     The connection record mapper that will be used to write the connection history records.
+     */
+    public HistoryTrackingConnection(User currentUser, String remoteHost, Connection connection, ConnectionRecordMapper connectionRecordMapper) {
+        super(connection);
+
+        this.currentUser = currentUser;
+        this.remoteHost = remoteHost;
+        this.connectionRecordMapper = connectionRecordMapper;
+    }
+
+    @Override
+    public GuacamoleTunnel connect(GuacamoleClientInformation info,
+            Map<String, String> tokens) throws GuacamoleException {
+
+        // Connect to the tunnel before writing the history entry, in case it fails
+        GuacamoleTunnel tunnel = super.connect(info, tokens);
+
+        // Create a connection record model, starting at the current date/time
+        ConnectionRecordModel connectionRecordModel = new ConnectionRecordModel();
+        connectionRecordModel.setStartDate(new Date());
+
+        // Set the user information
+        connectionRecordModel.setUsername(this.currentUser.getIdentifier());
+        connectionRecordModel.setRemoteHost(this.remoteHost);
+
+        // Set the connection information
+        connectionRecordModel.setConnectionName(this.getDelegateConnection().getName());
+
+        // Insert the connection history record to mark the start of this connection
+        connectionRecordMapper.insert(connectionRecordModel);

Review Comment:
   Good catch - fixed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] necouchman commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
necouchman commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r884317457


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingUserContext.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingUserContext;
+import org.apache.guacamole.net.auth.Directory;
+import org.apache.guacamole.net.auth.UserContext;
+
+/**
+ * DelegatingUserContext implementation which writes connection history records
+ * when connections are established and closed.
+ */
+public class HistoryTrackingUserContext extends DelegatingUserContext {
+
+    /**
+     * The remote host that the user associated with the user context
+     * connected from.
+     */
+    private final String remoteHost;
+
+    /**
+     * The connection record mapper to use when writing history entries for
+     * established connections.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * Creates a new HistoryTrackingUserContext which wraps the given
+     * UserContext, allowing for tracking of connection history external to
+     * this authentication provider.
+     *
+     * @param userContext
+     *     The UserContext to wrap.
+     *
+     * @param string

Review Comment:
   Looks like this identifies the type of parameter instead of the name, and is missing a description.



##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingTunnel.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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;
+
+import java.util.Date;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordModel;
+import org.apache.guacamole.net.DelegatingGuacamoleTunnel;
+import org.apache.guacamole.net.GuacamoleTunnel;
+
+/**
+ * Tunnel implementation which automatically writes an end date for the
+ * provided connection history record model using the provided connection
+ * history mapper, when the tunnel is closed.
+ */
+public class HistoryTrackingTunnel extends DelegatingGuacamoleTunnel {
+
+    /**
+     * The connection for which this tunnel was established.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * The user for which this tunnel was established.
+     */
+    private final ConnectionRecordModel connectionRecordModel;
+
+    /**
+     * Creates a new HistoryTrackingTunnel that wraps the given tunnel,
+     * automatically setting the end date for the provided connection history records,
+     * using the provided connection history record mapper.
+     */

Review Comment:
   Parameter documentation?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r886149448


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingConnection.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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;
+
+import java.util.Date;
+import java.util.Map;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordModel;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingConnection;
+import org.apache.guacamole.net.auth.User;
+import org.apache.guacamole.protocol.GuacamoleClientInformation;
+
+/**
+ * Connection implementation that creates a history record when the connection
+ * is established, and returns a HistoryTrackingTunnel to automatically set the
+ * end date when the connection is closed.
+ */
+public class HistoryTrackingConnection extends DelegatingConnection {
+
+    /**
+     * The current Guacamole user.
+     */
+    private final User currentUser;
+
+    /**
+     * The remote host that the user connected from.
+     */
+    private final String remoteHost;
+
+    /**
+     * The connection record mapper to use when writing history entries for
+     * established connections.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * Creates a new HistoryConnection that wraps the given connection,
+     * automatically creating a history record when the connection is
+     * established, and returning a HistoryTrackingTunnel to set the end
+     * date on the history entry when the connection is closed.
+     *
+     * @param currentUser
+     *     The current Guacamole user.
+     *
+     * @param remoteHost
+     *     The remote host that the user connected from.
+     *
+     * @param connection
+     *     The connection to wrap.
+     *
+     * @param connectionRecordMapper
+     *     The connection record mapper that will be used to write the connection history records.
+     */
+    public HistoryTrackingConnection(User currentUser, String remoteHost, Connection connection, ConnectionRecordMapper connectionRecordMapper) {
+        super(connection);
+
+        this.currentUser = currentUser;
+        this.remoteHost = remoteHost;
+        this.connectionRecordMapper = connectionRecordMapper;
+    }
+
+    @Override
+    public GuacamoleTunnel connect(GuacamoleClientInformation info,
+            Map<String, String> tokens) throws GuacamoleException {
+
+        // Connect to the tunnel before writing the history entry, in case it fails
+        GuacamoleTunnel tunnel = super.connect(info, tokens);
+
+        // Create a connection record model, starting at the current date/time
+        ConnectionRecordModel connectionRecordModel = new ConnectionRecordModel();
+        connectionRecordModel.setStartDate(new Date());
+
+        // Set the user information
+        connectionRecordModel.setUsername(this.currentUser.getIdentifier());
+        connectionRecordModel.setRemoteHost(this.remoteHost);
+
+        // Set the connection information
+        connectionRecordModel.setConnectionName(this.getDelegateConnection().getName());
+
+        // Insert the connection history record to mark the start of this connection
+        connectionRecordMapper.insert(connectionRecordModel);

Review Comment:
   Ok, it's done.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] mike-jumper merged pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
mike-jumper merged PR #730:
URL: https://github.com/apache/guacamole-client/pull/730


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r885815744


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingConnection.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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;
+
+import java.util.Date;
+import java.util.Map;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordModel;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingConnection;
+import org.apache.guacamole.net.auth.User;
+import org.apache.guacamole.protocol.GuacamoleClientInformation;
+
+/**
+ * Connection implementation that creates a history record when the connection
+ * is established, and returns a HistoryTrackingTunnel to automatically set the
+ * end date when the connection is closed.
+ */
+public class HistoryTrackingConnection extends DelegatingConnection {
+
+    /**
+     * The current Guacamole user.
+     */
+    private final User currentUser;
+
+    /**
+     * The remote host that the user connected from.
+     */
+    private final String remoteHost;
+
+    /**
+     * The connection record mapper to use when writing history entries for
+     * established connections.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * Creates a new HistoryConnection that wraps the given connection,
+     * automatically creating a history record when the connection is
+     * established, and returning a HistoryTrackingTunnel to set the end
+     * date on the history entry when the connection is closed.
+     *
+     * @param currentUser
+     *     The current Guacamole user.
+     *
+     * @param remoteHost
+     *     The remote host that the user connected from.
+     *
+     * @param connection
+     *     The connection to wrap.
+     *
+     * @param connectionRecordMapper
+     *     The connection record mapper that will be used to write the connection history records.
+     */
+    public HistoryTrackingConnection(User currentUser, String remoteHost, Connection connection, ConnectionRecordMapper connectionRecordMapper) {
+        super(connection);
+
+        this.currentUser = currentUser;
+        this.remoteHost = remoteHost;
+        this.connectionRecordMapper = connectionRecordMapper;
+    }
+
+    @Override
+    public GuacamoleTunnel connect(GuacamoleClientInformation info,
+            Map<String, String> tokens) throws GuacamoleException {
+
+        // Connect to the tunnel before writing the history entry, in case it fails
+        GuacamoleTunnel tunnel = super.connect(info, tokens);
+
+        // Create a connection record model, starting at the current date/time
+        ConnectionRecordModel connectionRecordModel = new ConnectionRecordModel();
+        connectionRecordModel.setStartDate(new Date());
+
+        // Set the user information
+        connectionRecordModel.setUsername(this.currentUser.getIdentifier());
+        connectionRecordModel.setRemoteHost(this.remoteHost);
+
+        // Set the connection information
+        connectionRecordModel.setConnectionName(this.getDelegateConnection().getName());
+
+        // Insert the connection history record to mark the start of this connection
+        connectionRecordMapper.insert(connectionRecordModel);
+
+        return new HistoryTrackingTunnel(
+                tunnel, this.connectionRecordMapper, connectionRecordModel);
+    }
+
+    /**
+     * Get the Connection wrapped by this HistoryTrackingConnection.
+     *
+     * @return The wrapped Connection.

Review Comment:
   Please format per convention (newline following `@return`, indent relevant documentation block).



##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingConnection.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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;
+
+import java.util.Date;
+import java.util.Map;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordModel;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingConnection;
+import org.apache.guacamole.net.auth.User;
+import org.apache.guacamole.protocol.GuacamoleClientInformation;
+
+/**
+ * Connection implementation that creates a history record when the connection
+ * is established, and returns a HistoryTrackingTunnel to automatically set the
+ * end date when the connection is closed.
+ */
+public class HistoryTrackingConnection extends DelegatingConnection {
+
+    /**
+     * The current Guacamole user.
+     */
+    private final User currentUser;
+
+    /**
+     * The remote host that the user connected from.
+     */
+    private final String remoteHost;
+
+    /**
+     * The connection record mapper to use when writing history entries for
+     * established connections.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * Creates a new HistoryConnection that wraps the given connection,
+     * automatically creating a history record when the connection is
+     * established, and returning a HistoryTrackingTunnel to set the end
+     * date on the history entry when the connection is closed.
+     *
+     * @param currentUser
+     *     The current Guacamole user.
+     *
+     * @param remoteHost
+     *     The remote host that the user connected from.
+     *
+     * @param connection
+     *     The connection to wrap.
+     *
+     * @param connectionRecordMapper
+     *     The connection record mapper that will be used to write the connection history records.
+     */
+    public HistoryTrackingConnection(User currentUser, String remoteHost, Connection connection, ConnectionRecordMapper connectionRecordMapper) {
+        super(connection);
+
+        this.currentUser = currentUser;
+        this.remoteHost = remoteHost;
+        this.connectionRecordMapper = connectionRecordMapper;
+    }
+
+    @Override
+    public GuacamoleTunnel connect(GuacamoleClientInformation info,
+            Map<String, String> tokens) throws GuacamoleException {
+
+        // Connect to the tunnel before writing the history entry, in case it fails
+        GuacamoleTunnel tunnel = super.connect(info, tokens);
+
+        // Create a connection record model, starting at the current date/time
+        ConnectionRecordModel connectionRecordModel = new ConnectionRecordModel();
+        connectionRecordModel.setStartDate(new Date());
+
+        // Set the user information
+        connectionRecordModel.setUsername(this.currentUser.getIdentifier());
+        connectionRecordModel.setRemoteHost(this.remoteHost);
+
+        // Set the connection information
+        connectionRecordModel.setConnectionName(this.getDelegateConnection().getName());
+
+        // Insert the connection history record to mark the start of this connection
+        connectionRecordMapper.insert(connectionRecordModel);

Review Comment:
   This should happen before `super.connect()` such that `HISTORY_UUID` can be injected as a parameter token:
   
   https://github.com/apache/guacamole-client/blob/b256250720386198273122af551d4d17ce850276/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java#L463-L465
   
   If the above is done, then history records for connections from other extensions may be automatically associated with session recordings. If not, the only history records for connections from the JDBC extension will be able to do this.
   
   The UUID is generated from the primary key ID of the inserted history record:
   
   https://github.com/apache/guacamole-client/blob/b256250720386198273122af551d4d17ce850276/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/ModeledActivityRecord.java#L114-L128
   
   https://github.com/apache/guacamole-client/blob/b256250720386198273122af551d4d17ce850276/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ModeledConnectionRecord.java#L46
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r885916619


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/HistoryTrackingConnection.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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;
+
+import java.util.Date;
+import java.util.Map;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordMapper;
+import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordModel;
+import org.apache.guacamole.net.GuacamoleTunnel;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.DelegatingConnection;
+import org.apache.guacamole.net.auth.User;
+import org.apache.guacamole.protocol.GuacamoleClientInformation;
+
+/**
+ * Connection implementation that creates a history record when the connection
+ * is established, and returns a HistoryTrackingTunnel to automatically set the
+ * end date when the connection is closed.
+ */
+public class HistoryTrackingConnection extends DelegatingConnection {
+
+    /**
+     * The current Guacamole user.
+     */
+    private final User currentUser;
+
+    /**
+     * The remote host that the user connected from.
+     */
+    private final String remoteHost;
+
+    /**
+     * The connection record mapper to use when writing history entries for
+     * established connections.
+     */
+    private final ConnectionRecordMapper connectionRecordMapper;
+
+    /**
+     * Creates a new HistoryConnection that wraps the given connection,
+     * automatically creating a history record when the connection is
+     * established, and returning a HistoryTrackingTunnel to set the end
+     * date on the history entry when the connection is closed.
+     *
+     * @param currentUser
+     *     The current Guacamole user.
+     *
+     * @param remoteHost
+     *     The remote host that the user connected from.
+     *
+     * @param connection
+     *     The connection to wrap.
+     *
+     * @param connectionRecordMapper
+     *     The connection record mapper that will be used to write the connection history records.
+     */
+    public HistoryTrackingConnection(User currentUser, String remoteHost, Connection connection, ConnectionRecordMapper connectionRecordMapper) {
+        super(connection);
+
+        this.currentUser = currentUser;
+        this.remoteHost = remoteHost;
+        this.connectionRecordMapper = connectionRecordMapper;
+    }
+
+    @Override
+    public GuacamoleTunnel connect(GuacamoleClientInformation info,
+            Map<String, String> tokens) throws GuacamoleException {
+
+        // Connect to the tunnel before writing the history entry, in case it fails
+        GuacamoleTunnel tunnel = super.connect(info, tokens);
+
+        // Create a connection record model, starting at the current date/time
+        ConnectionRecordModel connectionRecordModel = new ConnectionRecordModel();
+        connectionRecordModel.setStartDate(new Date());
+
+        // Set the user information
+        connectionRecordModel.setUsername(this.currentUser.getIdentifier());
+        connectionRecordModel.setRemoteHost(this.remoteHost);
+
+        // Set the connection information
+        connectionRecordModel.setConnectionName(this.getDelegateConnection().getName());
+
+        // Insert the connection history record to mark the start of this connection
+        connectionRecordMapper.insert(connectionRecordModel);
+
+        return new HistoryTrackingTunnel(
+                tunnel, this.connectionRecordMapper, connectionRecordModel);
+    }
+
+    /**
+     * Get the Connection wrapped by this HistoryTrackingConnection.
+     *
+     * @return The wrapped Connection.

Review Comment:
   Fixed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r884011162


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ModeledConnection.java:
##########
@@ -505,4 +506,13 @@ public boolean isFailoverOnly() {
         return getModel().isFailoverOnly();
     }
 
+    /**
+     * Returns the JDBC environment associated with this connection.
+     *
+     * @return the JDBC environment associated with this connection.
+     */
+    public JDBCEnvironment gEnvironment() {
+        return this.environment;
+    }

Review Comment:
   Oh, oops. Did not mean to include this. I'll remove.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #730: GUACAMOLE-1616: Track external connection history in JDBC extensions

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #730:
URL: https://github.com/apache/guacamole-client/pull/730#discussion_r884011450


##########
guacamole-ext/src/main/java/org/apache/guacamole/net/auth/UserContext.java:
##########
@@ -103,7 +103,7 @@ public interface UserContext {
      * connections and their configurations, but only as allowed by the
      * permissions given to the user.
      *
-     * @return A Directory whose operations are bound by the permissions of 
+     * @return A Directory whose operations are bound by the permissions of

Review Comment:
   It shouldn't be! Un-modified.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org