Skip to content

Commit d3b7a47

Browse files
jgrandjajgrandja
committed
Polish gh-4442 
1 parent da9f027 commit d3b7a47

File tree

13 files changed

+248
-202
lines changed

13 files changed

+248
-202
lines changed

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2ClientConfigurerTests.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@
7575
* Tests for {@link OAuth2ClientConfigurer}.
7676
*
7777
* @author Joe Grandja
78-
* @author Mark Heckler
7978
*/
8079
public class OAuth2ClientConfigurerTests {
8180
private static ClientRegistrationRepository clientRegistrationRepository;
@@ -139,8 +138,7 @@ public void configureWhenAuthorizationCodeRequestThenRedirectForAuthorization()
139138
assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" +
140139
"response_type=code&client_id=client-1&" +
141140
"scope=user&state=.{15,}&" +
142-
"redirect_uri=http://localhost/client-1&" +
143-
"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
141+
"redirect_uri=http://localhost/client-1");
144142
}
145143

146144
@Test
@@ -153,8 +151,7 @@ public void configureWhenOauth2ClientInLambdaThenRedirectForAuthorization() thro
153151
assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" +
154152
"response_type=code&client_id=client-1&" +
155153
"scope=user&state=.{15,}&" +
156-
"redirect_uri=http://localhost/client-1&" +
157-
"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
154+
"redirect_uri=http://localhost/client-1");
158155
}
159156

160157
@Test
@@ -206,8 +203,7 @@ public void configureWhenRequestCacheProvidedAndClientAuthorizationRequiredExcep
206203
assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" +
207204
"response_type=code&client_id=client-1&" +
208205
"scope=user&state=.{15,}&" +
209-
"redirect_uri=http://localhost/client-1&" +
210-
"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
206+
"redirect_uri=http://localhost/client-1");
211207

212208
verify(requestCache).saveRequest(any(HttpServletRequest.class), any(HttpServletResponse.class));
213209
}

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeAuthenticationProvider.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -158,21 +158,22 @@ public Authentication authenticate(Authentication authentication) throws Authent
158158
null);
159159
throw new OAuth2AuthenticationException(invalidIdTokenError, invalidIdTokenError.toString());
160160
}
161-
OidcIdToken idToken = createOidcToken(clientRegistration, accessTokenResponse);
161+
OidcIdToken idToken = createOidcToken(clientRegistration, accessTokenResponse);
162162

163+
// Validate nonce
163164
String requestNonce = authorizationRequest.getAttribute(OidcParameterNames.NONCE);
164165
if (requestNonce != null) {
165166
String nonceHash;
166-
167167
try {
168168
nonceHash = createHash(requestNonce);
169169
} catch (NoSuchAlgorithmException e) {
170-
throw new OAuth2AuthenticationException(new OAuth2Error(INVALID_NONCE_ERROR_CODE));
170+
OAuth2Error oauth2Error = new OAuth2Error(INVALID_NONCE_ERROR_CODE);
171+
throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
171172
}
172-
173-
String nonceHashClaim = idToken.getClaim(OidcParameterNames.NONCE);
173+
String nonceHashClaim = idToken.getNonce();
174174
if (nonceHashClaim == null || !nonceHashClaim.equals(nonceHash)) {
175-
throw new OAuth2AuthenticationException(new OAuth2Error(INVALID_NONCE_ERROR_CODE));
175+
OAuth2Error oauth2Error = new OAuth2Error(INVALID_NONCE_ERROR_CODE);
176+
throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
176177
}
177178
}
178179

@@ -234,7 +235,7 @@ private OidcIdToken createOidcToken(ClientRegistration clientRegistration, OAuth
234235
return idToken;
235236
}
236237

