Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GUACAMOLE-1239: Correct regressions introduced by case sensitive checks for usernames. #1026

Merged
merged 3 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,16 @@ Set<String> selectReadableIdentifiers(@Param("user") UserModel user,
*
* @param identifiers
* The identifiers of the objects to return.
*
* @param caseSensitive
* true if the query should evaluate identifiers in a case-sensitive
* manner, otherwise false.
*
* @return
* A Collection of all objects having the given identifiers.
*/
Collection<ModelType> select(@Param("identifiers") Collection<String> identifiers);
Collection<ModelType> select(@Param("identifiers") Collection<String> identifiers,
@Param("caseSensitive") boolean caseSensitive);

/**
* Selects all objects which have the given identifiers and are explicitly
Expand All @@ -99,13 +104,18 @@ Set<String> selectReadableIdentifiers(@Param("user") UserModel user,
* @param effectiveGroups
* The identifiers of any known effective groups that should be taken
* into account, such as those defined externally to the database.
*
* @param caseSensitive
* true if the query should evaluate identifiers in a case-sensitive
* manner, otherwise false.
*
* @return
* A Collection of all objects having the given identifiers.
*/
Collection<ModelType> selectReadable(@Param("user") UserModel user,
@Param("identifiers") Collection<String> identifiers,
@Param("effectiveGroups") Collection<String> effectiveGroups);
@Param("effectiveGroups") Collection<String> effectiveGroups,
@Param("caseSensitive") boolean caseSensitive);

/**
* Inserts the given object into the database. If the object already
Expand All @@ -125,11 +135,16 @@ Collection<ModelType> selectReadable(@Param("user") UserModel user,
*
* @param identifier
* The identifier of the object to delete.
*
* @param caseSensitive
* true if the query should evaluate the identifier in a
* case-sensitive manner, otherwise false.
*
* @return
* The number of rows deleted.
*/
int delete(@Param("identifier") String identifier);
int delete(@Param("identifier") String identifier,
@Param("caseSensitive") boolean caseSensitive);

/**
* Updates the given existing object in the database. If the object does
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,25 @@ public abstract class ModeledDirectoryObjectService<InternalType extends Modeled
*/
protected abstract InternalType getObjectInstance(ModeledAuthenticatedUser currentUser,
ModelType model) throws GuacamoleException;


/**
* Returns whether or not identifiers for objects provided by this service
* are handled in a case-sensitive manner or not.
*
* @return
* "true" if identifiers handled by this object service should be
* treated as case-sensitive, otherwise false.
*
* @throws GuacamoleException
* If an error occurs retrieving relevant configuration information.
*/
protected boolean getCaseSensitiveIdentifiers() throws GuacamoleException {

// By default identifiers are not case-sensitive.
return false;

}

/**
* Returns an instance of a model object which is based on the given
* object.
Expand Down Expand Up @@ -407,6 +425,8 @@ public Collection<InternalType> retrieveObjects(ModeledAuthenticatedUser user,
int batchSize = environment.getBatchSize();

boolean userIsPrivileged = user.isPrivileged();

boolean caseSensitive = getCaseSensitiveIdentifiers();

// Process the filteredIdentifiers in batches using Lists.partition() and flatMap
Collection<ModelType> allObjects = Lists.partition(filteredIdentifiers, batchSize).stream()
Expand All @@ -415,12 +435,12 @@ public Collection<InternalType> retrieveObjects(ModeledAuthenticatedUser user,

// Bypass permission checks if the user is privileged
if (userIsPrivileged)
objects = getObjectMapper().select(chunk);
objects = getObjectMapper().select(chunk, caseSensitive);

// Otherwise only return explicitly readable identifiers
else
objects = getObjectMapper().selectReadable(user.getUser().getModel(),
chunk, user.getEffectiveUserGroups());
chunk, user.getEffectiveUserGroups(), caseSensitive);

return objects.stream();
})
Expand Down Expand Up @@ -510,7 +530,7 @@ public void deleteObject(ModeledAuthenticatedUser user, String identifier)
beforeDelete(user, identifier);

// Delete object
getObjectMapper().delete(identifier);
getObjectMapper().delete(identifier, getCaseSensitiveIdentifiers());

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,18 @@ public interface ObjectRelationMapper<ParentModelType extends ObjectModel> {
* @param children
* The identifiers of the objects on the child side of the one-to-many
* relationship represented by the RelatedObjectSet.
*
* @param caseSensitive
* true if child identifiers should be treated as case-sensitive when
* performing lookups on them, or false if the queries should be done
* case-insensitively.
*
* @return
* The number of rows inserted.
*/
int insert(@Param("parent") ParentModelType parent,
@Param("children") Collection<String> children);
@Param("children") Collection<String> children,
@Param("caseSensitive") boolean caseSensitive);

