From bbeaeb06e384e91c9ae3ab25af01180dd07d77e6 Mon Sep 17 00:00:00 2001 From: Justin Richer Date: Thu, 18 Dec 2014 23:22:59 -0500 Subject: [PATCH] added option to send skip sending nonce if desired, closes #704, closes #683, --- .../client/OIDCAuthenticationFilter.java | 44 ++++++++++++------- .../impl/EncryptedAuthRequestUrlBuilder.java | 4 +- .../impl/PlainAuthRequestUrlBuilder.java | 4 +- .../impl/SignedAuthRequestUrlBuilder.java | 4 +- .../connect/config/ServerConfiguration.java | 41 ++++++++++++++++- 5 files changed, 77 insertions(+), 20 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 b744bb882..5dfd5ca5c 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 @@ -242,7 +242,10 @@ 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 = createNonce(session); + String nonce = null; + if (serverConfig.isNonceEnabled()) { + nonce = createNonce(session); + } // this value comes back in the auth code response String state = createState(session); @@ -543,21 +546,30 @@ public class OIDCAuthenticationFilter extends AbstractAuthenticationProcessingFi // compare the nonce to our stored claim String nonce = idClaims.getStringClaim("nonce"); - if (Strings.isNullOrEmpty(nonce)) { - - logger.error("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 + "."); - - 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 + "."); + + if (serverConfig.isNonceEnabled()) { + if (Strings.isNullOrEmpty(nonce)) { + + logger.error("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 + "."); + + 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); + } } // 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 be83e70ac..45074f7c3 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,7 +68,9 @@ public class EncryptedAuthRequestUrlBuilder implements AuthRequestUrlBuilder { claims.setClaim("redirect_uri", redirectUri); // this comes back in the id token - claims.setClaim("nonce", nonce); + if (nonce != null) { + 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 7c4cccc77..c61d17b8c 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,7 +54,9 @@ public class PlainAuthRequestUrlBuilder implements AuthRequestUrlBuilder { uriBuilder.addParameter("redirect_uri", redirectUri); - uriBuilder.addParameter("nonce", nonce); + if (nonce != null) { + 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 78fcd9bd7..d2a86916c 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 @@ -61,7 +61,9 @@ public class SignedAuthRequestUrlBuilder implements AuthRequestUrlBuilder { claims.setClaim("redirect_uri", redirectUri); // this comes back in the id token - claims.setClaim("nonce", nonce); + if (nonce != null) { + 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 01c7e41af..9e1887ead 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 @@ -205,6 +205,12 @@ public class ServerConfiguration { private Boolean requireRequestUriRegistration; private String opPolicyUri; private String opTosUri; + + // + // extensions to the discoverable methods + // + + // how do we send the access token to the userinfo endpoint? private UserInfoTokenMethod userInfoTokenMethod; public enum UserInfoTokenMethod { @@ -213,6 +219,9 @@ public class ServerConfiguration { QUERY; } + // do we create and send a nonce value? + private boolean nonceEnabled = true; + /** * @return the authorizationEndpointUri */ @@ -671,6 +680,23 @@ 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; @@ -731,6 +757,7 @@ 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 @@ -796,6 +823,10 @@ 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 @@ -812,6 +843,9 @@ 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) { @@ -942,6 +976,9 @@ 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; @@ -1077,6 +1114,9 @@ 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; @@ -1111,5 +1151,4 @@ public class ServerConfiguration { return true; } - }