Further cleanups. Still missing:

- All tests extend TestCase, should use annotations instead
- Several elements throw Exception
- Key Fetchers should use RESTTemplates and be in a separate utility set

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

@@ -388,7 +388,7 @@ public class AbstractOIDCAuthenticationFilter extends
 	 */
 	protected Authentication handleAuthorizationGrantResponse(
 			String authorizationGrant, HttpServletRequest request,
-			OIDCServerConfiguration serverConfig) throws Exception {
+			OIDCServerConfiguration serverConfig) {
 
 		final boolean debug = logger.isDebugEnabled();
 

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

@@ -48,6 +48,8 @@ public class OIDCServerConfiguration {
 	
 	private String jwkSigningUrl;
 	
+	
+	// TODO: these keys should be settable through other means beyond discovery
 	private Key encryptKey;
 	
 	private Key signingKey;
@@ -124,6 +126,7 @@ public class OIDCServerConfiguration {
 		this.jwkSigningUrl = jwkSigningUrl;
 	}
 	
+	// FIXME: this should not throw Exception
 	public Key getSigningKey() throws Exception {
 		if(signingKey == null){
 			if(x509SigningUrl != null){
@@ -140,6 +143,7 @@ public class OIDCServerConfiguration {
 		return signingKey;
 	}
 	
+	// FIXME: this should not throw Exception
 	public Key getEncryptionKey() throws Exception {
 		if(encryptKey == null){
 			if(x509EncryptUrl != null){
@@ -156,6 +160,7 @@ public class OIDCServerConfiguration {
 		return encryptKey;
 	}
 	
+	// FIXME: this should not throw exception
 	public void checkKeys() throws Exception {
 		encryptKey = null;
 		signingKey = null;
@@ -176,6 +181,7 @@ public class OIDCServerConfiguration {
 				+ jwkSigningUrl + "]";
 	}
 	
+	// TODO: what is this function for? nobody uses it, and it seems backwards for construction
 	public DynamicJwtSigningAndValidationService getDynamic() throws Exception{
 		dynamic = new DynamicJwtSigningAndValidationService(getX509SigningUrl(), getJwkSigningUrl(), getClientSecret());
 		return dynamic;

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

@@ -11,6 +11,10 @@ import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.security.core.userdetails.UserDetailsService;
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
 
+
+// TODO: what is this class for?
+
+
 public class OIDCUserDetailService implements UserDetailsService,
 		AuthenticationUserDetailsService<OpenIdConnectAuthenticationToken> {
 	

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

@@ -4,6 +4,8 @@ import org.springframework.validation.Errors;
 import org.springframework.validation.ValidationUtils;
 import org.springframework.validation.Validator;
 
+// TODO: is this used anywhere?
+
 public class UrlValidator implements Validator{
 	
 
@@ -19,14 +21,17 @@ public class UrlValidator implements Validator{
 		
 	}
 	
+	// TODO this isn't called anywhere
 	public void validate1(Object obj, Errors e) {
 		ValidationUtils.rejectIfEmpty(e, "x509SigningUrl", "x509SigningUrl.empty");
 	}
 	
+	// TODO this isn't called anywhere
 	public void validate2(Object obj, Errors e) {
 		ValidationUtils.rejectIfEmpty(e, "jwkEncryptUrl", "jwkEncryptUrl.empty");
 	}
 	
+	// TODO this isn't called anywhere
 	public void validate3(Object obj, Errors e) {
 		ValidationUtils.rejectIfEmpty(e, "jwkSigningUrl", "jwkSigningUrl.empty");
 	}

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

@@ -173,6 +173,7 @@ public class HmacSigner extends AbstractJwtSigner implements InitializingBean {
 		this.passphrase = passphrase;
 	}
 	
+	// TODO: this this indirection to a lazy constructor necessary?
 	private Mac getMac() throws NoSuchAlgorithmException {
 		if(mac == null){
 			mac = Mac.getInstance(JwsAlgorithm.getByName(super.getAlgorithm())

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

@@ -138,7 +138,7 @@ public class RsaSigner extends AbstractJwtSigner implements InitializingBean {
 	 * org.springframework.beans.factory.InitializingBean#afterPropertiesSet()
 	 */
 	@Override
-	public void afterPropertiesSet() throws Exception {
+	public void afterPropertiesSet() throws NoSuchAlgorithmException, GeneralSecurityException {
 
 		// unsupported algorithm will throw a NoSuchAlgorithmException
 		signer = Signature.getInstance(JwsAlgorithm.getByName(super.getAlgorithm()).getStandardName()); // ,PROVIDER);
@@ -230,7 +230,8 @@ public class RsaSigner extends AbstractJwtSigner implements InitializingBean {
 	public void setPrivateKey(RSAPrivateKey privateKey) {
 		this.privateKey = privateKey;
 	}
-	
+
+	// TODO: this this indirection to a lazy constructor really necessary?
 	private Signature getSigner() throws NoSuchAlgorithmException{
 		if(signer == null){
 			signer = Signature.getInstance(JwsAlgorithm.getByName(super.getAlgorithm()).getStandardName());

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

@@ -71,8 +71,6 @@ public interface JwtSigningAndValidationService {
 	 * @return the signed jwt
 	 * @throws NoSuchAlgorithmException 
 	 */
-	public boolean validateIssuedAt(Jwt jwt);
-	
 	/**
 	 * Checks to see when this JWT was issued
 	 * 
@@ -81,7 +79,7 @@ public interface JwtSigningAndValidationService {
 	 * @return true if the issued at is valid, false if not
 	 * @throws NoSuchAlgorithmException
 	 */
-	public boolean validateAudience(Jwt jwt, String clientId);
+	public boolean validateIssuedAt(Jwt jwt);
 	
 	/**
 	 * Checks the audience that the given JWT against the client_id of the Client
@@ -92,7 +90,7 @@ public interface JwtSigningAndValidationService {
 	 * @return true if the audience matches the clinet_id, false if otherwise
 	 * @throws NoSuchAlgorithmException
 	 */
-	public boolean validateNonce(Jwt jwt, String nonce);
+	public boolean validateAudience(Jwt jwt, String clientId);
 	
 	/**
 	 * Checks to see if the nonce parameter sent in the Authorization Request 
@@ -104,7 +102,7 @@ public interface JwtSigningAndValidationService {
 	 * @return true if both nonce parameters are equal, false if otherwise 
 	 * @throws NoSuchAlgorithmException
 	 */
-	public void signJwt(Jwt jwt) 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
@@ -114,6 +112,8 @@ public interface JwtSigningAndValidationService {
 	 * @param alg the name of the algorithm to use, as specified in JWS s.6
 	 * @return the signed jwt
 	 */
+	public void signJwt(Jwt jwt) throws NoSuchAlgorithmException;
+	
 	//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

@@ -22,11 +22,11 @@ public abstract class AbstractJwtSigningAndValidationService implements JwtSigni
 	
 		Date expiration = jwt.getClaims().getExpiration();
 	
-		if (expiration != null)
+		if (expiration != null) {
 			return new Date().after(expiration);
-		else
+		} else {
 			return false;
-	
+		}
 	}
 
 	@Override
@@ -34,8 +34,9 @@ public abstract class AbstractJwtSigningAndValidationService implements JwtSigni
 	
 		String iss = jwt.getClaims().getIssuer();
 		
-		if (iss.equals(expectedIssuer))
+		if (iss.equals(expectedIssuer)) {
 			return true;
+		}
 		
 		return false;
 	}
@@ -44,10 +45,10 @@ public abstract class AbstractJwtSigningAndValidationService implements JwtSigni
 	public boolean validateSignature(String jwtString) throws NoSuchAlgorithmException {
 	
 		for (JwtSigner signer : getSigners().values()) {
-			if (signer.verify(jwtString))
+			if (signer.verify(jwtString)) {
 				return true;
+			}
 		}
-	
 		return false;
 	}
 	
@@ -55,19 +56,19 @@ public abstract class AbstractJwtSigningAndValidationService implements JwtSigni
 	public boolean validateIssuedAt(Jwt jwt) {
 		Date issuedAt = jwt.getClaims().getIssuedAt();
 		
-		if (issuedAt != null)
+		if (issuedAt != null) {
 			return new Date().before(issuedAt);
-		else
+		} else {
 			return false;
+		}
 	}
 
 	@Override
-	public boolean validateAudience(Jwt jwt, String clientId) {
+	public boolean validateAudience(Jwt jwt, String expectedAudience) {
 		
-		if(jwt.getClaims().getAudience().equals(clientId)){
+		if(jwt.getClaims().getAudience().equals(expectedAudience)){
 			return true;
-		}
+		} else {
-		else{
 			return false;
 		}
 	}
@@ -76,8 +77,7 @@ public abstract class AbstractJwtSigningAndValidationService implements JwtSigni
 	public boolean validateNonce(Jwt jwt, String nonce) {
 		if(jwt.getClaims().getNonce().equals(nonce)){
 			return true;
-		}
+		} else {
-		else{
 			return false;
 		}
 	}

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

@@ -34,12 +34,13 @@ public class DynamicJwtSigningAndValidationService extends AbstractJwtSigningAnd
 	
 	private Map<String, ? extends JwtSigner> signers;
 	
-	public DynamicJwtSigningAndValidationService(String x509SigningUrl, String jwkSigningUrl, String clientSecret) throws Exception {
+	public DynamicJwtSigningAndValidationService(String x509SigningUrl, String jwkSigningUrl, String clientSecret) {
 		setX509SigningUrl(x509SigningUrl);
 		setJwkSigningUrl(jwkSigningUrl);
 		setClientSecret(clientSecret);
 	}
 	
+	// FIXME: this function should not throw Exception
 	public Key getSigningKey() throws Exception {
 		if(signingKey == null){
 			if(x509SigningUrl != null){
@@ -118,24 +119,27 @@ public class DynamicJwtSigningAndValidationService extends AbstractJwtSigningAnd
 		
 	}
 	
-	public JwtSigner getSigner(String str) throws Exception {
+	public JwtSigner getSigner(String str) {
 		JwtHeader header = Jwt.parse(str).getHeader();
 		String alg = header.getAlgorithm();
 		JwtSigner signer = null;
 		
 		if(alg.equals("HS256") || alg.equals("HS384") || alg.equals("HS512")){
 			signer = new HmacSigner(alg, clientSecret); // TODO: huh? no, we're not signing with the client secret
-		}
+		} else if (alg.equals("RS256") || alg.equals("RS384") || alg.equals("RS512")){
-		
+			
-		else if (alg.equals("RS256") || alg.equals("RS384") || alg.equals("RS512")){
+			PublicKey rsaSigningKey = null;
-			signer = new RsaSigner(alg, (PublicKey) getSigningKey(), null);
+            try {
-		}
+	            rsaSigningKey = (PublicKey) getSigningKey();
-		
+            } catch (Exception e) {
-		else if (alg.equals("none")){
+	            // FIXME this function call should not throw Exception
+	            e.printStackTrace();
+	            return null;
+            }
+			signer = new RsaSigner(alg, rsaSigningKey, null);
+		} else if (alg.equals("none")){
 			signer = new PlaintextSigner();
-		}
+		} else {
-		
-		else{
 			throw new IllegalArgumentException("Not an existing algorithm type");
 		}
 		

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

@@ -34,10 +34,11 @@ import org.springframework.beans.factory.annotation.Autowired;
 public class JwtSigningAndValidationServiceDefault extends AbstractJwtSigningAndValidationService implements
 		JwtSigningAndValidationService, InitializingBean {
 
-	@Autowired ConfigurationPropertiesBean configBean;
+	@Autowired 
+	private ConfigurationPropertiesBean configBean;
 	
 	// map of identifier to signer
-	Map<String, ? extends JwtSigner> signers = new HashMap<String, JwtSigner>();
+	private Map<String, ? extends JwtSigner> signers = new HashMap<String, JwtSigner>();
 
 	private static Log logger = LogFactory
 			.getLog(JwtSigningAndValidationServiceDefault.class);

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

@@ -89,6 +89,7 @@ public class KeyStore implements InitializingBean {
 			}
 		}
 
+		// TODO: a more specific exception perhaps? is an empty keystore even an exception?
 		if (keystore.size() == 0) {
 			throw new Exception("Keystore is empty; it has no entries");
 		}

openid-connect-common/src/main/java/org/mitre/util/Utility.java

@@ -70,6 +70,7 @@ public class Utility {
 		return issuer;
 	}
 	
+	// FIXME: this should use a rest template and sould not throw Exception
 	public static List<Jwk> retrieveJwk(URL path) throws Exception {
 		List<Jwk> keys = new ArrayList<Jwk>();
 		
@@ -95,6 +96,7 @@ public class Utility {
 		return keys;
 	}
 	
+	// FIXME: this should use a rest template and sould not throw Exception
 	public static Key retrieveX509Key(URL url) throws Exception {
 		
 		CertificateFactory factory = CertificateFactory.getInstance("X.509");
@@ -104,6 +106,7 @@ public class Utility {
 		return key;
 	}
 	
+	// FIXME: this should use a rest template and sould not throw Exception
 	public static Key retrieveJwkKey(URL url) throws Exception {
 		
 		JsonParser parser = new JsonParser();

openid-connect-server/src/main/java/org/mitre/swd/view/XrdJsonResponse.java

@@ -18,6 +18,7 @@
  */
 package org.mitre.swd.view;
 
+import java.io.IOException;
 import java.io.Writer;
 import java.util.Map;
 
@@ -44,7 +45,7 @@ public class XrdJsonResponse extends AbstractView {
 	 * @see org.springframework.web.servlet.view.AbstractView#renderMergedOutputModel(java.util.Map, javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)
 	 */
 	@Override
-	protected void renderMergedOutputModel(Map<String, Object> model, HttpServletRequest request, HttpServletResponse response) throws Exception {
+	protected void renderMergedOutputModel(Map<String, Object> model, HttpServletRequest request, HttpServletResponse response) {
 		Gson gson = new GsonBuilder().setExclusionStrategies(new ExclusionStrategy() {
 
 			@Override
@@ -67,7 +68,14 @@ public class XrdJsonResponse extends AbstractView {
 
 		response.setContentType("application/json");
 
-		Writer out = response.getWriter();
+		Writer out;
+        try {
+	        out = response.getWriter();
+        } catch (IOException e) {
+	        // TODO Auto-generated catch block
+	        e.printStackTrace();
+	        return; // if we can't get the writer, this is pointless
+        }
 
 		Map<String, String> links = (Map<String, String>) model.get("links");