Merge pull request #8221 from eclipse/jetty-10.0.x-8216-openid-logout

Issue #8216 - make processing of metadata in OpenIdConfiguration extensible
jetty · Jul 4, 2022 · 475d051 · 475d051
2 parents 2b817f0 + ec14acb
commit 475d051
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 26 deletions.
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
@@ -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 @@ protected void doStart() throws Exception
 
         if (authEndpoint == null || tokenEndpoint == null)
         {
-            Map<String, Object> discoveryDocument = fetchOpenIdConnectMetadata(issuer, httpClient);
-
-            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.");
+            Map<String, Object> discoveryDocument = fetchOpenIdConnectMetadata();
+            processMetadata(discoveryDocument);
         }
     }
 
-    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();
-        connector.setSslContextFactory(new SslContextFactory.Client(false));
-        return new HttpClient(new HttpClientTransportOverHTTP(connector));
+        authEndpoint = (String)discoveryDocument.get(AUTHORIZATION_ENDPOINT);
+        if (authEndpoint == null)
+            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);
@@ -167,7 +180,7 @@ private static Map<String, Object> fetchOpenIdConnectMetadata(String provider, H
         }
         catch (Exception e)
         {
-            throw new IllegalArgumentException("invalid identity provider " + provider, e);
+            throw new IllegalStateException("invalid identity provider " + provider, e);
         }
     }
 
@@ -227,6 +240,13 @@ public void setAuthenticateNewUsers(boolean authenticateNewUsers)
         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()
     {

diff --git a/...urity/src/main/java/org/eclipse/jetty/security/authentication/DeferredAuthentication.java b/...urity/src/main/java/org/eclipse/jetty/security/authentication/DeferredAuthentication.java
@@ -120,11 +120,8 @@ public Authentication logout(ServletRequest request)
         if (security != null)
         {
             security.logout(null);
-            if (_authenticator instanceof LoginAuthenticator)
-            {
-                ((LoginAuthenticator)_authenticator).logout(request);
-                return new LoggedOutAuthentication((LoginAuthenticator)_authenticator);
-            }
+            _authenticator.logout(request);
+            return new LoggedOutAuthentication(_authenticator);
         }
 
         return Authentication.UNAUTHENTICATED;