Skip to content

Commit

Permalink
GUACAMOLE-1289: Modify the Authentication Service to no longer explic…
Browse files Browse the repository at this point in the history
…itly compare state query string.
  • Loading branch information
aleitner committed Apr 3, 2024
1 parent b0e5ecd commit adec388
Show file tree
Hide file tree
Showing 9 changed files with 251 additions and 32 deletions.
7 changes: 7 additions & 0 deletions extensions/guacamole-auth-duo/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@
<artifactId>kotlin-stdlib-common</artifactId>
<version>1.4.10</version>
</dependency>

<!-- spring-web -->
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-web</artifactId>
<version>5.3.25</version>
</dependency>

</dependencies>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -58,7 +65,7 @@ public DuoAuthenticationProvider() throws GuacamoleException {

@Override
public String getIdentifier() {
return "duo";
return PROVIDER_IDENTIFER;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ protected void configure() {
// Bind Duo-specific services
bind(ConfigurationService.class);
bind(UserVerificationService.class);

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand All @@ -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
);

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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.
Expand Down
Loading

0 comments on commit adec388

Please sign in to comment.