necouchman commented on a change in pull request #546:
URL: https://github.com/apache/guacamole-client/pull/546#discussion_r514357759
##########
File path:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ConnectionService.java
##########
@@ -411,28 +411,16 @@ protected ConnectionRecord
getObjectInstance(ConnectionRecordModel model) {
ModeledConnection connection) throws GuacamoleException {
String identifier = connection.getIdentifier();
-
- // Retrieve history only if READ permission is granted
- if (hasObjectPermission(user, identifier, ObjectPermission.Type.READ))
{
Review comment:
Because this version of `retrieveHistory()` now simply brokers the call
to the next `retrieveHistory()` (below it), which uses `user.isPrivileged()`
and then `connectionRecordMapper.searchReadable()` to gather records that the
user should have access to:
https://github.com/apache/guacamole-client/pull/546/files/96af10d29ac0715afad4e70148865d9fb3bd0223#diff-463b715cf1341f9d584036345fa3fed272acee431bffc12d3db3d11a792b9540R458-R479
The only other concern would be `tunnelService.getActiveConnections()`,
which does its own checks for permissions. Thus it seems that checking for
readable permissions, here, is just verifying what we already know?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
