From 95f9d525aea99a40756ed8fe0e911c53e81cad50 Mon Sep 17 00:00:00 2001 From: David Bateman Date: Wed, 8 Apr 2026 16:57:35 +0200 Subject: [PATCH] Use SessionManager directly from getLoginURI to store PKCE removing the need for local REST redirect and callback function --- .../openid/AuthenticationProviderService.java | 152 +++++++-------- .../openid/OpenIDAuthenticationProvider.java | 2 +- .../OpenIDAuthenticationProviderModule.java | 1 - .../openid/OpenIDAuthenticationSession.java | 64 +------ .../OpenIDAuthenticationSessionManager.java | 33 ---- .../auth/openid/OpenIDRedirectResource.java | 173 ------------------ .../openid/conf/ConfigurationService.java | 2 +- .../auth/openid/conf/OpenIDEnvironment.java | 4 +- .../openid/token/TokenValidationService.java | 31 ++-- 9 files changed, 90 insertions(+), 372 deletions(-) delete mode 100644 extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDRedirectResource.java 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 7c48e0f22..111a35291 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 @@ -31,8 +31,8 @@ import java.util.Set; import javax.ws.rs.core.UriBuilder; import org.apache.guacamole.auth.openid.conf.ConfigurationService; import org.apache.guacamole.auth.openid.OpenIDAuthenticationSessionManager; -import org.apache.guacamole.auth.openid.OpenIDRedirectResource; import org.apache.guacamole.auth.openid.token.TokenValidationService; +import org.apache.guacamole.auth.openid.util.PKCEUtil; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.auth.sso.NonceService; import org.apache.guacamole.auth.sso.SSOAuthenticationProviderService; @@ -43,14 +43,25 @@ import org.apache.guacamole.language.TranslatableMessage; import org.apache.guacamole.net.auth.Credentials; import org.apache.guacamole.net.auth.credentials.CredentialsInfo; import org.apache.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException; +import org.apache.guacamole.net.auth.IdentifierGenerator; import org.jose4j.jwt.JwtClaims; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.guacamole.net.auth.IdentifierGenerator; + /** * Service that authenticates Guacamole users by processing OpenID tokens. */ @Singleton public class AuthenticationProviderService implements SSOAuthenticationProviderService { + /** + * Logger for this class. + */ + private final Logger logger = LoggerFactory.getLogger(AuthenticationProviderService.class); + /** * The standard HTTP parameter which will be included within the URL by all * OpenID services upon successful implicit flow authentication. @@ -63,7 +74,7 @@ public class AuthenticationProviderService implements SSOAuthenticationProviderS * the stored user state. */ public static final String CODE_TOKEN_PARAMETER_NAME = "code"; - + /** * The name of the query parameter that identifies an active authentication * session (in-progress OpenID authentication attempt). @@ -106,56 +117,6 @@ public class AuthenticationProviderService implements SSOAuthenticationProviderS @Inject private Provider authenticatedUserProvider; - @Override - public SSOAuthenticatedUser authenticateUser(Credentials credentials) - throws GuacamoleException { - if (isImplicit()) { - return authenticateUserImplicit(credentials); - } else { - return authenticateUserCode(credentials); - } - } - - private SSOAuthenticatedUser authenticateUserImplicit(Credentials credentials) - throws GuacamoleException { - - String username = null; - Set groups = null; - Map tokens = Collections.emptyMap(); - - // Validate OpenID token in request, if present, and derive username - String token = credentials.getParameter(IMPLICIT_TOKEN_PARAMETER_NAME); - if (token != null) { - JwtClaims claims = tokenService.validateTokenImplicit(token); - if (claims != null) { - username = tokenService.processUsername(claims); - groups = tokenService.processGroups(claims); - tokens = tokenService.processAttributes(claims); - } - } - - // If the username was successfully retrieved from the token, produce - // authenticated user - if (username != null) { - - // Create corresponding authenticated user - SSOAuthenticatedUser authenticatedUser = authenticatedUserProvider.get(); - authenticatedUser.init(username, credentials, groups, tokens); - return authenticatedUser; - - } - - // Request OpenID token (will automatically redirect the user to the - // OpenID authorization page via JavaScript) - throw new GuacamoleInvalidCredentialsException("Invalid login.", - new CredentialsInfo(Arrays.asList(new Field[] { - new RedirectField(IMPLICIT_TOKEN_PARAMETER_NAME, getLoginURI(), - new TranslatableMessage("LOGIN.INFO_IDP_REDIRECT_PENDING")) - })) - ); - - } - /** * Return the value of the session identifier associated with the given * credentials, or null if no session identifier is found in the @@ -175,22 +136,40 @@ public class AuthenticationProviderService implements SSOAuthenticationProviderS return credentials != null ? credentials.getParameter(AUTH_SESSION_QUERY_PARAM) : null; } - private SSOAuthenticatedUser authenticateUserCode(Credentials credentials) + @Override + public SSOAuthenticatedUser authenticateUser(Credentials credentials) throws GuacamoleException { String username = null; Set groups = null; Map tokens = Collections.emptyMap(); - // Recover session - String identifier = getSessionIdentifier(credentials); - OpenIDAuthenticationSession session = sessionManager.resume(identifier); - - // Validate OIDC token in request, if present, and derive username - if (session != null) { - String code = (String) session.getCode(); - if (code != null) { - JwtClaims claims = tokenService.validateTokenCode(code, session); + logger.debug("OpenID authentication with {} flow (ID: {}, Secret: {}, PKCE: {})", + confService.getFlowType(), + confService.getClientID(), + confService.getClientSecret(), + confService.isPKCERequired()); + + if (confService.getFlowType().equals("implicit")) { + String token = credentials.getParameter(IMPLICIT_TOKEN_PARAMETER_NAME); + JwtClaims claims = tokenService.validateToken(token); + if (claims != null) { + username = tokenService.processUsername(claims); + groups = tokenService.processGroups(claims); + tokens = tokenService.processAttributes(claims); + } + } else { + String verifier = null; + if (confService.isPKCERequired()) { + // Recover session + String identifier = getSessionIdentifier(credentials); + if (identifier != null) { + verifier = sessionManager.getVerifier(identifier); + } + } + String code = credentials.getParameter("code"); + if (code != null && (confService.isPKCERequired() == false || verifier != null)) { + JwtClaims claims = tokenService.validateCode(code, verifier); if (claims != null) { username = tokenService.processUsername(claims); groups = tokenService.processGroups(claims); @@ -206,7 +185,6 @@ public class AuthenticationProviderService implements SSOAuthenticationProviderS SSOAuthenticatedUser authenticatedUser = authenticatedUserProvider.get(); authenticatedUser.init(username, credentials, groups, tokens); return authenticatedUser; - } // Request OpenID token (will automatically redirect the user to the @@ -219,14 +197,9 @@ public class AuthenticationProviderService implements SSOAuthenticationProviderS ); } - private Boolean isImplicit() throws GuacamoleException { - String flowType = confService.getFlowType(); - return flowType.equals("implicit"); - } - @Override public URI getLoginURI() throws GuacamoleException { - if (isImplicit()) { + if (confService.getFlowType().equals("implicit")) { return UriBuilder.fromUri(confService.getAuthorizationEndpoint()) .queryParam("scope", confService.getScope()) .queryParam("response_type", IMPLICIT_TOKEN_PARAMETER_NAME) @@ -234,23 +207,36 @@ public class AuthenticationProviderService implements SSOAuthenticationProviderS .queryParam("redirect_uri", confService.getRedirectURI()) .queryParam("nonce", nonceService.generate(confService.getMaxNonceValidity() * 60000L)) .build(); - } else { - /** - * For Code flow and PKCE support we need access to the users session to - * store the PKCE verifier and be able to pass the returned code form the - * IdP to authenticateUser. Guacamole doesn't expose the user session here. - * We get around that by creating a redirect to a local REST api that then - * does the redirection/callback and stores the code and verifier in the - * user session. - */ - URI redirect = UriBuilder.fromUri(confService.getRedirectURI()) - .path(CODE_REDIRECT_API).build(); - return UriBuilder.fromUri(redirect) + } else { + UriBuilder builder = UriBuilder.fromUri(confService.getAuthorizationEndpoint()) .queryParam("scope", confService.getScope()) .queryParam("response_type", CODE_TOKEN_PARAMETER_NAME) .queryParam("client_id", confService.getClientID()) - .queryParam("redirect_uri", redirect) - .build(); + .queryParam("redirect_uri", confService.getRedirectURI()); + + if (confService.isPKCERequired()) { + String codeVerifier = PKCEUtil.generateCodeVerifier(); + String codeChallenge; + + try { + codeChallenge = PKCEUtil.generateCodeChallenge(codeVerifier); + } + catch (Exception e) { + throw new GuacamoleException("Unable to compute PKCE challenge", e); + } + + // Store verifier for authenticateUser + OpenIDAuthenticationSession session = new OpenIDAuthenticationSession(codeVerifier, + confService.getAuthenticationTimeout() * 60000L); + String identifier = IdentifierGenerator.generateIdentifier(); + sessionManager.defer(session, identifier); + + builder.queryParam("code_challenge", codeChallenge) + .queryParam("code_challenge_method", "S256") + .queryParam(AUTH_SESSION_QUERY_PARAM, identifier); + } + + return builder.build(); } } diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationProvider.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationProvider.java index 37353c31e..a760854a6 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationProvider.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationProvider.java @@ -35,7 +35,7 @@ public class OpenIDAuthenticationProvider extends SSOAuthenticationProvider { * against an OpenID service. */ public OpenIDAuthenticationProvider() { - super(AuthenticationProviderService.class, OpenIDRedirectResource.class, + super(AuthenticationProviderService.class, SSOResource.class, new OpenIDAuthenticationProviderModule()); } 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 59d278177..bbd5c4c34 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 @@ -24,7 +24,6 @@ import com.google.inject.Scopes; import org.apache.guacamole.auth.openid.conf.ConfigurationService; import org.apache.guacamole.auth.openid.conf.OpenIDEnvironment; import org.apache.guacamole.auth.openid.OpenIDAuthenticationSessionManager; -import org.apache.guacamole.auth.openid.OpenIDRedirectResource; import org.apache.guacamole.auth.sso.NonceService; import org.apache.guacamole.auth.openid.token.TokenValidationService; import org.apache.guacamole.environment.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 29eaa1efc..320f0609d 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 @@ -28,17 +28,7 @@ public class OpenIDAuthenticationSession extends AuthenticationSession { /** * The PKCE challenge verifier. */ - private String verifier = null; - - /** - * The redirect URI used by the identity provide. - */ - private String redirect_uri = null; - - /** - * The code returned by the identity provider use to exchange for a token - */ - private String code = null; + private final String verifier; /** * Creates a new AuthenticationSession representing an in-progress OpenID @@ -48,17 +38,8 @@ public class OpenIDAuthenticationSession extends AuthenticationSession { * The number of milliseconds that may elapse before this session must * be considered invalid. */ - public OpenIDAuthenticationSession(long expires) { + public OpenIDAuthenticationSession(String verifier, long expires) { super(expires); - } - - /** - * Set the pkce_verifier - * - * @param verifier - * The verifier to be stored - */ - public void setVerifier(String verifier) { this.verifier = verifier; } @@ -71,46 +52,5 @@ public class OpenIDAuthenticationSession extends AuthenticationSession { public String getVerifier() { return verifier; } - - /** - * Set the redirect URI sent to the identity provider - * - * @param redirect_uri - * The redirect UTI to be stored - */ - public void setRedirectURI(String redirect_uri) { - this.redirect_uri = redirect_uri; - } - - /** - * Returns the stored redirect URI sent to the identity provider - * - * @return - * The redirect URI - */ - public String getRedirectURI() { - return redirect_uri; - } - - /** - * Set the code returned by the identity provider to exchange for a token - * - * @param code - * The code to be stored - */ - public void setCode(String code) { - this.code = code; - } - - /** - * Returns the stored code returned by the identity provider to exchange for a token - * - * @return - * The stored code - */ - public String getCode() { - return code; - } - } diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationSessionManager.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationSessionManager.java index 7eac71678..8a2371259 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationSessionManager.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationSessionManager.java @@ -29,22 +29,6 @@ import org.apache.guacamole.net.auth.AuthenticationSessionManager; @Singleton public class OpenIDAuthenticationSessionManager extends AuthenticationSessionManager { - /** - * Returns the stored code returned by the identity provider to exchange for a token - * - * @param identifier - * The unique string returned by the call to defer(). For convenience, - * this value may safely be null. - * - * @return - * The code returned by the identity provider - */ - public String getCode(String identifier) { - OpenIDAuthenticationSession session = resume(identifier); - if (session != null) - return session.getCode(); - return null; - } /** * Returns the stored PKCE verifier used with the identity provider @@ -62,22 +46,5 @@ public class OpenIDAuthenticationSessionManager return session.getVerifier(); return null; } - - /** - * Returns the stored redirect URI used with the identity provider - * - * @param identifier - * The unique string returned by the call to defer(). For convenience, - * this value may safely be null. - * - * @return - * The redirect URIused with the identity provider - */ - public String getRedirectURI(String identifier) { - OpenIDAuthenticationSession session = resume(identifier); - if (session != null) - return session.getRedirectURI(); - return null; - } } diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDRedirectResource.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDRedirectResource.java deleted file mode 100644 index 6b37e47ec..000000000 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDRedirectResource.java +++ /dev/null @@ -1,173 +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.openid; - -import com.google.inject.Inject; -import com.google.inject.Singleton; -import java.net.URI; -import java.util.Map; -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.GET; -import javax.ws.rs.Path; -import javax.ws.rs.core.Context; -import javax.ws.rs.core.Response; -import javax.ws.rs.core.UriBuilder; - -import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.auth.openid.AuthenticationProviderService; -import org.apache.guacamole.auth.openid.conf.ConfigurationService; -import org.apache.guacamole.auth.openid.OpenIDAuthenticationSessionManager; -import org.apache.guacamole.auth.openid.util.PKCEUtil; -import org.apache.guacamole.auth.sso.SSOResource; -import org.apache.guacamole.net.auth.IdentifierGenerator; - -/** - * Local REST endpoint used by Guacamole to initiate the OIDC login with code flow and PKCE. - * - * This endpoint: - * - receives the request from Guacamole Web UI - * - generates PKCE code_verifier and code_challenge - * - stores code_verifier in an temporary OpenIDAuthenicationSession - * - redirects the browser to the Identity Provider (Keycloak, etc.) - * - * This endpoint is the place where PKCE MUST be handled because when getLoginURI() in - * AuthenticationProvider is called the authentication hasn't started yet - */ -public class OpenIDRedirectResource extends SSOResource { - - /** - * The configuration service for this module. - */ - @Inject - private ConfigurationService confService; - - /** - * Manager of active OpenID authentication attempts. - */ - @Inject - private OpenIDAuthenticationSessionManager sessionManager; - - /** - * Local redirect endpoint invoked by Guacamole to pass to the identity provider - * for code flow. Used to create and store PKCE challenges and store code values - * returned by the identity provider. - * - * @param request - * The HttpServletRequest from Guacamole or the identity provider - * - * @return - * A redirect to the identity provider if the query parameter 'code' doesn't - * exist or a redirect to Guacamole to continue the authentication process - * with the values necessary to code flow in authenticateUser - * - * @throws GuacamoleException - * If the PKCE challenge can not be generated or the verifier recovered - */ - @GET - @Path("/redirect") - public Response redirectToFromIdentityProvider(@Context HttpServletRequest request) - throws GuacamoleException { - String code = request.getParameter("code"); - - if (code == null) { - UriBuilder builder = UriBuilder.fromUri(confService.getAuthorizationEndpoint()); - - // Copy inbound request params - @SuppressWarnings("unchecked") - Map params = (Map) (Map) request.getParameterMap(); - for (Map.Entry entry : params.entrySet()) { - for (String value : entry.getValue()) { - builder = builder.queryParam(entry.getKey(), value); - } - } - - // Create a new authentication session to represent this attempt while - // it is in progress, using the request ID that was just issued - OpenIDAuthenticationSession session = new OpenIDAuthenticationSession( - confService.getAuthenticationTimeout() * 60000L); - - // PKCE support - if (confService.isPKCERequired()) { - - String codeVerifier = PKCEUtil.generateCodeVerifier(); - String codeChallenge; - - try { - codeChallenge = PKCEUtil.generateCodeChallenge(codeVerifier); - } - catch (Exception e) { - throw new GuacamoleException("Unable to compute PKCE challenge", e); - } - - // Store verifier for authenticateUser - session.setVerifier(codeVerifier); - - builder.queryParam("code_challenge", codeChallenge) - .queryParam("code_challenge_method", "S256"); - } - - // Store redirect_uri for exchange of code for token, requires exact same uri - String redirectURI = request.getParameter("redirect_uri"); - session.setRedirectURI(redirectURI); - - // Generate a unique ID to use to identify stored values - String identifier = IdentifierGenerator.generateIdentifier(); - builder.queryParam(AuthenticationProviderService.AUTH_SESSION_QUERY_PARAM, identifier); - - // Save the session with the stored values - sessionManager.defer(session, identifier); - - return Response.seeOther(builder.build()).build(); - } - - // Retrieve the stored session - String identifier = request.getParameter(AuthenticationProviderService.AUTH_SESSION_QUERY_PARAM); - OpenIDAuthenticationSession session = sessionManager.resume(identifier); - - if (confService.isPKCERequired()) { - // Retrieve stored PKCE verifier - String verifier = session.getVerifier(); - - if (verifier == null) - throw new GuacamoleException("Missing PKCE verifier from session."); - } - - // Retrieve stored redirect URI - String redirectURI = session.getRedirectURI(); - - if (redirectURI == null) - throw new GuacamoleException("Missing redirect URI from session."); - - // Store the authorization code for authenticateUser() - session.setCode(code); - - // Save the session with the stored values. Need to reactivate so it is - // available for the next resume - sessionManager.defer(session, identifier); - sessionManager.reactivateSession(identifier); - - // Redirect browser back to Guacamole UI to continue login - URI resume = UriBuilder.fromUri(confService.getRedirectURI()) - .queryParam(AuthenticationProviderService.AUTH_SESSION_QUERY_PARAM, identifier) - .build(); - return Response.seeOther(resume).build(); - } -} 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 2a35dfa4c..cf7bc1407 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 @@ -172,7 +172,7 @@ public class ConfigurationService { new IntegerGuacamoleProperty() { @Override public String getName() { return "openid-auth-timeout"; } - + }; diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/OpenIDEnvironment.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/OpenIDEnvironment.java index 440463081..cb80689a2 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/OpenIDEnvironment.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/OpenIDEnvironment.java @@ -27,7 +27,7 @@ import org.apache.guacamole.environment.LocalEnvironment; * configuration. */ public class OpenIDEnvironment extends DelegatingEnvironment { - + /** * Create a new instance of the configuration environment for the * OpenID SSO module, pulling the default instance of the LocalEnvironment. @@ -35,5 +35,5 @@ public class OpenIDEnvironment extends DelegatingEnvironment { public OpenIDEnvironment() { super(LocalEnvironment.getInstance()); } - + } 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 589edd005..fd9f4c16a 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 @@ -39,7 +39,6 @@ import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.UriBuilder; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.auth.openid.conf.ConfigurationService; -import org.apache.guacamole.auth.openid.OpenIDAuthenticationSession; import org.apache.guacamole.auth.sso.NonceService; import org.apache.guacamole.token.TokenName; import org.jose4j.json.JsonUtil; @@ -95,7 +94,7 @@ public class TokenValidationService { * @throws GuacamoleException * If guacamole.properties could not be parsed. */ - public JwtClaims validateTokenImplicit(String token) throws GuacamoleException { + public JwtClaims validateToken(String token) throws GuacamoleException { // Validating the token requires a JWKS key resolver HttpsJwks jwks = new HttpsJwks(confService.getJWKSEndpoint().toString()); HttpsJwksVerificationKeyResolver resolver = new HttpsJwksVerificationKeyResolver(jwks); @@ -150,8 +149,8 @@ public class TokenValidationService { * @param code * The code to validate and receive the id_token. * - * @param session - * A OpenIDAuthenicationSession storing the in progress authentication parameters + * @param verifier + * A PKCE verifier or null if not used. * * @return * The JWT claims contained within the given ID token if it passes tests, @@ -160,13 +159,13 @@ public class TokenValidationService { * @throws GuacamoleException * If guacamole.properties could not be parsed. */ - public JwtClaims validateTokenCode(String code, OpenIDAuthenticationSession session) throws GuacamoleException { + 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, session); + String token = exchangeCode(code, verifier); // Create JWT consumer for validating received token JwtConsumer jwtConsumer = new JwtConsumerBuilder() @@ -190,7 +189,7 @@ public class TokenValidationService { return null; } - + /** * URLEncodes a key/value pair * @@ -225,26 +224,25 @@ public class TokenValidationService { * @throws GuacamoleException * If a valid token is not returned. */ - private String exchangeCode(String code, OpenIDAuthenticationSession session) throws GuacamoleException { + private String exchangeCode(String code, String verifier) throws GuacamoleException { try { StringBuilder bodyBuilder = new StringBuilder(); bodyBuilder.append(urlencode("grant_type", "authorization_code")).append("&"); bodyBuilder.append(urlencode("code", code)).append("&"); - bodyBuilder.append(urlencode("redirect_uri", session.getRedirectURI())).append("&"); + bodyBuilder.append(urlencode("redirect_uri", confService.getRedirectURI().toString())).append("&"); bodyBuilder.append(urlencode("scope", confService.getScope())).append("&"); bodyBuilder.append(urlencode("client_id", confService.getClientID())); - + String clientSecret = confService.getClientSecret(); if (clientSecret != null && !clientSecret.trim().isEmpty()) { bodyBuilder.append("&").append(urlencode("client_secret", clientSecret)); } if (confService.isPKCERequired()) { - String codeVerifier = session.getVerifier(); - bodyBuilder.append("&").append(urlencode("code_verifier", codeVerifier)); + bodyBuilder.append("&").append(urlencode("code_verifier", verifier)); } - + // Build the final URI and convert to a URL URL url = confService.getTokenEndpoint().toURL(); @@ -289,14 +287,15 @@ public class TokenValidationService { return (String) json.get("id_token"); } catch (Exception e) { - throw new GuacamoleException("Token exchange failed.", e); + logger.info("Rejected invalid OpenID code exchange: {}", e.getMessage(), e); } + return null; } /** * Parses the given JwtClaims, returning the username contained * therein, as defined by the username claim type given in - * guacamole.properties. If the username claim type is missing or + * guacamole.properties. If the username claim type is missing or * is invalid, null is returned. * * @param claims @@ -360,7 +359,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); }