From ba7b8f365c04ed1ec5750e13b7a6ee2455334712 Mon Sep 17 00:00:00 2001 From: Darran Lofthouse Date: Mon, 31 Jul 2023 15:26:16 +0100 Subject: [PATCH] Resolves #365 AutoCloseable jakarta.ws.rs Object instances not being closed after being used. --- .../OpenIdAuthenticationMechanism.java | 22 ++---- .../ProviderMetadataController.java | 32 ++++----- .../openid/controller/TokenController.java | 69 +++++++++++++++---- .../openid/controller/UserInfoController.java | 54 ++++++++------- 4 files changed, 107 insertions(+), 70 deletions(-) diff --git a/impl/src/main/java/org/glassfish/soteria/mechanisms/OpenIdAuthenticationMechanism.java b/impl/src/main/java/org/glassfish/soteria/mechanisms/OpenIdAuthenticationMechanism.java index 5a5b3fe..1e365d3 100644 --- a/impl/src/main/java/org/glassfish/soteria/mechanisms/OpenIdAuthenticationMechanism.java +++ b/impl/src/main/java/org/glassfish/soteria/mechanisms/OpenIdAuthenticationMechanism.java @@ -54,6 +54,7 @@ import org.glassfish.soteria.mechanisms.openid.controller.AuthenticationController; import org.glassfish.soteria.mechanisms.openid.controller.StateController; import org.glassfish.soteria.mechanisms.openid.controller.TokenController; +import org.glassfish.soteria.mechanisms.openid.controller.TokenController.TokensResponse; import org.glassfish.soteria.mechanisms.openid.domain.LogoutConfiguration; import org.glassfish.soteria.mechanisms.openid.domain.OpenIdConfiguration; import org.glassfish.soteria.mechanisms.openid.domain.OpenIdContextImpl; @@ -67,10 +68,8 @@ import jakarta.enterprise.inject.Instance; import jakarta.enterprise.inject.Typed; import jakarta.inject.Inject; -import jakarta.json.Json; import jakarta.json.JsonNumber; import jakarta.json.JsonObject; -import jakarta.json.JsonReader; import jakarta.security.auth.message.callback.CallerPrincipalCallback; import jakarta.security.enterprise.AuthenticationException; import jakarta.security.enterprise.AuthenticationStatus; @@ -82,7 +81,6 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpSession; -import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.UriBuilder; /** @@ -351,9 +349,9 @@ private AuthenticationStatus validateAuthorizationCode(HttpMessageContext httpCo LOGGER.finer("Authorization Code received, now fetching Access token & Id token"); - Response tokenResponse = tokenController.getTokens(request); - JsonObject tokensObject = readJsonObject(tokenResponse.readEntity(String.class)); - if (tokenResponse.getStatus() == OK.getStatusCode()) { + TokensResponse tokensResponse = tokenController.getTokens(request); + JsonObject tokensObject = tokensResponse.getTokensObject(); + if (tokensResponse.getStatus() == OK.getStatusCode()) { // Successful Token Response updateContext(tokensObject); OpenIdCredential credential = new OpenIdCredential(tokensObject, httpContext, configuration.getTokenMinValidity()); @@ -412,10 +410,10 @@ private AuthenticationStatus reAuthenticate(HttpMessageContext httpContext) thro } private AuthenticationStatus refreshTokens(HttpMessageContext httpContext, RefreshToken refreshToken) { - Response response = tokenController.refreshTokens(refreshToken); - JsonObject tokensObject = readJsonObject(response.readEntity(String.class)); + TokensResponse response = tokenController.refreshTokens(refreshToken); + JsonObject tokensObject = response.getTokensObject(); - if (response.getStatus() == Response.Status.OK.getStatusCode()) { + if (response.getStatus() == OK.getStatusCode()) { // Successful Token Response updateContext(tokensObject); OpenIdCredential credential = new OpenIdCredential(tokensObject, httpContext, configuration.getTokenMinValidity()); @@ -482,12 +480,6 @@ private static void redirect(HttpServletResponse response, String uri) { } } - private JsonObject readJsonObject(String tokensBody) { - try (JsonReader reader = Json.createReader(new StringReader(tokensBody))) { - return reader.readObject(); - } - } - private void updateContext(JsonObject tokensObject) { context.setTokenType(tokensObject.getString(TOKEN_TYPE, null)); diff --git a/impl/src/main/java/org/glassfish/soteria/mechanisms/openid/controller/ProviderMetadataController.java b/impl/src/main/java/org/glassfish/soteria/mechanisms/openid/controller/ProviderMetadataController.java index 008bde6..50da14e 100644 --- a/impl/src/main/java/org/glassfish/soteria/mechanisms/openid/controller/ProviderMetadataController.java +++ b/impl/src/main/java/org/glassfish/soteria/mechanisms/openid/controller/ProviderMetadataController.java @@ -76,25 +76,25 @@ public JsonObject getDocument(String providerURI) { } // Call - Client client = ClientBuilder.newClient(); - WebTarget target = client.target(providerURI); - Response response = target.request() + try (Client client = ClientBuilder.newClient()) { + WebTarget target = client.target(providerURI); + try (Response response = target.request() .accept(APPLICATION_JSON) - .get(); + .get()) { - if (response.getStatus() == Status.OK.getStatusCode()) { - // Get back the result of the REST request - String responseBody = response.readEntity(String.class); - try (JsonReader reader = Json.createReader(new StringReader(responseBody))) { - JsonObject responseObject = reader.readObject(); - providerDocuments.put(providerURI, responseObject); + if (response.getStatus() == Status.OK.getStatusCode()) { + // Get back the result of the REST request + String responseBody = response.readEntity(String.class); + try (JsonReader reader = Json.createReader(new StringReader(responseBody))) { + JsonObject responseObject = reader.readObject(); + providerDocuments.put(providerURI, responseObject); + } + } else { + throw new IllegalStateException(String.format( + "Unable to retrieve OpenID Provider's [%s] configuration document, HTTP respons code : [%s] ", + providerURI, response.getStatus())); + } } - } else { - throw new IllegalStateException(String.format( - "Unable to retrieve OpenID Provider's [%s] configuration document, HTTP respons code : [%s] ", - providerURI, - response.getStatus() - )); } } } diff --git a/impl/src/main/java/org/glassfish/soteria/mechanisms/openid/controller/TokenController.java b/impl/src/main/java/org/glassfish/soteria/mechanisms/openid/controller/TokenController.java index bcdb657..68c0689 100644 --- a/impl/src/main/java/org/glassfish/soteria/mechanisms/openid/controller/TokenController.java +++ b/impl/src/main/java/org/glassfish/soteria/mechanisms/openid/controller/TokenController.java @@ -20,6 +20,7 @@ import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON; import static java.util.Collections.emptyMap; +import java.io.StringReader; import java.util.Map; import org.glassfish.soteria.mechanisms.openid.domain.AccessTokenImpl; @@ -32,6 +33,9 @@ import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; +import jakarta.json.Json; +import jakarta.json.JsonObject; +import jakarta.json.JsonReader; import jakarta.security.enterprise.authentication.mechanism.http.HttpMessageContext; import jakarta.security.enterprise.authentication.mechanism.http.openid.OpenIdConstant; import jakarta.security.enterprise.identitystore.openid.IdentityToken; @@ -68,10 +72,10 @@ public class TokenController { * Provider responds with an ID Token and an Access Token. * * @param request - * @return a JSON object representation of OpenID Connect token response - * from the Token endpoint. + * @return a {@code TokenResponse} object containing the status code and a JSON object + * representation of the OpenID Connect token response from the Token endpoint. */ - public Response getTokens(HttpServletRequest request) { + public TokensResponse getTokens(HttpServletRequest request) { /* * one-time authorization code that RP exchange for an Access / Id token */ @@ -94,11 +98,17 @@ public Response getTokens(HttpServletRequest request) { .param(OpenIdConstant.REDIRECT_URI, configuration.buildRedirectURI(request)); // ID Token and Access Token Request - Client client = ClientBuilder.newClient(); - WebTarget target = client.target(configuration.getProviderMetadata().getTokenEndpoint()); - return target.request() + try (Client client = ClientBuilder.newClient()) { + WebTarget target = client.target(configuration.getProviderMetadata().getTokenEndpoint()); + try (Response response = target.request() .accept(APPLICATION_JSON) - .post(Entity.form(form)); + .post(Entity.form(form))) { + + JsonObject tokensObject = readJsonObject(response.readEntity(String.class)); + + return new TokensResponse(response.getStatus(), tokensObject); + } + } } /** @@ -175,10 +185,10 @@ public Map validateAccessToken(AccessTokenImpl accessToken, Algo * responds with a new (updated) Access Token and Refreshs Token. * * @param refreshToken Refresh Token received from previous token request. - * @return a JSON object representation of OpenID Connect token response - * from the Token endpoint. + * @return a {@code TokenResponse} object containing the status code and a JSON object + * representation of the OpenID Connect token response from the Token endpoint. */ - public Response refreshTokens(RefreshToken refreshToken) { + public TokensResponse refreshTokens(RefreshToken refreshToken) { Form form = new Form() .param(OpenIdConstant.CLIENT_ID, configuration.getClientId()) .param(OpenIdConstant.CLIENT_SECRET, new String(configuration.getClientSecret())) @@ -186,11 +196,44 @@ public Response refreshTokens(RefreshToken refreshToken) { .param(OpenIdConstant.REFRESH_TOKEN, refreshToken.getToken()); // Access Token and RefreshToken Request - Client client = ClientBuilder.newClient(); + try (Client client = ClientBuilder.newClient()) { WebTarget target = client.target(configuration.getProviderMetadata().getTokenEndpoint()); - return target.request() + try (Response response = target.request() .accept(APPLICATION_JSON) - .post(Entity.form(form)); + .post(Entity.form(form))) { + + JsonObject tokensObject = readJsonObject(response.readEntity(String.class)); + + return new TokensResponse(response.getStatus(), tokensObject); + + } + } + } + + private JsonObject readJsonObject(String tokensBody) { + try (JsonReader reader = Json.createReader(new StringReader(tokensBody))) { + return reader.readObject(); + } + } + + public static class TokensResponse { + private final int status; + private final JsonObject tokensObject; + + TokensResponse(int status, JsonObject tokensObject) { + super(); + this.status = status; + this.tokensObject = tokensObject; + } + + public int getStatus() { + return status; + } + + public JsonObject getTokensObject() { + return tokensObject; + } + } } diff --git a/impl/src/main/java/org/glassfish/soteria/mechanisms/openid/controller/UserInfoController.java b/impl/src/main/java/org/glassfish/soteria/mechanisms/openid/controller/UserInfoController.java index 74395de..9c4fd54 100644 --- a/impl/src/main/java/org/glassfish/soteria/mechanisms/openid/controller/UserInfoController.java +++ b/impl/src/main/java/org/glassfish/soteria/mechanisms/openid/controller/UserInfoController.java @@ -74,37 +74,39 @@ public JsonObject getUserInfo(OpenIdConfiguration configuration, AccessToken acc LOGGER.finest("Sending the request to the userinfo endpoint"); JsonObject userInfo; - Client client = ClientBuilder.newClient(); - WebTarget target = client.target(configuration.getProviderMetadata().getUserinfoEndpoint()); - Response response = target.request() + try (Client client = ClientBuilder.newClient()) { + WebTarget target = client.target(configuration.getProviderMetadata().getUserinfoEndpoint()); + try (Response response = target.request() .accept(APPLICATION_JSON) .header(AUTHORIZATION_HEADER, BEARER_TYPE + accessToken) // 5.5. Requesting Claims using the "claims" Request Parameter ?? - .get(); - - String responseBody = response.readEntity(String.class); - - String contentType = response.getHeaderString(CONTENT_TYPE); - if (response.getStatus() == Status.OK.getStatusCode()) { - if (nonNull(contentType) && contentType.contains(APPLICATION_JSON)) { - // Successful UserInfo Response - try (JsonReader reader = Json.createReader(new StringReader(responseBody))) { - userInfo = reader.readObject(); + .get()) { + + String responseBody = response.readEntity(String.class); + + String contentType = response.getHeaderString(CONTENT_TYPE); + if (response.getStatus() == Status.OK.getStatusCode()) { + if (nonNull(contentType) && contentType.contains(APPLICATION_JSON)) { + // Successful UserInfo Response + try (JsonReader reader = Json.createReader(new StringReader(responseBody))) { + userInfo = reader.readObject(); + } + } else if (nonNull(contentType) && contentType.contains(APPLICATION_JWT)) { + throw new UnsupportedOperationException("application/jwt content-type not supported for userinfo endpoint"); + //If the UserInfo Response is signed and/or encrypted, then the Claims are returned in a JWT and the content-type MUST be application/jwt. The response MAY be encrypted without also being signed. If both signing and encryption are requested, the response MUST be signed then encrypted, with the result being a Nested JWT, ?? + //If signed, the UserInfo Response SHOULD contain the Claims iss (issuer) and aud (audience) as members. The iss value SHOULD be the OP's Issuer Identifier URL. The aud value SHOULD be or include the RP's Client ID value. + } else { + throw new IllegalStateException("Invalid response received from userinfo endpoint with content-type : " + contentType); + } + } else { + // UserInfo Error Response + JsonObject responseObject = Json.createReader(new StringReader(responseBody)).readObject(); + String error = responseObject.getString(OpenIdConstant.ERROR_PARAM, "Unknown Error"); + String errorDescription = responseObject.getString(ERROR_DESCRIPTION_PARAM, "Unknown"); + LOGGER.log(WARNING, "Error occurred in fetching user info: {0} caused by {1}", new Object[]{error, errorDescription}); + throw new IllegalStateException("Error occurred in fetching user info"); } - } else if (nonNull(contentType) && contentType.contains(APPLICATION_JWT)) { - throw new UnsupportedOperationException("application/jwt content-type not supported for userinfo endpoint"); - //If the UserInfo Response is signed and/or encrypted, then the Claims are returned in a JWT and the content-type MUST be application/jwt. The response MAY be encrypted without also being signed. If both signing and encryption are requested, the response MUST be signed then encrypted, with the result being a Nested JWT, ?? - //If signed, the UserInfo Response SHOULD contain the Claims iss (issuer) and aud (audience) as members. The iss value SHOULD be the OP's Issuer Identifier URL. The aud value SHOULD be or include the RP's Client ID value. - } else { - throw new IllegalStateException("Invalid response received from userinfo endpoint with content-type : " + contentType); } - } else { - // UserInfo Error Response - JsonObject responseObject = Json.createReader(new StringReader(responseBody)).readObject(); - String error = responseObject.getString(OpenIdConstant.ERROR_PARAM, "Unknown Error"); - String errorDescription = responseObject.getString(ERROR_DESCRIPTION_PARAM, "Unknown"); - LOGGER.log(WARNING, "Error occurred in fetching user info: {0} caused by {1}", new Object[]{error, errorDescription}); - throw new IllegalStateException("Error occurred in fetching user info"); } validateUserInfoClaims(userInfo);