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 logic for, and fix missing, case sensitivity settings. #1028

Merged
merged 8 commits into from
Nov 10, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -53,26 +53,5 @@ public String getHttpAuthHeader() throws GuacamoleException {
"REMOTE_USER"
);
}

/**
* Returns true if the usernames provided to the header authentication
* module should be treated as case-sensitive, or false if usernames
* should be treated as case-insensitive. This will default to the global
* Guacamole configuration for case-sensitivity, which defaults to true, but
* can be overridden for this extension, if desired.
*
* @return
* true if usernames should be treated as case-sensitive, otherwise
* false.
*
* @throws GuacamoleException
* If guacamole.properties cannot be parsed.
*/
public boolean getCaseSensitiveUsernames() throws GuacamoleException {
return environment.getProperty(
HTTPHeaderGuacamoleProperties.HTTP_AUTH_CASE_SENSITIVE_USERNAMES,
environment.getCaseSensitiveUsernames()
);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package org.apache.guacamole.auth.header;

import org.apache.guacamole.properties.BooleanGuacamoleProperty;
import org.apache.guacamole.properties.StringGuacamoleProperty;


Expand All @@ -44,17 +43,5 @@ private HTTPHeaderGuacamoleProperties() {}
public String getName() { return "http-auth-header"; }

};

/**
* A property used to configure whether or not usernames within the header
* module should be treated as case-sensitive.
*/
public static final BooleanGuacamoleProperty HTTP_AUTH_CASE_SENSITIVE_USERNAMES =
new BooleanGuacamoleProperty() {

@Override
public String getName() { return "http-auth-case-sensitive-usernames"; }

};

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,38 +20,23 @@
package org.apache.guacamole.auth.header.user;