237-
private String createHash(String nonce) throws NoSuchAlgorithmException {
238+
static String createHash(String nonce) throws NoSuchAlgorithmException {
238239
MessageDigest md = MessageDigest.getInstance("SHA-256");
239240
byte[] digest = md.digest(nonce.getBytes(StandardCharsets.US_ASCII));
240241
return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeReactiveAuthenticationManager.java

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
* to complete the authentication.
6666
*
6767
* @author Rob Winch
68+
* @author Mark Heckler
6869
* @since 5.1
6970
* @see OAuth2LoginAuthenticationToken
7071
* @see ReactiveOAuth2AccessTokenResponseClient
@@ -199,30 +200,28 @@ private Mono<OidcIdToken> createOidcToken(ClientRegistration clientRegistration,
199200
.map(jwt -> new OidcIdToken(jwt.getTokenValue(), jwt.getIssuedAt(), jwt.getExpiresAt(), jwt.getClaims()));
200201
}
201202

202-
private Mono<OidcIdToken> validateNonce(OAuth2AuthorizationCodeAuthenticationToken authorizationCodeAuthentication, OidcIdToken idToken) {
203-
String requestNonce = authorizationCodeAuthentication
204-
.getAuthorizationExchange()
205-
.getAuthorizationRequest()
206-
.getAttribute(OidcParameterNames.NONCE);
203+
private static Mono<OidcIdToken> validateNonce(OAuth2AuthorizationCodeAuthenticationToken authorizationCodeAuthentication, OidcIdToken idToken) {
204+
String requestNonce = authorizationCodeAuthentication.getAuthorizationExchange()
205+
.getAuthorizationRequest().getAttribute(OidcParameterNames.NONCE);
207206
if (requestNonce != null) {
208207
String nonceHash;
209-
210208
try {
211209
nonceHash = createHash(requestNonce);
212210
} catch (NoSuchAlgorithmException e) {
213-
throw new OAuth2AuthenticationException(new OAuth2Error(INVALID_NONCE_ERROR_CODE));
211+
OAuth2Error oauth2Error = new OAuth2Error(INVALID_NONCE_ERROR_CODE);
212+
throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
214213
}
215-
216-
String nonceHashClaim = idToken.getClaim(OidcParameterNames.NONCE);
214+
String nonceHashClaim = idToken.getNonce();
217215
if (nonceHashClaim == null || !nonceHashClaim.equals(nonceHash)) {
218-
throw new OAuth2AuthenticationException(new OAuth2Error(INVALID_NONCE_ERROR_CODE));
216+
OAuth2Error oauth2Error = new OAuth2Error(INVALID_NONCE_ERROR_CODE);
217+
throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
219218
}
220219
}
221220

222221
return Mono.just(idToken);
223222
}
224223

225-
private String createHash(String nonce) throws NoSuchAlgorithmException {
224+
static String createHash(String nonce) throws NoSuchAlgorithmException {
226225
MessageDigest md = MessageDigest.getInstance("SHA-256");
227226
byte[] digest = md.digest(nonce.getBytes(StandardCharsets.US_ASCII));
228227
return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,12 @@
2424
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
2525
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
2626
import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
27+
import org.springframework.security.oauth2.core.oidc.OidcScopes;
2728
import org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames;
2829
import org.springframework.security.web.util.UrlUtils;
2930
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
3031
import org.springframework.util.Assert;
32+
import org.springframework.util.CollectionUtils;
3133
import org.springframework.util.StringUtils;
3234
import org.springframework.web.util.UriComponents;
3335
import org.springframework.web.util.UriComponentsBuilder;
@@ -63,7 +65,7 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au
6365
private final ClientRegistrationRepository clientRegistrationRepository;
6466
private final AntPathRequestMatcher authorizationRequestMatcher;
6567
private final StringKeyGenerator stateGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder());
66-
private final StringKeyGenerator stringKeyGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96);
68+
private final StringKeyGenerator secureKeyGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96);
6769

6870
/**
6971
* Constructs a {@code DefaultOAuth2AuthorizationRequestResolver} using the provided parameters.
@@ -121,13 +123,16 @@ private OAuth2AuthorizationRequest resolve(HttpServletRequest request, String re
121123
if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
122124
builder = OAuth2AuthorizationRequest.authorizationCode();
123125
Map<String, Object> additionalParameters = new HashMap<>();
124-
125-
addNonceParameters(attributes, additionalParameters);
126-
126+
if (!CollectionUtils.isEmpty(clientRegistration.getScopes()) &&
127+
clientRegistration.getScopes().contains(OidcScopes.OPENID)) {
128+
// Section 3.1.2.1 Authentication Request - https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
129+
// scope
130+
// REQUIRED. OpenID Connect requests MUST contain the "openid" scope value.
131+
addNonceParameters(attributes, additionalParameters);
132+
}
127133
if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) {
128134
addPkceParameters(attributes, additionalParameters);
129135
}
130-
131136
builder.additionalParameters(additionalParameters);
132137
} else if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType())) {
133138
builder = OAuth2AuthorizationRequest.implicit();
@@ -208,24 +213,21 @@ private static String expandRedirectUri(HttpServletRequest request, ClientRegist
208213
}
209214

210215
/**
211-
* Creates nonce and its hash for use in OpenID Connect Authentication Requests
216+
* Creates nonce and its hash for use in OpenID Connect 1.0 Authentication Requests.
212217
*
213-
* @param attributes where {@link OidcParameterNames#NONCE} is stored for the token request
214-
* @param additionalParameters where hash of {@link OidcParameterNames#NONCE} is added to the authentication request
218+
* @param attributes where the {@link OidcParameterNames#NONCE} is stored for the authentication request
219+
* @param additionalParameters where the {@link OidcParameterNames#NONCE} hash is added for the authentication request
215220
*
216221
* @since 5.2
217-
* @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#NonceNotes">15.5.2. Nonce Implementation Notes</a>
218-
* @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation">3.1.3.7. ID Token Validation</a>
222+
* @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest">3.1.2.1. Authentication Request</a>
219223
*/
220224
private void addNonceParameters(Map<String, Object> attributes, Map<String, Object> additionalParameters) {
221225
try {
222-
String nonce = this.stringKeyGenerator.generateKey();
223-
attributes.put(OidcParameterNames.NONCE, nonce);
224-
226+
String nonce = this.secureKeyGenerator.generateKey();
225227
String nonceHash = createHash(nonce);
228+
attributes.put(OidcParameterNames.NONCE, nonce);
226229
additionalParameters.put(OidcParameterNames.NONCE, nonceHash);
227-
} catch (NoSuchAlgorithmException ignored) {
228-
}
230+
} catch (NoSuchAlgorithmException e) { }
229231
}
230232

231233
/**
@@ -241,7 +243,7 @@ private void addNonceParameters(Map<String, Object> attributes, Map<String, Obje
241243
* @see <a target="_blank" href="https://tools.ietf.org/html/rfc7636#section-4.2">4.2. Client Creates the Code Challenge</a>
242244
*/
243245
private void addPkceParameters(Map<String, Object> attributes, Map<String, Object> additionalParameters) {
244-
String codeVerifier = this.stringKeyGenerator.generateKey();
246+
String codeVerifier = this.secureKeyGenerator.generateKey();
245247
attributes.put(PkceParameterNames.CODE_VERIFIER, codeVerifier);
246248
try {
247249
String codeChallenge = createHash(codeVerifier);
@@ -252,7 +254,7 @@ private void addPkceParameters(Map<String, Object> attributes, Map<String, Objec
252254
}
253255
}
254256

255-
private String createHash(String value) throws NoSuchAlgorithmException {
257+
private static String createHash(String value) throws NoSuchAlgorithmException {
256258
MessageDigest md = MessageDigest.getInstance("SHA-256");
257259
byte[] digest = md.digest(value.getBytes(StandardCharsets.US_ASCII));
258260
return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,12 @@
2727
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
2828
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
2929
import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
30+
import org.springframework.security.oauth2.core.oidc.OidcScopes;
3031
import org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames;
3132
import org.springframework.security.web.server.util.matcher.PathPatternParserServerWebExchangeMatcher;
3233
import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher;
3334
import org.springframework.util.Assert;
35+
import org.springframework.util.CollectionUtils;
3436
import org.springframework.util.StringUtils;
3537
import org.springframework.web.server.ResponseStatusException;
3638
import org.springframework.web.server.ServerWebExchange;
@@ -77,7 +79,7 @@ public class DefaultServerOAuth2AuthorizationRequestResolver
7779

7880
private final StringKeyGenerator stateGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder());
7981

80-
private final StringKeyGenerator stringKeyGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96);
82+
private final StringKeyGenerator secureKeyGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96);
8183

8284
/**
8385
* Creates a new instance
@@ -135,13 +137,16 @@ private OAuth2AuthorizationRequest authorizationRequest(ServerWebExchange exchan
135137
if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
136138
builder = OAuth2AuthorizationRequest.authorizationCode();
137139
Map<String, Object> additionalParameters = new HashMap<>();
138-
139-
addNonceParameters(attributes, additionalParameters);
140-
140+
if (!CollectionUtils.isEmpty(clientRegistration.getScopes()) &&
141+
clientRegistration.getScopes().contains(OidcScopes.OPENID)) {
142+
// Section 3.1.2.1 Authentication Request - https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
143+
// scope
144+
// REQUIRED. OpenID Connect requests MUST contain the "openid" scope value.
145+
addNonceParameters(attributes, additionalParameters);
146+
}
141147
if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) {
142148
addPkceParameters(attributes, additionalParameters);
143149
}
144-
145150
builder.additionalParameters(additionalParameters);
146151
} else if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType())) {
147152
builder = OAuth2AuthorizationRequest.implicit();
@@ -212,24 +217,21 @@ private static String expandRedirectUri(ServerHttpRequest request, ClientRegistr
212217
}
213218

214219
/**
215-
* Creates nonce and its hash for use in OpenID Connect Authentication Requests
220+
* Creates nonce and its hash for use in OpenID Connect 1.0 Authentication Requests.
216221
*
217-
* @param attributes where {@link OidcParameterNames#NONCE} is stored for the token request
218-
* @param additionalParameters where hash of {@link OidcParameterNames#NONCE} is added to the authentication request
222+
* @param attributes where the {@link OidcParameterNames#NONCE} is stored for the authentication request
223+
* @param additionalParameters where the {@link OidcParameterNames#NONCE} hash is added for the authentication request
219224
*
220225
* @since 5.2
221-
* @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#NonceNotes">15.5.2. Nonce Implementation Notes</a>
222-
* @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation">3.1.3.7. ID Token Validation</a>
226+
* @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest">3.1.2.1. Authentication Request</a>
223227
*/
224228
private void addNonceParameters(Map<String, Object> attributes, Map<String, Object> additionalParameters) {
225229
try {
226-
String nonce = this.stringKeyGenerator.generateKey();
227-
attributes.put(OidcParameterNames.NONCE, nonce);
228-
230+
String nonce = this.secureKeyGenerator.generateKey();
229231
String nonceHash = createHash(nonce);
232+
attributes.put(OidcParameterNames.NONCE, nonce);
230233
additionalParameters.put(OidcParameterNames.NONCE, nonceHash);
231-
} catch (NoSuchAlgorithmException ignored) {
232-
}
234+
} catch (NoSuchAlgorithmException e) { }
233235
}
234236

235237
/**
@@ -245,7 +247,7 @@ private void addNonceParameters(Map<String, Object> attributes, Map<String, Obje
245247
* @see <a target="_blank" href="https://tools.ietf.org/html/rfc7636#section-4.2">4.2. Client Creates the Code Challenge</a>
246248
*/
247249
private void addPkceParameters(Map<String, Object> attributes, Map<String, Object> additionalParameters) {
248-
String codeVerifier = this.stringKeyGenerator.generateKey();
250+
String codeVerifier = this.secureKeyGenerator.generateKey();
249251
attributes.put(PkceParameterNames.CODE_VERIFIER, codeVerifier);
250252
try {
251253
String codeChallenge = createHash(codeVerifier);
@@ -256,7 +258,7 @@ private void addPkceParameters(Map<String, Object> attributes, Map<String, Objec
256258
}
257259
}
258260

259-
private String createHash(String value) throws NoSuchAlgorithmException {
261+
private static String createHash(String value) throws NoSuchAlgorithmException {
260262
MessageDigest md = MessageDigest.getInstance("SHA-256");
261263
byte[] digest = md.digest(value.getBytes(StandardCharsets.US_ASCII));
262264
return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);

0 commit comments

Comments
 (0)