diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java index 1608d7a5acb..14caa20af5b 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java @@ -61,8 +61,8 @@ public class OpenIdAuthenticator extends LoginAuthenticator { private static final Logger LOG = Log.getLogger(OpenIdAuthenticator.class); - public static final String __USER_CLAIMS = "org.eclipse.jetty.security.openid.user_claims"; - public static final String __RESPONSE_JSON = "org.eclipse.jetty.security.openid.response"; + public static final String __CLAIMS = "org.eclipse.jetty.security.openid.claims"; + public static final String __RESPONSE = "org.eclipse.jetty.security.openid.response"; public static final String __ERROR_PAGE = "org.eclipse.jetty.security.openid.error_page"; public static final String __J_URI = "org.eclipse.jetty.security.openid.URI"; public static final String __J_POST = "org.eclipse.jetty.security.openid.POST"; @@ -162,8 +162,8 @@ public class OpenIdAuthenticator extends LoginAuthenticator HttpSession session = ((HttpServletRequest)request).getSession(); Authentication cached = new SessionAuthentication(getAuthMethod(), user, credentials); session.setAttribute(SessionAuthentication.__J_AUTHENTICATED, cached); - session.setAttribute(__USER_CLAIMS, ((OpenIdCredentials)credentials).getClaims()); - session.setAttribute(__RESPONSE_JSON, ((OpenIdCredentials)credentials).getResponse()); + session.setAttribute(__CLAIMS, ((OpenIdCredentials)credentials).getClaims()); + session.setAttribute(__RESPONSE, ((OpenIdCredentials)credentials).getResponse()); } return user; } @@ -180,8 +180,8 @@ public class OpenIdAuthenticator extends LoginAuthenticator //clean up session session.removeAttribute(SessionAuthentication.__J_AUTHENTICATED); - session.removeAttribute(__USER_CLAIMS); - session.removeAttribute(__RESPONSE_JSON); + session.removeAttribute(__CLAIMS); + session.removeAttribute(__RESPONSE); } @Override 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 fe75553da1e..8284bb286ee 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 @@ -26,12 +26,16 @@ import java.util.Map; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.ajax.JSON; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; public class OpenIdConfiguration { + private static final Logger LOG = Log.getLogger(OpenIdConfiguration.class); private static String CONFIG_PATH = "/.well-known/openid-configuration"; private final String openIdProvider; + private final String issuer; private final String authEndpoint; private final String tokenEndpoint; private final String clientId; @@ -55,13 +59,16 @@ public class OpenIdConfiguration InputStream inputStream = providerUri.toURL().openConnection().getInputStream(); String content = IO.toString(inputStream); discoveryDocument = (Map)JSON.parse(content); + if (LOG.isDebugEnabled()) + LOG.debug("discovery document {}", discoveryDocument); } catch (Throwable e) { throw new IllegalArgumentException("invalid identity provider", e); } - if (discoveryDocument.get("issuer") == null) + issuer = (String)discoveryDocument.get("issuer"); + if (issuer == null) throw new IllegalArgumentException(); authEndpoint = (String)discoveryDocument.get("authorization_endpoint"); @@ -93,6 +100,11 @@ public class OpenIdConfiguration return clientSecret; } + public String getIssuer() + { + return issuer; + } + public String getOpenIdProvider() { return openIdProvider; 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 3d116d8c65f..7b787482ea3 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 @@ -91,7 +91,8 @@ public class OpenIdCredentials claims = decodeJWT(idToken); if (LOG.isDebugEnabled()) - LOG.debug("userClaims {}", claims); + LOG.debug("claims {}", claims); + validateClaims(); } finally { @@ -101,27 +102,35 @@ public class OpenIdCredentials } } - public boolean validate() + private void validateClaims() { - if (authCode != null) - return false; + // Issuer Identifier for the OpenID Provider MUST exactly match the value of the iss (issuer) Claim. + if (!configuration.getIssuer().equals(claims.get("iss"))) + { + LOG.warn("Invalid \"iss\" Claim: was {}, expected {}", claims.get("iss"), configuration.getIssuer()); + throw new IllegalArgumentException("invalid iss claim"); + } // The aud (audience) Claim MUST contain the client_id value. - String audience = (String)claims.get("aud"); - if (!configuration.getClientId().equals(audience)) + if (!configuration.getClientId().equals(claims.get("aud"))) { - LOG.warn("Audience claim MUST contain the value of the Issuer Identifier for the OP"); - return false; + LOG.warn("Audience Claim MUST contain the client_id value"); + throw new IllegalArgumentException("invalid aud claim"); } - // TODO: this does not work for microsoft - // Issuer Identifier for the OpenID Provider MUST exactly match the value of the iss (issuer) Claim. - String issuer = (String)claims.get("iss"); - if (!configuration.getOpenIdProvider().equals(issuer)) + // If an azp (authorized party) Claim is present, verify that its client_id is the Claim Value. + Object azp = claims.get("azp"); + if (azp != null && !configuration.getClientId().equals(azp)) { - //LOG.warn(""); - //return false; + LOG.warn("Authorized party claim value should be the client_id"); + throw new IllegalArgumentException("invalid azp claim"); } + } + + public boolean isExpired() + { + if (authCode != null || claims == null) + return true; // Check expiry long expiry = (Long)claims.get("exp"); @@ -130,10 +139,10 @@ public class OpenIdCredentials { if (LOG.isDebugEnabled()) LOG.debug("OpenId Credentials expired {}", this); - return false; + return true; } - return true; + return false; } protected Map decodeJWT(String jwt) throws IOException 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 4bfc75ac236..0b80fe8d735 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 @@ -18,7 +18,6 @@ package org.eclipse.jetty.security.openid; -import java.io.IOException; import java.security.Principal; import javax.security.auth.Subject; import javax.servlet.ServletRequest; @@ -72,10 +71,10 @@ public class OpenIdLoginService extends ContainerLifeCycle implements LoginServi try { openIdCredentials.redeemAuthCode(); - if (!openIdCredentials.validate()) + if (openIdCredentials.isExpired()) return null; } - catch (IOException e) + catch (Throwable e) { LOG.warn(e); return null; @@ -115,7 +114,7 @@ public class OpenIdLoginService extends ContainerLifeCycle implements LoginServi return false; OpenIdCredentials credentials = ((OpenIdUserPrincipal)userPrincipal).getCredentials(); - return credentials.validate(); + return !credentials.isExpired(); } @Override diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdUserIdentity.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdUserIdentity.java index 1477484c5e1..3b943c7a6e6 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdUserIdentity.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdUserIdentity.java @@ -29,11 +29,6 @@ public class OpenIdUserIdentity implements UserIdentity private final Principal userPrincipal; private final UserIdentity userIdentity; - public OpenIdUserIdentity(Subject subject, Principal userPrincipal) - { - this(subject, userPrincipal, null); - } - public OpenIdUserIdentity(Subject subject, Principal userPrincipal, UserIdentity userIdentity) { this.subject = subject; diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationDemo.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationDemo.java index d3e5168dc25..2d6ec2a0dde 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationDemo.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationDemo.java @@ -81,7 +81,7 @@ public class OpenIdAuthenticationDemo Principal userPrincipal = request.getUserPrincipal(); if (userPrincipal != null) { - Map userInfo = (Map)request.getSession().getAttribute(OpenIdAuthenticator.__USER_CLAIMS); + Map userInfo = (Map)request.getSession().getAttribute(OpenIdAuthenticator.__CLAIMS); response.getWriter().println("

