mike-jumper commented on code in PR #677:
URL: https://github.com/apache/guacamole-client/pull/677#discussion_r1169320150
##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java:
##########
@@ -416,6 +462,21 @@ private GuacamoleTunnel
assignGuacamoleTunnel(ActiveConnectionRecord activeConne
GuacamoleClientInformation info, Map<String, String> tokens,
boolean interceptErrors) throws GuacamoleException {
+ // Set up JDBC-specific tokens
+ tokens.put(JDBC_DATE_TOKEN,
+ new SimpleDateFormat(JDBC_DATE_TOKEN_FORMAT)
+ .format(activeConnection.getStartDate()));
+ tokens.put(JDBC_TIME_TOKEN,
+ new SimpleDateFormat(JDBC_TIME_TOKEN_FORMAT)
+ .format(activeConnection.getStartDate()));
Review Comment:
Did we already go through whether it would be better to overwrite
`GUAC_DATE` / `GUAC_TIME` with values that will match the history entry? I
remember there being a discussion about that at some point, though I don't
recall the consensus.
##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java:
##########
@@ -416,6 +462,21 @@ private GuacamoleTunnel
assignGuacamoleTunnel(ActiveConnectionRecord activeConne
GuacamoleClientInformation info, Map<String, String> tokens,
boolean interceptErrors) throws GuacamoleException {
+ // Set up JDBC-specific tokens
+ tokens.put(JDBC_DATE_TOKEN,
+ new SimpleDateFormat(JDBC_DATE_TOKEN_FORMAT)
+ .format(activeConnection.getStartDate()));
+ tokens.put(JDBC_TIME_TOKEN,
+ new SimpleDateFormat(JDBC_TIME_TOKEN_FORMAT)
+ .format(activeConnection.getStartDate()));
+ tokens.put(JDBC_CONNECTION_NAME_TOKEN,
activeConnection.getConnectionName());
+ tokens.put(JDBC_CONNECTION_ID_TOKEN,
activeConnection.getConnectionIdentifier());
Review Comment:
I'm not sure about adding tokens specific to JDBC for the identifier and
name. What about instead implementing `GUAC_CONNECTION_NAME` /
`GUAC_CONNECTION_ID` at the webapp level via `StandardTokenMap`?
And what is the intended behavior for these if a balancing connection group
is used?
##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java:
##########
@@ -416,6 +462,21 @@ private GuacamoleTunnel
assignGuacamoleTunnel(ActiveConnectionRecord activeConne
GuacamoleClientInformation info, Map<String, String> tokens,
boolean interceptErrors) throws GuacamoleException {
+ // Set up JDBC-specific tokens
+ tokens.put(JDBC_DATE_TOKEN,
+ new SimpleDateFormat(JDBC_DATE_TOKEN_FORMAT)
+ .format(activeConnection.getStartDate()));
+ tokens.put(JDBC_TIME_TOKEN,
+ new SimpleDateFormat(JDBC_TIME_TOKEN_FORMAT)
+ .format(activeConnection.getStartDate()));
+ tokens.put(JDBC_CONNECTION_NAME_TOKEN,
activeConnection.getConnectionName());
+ tokens.put(JDBC_CONNECTION_ID_TOKEN,
activeConnection.getConnectionIdentifier());
+ tokens.put(JDBC_CONNECTION_HOSTNAME_TOKEN,
+
activeConnection.getConnection().getConfiguration().getParameter(JDBC_CONNECTION_HOSTNAME_TOKEN_PARAMTER));
+ tokens.put(JDBC_CONNECTION_PROTOCOL_TOKEN,
+
activeConnection.getConnection().getConfiguration().getProtocol());
Review Comment:
Within `AbstractGuacamoleTunnelService`, I think any additional tokens
should be added alongside where we are already adding `HISTORY_UUID`:
https://github.com/apache/guacamole-client/blob/165bd413c0ab9c55b0c0e3614fcb829ef5f8ca95/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java#L463-L465
--
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]
