From 46d3a371cf18108e8bc7bb8e971319f4e6f674ee Mon Sep 17 00:00:00 2001 From: Travis Spencer Date: Mon, 30 Sep 2019 04:23:46 +0200 Subject: [PATCH] Use HttpClient instead of HttpURLConnection Signed-off-by: Travis Spencer --- jetty-openid/pom.xml | 1 - .../src/main/config/etc/jetty-openid.xml | 5 ++ .../src/main/config/modules/openid.mod | 4 + .../security/openid/OpenIdConfiguration.java | 46 ++++++---- .../security/openid/OpenIdCredentials.java | 88 +++++++++++++------ .../openid/OpenIdHttpClientFactory.java | 62 +++++++++++++ .../security/openid/OpenIdLoginService.java | 11 ++- .../openid/OpenIdAuthenticationTest.java | 2 +- 8 files changed, 170 insertions(+), 49 deletions(-) create mode 100644 jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdHttpClientFactory.java diff --git a/jetty-openid/pom.xml b/jetty-openid/pom.xml index b92d1260654..225ef588b5a 100644 --- a/jetty-openid/pom.xml +++ b/jetty-openid/pom.xml @@ -58,7 +58,6 @@ org.eclipse.jetty jetty-client ${project.version} - test diff --git a/jetty-openid/src/main/config/etc/jetty-openid.xml b/jetty-openid/src/main/config/etc/jetty-openid.xml index 83b6d798773..7d1f58e28b8 100644 --- a/jetty-openid/src/main/config/etc/jetty-openid.xml +++ b/jetty-openid/src/main/config/etc/jetty-openid.xml @@ -1,12 +1,16 @@ + + + + @@ -20,6 +24,7 @@ + diff --git a/jetty-openid/src/main/config/modules/openid.mod b/jetty-openid/src/main/config/modules/openid.mod index 7f1450c3fe6..78fc8b92aec 100644 --- a/jetty-openid/src/main/config/modules/openid.mod +++ b/jetty-openid/src/main/config/modules/openid.mod @@ -5,6 +5,7 @@ Adds OpenId Connect authentication. [depend] security +client [lib] lib/jetty-openid-${jetty.version}.jar @@ -21,6 +22,9 @@ etc/jetty-openid.xml ## The OpenID Identity Provider's issuer ID (the entire URL *before* ".well-known/openid-configuration") # jetty.openid.provider=https://id.example.com/~ +## Whether or not all certificates of the OpenID Connect provider's should be trusted. Only set to true during testing. +# jetty.openid.trustAllCertificates=false + ## The OpenID Identity Provider's authorization endpoint (optional if the metadata of the OP is accessible) # jetty.openid.provider.authorizationEndpoint=https://id.example.com/authorization diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdConfiguration.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdConfiguration.java index e628f1aaa09..920982828bc 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdConfiguration.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdConfiguration.java @@ -18,16 +18,15 @@ package org.eclipse.jetty.security.openid; -import java.io.InputStream; import java.io.Serializable; -import java.net.URI; import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.stream.Collectors; -import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.util.ajax.JSON; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -59,7 +58,7 @@ public class OpenIdConfiguration implements Serializable */ public OpenIdConfiguration(String provider, String clientId, String clientSecret) { - this(provider, null, null, clientId, clientSecret); + this(provider, null, null, clientId, clientSecret, null); } /** @@ -69,8 +68,10 @@ public class OpenIdConfiguration implements Serializable * @param tokenEndpoint the URL of the OpenID provider's token endpoint if configured. * @param clientId OAuth 2.0 Client Identifier valid at the Authorization Server. * @param clientSecret The client secret known only by the Client and the Authorization Server. + * @param factory A factory that can create the {@link HttpClient} which will discover the provider's metadata. */ - public OpenIdConfiguration(String issuer, String authorizationEndpoint, String tokenEndpoint, String clientId, String clientSecret) + public OpenIdConfiguration(String issuer, String authorizationEndpoint, String tokenEndpoint, + String clientId, String clientSecret, OpenIdHttpClientFactory factory) { this.issuer = issuer; this.clientId = clientId; @@ -81,7 +82,8 @@ public class OpenIdConfiguration implements Serializable if (tokenEndpoint == null || authorizationEndpoint == null) { - Map discoveryDocument = fetchOpenIdConnectMetadata(issuer); + Map discoveryDocument = fetchOpenIdConnectMetadata(issuer, factory == null ? + new HttpClient() : factory.createHttpClient()); this.authEndpoint = (String)discoveryDocument.get("authorization_endpoint"); if (this.authEndpoint == null) @@ -101,23 +103,37 @@ public class OpenIdConfiguration implements Serializable } } - private static Map fetchOpenIdConnectMetadata(String provider) + private static Map fetchOpenIdConnectMetadata(String provider, HttpClient httpClient) { try { if (provider.endsWith("/")) provider = provider.substring(0, provider.length() - 1); - URI providerUri = URI.create(provider + CONFIG_PATH); - InputStream inputStream = providerUri.toURL().openConnection().getInputStream(); - String content = IO.toString(inputStream); - Map discoveryDocument = (Map)JSON.parse(content); - if (LOG.isDebugEnabled()) - LOG.debug("discovery document {}", discoveryDocument); + Map result; + String responseBody = httpClient.GET(provider + CONFIG_PATH) + .getContentAsString(); + Object parsedResult = JSON.parse(responseBody); - return discoveryDocument; + if (parsedResult instanceof Map) + { + Map rawResult = (Map)parsedResult; + + result = rawResult.entrySet().stream() + .collect(Collectors.toMap(it -> it.getKey().toString(), Map.Entry::getValue)); + } + else + { + LOG.warn("OpenID provider did not return a proper JSON object response. Result was '{}'", responseBody); + + throw new IllegalStateException("Could not parse OpenID provider's malformed response"); + } + + LOG.debug("discovery document {}", result); + + return result; } - catch (Throwable e) + catch (Exception e) { throw new IllegalArgumentException("invalid identity provider", e); } diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java index da0f0fac7bd..f810fa00a0a 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java @@ -27,9 +27,18 @@ import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Base64; -import java.util.List; +import java.util.Collections; import java.util.Map; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.stream.Collectors; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.client.api.Request; +import org.eclipse.jetty.client.util.FormContentProvider; +import org.eclipse.jetty.util.Fields; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.UrlEncoded; import org.eclipse.jetty.util.ajax.JSON; @@ -42,7 +51,7 @@ import org.eclipse.jetty.util.log.Logger; * *

