GUACAMOLE-540: Move remote address processing to Credentials class for consistency.
Project: http://git-wip-us.apache.org/repos/asf/guacamole-client/repo Commit: http://git-wip-us.apache.org/repos/asf/guacamole-client/commit/cbd77b52 Tree: http://git-wip-us.apache.org/repos/asf/guacamole-client/tree/cbd77b52 Diff: http://git-wip-us.apache.org/repos/asf/guacamole-client/diff/cbd77b52 Branch: refs/heads/master Commit: cbd77b52ae5c68f0f61cfdb56e361cc3aa8e45bd Parents: bc1dc41 Author: Nick Couchman <[email protected]> Authored: Tue May 15 13:43:39 2018 -0400 Committer: Nick Couchman <[email protected]> Committed: Sat Jun 16 07:13:17 2018 -0400 ---------------------------------------------------------------------- .../auth/jdbc/user/ModeledUserContext.java | 2 +- .../auth/jdbc/user/RemoteAuthenticatedUser.java | 60 +------------------- .../apache/guacamole/net/auth/Credentials.java | 38 +++++++++++++ .../guacamole/rest/auth/TokenRESTService.java | 4 +- .../guacamole/rest/user/UserResource.java | 2 - 5 files changed, 42 insertions(+), 64 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/guacamole-client/blob/cbd77b52/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUserContext.java ---------------------------------------------------------------------- diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUserContext.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUserContext.java index cbc3448..d53164b 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUserContext.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUserContext.java @@ -137,7 +137,7 @@ public class ModeledUserContext extends RestrictedObject userRecord = new ActivityRecordModel(); userRecord.setUsername(currentUser.getIdentifier()); userRecord.setStartDate(new Date()); - userRecord.setRemoteHost(currentUser.getCredentials().getRemoteHostname()); + userRecord.setRemoteHost(currentUser.getCredentials().getRemoteAddress()); // Insert record representing login userRecordMapper.insert(userRecord); http://git-wip-us.apache.org/repos/asf/guacamole-client/blob/cbd77b52/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/RemoteAuthenticatedUser.java ---------------------------------------------------------------------- diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/RemoteAuthenticatedUser.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/RemoteAuthenticatedUser.java index 24118af..017a9c1 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/RemoteAuthenticatedUser.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/RemoteAuthenticatedUser.java @@ -19,9 +19,6 @@ package org.apache.guacamole.auth.jdbc.user; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -import javax.servlet.http.HttpServletRequest; import org.apache.guacamole.net.auth.AuthenticatedUser; import org.apache.guacamole.net.auth.AuthenticationProvider; import org.apache.guacamole.net.auth.Credentials; @@ -45,60 +42,7 @@ public abstract class RemoteAuthenticatedUser implements AuthenticatedUser { * The host from which this user authenticated. */ private final String remoteHost; - - /** - * Regular expression which matches any IPv4 address. - */ - private static final String IPV4_ADDRESS_REGEX = "([0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3})"; - - /** - * Regular expression which matches any IPv6 address. - */ - private static final String IPV6_ADDRESS_REGEX = "([0-9a-fA-F]*(:[0-9a-fA-F]*){0,7})"; - - /** - * Regular expression which matches any IP address, regardless of version. - */ - private static final String IP_ADDRESS_REGEX = "(" + IPV4_ADDRESS_REGEX + "|" + IPV6_ADDRESS_REGEX + ")"; - - /** - * Pattern which matches valid values of the de-facto standard - * "X-Forwarded-For" header. - */ - private static final Pattern X_FORWARDED_FOR = Pattern.compile("^" + IP_ADDRESS_REGEX + "(, " + IP_ADDRESS_REGEX + ")*$"); - - /** - * Derives the remote host of the authenticating user from the given - * credentials object. The remote host is derived from X-Forwarded-For - * in addition to the actual source IP of the request, and thus is not - * trusted. The derived remote host is really only useful for logging, - * unless the server is configured such that X-Forwarded-For is guaranteed - * to be trustworthy. - * - * @param credentials - * The credentials to derive the remote host from. - * - * @return - * The remote host from which the user with the given credentials is - * authenticating. - */ - private static String getRemoteHost(Credentials credentials) { - - HttpServletRequest request = credentials.getRequest(); - - // Use X-Forwarded-For, if present and valid - String header = request.getHeader("X-Forwarded-For"); - if (header != null) { - Matcher matcher = X_FORWARDED_FOR.matcher(header); - if (matcher.matches()) - return matcher.group(1); - } - - // If header absent or invalid, just use source IP - return request.getRemoteAddr(); - - } - + /** * Creates a new RemoteAuthenticatedUser, deriving the associated remote * host from the given credentials. @@ -113,7 +57,7 @@ public abstract class RemoteAuthenticatedUser implements AuthenticatedUser { Credentials credentials) { this.authenticationProvider = authenticationProvider; this.credentials = credentials; - this.remoteHost = getRemoteHost(credentials); + this.remoteHost = credentials.getRemoteAddress(); } @Override http://git-wip-us.apache.org/repos/asf/guacamole-client/blob/cbd77b52/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Credentials.java ---------------------------------------------------------------------- diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Credentials.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Credentials.java index 142c516..741d858 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Credentials.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Credentials.java @@ -20,6 +20,8 @@ package org.apache.guacamole.net.auth; import java.io.Serializable; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpSession; @@ -38,6 +40,27 @@ public class Credentials implements Serializable { * Unique identifier associated with this specific version of Credentials. */ private static final long serialVersionUID = 1L; + + /** + * Regular expression which matches any IPv4 address. + */ + private static final String IPV4_ADDRESS_REGEX = "([0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3})"; + + /** + * Regular expression which matches any IPv6 address. + */ + private static final String IPV6_ADDRESS_REGEX = "([0-9a-fA-F]*(:[0-9a-fA-F]*){0,7})"; + + /** + * Regular expression which matches any IP address, regardless of version. + */ + private static final String IP_ADDRESS_REGEX = "(" + IPV4_ADDRESS_REGEX + "|" + IPV6_ADDRESS_REGEX + ")"; + + /** + * Pattern which matches valid values of the de-facto standard + * "X-Forwarded-For" header. + */ + private static final Pattern X_FORWARDED_FOR = Pattern.compile("^" + IP_ADDRESS_REGEX + "(, " + IP_ADDRESS_REGEX + ")*$"); /** * An arbitrary username. @@ -124,6 +147,21 @@ public class Credentials implements Serializable { */ public void setRequest(HttpServletRequest request) { this.request = request; + + // Use X-Forwarded-For to get remote address, if present and valid + String header = request.getHeader("X-Forwarded-For"); + if (header != null) { + Matcher matcher = X_FORWARDED_FOR.matcher(header); + if (matcher.matches()) + setRemoteAddress(matcher.group(1)); + } + // Header not present, just use remote address + else { + setRemoteAddress(request.getRemoteAddr()); + } + + setRemoteHostname(request.getRemoteHost()); + } /** http://git-wip-us.apache.org/repos/asf/guacamole-client/blob/cbd77b52/guacamole/src/main/java/org/apache/guacamole/rest/auth/TokenRESTService.java ---------------------------------------------------------------------- diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/auth/TokenRESTService.java b/guacamole/src/main/java/org/apache/guacamole/rest/auth/TokenRESTService.java index e8c1c77..fb1d929 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/auth/TokenRESTService.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/auth/TokenRESTService.java @@ -55,7 +55,7 @@ public class TokenRESTService { * Logger for this class. */ private static final Logger logger = LoggerFactory.getLogger(TokenRESTService.class); - + /** * Service for authenticating users and managing their Guacamole sessions. */ @@ -122,8 +122,6 @@ public class TokenRESTService { credentials.setPassword(password); credentials.setRequest(request); credentials.setSession(request.getSession(false)); - credentials.setRemoteAddress(request.getRemoteAddr()); - credentials.setRemoteHostname(request.getRemoteHost()); return credentials; http://git-wip-us.apache.org/repos/asf/guacamole-client/blob/cbd77b52/guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java ---------------------------------------------------------------------- diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java b/guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java index db0aba5..8f3abfe 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java @@ -160,8 +160,6 @@ public class UserResource credentials.setPassword(userPasswordUpdate.getOldPassword()); credentials.setRequest(request); credentials.setSession(request.getSession(false)); - credentials.setRemoteAddress(request.getRemoteAddr()); - credentials.setRemoteHostname(request.getRemoteHost()); // Verify that the old password was correct try {
