fix claim validation

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2019-09-06 16:03:22 +10:00
parent f592e63711
commit 2770afb280
6 changed files with 51 additions and 36 deletions

View File

@ -61,8 +61,8 @@ public class OpenIdAuthenticator extends LoginAuthenticator
{ {
private static final Logger LOG = Log.getLogger(OpenIdAuthenticator.class); 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 __CLAIMS = "org.eclipse.jetty.security.openid.claims";
public static final String __RESPONSE_JSON = "org.eclipse.jetty.security.openid.response"; 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 __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_URI = "org.eclipse.jetty.security.openid.URI";
public static final String __J_POST = "org.eclipse.jetty.security.openid.POST"; 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(); HttpSession session = ((HttpServletRequest)request).getSession();
Authentication cached = new SessionAuthentication(getAuthMethod(), user, credentials); Authentication cached = new SessionAuthentication(getAuthMethod(), user, credentials);
session.setAttribute(SessionAuthentication.__J_AUTHENTICATED, cached); session.setAttribute(SessionAuthentication.__J_AUTHENTICATED, cached);
session.setAttribute(__USER_CLAIMS, ((OpenIdCredentials)credentials).getClaims()); session.setAttribute(__CLAIMS, ((OpenIdCredentials)credentials).getClaims());
session.setAttribute(__RESPONSE_JSON, ((OpenIdCredentials)credentials).getResponse()); session.setAttribute(__RESPONSE, ((OpenIdCredentials)credentials).getResponse());
} }
return user; return user;
} }
@ -180,8 +180,8 @@ public class OpenIdAuthenticator extends LoginAuthenticator
//clean up session //clean up session
session.removeAttribute(SessionAuthentication.__J_AUTHENTICATED); session.removeAttribute(SessionAuthentication.__J_AUTHENTICATED);
session.removeAttribute(__USER_CLAIMS); session.removeAttribute(__CLAIMS);
session.removeAttribute(__RESPONSE_JSON); session.removeAttribute(__RESPONSE);
} }
@Override @Override

View File

@ -26,12 +26,16 @@ import java.util.Map;
import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.ajax.JSON; import org.eclipse.jetty.util.ajax.JSON;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
public class OpenIdConfiguration public class OpenIdConfiguration
{ {
private static final Logger LOG = Log.getLogger(OpenIdConfiguration.class);
private static String CONFIG_PATH = "/.well-known/openid-configuration"; private static String CONFIG_PATH = "/.well-known/openid-configuration";
private final String openIdProvider; private final String openIdProvider;
private final String issuer;
private final String authEndpoint; private final String authEndpoint;
private final String tokenEndpoint; private final String tokenEndpoint;
private final String clientId; private final String clientId;
@ -55,13 +59,16 @@ public class OpenIdConfiguration
InputStream inputStream = providerUri.toURL().openConnection().getInputStream(); InputStream inputStream = providerUri.toURL().openConnection().getInputStream();
String content = IO.toString(inputStream); String content = IO.toString(inputStream);
discoveryDocument = (Map)JSON.parse(content); discoveryDocument = (Map)JSON.parse(content);
if (LOG.isDebugEnabled())
LOG.debug("discovery document {}", discoveryDocument);
} }
catch (Throwable e) catch (Throwable e)
{ {
throw new IllegalArgumentException("invalid identity provider", e); throw new IllegalArgumentException("invalid identity provider", e);
} }
if (discoveryDocument.get("issuer") == null) issuer = (String)discoveryDocument.get("issuer");
if (issuer == null)
throw new IllegalArgumentException(); throw new IllegalArgumentException();
authEndpoint = (String)discoveryDocument.get("authorization_endpoint"); authEndpoint = (String)discoveryDocument.get("authorization_endpoint");
@ -93,6 +100,11 @@ public class OpenIdConfiguration
return clientSecret; return clientSecret;
} }
public String getIssuer()
{
return issuer;
}
public String getOpenIdProvider() public String getOpenIdProvider()
{ {
return openIdProvider; return openIdProvider;

View File

@ -91,7 +91,8 @@ public class OpenIdCredentials
claims = decodeJWT(idToken); claims = decodeJWT(idToken);
if (LOG.isDebugEnabled()) if (LOG.isDebugEnabled())
LOG.debug("userClaims {}", claims); LOG.debug("claims {}", claims);
validateClaims();
} }
finally finally
{ {
@ -101,27 +102,35 @@ public class OpenIdCredentials
} }
} }
public boolean validate() private void validateClaims()
{ {
if (authCode != null) // Issuer Identifier for the OpenID Provider MUST exactly match the value of the iss (issuer) Claim.
return false; 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. // The aud (audience) Claim MUST contain the client_id value.
String audience = (String)claims.get("aud"); if (!configuration.getClientId().equals(claims.get("aud")))
if (!configuration.getClientId().equals(audience))
{ {
LOG.warn("Audience claim MUST contain the value of the Issuer Identifier for the OP"); LOG.warn("Audience Claim MUST contain the client_id value");
return false; throw new IllegalArgumentException("invalid aud claim");
} }
// TODO: this does not work for microsoft // If an azp (authorized party) Claim is present, verify that its client_id is the Claim Value.
// Issuer Identifier for the OpenID Provider MUST exactly match the value of the iss (issuer) Claim. Object azp = claims.get("azp");
String issuer = (String)claims.get("iss"); if (azp != null && !configuration.getClientId().equals(azp))
if (!configuration.getOpenIdProvider().equals(issuer))
{ {
//LOG.warn(""); LOG.warn("Authorized party claim value should be the client_id");
//return false; throw new IllegalArgumentException("invalid azp claim");
} }
}
public boolean isExpired()
{
if (authCode != null || claims == null)
return true;
// Check expiry // Check expiry
long expiry = (Long)claims.get("exp"); long expiry = (Long)claims.get("exp");
@ -130,10 +139,10 @@ public class OpenIdCredentials
{ {
if (LOG.isDebugEnabled()) if (LOG.isDebugEnabled())
LOG.debug("OpenId Credentials expired {}", this); LOG.debug("OpenId Credentials expired {}", this);
return false; return true;
} }
return true; return false;
} }
protected Map<String, Object> decodeJWT(String jwt) throws IOException protected Map<String, Object> decodeJWT(String jwt) throws IOException

