From b0e5ecd33e9391782b984f889336630480afee5d Mon Sep 17 00:00:00 2001 From: Alex Leitner Date: Mon, 25 Mar 2024 01:27:11 +0000 Subject: [PATCH] GUACAMOLE-1289: Handle resumable state for duo authentication. --- .../duo/DuoAuthenticationProviderModule.java | 11 --- .../auth/duo/DuoAuthenticationSession.java | 74 ----------------- .../duo/DuoAuthenticationSessionManager.java | 34 -------- .../auth/duo/UserVerificationService.java | 33 ++------ ...amoleInsufficientCredentialsException.java | 32 ++++++++ ...amoleInsufficientCredentialsException.java | 78 ++++++++++++++++++ .../rest/auth/AuthenticationService.java | 48 ++++++++++- .../auth/ResumableAuthenticationState.java | 81 +++++++++++++++++++ 8 files changed, 243 insertions(+), 148 deletions(-) delete mode 100644 extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationSession.java delete mode 100644 extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationSessionManager.java create mode 100644 guacamole/src/main/java/org/apache/guacamole/rest/auth/ResumableAuthenticationState.java diff --git a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationProviderModule.java b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationProviderModule.java index 2b5e5d0b27..b9a37b0708 100644 --- a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationProviderModule.java @@ -41,11 +41,6 @@ public class DuoAuthenticationProviderModule extends AbstractModule { * module has configured injection. */ private final AuthenticationProvider authProvider; - - /** - * The session manager that stores authentication attempts. - */ - private final DuoAuthenticationSessionManager authSessionManager; /** * Creates a new Duo authentication provider module which configures @@ -66,10 +61,6 @@ public DuoAuthenticationProviderModule(AuthenticationProvider authProvider) // Store associated auth provider this.authProvider = authProvider; - - // Create a new session manager - this.authSessionManager = new DuoAuthenticationSessionManager(); - } @Override @@ -80,11 +71,9 @@ protected void configure() { bind(Environment.class).toInstance(environment); // Bind Duo-specific services - bind(DuoAuthenticationSessionManager.class).toInstance(authSessionManager); bind(ConfigurationService.class); bind(UserVerificationService.class); - } } diff --git a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationSession.java b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationSession.java deleted file mode 100644 index 8dd3e8db1e..0000000000 --- a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationSession.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * 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.duo; - -import org.apache.guacamole.net.auth.AuthenticationSession; - -/** - * An AuthenticationSession that stores the information required for an - * in-progress Duo authentication attempt. - */ -public class DuoAuthenticationSession extends AuthenticationSession { - - /** - * The session state generated by the Duo client, which is used to track - * the session through the redirect and return process. - */ - private final String state; - - /** - * The username of the user who is authenticating with this session. - */ - private final String username; - - /** - * Create a new instance of this authenticaiton session, having the given length of time - * for expriation and the state generated by the Duo Client. - * - * @param expires - * The number of milliseconds before this session is invalid. - * - * @param state - * The session state, as generated by the Duo Client. - * - * @param username - * The username of the user who is attempting authentication with Duo. - */ - public DuoAuthenticationSession(long expires, String state, String username) { - super(expires); - this.state = state; - this.username = username; - } - - /** - * Return the stored session state. - * - * @return - * The stored session state. - */ - public String getState() { - return state; - } - - public String getUsername() { - return username; - } - -} diff --git a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationSessionManager.java b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationSessionManager.java deleted file mode 100644 index 9bfbcc4668..0000000000 --- a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationSessionManager.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * 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.duo; - -import com.google.inject.Singleton; -import org.apache.guacamole.net.auth.AuthenticationSessionManager; - -/** - * An AuthenticationSessionManager implementation that temporarily stores - * authentication attempts for Duo MFA while they are underway. - */ -@Singleton -public class DuoAuthenticationSessionManager extends AuthenticationSessionManager { - - // Nothing to see here. - -} diff --git a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java index d38a4c6c62..5606836868 100644 --- a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java +++ b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java @@ -45,7 +45,7 @@ */ public class UserVerificationService { - private static final Logger LOGGER = LoggerFactory.getLogger(UserVerificationService.class); + private static final Logger logger = LoggerFactory.getLogger(UserVerificationService.class); /** * The name of the parameter which Duo will return in it's GET call-back @@ -63,7 +63,7 @@ public class UserVerificationService { * The value that will be returned in the token if Duo authentication * was successful. */ - private static final String DUO_TOKEN_SUCCESS_VALUE = "ALLOW"; + private static final String DUO_TOKEN_SUCCESS_VALUE = "allow"; /** * Service for retrieving Duo configuration information. @@ -71,13 +71,6 @@ public class UserVerificationService { @Inject private ConfigurationService confService; - /** - * The authentication session manager that temporarily stores in-progress - * authentication attempts. - */ - @Inject - private DuoAuthenticationSessionManager duoSessionManager; - /** * Verifies the identity of the given user via the Duo multi-factor * authentication service. If a signed response from Duo has not already @@ -116,8 +109,7 @@ public void verifyAuthenticatedUser(AuthenticatedUser authenticatedUser) confService.getRedirectUrl().toString()) .build(); - duoClient.healthCheck(); - + duoClient.healthCheck(); // Retrieve signed Duo Code and State from the request String duoCode = request.getParameter(DUO_CODE_PARAMETER_NAME); @@ -128,10 +120,7 @@ public void verifyAuthenticatedUser(AuthenticatedUser authenticatedUser) // Get a new session state from the Duo client duoState = duoClient.generateState(); - LOGGER.debug(">>> DUO <<< STATE DEFER: {}", duoState); - - // Add this session - duoSessionManager.defer(new DuoAuthenticationSession(confService.getAuthTimeout(), duoState, username), duoState); + long expirationTimestamp = System.currentTimeMillis() + (confService.getAuthTimeout() * 1000L); // Request additional credentials throw new TranslatableGuacamoleInsufficientCredentialsException( @@ -143,27 +132,21 @@ public void verifyAuthenticatedUser(AuthenticatedUser authenticatedUser) new URI(duoClient.createAuthUrl(username, duoState)), new TranslatableMessage("LOGIN.INFO_DUO_REDIRECT_PENDING") ) - )) + )), + duoState, + expirationTimestamp ); } - LOGGER.debug(">>> DUO <<< STATE RESUME: {}", duoState); - - // Retrieve the deferred authenticaiton attempt - DuoAuthenticationSession duoSession = duoSessionManager.resume(duoState); - if (duoSession == null) - throw new GuacamoleServerException("Failed to resume Duo authentication session."); - // Get the token from the DuoClient using the code and username, and check status - Token token = duoClient.exchangeAuthorizationCodeFor2FAResult(duoCode, duoSession.getUsername()); + Token token = duoClient.exchangeAuthorizationCodeFor2FAResult(duoCode, username); if (token == null || token.getAuth_result() == null || !DUO_TOKEN_SUCCESS_VALUE.equals(token.getAuth_result().getStatus())) throw new TranslatableGuacamoleClientException("Provided Duo " + "validation code is incorrect.", "LOGIN.INFO_DUO_VALIDATION_CODE_INCORRECT"); - } catch (DuoException e) { throw new GuacamoleServerException("Duo Client error.", e); diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/language/TranslatableGuacamoleInsufficientCredentialsException.java b/guacamole-ext/src/main/java/org/apache/guacamole/language/TranslatableGuacamoleInsufficientCredentialsException.java index 5b51d2ce9c..648e15007c 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/language/TranslatableGuacamoleInsufficientCredentialsException.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/language/TranslatableGuacamoleInsufficientCredentialsException.java @@ -136,6 +136,38 @@ public TranslatableGuacamoleInsufficientCredentialsException(String message, this(message, new TranslatableMessage(key), credentialsInfo); } + /** + * Creates a new TranslatableGuacamoleInsufficientCredentialsException with the specified message, + * translation key, the credential information required for authentication, the state token, and + * an expiration timestamp for the state token. The message is provided in both a non-translatable + * form and as a translatable key which can be used to retrieve the localized message. + * + * @param message + * A human-readable description of the exception that occurred. This + * message should be readable on its own and as-written, without + * requiring a translation service. + * + * @param key + * The arbitrary key which can be used to look up the message to be + * displayed in the user's native language. + * + * @param credentialsInfo + * Information describing the form of valid credentials. + * + * @param state + * An opaque value that may be used by a client to maintain state across requests which are part + * of the same authentication transaction. + * + * @param expires + * The timestamp after which the state token associated with the authentication process expires, + * specified as the number of milliseconds since the UNIX epoch. + */ + public TranslatableGuacamoleInsufficientCredentialsException(String message, + String key, CredentialsInfo credentialsInfo, String state, long expires) { + super(message, credentialsInfo, state, expires); + this.translatableMessage = new TranslatableMessage(key); + } + @Override public TranslatableMessage getTranslatableMessage() { return translatableMessage; diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/credentials/GuacamoleInsufficientCredentialsException.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/credentials/GuacamoleInsufficientCredentialsException.java index 06ae3ea5c6..c7a903c959 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/credentials/GuacamoleInsufficientCredentialsException.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/credentials/GuacamoleInsufficientCredentialsException.java @@ -28,6 +28,57 @@ */ public class GuacamoleInsufficientCredentialsException extends GuacamoleCredentialsException { +/** + * The default state token to use when no specific state information is provided. + */ +private static final String DEFAULT_STATE = ""; + +/** + * The default expiration timestamp to use when no specific expiration is provided, + * effectively indicating that the state token does not expire. + */ +private static final long DEFAULT_EXPIRES = -1L; + +/** + * An opaque value that may be used by a client to maintain state across requests + * which are part of the same authentication transaction. + */ +protected final String state; + +/** + * The timestamp after which the state token associated with the authentication process + * should no longer be considered valid, expressed as the number of milliseconds since + * UNIX epoch. + */ +protected final long expires; + + /** + * Creates a new GuacamoleInsufficientCredentialsException with the specified + * message, the credential information required for authentication, the state + * token associated with the authentication process, and an expiration timestamp. + * + * @param message + * A human-readable description of the exception that occurred. + * + * @param credentialsInfo + * Information describing the form of valid credentials. + * + * @param state + * An opaque value that may be used by a client to maintain state + * across requests which are part of the same authentication transaction. + * + * @param expires + * The timestamp after which the state token associated with the + * authentication process should no longer be considered valid, expressed + * as the number of milliseconds since UNIX epoch. + */ + public GuacamoleInsufficientCredentialsException(String message, + CredentialsInfo credentialsInfo, String state, long expires) { + super(message, credentialsInfo); + this.state = state; + this.expires = expires; + } + /** * Creates a new GuacamoleInsufficientCredentialsException with the given * message, cause, and associated credential information. @@ -44,6 +95,8 @@ public class GuacamoleInsufficientCredentialsException extends GuacamoleCredenti public GuacamoleInsufficientCredentialsException(String message, Throwable cause, CredentialsInfo credentialsInfo) { super(message, cause, credentialsInfo); + this.state = DEFAULT_STATE; + this.expires = DEFAULT_EXPIRES; } /** @@ -58,6 +111,8 @@ public GuacamoleInsufficientCredentialsException(String message, Throwable cause */ public GuacamoleInsufficientCredentialsException(String message, CredentialsInfo credentialsInfo) { super(message, credentialsInfo); + this.state = DEFAULT_STATE; + this.expires = DEFAULT_EXPIRES; } /** @@ -72,6 +127,29 @@ public GuacamoleInsufficientCredentialsException(String message, CredentialsInfo */ public GuacamoleInsufficientCredentialsException(Throwable cause, CredentialsInfo credentialsInfo) { super(cause, credentialsInfo); + this.state = DEFAULT_STATE; + this.expires = DEFAULT_EXPIRES; + } + + /** + * Retrieves the state token associated with the authentication process. + * + * @return The opaque state token used to maintain consistency across multiple + * requests in the same authentication transaction. + */ + public String getState() { + return state; + } + + /** + * Retrieves the expiration timestamp of the state token, specified as the + * number of milliseconds since the UNIX epoch. + * + * @return The expiration timestamp of the state token, or a negative value if + * the token does not expire. + */ + public long getExpires() { + return expires; } } diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/auth/AuthenticationService.java b/guacamole/src/main/java/org/apache/guacamole/rest/auth/AuthenticationService.java index 305073de39..2946b12763 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/auth/AuthenticationService.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/auth/AuthenticationService.java @@ -21,11 +21,14 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + import javax.inject.Inject; +import javax.servlet.http.HttpServletRequest; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleSecurityException; -import org.apache.guacamole.GuacamoleServerException; import org.apache.guacamole.GuacamoleUnauthorizedException; import org.apache.guacamole.GuacamoleSession; import org.apache.guacamole.net.auth.AuthenticatedUser; @@ -43,9 +46,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.inject.Singleton; + /** * A service for performing authentication checks in REST endpoints. */ +@Singleton public class AuthenticationService { /** @@ -96,6 +102,11 @@ public class AuthenticationService { */ public static final String TOKEN_PARAMETER_NAME = "token"; + /** + * Map to store resumable authentication states with an expiration time. + */ + private Map resumableStateMap = new ConcurrentHashMap<>(); + /** * Attempts authentication against all AuthenticationProviders, in order, * using the provided credentials. The first authentication failure takes @@ -310,6 +321,17 @@ private List getUserContexts(GuacamoleSession existingSess try { userContext = authProvider.getUserContext(authenticatedUser); } + catch (GuacamoleInsufficientCredentialsException e) { + // Store state and expiration + String state = e.getState(); + long expiration = e.getExpires(); + + resumableStateMap.put(state, new ResumableAuthenticationState(expiration, credentials)); + + throw new GuacamoleAuthenticationProcessException("User " + + "authentication aborted during initial " + + "UserContext creation.", authProvider, e); + } catch (GuacamoleException | RuntimeException | Error e) { throw new GuacamoleAuthenticationProcessException("User " + "authentication aborted during initial " @@ -366,12 +388,30 @@ public String authenticate(Credentials credentials, String token) AuthenticatedUser authenticatedUser; String authToken; + Credentials actualCredentials = credentials; + String state; + ResumableAuthenticationState resumableState = null; + + // Retrieve signed State from the request + HttpServletRequest request = credentials.getRequest(); + + // If state is provided, attempt to resume authentication + if ((state = request.getParameter("state")) != null && (resumableState = resumableStateMap.get(state)) != null) { + // The resumableState is removed as it should be a single-use token + resumableStateMap.remove(state); + + // Check if the resumableState has expired + if (!resumableState.isExpired()) { + actualCredentials = resumableState.getCredentials(); + actualCredentials.setRequest(request); + } + } try { // Get up-to-date AuthenticatedUser and associated UserContexts - authenticatedUser = getAuthenticatedUser(existingSession, credentials); - List userContexts = getUserContexts(existingSession, authenticatedUser, credentials); + authenticatedUser = getAuthenticatedUser(existingSession, actualCredentials); + List userContexts = getUserContexts(existingSession, authenticatedUser, actualCredentials); // Update existing session, if it exists if (existingSession != null) { @@ -401,7 +441,7 @@ public String authenticate(Credentials credentials, String token) // Log and rethrow any authentication errors catch (GuacamoleAuthenticationProcessException e) { - listenerService.handleEvent(new AuthenticationFailureEvent(credentials, + listenerService.handleEvent(new AuthenticationFailureEvent(actualCredentials, e.getAuthenticationProvider(), e.getCause())); // Rethrow exception diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/auth/ResumableAuthenticationState.java b/guacamole/src/main/java/org/apache/guacamole/rest/auth/ResumableAuthenticationState.java new file mode 100644 index 0000000000..3e24f4f042 --- /dev/null +++ b/guacamole/src/main/java/org/apache/guacamole/rest/auth/ResumableAuthenticationState.java @@ -0,0 +1,81 @@ +/* + * 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.rest.auth; + +import org.apache.guacamole.net.auth.Credentials; + +/** + * Encapsulates the state information required for resuming an authentication + * process. This includes an expiration timestamp to determine state validity + * and the original credentials submitted by the user. + */ +public class ResumableAuthenticationState { + + /** + * The timestamp at which this state should no longer be considered valid, + * measured in milliseconds since the Unix epoch. + */ + private long expirationTimestamp; + + /** + * The original user credentials that were submitted at the start of the + * authentication process. + */ + private Credentials credentials; + + /** + * Constructs a new ResumableAuthenticationState object with the specified + * expiration timestamp and user credentials. + * + * @param expirationTimestamp + * The timestamp in milliseconds since the Unix epoch when this state + * expires and can no longer be used to resume authentication. + * + * @param credentials + * The Credentials object initially submitted by the user and associated + * with this resumable state. + */ + public ResumableAuthenticationState(long expirationTimestamp, Credentials credentials) { + this.expirationTimestamp = expirationTimestamp; + this.credentials = credentials; + } + + /** + * Checks if this resumable state has expired based on the stored expiration + * timestamp and the current system time. + * + * @return + * True if the current system time is after the expiration timestamp, + * indicating that the state is expired; false otherwise. + */ + public boolean isExpired() { + return System.currentTimeMillis() > expirationTimestamp; + } + + /** + * Retrieves the original credentials associated with this resumable state. + * + * @return + * The Credentials object containing user details that were submitted + * when the state was created. + */ + public Credentials getCredentials() { + return this.credentials; + } +}