Skip to content
This repository was archived by the owner on May 31, 2022. It is now read-only.

Commit ddd65cd

Browse files
committed
Validate authorization request on approval
Fixes gh-1481
1 parent 4a7385d commit ddd65cd

File tree

2 files changed

+237
-47
lines changed

2 files changed

+237
-47
lines changed

spring-security-oauth2/src/main/java/org/springframework/security/oauth2/provider/endpoint/AuthorizationEndpoint.java

Lines changed: 103 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
55
* the License. You may obtain a copy of the License at
@@ -10,23 +10,14 @@
1010
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
1111
* specific language governing permissions and limitations under the License.
1212
*/
13-
1413
package org.springframework.security.oauth2.provider.endpoint;
1514

16-
import java.net.URI;
17-
import java.security.Principal;
18-
import java.util.Collections;
19-
import java.util.Date;
20-
import java.util.HashMap;
21-
import java.util.LinkedHashMap;
22-
import java.util.Map;
23-
import java.util.Set;
24-
2515
import org.springframework.http.ResponseEntity;
2616
import org.springframework.security.access.AccessDeniedException;
2717
import org.springframework.security.authentication.InsufficientAuthenticationException;
2818
import org.springframework.security.core.Authentication;
2919
import org.springframework.security.core.AuthenticationException;
20+
import org.springframework.security.core.GrantedAuthority;
3021
import org.springframework.security.oauth2.common.OAuth2AccessToken;
3122
import org.springframework.security.oauth2.common.exceptions.BadClientCredentialsException;
3223
import org.springframework.security.oauth2.common.exceptions.ClientAuthenticationException;
@@ -51,6 +42,7 @@
5142
import org.springframework.security.oauth2.provider.code.InMemoryAuthorizationCodeServices;
5243
import org.springframework.security.oauth2.provider.implicit.ImplicitTokenRequest;
5344
import org.springframework.security.oauth2.provider.request.DefaultOAuth2RequestValidator;
45+
import org.springframework.util.ObjectUtils;
5446
import org.springframework.util.StringUtils;
5547
import org.springframework.web.HttpSessionRequiredException;
5648
import org.springframework.web.bind.annotation.ExceptionHandler;
@@ -68,6 +60,16 @@
6860
import org.springframework.web.util.UriComponents;
6961
import org.springframework.web.util.UriComponentsBuilder;
7062

63+
import java.net.URI;
64+
import java.security.Principal;
65+
import java.util.Collections;
66+
import java.util.Date;
67+
import java.util.HashMap;
68+
import java.util.HashSet;
69+
import java.util.LinkedHashMap;
70+
import java.util.Map;
71+
import java.util.Set;
72+
7173
/**
7274
* <p>
7375
* Implementation of the Authorization Endpoint from the OAuth2 specification. Accepts authorization requests, and
@@ -86,8 +88,11 @@
8688
*
8789
*/
8890
@FrameworkEndpoint
89-
@SessionAttributes("authorizationRequest")
91+
@SessionAttributes({AuthorizationEndpoint.AUTHORIZATION_REQUEST_ATTR_NAME, AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST_ATTR_NAME})
9092
public class AuthorizationEndpoint extends AbstractEndpoint {
93+
static final String AUTHORIZATION_REQUEST_ATTR_NAME = "authorizationRequest";
94+
95+
static final String ORIGINAL_AUTHORIZATION_REQUEST_ATTR_NAME = "org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST";
9196

9297
private AuthorizationCodeServices authorizationCodeServices = new InMemoryAuthorizationCodeServices();
9398

@@ -174,10 +179,10 @@ public ModelAndView authorize(Map<String, Object> model, @RequestParam Map<Strin
174179
}
175180
}
176181

177-
// Place auth request into the model so that it is stored in the session
178-
// for approveOrDeny to use. That way we make sure that auth request comes from the session,
179-
// so any auth request parameters passed to approveOrDeny will be ignored and retrieved from the session.
180-
model.put("authorizationRequest", authorizationRequest);
182+
// Store authorizationRequest AND an immutable Map of authorizationRequest in session
183+
// which will be used to validate against in approveOrDeny()
184+
model.put(AUTHORIZATION_REQUEST_ATTR_NAME, authorizationRequest);
185+
model.put(ORIGINAL_AUTHORIZATION_REQUEST_ATTR_NAME, unmodifiableMap(authorizationRequest));
181186

182187
return getUserApprovalPageResponse(model, authorizationRequest, (Authentication) principal);
183188

@@ -189,6 +194,33 @@ public ModelAndView authorize(Map<String, Object> model, @RequestParam Map<Strin
189194

190195
}
191196

