diff --git a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/AbstractOIDCAuthenticationFilter.java b/openid-connect-client/src/main/java/org/mitre/openid/connect/client/AbstractOIDCAuthenticationFilter.java index cecd8bb9c..2005886ff 100644 --- a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/AbstractOIDCAuthenticationFilter.java +++ b/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(); diff --git a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/OIDCServerConfiguration.java b/openid-connect-client/src/main/java/org/mitre/openid/connect/client/OIDCServerConfiguration.java index 675f54775..c5f27688f 100644 --- a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/OIDCServerConfiguration.java +++ b/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; diff --git a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/OIDCUserDetailService.java b/openid-connect-client/src/main/java/org/mitre/openid/connect/client/OIDCUserDetailService.java index 64a0980b1..301c71b4e 100644 --- a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/OIDCUserDetailService.java +++ b/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 { diff --git a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/UrlValidator.java b/openid-connect-client/src/main/java/org/mitre/openid/connect/client/UrlValidator.java index 66c723f0a..a89f216e6 100644 --- a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/UrlValidator.java +++ b/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"); } diff --git a/openid-connect-common/src/main/java/org/mitre/jwt/signer/impl/HmacSigner.java b/openid-connect-common/src/main/java/org/mitre/jwt/signer/impl/HmacSigner.java index 5f93ff13a..86a0ce452 100644 --- a/openid-connect-common/src/main/java/org/mitre/jwt/signer/impl/HmacSigner.java +++ b/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()) diff --git a/openid-connect-common/src/main/java/org/mitre/jwt/signer/impl/RsaSigner.java b/openid-connect-common/src/main/java/org/mitre/jwt/signer/impl/RsaSigner.java index 4b261a15e..b394d4153 100644 --- a/openid-connect-common/src/main/java/org/mitre/jwt/signer/impl/RsaSigner.java +++ b/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()); diff --git a/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/JwtSigningAndValidationService.java b/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/JwtSigningAndValidationService.java index 0cb60026f..042d3fc41 100644 --- a/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/JwtSigningAndValidationService.java +++ b/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); diff --git a/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/AbstractJwtSigningAndValidationService.java b/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/AbstractJwtSigningAndValidationService.java index 8d3a8d437..e3cc3a833 100644 --- a/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/AbstractJwtSigningAndValidationService.java +++ b/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; } } diff --git a/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/DynamicJwtSigningAndValidationService.java b/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/DynamicJwtSigningAndValidationService.java index b475bd266..0c327d890 100644 --- a/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/DynamicJwtSigningAndValidationService.java +++ b/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 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")){ - signer = new RsaSigner(alg, (PublicKey) getSigningKey(), null); - } - - else if (alg.equals("none")){ + } else if (alg.equals("RS256") || alg.equals("RS384") || alg.equals("RS512")){ + + PublicKey rsaSigningKey = null; + try { + rsaSigningKey = (PublicKey) getSigningKey(); + } catch (Exception e) { + // 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"); } diff --git a/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/JwtSigningAndValidationServiceDefault.java b/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/JwtSigningAndValidationServiceDefault.java index 77eb8cd83..c2a9256e0 100644 --- a/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/JwtSigningAndValidationServiceDefault.java +++ b/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 signers = new HashMap(); + private Map signers = new HashMap(); private static Log logger = LogFactory .getLog(JwtSigningAndValidationServiceDefault.class); diff --git a/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/KeyStore.java b/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/KeyStore.java index b3afc9122..2a59fca42 100644 --- a/openid-connect-common/src/main/java/org/mitre/jwt/signer/service/impl/KeyStore.java +++ b/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"); } diff --git a/openid-connect-common/src/main/java/org/mitre/util/Utility.java b/openid-connect-common/src/main/java/org/mitre/util/Utility.java index 5bc809bd5..35acac409 100644 --- a/openid-connect-common/src/main/java/org/mitre/util/Utility.java +++ b/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 retrieveJwk(URL path) throws Exception { List keys = new ArrayList(); @@ -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(); diff --git a/openid-connect-server/src/main/java/org/mitre/swd/view/XrdJsonResponse.java b/openid-connect-server/src/main/java/org/mitre/swd/view/XrdJsonResponse.java index 5f2a69c7b..c77c0a2d9 100644 --- a/openid-connect-server/src/main/java/org/mitre/swd/view/XrdJsonResponse.java +++ b/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 model, HttpServletRequest request, HttpServletResponse response) throws Exception { + protected void renderMergedOutputModel(Map 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 links = (Map) model.get("links");