import com.google.inject.Inject;
import org.apache.guacamole.GuacamoleException;
import org.apache.guacamole.auth.header.ConfigurationService;
import org.apache.guacamole.net.auth.AbstractAuthenticatedUser;
import org.apache.guacamole.net.auth.AuthenticationProvider;
import org.apache.guacamole.net.auth.Credentials;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* An HTTP header implementation of AuthenticatedUser, associating a
* username and particular set of credentials with the HTTP authentication
* provider.
*/
public class AuthenticatedUser extends AbstractAuthenticatedUser {

/**
* Logger for this class.
*/
private static final Logger LOGGER = LoggerFactory.getLogger(AuthenticatedUser.class);

/**
* Reference to the authentication provider associated with this
* authenticated user.
*/
@Inject
private AuthenticationProvider authProvider;

/**
* Service for retrieving header configuration information.
*/
@Inject
private ConfigurationService confService;

/**
* The credentials provided when this user was authenticated.
Expand All @@ -72,19 +57,6 @@ public void init(String username, Credentials credentials) {
this.credentials = credentials;
setIdentifier(username.toLowerCase());
}

@Override
public boolean isCaseSensitive() {
try {
return confService.getCaseSensitiveUsernames();
}
catch (GuacamoleException e) {
LOGGER.error("Error when trying to retrieve header configuration: {}."
+ " Usernames comparison will be case-sensitive.", e);
LOGGER.debug("Exception caught when retrieving header configuration.", e);
return true;
}
}

@Override
public AuthenticationProvider getAuthenticationProvider() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,6 @@ 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 @@ -106,7 +100,7 @@ public GuacamoleTunnel connect(GuacamoleClientInformation info,

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

// 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 @@ -513,7 +513,7 @@ public InternalType createObject(ModeledAuthenticatedUser user, ExternalType obj
// Add implicit permissions
Collection<ObjectPermissionModel> implicitPermissions = getImplicitPermissions(user, model);
if (!implicitPermissions.isEmpty())
getPermissionMapper().insert(implicitPermissions);
getPermissionMapper().insert(implicitPermissions, getCaseSensitiveIdentifiers());

// Add any arbitrary attributes
if (model.hasArbitraryAttributes())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -494,15 +494,15 @@ public List<ConnectionRecord> retrieveHistory(String identifier,
if (user.isPrivileged() || user.getUser().getEffectivePermissions().getSystemPermissions().hasPermission(SystemPermission.Type.AUDIT))
searchResults = connectionRecordMapper.search(identifier,
recordIdentifier, requiredContents, sortPredicates, limit,
environment.getCaseSensitiveUsernames());
user.isCaseSensitive());

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

return getObjectInstances(searchResults);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,12 @@ public void createPermissions(ModeledAuthenticatedUser user,
// Create permissions only if user has permission to do so
if (canAlterPermissions(user, targetEntity, permissions)) {

boolean caseSensitive = getCaseSensitiveIdentifiers();

batchPermissionUpdates(permissions, permissionSubset -> {
Collection<ObjectPermissionModel> models = getModelInstances(
targetEntity, permissionSubset);
getPermissionMapper().insert(models);
getPermissionMapper().insert(models, caseSensitive);
});

return;
Expand All @@ -156,10 +158,12 @@ public void deletePermissions(ModeledAuthenticatedUser user,
// Delete permissions only if user has permission to do so
if (canAlterPermissions(user, targetEntity, permissions)) {

boolean caseSensitive = getCaseSensitiveIdentifiers();

batchPermissionUpdates(permissions, permissionSubset -> {
Collection<ObjectPermissionModel> models = getModelInstances(
targetEntity, permissionSubset);
getPermissionMapper().delete(models);
getPermissionMapper().delete(models, caseSensitive);
});

return;
Expand All @@ -179,7 +183,7 @@ public boolean hasPermission(ModeledAuthenticatedUser user,
// Retrieve permissions only if allowed
if (canReadPermissions(user, targetEntity))
return getPermissionMapper().selectOne(targetEntity.getModel(),
type, identifier, effectiveGroups) != null;
type, identifier, effectiveGroups, getCaseSensitiveIdentifiers()) != null;

// User cannot read this entity's permissions
throw new GuacamoleSecurityException("Permission denied.");
Expand All @@ -205,7 +209,7 @@ public Collection<String> retrieveAccessibleIdentifiers(ModeledAuthenticatedUser
if (canReadPermissions(user, targetEntity))
return getPermissionMapper().selectAccessibleIdentifiers(
targetEntity.getModel(), permissions, identifiers,
effectiveGroups);
effectiveGroups, getCaseSensitiveIdentifiers());

// User cannot read this entity's permissions
throw new GuacamoleSecurityException("Permission denied.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,10 @@ public Set<PermissionType> retrievePermissions(ModeledAuthenticatedUser user,

// Retrieve permissions only if allowed
if (canReadPermissions(user, targetEntity))
return getPermissionInstances(getPermissionMapper().select(targetEntity.getModel(), effectiveGroups));
return getPermissionInstances(getPermissionMapper().select(
targetEntity.getModel(),
effectiveGroups,
getCaseSensitiveIdentifiers()));

// User cannot read this entity's permissions
throw new GuacamoleSecurityException("Permission denied.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ public interface ObjectPermissionMapper extends PermissionMapper<ObjectPermissio
* 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
* "true" if identifiers should be treated as case-sensitive, otherwise
* "false".
*
* @return
* The requested permission, or null if no such permission is granted
Expand All @@ -56,7 +60,8 @@ public interface ObjectPermissionMapper extends PermissionMapper<ObjectPermissio
ObjectPermissionModel selectOne(@Param("entity") EntityModel entity,
@Param("type") ObjectPermission.Type type,
@Param("identifier") String identifier,
@Param("effectiveGroups") Collection<String> effectiveGroups);
@Param("effectiveGroups") Collection<String> effectiveGroups,
@Param("caseSensitive") boolean caseSensitive);

/**
* Retrieves the subset of the given identifiers for which the given entity
Expand All @@ -79,6 +84,10 @@ ObjectPermissionModel selectOne(@Param("entity") EntityModel entity,
* 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
* "true" if identifiers should be treated as case-sensitive, otherwise
* "false".
*
* @return
* A collection containing the subset of identifiers for which at least
Expand All @@ -87,6 +96,7 @@ ObjectPermissionModel selectOne(@Param("entity") EntityModel entity,
Collection<String> selectAccessibleIdentifiers(@Param("entity") EntityModel entity,
@Param("permissions") Collection<ObjectPermission.Type> permissions,
@Param("identifiers") Collection<String> identifiers,
@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 @@ -43,35 +43,50 @@ public interface PermissionMapper<PermissionType> {
* 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
* "true" if identifiers should be treated as case-sensitive, otherwise
* "false".
*
* @return
* All permissions associated with the given entity.
*/
Collection<PermissionType> select(@Param("entity") EntityModel entity,
@Param("effectiveGroups") Collection<String> effectiveGroups);
@Param("effectiveGroups") Collection<String> effectiveGroups,
@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
* "true" if identifiers should be treated as case-sensitive, otherwise
* "false".
*
* @return
* The number of rows inserted.
*/
int insert(@Param("permissions") Collection<PermissionType> permissions);
int insert(@Param("permissions") Collection<PermissionType> permissions,
@Param("caseSensitive") boolean caseSensitive);

/**
* 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
* "true" if identifiers should be treated as case-sensitive, otherwise
* "false".
*
* @return
* The number of rows deleted.
*/
int delete(@Param("permissions") Collection<PermissionType> permissions);
int delete(@Param("permissions") Collection<PermissionType> permissions,
@Param("caseSensitive") boolean caseSensitive);

}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,24 @@
public interface PermissionService<PermissionSetType extends PermissionSet<PermissionType>,
PermissionType extends Permission> {

/**
* Return "true" if identifiers 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 related to
* case-sensitivity.
*/
default boolean getCaseSensitiveIdentifiers() throws GuacamoleException {

// By default identifiers are case-insensitive.
return false;
}

/**
* Returns a permission set that can be used to retrieve and manipulate the
* permissions of the given entity.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,13 @@ public void createPermissions(ModeledAuthenticatedUser user,
// system permissions
if (user.isPrivileged()) {

// Pull identifier case sensitivity
boolean caseSensitive = getCaseSensitiveIdentifiers();

batchPermissionUpdates(permissions, permissionSubset -> {
Collection<SystemPermissionModel> models = getModelInstances(
targetEntity, permissionSubset);
systemPermissionMapper.insert(models);
systemPermissionMapper.insert(models, caseSensitive);
});

return;
Expand All @@ -125,10 +128,13 @@ public void deletePermissions(ModeledAuthenticatedUser user,
if (user.getUser().getIdentifier().equals(targetEntity.getIdentifier()))
throw new GuacamoleUnsupportedException("Removing your own administrative permissions is not allowed.");

// Pull case sensitivity
boolean caseSensitive = getCaseSensitiveIdentifiers();

batchPermissionUpdates(permissions, permissionSubset -> {
Collection<SystemPermissionModel> models = getModelInstances(
targetEntity, permissionSubset);
systemPermissionMapper.delete(models);
systemPermissionMapper.delete(models, caseSensitive);
});

return;
Expand Down
Loading
Loading