Welcome: " + userInfo.get("name") + "

"); response.getWriter().println("Profile
"); response.getWriter().println("Admin
"); @@ -100,7 +100,7 @@ public class OpenIdAuthenticationDemo protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { response.setContentType(MimeTypes.Type.TEXT_HTML.asString()); - Map userInfo = (Map)request.getSession().getAttribute(OpenIdAuthenticator.__USER_CLAIMS); + Map userInfo = (Map)request.getSession().getAttribute(OpenIdAuthenticator.__CLAIMS); response.getWriter().println("\n" + "
\n" + @@ -180,7 +180,7 @@ public class OpenIdAuthenticationDemo /* // Microsoft Authentication OpenIdConfiguration configuration = new OpenIdConfiguration( - "https://login.microsoftonline.com/common/v2.0", + "https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0", "5f05dea8-2bd9-45de-b30f-cf5c102b8784", "IfhQJKi-5[vxhh_=ldqt0y4PkV3z_1ca"); */ @@ -211,7 +211,7 @@ public class OpenIdAuthenticationDemo hashLoginService.setHotReload(true); // Configure OpenIdLoginService optionally providing a base LoginService to provide user roles - OpenIdLoginService loginService = new OpenIdLoginService(configuration, hashLoginService); + OpenIdLoginService loginService = new OpenIdLoginService(configuration);//, hashLoginService); securityHandler.setLoginService(loginService); Authenticator authenticator = new OpenIdAuthenticator(configuration, "/error");