diff --git a/extensions/guacamole-auth-duo/pom.xml b/extensions/guacamole-auth-duo/pom.xml index 0614cb47d0..db28004573 100644 --- a/extensions/guacamole-auth-duo/pom.xml +++ b/extensions/guacamole-auth-duo/pom.xml @@ -85,6 +85,13 @@ kotlin-stdlib-common 1.4.10 + + + + org.springframework + spring-web + 5.3.25 + diff --git a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationProvider.java b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationProvider.java index 0b1a33dfbc..dc9999c984 100644 --- a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationProvider.java +++ b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/DuoAuthenticationProvider.java @@ -33,6 +33,13 @@ */ public class DuoAuthenticationProvider extends AbstractAuthenticationProvider { + /** + * The unique identifier for this authentication provider. This is used in + * various parts of the Guacamole client to distinguish this provider from + * others, particularly when multiple authentication providers are used. + */ + public static String PROVIDER_IDENTIFER = "duo"; + /** * Injector which will manage the object graph of this authentication * provider. @@ -58,7 +65,7 @@ public DuoAuthenticationProvider() throws GuacamoleException { @Override public String getIdentifier() { - return "duo"; + return PROVIDER_IDENTIFER; } @Override 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 b9a37b0708..372bbcc39b 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 @@ -73,7 +73,6 @@ protected void configure() { // Bind Duo-specific services bind(ConfigurationService.class); bind(UserVerificationService.class); - } } 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 5606836868..efd8a43572 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 @@ -39,6 +39,7 @@ import org.apache.guacamole.net.auth.credentials.CredentialsInfo; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.web.util.UriComponentsBuilder; /** * Service for verifying the identity of a user against Duo. @@ -51,13 +52,13 @@ public class UserVerificationService { * The name of the parameter which Duo will return in it's GET call-back * that contains the code that the client will use to generate a token. */ - private static final String DUO_CODE_PARAMETER_NAME = "duo_code"; + public static final String DUO_CODE_PARAMETER_NAME = "duo_code"; /** * The name of the parameter that will be used in the GET call-back that * contains the session state. */ - private static final String DUO_STATE_PARAMETER_NAME = "state"; + public static final String DUO_STATE_PARAMETER_NAME = "state"; /** * The value that will be returned in the token if Duo authentication @@ -101,12 +102,20 @@ public void verifyAuthenticatedUser(AuthenticatedUser authenticatedUser) try { + String redirectUrl = confService.getRedirectUrl().toString(); + + String builtUrl = UriComponentsBuilder + .fromUriString(redirectUrl) + .queryParam(Credentials.RESUME_QUERY, DuoAuthenticationProvider.PROVIDER_IDENTIFER) + .build() + .toUriString(); + // Set up the Duo Client Client duoClient = new Client.Builder( confService.getClientId(), confService.getClientSecret(), confService.getAPIHostname(), - confService.getRedirectUrl().toString()) + builtUrl) .build(); duoClient.healthCheck(); @@ -133,8 +142,8 @@ public void verifyAuthenticatedUser(AuthenticatedUser authenticatedUser) new TranslatableMessage("LOGIN.INFO_DUO_REDIRECT_PENDING") ) )), - duoState, - expirationTimestamp + duoState, DuoAuthenticationProvider.PROVIDER_IDENTIFER, + DUO_STATE_PARAMETER_NAME, expirationTimestamp ); } 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 648e15007c..f0c27bc365 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 @@ -157,15 +157,23 @@ public TranslatableGuacamoleInsufficientCredentialsException(String message, * @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 providerIdentifier + * The identifier of the authentication provider that this exception pertains to. + * + * @param queryIdentifier + * The identifier of the specific query parameter within the + * authentication process that this exception pertains to. * * @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); + String key, CredentialsInfo credentialsInfo, String state, String providerIdentifier, + String queryIdentifier, long expires) { + super(message, credentialsInfo, state, providerIdentifier, queryIdentifier, expires); + this.translatableMessage = new TranslatableMessage(key); } @Override diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Credentials.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Credentials.java index 6ad0e240b3..45eebe80df 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Credentials.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Credentials.java @@ -34,6 +34,16 @@ */ public class Credentials implements Serializable { + /** + * The RESUME_QUERY is a query parameter key used to determine which + * authentication provider's process should be resumed during multi-step + * authentication. The auth provider will set this parameter before + * redirecting to an external service, and it is checked upon return to + * Guacamole to ensure the correct authentication state is continued + * without starting over. + */ + public static final String RESUME_QUERY = "provider_id"; + /** * Unique identifier associated with this specific version of Credentials. */ 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 c7a903c959..def5350b6e 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 @@ -33,6 +33,20 @@ public class GuacamoleInsufficientCredentialsException extends GuacamoleCredenti */ private static final String DEFAULT_STATE = ""; +/** + * The default provider identifier to use when no specific provider is identified. + * This serves as a placeholder indicating that either no specific provider is + * responsible for the exception or the responsible provider has not been identified. + */ +private static final String DEFAULT_PROVIDER_IDENTIFIER = ""; + +/** + * The default query identifier to use when no specific query is identified. + * This serves as a placeholder and indicates that the specific query related to + * the provider's state resume operation has not been provided. + */ +private static final String DEFAULT_QUERY_IDENTIFIER = ""; + /** * The default expiration timestamp to use when no specific expiration is provided, * effectively indicating that the state token does not expire. @@ -45,6 +59,20 @@ public class GuacamoleInsufficientCredentialsException extends GuacamoleCredenti */ protected final String state; + /** + * The identifier for the authentication provider that threw this exception. + * This is used to link the exception back to the originating source of the + * authentication attempt, allowing clients to determine which provider's + * authentication process should be resumed. + */ + protected final String providerIdentifier; + + /** + * An identifier for the specific query within the URL for this provider that can + * be checked to resume the authentication state. + */ + protected final String queryIdentifier; + /** * 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 @@ -67,15 +95,25 @@ public class GuacamoleInsufficientCredentialsException extends GuacamoleCredenti * An opaque value that may be used by a client to maintain state * across requests which are part of the same authentication transaction. * + * @param providerIdentifier + * The identifier of the authentication provider that this exception pertains to. + * + * @param queryIdentifier + * The identifier of the specific query parameter within the + * authentication process that this exception pertains to. + * * @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) { + CredentialsInfo credentialsInfo, String state, String providerIdentifier, String queryIdentifier, + long expires) { super(message, credentialsInfo); this.state = state; + this.providerIdentifier = providerIdentifier; + this.queryIdentifier = queryIdentifier; this.expires = expires; } @@ -96,6 +134,8 @@ public GuacamoleInsufficientCredentialsException(String message, Throwable cause CredentialsInfo credentialsInfo) { super(message, cause, credentialsInfo); this.state = DEFAULT_STATE; + this.providerIdentifier = DEFAULT_PROVIDER_IDENTIFIER; + this.queryIdentifier = DEFAULT_QUERY_IDENTIFIER; this.expires = DEFAULT_EXPIRES; } @@ -112,6 +152,8 @@ public GuacamoleInsufficientCredentialsException(String message, Throwable cause public GuacamoleInsufficientCredentialsException(String message, CredentialsInfo credentialsInfo) { super(message, credentialsInfo); this.state = DEFAULT_STATE; + this.providerIdentifier = DEFAULT_PROVIDER_IDENTIFIER; + this.queryIdentifier = DEFAULT_QUERY_IDENTIFIER; this.expires = DEFAULT_EXPIRES; } @@ -128,6 +170,8 @@ public GuacamoleInsufficientCredentialsException(String message, CredentialsInfo public GuacamoleInsufficientCredentialsException(Throwable cause, CredentialsInfo credentialsInfo) { super(cause, credentialsInfo); this.state = DEFAULT_STATE; + this.providerIdentifier = DEFAULT_PROVIDER_IDENTIFIER; + this.queryIdentifier = DEFAULT_QUERY_IDENTIFIER; this.expires = DEFAULT_EXPIRES; } @@ -141,6 +185,27 @@ public String getState() { return state; } + /** + * Retrieves the identifier of the authentication provider responsible for this exception. + * + * @return The identifier of the authentication provider, allowing clients to know + * which provider's process should be resumed in response to this exception. + */ + public String getProviderIdentifier() { + return providerIdentifier; + } + + /** + * Retrieves the specific query identifier associated with the URL for the provider + * that can be checked to resume the authentication state. + * + * @return The query identifier that serves as a reference to a specific point or + * transaction within the provider's authentication process. + */ + public String getQueryIdentifier() { + return queryIdentifier; + } + /** * Retrieves the expiration timestamp of the state token, specified as the * number of milliseconds since the UNIX epoch. 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 2946b12763..1fafa4a300 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 @@ -47,6 +47,7 @@ import org.slf4j.LoggerFactory; import com.google.inject.Singleton; +import java.util.Iterator; /** * A service for performing authentication checks in REST endpoints. @@ -325,12 +326,15 @@ private List getUserContexts(GuacamoleSession existingSess // Store state and expiration String state = e.getState(); long expiration = e.getExpires(); + String queryIdentifier = e.getQueryIdentifier(); + String providerIdentifier = e.getProviderIdentifier(); - resumableStateMap.put(state, new ResumableAuthenticationState(expiration, credentials)); + resumableStateMap.put(state, new ResumableAuthenticationState(providerIdentifier, + queryIdentifier, expiration, credentials)); throw new GuacamoleAuthenticationProcessException("User " - + "authentication aborted during initial " - + "UserContext creation.", authProvider, e); + + "authentication aborted during initial " + + "UserContext creation.", authProvider, e); } catch (GuacamoleException | RuntimeException | Error e) { throw new GuacamoleAuthenticationProcessException("User " @@ -350,6 +354,82 @@ private List getUserContexts(GuacamoleSession existingSess return userContexts; } + + /** + * Resumes authentication using given credentials if a matching resumable + * state is found. + * + * @param credentials + * The initial credentials containing the request object. + * + * @return + * Resumed credentials if a valid resumable state is found; otherwise, + * returns {@code null}. + */ + private Credentials resumeAuthentication(Credentials credentials) { + + Credentials resumedCredentials = null; + + // Retrieve signed State from the request + HttpServletRequest request = credentials.getRequest(); + + // Retrieve the provider id from the query parameters. + String resumableProviderId = request.getParameter(Credentials.RESUME_QUERY); + // Check if a provider id is set. + if (resumableProviderId == null || resumableProviderId.isEmpty()) { + // return if a provider id is not set. + return null; + } + + // Use an iterator to safely remove entries while iterating + Iterator> iterator = resumableStateMap.entrySet().iterator(); + while (iterator.hasNext()) { + Map.Entry entry = iterator.next(); + ResumableAuthenticationState resumableState = entry.getValue(); + + // Check if the provider ID from the request matches the one in the map entry. + boolean providerMatches = resumableProviderId.equals(resumableState.getProviderIdentifier()); + if (!providerMatches) { + // If the provider doesn't match, skip to the next entry. + continue; + } + + // Use the query identifier from the entry to retrieve the corresponding state parameter. + String stateQueryParameter = resumableState.getQueryIdentifier(); + String stateFromParameter = request.getParameter(stateQueryParameter); + + // Check if the `state` parameter is set. + if (stateFromParameter == null || stateFromParameter.isEmpty()) { + // Remove and continue if `state` is not provided or is empty. + iterator.remove(); + continue; + } + + // If the key in the entry (state) matches the state parameter provided in the request. + if (entry.getKey().equals(stateFromParameter)) { + + // Remove the current entry from the map. + iterator.remove(); + + // Check if the resumableState has expired + if (!resumableState.isExpired()) { + + // Set the actualCredentials to the credentials from the matched entry. + resumedCredentials = resumableState.getCredentials(); + + if (resumedCredentials != null) { + resumedCredentials.setRequest(request); + } + + } + + // Exit the loop since we've found the matching state and it's unique. + break; + } + } + + return resumedCredentials; + } /** * Authenticates a user using the given credentials and optional @@ -388,24 +468,11 @@ 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); - } - } + // Retrieve credentials if resuming authentication + Credentials actualCredentials = resumeAuthentication(credentials); + if (actualCredentials == null) + actualCredentials = credentials; try { 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 index 3e24f4f042..f295a82088 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/auth/ResumableAuthenticationState.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/auth/ResumableAuthenticationState.java @@ -39,10 +39,31 @@ public class ResumableAuthenticationState { */ private Credentials credentials; + /** + * A unique string identifying the authentication provider related to the state. + * This field allows the client to know which provider's authentication process + * should be resumed using this state. + */ + private String providerIdentifier; + + /** + * A unique string that can be used to identify a specific query within the + * authentication process for the identified provider. This identifier can + * help the resumption of an authentication process. + */ + private String queryIdentifier; + /** * Constructs a new ResumableAuthenticationState object with the specified * expiration timestamp and user credentials. * + * @param providerIdentifier + * The identifier of the authentication provider to which this resumable state pertains. + * + * @param queryIdenifier + * The identifier of the specific query within the provider's + * authentication process that this state corresponds to. + * * @param expirationTimestamp * The timestamp in milliseconds since the Unix epoch when this state * expires and can no longer be used to resume authentication. @@ -51,9 +72,12 @@ public class ResumableAuthenticationState { * The Credentials object initially submitted by the user and associated * with this resumable state. */ - public ResumableAuthenticationState(long expirationTimestamp, Credentials credentials) { + public ResumableAuthenticationState(String providerIdentifier, String queryIdentifier, + long expirationTimestamp, Credentials credentials) { this.expirationTimestamp = expirationTimestamp; this.credentials = credentials; + this.providerIdentifier = providerIdentifier; + this.queryIdentifier = queryIdentifier; } /** @@ -78,4 +102,27 @@ public boolean isExpired() { public Credentials getCredentials() { return this.credentials; } + + /** + * Retrieves the identifier of the authentication provider associated with this state. + * + * @return + * The identifier of the authentication provider, providing context for this state + * within the overall authentication sequence. + */ + public String getProviderIdentifier() { + return this.providerIdentifier; + } + + /** + * Retrieves the identifier for a specific query in the authentication + * process that is associated with this state. + * + * @return + * The query identifier used for retrieving a value representing the state within + * the provider's authentication process that should be resumed. + */ + public String getQueryIdentifier() { + return this.queryIdentifier; + } }