197+
Map<String, Object> unmodifiableMap(AuthorizationRequest authorizationRequest) {
198+
Map<String, Object> authorizationRequestMap = new HashMap<String, Object>();
199+
200+
authorizationRequestMap.put(OAuth2Utils.CLIENT_ID, authorizationRequest.getClientId());
201+
authorizationRequestMap.put(OAuth2Utils.STATE, authorizationRequest.getState());
202+
authorizationRequestMap.put(OAuth2Utils.REDIRECT_URI, authorizationRequest.getRedirectUri());
203+
if (authorizationRequest.getResponseTypes() != null) {
204+
authorizationRequestMap.put(OAuth2Utils.RESPONSE_TYPE,
205+
Collections.unmodifiableSet(new HashSet<String>(authorizationRequest.getResponseTypes())));
206+
}
207+
if (authorizationRequest.getScope() != null) {
208+
authorizationRequestMap.put(OAuth2Utils.SCOPE,
209+
Collections.unmodifiableSet(new HashSet<String>(authorizationRequest.getScope())));
210+
}
211+
authorizationRequestMap.put("approved", authorizationRequest.isApproved());
212+
if (authorizationRequest.getResourceIds() != null) {
213+
authorizationRequestMap.put("resourceIds",
214+
Collections.unmodifiableSet(new HashSet<String>(authorizationRequest.getResourceIds())));
215+
}
216+
if (authorizationRequest.getAuthorities() != null) {
217+
authorizationRequestMap.put("authorities",
218+
Collections.unmodifiableSet(new HashSet<GrantedAuthority>(authorizationRequest.getAuthorities())));
219+
}
220+
221+
return Collections.unmodifiableMap(authorizationRequestMap);
222+
}
223+
192224
@RequestMapping(value = "/oauth/authorize", method = RequestMethod.POST, params = OAuth2Utils.USER_OAUTH_APPROVAL)
193225
public View approveOrDeny(@RequestParam Map<String, String> approvalParameters, Map<String, ?> model,
194226
SessionStatus sessionStatus, Principal principal) {
@@ -199,13 +231,20 @@ public View approveOrDeny(@RequestParam Map<String, String> approvalParameters,
199231
"User must be authenticated with Spring Security before authorizing an access token.");
200232
}
201233

202-
AuthorizationRequest authorizationRequest = (AuthorizationRequest) model.get("authorizationRequest");
234+
AuthorizationRequest authorizationRequest = (AuthorizationRequest) model.get(AUTHORIZATION_REQUEST_ATTR_NAME);
203235

204236
if (authorizationRequest == null) {
205237
sessionStatus.setComplete();
206238
throw new InvalidRequestException("Cannot approve uninitialized authorization request.");
207239
}
208240

241+
// Check to ensure the Authorization Request was not modified during the user approval step
242+
@SuppressWarnings("unchecked")
243+
Map<String, Object> originalAuthorizationRequest = (Map<String, Object>) model.get(ORIGINAL_AUTHORIZATION_REQUEST_ATTR_NAME);
244+
if (isAuthorizationRequestModified(authorizationRequest, originalAuthorizationRequest)) {
245+
throw new InvalidRequestException("Changes were detected from the original authorization request.");
246+
}
247+
209248
try {
210249
Set<String> responseTypes = authorizationRequest.getResponseTypes();
211250

@@ -238,6 +277,52 @@ public View approveOrDeny(@RequestParam Map<String, String> approvalParameters,
238277

239278
}
240279

280+
private boolean isAuthorizationRequestModified(
281+
AuthorizationRequest authorizationRequest, Map<String, Object> originalAuthorizationRequest) {
282+
if (!ObjectUtils.nullSafeEquals(
283+
authorizationRequest.getClientId(),
284+
originalAuthorizationRequest.get(OAuth2Utils.CLIENT_ID))) {
285+
return true;
286+
}
287+
if (!ObjectUtils.nullSafeEquals(
288+
authorizationRequest.getState(),
289+
originalAuthorizationRequest.get(OAuth2Utils.STATE))) {
290+
return true;
291+
}
292+
if (!ObjectUtils.nullSafeEquals(
293+
authorizationRequest.getRedirectUri(),
294+
originalAuthorizationRequest.get(OAuth2Utils.REDIRECT_URI))) {
295+
return true;
296+
}
297+
if (!ObjectUtils.nullSafeEquals(
298+
authorizationRequest.getResponseTypes(),
299+
originalAuthorizationRequest.get(OAuth2Utils.RESPONSE_TYPE))) {
300+
return true;
301+
}
302+
if (!ObjectUtils.nullSafeEquals(
303+
authorizationRequest.getScope(),
304+
originalAuthorizationRequest.get(OAuth2Utils.SCOPE))) {
305+
return true;
306+
}
307+
if (!ObjectUtils.nullSafeEquals(
308+
authorizationRequest.isApproved(),
309+
originalAuthorizationRequest.get("approved"))) {
310+
return true;
311+
}
312+
if (!ObjectUtils.nullSafeEquals(
313+
authorizationRequest.getResourceIds(),
314+
originalAuthorizationRequest.get("resourceIds"))) {
315+
return true;
316+
}
317+
if (!ObjectUtils.nullSafeEquals(
318+
authorizationRequest.getAuthorities(),
319+
originalAuthorizationRequest.get("authorities"))) {
320+
return true;
321+
}
322+
323+
return false;
324+
}
325+
241326
// We need explicit approval from the user.
242327
private ModelAndView getUserApprovalPageResponse(Map<String, Object> model,
243328
AuthorizationRequest authorizationRequest, Authentication principal) {
@@ -529,7 +614,7 @@ private AuthorizationRequest getAuthorizationRequestForError(ServletWebRequest w
529614

530615
// If it's already there then we are in the approveOrDeny phase and we can use the saved request
531616
AuthorizationRequest authorizationRequest = (AuthorizationRequest) sessionAttributeStore.retrieveAttribute(
532-
webRequest, "authorizationRequest");
617+
webRequest, AUTHORIZATION_REQUEST_ATTR_NAME);
533618
if (authorizationRequest != null) {
534619
return authorizationRequest;
535620
}

0 commit comments

Comments
 (0)