mike-jumper commented on code in PR #902:
URL: https://github.com/apache/guacamole-client/pull/902#discussion_r1741234096
##########
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/user/AuthenticatedUser.java:
##########
@@ -62,5 +68,10 @@ public AuthenticationProvider getAuthenticationProvider() {
public Credentials getCredentials() {
return credentials;
}
+
+ @Override
+ public boolean isCaseSensitive() {
+ return confService.getCaseSensitiveUsernames();
+ }
Review Comment:
The `GuacamoleException` potentially thrown by `getCaseSensitiveUsernames()`
will need to be handled here for this to build.
##########
extensions/guacamole-auth-header/src/main/java/org/apache/guacamole/auth/header/user/AuthenticatedUser.java:
##########
@@ -58,6 +66,16 @@ public void init(String username, Credentials credentials) {
setIdentifier(username.toLowerCase());
}
+ @Override
+ public boolean isCaseSensitive() {
+ try {
+ return confService.getCaseSensitiveUsernames();
+ }
+ catch (GuacamoleException e) {
+ return false;
+ }
Review Comment:
Should the failure and its implications be logged here?
##########
guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractUser.java:
##########
@@ -49,6 +49,11 @@ public String getPassword() {
public void setPassword(String password) {
this.password = password;
}
+
+ @Override
+ public boolean isCaseSensitive() {
+ return false;
+ }
Review Comment:
Should the default behavior at the `AbstractUser` level instead be
case-sensitive?
My main concern here would be unexpectedly changing the behavior of
third-party extensions that may make use of `AbstractUser`.
##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUser.java:
##########
@@ -780,5 +788,15 @@ public Permissions getEffectivePermissions() throws
GuacamoleException {
public boolean isSkeleton() {
return (getModel().getEntityID() == null);
}
+
+ @Override
+ public boolean isCaseSensitive() {
+ try {
+ return environment.getCaseSensitiveUsernames();
+ }
+ catch (GuacamoleException e) {
+ return true;
+ }
Review Comment:
Should the failure be logged here?
##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/java/org/apache/guacamole/auth/mysql/conf/MySQLGuacamoleProperties.java:
##########
@@ -301,6 +301,14 @@ private MySQLGuacamoleProperties() {}
@Override
public String getName() { return "mysql-batch-size"; }
- };
+ };
+
+ public static final BooleanGuacamoleProperty
MYSQL_CASE_SENSITIVE_USERNAMES =
+ new BooleanGuacamoleProperty() {
Review Comment:
Please document.
##########
extensions/guacamole-auth-json/src/main/java/org/apache/guacamole/auth/json/user/AuthenticatedUser.java:
##########
@@ -66,6 +75,16 @@ public void init(Credentials credentials, UserData userData)
{
this.userData = userData;
setIdentifier(userData.getUsername());
}
+
+ @Override
+ public boolean isCaseSensitive() {
+ try {
+ return confService.getCaseSensitiveUsernames();
+ }
+ catch (GuacamoleException e) {
+ return false;
+ }
Review Comment:
This will have the effect of preventing the configuration error from being
logged unless we explicitly log the error here. It would be good to log the
fact that an error occurred and that usernames will be presumed to be
case-sensitive.
##########
guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AbstractIdentifiable.java:
##########
@@ -34,12 +34,19 @@ public abstract class AbstractIdentifiable implements
Identifiable {
@Override
public String getIdentifier() {
- return identifier;
+ if (identifier == null || isCaseSensitive())
+ return identifier;
+
+ return identifier.toLowerCase();
}
@Override
public void setIdentifier(String identifier) {
- this.identifier = identifier;
+ if (isCaseSensitive())
+ this.identifier = identifier;
+ else
+ if (identifier != null)
+ this.identifier = identifier.toLowerCase();
Review Comment:
For clarity, this should either be rolled up into an `else if` or the `else`
block should have braces.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
