add changes from merge

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-ee10/jetty-ee10-openid/src/main/java/org/eclipse/jetty/ee10/security/openid/OpenIdConfiguration.java

@@ -40,6 +40,9 @@ public class OpenIdConfiguration extends ContainerLifeCycle
 {
     private static final Logger LOG = LoggerFactory.getLogger(OpenIdConfiguration.class);
     private static final String CONFIG_PATH = "/.well-known/openid-configuration";
+    private static final String AUTHORIZATION_ENDPOINT = "authorization_endpoint";
+    private static final String TOKEN_ENDPOINT = "token_endpoint";
+    private static final String ISSUER = "issuer";
 
     private final HttpClient httpClient;
     private final String issuer;
@@ -116,35 +119,45 @@ public class OpenIdConfiguration extends ContainerLifeCycle
 
         if (authEndpoint == null || tokenEndpoint == null)
         {
-            Map<String, Object> discoveryDocument = fetchOpenIdConnectMetadata(issuer, httpClient);
+            Map<String, Object> discoveryDocument = fetchOpenIdConnectMetadata();
-
+            processMetadata(discoveryDocument);
-            authEndpoint = (String)discoveryDocument.get("authorization_endpoint");
-            if (authEndpoint == null)
-                throw new IllegalArgumentException("authorization_endpoint");
-
-            tokenEndpoint = (String)discoveryDocument.get("token_endpoint");
-            if (tokenEndpoint == null)
-                throw new IllegalArgumentException("token_endpoint");
-
-            if (!Objects.equals(discoveryDocument.get("issuer"), issuer))
-                LOG.warn("The issuer in the metadata is not correct.");
         }
     }
 
-    private static HttpClient newHttpClient()
+    /**
+     * Process the OpenID Connect metadata discovered by {@link #fetchOpenIdConnectMetadata()}.
+     * By default, only the {@link #AUTHORIZATION_ENDPOINT} and {@link #TOKEN_ENDPOINT} claims are extracted.
+     * @see <a href="https://openid.net/specs/openid-connect-discovery-1_0.html">OpenID Connect Discovery 1.0</a>
+     * @throws IllegalStateException if a required field is not present in the metadata.
+     */
+    protected void processMetadata(Map<String, Object> discoveryDocument)
     {
-        ClientConnector connector = new ClientConnector();
+        authEndpoint = (String)discoveryDocument.get(AUTHORIZATION_ENDPOINT);
-        connector.setSslContextFactory(new SslContextFactory.Client(false));
+        if (authEndpoint == null)
-        return new HttpClient(new HttpClientTransportOverHTTP(connector));
+            throw new IllegalStateException(AUTHORIZATION_ENDPOINT);
+
+        tokenEndpoint = (String)discoveryDocument.get(TOKEN_ENDPOINT);
+        if (tokenEndpoint == null)
+            throw new IllegalStateException(TOKEN_ENDPOINT);
+
+        // We are lenient and not throw here as some major OIDC providers do not conform to this.
+        if (!Objects.equals(discoveryDocument.get(ISSUER), issuer))
+            LOG.warn("The issuer in the metadata is not correct.");
     }
 