/**
* Deletes rows as necessary to modify the one-to-many relationship
Expand All @@ -69,12 +75,18 @@ int insert(@Param("parent") ParentModelType parent,
* @param children
* The identifiers of the objects on the child side of the one-to-many
* relationship represented by the RelatedObjectSet.
*
* @param caseSensitive
* true if child identifiers should be treated as case-sensitive when
* performing lookups on them, or false if the queries should be done
* case-insensitively.
*
* @return
* The number of rows deleted.
*/
int delete(@Param("parent") ParentModelType parent,
@Param("children") Collection<String> children);
@Param("children") Collection<String> children,
@Param("caseSensitive") boolean caseSensitive);

/**
* Retrieves the identifiers of all objects on the child side of the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,24 @@ public void init(ModeledAuthenticatedUser currentUser, ParentObjectType parent)
this.parent = parent;
}

/**
* Return "true" if identifiers within a related object set should be treated
* as case-sensitive, otherwise false.
*
* @return
* "true" if identifiers should be treated as case-sensitive, otherwise
* "false".
*
* @throws GuacamoleException
* If an error occurs retrieving configuration information on
* case-sensitivity.
*/
protected boolean getCaseSensitiveIdentifiers() throws GuacamoleException {

// Identifiers are not case-sensitive by default.
return false;
}

/**
* Returns the mapper which provides low-level access to the database
* models which drive the relation represented by this RelatedObjectSet.
Expand Down Expand Up @@ -184,7 +202,7 @@ public void addObjects(Set<String> identifiers) throws GuacamoleException {

// Create relations only if permission is granted
if (canAlterRelation(identifiers))
getObjectRelationMapper().insert(parent.getModel(), identifiers);
getObjectRelationMapper().insert(parent.getModel(), identifiers, getCaseSensitiveIdentifiers());

// User lacks permission to add user groups
else
Expand All @@ -201,7 +219,7 @@ public void removeObjects(Set<String> identifiers) throws GuacamoleException {

// Delete relations only if permission is granted
if (canAlterRelation(identifiers))
getObjectRelationMapper().delete(parent.getModel(), identifiers);
getObjectRelationMapper().delete(parent.getModel(), identifiers, getCaseSensitiveIdentifiers());

// User lacks permission to remove user groups
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,6 @@ Collection<String> selectAccessibleIdentifiers(@Param("entity") EntityModel enti
@Param("permissions") Collection<ObjectPermission.Type> permissions,
@Param("identifiers") Collection<String> identifiers,
@Param("effectiveGroups") Collection<String> effectiveGroups,
@Param("caseSenstive") boolean caseSensitive);
@Param("caseSensitive") boolean caseSensitive);

}
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ private List<ModeledConnection> getBalancedConnections(ModeledAuthenticatedUser
identifiers = getPreferredConnections(user, identifiers);

// Retrieve all children
Collection<ConnectionModel> models = connectionMapper.select(identifiers);
Collection<ConnectionModel> models = connectionMapper.select(identifiers, false);
List<ModeledConnection> connections = new ArrayList<ModeledConnection>(models.size());

// Convert each retrieved model to a modeled connection
Expand Down Expand Up @@ -679,7 +679,7 @@ public Collection<ActiveConnectionRecord> getActiveConnections(ModeledAuthentica
// Produce collection of readable connection identifiers
Collection<ConnectionModel> connections =
connectionMapper.selectReadable(user.getUser().getModel(),
identifiers, user.getEffectiveUserGroups());
identifiers, user.getEffectiveUserGroups(), false);

// Ensure set contains only identifiers of readable connections
identifiers.clear();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

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

import java.util.Collection;
import org.apache.guacamole.auth.jdbc.base.ModeledDirectoryObjectMapper;
import org.apache.ibatis.annotations.Param;

Expand All @@ -36,80 +35,13 @@ public interface UserMapper extends ModeledDirectoryObjectMapper<UserModel> {
* The username of the user to return.
*
* @param caseSensitive
* true if the search should evaluate usernames in a case-sensitive
* true if the search should evaluate the username in a case-sensitive
* manner, otherwise false.
*
* @return
* The user having the given username, or null if no such user exists.
*/
UserModel selectOne(@Param("username") String username,
@Param("caseSensitive") boolean caseSensitive);