* This is constructed with an authorization code from the authentication request. This authorization code - * is then exchanged using {@link #redeemAuthCode()} for a response containing the ID Token and Access Token. + * is then exchanged using {@link #claimAuthCode} for a response containing the ID Token and Access Token. * The response is then validated against the {@link OpenIdConfiguration}. *

*/ @@ -79,7 +88,7 @@ public class OpenIdCredentials implements Serializable return response; } - public void redeemAuthCode() throws IOException + public void redeemAuthCode(HttpClient httpClient) throws IOException { if (LOG.isDebugEnabled()) LOG.debug("redeemAuthCode() {}", this); @@ -88,7 +97,7 @@ public class OpenIdCredentials implements Serializable { try { - response = claimAuthCode(authCode); + response = claimAuthCode(httpClient, authCode); if (LOG.isDebugEnabled()) LOG.debug("response: {}", response); @@ -120,8 +129,14 @@ public class OpenIdCredentials implements Serializable private void validateClaims() { // Issuer Identifier for the OpenID Provider MUST exactly match the value of the iss (issuer) Claim. - if (!configuration.getIssuer().equals(claims.get("iss"))) + Object assertedIssuer = claims.get("iss"); + String configuredIssuer = configuration.getIssuer(); + + if (!configuredIssuer.equals(assertedIssuer)) + { + LOG.warn("Issuers don't match. Configured = {}, asserted = {}", configuredIssuer, assertedIssuer); throw new IllegalArgumentException("Issuer Identifier MUST exactly match the iss Claim"); + } // The aud (audience) Claim MUST contain the client_id value. validateAudience(); @@ -224,40 +239,57 @@ public class OpenIdCredentials implements Serializable return paddedEncodedJwtSection; } - private Map claimAuthCode(String authCode) throws IOException + private Map claimAuthCode(HttpClient httpClient, String authCode) { - if (LOG.isDebugEnabled()) - LOG.debug("claimAuthCode {}", authCode); + Map result; - // Use the authorization code to get the id_token from the OpenID Provider - String urlParameters = "code=" + authCode + - "&client_id=" + UrlEncoded.encodeString(configuration.getClientId(), StandardCharsets.UTF_8) + - "&client_secret=" + UrlEncoded.encodeString(configuration.getClientSecret(), StandardCharsets.UTF_8) + - "&redirect_uri=" + UrlEncoded.encodeString(redirectUri, StandardCharsets.UTF_8) + - "&grant_type=authorization_code"; - - URL url = new URL(configuration.getTokenEndpoint()); - HttpURLConnection connection = (HttpURLConnection)url.openConnection(); try { - connection.setDoOutput(true); - connection.setRequestMethod("POST"); - connection.setRequestProperty("Host", configuration.getIssuer()); - connection.setRequestProperty("Content-Type", "application/x-www-form-urlencoded"); + Fields fields = new Fields(); + fields.add("code", authCode); + fields.add("client_id", configuration.getClientId()); + fields.add("client_secret", configuration.getClientSecret()); + fields.add("redirect_uri", redirectUri); + fields.add("grant_type", "authorization_code"); + FormContentProvider formContentProvider = new FormContentProvider(fields); + Request request = httpClient.POST(configuration.getTokenEndpoint()) + .content(formContentProvider) + .timeout(10, TimeUnit.SECONDS); + ContentResponse response = request.send(); + String responseBody = response.getContentAsString(); + Object parsedResult = JSON.parse(responseBody); - try (DataOutputStream wr = new DataOutputStream(connection.getOutputStream())) + if (parsedResult instanceof Map) { - wr.write(urlParameters.getBytes(StandardCharsets.UTF_8)); + Map rawResult = (Map)parsedResult; + + result = rawResult.entrySet().stream().collect(Collectors.toMap(it -> it.getKey().toString(), Map.Entry::getValue)); + + LOG.debug("Got result from token server: {}", result); } - - try (InputStream content = (InputStream)connection.getContent()) + else { - return (Map)JSON.parse(IO.toString(content)); + LOG.warn("OpenID provider did not return a proper JSON object response. Result was '{}'", responseBody); + + throw new IllegalStateException("Could not parse OpenID provider's malformed response"); } } - finally + catch (InterruptedException e) { - connection.disconnect(); + LOG.debug("Call to token endpoint was interrupted", e); + result = Collections.emptyMap(); } + catch (ExecutionException e) + { + LOG.warn("Call to token endpoint was failed", e); + result = Collections.emptyMap(); + } + catch (TimeoutException e) + { + LOG.warn("Call to token endpoint was did not complete in a timely manner", e); + result = Collections.emptyMap(); + } + + return result; } } diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdHttpClientFactory.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdHttpClientFactory.java new file mode 100644 index 00000000000..a8e93c5cc5b --- /dev/null +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdHttpClientFactory.java @@ -0,0 +1,62 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.security.openid; + +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.HttpClientTransport; +import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; +import org.eclipse.jetty.util.ssl.SslContextFactory; + +public final class OpenIdHttpClientFactory +{ + private final boolean trustAll; + + public OpenIdHttpClientFactory() + { + this(false); + } + + public OpenIdHttpClientFactory(boolean trustAll) + { + this.trustAll = trustAll; + } + + public HttpClient createHttpClient() + { + SslContextFactory.Client sslContextFactory = new SslContextFactory.Client(trustAll); + + sslContextFactory.setEndpointIdentificationAlgorithm("https"); + + HttpClientTransport transport = new HttpClientTransportOverHTTP(); + HttpClient client = new HttpClient(transport, sslContextFactory); + + client.setFollowRedirects(false); + + try + { + client.start(); + } + catch (Exception e) + { + throw new RuntimeException(e); + } + + return client; + } +} diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java index 67ec142825b..d7acc0afe8f 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java @@ -22,6 +22,7 @@ import java.security.Principal; import javax.security.auth.Subject; import javax.servlet.ServletRequest; +import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.security.IdentityService; import org.eclipse.jetty.security.LoginService; import org.eclipse.jetty.server.UserIdentity; @@ -42,12 +43,13 @@ public class OpenIdLoginService extends ContainerLifeCycle implements LoginServi private final OpenIdConfiguration _configuration; private final LoginService loginService; + private final HttpClient httpClient; private IdentityService identityService; private boolean authenticateNewUsers; - public OpenIdLoginService(OpenIdConfiguration configuration) + public OpenIdLoginService(OpenIdConfiguration configuration, OpenIdHttpClientFactory factory) { - this(configuration, null); + this(configuration, null, factory); } /** @@ -57,10 +59,11 @@ public class OpenIdLoginService extends ContainerLifeCycle implements LoginServi * @param configuration the OpenID configuration to use. * @param loginService the wrapped LoginService to defer to for user roles. */ - public OpenIdLoginService(OpenIdConfiguration configuration, LoginService loginService) + public OpenIdLoginService(OpenIdConfiguration configuration, LoginService loginService, OpenIdHttpClientFactory factory) { _configuration = configuration; this.loginService = loginService; + this.httpClient = factory.createHttpClient(); addBean(this.loginService); } @@ -84,7 +87,7 @@ public class OpenIdLoginService extends ContainerLifeCycle implements LoginServi OpenIdCredentials openIdCredentials = (OpenIdCredentials)credentials; try { - openIdCredentials.redeemAuthCode(); + openIdCredentials.redeemAuthCode(httpClient); if (openIdCredentials.isExpired()) return null; } diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java index 54dd613c52d..0a6da13a8cd 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java @@ -102,7 +102,7 @@ public class OpenIdAuthenticationTest OpenIdConfiguration configuration = new OpenIdConfiguration(openIdProvider.getProvider(), CLIENT_ID, CLIENT_SECRET); // Configure OpenIdLoginService optionally providing a base LoginService to provide user roles - OpenIdLoginService loginService = new OpenIdLoginService(configuration);//, hashLoginService); + OpenIdLoginService loginService = new OpenIdLoginService(configuration, new OpenIdHttpClientFactory()); securityHandler.setLoginService(loginService); Authenticator authenticator = new OpenIdAuthenticator(configuration, "/error");