diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/AuthenticationProviderService.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/AuthenticationProviderService.java index 5f5c8c45c..bb61c8126 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/AuthenticationProviderService.java @@ -146,14 +146,15 @@ public class AuthenticationProviderService implements SSOAuthenticationProviderS if (confService.isImplicitFlow()) { String token = credentials.getParameter(IMPLICIT_TOKEN_PARAMETER_NAME); if (token != null) { - JwtClaims claims = tokenService.validateToken(token); + JwtClaims claims = tokenService.validateTokenOrCode(token, ""); if (claims != null) { username = tokenService.processUsername(claims); groups = tokenService.processGroups(claims); tokens = tokenService.processAttributes(claims); } } - } else { + } + else { String verifier = null; if (confService.isPKCERequired()) { // Recover session @@ -164,7 +165,7 @@ public class AuthenticationProviderService implements SSOAuthenticationProviderS } String code = credentials.getParameter("code"); if (code != null && (confService.isPKCERequired() == false || verifier != null)) { - JwtClaims claims = tokenService.validateCode(code, verifier); + JwtClaims claims = tokenService.validateTokenOrCode(code, verifier); if (claims != null) { username = tokenService.processUsername(claims); groups = tokenService.processGroups(claims); @@ -203,7 +204,8 @@ public class AuthenticationProviderService implements SSOAuthenticationProviderS if (confService.isImplicitFlow()) { builder.queryParam("nonce", nonceService.generate(confService.getMaxNonceValidity() * 60000L)); - } else { + } + else { if (confService.isPKCERequired()) { String codeVerifier = PKCEUtil.generateCodeVerifier(); String codeChallenge; diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationProviderModule.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationProviderModule.java index a74832ab7..f533aa6e4 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationProviderModule.java @@ -46,6 +46,7 @@ public class OpenIDAuthenticationProviderModule extends AbstractModule { bind(NonceService.class).in(Scopes.SINGLETON); bind(TokenValidationService.class); bind(OpenIDAuthenticationSessionManager.class); + bind(Environment.class).toInstance(environment); } diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationSession.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationSession.java index 320f0609d..92fa014ac 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationSession.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationSession.java @@ -24,6 +24,7 @@ import org.apache.guacamole.net.auth.AuthenticationSession; /** * Representation of an in-progress OpenID authentication attempt. */ + public class OpenIDAuthenticationSession extends AuthenticationSession { /** * The PKCE challenge verifier. diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/ConfigurationService.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/ConfigurationService.java index 0dc6576a4..f0727f3e4 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/ConfigurationService.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/ConfigurationService.java @@ -20,7 +20,6 @@ package org.apache.guacamole.auth.openid.conf; import com.google.inject.Inject; - import java.net.URI; import java.util.Collection; import java.util.Collections; @@ -125,10 +124,9 @@ public class ConfigurationService { */ private static final URIGuacamoleProperty OPENID_TOKEN_ENDPOINT = new URIGuacamoleProperty() { + @Override - public String getName() { - return "openid-token-endpoint"; - } + public String getName() { return "openid-token-endpoint"; } }; /** @@ -148,10 +146,9 @@ public class ConfigurationService { */ private static final StringGuacamoleProperty OPENID_CLIENT_SECRET = new StringGuacamoleProperty() { + @Override - public String getName() { - return "openid-client-secret"; - } + public String getName() { return "openid-client-secret"; } }; /** @@ -159,10 +156,9 @@ public class ConfigurationService { */ private static final BooleanGuacamoleProperty OPENID_PKCE_REQUIRED = new BooleanGuacamoleProperty() { + @Override - public String getName() { - return "openid-pkce-required"; - } + public String getName() { return "openid-pkce-required"; } }; /** @@ -174,9 +170,9 @@ public class ConfigurationService { */ private static final IntegerGuacamoleProperty OPENID_AUTH_TIMEOUT = new IntegerGuacamoleProperty() { + @Override public String getName() { return "openid-auth-timeout"; } - }; @@ -447,22 +443,6 @@ public class ConfigurationService { } return token_endpoint; } - - /** - * Returns the well-known endpoint (URI) of the OIDC service as - * configured with guacamole.properties. - * - * @return - * The well-known endpoint of the OIDC service, as configured with - * guacamole.properties. - * - * @throws GuacamoleException - * If guacamole.properties cannot be parsed, or if the well-known - * endpoint property is missing. - */ - public URI getWellKnownEndpoint() throws GuacamoleException { - return confWellKnown.getWellKnownEndpoint(); - } /** * Returns the reponse type of the OpenID service as configured with guacamole.properties. diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/OpenIDWellKnown.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/OpenIDWellKnown.java index 108fd7ecc..db61639d0 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/OpenIDWellKnown.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/OpenIDWellKnown.java @@ -46,7 +46,7 @@ public class OpenIDWellKnown { /** * Logger for this class. */ - private final Logger logger = LoggerFactory.getLogger(OpenIDWellKnown.class); + private static final Logger logger = LoggerFactory.getLogger(OpenIDWellKnown.class); /** @@ -62,32 +62,37 @@ public class OpenIDWellKnown { /** * The detected issuer */ - private static String issuer = null; + private static String issuer = null; /** * The detected authorization edpoint */ - private static URI authorization_endpoint = null; + private static URI authorization_endpoint = null; /** * The detected token edpoint */ - private static URI token_endpoint = null; + private static URI token_endpoint = null; /** * The detected jwks_uri */ - private static URI jwks_uri = null; + private static URI jwks_uri = null; + + /** + * Empty constructor of the class to populate data recovered from a OIDC + * well-known URL. The class will be populated on injection by Guice + */ + public OpenIDWellKnown() { } /** * The well-known endpoint (URI) of the OIDC service. */ private static final URIGuacamoleProperty OPENID_WELL_KNOWN_ENDPOINT = new URIGuacamoleProperty() { + @Override - public String getName() { - return "openid-well-known-endpoint"; - } + public String getName() { return "openid-well-known-endpoint"; } }; /** @@ -102,7 +107,7 @@ public class OpenIDWellKnown { * If guacamole.properties cannot be parsed, or if the authorization * endpoint property is missing. */ - public URI getWellKnownEndpoint() throws GuacamoleException { + private URI getWellKnownEndpoint() throws GuacamoleException { return environment.getProperty(OPENID_WELL_KNOWN_ENDPOINT); } @@ -163,15 +168,22 @@ public class OpenIDWellKnown { private Environment environment; /* - * Creates an OpenIDWellKnown class that reads the json from an OIDC - * well-known endpoint and saves these values for later use. Use Guice - * to ensure environment exists before initializing. + * On injection, when the environment is non null, populates the OpenIDWellKnown + * class by reading the json from an OIDC well-known endpoint and saves these values + * for later use. Use Guice to ensure environment exists before initializing. */ - public OpenIDWellKnown() { - } - @Inject public void init() { + // Fast return if there is no well-known endpoint or its unreadable + try { + if (getWellKnownEndpoint() == null) { + return; + } + } + catch (Exception e) { + return; + } + // Call to well-known endpoint might fail, so allow several tries before giving up ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); @@ -198,7 +210,8 @@ public class OpenIDWellKnown { scheduler.shutdown(); return; - } catch (Exception e) { + } + catch (Exception e) { logger.debug("Rejecting well-known endpoint : {}", e.getMessage()); } diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java index 583761962..7a7d83f17 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java @@ -30,7 +30,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; - import javax.ws.rs.core.UriBuilder; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.auth.openid.conf.ConfigurationService; @@ -77,20 +76,26 @@ public class TokenValidationService { private NonceService nonceService; /** - * Validates the given ID token, using implicit flow, returning the JwtClaims - * contained therein. If the ID token is invalid, null is returned. + * Validates the given ID token, using implicit flow. Also validates codes, + * exchanging them for id_tokens before validation. If the id_token or + * code is invalid, null is returned, otherwise the JwtClaims in the + * id_token are returned. * * @param token - * The ID token to validate. + * The ID token to validate if implicit flow or the code to exchange for + * an id_token and then validate. + * + * @param verifier + * A PKCE verifier or null if not used. Only used with code flow * * @return - * The JWT claims contained within the given ID token if it passes tests, - * or null if the token is not valid. + * The JWT claims contained within the id_token if it passes tests, + * or null if the id_token is not valid. * * @throws GuacamoleException * If guacamole.properties could not be parsed. */ - public JwtClaims validateToken(String token) throws GuacamoleException { + public JwtClaims validateTokenOrCode(String token, String verifier) throws GuacamoleException { // Validating the token requires a JWKS key resolver HttpsJwks jwks = new HttpsJwks(confService.getJWKSEndpoint().toString()); HttpsJwksVerificationKeyResolver resolver = new HttpsJwksVerificationKeyResolver(jwks); @@ -105,26 +110,36 @@ public class TokenValidationService { .setExpectedAudience(confService.getClientID()) .setVerificationKeyResolver(resolver) .build(); - + + /* Exchange code → token */ + if (! confService.isImplicitFlow()) { + token = exchangeCode(token, verifier); + } + try { // Validate JWT JwtClaims claims = jwtConsumer.processToClaims(token); - // Verify a nonce is present - String nonce = claims.getStringClaimValue("nonce"); - if (nonce != null) { - // Verify that we actually generated the nonce, and that it has not - // already been used - if (nonceService.isValid(nonce)) { - // nonce is valid, consider claims valid - return claims; + if (confService.isImplicitFlow()) { + // Verify a nonce is present + String nonce = claims.getStringClaimValue("nonce"); + if (nonce != null) { + // Verify that we actually generated the nonce, and that it has not + // already been used + if (nonceService.isValid(nonce)) { + // nonce is valid, consider claims valid + return claims; + } + else { + logger.info("Rejected OpenID token with invalid/old nonce."); + } } else { - logger.info("Rejected OpenID token with invalid/old nonce."); + logger.info("Rejected OpenID token without nonce."); } } else { - logger.info("Rejected OpenID token without nonce."); + return claims; } } // Log any failures to validate/parse the JWT @@ -138,54 +153,6 @@ public class TokenValidationService { return null; } - /** - * Validates the given ID token, using code flow, returning the JwtClaims - * contained therein. If the ID token is invalid, null is returned. - * - * @param code - * The code to validate and receive the id_token. - * - * @param verifier - * A PKCE verifier or null if not used. - * - * @return - * The JWT claims contained within the given ID token if it passes tests, - * or null if the token is not valid. - * - * @throws GuacamoleException - * If guacamole.properties could not be parsed. - */ - public JwtClaims validateCode(String code, String verifier) throws GuacamoleException { - // Validating the token requires a JWKS key resolver - HttpsJwks jwks = new HttpsJwks(confService.getJWKSEndpoint().toString()); - HttpsJwksVerificationKeyResolver resolver = new HttpsJwksVerificationKeyResolver(jwks); - - /* Exchange code → token */ - String token = exchangeCode(code, verifier); - - // Create JWT consumer for validating received token - JwtConsumer jwtConsumer = new JwtConsumerBuilder() - .setRequireExpirationTime() - .setMaxFutureValidityInMinutes(confService.getMaxTokenValidity()) - .setAllowedClockSkewInSeconds(confService.getAllowedClockSkew()) - .setRequireSubject() - .setExpectedIssuer(confService.getIssuer()) - .setExpectedAudience(confService.getClientID()) - .setVerificationKeyResolver(resolver) - .build(); - - try { - // Validate JWT - return jwtConsumer.processToClaims(token); - } - // Log any failures to validate/parse the JWT - catch (InvalidJwtException e) { - logger.info("Rejected invalid OpenID token: {}", e.getMessage(), e); - } - - return null; - } - /** * URLEncodes a key/value pair * @@ -211,6 +178,7 @@ public class TokenValidationService { * * @param code * The authorization code received from the IdP. + * * @param codeVerifier * The PKCE verifier (or null if PKCE is disabled). * @@ -245,7 +213,8 @@ public class TokenValidationService { return (String) json.get("id_token"); - } catch (Exception e) { + } + catch (Exception e) { logger.info("Rejected invalid OpenID code exchange: {}", e.getMessage(), e); } return null; @@ -318,7 +287,7 @@ public class TokenValidationService { List oidcGroups = claims.getStringListClaimValue(groupsClaim); if (oidcGroups != null && !oidcGroups.isEmpty()) return Collections.unmodifiableSet(new HashSet<>(oidcGroups)); - } + } catch (MalformedClaimException e) { logger.info("Rejected OpenID token with malformed claim: {}", e.getMessage(), e); } diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/util/JsonUrlReader.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/util/JsonUrlReader.java index b24299865..961033067 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/util/JsonUrlReader.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/util/JsonUrlReader.java @@ -20,13 +20,13 @@ package org.apache.guacamole.auth.openid.util; import java.io.BufferedReader; +import java.io.IOException; import java.io.OutputStream; import java.io.InputStreamReader; import java.net.HttpURLConnection; import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.Map; -import org.apache.guacamole.GuacamoleException; import org.jose4j.json.JsonUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,17 +35,38 @@ import org.slf4j.LoggerFactory; * Utility class to open a http connection to a URL, send a body * and receive a response in the form of a parsed JSON */ + public final class JsonUrlReader { /** * Logger for this class. */ private static final Logger logger = LoggerFactory.getLogger(JsonUrlReader.class); + /** + * Class to GET and POST to a URL and read the returned JSON. This class should + * not be instantiated. + */ private JsonUrlReader() {} - public static Map fetch(String method, URL url, String body) throws GuacamoleException { - if (url == null) { - throw new GuacamoleException("JsonUrlReader : Missing URL"); + /** + * Method to POST or GET to a URL and recover the JSON in the form of a Map + * + * @param String method + * The htpp method to use. Should be "GET", "POST" or "PATCH". + * + * @param URL url + * A URL value giving the address where to recover the JSON + * + * @param String body + * A pre-encoded body string to be sent to the address if the method is + * "POST" or "PATCH". Ignored if the method is "GET". + * + * @return + * A Map containing the decoded json values. + */ + public static Map fetch(String method, URL url, String body) throws IOException { + if (url == null || url.toString() == "") { + throw new IOException("JsonUrlReader : Missing URL"); } try { @@ -55,7 +76,7 @@ public final class JsonUrlReader { conn.setRequestMethod(method); conn.setRequestProperty("Content-Type", "application/x-www-form-urlencoded; charset=UTF-8"); - if (! method.equalsIgnoreCase("GET")) { + if (! method.equals("GET")) { conn.setDoOutput(true); try (OutputStream out = conn.getOutputStream()) { byte [] bodyBytes = body.getBytes(StandardCharsets.UTF_8); @@ -87,12 +108,13 @@ public final class JsonUrlReader { Map json = JsonUtil.parseJson(responseBody.toString()); if (status < 200 || status >= 300) { - throw new GuacamoleException("(status: " + status + "): " + json.toString()); + throw new IOException("(status: " + status + "): " + json.toString()); } return json; - } catch (Exception e) { - throw new GuacamoleException("JsonUrlreader error : " + method + " : " + e.getMessage()); + } + catch (Exception e) { + throw new IOException("JsonUrlreader error : " + e.getMessage()); } } } diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/util/PKCEUtil.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/util/PKCEUtil.java index a92876d95..8257a8a3c 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/util/PKCEUtil.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/util/PKCEUtil.java @@ -30,13 +30,21 @@ import java.security.SecureRandom; * - code_challenge (S256) */ public final class PKCEUtil { - + /** + * Get the verifier data from a secure random source + */ private static final SecureRandom RANDOM = new SecureRandom(); + /* + * Class to create PKCE challenges and verifiers. This class should not be instantiated + */ private PKCEUtil() {} /** * Generates a high-entropy PKCE code_verifier. + * + * @return + * A 256bit or 64 byte random Base64 URL encode string */ public static String generateCodeVerifier() { byte[] bytes = new byte[64]; @@ -46,6 +54,13 @@ public final class PKCEUtil { /** * Computes the PKCE code_challenge = BASE64URL(SHA256(code_verifier)). + * + * @param String verifier + * A string containing the S56 verifier calculated bu generateCodeVerifier + * + * @return + * The generated S256 code challenge used for the PKCE request encoded + * in Base64 URL format. */ public static String generateCodeChallenge(String verifier) throws Exception { MessageDigest sha = MessageDigest.getInstance("SHA-256"); @@ -55,6 +70,12 @@ public final class PKCEUtil { /** * Base64URL encoding without padding. + * + * @param bytes + * The bytes to be Base54 URL encoded + * + * @return + * The Base64 URL encoded string value corresponding to the bytes */ public static String base64Url(byte[] bytes) { return java.util.Base64.getUrlEncoder()