From c325f443a56c67e95a670a67c6ab958fc22e83d2 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 7 Aug 2013 14:51:14 -0700 Subject: [PATCH] Update user context appropriately. Use UserContext for events. --- .../net/event/AuthenticationSuccessEvent.java | 22 ++- .../guacamole/net/event/TunnelCloseEvent.java | 20 +-- .../net/event/TunnelConnectEvent.java | 20 +-- .../guacamole/net/event/UserEvent.java | 21 +++ .../net/basic/AuthenticatingHttpServlet.java | 140 ++++++++---------- .../basic/BasicGuacamoleTunnelServlet.java | 30 ++-- 6 files changed, 136 insertions(+), 117 deletions(-) create mode 100644 guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/UserEvent.java diff --git a/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/AuthenticationSuccessEvent.java b/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/AuthenticationSuccessEvent.java index 80986fd6c..f8ff3e4b0 100644 --- a/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/AuthenticationSuccessEvent.java +++ b/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/AuthenticationSuccessEvent.java @@ -1,15 +1,25 @@ package net.sourceforge.guacamole.net.event; import net.sourceforge.guacamole.net.auth.Credentials; +import net.sourceforge.guacamole.net.auth.UserContext; /** * An event which is triggered whenever a user's credentials pass * authentication. The credentials that passed authentication are included * within this event, and can be retrieved using getCredentials(). + * + * Note that this event is only triggered when the UserContext is initially + * created. Any further updates to the UserContext to not trigger this event. * * @author Michael Jumper */ -public class AuthenticationSuccessEvent implements CredentialEvent { +public class AuthenticationSuccessEvent implements UserEvent, CredentialEvent { + + /** + * The UserContext associated with the request that is connecting the + * tunnel, if any. + */ + private UserContext context; /** * The credentials which passed authentication. @@ -20,12 +30,20 @@ public class AuthenticationSuccessEvent implements CredentialEvent { * Creates a new AuthenticationSuccessEvent which represents a successful * authentication attempt with the given credentials. * + * @param context The UserContext created as a result of successful + * authentication. * @param credentials The credentials which passed authentication. */ - public AuthenticationSuccessEvent(Credentials credentials) { + public AuthenticationSuccessEvent(UserContext context, Credentials credentials) { + this.context = context; this.credentials = credentials; } + @Override + public UserContext getUserContext() { + return context; + } + @Override public Credentials getCredentials() { return credentials; diff --git a/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/TunnelCloseEvent.java b/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/TunnelCloseEvent.java index d89715514..55670d0ed 100644 --- a/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/TunnelCloseEvent.java +++ b/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/TunnelCloseEvent.java @@ -1,7 +1,7 @@ package net.sourceforge.guacamole.net.event; import net.sourceforge.guacamole.net.GuacamoleTunnel; -import net.sourceforge.guacamole.net.auth.Credentials; +import net.sourceforge.guacamole.net.auth.UserContext; /** * An event which is triggered whenever a tunnel is being closed. The tunnel @@ -11,13 +11,13 @@ import net.sourceforge.guacamole.net.auth.Credentials; * * @author Michael Jumper */ -public class TunnelCloseEvent implements CredentialEvent, TunnelEvent { +public class TunnelCloseEvent implements UserEvent, TunnelEvent { /** - * The credentials associated with the request that is closing the + * The UserContext associated with the request that is connecting the * tunnel, if any. */ - private Credentials credentials; + private UserContext context; /** * The tunnel being closed. @@ -28,18 +28,18 @@ public class TunnelCloseEvent implements CredentialEvent, TunnelEvent { * Creates a new TunnelCloseEvent which represents the closing of the * given tunnel via a request associated with the given credentials. * - * @param credentials The credentials associated with the request - * closing the tunnel. + * @param context The UserContext associated with the request connecting + * the tunnel. * @param tunnel The tunnel being closed. */ - public TunnelCloseEvent(Credentials credentials, GuacamoleTunnel tunnel) { - this.credentials = credentials; + public TunnelCloseEvent(UserContext context, GuacamoleTunnel tunnel) { + this.context = context; this.tunnel = tunnel; } @Override - public Credentials getCredentials() { - return credentials; + public UserContext getUserContext() { + return context; } @Override diff --git a/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/TunnelConnectEvent.java b/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/TunnelConnectEvent.java index b1608111e..5fce47a63 100644 --- a/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/TunnelConnectEvent.java +++ b/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/TunnelConnectEvent.java @@ -1,7 +1,7 @@ package net.sourceforge.guacamole.net.event; import net.sourceforge.guacamole.net.GuacamoleTunnel; -import net.sourceforge.guacamole.net.auth.Credentials; +import net.sourceforge.guacamole.net.auth.UserContext; /** * An event which is triggered whenever a tunnel is being connected. The tunnel @@ -11,13 +11,13 @@ import net.sourceforge.guacamole.net.auth.Credentials; * * @author Michael Jumper */ -public class TunnelConnectEvent implements CredentialEvent, TunnelEvent { +public class TunnelConnectEvent implements UserEvent, TunnelEvent { /** - * The credentials associated with the request that is connecting the + * The UserContext associated with the request that is connecting the * tunnel, if any. */ - private Credentials credentials; + private UserContext context; /** * The tunnel being connected. @@ -28,18 +28,18 @@ public class TunnelConnectEvent implements CredentialEvent, TunnelEvent { * Creates a new TunnelConnectEvent which represents the connecting of the * given tunnel via a request associated with the given credentials. * - * @param credentials The credentials associated with the request - * connecting the tunnel. + * @param context The UserContext associated with the request connecting + * the tunnel. * @param tunnel The tunnel being connected. */ - public TunnelConnectEvent(Credentials credentials, GuacamoleTunnel tunnel) { - this.credentials = credentials; + public TunnelConnectEvent(UserContext context, GuacamoleTunnel tunnel) { + this.context = context; this.tunnel = tunnel; } @Override - public Credentials getCredentials() { - return credentials; + public UserContext getUserContext() { + return context; } @Override diff --git a/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/UserEvent.java b/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/UserEvent.java new file mode 100644 index 000000000..8b3578c27 --- /dev/null +++ b/guacamole-ext/src/main/java/net/sourceforge/guacamole/net/event/UserEvent.java @@ -0,0 +1,21 @@ +package net.sourceforge.guacamole.net.event; + +import net.sourceforge.guacamole.net.auth.UserContext; + +/** + * Abstract basis for events which may have an associated UserContext when + * triggered. + * + * @author Michael Jumper + */ +public interface UserEvent { + + /** + * Returns the current UserContext of the user triggering the event, if any. + * + * @return The current UserContext of the user triggering the event, if + * any, or null if no UserContext is associated with the event. + */ + UserContext getUserContext(); + +} diff --git a/guacamole/src/main/java/net/sourceforge/guacamole/net/basic/AuthenticatingHttpServlet.java b/guacamole/src/main/java/net/sourceforge/guacamole/net/basic/AuthenticatingHttpServlet.java index 42fcc3a7d..9b3bb73b9 100644 --- a/guacamole/src/main/java/net/sourceforge/guacamole/net/basic/AuthenticatingHttpServlet.java +++ b/guacamole/src/main/java/net/sourceforge/guacamole/net/basic/AuthenticatingHttpServlet.java @@ -72,11 +72,6 @@ public abstract class AuthenticatingHttpServlet extends HttpServlet { */ private static final String CONTEXT_ATTRIBUTE = "GUAC_CONTEXT"; - /** - * The session attribute holding the credentials authorizing this session. - */ - private static final String CREDENTIALS_ATTRIBUTE = "GUAC_CREDS"; - /** * The AuthenticationProvider to use to authenticate all requests. */ @@ -127,6 +122,8 @@ public abstract class AuthenticatingHttpServlet extends HttpServlet { * successful. * * @param listeners A collection of all listeners that should be notified. + * @param context The UserContext created as a result of authentication + * success. * @param credentials The credentials associated with the authentication * request that succeeded. * @return true if all listeners are allowing the authentication success, @@ -138,11 +135,12 @@ public abstract class AuthenticatingHttpServlet extends HttpServlet { * error, the success is canceled, and no other * listeners will run. */ - private boolean notifySuccess(Collection listeners, Credentials credentials) - throws GuacamoleException { + private boolean notifySuccess(Collection listeners, UserContext context, + Credentials credentials) throws GuacamoleException { // Build event for auth success - AuthenticationSuccessEvent event = new AuthenticationSuccessEvent(credentials); + AuthenticationSuccessEvent event = + new AuthenticationSuccessEvent(context, credentials); // Notify all listeners for (Object listener : listeners) { @@ -203,17 +201,6 @@ public abstract class AuthenticatingHttpServlet extends HttpServlet { } - - /** - * Returns the credentials associated with the given session. - * - * @param session The session to retrieve credentials from. - * @return The credentials associated with the given session. - */ - protected Credentials getCredentials(HttpSession session) { - return (Credentials) session.getAttribute(CREDENTIALS_ATTRIBUTE); - } - /** * Returns the UserContext associated with the given session. * @@ -239,89 +226,90 @@ public abstract class AuthenticatingHttpServlet extends HttpServlet { HttpSession httpSession = request.getSession(true); + SessionListenerCollection listeners; + try { + listeners = new SessionListenerCollection(httpSession); + } + catch (GuacamoleException e) { + logger.error("Failed to retrieve listeners. Authentication canceled.", e); + failAuthentication(response); + return; + } + + // Build credentials object + Credentials credentials = new Credentials(); + credentials.setSession(httpSession); + credentials.setRequest(request); + // Try to get user context from session UserContext context = getUserContext(httpSession); - // If no context, try to authenticate the user to get the context using - // this request. + // If no cached context, attempt to get new context if (context == null) { - SessionListenerCollection listeners; - try { - listeners = new SessionListenerCollection(httpSession); - } - catch (GuacamoleException e) { - logger.error("Failed to retrieve listeners. Authentication canceled.", e); - failAuthentication(response); - return; - } - - // Build credentials object - Credentials credentials = new Credentials(); - credentials.setSession(httpSession); - credentials.setRequest(request); - - // Get authorized context try { context = authProvider.getUserContext(credentials); } - - /******** HANDLE FAILED AUTHENTICATION ********/ - - // If error retrieving context, fail authentication, notify listeners + // Log any authentication errors catch (GuacamoleException e) { logger.error("Error retrieving context for user \"{}\".", credentials.getUsername(), e); - - notifyFailed(listeners, credentials); - failAuthentication(response); - return; } - // If no context, fail authentication, notify listeners - if (context == null) { - logger.warn("Authentication attempt from {} for user \"{}\" failed.", - request.getRemoteAddr(), credentials.getUsername()); - - notifyFailed(listeners, credentials); - failAuthentication(response); - return; - } - - - /******** HANDLE SUCCESSFUL AUTHENTICATION ********/ - - try { - - // Otherwise, authentication has been succesful + // If successful, log success and notify listeners + if (context != null) { + + // Log successful authentication logger.info("User \"{}\" successfully authenticated from {}.", - credentials.getUsername(), request.getRemoteAddr()); + context.self().getUsername(), request.getRemoteAddr()); - // Notify of success, cancel if requested - if (!notifySuccess(listeners, credentials)) { - logger.info("Successful authentication canceled by hook."); - failAuthentication(response); - return; + // Notify any listeners of success, cancel if requested + try { + if (!notifySuccess(listeners, context, credentials)) { + logger.info("Successful authentication canceled by hook."); + context = null; + } } - } - catch (GuacamoleException e) { - // Cancel authentication success if hook throws exception - logger.error("Successful authentication canceled by error in hook.", e); - failAuthentication(response); - return; + catch (GuacamoleException e) { + logger.error("Successful authentication canceled by error in hook.", e); + context = null; + } + } // end if auth success + + } // end if no cached context + + // Otherwise, update existing context + else { + + try { + context = authProvider.updateUserContext(context, credentials); } - // Associate context and credentials with session - httpSession.setAttribute(CONTEXT_ATTRIBUTE, context); - httpSession.setAttribute(CREDENTIALS_ATTRIBUTE, credentials); + // If error updating context, fail authentication, notify listeners + catch (GuacamoleException e) { + logger.error("Error updating context for user \"{}\".", + context.self().getUsername(), e); + } + } // end if cached context + // If no context, fail authentication, notify listeners + if (context == null) { + logger.warn("Authentication attempt from {} for user \"{}\" failed.", + request.getRemoteAddr(), credentials.getUsername()); + + notifyFailed(listeners, credentials); + failAuthentication(response); + return; } + // Associate context and credentials with session + httpSession.setAttribute(CONTEXT_ATTRIBUTE, context); + try { // Allow servlet to run now that authentication has been validated diff --git a/guacamole/src/main/java/net/sourceforge/guacamole/net/basic/BasicGuacamoleTunnelServlet.java b/guacamole/src/main/java/net/sourceforge/guacamole/net/basic/BasicGuacamoleTunnelServlet.java index f61a8bf1b..f02aaacda 100644 --- a/guacamole/src/main/java/net/sourceforge/guacamole/net/basic/BasicGuacamoleTunnelServlet.java +++ b/guacamole/src/main/java/net/sourceforge/guacamole/net/basic/BasicGuacamoleTunnelServlet.java @@ -30,7 +30,6 @@ import net.sourceforge.guacamole.GuacamoleSecurityException; import net.sourceforge.guacamole.net.GuacamoleSocket; import net.sourceforge.guacamole.net.GuacamoleTunnel; import net.sourceforge.guacamole.net.auth.Connection; -import net.sourceforge.guacamole.net.auth.Credentials; import net.sourceforge.guacamole.net.auth.Directory; import net.sourceforge.guacamole.net.auth.UserContext; import net.sourceforge.guacamole.net.basic.event.SessionListenerCollection; @@ -85,8 +84,7 @@ public class BasicGuacamoleTunnelServlet extends AuthenticatingHttpServlet { * connected. * * @param listeners A collection of all listeners that should be notified. - * @param credentials The credentials associated with the authentication - * request that connected the tunnel. + * @param context The UserContext associated with the current session. * @param tunnel The tunnel being connected. * @return true if all listeners are allowing the tunnel to connect, * or if there are no listeners, and false if any listener is @@ -98,11 +96,11 @@ public class BasicGuacamoleTunnelServlet extends AuthenticatingHttpServlet { * listeners will run. */ private boolean notifyConnect(Collection listeners, - Credentials credentials, GuacamoleTunnel tunnel) + UserContext context, GuacamoleTunnel tunnel) throws GuacamoleException { // Build event for auth success - TunnelConnectEvent event = new TunnelConnectEvent(credentials, tunnel); + TunnelConnectEvent event = new TunnelConnectEvent(context, tunnel); // Notify all listeners for (Object listener : listeners) { @@ -124,8 +122,7 @@ public class BasicGuacamoleTunnelServlet extends AuthenticatingHttpServlet { * closed. * * @param listeners A collection of all listeners that should be notified. - * @param credentials The credentials associated with the authentication - * request that closed the tunnel. + * @param context The UserContext associated with the current session. * @param tunnel The tunnel being closed. * @return true if all listeners are allowing the tunnel to close, * or if there are no listeners, and false if any listener is @@ -137,11 +134,11 @@ public class BasicGuacamoleTunnelServlet extends AuthenticatingHttpServlet { * listeners will run. */ private boolean notifyClose(Collection listeners, - Credentials credentials, GuacamoleTunnel tunnel) + UserContext context, GuacamoleTunnel tunnel) throws GuacamoleException { // Build event for auth success - TunnelCloseEvent event = new TunnelCloseEvent(credentials, tunnel); + TunnelCloseEvent event = new TunnelCloseEvent(context, tunnel); // Notify all listeners for (Object listener : listeners) { @@ -182,19 +179,14 @@ public class BasicGuacamoleTunnelServlet extends AuthenticatingHttpServlet { // Get ID of connection String id = request.getParameter("id"); - // Get credentials - final Credentials credentials = getCredentials(httpSession); - // Get context - UserContext context = getUserContext(httpSession); + final UserContext context = getUserContext(httpSession); + if (context == null) + throw new GuacamoleSecurityException("Cannot connect - user not logged in."); // Get connection directory Directory directory = context.getConnectionDirectory(); - // If no credentials in session, not authorized - if (credentials == null) - throw new GuacamoleSecurityException("Cannot connect - user not logged in."); - // Get authorized connection Connection connection = directory.get(id); if (connection == null) { @@ -237,7 +229,7 @@ public class BasicGuacamoleTunnelServlet extends AuthenticatingHttpServlet { public void close() throws GuacamoleException { // Only close if not canceled - if (!notifyClose(listeners, credentials, this)) + if (!notifyClose(listeners, context, this)) throw new GuacamoleException("Tunnel close canceled by listener."); // Close if no exception due to listener @@ -248,7 +240,7 @@ public class BasicGuacamoleTunnelServlet extends AuthenticatingHttpServlet { }; // Notify listeners about connection - if (!notifyConnect(listeners, credentials, tunnel)) { + if (!notifyConnect(listeners, context, tunnel)) { logger.info("Connection canceled by listener."); return null; }