Skip to content

Commit

Permalink
Validate authorization request on approval
Browse files Browse the repository at this point in the history 
Signed-off-by: Jaskanwal Pawar <jpawar@pivotal.io>
Co-authored-by: Jaskanwal Pawar <jpawar@pivotal.io>
  • Loading branch information
@DennisDenuto
DennisDenuto and Jaskanwal Pawar committed Oct 24, 2018
1 parent 46c122d commit 3f0730a
Show file tree
Hide file tree
Showing 4 changed files with 281 additions and 14 deletions.
93 changes: 85 additions & 8 deletions server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationEndpoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.springframework.security.authentication.InsufficientAuthenticationException;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.oauth2.common.OAuth2AccessToken;
import org.springframework.security.oauth2.common.exceptions.BadClientCredentialsException;
import org.springframework.security.oauth2.common.exceptions.ClientAuthenticationException;
Expand Down Expand Up @@ -58,6 +59,7 @@
import org.springframework.security.web.AuthenticationEntryPoint;
import org.springframework.security.web.util.UrlUtils;
import org.springframework.stereotype.Controller;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;
import org.springframework.web.HttpSessionRequiredException;
import org.springframework.web.bind.annotation.ExceptionHandler;
Expand All @@ -82,13 +84,7 @@
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.security.Principal;
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.*;