-    private static Map<String, Object> fetchOpenIdConnectMetadata(String provider, HttpClient httpClient)
+    /**
+     * Obtain the JSON metadata from OpenID Connect Discovery Configuration Endpoint.
+     * @return a set of Claims about the OpenID Provider's configuration in JSON format.
+     * @throws IllegalStateException if metadata could not be fetched from the OP.
+     */
+    protected Map<String, Object> fetchOpenIdConnectMetadata()
     {
+        String provider = issuer;
+        if (provider.endsWith("/"))
+            provider = provider.substring(0, provider.length() - 1);
+
         try
         {
-            if (provider.endsWith("/"))
-                provider = provider.substring(0, provider.length() - 1);
-
             Map<String, Object> result;
             String responseBody = httpClient.GET(provider + CONFIG_PATH).getContentAsString();
             Object parsedResult = new JSON().fromJSON(responseBody);
@@ -153,8 +166,8 @@ public class OpenIdConfiguration extends ContainerLifeCycle
             {
                 Map<?, ?> rawResult = (Map<?, ?>)parsedResult;
                 result = rawResult.entrySet().stream()
-                        .filter(entry -> entry.getValue() != null)
+                    .filter(entry -> entry.getValue() != null)
-                        .collect(Collectors.toMap(it -> it.getKey().toString(), Map.Entry::getValue));
+                    .collect(Collectors.toMap(it -> it.getKey().toString(), Map.Entry::getValue));
                 if (LOG.isDebugEnabled())
                     LOG.debug("discovery document {}", result);
                 return result;
@@ -167,7 +180,7 @@ public class OpenIdConfiguration extends ContainerLifeCycle
         }
         catch (Exception e)
         {
-            throw new IllegalArgumentException("invalid identity provider " + provider, e);
+            throw new IllegalStateException("invalid identity provider " + provider, e);
         }
     }
 
@@ -227,6 +240,13 @@ public class OpenIdConfiguration extends ContainerLifeCycle
         this.authenticateNewUsers = authenticateNewUsers;
     }
 
