necouchman commented on code in PR #931:
URL: https://github.com/apache/guacamole-client/pull/931#discussion_r1554792021
##########
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java:
##########
@@ -28,6 +28,9 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
Review Comment:
Please remove this extra line -we do not split `import` lines.
##########
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java:
##########
@@ -308,25 +316,49 @@ public LDAPAuthenticatedUser authenticateUser(Credentials
credentials)
}
/**
- * Returns parameter tokens generated from LDAP attributes on the user
- * currently bound under the given LDAP connection. The attributes to be
- * converted into parameter tokens must be explicitly listed in
- * guacamole.properties. If no attributes are specified or none are
- * found on the LDAP user object, an empty map is returned.
+ * Returns the current LDAP domain token from the provided user credentials
+ * @param credentials
+ * The credentials used for authentication.
+ *
+ * @return
+ * Domain name by splitting login username or null if no domain is
detected.
+ */
+ private String getDomainToken(Credentials credentials) {
+ String username = credentials.getUsername();
+ Pattern pattern = Pattern.compile("^(.+)\\\\.*$|^.*@(.+)$");
Review Comment:
Might be nice to document what you're doing, here - regular expressions can
be obscure enough that it isn't obvious what this RegEx does, and would benefit
from a one line description of it.
##########
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java:
##########
@@ -308,25 +316,49 @@ public LDAPAuthenticatedUser authenticateUser(Credentials
credentials)
}
/**
- * Returns parameter tokens generated from LDAP attributes on the user
- * currently bound under the given LDAP connection. The attributes to be
- * converted into parameter tokens must be explicitly listed in
- * guacamole.properties. If no attributes are specified or none are
- * found on the LDAP user object, an empty map is returned.
+ * Returns the current LDAP domain token from the provided user credentials
+ * @param credentials
+ * The credentials used for authentication.
+ *
+ * @return
+ * Domain name by splitting login username or null if no domain is
detected.
+ */
+ private String getDomainToken(Credentials credentials) {
+ String username = credentials.getUsername();
+ Pattern pattern = Pattern.compile("^(.+)\\\\.*$|^.*@(.+)$");
+ Matcher matcher = pattern.matcher(username);
+ if (matcher.find()) {
+ return matcher.group(1) != null ? matcher.group(1) :
matcher.group(2);
+ }
+ return null;
+ }
+
+ /**
+ * Returns parameter tokens generated based on details specific to the
user currently bound under the given
+ * LDAP connection. Both LDAP attributes on the user's associated LDAP
object and the credentials submitted
+ * by the user to Guacamole are considered. If any tokens are to be
derived from LDAP attributes, those
+ * attributes must be explicitly listed in guacamole.properties. If no
tokens are applicable, an empty map is
+ * returned.
*
* @param config
* The configuration of the LDAP server being queried.
*
+ * @param credentials
+ * The credentials to use for authentication.
+ *
* @return
- * A map of parameter tokens generated from attributes on the user
- * currently bound under the given LDAP connection, as a map of token
- * name to corresponding value, or an empty map if no attributes are
- * specified or none are found on the user object.
+ * A map of parameter tokens. These tokens are generated based on
+ * the attributes of the user currently bound under the given LDAP
connection
+ * and the user's credentials. The map's keys are the canonicalized
attribute
+ * names with an "LDAP_" prefix, and the values are the corresponding
attribute
+ * values. If the domain name is extracted from the user's
credentials, it is added
+ * to the map with the key "LDAP_DOMAIN". If no applicable tokens are
found,
+ * the method returns an empty map.
Review Comment:
Looks like these lines are all indented by an extra space?
##########
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java:
##########
@@ -308,25 +316,49 @@ public LDAPAuthenticatedUser authenticateUser(Credentials
credentials)
}
/**
- * Returns parameter tokens generated from LDAP attributes on the user
- * currently bound under the given LDAP connection. The attributes to be
- * converted into parameter tokens must be explicitly listed in
- * guacamole.properties. If no attributes are specified or none are
- * found on the LDAP user object, an empty map is returned.
+ * Returns the current LDAP domain token from the provided user credentials
+ * @param credentials
Review Comment:
There should be a line between this one and the one before it.
##########
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java:
##########
@@ -308,25 +316,49 @@ public LDAPAuthenticatedUser authenticateUser(Credentials
credentials)
}
/**
- * Returns parameter tokens generated from LDAP attributes on the user
- * currently bound under the given LDAP connection. The attributes to be
- * converted into parameter tokens must be explicitly listed in
- * guacamole.properties. If no attributes are specified or none are
- * found on the LDAP user object, an empty map is returned.
+ * Returns the current LDAP domain token from the provided user credentials
+ * @param credentials
+ * The credentials used for authentication.
+ *
+ * @return
+ * Domain name by splitting login username or null if no domain is
detected.
+ */
+ private String getDomainToken(Credentials credentials) {
+ String username = credentials.getUsername();
+ Pattern pattern = Pattern.compile("^(.+)\\\\.*$|^.*@(.+)$");
+ Matcher matcher = pattern.matcher(username);
+ if (matcher.find()) {
+ return matcher.group(1) != null ? matcher.group(1) :
matcher.group(2);
+ }
+ return null;
+ }
+
+ /**
+ * Returns parameter tokens generated based on details specific to the
user currently bound under the given
+ * LDAP connection. Both LDAP attributes on the user's associated LDAP
object and the credentials submitted
+ * by the user to Guacamole are considered. If any tokens are to be
derived from LDAP attributes, those
+ * attributes must be explicitly listed in guacamole.properties. If no
tokens are applicable, an empty map is
+ * returned.
Review Comment:
These lines ended up being substantially longer than most of the other lines
in the file, well beyond our sort of "soft target" of 80 columns as a wrap
point.
--
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]
