mike-jumper commented on a change in pull request #698:
URL: https://github.com/apache/guacamole-client/pull/698#discussion_r806084940
##########
File path:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ConnectionRecordMapper.java
##########
@@ -19,113 +19,9 @@
package org.apache.guacamole.auth.jdbc.connection;
-import java.util.Collection;
-import java.util.List;
-import org.apache.guacamole.auth.jdbc.base.ActivityRecordSearchTerm;
-import org.apache.guacamole.auth.jdbc.base.ActivityRecordSortPredicate;
-import org.apache.ibatis.annotations.Param;
-import org.apache.guacamole.auth.jdbc.user.UserModel;
+import org.apache.guacamole.auth.jdbc.base.ActivityRecordMapper;
/**
* Mapper for connection record objects.
*/
-public interface ConnectionRecordMapper {
-
- /**
- * Returns a collection of all connection records associated with the
- * connection having the given identifier.
- *
- * @param identifier
- * The identifier of the connection whose records are to be retrieved.
- *
- * @return
- * A collection of all connection records associated with the
- * connection having the given identifier. This collection will be
- * empty if no such connection exists.
- */
- List<ConnectionRecordModel> select(@Param("identifier") String identifier);
-
- /**
- * Inserts the given connection record.
- *
- * @param record
- * The connection record to insert.
- *
- * @return
- * The number of rows inserted.
- */
- int insert(@Param("record") ConnectionRecordModel record);
-
- /**
- * Searches for up to <code>limit</code> connection records that contain
- * the given terms, sorted by the given predicates, regardless of whether
- * the data they are associated with is is readable by any particular user.
- * This should only be called on behalf of a system administrator. If
- * records are needed by a non-administrative user who must have explicit
- * read rights, use {@link searchReadable()} instead.
- *
- * @param identifier
- * The optional connection identifier to which records should be
limited,
- * or null if all records should be retrieved.
- *
- * @param terms
- * The search terms that must match the returned records.
- *
- * @param sortPredicates
- * A list of predicates to sort the returned records by, in order of
- * priority.
- *
- * @param limit
- * The maximum number of records that should be returned.
- *
- * @return
- * The results of the search performed with the given parameters.
- */
- List<ConnectionRecordModel> search(@Param("identifier") String identifier,
- @Param("terms") Collection<ActivityRecordSearchTerm> terms,
- @Param("sortPredicates") List<ActivityRecordSortPredicate>
sortPredicates,
- @Param("limit") int limit);
-
- /**
- * Searches for up to <code>limit</code> connection records that contain
- * the given terms, sorted by the given predicates. Only records that are
- * associated with data explicitly readable by the given user will be
- * returned. If records are needed by a system administrator (who, by
- * definition, does not need explicit read rights), use {@link search()}
- * instead.
- *
- * @param identifier
- * The optional connection identifier for which records should be
- * retrieved, or null if all readable records should be retrieved.
- *
- * @param user
- * The user whose permissions should determine whether a record is
- * returned.
- *
- * @param terms
- * The search terms that must match the returned records.
- *
- * @param sortPredicates
- * A list of predicates to sort the returned records by, in order of
- * priority.
- *
- * @param limit
- * The maximum number of records that should be returned.
- *
- * @param effectiveGroups
- * The identifiers of all groups that should be taken into account
- * when determining the permissions effectively granted to the user. If
- * no groups are given, only permissions directly granted to the user
- * will be used.
- *
- * @return
- * The results of the search performed with the given parameters.
- */
- List<ConnectionRecordModel> searchReadable(@Param("identifier") String
identifier,
- @Param("user") UserModel user,
- @Param("terms") Collection<ActivityRecordSearchTerm> terms,
- @Param("sortPredicates") List<ActivityRecordSortPredicate>
sortPredicates,
- @Param("limit") int limit,
- @Param("effectiveGroups") Collection<String> effectiveGroups);
-
-}
+public interface ConnectionRecordMapper extends
ActivityRecordMapper<ConnectionRecordModel> {}
Review comment:
There would be no downstream impact as it's not a part of the public API
(these mappers are internal pieces of the database auth implementation). I
don't think we can remove it, though, as MyBatis needs to be able to determine
with XML mapping file corresponds to which mapper.
##########
File path:
extensions/guacamole-history-recording-storage/src/main/java/org/apache/guacamole/history/connection/HistoryConnectionRecord.java
##########
@@ -0,0 +1,309 @@
+/*
+ * 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.history.connection;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.net.MalformedURLException;
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.environment.Environment;
+import org.apache.guacamole.environment.LocalEnvironment;
+import org.apache.guacamole.io.GuacamoleReader;
+import org.apache.guacamole.io.ReaderGuacamoleReader;
+import org.apache.guacamole.language.TranslatableMessage;
+import org.apache.guacamole.net.auth.ActivityLog;
+import org.apache.guacamole.net.auth.ConnectionRecord;
+import org.apache.guacamole.net.auth.DelegatingConnectionRecord;
+import org.apache.guacamole.net.auth.FileActivityLog;
+import org.apache.guacamole.properties.FileGuacamoleProperty;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * ConnectionRecord implementation that automatically defines ActivityLogs for
+ * files that relate to the wrapped record.
+ */
+public class HistoryConnectionRecord extends DelegatingConnectionRecord {
+
+ /**
+ * Logger for this class.
+ */
+ private static final Logger logger =
LoggerFactory.getLogger(HistoryConnectionRecord.class);
+
+ /**
+ * The namespace for URL UUIDs as defined by RFC 4122.
+ */
+ private static final UUID UUID_NAMESPACE_URL =
UUID.fromString("6ba7b811-9dad-11d1-80b4-00c04fd430c8");
+
+ /**
+ * The filename suffix of typescript timing files.
+ */
+ private static final String TIMING_FILE_SUFFIX = ".timing";
+
+ /**
+ * The default directory to search for associated session recordings, if
+ * not overridden with the "recording-search-path" property.
+ */
+ private static final File DEFAULT_RECORDING_SEARCH_PATH = new
File("/var/lib/guacamole/recordings");
+
+ /**
+ * The directory to search for associated session recordings. By default,
+ * "/var/lib/guacamole/recordings" will be used.
+ */
+ private static final FileGuacamoleProperty RECORDING_SEARCH_PATH = new
FileGuacamoleProperty() {
+
+ @Override
+ public String getName() {
+ return "recording-search-path";
+ }
+
+ };
+
+ /**
+ * The recording file associated with the wrapped connection record. This
+ * may be a single file or a directory that may contain any number of
+ * relevant recordings.
+ */
+ private final File recording;
+
+ /**
+ * Creates a new HistoryConnectionRecord that wraps the given
+ * ConnectionRecord, automatically associating ActivityLogs based on
+ * related files (session recordings, typescripts, etc.).
+ *
+ * @param record
+ * The ConnectionRecord to wrap.
+ *
+ * @throws GuacamoleException
+ * If the configured path for stored recordings cannot be read.
+ */
+ public HistoryConnectionRecord(ConnectionRecord record) throws
GuacamoleException {
+ super(record);
+
+ Environment environment = LocalEnvironment.getInstance();
+ File recordingPath = environment.getProperty(RECORDING_SEARCH_PATH,
+ DEFAULT_RECORDING_SEARCH_PATH);
+
+ String uuid = record.getUUID().toString();
+ File recordingFile = new File(recordingPath, uuid);
+ this.recording = recordingFile.canRead() ? recordingFile : null;
+
+ }
+
+ /**
+ * Returns whether the given file appears to be a Guacamole session
+ * recording. As there is no standard extension for session recordings,
+ * this is determined by attempting to read a single Guacamole instruction
+ * from the file.
+ *
+ * @param file
+ * The file to test.
+ *
+ * @return
+ * true if the file appears to be a Guacamole session recording, false
+ * otherwise.
+ */
+ private boolean isSessionRecording(File file) {
+
+ try (Reader reader = new InputStreamReader(new FileInputStream(file),
StandardCharsets.UTF_8)) {
+
+ GuacamoleReader guacReader = new ReaderGuacamoleReader(reader);
+ if (guacReader.readInstruction() != null)
+ return true;
+
+ }
+ catch (GuacamoleException e) {
+ logger.debug("File \"{}\" does not appear to be a session "
+ + "recording, as it could not be parsed as Guacamole "
+ + "protocol data.", file, e);
+ }
+ catch (IOException e) {
+ logger.warn("Possible session recording \"{}\" could not be "
+ + "identified as it cannot be read: {}", file,
e.getMessage());
+ logger.debug("Possible session recording \"{}\" could not be
read.", file, e);
+ }
+
+ return false;
+
+ }
+
+ /**
+ * Returns whether the given file appears to be a typescript (text
+ * recording of a terminal session). As there is no standard extension for
+ * session recordings, this is determined by testing whether there is an
+ * associated timing file. Guacamole will always include a timing file for
+ * its typescripts.
+ *
+ * @param file
+ * The file to test.
+ *
+ * @return
+ * true if the file appears to be a typescript, false otherwise.
+ */
+ private boolean isTypescript(File file) {
+ return new File(file.getAbsolutePath() + TIMING_FILE_SUFFIX).exists();
+ }
+
+ /**
+ * Returns whether the given file appears to be a typescript timing file.
+ * Typescript timing files have the standard extension ".timing".
+ *
+ * @param file
+ * The file to test.
+ *
+ * @return
+ * true if the file appears to be a typescript timing file, false
+ * otherwise.
+ */
+ private boolean isTypescriptTiming(File file) {
+ return file.getName().endsWith(TIMING_FILE_SUFFIX);
+ }
+
+ /**
+ * Returns the type of session recording or log contained within the given
+ * file by inspecting its name and contents.
+ *
+ * @param file
+ * The file to test.
+ *
+ * @return
+ * The type of session recording or log contained within the given
+ * file, or null if this cannot be determined.
Review comment:
No, you're right - it will not currently return `null` under any
circumstance. I defined explicit meaning and handling for `null` here to ensure
that the caller does not assume that this will always be the case. While the
function will currently never return `null`, that's due to an implementation
detail of the function and not some inherent property of what the function does.
##########
File path:
guacamole/src/main/frontend/src/app/rest/types/ConnectionHistoryEntry.js
##########
@@ -38,6 +38,23 @@ angular.module('rest').factory('ConnectionHistoryEntry',
[function defineConnect
// Use empty object by default
template = template || {};
+ /**
+ * An arbitrary unique identifier that uniquely identifies this record
Review comment:
I agree this sounds pretty redundant, but I also think it's important to
highlight the specific scope of the identifier's uniqueness. I'll reword to
"arbitrary identifier that uniquely identifies".
##########
File path:
guacamole/src/main/java/org/apache/guacamole/rest/history/APIActivityRecord.java
##########
@@ -55,6 +58,30 @@
*/
private final boolean active;
+ /**
+ * The unique identifier assigned to this record, or null if this record
+ * has no such identifier.
+ */
+ private final String identifier;
+
+ /**
+ * A UUID uniquely identifies this record, or null if no such unique
Review comment:
Oops - I'll add that "that".
I think "uniquely identifies" should remain, as it is specifically this
record that is uniquely identified. The value _should_ be unique relative to
all possible UUIDs, but the specification for the record UUIDs given by
`getUUID()` of `ActivityRecord` allows some wiggle room here so long as it's at
least unique relative to other records in the same set.
##########
File path:
guacamole/src/main/java/org/apache/guacamole/rest/history/ActivityLogResource.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.rest.history;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.core.Response.ResponseBuilder;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.net.auth.ActivityLog;
+
+/**
+ * A REST resource which exposes the contents of a given ActivityLog.
+ */
+public class ActivityLogResource {
+
+ /**
+ * The ActivityLog whose contents are being exposed.
+ */
+ private final ActivityLog log;
+
+ /**
+ * Creates a new ActivityRecordSetResource which exposes the records within
+ * the given ActivityRecordSet.
Review comment:
Yup - definitely copypasta.
##########
File path:
guacamole/src/main/java/org/apache/guacamole/rest/history/ActivityLogResource.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.rest.history;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.core.Response.ResponseBuilder;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.net.auth.ActivityLog;
+
+/**
+ * A REST resource which exposes the contents of a given ActivityLog.
+ */
+public class ActivityLogResource {
+
+ /**
+ * The ActivityLog whose contents are being exposed.
+ */
+ private final ActivityLog log;
+
+ /**
+ * Creates a new ActivityRecordSetResource which exposes the records within
+ * the given ActivityRecordSet.
+ *
+ * @param log
+ * The ActivityLog whose contents should be exposed.
+ */
+ public ActivityLogResource(ActivityLog log) {
+ this.log = log;
+ }
+
+ /**
+ * Returns the raw contents of the underlying ActivityLog. If the size of
+ * the ActivityLog is known, this size is included as the "Content-Length"
+ * of the response.
+ *
+ * @return
+ * A Response containing the raw contents of the underlying
+ * ActivityLog.
+ *
+ * @throws GuacamoleException
+ * If an error prevents retrieving the content of the log or its size.
+ */
+ @GET
+ public Response getContents() throws GuacamoleException {
+
+ // Build base response exposing the raw contents of the underlying log
+ ResponseBuilder response = Response.ok(log.getContent(),
+ log.getType().getContentType());
+
+ // Include size, if knopwn
Review comment:
Nah, this one is a loud "p", and it must be spoken with as much volume
and enunciation as possible.
Also yes. I'll pfix this...
##########
File path: guacamole/src/main/frontend/src/app/rest/types/ActivityLog.js
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.
+ */
+
+/**
+ * Service which defines the ActivityLog class.
+ */
+angular.module('rest').factory('ActivityLog', [function defineActivityLog() {
+
+ /**
+ * The object returned by REST API calls when representing the a log or
Review comment:
Fixed via rebase.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
