Skip to content

Commit caf85b9

Browse files
committed
Revert "added option to send skip sending nonce if desired, closes mitreid-connect#704, closes mitreid-connect#683,"
This reverts commit bbeaeb0. 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
1 parent d32118d commit caf85b9

File tree

5 files changed

+16
-67
lines changed

5 files changed

+16
-67
lines changed

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

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -254,10 +254,7 @@ protected void handleAuthorizationRequest(HttpServletRequest request, HttpServle
254254
session.setAttribute(REDIRECT_URI_SESION_VARIABLE, redirectUri);
255255

256256
// this value comes back in the id token and is checked there
257-
String nonce = null;
258-
if (serverConfig.isNonceEnabled()) {
259-
nonce = createNonce(session);
260-
}
257+
String nonce = createNonce(session);
261258

262259
// this value comes back in the auth code response
263260
String state = createState(session);
@@ -565,30 +562,21 @@ protected ClientHttpRequest createRequest(URI url, HttpMethod method) throws IOE
565562

566563
// compare the nonce to our stored claim
567564
String nonce = idClaims.getStringClaim("nonce");
565+
if (Strings.isNullOrEmpty(nonce)) {
568566

569-
if (serverConfig.isNonceEnabled()) {
570-
if (Strings.isNullOrEmpty(nonce)) {
571-
572-
logger.error("ID token did not contain a nonce claim.");
567+
logger.error("ID token did not contain a nonce claim.");
573568

574-
throw new AuthenticationServiceException("ID token did not contain a nonce claim.");
575-
}
569+
throw new AuthenticationServiceException("ID token did not contain a nonce claim.");
570+
}
576571

577-
String storedNonce = getStoredNonce(session);
578-
if (!nonce.equals(storedNonce)) {
579-
logger.error("Possible replay attack detected! The comparison of the nonce in the returned "
580-
+ "ID Token to the session " + NONCE_SESSION_VARIABLE + " failed. Expected " + storedNonce + " got " + nonce + ".");
572+
String storedNonce = getStoredNonce(session);
573+
if (!nonce.equals(storedNonce)) {
574+
logger.error("Possible replay attack detected! The comparison of the nonce in the returned "
575+
+ "ID Token to the session " + NONCE_SESSION_VARIABLE + " failed. Expected " + storedNonce + " got " + nonce + ".");
581576

582-
throw new AuthenticationServiceException(
583-
"Possible replay attack detected! The comparison of the nonce in the returned "
584-
+ "ID Token to the session " + NONCE_SESSION_VARIABLE + " failed. Expected " + storedNonce + " got " + nonce + ".");
585-
}
586-
} else {
587-
if (!Strings.isNullOrEmpty(nonce)) {
588-
logger.error("Possible injection attack! The server returned a nonce value where none was sent or expected: " + nonce);
589-
throw new AuthenticationServiceException(
590-
"Possible injection attack! The server returned a nonce value where none was sent or expected: " + nonce);
591-
}
577+
throw new AuthenticationServiceException(
578+
"Possible replay attack detected! The comparison of the nonce in the returned "
579+
+ "ID Token to the session " + NONCE_SESSION_VARIABLE + " failed. Expected " + storedNonce + " got " + nonce + ".");
592580
}
593581

594582
// pull the subject (user id) out as a claim on the id_token

openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/EncryptedAuthRequestUrlBuilder.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,7 @@ public String buildAuthRequestUrl(ServerConfiguration serverConfig, RegisteredCl
6868
claims.setClaim("redirect_uri", redirectUri);
6969

7070
// this comes back in the id token
71-
if (nonce != null) {
72-
claims.setClaim("nonce", nonce);
73-
}
71+
claims.setClaim("nonce", nonce);
7472

7573
// this comes back in the auth request return
7674
claims.setClaim("state", state);

openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/PlainAuthRequestUrlBuilder.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,7 @@ public String buildAuthRequestUrl(ServerConfiguration serverConfig, RegisteredCl
5454

5555
uriBuilder.addParameter("redirect_uri", redirectUri);
5656

57-
if (nonce != null) {
58-
uriBuilder.addParameter("nonce", nonce);
59-
}
57+
uriBuilder.addParameter("nonce", nonce);
6058

6159
uriBuilder.addParameter("state", state);
6260

openid-connect-client/src/main/java/org/mitre/openid/connect/client/service/impl/SignedAuthRequestUrlBuilder.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,7 @@ public String buildAuthRequestUrl(ServerConfiguration serverConfig, RegisteredCl
6262
claims.setClaim("redirect_uri", redirectUri);
6363

6464
// this comes back in the id token
65-
if (nonce != null) {
66-
claims.setClaim("nonce", nonce);
67-
}
65+
claims.setClaim("nonce", nonce);
6866

6967
// this comes back in the auth request return
7068
claims.setClaim("state", state);

openid-connect-common/src/main/java/org/mitre/openid/connect/config/ServerConfiguration.java

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,6 @@ public enum UserInfoTokenMethod {
219219
QUERY;
220220
}
221221

222-
// do we create and send a nonce value?
223-
private boolean nonceEnabled = true;
224-
225222
/**
226223
* @return the authorizationEndpointUri
227224
*/
@@ -680,23 +677,6 @@ public UserInfoTokenMethod getUserInfoTokenMethod() {
680677
public void setUserInfoTokenMethod(UserInfoTokenMethod userInfoTokenMethod) {
681678
this.userInfoTokenMethod = userInfoTokenMethod;
682679
}
683-
684-
685-
/**
686-
* @return the nonceEnabled
687-
*/
688-
public boolean isNonceEnabled() {
689-
return nonceEnabled;
690-
}
691-
/**
692-
* @param nonceEnabled the nonceEnabled to set
693-
*/
694-
public void setNonceEnabled(boolean nonceEnabled) {
695-
this.nonceEnabled = nonceEnabled;
696-
}
697-
/* (non-Javadoc)
698-
* @see java.lang.Object#hashCode()
699-
*/
700680
@Override
701681
public int hashCode() {
702682
final int prime = 31;
@@ -757,7 +737,6 @@ public int hashCode() {
757737
: introspectionEndpointUri.hashCode());
758738
result = prime * result + ((issuer == null) ? 0 : issuer.hashCode());
759739
result = prime * result + ((jwksUri == null) ? 0 : jwksUri.hashCode());
760-
result = prime * result + (nonceEnabled ? 1231 : 1237);
761740
result = prime * result
762741
+ ((opPolicyUri == null) ? 0 : opPolicyUri.hashCode());
763742
result = prime * result
@@ -823,10 +802,6 @@ public int hashCode() {
823802
* result
824803
+ ((uiLocalesSupported == null) ? 0 : uiLocalesSupported
825804
.hashCode());
826-
result = prime
827-
* result
828-
+ ((userInfoTokenMethod == null) ? 0 : userInfoTokenMethod
829-
.hashCode());
830805
result = prime * result
831806
+ ((userInfoUri == null) ? 0 : userInfoUri.hashCode());
832807
result = prime
@@ -843,9 +818,6 @@ public int hashCode() {
843818
: userinfoSigningAlgValuesSupported.hashCode());
844819
return result;
845820
}
846-
/* (non-Javadoc)
847-
* @see java.lang.Object#equals(java.lang.Object)
848-
*/
849821
@Override
850822
public boolean equals(Object obj) {
851823
if (this == obj) {
@@ -976,9 +948,6 @@ public boolean equals(Object obj) {
976948
} else if (!jwksUri.equals(other.jwksUri)) {
977949
return false;
978950
}
979-
if (nonceEnabled != other.nonceEnabled) {
980-
return false;
981-
}
982951
if (opPolicyUri == null) {
983952
if (other.opPolicyUri != null) {
984953
return false;
@@ -1114,9 +1083,6 @@ public boolean equals(Object obj) {
11141083
} else if (!uiLocalesSupported.equals(other.uiLocalesSupported)) {
11151084
return false;
11161085
}
1117-
if (userInfoTokenMethod != other.userInfoTokenMethod) {
1118-
return false;
1119-
}
11201086
if (userInfoUri == null) {
11211087
if (other.userInfoUri != null) {
11221088
return false;
@@ -1151,4 +1117,5 @@ public boolean equals(Object obj) {
11511117
return true;
11521118
}
11531119

1120+
11541121
}

0 commit comments

Comments
 (0)