mike-jumper commented on code in PR #1028:
URL: https://github.com/apache/guacamole-client/pull/1028#discussion_r1835316554
##########
guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractAuthenticatedUser.java:
##########
@@ -29,13 +34,39 @@
public abstract class AbstractAuthenticatedUser extends AbstractIdentifiable
implements AuthenticatedUser {
+ /**
+ * The logger for this class.
+ */
+ private static final Logger LOGGER =
LoggerFactory.getLogger(AbstractAuthenticatedUser.class);
+
+ /**
+ * The server environment in which this Guacamole Client instance is
+ * running.
+ */
+ private final Environment environment = LocalEnvironment.getInstance();
+
// Prior functionality now resides within AbstractIdentifiable
@Override
public Set<String> getEffectiveUserGroups() {
return Collections.<String>emptySet();
}
+ @Override
+ public boolean isCaseSensitive() {
+ try {
+ return environment.getCaseSensitivity().caseSensitiveUsernames();
+ }
+ catch (GuacamoleException e) {
+ LOGGER.warn("Exception attempting to read the Guacamole
configuration, "
+ + "usernames will be treated as case-sensitive.",
e.getMessage());
Review Comment:
`{}` placeholder for `e.getMessage()` missing here.
##########
guacamole-ext/src/main/java/org/apache/guacamole/environment/Environment.java:
##########
@@ -70,16 +72,12 @@ public interface Environment {
};
- /**
- * A property that configures whether or not Guacamole will take case
- * into account when comparing and processing usernames.
- */
- public static final BooleanGuacamoleProperty CASE_SENSITIVE_USERNAMES =
- new BooleanGuacamoleProperty() {
-
+ public static final EnumGuacamoleProperty<CaseSensitivity>
CASE_SENSITIVITY =
Review Comment:
Please document.
##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/EntityService.java:
##########
@@ -76,9 +85,22 @@ public class EntityService {
public Set<String> retrieveEffectiveGroups(ModeledPermissions<? extends
EntityModel> entity,
Collection<String> 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("Got exception while trying to get group name case-"
+ + "sensitivity configuration.", e);
Review Comment:
Unlike "case-sensitive" and "case-insensitive", "case sensitivity" shouldn't
have a hyphen.
##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/ModeledUserGroup.java:
##########
@@ -187,5 +202,20 @@ public RelatedObjectSet getMemberUserGroups() throws
GuacamoleException {
memberUserGroupSet.init(getCurrentUser(), this);
return memberUserGroupSet;
}
+
+ @Override
+ public boolean isCaseSensitive() {
+ try {
+ return environment.getCaseSensitivity().caseSensitiveGroupNames();
+ }
+ catch (GuacamoleException e) {
+ LOGGER.warn("Unable to retrieve environment setting for "
+ + "case-sensitivity, group names will default to being "
+ + "case-sensitive.", e.getMessage());
Review Comment:
`{}` placeholder for `e.getMessage()` missing here.
##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUser.java:
##########
@@ -792,11 +792,12 @@ public boolean isSkeleton() {
@Override
public boolean isCaseSensitive() {
try {
- return environment.getCaseSensitiveUsernames();
+ return environment.getCaseSensitivity().caseSensitiveUsernames();
}
catch (GuacamoleException e) {
- logger.error("Failed to retrieve the configuration for
case-sensitive usernames: {}."
- + " Usernames comparisons will be case-sensitive.",
e.getMessage());
+ logger.error("Failed to retrieve the configuration for
case-sensitivity, "
+ + "username comparisons will be case-sensitive.",
+ e.getMessage());
Review Comment:
The `{}` placeholder that will receive `e.getMessage()` is missing here.
##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java:
##########
@@ -637,8 +638,20 @@ private List<ModeledConnection>
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, "
+ + "usernames and group names will be treated as case-"
+ + "sensitive.", e);
Review Comment:
The `e` parameter here will result in the stack trace being included in the
logs. We should instead use a `{}` placeholder and `e.getMessage()` for
messages that are not at the debug level.
##########
guacamole-ext/src/main/java/org/apache/guacamole/properties/CaseSensitivity.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.properties;
+
+import org.apache.guacamole.properties.EnumGuacamoleProperty.PropertyValue;
+
+/**
+ * An enum that supports configuring various user and group case-sensitivity
+ * settings.
+ */
+public enum CaseSensitivity {
+
+ /**
+ * Case-sensitivity enabled for both usernames and group names.
+ */
+ @PropertyValue("enabled")
+ ENABLED(true, true),
+
+ /**
+ * Case-sensitivity enabled for usernames but disabled for group names.
+ */
+ @PropertyValue("usernames")
+ USERS(true, false),
+
+ /**
+ * Case-sensitivity disabled for usernames but enabled for group names.
+ */
+ @PropertyValue("groupnames")
Review Comment:
I think this should be `group-names` (since it's a "group name" and not a
"groupname").
##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/permission/PermissionService.java:
##########
@@ -43,6 +44,41 @@
public interface PermissionService<PermissionSetType extends
PermissionSet<PermissionType>,
PermissionType extends Permission> {
+ /**
+ * Return the current case-sensitivity setting, allowing the system to
+ * determine if usernames and/or group names should be treated as case-
+ * sensitive.
+ *
+ * @return
+ * The current case-sensitivity configuration.
+ *
+ * @throws GuacamoleException
+ * If an error occurs retrieving configuration information related to
+ * case-sensitivity.
+ */
+ default CaseSensitivity getCaseSensitivity() throws GuacamoleException {
+
+ // By default identifiers are case-sensitive.
+ return CaseSensitivity.ENABLED;
+ }
+
+ /**
+ * Return "true" if group names should be treated as case-sensitive,
+ * otherwise "false".
+ *
+ * @return
+ * "true" if group names should be treated as case-sensitive, otherwise
+ * "false".
+ *
+ * @throws GuacamoleException
+ * If an error occurs retrieving configuration information related to
+ * case-sensitivity.
+ */
+ default boolean getCaseSensitiveGroupNames() throws GuacamoleException {
+ // By default group names are case-sensitive.
+ return true;
+ }
Review Comment:
Is this function used? I don't see any calls in the diff.
--
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.
To unsubscribe, e-mail: dev-unsubscr...@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