+    private static HttpClient newHttpClient()
+    {
+        ClientConnector connector = new ClientConnector();
+        connector.setSslContextFactory(new SslContextFactory.Client(false));
+        return new HttpClient(new HttpClientTransportOverHTTP(connector));
+    }
+
     @Override
     public String toString()
     {

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/authentication/DeferredAuthentication.java

@@ -113,11 +113,8 @@ public class DeferredAuthentication implements Authentication.Deferred
         if (security != null)
         {
             security.logout(null);
-            if (_authenticator instanceof LoginAuthenticator)
+            _authenticator.logout(request);
-            {
+            return new LoggedOutAuthentication(_authenticator);
-                ((LoginAuthenticator)_authenticator).logout(request);
-                return new LoggedOutAuthentication((LoginAuthenticator)_authenticator);
-            }
         }
 
         return Authentication.UNAUTHENTICATED;

jetty-ee9/jetty-ee9-openid/src/main/java/org/eclipse/jetty/ee9/security/openid/OpenIdConfiguration.java

@@ -40,6 +40,9 @@ public class OpenIdConfiguration extends ContainerLifeCycle
 {
     private static final Logger LOG = LoggerFactory.getLogger(OpenIdConfiguration.class);
     private static final String CONFIG_PATH = "/.well-known/openid-configuration";
+    private static final String AUTHORIZATION_ENDPOINT = "authorization_endpoint";
+    private static final String TOKEN_ENDPOINT = "token_endpoint";
+    private static final String ISSUER = "issuer";
 
     private final HttpClient httpClient;
     private final String issuer;
@@ -116,35 +119,45 @@ public class OpenIdConfiguration extends ContainerLifeCycle
 
         if (authEndpoint == null || tokenEndpoint == null)
         {
-            Map<String, Object> discoveryDocument = fetchOpenIdConnectMetadata(issuer, httpClient);
+            Map<String, Object> discoveryDocument = fetchOpenIdConnectMetadata();
-
+            processMetadata(discoveryDocument);
-            authEndpoint = (String)discoveryDocument.get("authorization_endpoint");
-            if (authEndpoint == null)
-                throw new IllegalArgumentException("authorization_endpoint");
-
-            tokenEndpoint = (String)discoveryDocument.get("token_endpoint");
-            if (tokenEndpoint == null)
-                throw new IllegalArgumentException("token_endpoint");
-
-            if (!Objects.equals(discoveryDocument.get("issuer"), issuer))
-                LOG.warn("The issuer in the metadata is not correct.");
         }
     }
 
-    private static HttpClient newHttpClient()
+    /**
+     * Process the OpenID Connect metadata discovered by {@link #fetchOpenIdConnectMetadata()}.
+     * By default, only the {@link #AUTHORIZATION_ENDPOINT} and {@link #TOKEN_ENDPOINT} claims are extracted.
+     * @see <a href="https://openid.net/specs/openid-connect-discovery-1_0.html">OpenID Connect Discovery 1.0</a>
+     * @throws IllegalStateException if a required field is not present in the metadata.
+     */
+    protected void processMetadata(Map<String, Object> discoveryDocument)
     {
-        ClientConnector connector = new ClientConnector();
+        authEndpoint = (String)discoveryDocument.get(AUTHORIZATION_ENDPOINT);
-        connector.setSslContextFactory(new SslContextFactory.Client(false));
+        if (authEndpoint == null)
-        return new HttpClient(new HttpClientTransportOverHTTP(connector));
+            throw new IllegalStateException(AUTHORIZATION_ENDPOINT);
+
+        tokenEndpoint = (String)discoveryDocument.get(TOKEN_ENDPOINT);
+        if (tokenEndpoint == null)
+            throw new IllegalStateException(TOKEN_ENDPOINT);
+
+        // We are lenient and not throw here as some major OIDC providers do not conform to this.
+        if (!Objects.equals(discoveryDocument.get(ISSUER), issuer))
+            LOG.warn("The issuer in the metadata is not correct.");
     }
 
-    private static Map<String, Object> fetchOpenIdConnectMetadata(String provider, HttpClient httpClient)
+    /**
+     * Obtain the JSON metadata from OpenID Connect Discovery Configuration Endpoint.
+     * @return a set of Claims about the OpenID Provider's configuration in JSON format.
+     * @throws IllegalStateException if metadata could not be fetched from the OP.
+     */
+    protected Map<String, Object> fetchOpenIdConnectMetadata()
     {
+        String provider = issuer;
+        if (provider.endsWith("/"))
+            provider = provider.substring(0, provider.length() - 1);
+
         try
         {
-            if (provider.endsWith("/"))
-                provider = provider.substring(0, provider.length() - 1);
-
             Map<String, Object> result;
             String responseBody = httpClient.GET(provider + CONFIG_PATH).getContentAsString();
             Object parsedResult = new JSON().fromJSON(responseBody);
@@ -153,8 +166,8 @@ public class OpenIdConfiguration extends ContainerLifeCycle
             {
                 Map<?, ?> rawResult = (Map<?, ?>)parsedResult;
                 result = rawResult.entrySet().stream()
-                        .filter(entry -> entry.getValue() != null)
+                    .filter(entry -> entry.getValue() != null)
-                        .collect(Collectors.toMap(it -> it.getKey().toString(), Map.Entry::getValue));
+                    .collect(Collectors.toMap(it -> it.getKey().toString(), Map.Entry::getValue));
                 if (LOG.isDebugEnabled())
                     LOG.debug("discovery document {}", result);
                 return result;
@@ -167,7 +180,7 @@ public class OpenIdConfiguration extends ContainerLifeCycle
         }
         catch (Exception e)
         {
-            throw new IllegalArgumentException("invalid identity provider " + provider, e);
+            throw new IllegalStateException("invalid identity provider " + provider, e);
         }
     }
 
@@ -227,6 +240,13 @@ public class OpenIdConfiguration extends ContainerLifeCycle
         this.authenticateNewUsers = authenticateNewUsers;
     }
 
+    private static HttpClient newHttpClient()
+    {
+        ClientConnector connector = new ClientConnector();
+        connector.setSslContextFactory(new SslContextFactory.Client(false));
+        return new HttpClient(new HttpClientTransportOverHTTP(connector));
+    }
+
     @Override
     public String toString()
     {

jetty-ee9/jetty-ee9-security/src/main/java/org/eclipse/jetty/ee9/security/authentication/DeferredAuthentication.java

@@ -120,11 +120,8 @@ public class DeferredAuthentication implements Authentication.Deferred
         if (security != null)
         {
             security.logout(null);
-            if (_authenticator instanceof LoginAuthenticator)
+            _authenticator.logout(request);
-            {
+            return new LoggedOutAuthentication(_authenticator);
-                ((LoginAuthenticator)_authenticator).logout(request);
-                return new LoggedOutAuthentication((LoginAuthenticator)_authenticator);
-            }
         }
 
         return Authentication.UNAUTHENTICATED;