From 4f8c853daa34d85b68e40c54b92a7f09e6eeac73 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 27 Aug 2017 22:58:12 -0700 Subject: [PATCH] GUACAMOLE-210: Re-request ID token if validation or username retrieval fails. --- .../openid/AuthenticationProviderService.java | 18 ++++---- .../openid/token/TokenValidationService.java | 41 +++++++++++++------ 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/AuthenticationProviderService.java b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/AuthenticationProviderService.java index 10dea3d43..1423b8dfd 100644 --- a/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/AuthenticationProviderService.java @@ -82,19 +82,23 @@ public class AuthenticationProviderService { public AuthenticatedUser authenticateUser(Credentials credentials) throws GuacamoleException { - String token = null; + String username = null; - // Pull OpenID token from request if present + // Validate OpenID token in request, if present, and derive username HttpServletRequest request = credentials.getRequest(); - if (request != null) - token = request.getParameter(TokenField.PARAMETER_NAME); + if (request != null) { + String token = request.getParameter(TokenField.PARAMETER_NAME); + if (token != null) + username = tokenService.processUsername(token); + } - // If token provided, validate and produce authenticated user - if (token != null) { + // If the username was successfully retrieved from the token, produce + // authenticated user + if (username != null) { // Create corresponding authenticated user AuthenticatedUser authenticatedUser = authenticatedUserProvider.get(); - authenticatedUser.init(tokenService.processUsername(token), credentials); + authenticatedUser.init(username, credentials); return authenticatedUser; } diff --git a/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java index b1a8a28f7..3e1a58dbb 100644 --- a/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java +++ b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java @@ -31,6 +31,8 @@ import org.jose4j.jwt.consumer.InvalidJwtException; import org.jose4j.jwt.consumer.JwtConsumer; import org.jose4j.jwt.consumer.JwtConsumerBuilder; import org.jose4j.keys.resolvers.HttpsJwksVerificationKeyResolver; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Service for validating ID tokens forwarded to us by the client, verifying @@ -38,6 +40,11 @@ import org.jose4j.keys.resolvers.HttpsJwksVerificationKeyResolver; */ public class TokenValidationService { + /** + * Logger for this class. + */ + private final Logger logger = LoggerFactory.getLogger(TokenValidationService.class); + /** * Service for retrieving OpenID configuration information. */ @@ -48,17 +55,17 @@ public class TokenValidationService { * Validates and parses the given ID token, returning the username contained * therein, as defined by the username claim type given in * guacamole.properties. If the username claim type is missing or the ID - * token is invalid, an exception is thrown instead. + * token is invalid, null is returned. * * @param token * The ID token to validate and parse. * * @return - * The username contained within the given ID token. + * The username contained within the given ID token, or null if the ID + * token is not valid or the username claim type is missing, * * @throws GuacamoleException - * If the ID token is not valid, the username claim type is missing, or - * guacamole.properties could not be parsed. + * If guacamole.properties could not be parsed. */ public String processUsername(String token) throws GuacamoleException { @@ -79,27 +86,37 @@ public class TokenValidationService { try { + String usernameClaim = confService.getUsernameClaimType(); + // Validate JWT JwtClaims claims = jwtConsumer.processToClaims(token); // Pull username from claims - String username = claims.getStringClaimValue(confService.getUsernameClaimType()); - if (username == null) - throw new GuacamoleSecurityException("Username missing from token"); + String username = claims.getStringClaimValue(usernameClaim); + if (username != null) + return username; - // Username successfully retrieved from the JWT - return username; + // Warn if username was not present in token, as it likely means + // the system is not set up correctly + logger.warn("Username claim \"{}\" missing from token. Perhaps the " + + "OpenID scope and/or username claim type are " + + "misconfigured?", usernameClaim); } - // Rethrow any failures to validate/parse the JWT + // Log any failures to validate/parse the JWT catch (InvalidJwtException e) { - throw new GuacamoleSecurityException("Invalid ID token.", e); + logger.info("Rejected invalid OpenID token: {}", e.getMessage()); + logger.debug("Invalid JWT received.", e); } catch (MalformedClaimException e) { - throw new GuacamoleServerException("Unable to parse JWT claims.", e); + logger.info("Rejected OpenID token with malformed claim: {}", e.getMessage()); + logger.debug("Malformed claim within received JWT.", e); } + // Could not retrieve username from JWT + return null; + } }