diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/AuthenticationSessionManager.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/AuthenticationSessionManager.java index 3261228b8..11ef307e2 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/AuthenticationSessionManager.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/AuthenticationSessionManager.java @@ -19,8 +19,10 @@ package org.apache.guacamole.auth.sso; -import com.google.common.base.Predicates; import com.google.inject.Inject; +import com.google.inject.Singleton; + +import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.Executors; @@ -37,6 +39,7 @@ import java.util.concurrent.TimeUnit; * @param * The type of sessions managed by this session manager. */ +@Singleton public class AuthenticationSessionManager { /** @@ -51,6 +54,16 @@ public class AuthenticationSessionManager { */ private final ConcurrentMap sessions = new ConcurrentHashMap<>(); + /** + * Set of identifiers of all sessions that are in a pending state, meaning + * that the session was successfully created, but the overall auth result + * has not yet been determined. + * + * Exposed as a ConcurrentMap instead of a Set because there is no + * ConcurrentSet class offering the required atomic operations. + */ + private final ConcurrentMap pendingSessions = new ConcurrentHashMap<>(); + /** * Executor service which runs the periodic cleanup task */ @@ -64,7 +77,13 @@ public class AuthenticationSessionManager { */ public AuthenticationSessionManager() { executor.scheduleAtFixedRate(() -> { - sessions.values().removeIf(Predicates.not(AuthenticationSession::isValid)); + + // Invalidate any stale sessions + for (Map.Entry entry : sessions.entrySet()) { + if (!entry.getValue().isValid()) + invalidateSession(entry.getKey()); + } + }, 1, 1, TimeUnit.MINUTES); } @@ -82,6 +101,43 @@ public class AuthenticationSessionManager { return idGenerator.generateIdentifier(); } + /** + * Remove the session associated with the given identifier, if any, from the + * map of sessions, and the set of pending sessions. + * + * @param identifier + * The identifier of the session to remove, if one exists. + */ + public void invalidateSession(String identifier) { + + // Do not attempt to remove a null identifier + if (identifier == null) + return; + + // Remove from the overall list of sessions + sessions.remove(identifier); + + // Remove from the set of pending sessions + pendingSessions.remove(identifier); + + } + + /** + * Reactivate (remove from pending) the session associated with the given + * session identifier, if any. After calling this method, any session with + * the given identifier will be ready to be resumed again. + * + * @param identifier + * The identifier of the session to reactivate, if one exists. + */ + public void reactivateSession(String identifier) { + + // Remove from the set of pending sessions to reactivate + if (identifier != null) + pendingSessions.remove(identifier); + + } + /** * Resumes the Guacamole side of the authentication process that was * previously deferred through a call to defer(). Once invoked, the @@ -97,9 +153,21 @@ public class AuthenticationSessionManager { * value was returned by defer(). */ public T resume(String identifier) { - if (identifier != null) { - T session = sessions.remove(identifier); + + T session = sessions.get(identifier); + + // Mark the session as pending. NOTE: Unless explicitly removed + // from pending status via a call to reactivateSession(), + // the next attempt to resume this session will fail + if (pendingSessions.putIfAbsent(identifier, true) != null) { + + // If the session was already marked as pending, invalidate it + invalidateSession(identifier); + return null; + + } + if (session != null && session.isValid()) return session; } diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/SSOAuthenticationEventListener.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/SSOAuthenticationEventListener.java new file mode 100644 index 000000000..46df4fd49 --- /dev/null +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/SSOAuthenticationEventListener.java @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.auth.sso; + +import org.apache.guacamole.GuacamoleClientException; +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.GuacamoleSecurityException; +import org.apache.guacamole.net.auth.Credentials; +import org.apache.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException; +import org.apache.guacamole.net.event.AuthenticationFailureEvent; +import org.apache.guacamole.net.event.AuthenticationRequestReceivedEvent; +import org.apache.guacamole.net.event.CredentialEvent; +import org.apache.guacamole.net.event.listener.Listener; + +/** + * A Listener that will reactivate or invalidate SSO auth sessions depending on + * overall auth success or failure. + */ +public abstract class SSOAuthenticationEventListener implements Listener { + + @Override + public void handleEvent(Object event) throws GuacamoleException { + + // If the authentication attempt is incomplete or credentials cannot be + // extracted, there's nothing to do + if (event instanceof AuthenticationRequestReceivedEvent + || !(event instanceof CredentialEvent)) + return; + + // Look for a session identifier associated with these credentials + String sessionIdentifier = getSessionIdentifier( + ((CredentialEvent) event).getCredentials()); + + // If no session is associated with these credentials, there's + // nothing to do + if (sessionIdentifier == null) + return; + + // If the SSO auth succeeded, but other auth providers failed to + // authenticate the user associated with the credentials in this + // failure event, they may wish to make another login attempt. To + // avoid an infinite login attempt loop, re-enable the session + // associated with these credentials, allowing the auth attempt to be + // resumed without requiring another round trip to the SSO service. + if (event instanceof AuthenticationFailureEvent) { + Throwable failure = ((AuthenticationFailureEvent) event).getFailure(); + + // If and only if the failure was associated with missing or + // credentials, or a non-security related request issue, + // reactivate the session + if (failure instanceof GuacamoleInsufficientCredentialsException + || ((failure instanceof GuacamoleClientException) + && !(failure instanceof GuacamoleSecurityException))) { + + reactivateSession(sessionIdentifier); + return; + + } + + } + + // Invalidate the session in all other cases + invalidateSession(sessionIdentifier); + + } + + /** + * Get the session identifier associated with the provided credentials, + * if any. If no session is associated with the credentials, null will + * be returned. + * + * @param credentials + * The credentials assoociated with the deferred SSO authentication + * session to reactivate. + * + * @return + * The session identifier associated with the provided credentials, + * or null if no session is found. + */ + protected abstract String getSessionIdentifier(Credentials credentials); + + /** + * Reactivate the session identified by the provided identifier, if any. + * + * @param sessionIdentifier + * The identifier of the session to reactivate. + */ + protected abstract void reactivateSession(String sessionIdentifier); + + /** + * Invalidate the session identified by the provided identifier, if any. + * + * @param sessionIdentifier + * The identifier of the session to invalidate. + */ + protected abstract void invalidateSession(String sessionIdentifier); + +} diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/AuthenticationProviderService.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/AuthenticationProviderService.java index 982028f16..2c7990c4f 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/AuthenticationProviderService.java @@ -69,6 +69,27 @@ public class AuthenticationProviderService implements SSOAuthenticationProviderS @Inject private SAMLService saml; + /** + * Return the value of the session identifier associated with the given + * credentials, or null if no session identifier is found in the + * credentials. + * + * @param credentials + * The credentials from which to extract the session identifier. + * + * @return + * The session identifier associated with the given credentials, or + * null if no identifier is found. + */ + public static String getSessionIdentifier(Credentials credentials) { + + // Return the session identifier from the request params, if set, or + // null otherwise + return credentials != null && credentials.getRequest() != null + ? credentials.getRequest().getParameter(AUTH_SESSION_QUERY_PARAM) + : null; + } + @Override public SAMLAuthenticatedUser authenticateUser(Credentials credentials) throws GuacamoleException { @@ -80,7 +101,9 @@ public class AuthenticationProviderService implements SSOAuthenticationProviderS return null; // Use established SAML identity if already provided by the SAML IdP - AssertedIdentity identity = sessionManager.getIdentity(request.getParameter(AUTH_SESSION_QUERY_PARAM)); + AssertedIdentity identity = sessionManager.getIdentity( + getSessionIdentifier(credentials)); + if (identity != null) { // Back-port the username to the credentials diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/SAMLAuthenticationEventListener.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/SAMLAuthenticationEventListener.java new file mode 100644 index 000000000..f6bb3b06c --- /dev/null +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/SAMLAuthenticationEventListener.java @@ -0,0 +1,66 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.auth.saml; + +import org.apache.guacamole.auth.saml.acs.SAMLAuthenticationSessionManager; +import org.apache.guacamole.auth.sso.SSOAuthenticationEventListener; +import org.apache.guacamole.net.auth.Credentials; + +import com.google.inject.Inject; + +/** + * A Listener that will reactivate or invalidate SAML auth sessions depending on + * overall auth success or failure. + */ +public class SAMLAuthenticationEventListener extends SSOAuthenticationEventListener { + + /** + * Session manager for generating and maintaining unique tokens to + * represent the authentication flow of a user who has only partially + * authenticated. + * + * Requires static injection due to the fact that the webapp just calls the + * constructor directly when creating new Listeners. The instances will not + * be constructed by guice. + * + * Note that is possible to instead inject an AuthenticationSessionManager + * instance directly into the base class, but this results in different + * instances of the session manager injected here and in the rest of the + * extension, regardless of singleton configuration for the service. + */ + @Inject + protected static SAMLAuthenticationSessionManager sessionManager; + + @Override + protected String getSessionIdentifier(Credentials credentials) { + return AuthenticationProviderService.getSessionIdentifier(credentials); + } + + @Override + protected void reactivateSession(String sessionIdentifier) { + sessionManager.reactivateSession(sessionIdentifier); + } + + @Override + protected void invalidateSession(String sessionIdentifier) { + sessionManager.invalidateSession(sessionIdentifier); + } + +} diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/SAMLAuthenticationProviderModule.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/SAMLAuthenticationProviderModule.java index 7c7dd49ed..e01880a62 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/SAMLAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/SAMLAuthenticationProviderModule.java @@ -36,6 +36,8 @@ public class SAMLAuthenticationProviderModule extends AbstractModule { bind(ConfigurationService.class); bind(SAMLAuthenticationSessionManager.class); bind(SAMLService.class); + + requestStaticInjection(SAMLAuthenticationEventListener.class); } } diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/resources/guac-manifest.json b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/resources/guac-manifest.json index e9ec44ec3..bfe0e4211 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/resources/guac-manifest.json +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/resources/guac-manifest.json @@ -9,6 +9,10 @@ "org.apache.guacamole.auth.saml.SAMLAuthenticationProvider" ], + "listeners" : [ + "org.apache.guacamole.auth.saml.SAMLAuthenticationEventListener" + ], + "css" : [ "styles/sso-providers.css" ], diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/AuthenticationProviderService.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/AuthenticationProviderService.java index 807df0cae..49d8daaff 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/AuthenticationProviderService.java @@ -75,22 +75,42 @@ public class AuthenticationProviderService implements SSOAuthenticationProviderS private static final String AUTH_SESSION_PARAMETER_NAME = "state"; /** - * Processes the given HTTP request, returning the identity represented by - * the auth session token present in that request. If no such token is - * present, or the token does not represent a valid identity, null is - * returned. + * Return the value of the session identifier associated with the given + * credentials, or null if no session identifier is found in the credentials. * - * @param request - * The HTTP request to process. + * @param credentials + * The credentials from which to extract the session identifier. + * + * @return + * The session identifier associated with the given credentials, or + * null if no identifier is found. + */ + public static String getSessionIdentifier(Credentials credentials) { + + // Return the session identifier from the request params, if set, or + // null otherwise + return credentials != null && credentials.getRequest() != null + ? credentials.getRequest().getParameter(AUTH_SESSION_PARAMETER_NAME) + : null; + } + + /** + * Processes the given credentials, returning the identity represented by + * the auth session token present in that request associated with the + * credentials. If no such token is present, or the token does not represent + * a valid identity, null is returned. + * + * @param credentials + * The credentials to extract the auth session token from. * * @return * The identity represented by the auth session token in the request, * or null if there is no such token or the token does not represent a * valid identity. */ - private SSOAuthenticatedUser processIdentity(Credentials credentials, HttpServletRequest request) { + private SSOAuthenticatedUser processIdentity(Credentials credentials) { - String state = request.getParameter(AUTH_SESSION_PARAMETER_NAME); + String state = getSessionIdentifier(credentials); String username = sessionManager.getIdentity(state); if (username == null) return null; @@ -153,7 +173,7 @@ public class AuthenticationProviderService implements SSOAuthenticationProviderS // if (confService.isPrimaryHostname(host)) - return processIdentity(credentials, request); + return processIdentity(credentials); // All other requests are not allowed - redirect to proper hostname throw new GuacamoleInvalidCredentialsException("Authentication is " diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/SSLAuthenticationEventListener.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/SSLAuthenticationEventListener.java new file mode 100644 index 000000000..26769ced9 --- /dev/null +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/SSLAuthenticationEventListener.java @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.auth.ssl; + +import com.google.inject.Inject; +import com.google.inject.Singleton; +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.auth.ssl.SSLAuthenticationSessionManager; +import org.apache.guacamole.auth.sso.SSOAuthenticationEventListener; +import org.apache.guacamole.net.auth.Credentials; +import org.apache.guacamole.net.event.AuthenticationFailureEvent; +import org.apache.guacamole.net.event.AuthenticationSuccessEvent; +import org.apache.guacamole.net.event.listener.Listener; + +/** + * A Listener that will reactivate or invalidate SSL auth sessions depending on + * overall auth success or failure. + */ +public class SSLAuthenticationEventListener extends SSOAuthenticationEventListener { + + /** + * Session manager for generating and maintaining unique tokens to + * represent the authentication flow of a user who has only partially + * authenticated. + * + * Requires static injection due to the fact that the webapp just calls the + * constructor directly when creating new Listeners. The instances will not + * be constructed by guice. + */ + @Inject + protected static SSLAuthenticationSessionManager sessionManager; + + @Override + protected String getSessionIdentifier(Credentials credentials) { + return AuthenticationProviderService.getSessionIdentifier(credentials); + } + + @Override + protected void reactivateSession(String sessionIdentifier) { + sessionManager.reactivateSession(sessionIdentifier); + } + + @Override + protected void invalidateSession(String sessionIdentifier) { + sessionManager.invalidateSession(sessionIdentifier); + } + +} diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/SSLAuthenticationProviderModule.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/SSLAuthenticationProviderModule.java index 46eeaa94f..9f7b15aff 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/SSLAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/SSLAuthenticationProviderModule.java @@ -35,6 +35,8 @@ public class SSLAuthenticationProviderModule extends AbstractModule { bind(ConfigurationService.class); bind(NonceService.class).in(Scopes.SINGLETON); bind(SSLAuthenticationSessionManager.class); + + requestStaticInjection(SSLAuthenticationEventListener.class); } } diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/resources/guac-manifest.json b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/resources/guac-manifest.json index d2d21741b..fe8dad84f 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/resources/guac-manifest.json +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/resources/guac-manifest.json @@ -9,6 +9,10 @@ "org.apache.guacamole.auth.ssl.SSLAuthenticationProvider" ], + "listeners" : [ + "org.apache.guacamole.auth.ssl.SSLAuthenticationEventListener" + ], + "css" : [ "styles/sso-providers.css" ], diff --git a/guacamole/src/main/frontend/src/app/login/directives/login.js b/guacamole/src/main/frontend/src/app/login/directives/login.js index 80ce00fe4..60272086a 100644 --- a/guacamole/src/main/frontend/src/app/login/directives/login.js +++ b/guacamole/src/main/frontend/src/app/login/directives/login.js @@ -66,6 +66,7 @@ angular.module('login').directive('guacLogin', [function guacLogin() { var Field = $injector.get('Field'); // Required services + var $location = $injector.get('$location'); var $rootScope = $injector.get('$rootScope'); var $route = $injector.get('$route'); var authenticationService = $injector.get('authenticationService'); @@ -173,11 +174,24 @@ angular.module('login').directive('guacLogin', [function guacLogin() { }); /** - * Submits the currently-specified username and password to the - * authentication service, redirecting to the main view if successful. + * Submits the currently-specified fields to the authentication service, + * as well as any URL parameters set for the current page, preferring + * the values from the fields, and redirecting to the main view if + * successful. */ $scope.login = function login() { - authenticationService.authenticate($scope.enteredValues)['catch'](requestService.IGNORE); + + // Any values from URL paramters + const urlValues = $location.search(); + + // Values from the fields + const fieldValues = $scope.enteredValues; + + // All the values to be submitted in the auth attempt, preferring + // any values from fields over those in the URL + const authParams = {...urlValues, ...fieldValues}; + + authenticationService.authenticate(authParams)['catch'](requestService.IGNORE); }; /**