Skip to content

Commit

Permalink
GUACAMOLE-1239: Update JDBC queries to handle case-sensitivity.
Browse files Browse the repository at this point in the history
  • Loading branch information
necouchman committed Sep 17, 2024
1 parent 44f7b85 commit f48d596
Show file tree
Hide file tree
Showing 37 changed files with 1,226 additions and 168 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.guacamole.auth.jdbc;

import com.google.inject.Inject;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -55,6 +56,12 @@ public class HistoryTrackingConnection extends DelegatingConnection {
* established connections.
*/
private final ConnectionRecordMapper connectionRecordMapper;

/**
* The Guacamole server environment.
*/
@Inject
private JDBCEnvironment environment;

/**
* Creates a new HistoryConnection that wraps the given connection,
Expand Down Expand Up @@ -98,7 +105,8 @@ public GuacamoleTunnel connect(GuacamoleClientInformation info,
connectionRecordModel.setConnectionName(this.getDelegateConnection().getName());

// Insert the connection history record to mark the start of this connection
connectionRecordMapper.insert(connectionRecordModel);
connectionRecordMapper.insert(connectionRecordModel,
environment.getCaseSensitiveUsernames());

// Include history record UUID as token
ModeledConnectionRecord modeledRecord = new ModeledConnectionRecord(connectionRecordModel);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,16 +273,30 @@ public boolean shouldUseBatchExecutor() {
}

/**
* Returns a boolean value that indicates whether or not usernames should
* be treated as case-sensitive.
* Returns true if the usernames provided to the JDBC authentication
* module should be treated as case-sensitive, or false if usernames
* should be treated as case-insensitive. The default is true, usernames
* will be case-sensitive in keeping with the past behavior of Guacamole
* prior to the addition of this option.
*
* @return
* true if usernames should be treated as case-sensitive, or false if
* usernames should be treated as case-insensitive.
* true if usernames should be treated as case-sensitive, otherwise
* false.
*
* @throws GuacamoleException
* If guacamole.properties cannot be parsed.
*/
public abstract boolean getCaseSensitiveUsernames() throws GuacamoleException;
public boolean getCaseSensitiveUsernames() throws GuacamoleException {

// In keeping with past behavior of Guacamole prior to the introduction
// of this feature, we default to case-sensitive username comparisons.
// However, it is worth noting that some databases do not do
// case-sensitive string comparisons by default, and additional
// configuration is required to make string comparisons case-sensitive
// If you need that functionality, please test with your database and
// configure both that and Guacamole appropriately.
return true;

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,16 @@ public interface ActivityRecordMapper<ModelType> {
*
* @param record
* The activity record to insert.
*
* @param caseSensitive
* Whether or not string comparisons should be done in a case-sensitive
* manner.
*
* @return
* The number of rows inserted.
*/
int insert(@Param("record") ModelType record);
int insert(@Param("record") ModelType record,
@Param("caseSensitive") boolean caseSensitive);

/**
* Updates the given activity record in the database, assigning an end
Expand Down Expand Up @@ -85,6 +90,10 @@ public interface ActivityRecordMapper<ModelType> {
*
* @param limit
* The maximum number of records that should be returned.
*
* @param caseSensitive
* Whether or not string comparisons should be done in a case-sensitive
* manner.
*
* @return
* The results of the search performed with the given parameters.
Expand All @@ -93,7 +102,8 @@ List<ModelType> search(@Param("identifier") String identifier,
@Param("recordIdentifier") String recordIdentifier,
@Param("terms") Collection<ActivityRecordSearchTerm> terms,
@Param("sortPredicates") List<ActivityRecordSortPredicate> sortPredicates,
@Param("limit") int limit);
@Param("limit") int limit,
@Param("caseSensitive") boolean caseSensitive);

/**
* Searches for up to <code>limit</code> activity records that contain
Expand Down Expand Up @@ -132,6 +142,10 @@ List<ModelType> search(@Param("identifier") String identifier,
* when determining the permissions effectively granted to the user. If
* no groups are given, only permissions directly granted to the user
* will be used.
*
* @param caseSensitive
* Whether or not string comparisons should be done in a case-sensitive
* manner.
*
* @return
* The results of the search performed with the given parameters.
Expand All @@ -142,6 +156,7 @@ List<ModelType> searchReadable(@Param("identifier") String identifier,
@Param("terms") Collection<ActivityRecordSearchTerm> terms,
@Param("sortPredicates") List<ActivityRecordSortPredicate> sortPredicates,
@Param("limit") int limit,
@Param("effectiveGroups") Collection<String> effectiveGroups);
@Param("effectiveGroups") Collection<String> effectiveGroups,
@Param("caseSensitive") boolean caseSensitive);

}
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,20 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.apache.guacamole.auth.jdbc.user.ModeledAuthenticatedUser;
import org.apache.guacamole.language.TranslatableGuacamoleClientOverrunException;
import org.apache.guacamole.language.TranslatableMessage;
import org.apache.guacamole.auth.jdbc.base.ModeledDirectoryObjectMapper;
import org.apache.guacamole.auth.jdbc.tunnel.GuacamoleTunnelService;
import org.apache.guacamole.GuacamoleClientException;
import org.apache.guacamole.GuacamoleException;
import org.apache.guacamole.GuacamoleSecurityException;
import org.apache.guacamole.auth.jdbc.JDBCEnvironment;
import org.apache.guacamole.auth.jdbc.base.ActivityRecordSearchTerm;
import org.apache.guacamole.auth.jdbc.base.ActivityRecordSortPredicate;
import org.apache.guacamole.auth.jdbc.user.ModeledAuthenticatedUser;
import org.apache.guacamole.auth.jdbc.base.ModeledChildDirectoryObjectService;
import org.apache.guacamole.auth.jdbc.base.ModeledDirectoryObjectMapper;
import org.apache.guacamole.auth.jdbc.permission.ConnectionPermissionMapper;
import org.apache.guacamole.auth.jdbc.permission.ObjectPermissionMapper;
import org.apache.guacamole.auth.jdbc.tunnel.GuacamoleTunnelService;
import org.apache.guacamole.language.TranslatableGuacamoleClientOverrunException;
import org.apache.guacamole.language.TranslatableMessage;
import org.apache.guacamole.net.GuacamoleTunnel;
import org.apache.guacamole.net.auth.Connection;
import org.apache.guacamole.net.auth.ConnectionRecord;
Expand Down Expand Up @@ -85,6 +86,12 @@ public class ConnectionService extends ModeledChildDirectoryObjectService<Modele
*/
@Inject
private Provider<ModeledConnection> connectionProvider;

/**
* The server environment for retrieving configuration.
*/
@Inject
private JDBCEnvironment environment;

/**
* Service for creating and tracking tunnels.
Expand Down Expand Up @@ -486,14 +493,16 @@ public List<ConnectionRecord> retrieveHistory(String identifier,
// Bypass permission checks if the user is privileged or has System-level audit permissions
if (user.isPrivileged() || user.getUser().getEffectivePermissions().getSystemPermissions().hasPermission(SystemPermission.Type.AUDIT))
searchResults = connectionRecordMapper.search(identifier,
recordIdentifier, requiredContents, sortPredicates, limit);
recordIdentifier, requiredContents, sortPredicates, limit,
environment.getCaseSensitiveUsernames());

// Otherwise only return explicitly readable history records
else
searchResults = connectionRecordMapper.searchReadable(identifier,
user.getUser().getModel(), recordIdentifier,
requiredContents, sortPredicates, limit,
user.getEffectiveUserGroups());
user.getEffectiveUserGroups(),
environment.getCaseSensitiveUsernames());

return getObjectInstances(searchResults);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,142 @@

package org.apache.guacamole.auth.jdbc.permission;

import java.util.Collection;
import org.apache.guacamole.auth.jdbc.base.EntityModel;
import org.apache.guacamole.net.auth.permission.ObjectPermission;
import org.apache.ibatis.annotations.Param;

/**
* Mapper for user permissions.
*/
public interface UserPermissionMapper extends ObjectPermissionMapper {}
public interface UserPermissionMapper extends ObjectPermissionMapper {

/**
* Deletes the given permissions from the database. If any permissions do
* not exist, they will be ignored.
*
* @param permissions
* The permissions to delete.
*
* @param caseSensitive
* Whether or not string comparisons for usernames will be done in a
* case-sensitive manner.
*
* @return
* The number of rows deleted.
*/
int delete(@Param("permissions") Collection<ObjectPermission.Type> permissions,
@Param("caseSensitive") boolean caseSensitive);

/**
* Inserts the given permissions into the database. If any permissions
* already exist, they will be ignored.
*
* @param permissions
* The permissions to insert.
*
* @param caseSensitive
* Whether or not string comparisons for usernames will be done in a
* case-sensitive manner.
*
* @return
* The number of rows inserted.
*/
int insert(@Param("permissions") Collection<ObjectPermission.Type> permissions,
@Param("caseSensitive") boolean caseSensitive);

/**
* Retrieves all permissions associated with the given entity (user or user
* group).
*
* @param entity
* The entity to retrieve permissions for.
*
* @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.
*
* @param caseSensitive
* Whether or not string comparisons for usernames will be done in a
* case-sensitive manner.
*
* @return
* All permissions associated with the given entity.
*/
Collection<ObjectPermission.Type> select(@Param("entity") EntityModel entity,
@Param("effectiveGroups") Collection<String> effectiveGroups,
@Param("caseSensitive") boolean caseSensitive);

/**
* Retrieve the permission of the given type associated with the given
* entity and object, if it exists. If no such permission exists, null is
* returned.
*
* @param entity
* The entity to retrieve permissions for.
*
* @param type
* The type of permission to return.
*
* @param identifier
* The identifier of the object affected by the permission to return.
*
* @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.
*
* @param caseSensitive
* Whether or not string comparisons for usernames will be done in a
* case-sensitive manner.
*
* @return
* The requested permission, or null if no such permission is granted
* to the given entity for the given object.
*/
ObjectPermissionModel selectOne(@Param("entity") EntityModel entity,
@Param("type") ObjectPermission.Type type,
@Param("identifier") String identifier,
@Param("effectiveGroups") Collection<String> effectiveGroups,
@Param("caseSensitive") boolean caseSensitive);

/**
* Retrieves the subset of the given identifiers for which the given entity
* has at least one of the given permissions.
*
* @param entity
* The entity to check permissions of.
*
* @param permissions
* The permissions to check. An identifier will be included in the
* resulting collection if at least one of these permissions is granted
* for the associated object
*
* @param identifiers
* The identifiers of the objects affected by the permissions being
* checked.
*
* @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.
*
* @param caseSensitive
* Whether or not string comparisons for usernames will be done in a
* case-sensitive manner.
*
* @return
* A collection containing the subset of identifiers for which at least
* one of the specified permissions is granted.
*/
Collection<String> selectAccessibleIdentifiers(@Param("entity") EntityModel entity,
@Param("permissions") Collection<ObjectPermission.Type> permissions,
@Param("identifiers") Collection<String> identifiers,
@Param("effectiveGroups") Collection<String> effectiveGroups,
@Param("caseSenstive") boolean caseSensitive);

}
Original file line number Diff line number Diff line change
Expand Up @@ -131,16 +131,21 @@ private boolean matches(String str, Pattern... patterns) {
* @return
* true if the given password matches any of the user's previous
* passwords, up to the specified limit, false otherwise.
*
* @throws GuacamoleException
* If an error occurs accessing environment information used for
* retrieving configuration values.
*/
private boolean matchesPreviousPasswords(String password, String username,
int historySize) {
int historySize) throws GuacamoleException {

// No need to compare if no history is relevant
if (historySize <= 0)
return false;

// Check password against all recorded hashes
List<PasswordRecordModel> history = passwordRecordMapper.select(username, historySize);
List<PasswordRecordModel> history = passwordRecordMapper.select(username,
historySize, environment.getCaseSensitiveUsernames());
for (PasswordRecordModel record : history) {

byte[] hash = encryptionService.createPasswordHash(password, record.getPasswordSalt());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.guacamole.auth.jdbc.JDBCEnvironment;
import org.apache.guacamole.auth.jdbc.user.ModeledAuthenticatedUser;
import org.apache.guacamole.auth.jdbc.connection.ModeledConnection;
import org.apache.guacamole.auth.jdbc.connectiongroup.ModeledConnectionGroup;
Expand Down Expand Up @@ -167,6 +168,12 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS
*/
@Inject
private SharedConnectionMap connectionMap;

/**
* The Guacamole server environment.
*/
@Inject
private JDBCEnvironment environment;

/**
* All active connections through the tunnel having a given UUID.
Expand Down Expand Up @@ -470,7 +477,9 @@ private GuacamoleTunnel assignGuacamoleTunnel(ActiveConnectionRecord activeConne
// Record new active connection
Runnable cleanupTask = new ConnectionCleanupTask(activeConnection);
try {
connectionRecordMapper.insert(activeConnection.getModel()); // This MUST happen before getUUID() is invoked, to ensure the ID driving the UUID exists
// This MUST happen before getUUID() is invoked, to ensure the ID driving the UUID exists
connectionRecordMapper.insert(activeConnection.getModel(),
environment.getCaseSensitiveUsernames());
activeTunnels.put(activeConnection.getUUID().toString(), activeConnection);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,9 @@ public boolean isCaseSensitive() {
return environment.getCaseSensitiveUsernames();
}
catch (GuacamoleException e) {
logger.error("Failed to retrieve the configuration for case-sensitive usernames: {}."
+ " Usernames comparisons will be case-sensitive.", e.getMessage());
logger.debug("Exception caught when attempting to read the configuration.", e);
return true;
}
}
Expand Down
Loading

0 comments on commit f48d596

Please sign in to comment.