/**
* Selects all users which have the given identifiers. If an identifier
* has no corresponding object, it will be ignored. This should only be
* called on behalf of a system administrator. If users are needed by a
* non-administrative user who must have explicit read rights, use
* selectReadable() instead.
*
* @param identifiers
* The identifiers of the users to return.
*
* @param caseSensitive
* true if the query should evaluate username identifiers in a
* case-sensitive manner, otherwise false.
*
* @return
* A Collection of all objects having the given identifiers.
*/
Collection<UserModel> select(@Param("identifiers") Collection<String> identifiers,
@Param("caseSensitive") boolean caseSensitive);

/**
* Selects all users which have the given identifiers and are explicitly
* readable by the given user. If an identifier has no corresponding
* object, or the corresponding user is unreadable, it will be ignored.
* If users are needed by a system administrator (who, by definition,
* does not need explicit read rights), use select() instead.
*
* @param user
* The user whose permissions should determine whether an object
* is returned.
*
* @param identifiers
* The identifiers of the users to return.
*
* @param effectiveGroups
* The identifiers of any known effective groups that should be taken
* into account, such as those defined externally to the database.
*
* @param caseSensitive
* true if the query should evaluate username identifiers in a
* case-sensitive manner, otherwise false.
*
* @return
* A Collection of all objects having the given identifiers.
*/
Collection<UserModel> selectReadable(@Param("user") UserModel user,
@Param("identifiers") Collection<String> identifiers,
@Param("effectiveGroups") Collection<String> effectiveGroups,
@Param("caseSensitive") boolean caseSensitive);

/**
* Deletes the given user from the database. If the user does not
* exist, this operation has no effect.
*
* @param identifier
* The identifier of the user to delete.
*
* @param caseSensitive
* true if the query should evaluate username identifiers in a
* case-sensitive manner, otherwise false.
*
* @return
* The number of rows deleted.
*/
int delete(@Param("identifier") String identifier,
@Param("caseSensitive") boolean caseSensitive);

}
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ protected UserModel getModelInstance(ModeledAuthenticatedUser currentUser,
return model;

}

@Override
protected boolean getCaseSensitiveIdentifiers() throws GuacamoleException {
return environment.getCaseSensitiveUsernames();
}

@Override
protected boolean hasCreatePermission(ModeledAuthenticatedUser user)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,59 +27,4 @@
* Mapper for the one-to-many relationship between a user group and its user
* members.
*/
public interface UserGroupMemberUserMapper extends ObjectRelationMapper<UserGroupModel> {

/**
* Inserts rows as necessary to establish the one-to-many relationship
* represented by the RelatedObjectSet between the given parent and
* children. If the relation for any parent/child pair is already present,
* no attempt is made to insert a new row for that relation.
*
* @param parent
* The model of the object on the parent side of the one-to-many
* relationship represented by the RelatedObjectSet.
*
* @param children
* The identifiers of the objects on the child side of the one-to-many
* relationship represented by the RelatedObjectSet.
*
* @param caseSensitive
* True if username case should be respected when looking up the username
* in the guacamole_entity table, or false if the query to the
* guacamole_entity table should be done case-insensitively.
*
* @return
* The number of rows inserted.
*/
int insert(@Param("parent") UserGroupModel parent,
@Param("children") Collection<String> children,
@Param("caseSensitive") boolean caseSensitive);

/**
* Deletes rows as necessary to modify the one-to-many relationship
* represented by the RelatedObjectSet between the given parent and
* children. If the relation for any parent/child pair does not exist,
* that specific relation is ignored, and deletion proceeds with the
* remaining relations.
*
* @param parent
* The model of the object on the parent side of the one-to-many
* relationship represented by the RelatedObjectSet.
*
* @param children
* The identifiers of the objects on the child side of the one-to-many
* relationship represented by the RelatedObjectSet.
*
* @param caseSensitive
* True if username case should be respected when looking up the username
* in the guacamole_entity table, or false if the query to the
* guacamole_entity table should be done case-insensitively.
*
* @return
* The number of rows deleted.
*/
int delete(@Param("parent") UserGroupModel parent,
@Param("children") Collection<String> children,
@Param("caseSesitive") boolean caseSensitive);

}
public interface UserGroupMemberUserMapper extends ObjectRelationMapper<UserGroupModel> {}
Loading
Loading