Merge remote-tracking branch 'travisspencer/openid-feature-use-http-client' into jetty-9.4.x-4189-OpenIdHttpClient

jetty-openid/pom.xml

@@ -58,7 +58,6 @@
       <groupId>org.eclipse.jetty</groupId>
       <artifactId>jetty-client</artifactId>
       <version>${project.version}</version>
-      <scope>test</scope>
     </dependency>
   </dependencies>
 </project>

jetty-openid/src/main/config/etc/jetty-openid.xml

@@ -1,12 +1,16 @@
 <?xml version="1.0"?>
 <!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "http://www.eclipse.org/jetty/configure_9_3.dtd">
 <Configure id="Server" class="org.eclipse.jetty.server.Server">
+  <New id="HttpClientFactory" class="org.eclipse.jetty.security.openid.OpenIdHttpClientFactory">
+    <Arg type="Boolean"><Property name="jetty.openid.trustAllCertificates" default="false"/></Arg>
+  </New>
   <New id="OpenIdConfiguration" class="org.eclipse.jetty.security.openid.OpenIdConfiguration">
     <Arg><Property name="jetty.openid.provider" deprecated="jetty.openid.openIdProvider"/></Arg>
     <Arg><Property name="jetty.openid.provider.authorizationEndpoint"/></Arg>
     <Arg><Property name="jetty.openid.provider.tokenEndpoint"/></Arg>
     <Arg><Property name="jetty.openid.clientId"/></Arg>
     <Arg><Property name="jetty.openid.clientSecret"/></Arg>
+    <Arg><Ref refid="HttpClientFactory"/></Arg>
     <Call name="addScopes">
       <Arg>
         <Call class="org.eclipse.jetty.util.StringUtil" name="csvSplit">
@@ -20,6 +24,7 @@
       <New class="org.eclipse.jetty.security.openid.OpenIdLoginService">
         <Arg><Ref refid="OpenIdConfiguration"/></Arg>
         <Arg><Ref refid="BaseLoginService"/></Arg>
+        <Arg><Ref refid="HttpClientFactory"/></Arg>
         <Call name="setAuthenticateNewUsers">
           <Arg type="boolean">
             <Property name="jetty.openid.authenticateNewUsers" default="false"/>

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
 

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<String, Object> discoveryDocument = fetchOpenIdConnectMetadata(issuer);
+            Map<String, Object> 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<String, Object> fetchOpenIdConnectMetadata(String provider)
+    private static Map<String, Object> fetchOpenIdConnectMetadata(String provider, HttpClient httpClient)
     {
         try
         {
             if (provider.endsWith("/"))
                 provider = provider.substring(0, provider.length() - 1);
 
-            URI providerUri = URI.create(provider + CONFIG_PATH);
+            Map<String, Object> result;
-            InputStream inputStream = providerUri.toURL().openConnection().getInputStream();
+            String responseBody = httpClient.GET(provider + CONFIG_PATH)
-            String content = IO.toString(inputStream);
+                    .getContentAsString();
-            Map<String, Object> discoveryDocument = (Map)JSON.parse(content);
+            Object parsedResult = JSON.parse(responseBody);
-            if (LOG.isDebugEnabled())
-                LOG.debug("discovery document {}", discoveryDocument);
 
-            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);
         }

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;
  *
  * <p>
  * 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}.
  * </p>
  */
@@ -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<String, Object> claimAuthCode(String authCode) throws IOException
+    private Map<String, Object> claimAuthCode(HttpClient httpClient, String authCode)
     {
-        if (LOG.isDebugEnabled())
+        Map<String, Object> result;
-            LOG.debug("claimAuthCode {}", authCode);
 
-        // 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);
+            Fields fields = new Fields();
-            connection.setRequestMethod("POST");
+            fields.add("code", authCode);
-            connection.setRequestProperty("Host", configuration.getIssuer());
+            fields.add("client_id", configuration.getClientId());
-            connection.setRequestProperty("Content-Type", "application/x-www-form-urlencoded");
+            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);
             }
-
+            else
-            try (InputStream content = (InputStream)connection.getContent())
             {
-                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;
     }
 }

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;
+    }
+}

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;
         }

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");