From caf85b990dc822f19822383b395327a2c8e5cd84 Mon Sep 17 00:00:00 2001 From: Justin Richer Date: Thu, 28 May 2015 16:44:26 -0400 Subject: [PATCH] Revert "added option to send skip sending nonce if desired, closes #704, closes #683," This reverts commit bbeaeb06e384e91c9ae3ab25af01180dd07d77e6. Conflicts: openid-connect-client/src/main/java/org/mitre/openid/connect/client/OIDCAuthenticationFilter.java openid-connect-common/src/main/java/org/mitre/openid/connect/config/ServerConfiguration.java --- .../client/OIDCAuthenticationFilter.java | 36 +++++++------------ .../impl/EncryptedAuthRequestUrlBuilder.java | 4 +-- .../impl/PlainAuthRequestUrlBuilder.java | 4 +-- .../impl/SignedAuthRequestUrlBuilder.java | 4 +-- .../connect/config/ServerConfiguration.java | 35 +----------------- 5 files changed, 16 insertions(+), 67 deletions(-) diff --git a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/OIDCAuthenticationFilter.java b/openid-connect-client/src/main/java/org/mitre/openid/connect/client/OIDCAuthenticationFilter.java index add39dfc3..7e83a1355 100644 --- a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/OIDCAuthenticationFilter.java +++ b/openid-connect-client/src/main/java/org/mitre/openid/connect/client/OIDCAuthenticationFilter.java @@ -254,10 +254,7 @@ public class OIDCAuthenticationFilter extends AbstractAuthenticationProcessingFi session.setAttribute(REDIRECT_URI_SESION_VARIABLE, redirectUri); // this value comes back in the id token and is checked there - String nonce = null; - if (serverConfig.isNonceEnabled()) { - nonce = createNonce(session); - } + String nonce = createNonce(session); // this value comes back in the auth code response String state = createState(session); @@ -565,30 +562,21 @@ public class OIDCAuthenticationFilter extends AbstractAuthenticationProcessingFi // compare the nonce to our stored claim String nonce = idClaims.getStringClaim("nonce"); + if (Strings.isNullOrEmpty(nonce)) { - if (serverConfig.isNonceEnabled()) { - if (Strings.isNullOrEmpty(nonce)) { + logger.error("ID token did not contain a nonce claim."); - logger.error("ID token did not contain a nonce claim."); + throw new AuthenticationServiceException("ID token did not contain a nonce claim."); + } - throw new AuthenticationServiceException("ID token did not contain a nonce claim."); - } + String storedNonce = getStoredNonce(session); + if (!nonce.equals(storedNonce)) { + logger.error("Possible replay attack detected! The comparison of the nonce in the returned " + + "ID Token to the session " + NONCE_SESSION_VARIABLE + " failed. Expected " + storedNonce + " got " + nonce + "."); - String storedNonce = getStoredNonce(session); - if (!nonce.equals(storedNonce)) { - logger.error("Possible replay attack detected! The comparison of the nonce in the returned " - + "ID Token to the session " + NONCE_SESSION_VARIABLE + " failed. Expected " + storedNonce + " got " + nonce + "."); - - throw new AuthenticationServiceException( - "Possible replay attack detected! The comparison of the nonce in the returned " - + "ID Token to the session " + NONCE_SESSION_VARIABLE + " failed. Expected " + storedNonce + " got " + nonce + "."); - } - } else { - if (!Strings.isNullOrEmpty(nonce)) { - logger.error("Possible injection attack! The server returned a nonce value where none was sent or expected: " + nonce); - throw new AuthenticationServiceException( - "Possible injection attack! The server returned a nonce value where none was sent or expected: " + nonce); - } + throw new AuthenticationServiceException( + "Possible replay attack detected! The comparison of the nonce in the returned " + + "ID Token to the session " + NONCE_SESSION_VARIABLE + " failed. Expected " + storedNonce + " got " + nonce + "."); } // pull the subject (user id) out as a claim on the id_token diff --git a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/EncryptedAuthRequestUrlBuilder.java b/openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/EncryptedAuthRequestUrlBuilder.java index fefd3a859..72438576f 100644 --- a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/EncryptedAuthRequestUrlBuilder.java +++ b/openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/EncryptedAuthRequestUrlBuilder.java @@ -68,9 +68,7 @@ public class EncryptedAuthRequestUrlBuilder implements AuthRequestUrlBuilder { claims.setClaim("redirect_uri", redirectUri); // this comes back in the id token - if (nonce != null) { - claims.setClaim("nonce", nonce); - } + claims.setClaim("nonce", nonce); // this comes back in the auth request return claims.setClaim("state", state); diff --git a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/PlainAuthRequestUrlBuilder.java b/openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/PlainAuthRequestUrlBuilder.java index a13b51f46..1adfd87c6 100644 --- a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/PlainAuthRequestUrlBuilder.java +++ b/openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/PlainAuthRequestUrlBuilder.java @@ -54,9 +54,7 @@ public class PlainAuthRequestUrlBuilder implements AuthRequestUrlBuilder { uriBuilder.addParameter("redirect_uri", redirectUri); - if (nonce != null) { - uriBuilder.addParameter("nonce", nonce); - } + uriBuilder.addParameter("nonce", nonce); uriBuilder.addParameter("state", state); diff --git a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/SignedAuthRequestUrlBuilder.java b/openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/SignedAuthRequestUrlBuilder.java index b54d0b1ef..493e16e55 100644 --- a/openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/SignedAuthRequestUrlBuilder.java +++ b/openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/SignedAuthRequestUrlBuilder.java @@ -62,9 +62,7 @@ public class SignedAuthRequestUrlBuilder implements AuthRequestUrlBuilder { claims.setClaim("redirect_uri", redirectUri); // this comes back in the id token - if (nonce != null) { - claims.setClaim("nonce", nonce); - } + claims.setClaim("nonce", nonce); // this comes back in the auth request return claims.setClaim("state", state); diff --git a/openid-connect-common/src/main/java/org/mitre/openid/connect/config/ServerConfiguration.java b/openid-connect-common/src/main/java/org/mitre/openid/connect/config/ServerConfiguration.java index 75bd8ff25..6e7784828 100644 --- a/openid-connect-common/src/main/java/org/mitre/openid/connect/config/ServerConfiguration.java +++ b/openid-connect-common/src/main/java/org/mitre/openid/connect/config/ServerConfiguration.java @@ -219,9 +219,6 @@ public class ServerConfiguration { QUERY; } - // do we create and send a nonce value? - private boolean nonceEnabled = true; - /** * @return the authorizationEndpointUri */ @@ -680,23 +677,6 @@ public class ServerConfiguration { public void setUserInfoTokenMethod(UserInfoTokenMethod userInfoTokenMethod) { this.userInfoTokenMethod = userInfoTokenMethod; } - - - /** - * @return the nonceEnabled - */ - public boolean isNonceEnabled() { - return nonceEnabled; - } - /** - * @param nonceEnabled the nonceEnabled to set - */ - public void setNonceEnabled(boolean nonceEnabled) { - this.nonceEnabled = nonceEnabled; - } - /* (non-Javadoc) - * @see java.lang.Object#hashCode() - */ @Override public int hashCode() { final int prime = 31; @@ -757,7 +737,6 @@ public class ServerConfiguration { : introspectionEndpointUri.hashCode()); result = prime * result + ((issuer == null) ? 0 : issuer.hashCode()); result = prime * result + ((jwksUri == null) ? 0 : jwksUri.hashCode()); - result = prime * result + (nonceEnabled ? 1231 : 1237); result = prime * result + ((opPolicyUri == null) ? 0 : opPolicyUri.hashCode()); result = prime * result @@ -823,10 +802,6 @@ public class ServerConfiguration { * result + ((uiLocalesSupported == null) ? 0 : uiLocalesSupported .hashCode()); - result = prime - * result - + ((userInfoTokenMethod == null) ? 0 : userInfoTokenMethod - .hashCode()); result = prime * result + ((userInfoUri == null) ? 0 : userInfoUri.hashCode()); result = prime @@ -843,9 +818,6 @@ public class ServerConfiguration { : userinfoSigningAlgValuesSupported.hashCode()); return result; } - /* (non-Javadoc) - * @see java.lang.Object#equals(java.lang.Object) - */ @Override public boolean equals(Object obj) { if (this == obj) { @@ -976,9 +948,6 @@ public class ServerConfiguration { } else if (!jwksUri.equals(other.jwksUri)) { return false; } - if (nonceEnabled != other.nonceEnabled) { - return false; - } if (opPolicyUri == null) { if (other.opPolicyUri != null) { return false; @@ -1114,9 +1083,6 @@ public class ServerConfiguration { } else if (!uiLocalesSupported.equals(other.uiLocalesSupported)) { return false; } - if (userInfoTokenMethod != other.userInfoTokenMethod) { - return false; - } if (userInfoUri == null) { if (other.userInfoUri != null) { return false; @@ -1151,4 +1117,5 @@ public class ServerConfiguration { return true; } + }