import static java.util.Arrays.stream;
import static java.util.Collections.EMPTY_SET;
Expand All @@ -108,7 +104,7 @@
* https://github.com/fhanik/spring-security-oauth/compare/feature/extendable-redirect-generator?expand=1
*/
@Controller
@SessionAttributes("authorizationRequest")
@SessionAttributes({"authorizationRequest", "org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST"})
public class UaaAuthorizationEndpoint extends AbstractEndpoint implements AuthenticationEntryPoint {

private AuthorizationCodeServices authorizationCodeServices = new InMemoryAuthorizationCodeServices();
Expand Down Expand Up @@ -233,6 +229,8 @@ public ModelAndView authorize(Map<String, Object> model,
// so any auth request parameters passed to approveOrDeny will be ignored and retrieved from the session.
model.put("authorizationRequest", authorizationRequest);
model.put("original_uri", UrlUtils.buildFullRequestUrl(request));
model.put("org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST", unmodifiableMap(authorizationRequest));

return getUserApprovalPageResponse(model, authorizationRequest, (Authentication) principal);
}
} catch (RedirectMismatchException e) {
Expand Down Expand Up @@ -317,6 +315,36 @@ private ModelAndView switchIdp(Map<String, Object> model, ClientDetails client,
return new ModelAndView("switch_idp", model, HttpStatus.UNAUTHORIZED);
}

Map<String, Object> unmodifiableMap(AuthorizationRequest authorizationRequest) {
Map<String, Object> authorizationRequestMap = new HashMap<>();

authorizationRequestMap.put(OAuth2Utils.CLIENT_ID, authorizationRequest.getClientId());
authorizationRequestMap.put(OAuth2Utils.STATE, authorizationRequest.getState());
authorizationRequestMap.put(OAuth2Utils.REDIRECT_URI, authorizationRequest.getRedirectUri());

if (authorizationRequest.getResponseTypes() != null) {
authorizationRequestMap.put(OAuth2Utils.RESPONSE_TYPE,
Collections.unmodifiableSet(new HashSet<>(authorizationRequest.getResponseTypes())));
}
if (authorizationRequest.getScope() != null) {
authorizationRequestMap.put(OAuth2Utils.SCOPE,
Collections.unmodifiableSet(new HashSet<>(authorizationRequest.getScope())));
}

authorizationRequestMap.put("approved", authorizationRequest.isApproved());

if (authorizationRequest.getResourceIds() != null) {
authorizationRequestMap.put("resourceIds",
Collections.unmodifiableSet(new HashSet<>(authorizationRequest.getResourceIds())));
}
if (authorizationRequest.getAuthorities() != null) {
authorizationRequestMap.put("authorities",
Collections.unmodifiableSet(new HashSet<GrantedAuthority>(authorizationRequest.getAuthorities())));
}

return authorizationRequestMap;
}

@RequestMapping(value = "/oauth/authorize", method = RequestMethod.POST, params = OAuth2Utils.USER_OAUTH_APPROVAL)
public View approveOrDeny(@RequestParam Map<String, String> approvalParameters, Map<String, ?> model,
SessionStatus sessionStatus, Principal principal) {
Expand All @@ -334,6 +362,13 @@ public View approveOrDeny(@RequestParam Map<String, String> approvalParameters,
throw new InvalidRequestException("Cannot approve uninitialized authorization request.");
}

// Check to ensure the Authorization Request was not modified during the user approval step
@SuppressWarnings("unchecked")
Map<String, Object> originalAuthorizationRequest = (Map<String, Object>) model.get("org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST");
if (isAuthorizationRequestModified(authorizationRequest, originalAuthorizationRequest)) {
throw new InvalidRequestException("Changes were detected from the original authorization request.");
}

try {
Set<String> responseTypes = authorizationRequest.getResponseTypes();
String grantType = deriveGrantTypeFromResponseType(responseTypes);
Expand Down Expand Up @@ -370,6 +405,48 @@ public View approveOrDeny(@RequestParam Map<String, String> approvalParameters,

}

private boolean isAuthorizationRequestModified(AuthorizationRequest authorizationRequest, Map<String, Object> originalAuthorizationRequest) {
if (!ObjectUtils.nullSafeEquals(
authorizationRequest.getClientId(),
originalAuthorizationRequest.get(OAuth2Utils.CLIENT_ID))) {
return true;
}
if (!ObjectUtils.nullSafeEquals(
authorizationRequest.getState(),
originalAuthorizationRequest.get(OAuth2Utils.STATE))) {
return true;
}
if (!ObjectUtils.nullSafeEquals(
authorizationRequest.getRedirectUri(),
originalAuthorizationRequest.get(OAuth2Utils.REDIRECT_URI))) {
return true;
}
if (!ObjectUtils.nullSafeEquals(
authorizationRequest.getResponseTypes(),
originalAuthorizationRequest.get(OAuth2Utils.RESPONSE_TYPE))) {
return true;
}
if (!ObjectUtils.nullSafeEquals(
authorizationRequest.isApproved(),
originalAuthorizationRequest.get("approved"))) {
return true;
}
if (!ObjectUtils.nullSafeEquals(
authorizationRequest.getResourceIds(),
originalAuthorizationRequest.get("resourceIds"))) {
return true;
}
if (!ObjectUtils.nullSafeEquals(
authorizationRequest.getAuthorities(),
originalAuthorizationRequest.get("authorities"))) {
return true;
}

return !ObjectUtils.nullSafeEquals(
authorizationRequest.getScope(),
originalAuthorizationRequest.get(OAuth2Utils.SCOPE));
}

protected String deriveGrantTypeFromResponseType(Set<String> responseTypes) {
if (responseTypes.contains("token")) {
return GRANT_TYPE_IMPLICIT;
Expand Down
166 changes: 160 additions & 6 deletions server/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationEndpointTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,26 @@
import org.cloudfoundry.identity.uaa.oauth.token.CompositeToken;
import org.junit.Before;
import org.junit.Test;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.authority.AuthorityUtils;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.oauth2.common.exceptions.InvalidRequestException;
import org.springframework.security.oauth2.common.util.OAuth2Utils;
import org.springframework.security.oauth2.provider.AuthorizationRequest;
import org.springframework.security.oauth2.provider.OAuth2Authentication;
import org.springframework.security.oauth2.provider.OAuth2Request;
import org.springframework.security.oauth2.provider.OAuth2RequestFactory;
import org.springframework.security.oauth2.provider.code.AuthorizationCodeServices;
import org.springframework.web.bind.support.SimpleSessionStatus;
import org.springframework.web.servlet.View;

import java.util.Calendar;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Set;
import java.util.*;

import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_AUTHORIZATION_CODE;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_IMPLICIT;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.mockito.ArgumentMatchers.any;
Expand All @@ -35,6 +40,10 @@ public class UaaAuthorizationEndpointTest {
private Set<String> responseTypes;
private OpenIdSessionStateCalculator openIdSessionStateCalculator;

private HashMap<String, Object> model = new HashMap<>();
private SimpleSessionStatus sessionStatus = new SimpleSessionStatus();
private UsernamePasswordAuthenticationToken principal = new UsernamePasswordAuthenticationToken("foo", "bar", Collections.singleton(new SimpleGrantedAuthority("ROLE_USER")));

@Before
public void setup() {
oAuth2RequestFactory = mock(OAuth2RequestFactory.class);
Expand All @@ -47,6 +56,7 @@ public void setup() {
responseTypes = new HashSet<>();

when(openIdSessionStateCalculator.calculate("userid", null, "http://example.com")).thenReturn("opbshash");
when(authorizationCodeServices.createAuthorizationCode(any(OAuth2Authentication.class))).thenReturn("code");
}


Expand Down Expand Up @@ -150,4 +160,148 @@ public void buildRedirectURI_includesSessionStateForPromptEqualsNone() {

assertThat(result, containsString("session_state=opbshash"));
}
}

@Test
public void approveUnmodifiedRequest() {
AuthorizationRequest authorizationRequest = getAuthorizationRequest("foo", "http://anywhere.com", "state-1234", "read", Collections.singleton("code"));
model.put("authorizationRequest", authorizationRequest);
model.put("org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST", uaaAuthorizationEndpoint.unmodifiableMap(authorizationRequest));

Map<String, String> approvalParameters = new HashMap<>();
approvalParameters.put("user_oauth_approval", "true");

when(authorizationCodeServices.createAuthorizationCode(any(OAuth2Authentication.class))).thenReturn("code");

View view = uaaAuthorizationEndpoint.approveOrDeny(approvalParameters, model, sessionStatus, principal);
assertThat(view, notNullValue());
}

@Test(expected = InvalidRequestException.class)
public void testApproveWithModifiedScope() {
AuthorizationRequest authorizationRequest = getAuthorizationRequest(
"foo", "http://anywhere.com", "state-1234", "read", Collections.singleton("code"));
model.put("authorizationRequest", authorizationRequest);
model.put("org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST", uaaAuthorizationEndpoint.unmodifiableMap(authorizationRequest));

authorizationRequest.setScope(Arrays.asList("read", "write")); // Modify authorization request
Map<String, String> approvalParameters = new HashMap<>();
approvalParameters.put("user_oauth_approval", "true");

uaaAuthorizationEndpoint.approveOrDeny(approvalParameters, model, sessionStatus, principal);
}

@Test(expected = InvalidRequestException.class)
public void testApproveWithModifiedClientId() {
AuthorizationRequest authorizationRequest = getAuthorizationRequest(
"foo", "http://anywhere.com", "state-1234", "read", Collections.singleton("code"));
model.put("authorizationRequest", authorizationRequest);
model.put("org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST", uaaAuthorizationEndpoint.unmodifiableMap(authorizationRequest));
authorizationRequest.setClientId("bar"); // Modify authorization request
Map<String, String> approvalParameters = new HashMap<>();
approvalParameters.put("user_oauth_approval", "true");

uaaAuthorizationEndpoint.approveOrDeny(approvalParameters, model, sessionStatus, principal);
}

@Test(expected = InvalidRequestException.class)
public void testApproveWithModifiedState() {
AuthorizationRequest authorizationRequest = getAuthorizationRequest(
"foo", "http://anywhere.com", "state-1234", "read", Collections.singleton("code"));
model.put("authorizationRequest", authorizationRequest);
model.put("org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST", uaaAuthorizationEndpoint.unmodifiableMap(authorizationRequest));
authorizationRequest.setState("state-5678"); // Modify authorization request
Map<String, String> approvalParameters = new HashMap<>();
approvalParameters.put("user_oauth_approval", "true");

uaaAuthorizationEndpoint.approveOrDeny(approvalParameters, model, sessionStatus, principal);
}

@Test(expected = InvalidRequestException.class)
public void testApproveWithModifiedRedirectUri() {
AuthorizationRequest authorizationRequest = getAuthorizationRequest(
"foo", "http://anywhere.com", "state-1234", "read", Collections.singleton("code"));
model.put("authorizationRequest", authorizationRequest);
model.put("org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST", uaaAuthorizationEndpoint.unmodifiableMap(authorizationRequest));
authorizationRequest.setRedirectUri("http://somewhere.com"); // Modify authorization request
Map<String, String> approvalParameters = new HashMap<>();
approvalParameters.put("user_oauth_approval", "true");

uaaAuthorizationEndpoint.approveOrDeny(approvalParameters, model, sessionStatus, principal);
}

@Test(expected = InvalidRequestException.class)
public void testApproveWithModifiedResponseTypes() {
AuthorizationRequest authorizationRequest = getAuthorizationRequest(
"foo", "http://anywhere.com", "state-1234", "read", Collections.singleton("code"));
model.put("authorizationRequest", authorizationRequest);
model.put("org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST", uaaAuthorizationEndpoint.unmodifiableMap(authorizationRequest));
authorizationRequest.setResponseTypes(Collections.singleton("implicit")); // Modify authorization request
Map<String, String> approvalParameters = new HashMap<>();
approvalParameters.put("user_oauth_approval", "true");

uaaAuthorizationEndpoint.approveOrDeny(approvalParameters, model, sessionStatus, principal);
}

@Test(expected = InvalidRequestException.class)
public void testApproveWithModifiedApproved() {
AuthorizationRequest authorizationRequest = getAuthorizationRequest(
"foo", "http://anywhere.com", "state-1234", "read", Collections.singleton("code"));
authorizationRequest.setApproved(false);
model.put("authorizationRequest", authorizationRequest);
model.put("org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST", uaaAuthorizationEndpoint.unmodifiableMap(authorizationRequest));
authorizationRequest.setApproved(true); // Modify authorization request
Map<String, String> approvalParameters = new HashMap<>();
approvalParameters.put("user_oauth_approval", "true");

uaaAuthorizationEndpoint.approveOrDeny(approvalParameters, model, sessionStatus, principal);
}

@Test(expected = InvalidRequestException.class)
public void testApproveWithModifiedResourceIds() {
AuthorizationRequest authorizationRequest = getAuthorizationRequest(
"foo", "http://anywhere.com", "state-1234", "read", Collections.singleton("code"));
model.put("authorizationRequest", authorizationRequest);
model.put("org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST", uaaAuthorizationEndpoint.unmodifiableMap(authorizationRequest));
authorizationRequest.setResourceIds(Collections.singleton("resource-other")); // Modify authorization request
Map<String, String> approvalParameters = new HashMap<>();
approvalParameters.put("user_oauth_approval", "true");

uaaAuthorizationEndpoint.approveOrDeny(approvalParameters, model, sessionStatus, principal);
}

@Test(expected = InvalidRequestException.class)
public void testApproveWithModifiedAuthorities() {
AuthorizationRequest authorizationRequest = getAuthorizationRequest(
"foo", "http://anywhere.com", "state-1234", "read", Collections.singleton("code"));
model.put("authorizationRequest", authorizationRequest);
model.put("org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST", uaaAuthorizationEndpoint.unmodifiableMap(authorizationRequest));
authorizationRequest.setAuthorities(AuthorityUtils.commaSeparatedStringToAuthorityList("authority-other")); // Modify authorization request
Map<String, String> approvalParameters = new HashMap<>();
approvalParameters.put("user_oauth_approval", "true");

uaaAuthorizationEndpoint.approveOrDeny(approvalParameters, model, sessionStatus, principal);
}

private AuthorizationRequest getAuthorizationRequest(String clientId, String redirectUri, String state,
String scope, Set<String> responseTypes) {
HashMap<String, String> parameters = new HashMap<>();
parameters.put(OAuth2Utils.CLIENT_ID, clientId);
if (redirectUri != null) {
parameters.put(OAuth2Utils.REDIRECT_URI, redirectUri);
}
if (state != null) {
parameters.put(OAuth2Utils.STATE, state);
}
if (scope != null) {
parameters.put(OAuth2Utils.SCOPE, scope);
}
if (responseTypes != null) {
parameters.put(OAuth2Utils.RESPONSE_TYPE, OAuth2Utils.formatParameterList(responseTypes));
}
return new AuthorizationRequest(parameters, Collections.emptyMap(),
parameters.get(OAuth2Utils.CLIENT_ID),
OAuth2Utils.parseParameterList(parameters.get(OAuth2Utils.SCOPE)), null, null, false,
parameters.get(OAuth2Utils.STATE), parameters.get(OAuth2Utils.REDIRECT_URI),
OAuth2Utils.parseParameterList(parameters.get(OAuth2Utils.RESPONSE_TYPE)));
}
}
Loading

0 comments on commit 3f0730a

Please sign in to comment.