From 51712f272799fcdbe493d387d4b76b440afbcc65 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 15 Oct 2015 15:13:31 -0700 Subject: [PATCH 1/9] GUAC-1364: GuacamoleCredentialsException should be a GuacamoleUnauthorizedException (not just a generic "security" exception). --- .../net/auth/credentials/GuacamoleCredentialsException.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/guacamole-ext/src/main/java/org/glyptodon/guacamole/net/auth/credentials/GuacamoleCredentialsException.java b/guacamole-ext/src/main/java/org/glyptodon/guacamole/net/auth/credentials/GuacamoleCredentialsException.java index 55fffea34..6ece39a76 100644 --- a/guacamole-ext/src/main/java/org/glyptodon/guacamole/net/auth/credentials/GuacamoleCredentialsException.java +++ b/guacamole-ext/src/main/java/org/glyptodon/guacamole/net/auth/credentials/GuacamoleCredentialsException.java @@ -22,7 +22,7 @@ package org.glyptodon.guacamole.net.auth.credentials; -import org.glyptodon.guacamole.GuacamoleSecurityException; +import org.glyptodon.guacamole.GuacamoleUnauthorizedException; /** * A security-related exception thrown when access is denied to a user because @@ -31,7 +31,7 @@ import org.glyptodon.guacamole.GuacamoleSecurityException; * * @author Michael Jumper */ -public class GuacamoleCredentialsException extends GuacamoleSecurityException { +public class GuacamoleCredentialsException extends GuacamoleUnauthorizedException { /** * Information describing the form of valid credentials. From 25dff1a322f815dd9e606d65103c1c6765cab0c3 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 15 Oct 2015 16:04:46 -0700 Subject: [PATCH 2/9] GUAC-1364: Pull auth token from the parameters of methods which throw a GuacamoleUnauthorizedException. Stub out automatic invalidation of that token. --- .../net/basic/rest/RESTExceptionWrapper.java | 113 +++++++++++++++++- 1 file changed, 109 insertions(+), 4 deletions(-) diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java index 9cb0bc807..7f52d95b9 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java @@ -22,12 +22,17 @@ package org.glyptodon.guacamole.net.basic.rest; +import java.lang.reflect.Method; +import java.lang.reflect.Parameter; +import javax.ws.rs.FormParam; +import javax.ws.rs.QueryParam; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.glyptodon.guacamole.GuacamoleClientException; import org.glyptodon.guacamole.GuacamoleException; import org.glyptodon.guacamole.GuacamoleResourceNotFoundException; import org.glyptodon.guacamole.GuacamoleSecurityException; +import org.glyptodon.guacamole.GuacamoleUnauthorizedException; import org.glyptodon.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException; import org.glyptodon.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException; import org.slf4j.Logger; @@ -45,14 +50,112 @@ import org.slf4j.LoggerFactory; */ public class RESTExceptionWrapper implements MethodInterceptor { + /** + * Logger for this class. + */ + private final Logger logger = LoggerFactory.getLogger(RESTExceptionWrapper.class); + + /** + * Determines whether the given parameter is associated with the HTTP + * request parameter of the given name. For a parameter to be associated + * with an HTTP request parameter, it must be annotated with either the + * @QueryParam or @FormParam annotations. + * + * @param parameter + * The Java parameter to check. + * + * @param name + * The name of the HTTP request parameter. + * + * @return + * true if the given parameter is associated with the HTTP request + * parameter having the given name, false otherwise. + */ + private boolean isRequestParameter(Parameter parameter, String name) { + + // Check if parameter is associated with the HTTP query string + QueryParam queryParam = parameter.getAnnotation(QueryParam.class); + if (queryParam != null && name.equals(queryParam.value())) + return true; + + // Failing that, check whether the parameter is associated with the + // HTTP request body + FormParam formParam = parameter.getAnnotation(FormParam.class); + return formParam != null && name.equals(formParam.value()); + + } + + /** + * Returns the authentication token that was passed in the given method + * invocation. If the given method invocation is not associated with an + * HTTP request (it lacks the appropriate JAX-RS annotations) or there is + * no authentication token, null is returned. + * + * @param invocation + * The method invocation whose corresponding authentication token + * should be determined. + * + * @return + * The authentication token passed in the given method invocation, or + * null if there is no such token. + */ + private String getAuthenticationToken(MethodInvocation invocation) { + + Method method = invocation.getMethod(); + + // Iterate through all parameters, looking for the authentication token + Parameter[] parameters = method.getParameters(); + for (int i = 0; i < parameters.length; i++) { + + // Get current parameter + Parameter parameter = parameters[i]; + + // Only inspect String parameters + if (parameter.getType() != String.class) + continue; + + // Parameter must be declared as a REST service parameter + if (!isRequestParameter(parameter, "token")) + continue; + + // The token parameter has been found - return its value + Object[] args = invocation.getArguments(); + return (String) args[i]; + + } + + // No token parameter is defined + return null; + + } + @Override public Object invoke(MethodInvocation invocation) throws Throwable { - // Get the logger for the intercepted class - Logger logger = LoggerFactory.getLogger(invocation.getMethod().getDeclaringClass()); - try { - return invocation.proceed(); + + // Invoke wrapped method + try { + return invocation.proceed(); + } + + // Ensure any associated session is invalidated if unauthorized + catch (GuacamoleUnauthorizedException e) { + + // Pull authentication token from request + String token = getAuthenticationToken(invocation); + + // If there is an associated auth token, invalidate it + if (token != null) { + logger.debug("Implicitly invalidating token \"{}\" due to GuacamoleUnauthorizedException.", token); + // STUB - Does not actually invalidate anything at the moment + } + + // Continue with exception processing + throw e; + + } + } // Additional credentials are needed @@ -138,7 +241,9 @@ public class RESTExceptionWrapper implements MethodInterceptor { if (message == null) message = "Unexpected server error."; + // Ensure internal errors are logged at the debug level logger.debug("Unexpected exception in REST endpoint.", e); + throw new APIException( APIError.Type.INTERNAL_ERROR, message From 226f37e7e7e5828dd90e6eea321bc3f86bc14473 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 15 Oct 2015 16:42:03 -0700 Subject: [PATCH 3/9] GUAC-1364: Restrict use of reflection API to objects compatible with Java 6. --- .../net/basic/rest/RESTExceptionWrapper.java | 54 +++++++++++-------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java index 7f52d95b9..758e40e3a 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java @@ -22,8 +22,8 @@ package org.glyptodon.guacamole.net.basic.rest; +import java.lang.annotation.Annotation; import java.lang.reflect.Method; -import java.lang.reflect.Parameter; import javax.ws.rs.FormParam; import javax.ws.rs.QueryParam; import org.aopalliance.intercept.MethodInterceptor; @@ -56,32 +56,39 @@ public class RESTExceptionWrapper implements MethodInterceptor { private final Logger logger = LoggerFactory.getLogger(RESTExceptionWrapper.class); /** - * Determines whether the given parameter is associated with the HTTP + * Determines whether the given set of annotations describes an HTTP * request parameter of the given name. For a parameter to be associated * with an HTTP request parameter, it must be annotated with either the * @QueryParam or @FormParam annotations. * - * @param parameter - * The Java parameter to check. + * @param annotations + * The annotations associated with the Java parameter being checked. * * @param name * The name of the HTTP request parameter. * * @return - * true if the given parameter is associated with the HTTP request + * true if the given set of annotations describes an HTTP request * parameter having the given name, false otherwise. */ - private boolean isRequestParameter(Parameter parameter, String name) { + private boolean isRequestParameter(Annotation[] annotations, String name) { - // Check if parameter is associated with the HTTP query string - QueryParam queryParam = parameter.getAnnotation(QueryParam.class); - if (queryParam != null && name.equals(queryParam.value())) - return true; + // Search annotations for associated HTTP parameters + for (Annotation annotation : annotations) { - // Failing that, check whether the parameter is associated with the - // HTTP request body - FormParam formParam = parameter.getAnnotation(FormParam.class); - return formParam != null && name.equals(formParam.value()); + // Check if parameter is associated with the HTTP query string + if (annotation instanceof QueryParam && name.equals(((QueryParam) annotation).value())) + return true; + + // Failing that, check whether the parameter is associated with the + // HTTP request body + if (annotation instanceof FormParam && name.equals(((FormParam) annotation).value())) + return true; + + } + + // No parameter annotations are present + return false; } @@ -103,19 +110,24 @@ public class RESTExceptionWrapper implements MethodInterceptor { Method method = invocation.getMethod(); - // Iterate through all parameters, looking for the authentication token - Parameter[] parameters = method.getParameters(); - for (int i = 0; i < parameters.length; i++) { + // Get the types and annotations associated with each parameter + Annotation[][] parameterAnnotations = method.getParameterAnnotations(); + Class[] parameterTypes = method.getParameterTypes(); - // Get current parameter - Parameter parameter = parameters[i]; + // The Java standards require these to be parallel arrays + assert(parameterAnnotations.length == parameterTypes.length); + + // Iterate through all parameters, looking for the authentication token + for (int i = 0; i < parameterTypes.length; i++) { // Only inspect String parameters - if (parameter.getType() != String.class) + Class parameterType = parameterTypes[i]; + if (parameterType != String.class) continue; // Parameter must be declared as a REST service parameter - if (!isRequestParameter(parameter, "token")) + Annotation[] annotations = parameterAnnotations[i]; + if (!isRequestParameter(annotations, "token")) continue; // The token parameter has been found - return its value From 6dc4adf6c9fed18ce287f942f0d0de135e5c491f Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 16 Oct 2015 11:46:28 -0700 Subject: [PATCH 4/9] GUAC-1364: Implicitly invalidate auth tokens when handling GuacamoleUnauthorizedExceptions within REST services. --- .../basic/BasicServletContextListener.java | 6 +- .../net/basic/rest/RESTAuthModule.java | 75 ------------------- .../net/basic/rest/RESTExceptionWrapper.java | 25 ++++++- ...vletModule.java => RESTServiceModule.java} | 36 ++++++++- 4 files changed, 56 insertions(+), 86 deletions(-) delete mode 100644 guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTAuthModule.java rename guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/{RESTServletModule.java => RESTServiceModule.java} (68%) diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/BasicServletContextListener.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/BasicServletContextListener.java index 60eeffdbb..d9962fef9 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/BasicServletContextListener.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/BasicServletContextListener.java @@ -32,8 +32,7 @@ import org.glyptodon.guacamole.environment.Environment; import org.glyptodon.guacamole.environment.LocalEnvironment; import org.glyptodon.guacamole.net.basic.extension.ExtensionModule; import org.glyptodon.guacamole.net.basic.log.LogModule; -import org.glyptodon.guacamole.net.basic.rest.RESTAuthModule; -import org.glyptodon.guacamole.net.basic.rest.RESTServletModule; +import org.glyptodon.guacamole.net.basic.rest.RESTServiceModule; import org.glyptodon.guacamole.net.basic.rest.auth.BasicTokenSessionMap; import org.glyptodon.guacamole.net.basic.rest.auth.TokenSessionMap; import org.slf4j.Logger; @@ -85,8 +84,7 @@ public class BasicServletContextListener extends GuiceServletContextListener { new EnvironmentModule(environment), new LogModule(environment), new ExtensionModule(environment), - new RESTAuthModule(sessionMap), - new RESTServletModule(), + new RESTServiceModule(sessionMap), new TunnelModule() ); } diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTAuthModule.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTAuthModule.java deleted file mode 100644 index ddc5fff78..000000000 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTAuthModule.java +++ /dev/null @@ -1,75 +0,0 @@ -/* - * Copyright (C) 2015 Glyptodon LLC - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -package org.glyptodon.guacamole.net.basic.rest; - -import com.google.inject.AbstractModule; -import org.glyptodon.guacamole.net.basic.rest.auth.AuthTokenGenerator; -import org.glyptodon.guacamole.net.basic.rest.auth.AuthenticationService; -import org.glyptodon.guacamole.net.basic.rest.auth.SecureRandomAuthTokenGenerator; -import org.glyptodon.guacamole.net.basic.rest.auth.TokenSessionMap; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * A Guice Module for setting up authentication-specific dependency injection. - * - * @author James Muehlner - * @author Michael Jumper - */ -public class RESTAuthModule extends AbstractModule { - - /** - * Logger for this class. - */ - private final Logger logger = LoggerFactory.getLogger(RESTAuthModule.class); - - /** - * Singleton instance of TokenSessionMap. - */ - private final TokenSessionMap tokenSessionMap; - - /** - * Creates a module which handles binding of authentication-related - * objects, including the singleton TokenSessionMap. - * - * @param tokenSessionMap - * An instance of TokenSessionMap to inject as a singleton wherever - * needed. - */ - public RESTAuthModule(TokenSessionMap tokenSessionMap) { - this.tokenSessionMap = tokenSessionMap; - } - - @Override - protected void configure() { - - // Bind session map - bind(TokenSessionMap.class).toInstance(tokenSessionMap); - - // Bind low-level services - bind(AuthenticationService.class); - bind(AuthTokenGenerator.class).to(SecureRandomAuthTokenGenerator.class); - - } - -} diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java index 758e40e3a..14de30c92 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java @@ -35,6 +35,7 @@ import org.glyptodon.guacamole.GuacamoleSecurityException; import org.glyptodon.guacamole.GuacamoleUnauthorizedException; import org.glyptodon.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException; import org.glyptodon.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException; +import org.glyptodon.guacamole.net.basic.rest.auth.TokenSessionMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -55,6 +56,24 @@ public class RESTExceptionWrapper implements MethodInterceptor { */ private final Logger logger = LoggerFactory.getLogger(RESTExceptionWrapper.class); + /** + * Singleton instance of TokenSessionMap. + */ + private final TokenSessionMap tokenSessionMap; + + /** + * Creates an interceptor which automatically handles GuacamoleExceptions + * within the REST services, including implicit invalidation of + * authentication tokens. + * + * @param tokenSessionMap + * The singleton instance of TokenSessionMap to use if management of + * authentication tokens is required to handle a particular error. + */ + public RESTExceptionWrapper(TokenSessionMap tokenSessionMap) { + this.tokenSessionMap = tokenSessionMap; + } + /** * Determines whether the given set of annotations describes an HTTP * request parameter of the given name. For a parameter to be associated @@ -158,10 +177,8 @@ public class RESTExceptionWrapper implements MethodInterceptor { String token = getAuthenticationToken(invocation); // If there is an associated auth token, invalidate it - if (token != null) { - logger.debug("Implicitly invalidating token \"{}\" due to GuacamoleUnauthorizedException.", token); - // STUB - Does not actually invalidate anything at the moment - } + if (token != null && tokenSessionMap.remove(token) != null) + logger.debug("Implicitly invalidated token \"{}\" due to GuacamoleUnauthorizedException.", token); // Continue with exception processing throw e; diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTServletModule.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTServiceModule.java similarity index 68% rename from guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTServletModule.java rename to guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTServiceModule.java index 044fa5ae9..c194495c5 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTServletModule.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTServiceModule.java @@ -31,26 +31,56 @@ import org.glyptodon.guacamole.net.basic.rest.auth.TokenRESTService; import org.glyptodon.guacamole.net.basic.rest.connection.ConnectionRESTService; import org.glyptodon.guacamole.net.basic.rest.connectiongroup.ConnectionGroupRESTService; import org.glyptodon.guacamole.net.basic.rest.activeconnection.ActiveConnectionRESTService; +import org.glyptodon.guacamole.net.basic.rest.auth.AuthTokenGenerator; +import org.glyptodon.guacamole.net.basic.rest.auth.AuthenticationService; +import org.glyptodon.guacamole.net.basic.rest.auth.SecureRandomAuthTokenGenerator; +import org.glyptodon.guacamole.net.basic.rest.auth.TokenSessionMap; import org.glyptodon.guacamole.net.basic.rest.history.HistoryRESTService; import org.glyptodon.guacamole.net.basic.rest.language.LanguageRESTService; import org.glyptodon.guacamole.net.basic.rest.schema.SchemaRESTService; import org.glyptodon.guacamole.net.basic.rest.user.UserRESTService; /** - * A Guice Module to set up the servlet mappings for the Guacamole REST API. + * A Guice Module to set up the servlet mappings and authentication-specific + * dependency injection for the Guacamole REST API. * * @author James Muehlner + * @author Michael Jumper */ -public class RESTServletModule extends ServletModule { +public class RESTServiceModule extends ServletModule { + + /** + * Singleton instance of TokenSessionMap. + */ + private final TokenSessionMap tokenSessionMap; + + /** + * Creates a module which handles binding of REST services and related + * authentication objects, including the singleton TokenSessionMap. + * + * @param tokenSessionMap + * An instance of TokenSessionMap to inject as a singleton wherever + * needed. + */ + public RESTServiceModule(TokenSessionMap tokenSessionMap) { + this.tokenSessionMap = tokenSessionMap; + } @Override protected void configureServlets() { + // Bind session map + bind(TokenSessionMap.class).toInstance(tokenSessionMap); + + // Bind low-level services + bind(AuthenticationService.class); + bind(AuthTokenGenerator.class).to(SecureRandomAuthTokenGenerator.class); + // Automatically translate GuacamoleExceptions for REST methods bindInterceptor( Matchers.any(), new RESTMethodMatcher(), - new RESTExceptionWrapper() + new RESTExceptionWrapper(tokenSessionMap) ); // Bind convenience services used by the REST API From c606d72c8a3b4f41f758f8237ad605e61f6b1b63 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 16 Oct 2015 12:00:15 -0700 Subject: [PATCH 5/9] GUAC-1364: Invalidate session after token has been invalidated. --- .../guacamole/net/basic/rest/RESTExceptionWrapper.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java index 14de30c92..1b3e834b3 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java @@ -35,6 +35,7 @@ import org.glyptodon.guacamole.GuacamoleSecurityException; import org.glyptodon.guacamole.GuacamoleUnauthorizedException; import org.glyptodon.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException; import org.glyptodon.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException; +import org.glyptodon.guacamole.net.basic.GuacamoleSession; import org.glyptodon.guacamole.net.basic.rest.auth.TokenSessionMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -177,8 +178,11 @@ public class RESTExceptionWrapper implements MethodInterceptor { String token = getAuthenticationToken(invocation); // If there is an associated auth token, invalidate it - if (token != null && tokenSessionMap.remove(token) != null) - logger.debug("Implicitly invalidated token \"{}\" due to GuacamoleUnauthorizedException.", token); + GuacamoleSession session = tokenSessionMap.remove(token); + if (session != null) { + session.invalidate(); + logger.debug("Implicitly invalidated session for token \"{}\".", token); + } // Continue with exception processing throw e; From 4cded89c83a83bbe50d08ebf2ae5fa62774b2cdc Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 16 Oct 2015 12:34:53 -0700 Subject: [PATCH 6/9] GUAC-1364: Check authentication status at the end of each connection. --- .../client/controllers/clientController.js | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/guacamole/src/main/webapp/app/client/controllers/clientController.js b/guacamole/src/main/webapp/app/client/controllers/clientController.js index 2fab8e37b..9e5d77dce 100644 --- a/guacamole/src/main/webapp/app/client/controllers/clientController.js +++ b/guacamole/src/main/webapp/app/client/controllers/clientController.js @@ -33,11 +33,12 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams var ScrollState = $injector.get('ScrollState'); // Required services - var $location = $injector.get('$location'); - var guacClientManager = $injector.get('guacClientManager'); - var guacNotification = $injector.get('guacNotification'); - var preferenceService = $injector.get('preferenceService'); - var userPageService = $injector.get('userPageService'); + var $location = $injector.get('$location'); + var authenticationService = $injector.get('authenticationService'); + var guacClientManager = $injector.get('guacClientManager'); + var guacNotification = $injector.get('guacNotification'); + var preferenceService = $injector.get('preferenceService'); + var userPageService = $injector.get('userPageService'); /** * The minimum number of pixels a drag gesture must move to result in the @@ -409,6 +410,30 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams $scope.page.title = name; }); + /** + * Displays a notification at the end of a Guacamole connection, whether + * that connection is ending normally or due to an error. As the end of + * a Guacamole connection may be due to changes in authentication status, + * this will also implicitly peform a re-authentication attempt to check + * for such changes, possibly resulting in auth-related events like + * guacInvalidCredentials. + * + * @param {Notification|Boolean|Object} status + * The status notification to show, as would be accepted by + * guacNotification.showStatus(). + */ + var notifyConnectionClosed = function notifyConnectionClosed(status) { + + // Re-authenticate to verify auth status at end of connection + authenticationService.updateCurrentToken($location.search()) + + // If authentication is OK, show the requested status + .then(function authenticationCheckSuccessful() { + guacNotification.showStatus(status); + }); + + }; + // Show status dialog when connection status changes $scope.$watch('client.clientState.connectionState', function clientStateChanged(connectionState) { @@ -448,7 +473,7 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams var countdown = (status in CLIENT_AUTO_RECONNECT) ? RECONNECT_COUNTDOWN : null; // Show error status - guacNotification.showStatus({ + notifyConnectionClosed({ className : "error", title : "CLIENT.DIALOG_HEADER_CONNECTION_ERROR", text : "CLIENT.ERROR_CLIENT_" + errorName, @@ -468,7 +493,7 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams var countdown = (status in TUNNEL_AUTO_RECONNECT) ? RECONNECT_COUNTDOWN : null; // Show error status - guacNotification.showStatus({ + notifyConnectionClosed({ className : "error", title : "CLIENT.DIALOG_HEADER_CONNECTION_ERROR", text : "CLIENT.ERROR_TUNNEL_" + errorName, @@ -480,7 +505,7 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams // Disconnected else if (connectionState === ManagedClientState.ConnectionState.DISCONNECTED) { - guacNotification.showStatus({ + notifyConnectionClosed({ title : "CLIENT.DIALOG_HEADER_DISCONNECTED", text : "CLIENT.TEXT_CLIENT_STATUS_" + connectionState.toUpperCase(), actions : actions From 1f316e5e687c637413939e6115c4b67a6f79b0a2 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 16 Oct 2015 13:59:43 -0700 Subject: [PATCH 7/9] GUAC-1364: Move session management into AuthenticationService. The token map should not appear anywhere else (except where needed to configure dependency injection of the same). --- .../net/basic/rest/RESTExceptionWrapper.java | 27 +- .../net/basic/rest/RESTServiceModule.java | 9 +- .../rest/auth/AuthenticationService.java | 395 +++++++++++++++++- .../net/basic/rest/auth/TokenRESTService.java | 342 +-------------- 4 files changed, 416 insertions(+), 357 deletions(-) diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java index 1b3e834b3..92bb63f06 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTExceptionWrapper.java @@ -22,6 +22,7 @@ package org.glyptodon.guacamole.net.basic.rest; +import com.google.inject.Inject; import java.lang.annotation.Annotation; import java.lang.reflect.Method; import javax.ws.rs.FormParam; @@ -35,8 +36,7 @@ import org.glyptodon.guacamole.GuacamoleSecurityException; import org.glyptodon.guacamole.GuacamoleUnauthorizedException; import org.glyptodon.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException; import org.glyptodon.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException; -import org.glyptodon.guacamole.net.basic.GuacamoleSession; -import org.glyptodon.guacamole.net.basic.rest.auth.TokenSessionMap; +import org.glyptodon.guacamole.net.basic.rest.auth.AuthenticationService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -58,22 +58,10 @@ public class RESTExceptionWrapper implements MethodInterceptor { private final Logger logger = LoggerFactory.getLogger(RESTExceptionWrapper.class); /** - * Singleton instance of TokenSessionMap. + * Service for authenticating users and managing their Guacamole sessions. */ - private final TokenSessionMap tokenSessionMap; - - /** - * Creates an interceptor which automatically handles GuacamoleExceptions - * within the REST services, including implicit invalidation of - * authentication tokens. - * - * @param tokenSessionMap - * The singleton instance of TokenSessionMap to use if management of - * authentication tokens is required to handle a particular error. - */ - public RESTExceptionWrapper(TokenSessionMap tokenSessionMap) { - this.tokenSessionMap = tokenSessionMap; - } + @Inject + private AuthenticationService authenticationService; /** * Determines whether the given set of annotations describes an HTTP @@ -178,11 +166,8 @@ public class RESTExceptionWrapper implements MethodInterceptor { String token = getAuthenticationToken(invocation); // If there is an associated auth token, invalidate it - GuacamoleSession session = tokenSessionMap.remove(token); - if (session != null) { - session.invalidate(); + if (authenticationService.destroyGuacamoleSession(token)) logger.debug("Implicitly invalidated session for token \"{}\".", token); - } // Continue with exception processing throw e; diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTServiceModule.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTServiceModule.java index c194495c5..40ee2d330 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTServiceModule.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/RESTServiceModule.java @@ -26,6 +26,7 @@ import com.google.inject.Scopes; import com.google.inject.matcher.Matchers; import com.google.inject.servlet.ServletModule; import com.sun.jersey.guice.spi.container.servlet.GuiceContainer; +import org.aopalliance.intercept.MethodInterceptor; import org.codehaus.jackson.jaxrs.JacksonJsonProvider; import org.glyptodon.guacamole.net.basic.rest.auth.TokenRESTService; import org.glyptodon.guacamole.net.basic.rest.connection.ConnectionRESTService; @@ -77,11 +78,9 @@ public class RESTServiceModule extends ServletModule { bind(AuthTokenGenerator.class).to(SecureRandomAuthTokenGenerator.class); // Automatically translate GuacamoleExceptions for REST methods - bindInterceptor( - Matchers.any(), - new RESTMethodMatcher(), - new RESTExceptionWrapper(tokenSessionMap) - ); + MethodInterceptor interceptor = new RESTExceptionWrapper(); + requestInjection(interceptor); + bindInterceptor(Matchers.any(), new RESTMethodMatcher(), interceptor); // Bind convenience services used by the REST API bind(ObjectRetrievalService.class); diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/auth/AuthenticationService.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/auth/AuthenticationService.java index 7667bb43c..a4727686f 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/auth/AuthenticationService.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/auth/AuthenticationService.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2014 Glyptodon LLC + * Copyright (C) 2015 Glyptodon LLC * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -23,30 +23,391 @@ package org.glyptodon.guacamole.net.basic.rest.auth; import com.google.inject.Inject; +import java.util.ArrayList; import java.util.List; +import java.util.regex.Pattern; +import javax.servlet.http.HttpServletRequest; import org.glyptodon.guacamole.GuacamoleException; +import org.glyptodon.guacamole.GuacamoleSecurityException; import org.glyptodon.guacamole.GuacamoleUnauthorizedException; +import org.glyptodon.guacamole.environment.Environment; +import org.glyptodon.guacamole.net.auth.AuthenticatedUser; +import org.glyptodon.guacamole.net.auth.AuthenticationProvider; +import org.glyptodon.guacamole.net.auth.Credentials; import org.glyptodon.guacamole.net.auth.UserContext; +import org.glyptodon.guacamole.net.auth.credentials.CredentialsInfo; +import org.glyptodon.guacamole.net.auth.credentials.GuacamoleCredentialsException; +import org.glyptodon.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException; import org.glyptodon.guacamole.net.basic.GuacamoleSession; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A service for performing authentication checks in REST endpoints. * * @author James Muehlner + * @author Michael Jumper */ public class AuthenticationService { - + + /** + * Logger for this class. + */ + private static final Logger logger = LoggerFactory.getLogger(AuthenticationService.class); + + /** + * The Guacamole server environment. + */ + @Inject + private Environment environment; + + /** + * All configured authentication providers which can be used to + * authenticate users or retrieve data associated with authenticated users. + */ + @Inject + private List authProviders; + /** * The map of auth tokens to sessions for the REST endpoints. */ @Inject private TokenSessionMap tokenSessionMap; - + + /** + * A generator for creating new auth tokens. + */ + @Inject + private AuthTokenGenerator authTokenGenerator; + + /** + * 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 + ")*$"); + + /** + * Returns a formatted string containing an IP address, or list of IP + * addresses, which represent the HTTP client and any involved proxies. As + * the headers used to determine proxies can easily be forged, this data is + * superficially validated to ensure that it at least looks like a list of + * IPs. + * + * @param request + * The HTTP request to format. + * + * @return + * A formatted string containing one or more IP addresses. + */ + private String getLoggableAddress(HttpServletRequest request) { + + // Log X-Forwarded-For, if present and valid + String header = request.getHeader("X-Forwarded-For"); + if (header != null && X_FORWARDED_FOR.matcher(header).matches()) + return "[" + header + ", " + request.getRemoteAddr() + "]"; + + // If header absent or invalid, just use source IP + return request.getRemoteAddr(); + + } + + /** + * Attempts authentication against all AuthenticationProviders, in order, + * using the provided credentials. The first authentication failure takes + * priority, but remaining AuthenticationProviders are attempted. If any + * AuthenticationProvider succeeds, the resulting AuthenticatedUser is + * returned, and no further AuthenticationProviders are tried. + * + * @param credentials + * The credentials to use for authentication. + * + * @return + * The AuthenticatedUser given by the highest-priority + * AuthenticationProvider for which the given credentials are valid. + * + * @throws GuacamoleException + * If the given credentials are not valid for any + * AuthenticationProvider, or if an error occurs while authenticating + * the user. + */ + private AuthenticatedUser authenticateUser(Credentials credentials) + throws GuacamoleException { + + GuacamoleCredentialsException authFailure = null; + + // Attempt authentication against each AuthenticationProvider + for (AuthenticationProvider authProvider : authProviders) { + + // Attempt authentication + try { + AuthenticatedUser authenticatedUser = authProvider.authenticateUser(credentials); + if (authenticatedUser != null) + return authenticatedUser; + } + + // First failure takes priority for now + catch (GuacamoleCredentialsException e) { + if (authFailure == null) + authFailure = e; + } + + } + + // If a specific failure occured, rethrow that + if (authFailure != null) + throw authFailure; + + // Otherwise, request standard username/password + throw new GuacamoleInvalidCredentialsException( + "Permission Denied.", + CredentialsInfo.USERNAME_PASSWORD + ); + + } + + /** + * Re-authenticates the given AuthenticatedUser against the + * AuthenticationProvider that originally created it, using the given + * Credentials. + * + * @param authenticatedUser + * The AuthenticatedUser to re-authenticate. + * + * @param credentials + * The Credentials to use to re-authenticate the user. + * + * @return + * A AuthenticatedUser which may have been updated due to re- + * authentication. + * + * @throws GuacamoleException + * If an error prevents the user from being re-authenticated. + */ + private AuthenticatedUser updateAuthenticatedUser(AuthenticatedUser authenticatedUser, + Credentials credentials) throws GuacamoleException { + + // Get original AuthenticationProvider + AuthenticationProvider authProvider = authenticatedUser.getAuthenticationProvider(); + + // Re-authenticate the AuthenticatedUser against the original AuthenticationProvider only + authenticatedUser = authProvider.updateAuthenticatedUser(authenticatedUser, credentials); + if (authenticatedUser == null) + throw new GuacamoleSecurityException("User re-authentication failed."); + + return authenticatedUser; + + } + + /** + * Returns the AuthenticatedUser associated with the given session and + * credentials, performing a fresh authentication and creating a new + * AuthenticatedUser if necessary. + * + * @param existingSession + * The current GuacamoleSession, or null if no session exists yet. + * + * @param credentials + * The Credentials to use to authenticate the user. + * + * @return + * The AuthenticatedUser associated with the given session and + * credentials. + * + * @throws GuacamoleException + * If an error occurs while authenticating or re-authenticating the + * user. + */ + private AuthenticatedUser getAuthenticatedUser(GuacamoleSession existingSession, + Credentials credentials) throws GuacamoleException { + + try { + + // Re-authenticate user if session exists + if (existingSession != null) + return updateAuthenticatedUser(existingSession.getAuthenticatedUser(), credentials); + + // Otherwise, attempt authentication as a new user + AuthenticatedUser authenticatedUser = AuthenticationService.this.authenticateUser(credentials); + if (logger.isInfoEnabled()) + logger.info("User \"{}\" successfully authenticated from {}.", + authenticatedUser.getIdentifier(), + getLoggableAddress(credentials.getRequest())); + + return authenticatedUser; + + } + + // Log and rethrow any authentication errors + catch (GuacamoleException e) { + + // Get request and username for sake of logging + HttpServletRequest request = credentials.getRequest(); + String username = credentials.getUsername(); + + // Log authentication failures with associated usernames + if (username != null) { + if (logger.isWarnEnabled()) + logger.warn("Authentication attempt from {} for user \"{}\" failed.", + getLoggableAddress(request), username); + } + + // Log anonymous authentication failures + else if (logger.isDebugEnabled()) + logger.debug("Anonymous authentication attempt from {} failed.", + getLoggableAddress(request)); + + // Rethrow exception + throw e; + + } + + } + + /** + * Returns all UserContexts associated with the given AuthenticatedUser, + * updating existing UserContexts, if any. If no UserContexts are yet + * associated with the given AuthenticatedUser, new UserContexts are + * generated by polling each available AuthenticationProvider. + * + * @param existingSession + * The current GuacamoleSession, or null if no session exists yet. + * + * @param authenticatedUser + * The AuthenticatedUser that has successfully authenticated or re- + * authenticated. + * + * @return + * A List of all UserContexts associated with the given + * AuthenticatedUser. + * + * @throws GuacamoleException + * If an error occurs while creating or updating any UserContext. + */ + private List getUserContexts(GuacamoleSession existingSession, + AuthenticatedUser authenticatedUser) throws GuacamoleException { + + List userContexts = new ArrayList(authProviders.size()); + + // If UserContexts already exist, update them and add to the list + if (existingSession != null) { + + // Update all old user contexts + List oldUserContexts = existingSession.getUserContexts(); + for (UserContext oldUserContext : oldUserContexts) { + + // Update existing UserContext + AuthenticationProvider authProvider = oldUserContext.getAuthenticationProvider(); + UserContext userContext = authProvider.updateUserContext(oldUserContext, authenticatedUser); + + // Add to available data, if successful + if (userContext != null) + userContexts.add(userContext); + + // If unsuccessful, log that this happened, as it may be a bug + else + logger.debug("AuthenticationProvider \"{}\" retroactively destroyed its UserContext.", + authProvider.getClass().getName()); + + } + + } + + // Otherwise, create new UserContexts from available AuthenticationProviders + else { + + // Get UserContexts from each available AuthenticationProvider + for (AuthenticationProvider authProvider : authProviders) { + + // Generate new UserContext + UserContext userContext = authProvider.getUserContext(authenticatedUser); + + // Add to available data, if successful + if (userContext != null) + userContexts.add(userContext); + + } + + } + + return userContexts; + + } + + /** + * Authenticates a user using the given credentials and optional + * authentication token, returning the authentication token associated with + * the user's Guacamole session, which may be newly generated. If an + * existing token is provided, the authentication procedure will attempt to + * update or reuse the provided token, but it is possible that a new token + * will be returned. Note that this function CANNOT return null. + * + * @param credentials + * The credentials to use when authenticating the user. + * + * @param token + * The authentication token to use if attempting to re-authenticate an + * existing session, or null to request a new token. + * + * @return + * The authentication token associated with the newly created or + * existing session. + * + * @throws GuacamoleException + * If the authentication or re-authentication attempt fails. + */ + public String authenticate(Credentials credentials, String token) + throws GuacamoleException { + + // Pull existing session if token provided + GuacamoleSession existingSession; + if (token != null) + existingSession = tokenSessionMap.get(token); + else + existingSession = null; + + // Get up-to-date AuthenticatedUser and associated UserContexts + AuthenticatedUser authenticatedUser = getAuthenticatedUser(existingSession, credentials); + List userContexts = getUserContexts(existingSession, authenticatedUser); + + // Update existing session, if it exists + String authToken; + if (existingSession != null) { + authToken = token; + existingSession.setAuthenticatedUser(authenticatedUser); + existingSession.setUserContexts(userContexts); + } + + // If no existing session, generate a new token/session pair + else { + authToken = authTokenGenerator.getToken(); + tokenSessionMap.put(authToken, new GuacamoleSession(environment, authenticatedUser, userContexts)); + logger.debug("Login was successful for user \"{}\".", authenticatedUser.getIdentifier()); + } + + return authToken; + + } + /** * Finds the Guacamole session for a given auth token, if the auth token * represents a currently logged in user. Throws an unauthorized error * otherwise. - * + * * @param authToken The auth token to check against the map of logged in users. * @return The session that corresponds to the provided auth token. * @throws GuacamoleException If the auth token does not correspond to any @@ -66,6 +427,32 @@ public class AuthenticationService { } + /** + * Invalidates a specific authentication token and its corresponding + * Guacamole session, effectively logging out the associated user. If the + * authentication token is not valid, this function has no effect. + * + * @param authToken + * The token being invalidated. + * + * @return + * true if the given authentication token was valid and the + * corresponding Guacamole session was destroyed, false if the given + * authentication token was not valid and no action was taken. + */ + public boolean destroyGuacamoleSession(String authToken) { + + // Remove corresponding GuacamoleSession if the token is valid + GuacamoleSession session = tokenSessionMap.remove(authToken); + if (session == null) + return false; + + // Invalidate the removed session + session.invalidate(); + return true; + + } + /** * Returns all UserContexts associated with a given auth token, if the auth * token represents a currently logged in user. Throws an unauthorized diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/auth/TokenRESTService.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/auth/TokenRESTService.java index 4351b890e..da64316c6 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/auth/TokenRESTService.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/auth/TokenRESTService.java @@ -26,7 +26,6 @@ import com.google.inject.Inject; import java.io.UnsupportedEncodingException; import java.util.ArrayList; import java.util.List; -import java.util.regex.Pattern; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.DELETE; import javax.ws.rs.FormParam; @@ -40,15 +39,9 @@ import javax.ws.rs.core.MultivaluedMap; import javax.xml.bind.DatatypeConverter; import org.glyptodon.guacamole.GuacamoleException; import org.glyptodon.guacamole.GuacamoleResourceNotFoundException; -import org.glyptodon.guacamole.GuacamoleSecurityException; -import org.glyptodon.guacamole.environment.Environment; import org.glyptodon.guacamole.net.auth.AuthenticatedUser; -import org.glyptodon.guacamole.net.auth.AuthenticationProvider; import org.glyptodon.guacamole.net.auth.Credentials; import org.glyptodon.guacamole.net.auth.UserContext; -import org.glyptodon.guacamole.net.auth.credentials.CredentialsInfo; -import org.glyptodon.guacamole.net.auth.credentials.GuacamoleCredentialsException; -import org.glyptodon.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException; import org.glyptodon.guacamole.net.basic.GuacamoleSession; import org.glyptodon.guacamole.net.basic.rest.APIRequest; import org.slf4j.Logger; @@ -64,81 +57,16 @@ import org.slf4j.LoggerFactory; @Produces(MediaType.APPLICATION_JSON) public class TokenRESTService { - /** - * The Guacamole server environment. - */ - @Inject - private Environment environment; - - /** - * All configured authentication providers which can be used to - * authenticate users or retrieve data associated with authenticated users. - */ - @Inject - private List authProviders; - - /** - * The map of auth tokens to sessions for the REST endpoints. - */ - @Inject - private TokenSessionMap tokenSessionMap; - - /** - * A generator for creating new auth tokens. - */ - @Inject - private AuthTokenGenerator authTokenGenerator; - /** * Logger for this class. */ private static final Logger logger = LoggerFactory.getLogger(TokenRESTService.class); /** - * Regular expression which matches any IPv4 address. + * Service for authenticating users and managing their Guacamole sessions. */ - 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 + ")*$"); - - /** - * Returns a formatted string containing an IP address, or list of IP - * addresses, which represent the HTTP client and any involved proxies. As - * the headers used to determine proxies can easily be forged, this data is - * superficially validated to ensure that it at least looks like a list of - * IPs. - * - * @param request - * The HTTP request to format. - * - * @return - * A formatted string containing one or more IP addresses. - */ - private String getLoggableAddress(HttpServletRequest request) { - - // Log X-Forwarded-For, if present and valid - String header = request.getHeader("X-Forwarded-For"); - if (header != null && X_FORWARDED_FOR.matcher(header).matches()) - return "[" + header + ", " + request.getRemoteAddr() + "]"; - - // If header absent or invalid, just use source IP - return request.getRemoteAddr(); - - } + @Inject + private AuthenticationService authenticationService; /** * Returns the credentials associated with the given request, using the @@ -205,228 +133,6 @@ public class TokenRESTService { } - /** - * Attempts authentication against all AuthenticationProviders, in order, - * using the provided credentials. The first authentication failure takes - * priority, but remaining AuthenticationProviders are attempted. If any - * AuthenticationProvider succeeds, the resulting AuthenticatedUser is - * returned, and no further AuthenticationProviders are tried. - * - * @param credentials - * The credentials to use for authentication. - * - * @return - * The AuthenticatedUser given by the highest-priority - * AuthenticationProvider for which the given credentials are valid. - * - * @throws GuacamoleException - * If the given credentials are not valid for any - * AuthenticationProvider, or if an error occurs while authenticating - * the user. - */ - private AuthenticatedUser authenticateUser(Credentials credentials) - throws GuacamoleException { - - GuacamoleCredentialsException authFailure = null; - - // Attempt authentication against each AuthenticationProvider - for (AuthenticationProvider authProvider : authProviders) { - - // Attempt authentication - try { - AuthenticatedUser authenticatedUser = authProvider.authenticateUser(credentials); - if (authenticatedUser != null) - return authenticatedUser; - } - - // First failure takes priority for now - catch (GuacamoleCredentialsException e) { - if (authFailure == null) - authFailure = e; - } - - } - - // If a specific failure occured, rethrow that - if (authFailure != null) - throw authFailure; - - // Otherwise, request standard username/password - throw new GuacamoleInvalidCredentialsException( - "Permission Denied.", - CredentialsInfo.USERNAME_PASSWORD - ); - - } - - /** - * Re-authenticates the given AuthenticatedUser against the - * AuthenticationProvider that originally created it, using the given - * Credentials. - * - * @param authenticatedUser - * The AuthenticatedUser to re-authenticate. - * - * @param credentials - * The Credentials to use to re-authenticate the user. - * - * @return - * A AuthenticatedUser which may have been updated due to re- - * authentication. - * - * @throws GuacamoleException - * If an error prevents the user from being re-authenticated. - */ - private AuthenticatedUser updateAuthenticatedUser(AuthenticatedUser authenticatedUser, - Credentials credentials) throws GuacamoleException { - - // Get original AuthenticationProvider - AuthenticationProvider authProvider = authenticatedUser.getAuthenticationProvider(); - - // Re-authenticate the AuthenticatedUser against the original AuthenticationProvider only - authenticatedUser = authProvider.updateAuthenticatedUser(authenticatedUser, credentials); - if (authenticatedUser == null) - throw new GuacamoleSecurityException("User re-authentication failed."); - - return authenticatedUser; - - } - - /** - * Returns the AuthenticatedUser associated with the given session and - * credentials, performing a fresh authentication and creating a new - * AuthenticatedUser if necessary. - * - * @param existingSession - * The current GuacamoleSession, or null if no session exists yet. - * - * @param credentials - * The Credentials to use to authenticate the user. - * - * @return - * The AuthenticatedUser associated with the given session and - * credentials. - * - * @throws GuacamoleException - * If an error occurs while authenticating or re-authenticating the - * user. - */ - private AuthenticatedUser getAuthenticatedUser(GuacamoleSession existingSession, - Credentials credentials) throws GuacamoleException { - - try { - - // Re-authenticate user if session exists - if (existingSession != null) - return updateAuthenticatedUser(existingSession.getAuthenticatedUser(), credentials); - - // Otherwise, attempt authentication as a new user - AuthenticatedUser authenticatedUser = authenticateUser(credentials); - if (logger.isInfoEnabled()) - logger.info("User \"{}\" successfully authenticated from {}.", - authenticatedUser.getIdentifier(), - getLoggableAddress(credentials.getRequest())); - - return authenticatedUser; - - } - - // Log and rethrow any authentication errors - catch (GuacamoleException e) { - - // Get request and username for sake of logging - HttpServletRequest request = credentials.getRequest(); - String username = credentials.getUsername(); - - // Log authentication failures with associated usernames - if (username != null) { - if (logger.isWarnEnabled()) - logger.warn("Authentication attempt from {} for user \"{}\" failed.", - getLoggableAddress(request), username); - } - - // Log anonymous authentication failures - else if (logger.isDebugEnabled()) - logger.debug("Anonymous authentication attempt from {} failed.", - getLoggableAddress(request)); - - // Rethrow exception - throw e; - - } - - } - - /** - * Returns all UserContexts associated with the given AuthenticatedUser, - * updating existing UserContexts, if any. If no UserContexts are yet - * associated with the given AuthenticatedUser, new UserContexts are - * generated by polling each available AuthenticationProvider. - * - * @param existingSession - * The current GuacamoleSession, or null if no session exists yet. - * - * @param authenticatedUser - * The AuthenticatedUser that has successfully authenticated or re- - * authenticated. - * - * @return - * A List of all UserContexts associated with the given - * AuthenticatedUser. - * - * @throws GuacamoleException - * If an error occurs while creating or updating any UserContext. - */ - private List getUserContexts(GuacamoleSession existingSession, - AuthenticatedUser authenticatedUser) throws GuacamoleException { - - List userContexts = new ArrayList(authProviders.size()); - - // If UserContexts already exist, update them and add to the list - if (existingSession != null) { - - // Update all old user contexts - List oldUserContexts = existingSession.getUserContexts(); - for (UserContext oldUserContext : oldUserContexts) { - - // Update existing UserContext - AuthenticationProvider authProvider = oldUserContext.getAuthenticationProvider(); - UserContext userContext = authProvider.updateUserContext(oldUserContext, authenticatedUser); - - // Add to available data, if successful - if (userContext != null) - userContexts.add(userContext); - - // If unsuccessful, log that this happened, as it may be a bug - else - logger.debug("AuthenticationProvider \"{}\" retroactively destroyed its UserContext.", - authProvider.getClass().getName()); - - } - - } - - // Otherwise, create new UserContexts from available AuthenticationProviders - else { - - // Get UserContexts from each available AuthenticationProvider - for (AuthenticationProvider authProvider : authProviders) { - - // Generate new UserContext - UserContext userContext = authProvider.getUserContext(authenticatedUser); - - // Add to available data, if successful - if (userContext != null) - userContexts.add(userContext); - - } - - } - - return userContexts; - - } - /** * Authenticates a user, generates an auth token, associates that auth token * with the user's UserContext for use by further requests. If an existing @@ -472,43 +178,27 @@ public class TokenRESTService { // Reconstitute the HTTP request with the map of parameters HttpServletRequest request = new APIRequest(consumedRequest, parameters); - // Pull existing session if token provided - GuacamoleSession existingSession; - if (token != null) - existingSession = tokenSessionMap.get(token); - else - existingSession = null; - // Build credentials from request Credentials credentials = getCredentials(request, username, password); - // Get up-to-date AuthenticatedUser and associated UserContexts - AuthenticatedUser authenticatedUser = getAuthenticatedUser(existingSession, credentials); - List userContexts = getUserContexts(existingSession, authenticatedUser); + // Create/update session producing possibly-new token + token = authenticationService.authenticate(credentials, token); - // Update existing session, if it exists - String authToken; - if (existingSession != null) { - authToken = token; - existingSession.setAuthenticatedUser(authenticatedUser); - existingSession.setUserContexts(userContexts); - } - - // If no existing session, generate a new token/session pair - else { - authToken = authTokenGenerator.getToken(); - tokenSessionMap.put(authToken, new GuacamoleSession(environment, authenticatedUser, userContexts)); - logger.debug("Login was successful for user \"{}\".", authenticatedUser.getIdentifier()); - } + // Pull corresponding session + GuacamoleSession session = authenticationService.getGuacamoleSession(token); + if (session == null) + throw new GuacamoleResourceNotFoundException("No such token."); // Build list of all available auth providers + List userContexts = session.getUserContexts(); List authProviderIdentifiers = new ArrayList(userContexts.size()); for (UserContext userContext : userContexts) authProviderIdentifiers.add(userContext.getAuthenticationProvider().getIdentifier()); // Return possibly-new auth token + AuthenticatedUser authenticatedUser = session.getAuthenticatedUser(); return new APIAuthenticationResult( - authToken, + token, authenticatedUser.getIdentifier(), authenticatedUser.getAuthenticationProvider().getIdentifier(), authProviderIdentifiers @@ -530,12 +220,10 @@ public class TokenRESTService { @Path("/{token}") public void invalidateToken(@PathParam("token") String authToken) throws GuacamoleException { - - GuacamoleSession session = tokenSessionMap.remove(authToken); - if (session == null) - throw new GuacamoleResourceNotFoundException("No such token."); - session.invalidate(); + // Invalidate session, if it exists + if (!authenticationService.destroyGuacamoleSession(authToken)) + throw new GuacamoleResourceNotFoundException("No such token."); } From 2a155cdbac5fa3af448e6f14c664d33e6b821190 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 16 Oct 2015 14:07:08 -0700 Subject: [PATCH 8/9] GUAC-1364: Implicitly invalidate user session if unauthorized upon connect. --- .../net/basic/TunnelRequestService.java | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/TunnelRequestService.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/TunnelRequestService.java index 9f8fde40b..eea85a7f1 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/TunnelRequestService.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/TunnelRequestService.java @@ -27,6 +27,7 @@ import com.google.inject.Singleton; import java.util.List; import org.glyptodon.guacamole.GuacamoleException; import org.glyptodon.guacamole.GuacamoleSecurityException; +import org.glyptodon.guacamole.GuacamoleUnauthorizedException; import org.glyptodon.guacamole.net.DelegatingGuacamoleTunnel; import org.glyptodon.guacamole.net.GuacamoleTunnel; import org.glyptodon.guacamole.net.auth.Connection; @@ -228,8 +229,8 @@ public class TunnelRequestService { * @throws GuacamoleException * If an error occurs while obtaining the tunnel. */ - protected GuacamoleTunnel createAssociatedTunnel(final GuacamoleSession session, - GuacamoleTunnel tunnel, final TunnelRequest.Type type, + protected GuacamoleTunnel createAssociatedTunnel(GuacamoleTunnel tunnel, + final GuacamoleSession session, final TunnelRequest.Type type, final String id) throws GuacamoleException { // Monitor tunnel closure and data @@ -305,13 +306,30 @@ public class TunnelRequestService { String authProviderIdentifier = request.getAuthenticationProviderIdentifier(); GuacamoleClientInformation info = getClientInformation(request); - // Create connected tunnel using provided connection ID and client information GuacamoleSession session = authenticationService.getGuacamoleSession(authToken); UserContext userContext = retrievalService.retrieveUserContext(session, authProviderIdentifier); - GuacamoleTunnel tunnel = createConnectedTunnel(userContext, type, id, info); - // Associate tunnel with session - return createAssociatedTunnel(session, tunnel, type, id); + try { + + // Create connected tunnel using provided connection ID and client information + GuacamoleTunnel tunnel = createConnectedTunnel(userContext, type, id, info); + + // Associate tunnel with session + return createAssociatedTunnel(tunnel, session, type, id); + + } + + // Ensure any associated session is invalidated if unauthorized + catch (GuacamoleUnauthorizedException e) { + + // If there is an associated auth token, invalidate it + if (authenticationService.destroyGuacamoleSession(authToken)) + logger.debug("Implicitly invalidated session for token \"{}\".", authToken); + + // Continue with exception processing + throw e; + + } } From 7ae531d3c2e284a3ca1c308a74e94d0746bf3228 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 16 Oct 2015 14:55:19 -0700 Subject: [PATCH 9/9] GUAC-1364: Implicitly invalidate user session if unauthorized upon disconnect. --- .../net/basic/TunnelRequestService.java | 35 +++++++++++++++---- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/TunnelRequestService.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/TunnelRequestService.java index eea85a7f1..6a2ef7a3b 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/TunnelRequestService.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/TunnelRequestService.java @@ -212,6 +212,12 @@ public class TunnelRequestService { * @param tunnel * The connected tunnel to wrap and monitor. * + * @param authToken + * The authentication token associated with the given session. If + * provided, this token will be automatically invalidated (and the + * corresponding session destroyed) if tunnel errors imply that the + * user is no longer authorized. + * * @param session * The Guacamole session to associate the tunnel with. * @@ -230,8 +236,9 @@ public class TunnelRequestService { * If an error occurs while obtaining the tunnel. */ protected GuacamoleTunnel createAssociatedTunnel(GuacamoleTunnel tunnel, - final GuacamoleSession session, final TunnelRequest.Type type, - final String id) throws GuacamoleException { + final String authToken, final GuacamoleSession session, + final TunnelRequest.Type type, final String id) + throws GuacamoleException { // Monitor tunnel closure and data GuacamoleTunnel monitoredTunnel = new DelegatingGuacamoleTunnel(tunnel) { @@ -269,9 +276,25 @@ public class TunnelRequestService { } - // Close and clean up tunnel - session.removeTunnel(getUUID().toString()); - super.close(); + try { + + // Close and clean up tunnel + session.removeTunnel(getUUID().toString()); + super.close(); + + } + + // Ensure any associated session is invalidated if unauthorized + catch (GuacamoleUnauthorizedException e) { + + // If there is an associated auth token, invalidate it + if (authenticationService.destroyGuacamoleSession(authToken)) + logger.debug("Implicitly invalidated session for token \"{}\".", authToken); + + // Continue with exception processing + throw e; + + } } @@ -315,7 +338,7 @@ public class TunnelRequestService { GuacamoleTunnel tunnel = createConnectedTunnel(userContext, type, id, info); // Associate tunnel with session - return createAssociatedTunnel(tunnel, session, type, id); + return createAssociatedTunnel(tunnel, authToken, session, type, id); }