View File

@ -18,7 +18,6 @@
package org.eclipse.jetty.security.openid; package org.eclipse.jetty.security.openid;
import java.io.IOException;
import java.security.Principal; import java.security.Principal;
import javax.security.auth.Subject; import javax.security.auth.Subject;
import javax.servlet.ServletRequest; import javax.servlet.ServletRequest;
@ -72,10 +71,10 @@ public class OpenIdLoginService extends ContainerLifeCycle implements LoginServi
try try
{ {
openIdCredentials.redeemAuthCode(); openIdCredentials.redeemAuthCode();
if (!openIdCredentials.validate()) if (openIdCredentials.isExpired())
return null; return null;
} }
catch (IOException e) catch (Throwable e)
{ {
LOG.warn(e); LOG.warn(e);
return null; return null;
@ -115,7 +114,7 @@ public class OpenIdLoginService extends ContainerLifeCycle implements LoginServi
return false; return false;
OpenIdCredentials credentials = ((OpenIdUserPrincipal)userPrincipal).getCredentials(); OpenIdCredentials credentials = ((OpenIdUserPrincipal)userPrincipal).getCredentials();
return credentials.validate(); return !credentials.isExpired();
} }
@Override @Override

View File

@ -29,11 +29,6 @@ public class OpenIdUserIdentity implements UserIdentity
private final Principal userPrincipal; private final Principal userPrincipal;
private final UserIdentity userIdentity; private final UserIdentity userIdentity;
public OpenIdUserIdentity(Subject subject, Principal userPrincipal)
{
this(subject, userPrincipal, null);
}
public OpenIdUserIdentity(Subject subject, Principal userPrincipal, UserIdentity userIdentity) public OpenIdUserIdentity(Subject subject, Principal userPrincipal, UserIdentity userIdentity)
{ {
this.subject = subject; this.subject = subject;

View File

@ -81,7 +81,7 @@ public class OpenIdAuthenticationDemo
Principal userPrincipal = request.getUserPrincipal(); Principal userPrincipal = request.getUserPrincipal();
if (userPrincipal != null) if (userPrincipal != null)
{ {
Map<String, Object> userInfo = (Map)request.getSession().getAttribute(OpenIdAuthenticator.__USER_CLAIMS); Map<String, Object> userInfo = (Map)request.getSession().getAttribute(OpenIdAuthenticator.__CLAIMS);
response.getWriter().println("<p>Welcome: " + userInfo.get("name") + "</p>"); response.getWriter().println("<p>Welcome: " + userInfo.get("name") + "</p>");
response.getWriter().println("<a href=\"/profile\">Profile</a><br>"); response.getWriter().println("<a href=\"/profile\">Profile</a><br>");
response.getWriter().println("<a href=\"/admin\">Admin</a><br>"); response.getWriter().println("<a href=\"/admin\">Admin</a><br>");
@ -100,7 +100,7 @@ public class OpenIdAuthenticationDemo
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException
{ {
response.setContentType(MimeTypes.Type.TEXT_HTML.asString()); response.setContentType(MimeTypes.Type.TEXT_HTML.asString());
Map<String, Object> userInfo = (Map)request.getSession().getAttribute(OpenIdAuthenticator.__USER_CLAIMS); Map<String, Object> userInfo = (Map)request.getSession().getAttribute(OpenIdAuthenticator.__CLAIMS);
response.getWriter().println("<!-- Add icon library -->\n" + response.getWriter().println("<!-- Add icon library -->\n" +
"<div class=\"card\">\n" + "<div class=\"card\">\n" +
@ -180,7 +180,7 @@ public class OpenIdAuthenticationDemo
/* /*
// Microsoft Authentication // Microsoft Authentication
OpenIdConfiguration configuration = new OpenIdConfiguration( 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", "5f05dea8-2bd9-45de-b30f-cf5c102b8784",
"IfhQJKi-5[vxhh_=ldqt0y4PkV3z_1ca"); "IfhQJKi-5[vxhh_=ldqt0y4PkV3z_1ca");
*/ */
@ -211,7 +211,7 @@ public class OpenIdAuthenticationDemo
hashLoginService.setHotReload(true); hashLoginService.setHotReload(true);
// Configure OpenIdLoginService optionally providing a base LoginService to provide user roles // 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); securityHandler.setLoginService(loginService);
Authenticator authenticator = new OpenIdAuthenticator(configuration, "/error"); Authenticator authenticator = new OpenIdAuthenticator(configuration, "/error");