Updated validator structure and id token checking.

openid-connect-client/src/main/java/org/mitre/openid/connect/client/AbstractOIDCAuthenticationFilter.java

@@ -19,33 +19,25 @@ import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.math.BigInteger;
 import java.net.URLEncoder;
-import java.security.GeneralSecurityException;
-import java.security.KeyPair;
-import java.security.KeyPairGenerator;
-import java.security.PrivateKey;
 import java.security.PublicKey;
 import java.security.SecureRandom;
-import java.security.Signature;
 import java.security.interfaces.RSAPublicKey;
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
+import java.util.Date;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
 import javax.servlet.ServletException;
-import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletRequestWrapper;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
-import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.lang.StringUtils;
 import org.apache.http.client.HttpClient;
 import org.apache.http.impl.client.DefaultHttpClient;
+import org.mitre.jwt.model.JwtClaims;
 import org.mitre.jwt.signer.JwsAlgorithm;
 import org.mitre.jwt.signer.JwtSigner;
 import org.mitre.jwt.signer.impl.RsaSigner;
@@ -64,7 +56,6 @@ import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
 import org.springframework.web.client.HttpClientErrorException;
 import org.springframework.web.client.RestTemplate;
-import org.springframework.web.util.WebUtils;
 
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
@@ -212,9 +203,7 @@ public class AbstractOIDCAuthenticationFilter extends
 	 * javax.servlet.http.HttpServletResponse)
 	 */
 	@Override
