From 16ab5206648ffc1d5adc426ce184ee8954a515db Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 24 Nov 2024 23:00:31 -0800 Subject: [PATCH] GUACAMOLE-1239: Refactor away need for isCaseSensitive() function of Identifiable. --- .../auth/jdbc/base/EntityService.java | 22 +-- .../AbstractGuacamoleTunnelService.java | 15 +- .../jdbc/user/ModeledAuthenticatedUser.java | 5 - .../guacamole/auth/jdbc/user/ModeledUser.java | 48 ++--- .../guacamole/auth/jdbc/user/UserService.java | 3 +- .../auth/jdbc/usergroup/ModeledUserGroup.java | 59 +++--- .../auth/jdbc/usergroup/UserGroupService.java | 3 +- .../environment/DefaultEnvironment.java | 170 ++++++++++++++++++ .../environment/DelegatingEnvironment.java | 2 +- .../guacamole/environment/Environment.java | 80 ++------- .../net/auth/AbstractAuthenticatedUser.java | 58 +++--- .../net/auth/AbstractIdentifiable.java | 77 +++++--- .../guacamole/net/auth/AbstractUser.java | 44 ++++- .../guacamole/net/auth/AbstractUserGroup.java | 57 +++--- .../guacamole/net/auth/DelegatingUser.java | 5 - .../guacamole/net/auth/Identifiable.java | 13 -- .../guacamole/properties/CaseSensitivity.java | 80 ++++++++- 17 files changed, 464 insertions(+), 277 deletions(-) create mode 100644 guacamole-ext/src/main/java/org/apache/guacamole/environment/DefaultEnvironment.java diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/EntityService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/EntityService.java index d1f9b5ca1a..2dcf54e1b9 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/EntityService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/EntityService.java @@ -22,13 +22,10 @@ import com.google.inject.Inject; import java.util.Collection; import java.util.Set; -import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.auth.jdbc.JDBCEnvironment; import org.apache.guacamole.properties.CaseSensitivity; import org.apache.ibatis.session.SqlSession; import org.mybatis.guice.transactional.Transactional; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Service which provides convenience methods for creating, retrieving, and @@ -36,11 +33,6 @@ */ public class EntityService { - /** - * The Logger for this class. - */ - private static final Logger LOGGER = LoggerFactory.getLogger(EntityService.class); - /** * The Guacamole server environment. */ @@ -85,18 +77,8 @@ public class EntityService { public Set retrieveEffectiveGroups(ModeledPermissions entity, Collection effectiveGroups) { - CaseSensitivity caseSensitivity = CaseSensitivity.ENABLED; - try { - caseSensitivity = environment.getCaseSensitivity(); - } - catch (GuacamoleException e) { - LOGGER.warn("Unable to retrieve configuration setting for group " - + "name case sensitivity: {}. Group names will be treated " - + "as case-sensitive.", e.getMessage()); - LOGGER.debug("An exception was caught while trying to get group name" - + "case sensitivity configuration.", e); - } - + CaseSensitivity caseSensitivity = environment.getCaseSensitivity(); + // Retrieve the effective user groups of the given entity, recursively if possible boolean recursive = environment.isRecursiveQuerySupported(sqlSession); Set identifiers = entityMapper.selectEffectiveGroupIdentifiers( diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java index 18525723af..260068999d 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java @@ -638,21 +638,8 @@ private List getBalancedConnections(ModeledAuthenticatedUser if (connectionGroup.isSessionAffinityEnabled()) identifiers = getPreferredConnections(user, identifiers); - CaseSensitivity caseSensitivity = CaseSensitivity.ENABLED; - try { - caseSensitivity = environment.getCaseSensitivity(); - } - catch (GuacamoleException e) { - logger.warn("Error trying to retrieve case sensitivity configuration: {}." - + "Both usernames and group names will be treated as case-" - + "sensitive.", e.getMessage()); - logger.debug("An exception was received while trying to retrieve the " - + "case sensitivity configuration.", e); - } - // Retrieve all children - Collection models = connectionMapper.select(identifiers, - caseSensitivity); + Collection models = connectionMapper.select(identifiers, environment.getCaseSensitivity()); List connections = new ArrayList(models.size()); // Convert each retrieved model to a modeled connection diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledAuthenticatedUser.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledAuthenticatedUser.java index 6c5ca16f4d..5fd209a683 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledAuthenticatedUser.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledAuthenticatedUser.java @@ -195,9 +195,4 @@ public boolean isPrivileged() throws GuacamoleException { return getUser().isPrivileged(); } - @Override - public boolean isCaseSensitive() { - return user.isCaseSensitive(); - } - } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUser.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUser.java index 8cd74905b9..8bf7acb698 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUser.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUser.java @@ -36,7 +36,6 @@ import org.apache.guacamole.auth.jdbc.security.PasswordEncryptionService; import org.apache.guacamole.auth.jdbc.security.SaltService; import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.auth.jdbc.JDBCEnvironment; import org.apache.guacamole.auth.jdbc.base.ModeledPermissions; import org.apache.guacamole.form.BooleanField; import org.apache.guacamole.form.DateField; @@ -51,6 +50,7 @@ import org.apache.guacamole.net.auth.Permissions; import org.apache.guacamole.net.auth.RelatedObjectSet; import org.apache.guacamole.net.auth.User; +import org.apache.guacamole.properties.CaseSensitivity; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -151,12 +151,6 @@ public class ModeledUser extends ModeledPermissions implements User { TIMEZONE_ATTRIBUTE_NAME ))); - /** - * Service for managing users. - */ - @Inject - private UserService userService; - /** * Service for hashing passwords. */ @@ -181,13 +175,6 @@ public class ModeledUser extends ModeledPermissions implements User { */ @Inject private Provider userRecordSetProvider; - - /** - * The environment associated with this instance of the JDBC authentication - * module. - */ - @Inject - private JDBCEnvironment environment; /** * Whether attributes which control access restrictions should be exposed @@ -195,6 +182,11 @@ public class ModeledUser extends ModeledPermissions implements User { */ private boolean exposeRestrictedAttributes = false; + /** + * Whether usernames should be considered case-sensitive. + */ + private boolean caseSensitive = true; + /** * Initializes this ModeledUser, associating it with the current * authenticated user and populating it with data from the given user @@ -212,9 +204,10 @@ public class ModeledUser extends ModeledPermissions implements User { * setAttributes(). */ public void init(ModeledAuthenticatedUser currentUser, UserModel model, - boolean exposeRestrictedAttributes) { + boolean exposeRestrictedAttributes, boolean caseSensitive) { super.init(currentUser, model); this.exposeRestrictedAttributes = exposeRestrictedAttributes; + this.caseSensitive = caseSensitive; } /** @@ -249,6 +242,16 @@ public void setModel(UserModel model) { } + @Override + public String getIdentifier() { + return CaseSensitivity.canonicalize(super.getIdentifier(), caseSensitive); + } + + @Override + public void setIdentifier(String identifier) { + super.setIdentifier(CaseSensitivity.canonicalize(identifier, caseSensitive)); + } + @Override public String getPassword() { return password; @@ -789,19 +792,4 @@ public boolean isSkeleton() { return (getModel().getEntityID() == null); } - @Override - public boolean isCaseSensitive() { - try { - return environment.getCaseSensitivity().caseSensitiveUsernames(); - } - catch (GuacamoleException e) { - logger.error("Failed to retrieve the configuration for case sensitivity: {}. " - + "Username comparisons will be case-sensitive.", - e.getMessage()); - logger.debug("An exception was caught when attempting to retrieve the " - + "case sensitivity configuration.", e); - return true; - } - } - } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java index 4ffa850ac6..c85d78bc7f 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java @@ -197,7 +197,8 @@ else if (currentUser != null) // Produce ModeledUser exposing only those attributes for which the // current user has permission ModeledUser user = userProvider.get(); - user.init(currentUser, model, exposeRestrictedAttributes); + user.init(currentUser, model, exposeRestrictedAttributes, + environment.getCaseSensitivity().caseSensitiveUsernames()); return user; } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/ModeledUserGroup.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/ModeledUserGroup.java index dcdb5979e5..cf2c1db7e6 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/ModeledUserGroup.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/ModeledUserGroup.java @@ -21,23 +21,17 @@ import com.google.inject.Inject; import com.google.inject.Provider; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; import java.util.Map; import java.util.Set; import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.auth.jdbc.JDBCEnvironment; import org.apache.guacamole.auth.jdbc.base.ModeledPermissions; import org.apache.guacamole.auth.jdbc.user.ModeledAuthenticatedUser; -import org.apache.guacamole.form.BooleanField; -import org.apache.guacamole.form.Field; import org.apache.guacamole.form.Form; import org.apache.guacamole.net.auth.RelatedObjectSet; import org.apache.guacamole.net.auth.UserGroup; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.apache.guacamole.properties.CaseSensitivity; /** * An implementation of the UserGroup object which is backed by a database model. @@ -45,11 +39,6 @@ public class ModeledUserGroup extends ModeledPermissions implements UserGroup { - /** - * The Logger for this class. - */ - private static final Logger LOGGER = LoggerFactory.getLogger(ModeledUserGroup.class); - /** * All possible attributes of user groups organized as individual, * logical forms. @@ -83,19 +72,17 @@ public class ModeledUserGroup extends ModeledPermissions @Inject private Provider memberUserGroupSetProvider; - /** - * The environment associated with this instance of the JDBC authentication - * module. - */ - @Inject - private JDBCEnvironment environment; - /** * Whether attributes which control access restrictions should be exposed * via getAttributes() or allowed to be set via setAttributes(). */ private boolean exposeRestrictedAttributes = false; + /** + * Whether group names should be considered case-sensitive. + */ + private boolean caseSensitive = true; + /** * Initializes this ModeledUserGroup, associating it with the current * authenticated user and populating it with data from the given user group @@ -111,13 +98,28 @@ public class ModeledUserGroup extends ModeledPermissions * Whether attributes which control access restrictions should be * exposed via getAttributes() or allowed to be set via * setAttributes(). + * + * @param caseSensitive + * true if group names should be considered case-sensitive, false + * otherwise. */ public void init(ModeledAuthenticatedUser currentUser, UserGroupModel model, - boolean exposeRestrictedAttributes) { + boolean exposeRestrictedAttributes, boolean caseSensitive) { super.init(currentUser, model); this.exposeRestrictedAttributes = exposeRestrictedAttributes; + this.caseSensitive = caseSensitive; } - + + @Override + public String getIdentifier() { + return CaseSensitivity.canonicalize(super.getIdentifier(), caseSensitive); + } + + @Override + public void setIdentifier(String identifier) { + super.setIdentifier(CaseSensitivity.canonicalize(identifier, caseSensitive)); + } + @Override public boolean isDisabled() { return getModel().isDisabled(); @@ -203,19 +205,4 @@ public RelatedObjectSet getMemberUserGroups() throws GuacamoleException { return memberUserGroupSet; } - @Override - public boolean isCaseSensitive() { - try { - return environment.getCaseSensitivity().caseSensitiveGroupNames(); - } - catch (GuacamoleException e) { - LOGGER.error("Error while retrieving case sensitivity configuration: {}. " - + "Group names comparisons will be case-sensitive.", - e.getMessage()); - LOGGER.debug("An exception was caught when attempting to retrieve the " - + "case sensitivity configuration.", e); - return true; - } - } - } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/UserGroupService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/UserGroupService.java index 525233fc44..6e4b4434cc 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/UserGroupService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/UserGroupService.java @@ -101,7 +101,8 @@ protected ModeledUserGroup getObjectInstance(ModeledAuthenticatedUser currentUse // Produce ModeledUserGroup exposing only those attributes for which the // current user has permission ModeledUserGroup group = userGroupProvider.get(); - group.init(currentUser, model, exposeRestrictedAttributes); + group.init(currentUser, model, exposeRestrictedAttributes, + environment.getCaseSensitivity().caseSensitiveGroupNames()); return group; } diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/environment/DefaultEnvironment.java b/guacamole-ext/src/main/java/org/apache/guacamole/environment/DefaultEnvironment.java new file mode 100644 index 0000000000..a3db06c190 --- /dev/null +++ b/guacamole-ext/src/main/java/org/apache/guacamole/environment/DefaultEnvironment.java @@ -0,0 +1,170 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.environment; + +import org.apache.guacamole.properties.GuacamoleProperties; +import java.util.Collection; +import java.util.Collections; +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.GuacamoleUnsupportedException; +import org.apache.guacamole.properties.CaseSensitivity; +import org.apache.guacamole.properties.GuacamoleProperty; +import org.apache.guacamole.properties.StringGuacamoleProperty; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Internal implementation of Environment that provides the default + * implementations for any functions that are added to the Environment + * interface. This is primarily necessary to allow those default implementations + * to log warnings or informational messages without needing to repeatedly + * recreate the Logger. + */ +class DefaultEnvironment extends DelegatingEnvironment { + + /** + * Logger for this class. + */ + private static final Logger logger = LoggerFactory.getLogger(DefaultEnvironment.class); + + /** + * Creates a new DefaultEnvironment that provides default implementations + * for functions that may not be implemented by the given Environment + * implementation. The versions provided by DefaultEnvironment must still + * be manually called by actual public default functions on + * Environment to have any effect. + * + * @param environment + * The environment that may not provide implementations for all + * functions defined by the Environment interface. + */ + protected DefaultEnvironment(Environment environment) { + super(environment); + } + + @Override + public Collection getPropertyCollection( + GuacamoleProperty property) throws GuacamoleException { + + /* Pull the given property as a string. */ + StringGuacamoleProperty stringProperty = new StringGuacamoleProperty() { + + @Override + public String getName() { return property.getName(); } + + }; + + /* Parse the string to a Collection of the desired type. */ + return property.parseValueCollection(getProperty(stringProperty)); + + } + + @Override + public Collection getPropertyCollection( + GuacamoleProperty property, Type defaultValue) + throws GuacamoleException { + + /* Pull the given property as a string. */ + StringGuacamoleProperty stringProperty = new StringGuacamoleProperty() { + + @Override + public String getName() { return property.getName(); } + + }; + + /* Check the value and return the default if null. */ + String stringValue = getProperty(stringProperty); + if (stringValue == null) + return Collections.singletonList(defaultValue); + + /* Parse the string and return the collection. */ + return property.parseValueCollection(stringValue); + + } + + @Override + public Collection getPropertyCollection( + GuacamoleProperty property, Collection defaultValue) + throws GuacamoleException { + + /* Pull the given property as a string. */ + StringGuacamoleProperty stringProperty = new StringGuacamoleProperty() { + + @Override + public String getName() { return property.getName(); } + + }; + + /* Check the value and return the default if null. */ + String stringValue = getProperty(stringProperty); + if (stringValue == null) + return defaultValue; + + /* Parse the string and return the collection. */ + return property.parseValueCollection(stringValue); + + } + + @Override + public Collection getRequiredPropertyCollection( + GuacamoleProperty property) throws GuacamoleException { + + /* Pull the given property as a string. */ + StringGuacamoleProperty stringProperty = new StringGuacamoleProperty() { + + @Override + public String getName() { return property.getName(); } + + }; + + /* Parse the string to a Collection of the desired type. */ + return property.parseValueCollection(getRequiredProperty(stringProperty)); + + } + + @Override + public void addGuacamoleProperties(GuacamoleProperties properties) + throws GuacamoleException { + throw new GuacamoleUnsupportedException(String.format("%s does not " + + "support dynamic definition of Guacamole properties.", + getClass())); + } + + @Override + public CaseSensitivity getCaseSensitivity() { + + try { + return DefaultEnvironment.this.getProperty(CASE_SENSITIVITY, CaseSensitivity.ENABLED); + } + catch (GuacamoleException e) { + + logger.error("Defaulting to case-sensitive handling of " + + "usernames and group names as the desired case " + + "sensitivity configuration could not be read: {}", + e.getMessage()); + + logger.debug("Error reading case sensitivity configuration.", e); + return CaseSensitivity.ENABLED; + + } + + } + +} diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/environment/DelegatingEnvironment.java b/guacamole-ext/src/main/java/org/apache/guacamole/environment/DelegatingEnvironment.java index 593d7f74b3..8b3cf9c1aa 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/environment/DelegatingEnvironment.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/environment/DelegatingEnvironment.java @@ -116,7 +116,7 @@ public void addGuacamoleProperties(GuacamoleProperties properties) throws Guacam } @Override - public CaseSensitivity getCaseSensitivity() throws GuacamoleException { + public CaseSensitivity getCaseSensitivity() { return environment.getCaseSensitivity(); } diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/environment/Environment.java b/guacamole-ext/src/main/java/org/apache/guacamole/environment/Environment.java index 572ad3d2e8..01d81bb074 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/environment/Environment.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/environment/Environment.java @@ -24,6 +24,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Map; +import java.util.function.Supplier; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleUnsupportedException; import org.apache.guacamole.net.auth.GuacamoleProxyConfiguration; @@ -34,6 +35,8 @@ import org.apache.guacamole.properties.IntegerGuacamoleProperty; import org.apache.guacamole.properties.StringGuacamoleProperty; import org.apache.guacamole.protocols.ProtocolInfo; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * The environment of an arbitrary Guacamole instance, describing available @@ -82,7 +85,7 @@ public interface Environment { @Override public String getName() { return "case-sensitivity"; } - + }; /** @@ -183,18 +186,7 @@ public Type getProperty(GuacamoleProperty property, */ public default Collection getPropertyCollection( GuacamoleProperty property) throws GuacamoleException { - - /* Pull the given property as a string. */ - StringGuacamoleProperty stringProperty = new StringGuacamoleProperty() { - - @Override - public String getName() { return property.getName(); } - - }; - - /* Parse the string to a Collection of the desired type. */ - return property.parseValueCollection(getProperty(stringProperty)); - + return new DefaultEnvironment(this).getPropertyCollection(property); } /** @@ -227,23 +219,7 @@ public default Collection getPropertyCollection( public default Collection getPropertyCollection( GuacamoleProperty property, Type defaultValue) throws GuacamoleException { - - /* Pull the given property as a string. */ - StringGuacamoleProperty stringProperty = new StringGuacamoleProperty() { - - @Override - public String getName() { return property.getName(); } - - }; - - /* Check the value and return the default if null. */ - String stringValue = getProperty(stringProperty); - if (stringValue == null) - return Collections.singletonList(defaultValue); - - /* Parse the string and return the collection. */ - return property.parseValueCollection(stringValue); - + return new DefaultEnvironment(this).getPropertyCollection(property, defaultValue); } /** @@ -276,23 +252,7 @@ public default Collection getPropertyCollection( public default Collection getPropertyCollection( GuacamoleProperty property, Collection defaultValue) throws GuacamoleException { - - /* Pull the given property as a string. */ - StringGuacamoleProperty stringProperty = new StringGuacamoleProperty() { - - @Override - public String getName() { return property.getName(); } - - }; - - /* Check the value and return the default if null. */ - String stringValue = getProperty(stringProperty); - if (stringValue == null) - return defaultValue; - - /* Parse the string and return the collection. */ - return property.parseValueCollection(stringValue); - + return new DefaultEnvironment(this).getPropertyCollection(property, defaultValue); } /** @@ -335,18 +295,7 @@ public Type getRequiredProperty(GuacamoleProperty property) */ public default Collection getRequiredPropertyCollection( GuacamoleProperty property) throws GuacamoleException { - - /* Pull the given property as a string. */ - StringGuacamoleProperty stringProperty = new StringGuacamoleProperty() { - - @Override - public String getName() { return property.getName(); } - - }; - - /* Parse the string to a Collection of the desired type. */ - return property.parseValueCollection(getRequiredProperty(stringProperty)); - + return new DefaultEnvironment(this).getRequiredPropertyCollection(property); } /** @@ -378,11 +327,9 @@ public GuacamoleProxyConfiguration getDefaultGuacamoleProxyConfiguration() */ public default void addGuacamoleProperties(GuacamoleProperties properties) throws GuacamoleException { - throw new GuacamoleUnsupportedException(String.format("%s does not " - + "support dynamic definition of Guacamole properties.", - getClass())); + new DefaultEnvironment(this).addGuacamoleProperties(properties); } - + /** * Returns the case sensitivity configuration for Guacamole as defined * in guacamole.properties, or the default of enabling case sensitivity @@ -391,12 +338,9 @@ public default void addGuacamoleProperties(GuacamoleProperties properties) * @return * The case sensitivity setting as configured in guacamole.properties, * or the default of enabling case sensitivity. - * - * @throws GuacamoleException - * If guacamole.properties cannot be read or parsed. */ - public default CaseSensitivity getCaseSensitivity() throws GuacamoleException { - return getProperty(CASE_SENSITIVITY, CaseSensitivity.ENABLED); + public default CaseSensitivity getCaseSensitivity() { + return new DefaultEnvironment(this).getCaseSensitivity(); } } diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractAuthenticatedUser.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractAuthenticatedUser.java index 0fa477ccab..10107c2f5f 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractAuthenticatedUser.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractAuthenticatedUser.java @@ -21,11 +21,8 @@ import java.util.Collections; import java.util.Set; -import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.environment.Environment; import org.apache.guacamole.environment.LocalEnvironment; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Basic implementation of an AuthenticatedUser which uses the username to @@ -35,16 +32,42 @@ public abstract class AbstractAuthenticatedUser extends AbstractIdentifiable implements AuthenticatedUser { /** - * The logger for this class. + * Creates a new AbstractAuthenticatedUser that considers usernames to be + * case-sensitive or case-insensitive based on the provided case sensitivity + * flag. + * + * @param caseSensitive + * true if usernames should be considered case-sensitive, false + * otherwise. */ - private static final Logger LOGGER = LoggerFactory.getLogger(AbstractAuthenticatedUser.class); - + public AbstractAuthenticatedUser(boolean caseSensitive) { + super(caseSensitive); + } + + /** + * Creates a new AbstractAuthenticatedUser that considers usernames to be + * case-sensitive or case-insensitive based on the case sensitivity setting + * of the provided {@link Environment}, as returned by + * {@link Environment#getCaseSensitivity()}. + * + * @param environment + * The Environment that should determine whether this + * AbstractAuthenticatedUser considers usernames to be case-sensitive. + */ + public AbstractAuthenticatedUser(Environment environment) { + this(environment.getCaseSensitivity().caseSensitiveUsernames()); + } + /** - * The server environment in which this Guacamole Client instance is - * running. + * Creates a new AbstractAuthenticatedUser that considers usernames to be + * case-sensitive or case-insensitive based on the case sensitivity setting + * of an instance of {@link LocalEnvironment}, as returned by + * {@link LocalEnvironment#getCaseSensitivity()}. */ - private final Environment environment = LocalEnvironment.getInstance(); - + public AbstractAuthenticatedUser() { + this(LocalEnvironment.getInstance()); + } + // Prior functionality now resides within AbstractIdentifiable @Override @@ -52,21 +75,6 @@ public Set getEffectiveUserGroups() { return Collections.emptySet(); } - @Override - public boolean isCaseSensitive() { - try { - return environment.getCaseSensitivity().caseSensitiveUsernames(); - } - catch (GuacamoleException e) { - LOGGER.error("Failed to retrieve the configuration for case sensitivity: {}. " - + "Username comparisons will be case-sensitive.", - e.getMessage()); - LOGGER.debug("An exception was caught when attempting to retrieve the " - + "case sensitivity configuration.", e); - return true; - } - } - @Override public void invalidate() { // Nothing to invalidate diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractIdentifiable.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractIdentifiable.java index 73728b4ab9..eed7ebeefb 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractIdentifiable.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractIdentifiable.java @@ -19,10 +19,17 @@ package org.apache.guacamole.net.auth; +import org.apache.guacamole.properties.CaseSensitivity; + /** * Abstract implementation of Identifiable which provides equals() and * hashCode() implementations which use the identifier to determine equality. - * The identifier comparison is case-sensitive. + * The identifier comparison is case-sensitive unless configured otherwise via + * the {@link AbstractIdentifiable#AbstractIdentifiable(boolean)} constructor. + * + * If using case-insensitive identifiers, any identifiers that are retrieved + * from or assigned to this object will first be canonicalized to a + * case-insensitive form using {@link CaseSensitivity#canonicalize(java.lang.String, boolean)}. */ public abstract class AbstractIdentifiable implements Identifiable { @@ -31,32 +38,58 @@ public abstract class AbstractIdentifiable implements Identifiable { */ private String identifier; + /** + * Whether this object's identifier should be compared in a case-sensitive + * manner. If NOT case-sensitive, the identifier will be transformed into a + * canonical, case-insensitive form before use, including during assignment + * and retrieval. This affects the behavior of getIdentifier() and + * setIdentifier(). + */ + private final boolean caseSensitive; + + /** + * Creates a new AbstractIdentifiable that compares identifiers according + * to the provided case sensitivity flag. If using case-insensitive + * identifiers, any identifiers that are retrieved from or assigned to this + * object will first be canonicalized to a case-insensitive form using + * {@link CaseSensitivity#canonicalize(java.lang.String, boolean)}. + * + * @param caseSensitive + * true if identifiers should be compared in a case-sensitive manner, + * false otherwise. + */ + public AbstractIdentifiable(boolean caseSensitive) { + this.caseSensitive = caseSensitive; + } + + /** + * Creates a new AbstractIdentifiable that compares identifiers in a + * case-sensitive manner. This is equivalent to invoking {@link #AbstractIdentifiable(boolean)} + * with the case sensitivity flag set to true. + */ + public AbstractIdentifiable() { + this(true); + } + @Override public String getIdentifier() { - if (identifier == null || isCaseSensitive()) - return identifier; - - return identifier.toLowerCase(); + return CaseSensitivity.canonicalize(identifier, caseSensitive); } @Override public void setIdentifier(String identifier) { - if (isCaseSensitive() || identifier == null) - this.identifier = identifier; - else - this.identifier = identifier.toLowerCase(); + this.identifier = CaseSensitivity.canonicalize(identifier, caseSensitive); } @Override public int hashCode() { - if (identifier == null) + String thisIdentifier = getIdentifier(); + if (thisIdentifier == null) return 0; - if (isCaseSensitive()) - return identifier.hashCode(); - - return identifier.toLowerCase().hashCode(); + return thisIdentifier.hashCode(); + } @Override @@ -66,20 +99,16 @@ public boolean equals(Object other) { if (other == null || getClass() != other.getClass()) return false; - // Get identifier of other object + // Get identifiers of objects being compared + String thisIdentifier = getIdentifier(); String otherIdentifier = ((AbstractIdentifiable) other).getIdentifier(); // If null, equal only if this identifier is null if (otherIdentifier == null) - return identifier == null; - - // If either this identifier or the one we're comparing to is - // case-sensitive, evaluate with case sensitivity. - if (isCaseSensitive() || ((AbstractIdentifiable) other).isCaseSensitive()) - return otherIdentifier.equals(identifier); - - // Both identifiers can be evaluated in a case-insensitive manner. - return otherIdentifier.equalsIgnoreCase(identifier); + return thisIdentifier == null; + + // Otherwise, equal only if strings are identical + return otherIdentifier.equals(thisIdentifier); } diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractUser.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractUser.java index 472a3325c2..37f7af7023 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractUser.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractUser.java @@ -23,6 +23,8 @@ import java.util.Date; import java.util.Map; import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.environment.Environment; +import org.apache.guacamole.environment.LocalEnvironment; import org.apache.guacamole.net.auth.permission.ObjectPermissionSet; import org.apache.guacamole.net.auth.permission.SystemPermissionSet; @@ -40,6 +42,40 @@ public abstract class AbstractUser extends AbstractIdentifiable */ private String password; + /** + * Creates a new AbstractUser that considers usernames to be case-sensitive + * or case-insensitive based on the provided case sensitivity flag. + * + * @param caseSensitive + * true if usernames should be considered case-sensitive, false + * otherwise. + */ + public AbstractUser(boolean caseSensitive) { + super(caseSensitive); + } + + /** + * Creates a new AbstractUser that considers usernames to be case-sensitive + * or case-insensitive based on the case sensitivity setting of the provided + * {@link Environment}, as returned by {@link Environment#getCaseSensitivity()}. + * + * @param environment + * The Environment that should determine whether this AbstractUser + * considers usernames to be case-sensitive. + */ + public AbstractUser(Environment environment) { + this(environment.getCaseSensitivity().caseSensitiveUsernames()); + } + + /** + * Creates a new AbstractUser that considers usernames to be case-sensitive + * or case-insensitive based on the case sensitivity setting of an instance + * of {@link LocalEnvironment}, as returned by @link LocalEnvironment#getCaseSensitivity()}. + */ + public AbstractUser() { + this(LocalEnvironment.getInstance()); + } + @Override public String getPassword() { return password; @@ -50,14 +86,6 @@ public void setPassword(String password) { this.password = password; } - @Override - public boolean isCaseSensitive() { - - // In order to avoid causing incompatibility with other extensions, - // this class maintains case-sensitive comparisons. - return true; - } - /** * {@inheritDoc} * diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractUserGroup.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractUserGroup.java index 306d34f974..1de3aae7d4 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractUserGroup.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractUserGroup.java @@ -26,8 +26,6 @@ import org.apache.guacamole.environment.LocalEnvironment; import org.apache.guacamole.net.auth.permission.ObjectPermissionSet; import org.apache.guacamole.net.auth.permission.SystemPermissionSet; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Base implementation of UserGroup which provides default implementations of @@ -36,16 +34,42 @@ public abstract class AbstractUserGroup extends AbstractIdentifiable implements UserGroup { /** - * The logger for this class. + * Creates a new AbstractUserGroup that considers group names to be + * case-sensitive or case-insensitive based on the provided case + * sensitivity flag. + * + * @param caseSensitive + * true if group names should be considered case-sensitive, false + * otherwise. + */ + public AbstractUserGroup(boolean caseSensitive) { + super(caseSensitive); + } + + /** + * Creates a new AbstractUserGroup that considers group names to be + * case-sensitive or case-insensitive based on the case sensitivity setting + * of the provided {@link Environment}, as returned by + * {@link Environment#getCaseSensitivity()}. + * + * @param environment + * The Environment that should determine whether this AbstractUserGroup + * considers group names to be case-sensitive. */ - private static final Logger LOGGER = LoggerFactory.getLogger(AbstractUserGroup.class); - + public AbstractUserGroup(Environment environment) { + this(environment.getCaseSensitivity().caseSensitiveGroupNames()); + } + /** - * The server environment in which this Guacamole Client instance is - * running. + * Creates a new AbstractUserGroup that considers group names to be + * case-sensitive or case-insensitive based on the case sensitivity setting + * of an instance of {@link LocalEnvironment}, as returned by + * {@link LocalEnvironment#getCaseSensitivity()}. */ - private final Environment environment = LocalEnvironment.getInstance(); - + public AbstractUserGroup() { + this(LocalEnvironment.getInstance()); + } + /** * {@inheritDoc} * @@ -194,20 +218,5 @@ public RelatedObjectSet getMemberUsers() throws GuacamoleException { public RelatedObjectSet getMemberUserGroups() throws GuacamoleException { return RelatedObjectSet.EMPTY_SET; } - - @Override - public boolean isCaseSensitive() { - try { - return environment.getCaseSensitivity().caseSensitiveGroupNames(); - } - catch (GuacamoleException e) { - LOGGER.warn("Unable to retrieve server configuration, group names " - + "will default to case-sensitive."); - LOGGER.debug("Received an exception attempting to retrieve the " - + "property for group name case sensitivity, group names" - + "will be treated as case-sensitive.", e); - return true; - } - } } diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingUser.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingUser.java index 0777081f6c..9f2d93b23d 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingUser.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/DelegatingUser.java @@ -83,11 +83,6 @@ public boolean isDisabled() { return user.isDisabled(); } - @Override - public boolean isCaseSensitive() { - return user.isCaseSensitive(); - } - @Override public void setDisabled(boolean disabled) { user.setDisabled(disabled); diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Identifiable.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Identifiable.java index 0bfd6caf85..d32fec0636 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Identifiable.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Identifiable.java @@ -43,18 +43,5 @@ public interface Identifiable { * The identifier to assign. */ public void setIdentifier(String identifier); - - /** - * Whether or not this identifier should be evaluated in a case-sensitive - * manner. By default this returns true and the identifier will be - * evaluated in a case-sensitive manner. - * - * @return - * True if the comparisons of this identifier should be case-sensitive, - * otherwise false. - */ - default public boolean isCaseSensitive() { - return true; - } } diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/properties/CaseSensitivity.java b/guacamole-ext/src/main/java/org/apache/guacamole/properties/CaseSensitivity.java index 8c5ca6b623..15bc2431c8 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/properties/CaseSensitivity.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/properties/CaseSensitivity.java @@ -19,6 +19,7 @@ package org.apache.guacamole.properties; +import java.util.Locale; import org.apache.guacamole.properties.EnumGuacamoleProperty.PropertyValue; /** @@ -60,7 +61,19 @@ public enum CaseSensitivity { * Whether or not case sensitivity should be enabled for group names. */ private final boolean groupNames; - + + /** + * Creates a new CaseSensitivity value that represents a setting controlling + * whether usernames and group names are case-sensitive. + * + * @param usernames + * true if usernames should be considered case-sensitive, false if + * usernames should be considered case-insensitive. + * + * @param groupNames + * true if group names should be considered case-sensitive, false if + * group names should be considered case-insensitive. + */ CaseSensitivity(boolean usernames, boolean groupNames) { this.usernames = usernames; this.groupNames = groupNames; @@ -88,5 +101,68 @@ public boolean caseSensitiveUsernames() { public boolean caseSensitiveGroupNames() { return groupNames; } - + + /** + * Converts the given identifier into a canonical form which ensures simple + * verbatim string comparisons are consistent with the case sensitivity + * setting. If case-sensitive, identifiers are simply returned without + * modification. If case-insensitive, identifiers are converted to + * lowercase with respect to the {@link Locale#ROOT} locale. + * + * @param identifier + * The identifier to convert into a canonical form. + * + * @param caseSensitive + * Whether the given identifier is case-sensitive. + * + * @return + * The given identifier, transformed as necessary to ensure that + * verbatim string comparisons are consistent with the case sensitivity + * setting. + */ + public static String canonicalize(String identifier, boolean caseSensitive) { + + if (identifier == null) + return null; + + return caseSensitive ? identifier : identifier.toLowerCase(Locale.ROOT); + + } + + /** + * Canonicalizes the given username according to whether usernames are + * case-sensitive under this case sensitivity setting. This function is + * equivalent to manually invoking {@link #canonicalize(java.lang.String, boolean)} + * with the value of {@link #caseSensitiveUsernames()}. + * + * @param username + * The username to canonicalize. + * + * @return + * The given username, transformed as necessary to ensure that + * verbatim string comparisons are consistent with the case sensitivity + * setting for usernames. + */ + public String canonicalizeUsername(String username) { + return canonicalize(username, usernames); + } + + /** + * Canonicalizes the given group name according to whether group names are + * case-sensitive under this case sensitivity setting. This function is + * equivalent to manually invoking {@link #canonicalize(java.lang.String, boolean)} + * with the value of {@link #caseSensitiveGroupNames()}. + * + * @param groupName + * The group name to canonicalize. + * + * @return + * The given group name, transformed as necessary to ensure that + * verbatim string comparisons are consistent with the case sensitivity + * setting for group names. + */ + public String canonicalizeGroupName(String groupName) { + return canonicalize(groupName, groupNames); + } + }