From 4e7fa4b943224a7e05b9648d541b5ac73cb0621b Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 4 Jul 2022 17:53:46 +1000 Subject: [PATCH] add changes from merge Signed-off-by: Lachlan Roberts --- .../security/openid/OpenIdConfiguration.java | 66 ++++++++++++------- .../DeferredAuthentication.java | 7 +- .../security/openid/OpenIdConfiguration.java | 66 ++++++++++++------- .../DeferredAuthentication.java | 7 +- 4 files changed, 90 insertions(+), 56 deletions(-) diff --git a/jetty-ee10/jetty-ee10-openid/src/main/java/org/eclipse/jetty/ee10/security/openid/OpenIdConfiguration.java b/jetty-ee10/jetty-ee10-openid/src/main/java/org/eclipse/jetty/ee10/security/openid/OpenIdConfiguration.java index db46c9c80f3..1b0ff83caeb 100644 --- a/jetty-ee10/jetty-ee10-openid/src/main/java/org/eclipse/jetty/ee10/security/openid/OpenIdConfiguration.java +++ b/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 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 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 OpenID Connect Discovery 1.0 + * @throws IllegalStateException if a required field is not present in the metadata. + */ + protected void processMetadata(Map 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 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 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 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) - .collect(Collectors.toMap(it -> it.getKey().toString(), Map.Entry::getValue)); + .filter(entry -> entry.getValue() != null) + .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() { diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/authentication/DeferredAuthentication.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/authentication/DeferredAuthentication.java index 2601f824626..8da27bf31b1 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/authentication/DeferredAuthentication.java +++ b/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) - { - ((LoginAuthenticator)_authenticator).logout(request); - return new LoggedOutAuthentication((LoginAuthenticator)_authenticator); - } + _authenticator.logout(request); + return new LoggedOutAuthentication(_authenticator); } return Authentication.UNAUTHENTICATED; diff --git a/jetty-ee9/jetty-ee9-openid/src/main/java/org/eclipse/jetty/ee9/security/openid/OpenIdConfiguration.java b/jetty-ee9/jetty-ee9-openid/src/main/java/org/eclipse/jetty/ee9/security/openid/OpenIdConfiguration.java index 992f5ab6651..b5d55e05ea8 100644 --- a/jetty-ee9/jetty-ee9-openid/src/main/java/org/eclipse/jetty/ee9/security/openid/OpenIdConfiguration.java +++ b/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 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 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 OpenID Connect Discovery 1.0 + * @throws IllegalStateException if a required field is not present in the metadata. + */ + protected void processMetadata(Map 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 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 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 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) - .collect(Collectors.toMap(it -> it.getKey().toString(), Map.Entry::getValue)); + .filter(entry -> entry.getValue() != null) + .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() { diff --git a/jetty-ee9/jetty-ee9-security/src/main/java/org/eclipse/jetty/ee9/security/authentication/DeferredAuthentication.java b/jetty-ee9/jetty-ee9-security/src/main/java/org/eclipse/jetty/ee9/security/authentication/DeferredAuthentication.java index 58bbd267e97..a8f1e5d65da 100644 --- a/jetty-ee9/jetty-ee9-security/src/main/java/org/eclipse/jetty/ee9/security/authentication/DeferredAuthentication.java +++ b/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) - { - ((LoginAuthenticator)_authenticator).logout(request); - return new LoggedOutAuthentication((LoginAuthenticator)_authenticator); - } + _authenticator.logout(request); + return new LoggedOutAuthentication(_authenticator); } return Authentication.UNAUTHENTICATED;