-	public Authentication attemptAuthentication(HttpServletRequest request,
+	public Authentication attemptAuthentication(HttpServletRequest request, HttpServletResponse response) throws AuthenticationException, IOException, ServletException {
-			HttpServletResponse response) throws AuthenticationException,
-			IOException, ServletException {
 
 		logger.debug("Request: "
 				+ request.getRequestURI()
@@ -237,9 +226,7 @@ public class AbstractOIDCAuthenticationFilter extends
 	 * @throws Exception 
 	 * @throws UnsupportedEncodingException
 	 */
-	protected Authentication handleAuthorizationGrantResponse(
+	protected Authentication handleAuthorizationGrantResponse(String authorizationCode, HttpServletRequest request, OIDCServerConfiguration serverConfig) {
-			String authorizationCode, HttpServletRequest request,
-			OIDCServerConfiguration serverConfig) {
 
 		final boolean debug = logger.isDebugEnabled();
 
@@ -356,58 +343,59 @@ public class AbstractOIDCAuthenticationFilter extends
 				refreshTokenValue = tokenResponse.get("refresh_token").getAsString();
 			}
 			
-			JwtSigningAndValidationService jwtValidator = getValidatorForServer(serverConfig); 
+			IdToken idToken = IdToken.parse(idTokenValue); // TODO: catch parsing errors?
-			IdToken idToken = null;
-			
-			try {
-				idToken = IdToken.parse(idTokenValue);
-			
-			} catch (AuthenticationServiceException e) {
-
-				// I suspect this could happen
-
-				logger.error("Problem parsing id_token:  " + e);
-				// e.printStackTrace();
-
-				throw new AuthenticationServiceException("Problem parsing id_token return from Token endpoint: " + e);
-			}
 
 			// validate our ID Token over a number of tests
+			JwtClaims idClaims = idToken.getClaims();
 			
-			if(!jwtValidator.validateSignature(idToken.toString())) {
+			// check the signature
-				throw new AuthenticationServiceException("Signature not validated");
+			JwtSigningAndValidationService jwtValidator = getValidatorForServer(serverConfig); 
+			if (jwtValidator != null) {
+				if(!jwtValidator.validateSignature(idToken.toString())) {
+					throw new AuthenticationServiceException("Signature validation failed");
+				}
+			} else {
+				logger.info("No validation service found. Skipping signature validation");
 			}
 			
-			if(idToken.getClaims().getIssuer() == null) {
+			// check the issuer
-				throw new AuthenticationServiceException("Issuer is null");
+			if (idClaims.getIssuer() == null) {
+				throw new AuthenticationServiceException("Id Token Issuer is null");
+			} else if (!idClaims.getIssuer().equals(serverConfig.getIssuer())){
+				throw new AuthenticationServiceException("Issuers do not match, expected " + serverConfig.getIssuer() + " got " + idClaims.getIssuer());
 			}
 			
-			if(!idToken.getClaims().getIssuer().equals(serverConfig.getIssuer())){
+			// check expiration
-				throw new AuthenticationServiceException("Issuers do not match");
+			if (idClaims.getExpiration() == null) {
+				throw new AuthenticationServiceException("Id Token does not have required expiration claim");
+			} else {
+				// it's not null, see if it's expired
+				Date now = new Date();
+				if (!now.after(idClaims.getExpiration())) {
+					throw new AuthenticationServiceException("Id Token is expired: " + idClaims.getExpiration());
+				}
 			}
 			
-			if(jwtValidator.isJwtExpired(idToken)) {
+			// check audience
-				throw new AuthenticationServiceException("Id Token is expired");
+			if (idClaims.getAudience() == null) {
+				throw new AuthenticationServiceException("Id token audience is null");
+			} else if (!idClaims.getAudience().equals(serverConfig.getClientId())) {
+				throw new AuthenticationServiceException("Audience does not match, expected " + serverConfig.getClientId() + " got " + idClaims.getAudience());
 			}
 			
-			if(!jwtValidator.validateIssuedAt(idToken)) {
+			// check issued at
-				throw new AuthenticationServiceException("Id Token issuedAt failed");
+			if (idClaims.getIssuedAt() == null) {
+				throw new AuthenticationServiceException("Id Token does not have required issued-at claim");				
+			} else {
+				// since it's not null, see if it was issued in the future
+				Date now = new Date();
+				if (now.before(idClaims.getIssuedAt())) {
+					throw new AuthenticationServiceException("Id Token was issued in the future: " + idClaims.getIssuedAt());
+				}
 			}
 
-			// Clients are required to compare nonce claim in ID token to 
+			// compare the nonce to our stored claim
-			// the nonce sent in the Authorization request.  The client 
+			String nonce = idClaims.getNonce();			
-			// stores this value as a signed session cookie to detect a 
-			// replay by third parties.
-			//
-			// See: OpenID Connect Messages Section 2.1.1 entitled "ID Token"
-			//
-			// http://openid.net/specs/openid-connect-messages-1_0.html#id_token
-			//
-			
-			//String nonce = idToken.getClaims().getClaimAsString("nonce");
-
-			String nonce = idToken.getClaims().getNonce();
-			
 			if (StringUtils.isBlank(nonce)) {
 				
 				logger.error("ID token did not contain a nonce claim.");
@@ -429,10 +417,9 @@ public class AbstractOIDCAuthenticationFilter extends
 			
 			String userId = idToken.getTokenClaims().getUserId();
 			
-			// construct an OpenIdConnectAuthenticationToken and return 
+			// construct an OpenIdConnectAuthenticationToken and return a Authentication object w/the userId and the idToken
-			// a Authentication object w/the userId and the idToken
 			
-			OpenIdConnectAuthenticationToken token = new OpenIdConnectAuthenticationToken(userId, idToken.getClaims().getIssuer(), serverConfig, idTokenValue, accessTokenValue, refreshTokenValue);
+			OpenIdConnectAuthenticationToken token = new OpenIdConnectAuthenticationToken(userId, idClaims.getIssuer(), serverConfig, idTokenValue, accessTokenValue, refreshTokenValue);
 
 			Authentication authentication = this.getAuthenticationManager().authenticate(token);
 
@@ -615,13 +602,15 @@ public class AbstractOIDCAuthenticationFilter extends
 			KeyFetcher keyFetch = new KeyFetcher();
 			PublicKey signingKey = null;
 			
-			// If the config has the X509 url, prefer that to the JWK
+			if (serverConfig.getJwkSigningUrl() != null) {
-			if(serverConfig.getX509SigningUrl() != null) {
+				// prefer the JWK
-				signingKey = keyFetch.retrieveX509Key(serverConfig);
-				
-			} else {
-				// otherwise prefer the JWK
 				signingKey = keyFetch.retrieveJwkKey(serverConfig);
+			} else if (serverConfig.getX509SigningUrl() != null) {
+				// use the x509 only if JWK isn't configured
+				signingKey = keyFetch.retrieveX509Key(serverConfig);				
+			} else {
+				// no keys configured
+				logger.warn("No server key URLs configured for " + serverConfig.getIssuer());
 			}
 			
 			if (signingKey != null) {
@@ -648,6 +637,7 @@ public class AbstractOIDCAuthenticationFilter extends
 				return signingAndValidationService;
 				
 			} else {
+				// there were either no keys returned or no URLs configured to fetch them, assume no checking on key signatures
 				return null;
 			}
 		}

openid-connect-common/src/main/java/org/mitre/jwt/signer/service/JwtSigningAndValidationService.java

@@ -30,17 +30,6 @@ public interface JwtSigningAndValidationService {
 	 */
 	public Map<String, JwtSigner> getAllSigners();
 
-	/**
-	 * Check to see if this JWT has expired or not
-	 * 
-	 * @param jwt
-	 *            the JWT to check
-	 * @return true if this JWT has an expiration and it has passed, false if
-	 *         the JWT has no expiration or it has an expiration and the
-	 *         expiration has not passed
-	 */
-	public boolean isJwtExpired(Jwt jwt);
-
 	/**
 	 * Checks the signature of the given JWT against all configured signers,
 	 * returns true if at least one of the signers validates it.
@@ -53,36 +42,6 @@ public interface JwtSigningAndValidationService {
 	public boolean validateSignature(String jwtString);
 	
 
-	/**
-	 * Checks to see when this JWT was issued
-	 * 
-	 * @param jwt
-	 * 		the JWT to check
-	 * @return true if the issued at is valid, false if not
-	 * @throws NoSuchAlgorithmException
-	 */
-	public boolean validateIssuedAt(Jwt jwt);
-	
-	/**
-	 * Checks to see if the nonce parameter sent in the Authorization Request 
-	 * is equal to the nonce parameter in the id token
-	 * 
-	 * @param jwt
-	 * @param nonce
-	 * 			the string representation of the Nonce
-	 * @return true if both nonce parameters are equal, false if otherwise 
-	 * @throws NoSuchAlgorithmException
-	 */
-	public boolean validateNonce(Jwt jwt, String nonce);
-	
-	/**
-	 * Sign a jwt using the selected algorithm. The algorithm is selected using the String parameter values specified
-	 * in the JWT spec, section 6. I.E., "HS256" means HMAC with SHA-256 and corresponds to our HmacSigner class.
-	 * 
-	 * @param jwt the jwt to sign
-	 * @param alg the name of the algorithm to use, as specified in JWS s.6
-	 * @return the signed jwt
-	 */
 	
 	/**
 	 * Called to sign a jwt in place for a client that hasn't registered a preferred signing algorithm.
@@ -94,6 +53,14 @@ public interface JwtSigningAndValidationService {
 	 */
 	public void signJwt(Jwt jwt) throws NoSuchAlgorithmException;
 	
+	/**
+	 * Sign a jwt using the selected algorithm. The algorithm is selected using the String parameter values specified
+	 * in the JWT spec, section 6. I.E., "HS256" means HMAC with SHA-256 and corresponds to our HmacSigner class.
+	 * 
+	 * @param jwt the jwt to sign
+	 * @param alg the name of the algorithm to use, as specified in JWS s.6
+	 * @return the signed jwt
+	 */
 	//TODO: implement later; only need signJwt(Jwt jwt) for now
 	//public Jwt signJwt(Jwt jwt, String alg);
 	

openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/AbstractJwtSigningAndValidationService.java

@@ -1,69 +0,0 @@
-package org.mitre.jwt.signer.service.impl;
-
-import java.security.NoSuchAlgorithmException;
-import java.util.Date;
-import java.util.Map;
-
-import org.mitre.jwt.model.Jwt;
-import org.mitre.jwt.signer.JwtSigner;
-import org.mitre.jwt.signer.service.JwtSigningAndValidationService;
-
-public abstract class AbstractJwtSigningAndValidationService implements JwtSigningAndValidationService{
-
-	/**
-	 * Return the JwtSigners associated with this service
-	 * 
-	 * @return
-	 */
-	public abstract Map<String, ? extends JwtSigner> getSigners(); 
-
-	@Override
-	public boolean isJwtExpired(Jwt jwt) {
-	
-		Date expiration = jwt.getClaims().getExpiration();
-	
-		if (expiration != null) {
-			return new Date().after(expiration);
-		} else {
-			return false;
-		}
-	}
-
-	@Override
-	public boolean validateSignature(String jwtString) {
-	
-		for (JwtSigner signer : getSigners().values()) {
-			try {
-	            if (signer.verify(jwtString)) {
-	            	return true;
-	            }
-            } catch (NoSuchAlgorithmException e) {
-            	// ignore, signer didn't verify signature, try the next one
-            	e.printStackTrace();
-            }
-		}
-		return false;
-	}
-	
-	@Override
-	public boolean validateIssuedAt(Jwt jwt) {
-		Date issuedAt = jwt.getClaims().getIssuedAt();
-		
-		if (issuedAt != null) {
-			// make sure the token was issued in the past
-			return new Date().after(issuedAt);
-		} else {
-			return false;
-		}
-	}
-
-	@Override
-	public boolean validateNonce(Jwt jwt, String nonce) {
-		if(jwt.getClaims().getNonce().equals(nonce)){
-			return true;
-		} else {
-			return false;
-		}
-	}
-
-}

openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/JwtSigningAndValidationServiceDefault.java

@@ -28,8 +28,7 @@ import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 
-public class JwtSigningAndValidationServiceDefault extends AbstractJwtSigningAndValidationService implements
+public class JwtSigningAndValidationServiceDefault implements JwtSigningAndValidationService, InitializingBean {
-		JwtSigningAndValidationService, InitializingBean {
 
 	@Autowired 
 	private ConfigurationPropertiesBean configBean;
@@ -51,8 +50,7 @@ public class JwtSigningAndValidationServiceDefault extends AbstractJwtSigningAnd
 	 * @param signer
 	 *            List of JwtSigners to associate with this service
 	 */
-	public JwtSigningAndValidationServiceDefault(
+	public JwtSigningAndValidationServiceDefault(Map<String, ? extends JwtSigner> signer) {
-			Map<String, ? extends JwtSigner> signer) {
 		setSigners(signer);
 	}
 	
@@ -106,8 +104,7 @@ public class JwtSigningAndValidationServiceDefault extends AbstractJwtSigningAnd
 	 */
 	@Override
 	public String toString() {
-		return "JwtSigningAndValidationServiceDefault [signers=" + signers
+		return "JwtSigningAndValidationServiceDefault [signers=" + signers + "]";
-				+ "]";
 	}
 
 	/**
@@ -150,4 +147,20 @@ public class JwtSigningAndValidationServiceDefault extends AbstractJwtSigningAnd
 		return signers;
 	}
 
+	@Override
+	public boolean validateSignature(String jwtString) {
+
+		for (JwtSigner signer : getSigners().values()) {
+			try {
+				if (signer.verify(jwtString)) {
+					return true;
+				}
+			} catch (NoSuchAlgorithmException e) {
+				// ignore, signer didn't verify signature, try the next one
+				e.printStackTrace();
+			}
+		}
+		return false;
+	}
+
 }