Skip to content

Commit

Permalink
Validate authz parameters against original authz request
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 3f0730a commit 95b7d9e
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 18 deletions.
25 changes: 15 additions & 10 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 @@ -29,16 +29,7 @@
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;
import org.springframework.security.oauth2.common.exceptions.InvalidClientException;
import org.springframework.security.oauth2.common.exceptions.InvalidRequestException;
import org.springframework.security.oauth2.common.exceptions.OAuth2Exception;
import org.springframework.security.oauth2.common.exceptions.RedirectMismatchException;
import org.springframework.security.oauth2.common.exceptions.UnapprovedClientAuthenticationException;
import org.springframework.security.oauth2.common.exceptions.UnauthorizedClientException;
import org.springframework.security.oauth2.common.exceptions.UnsupportedResponseTypeException;
import org.springframework.security.oauth2.common.exceptions.UserDeniedAuthorizationException;
import org.springframework.security.oauth2.common.exceptions.*;
import org.springframework.security.oauth2.common.util.OAuth2Utils;
import org.springframework.security.oauth2.provider.AuthorizationRequest;
import org.springframework.security.oauth2.provider.ClientDetails;
Expand Down Expand Up @@ -95,6 +86,7 @@
import static org.cloudfoundry.identity.uaa.util.JsonUtils.hasText;
import static org.cloudfoundry.identity.uaa.util.UaaUrlUtils.addFragmentComponent;
import static org.cloudfoundry.identity.uaa.util.UaaUrlUtils.addQueryParameter;
import static org.springframework.security.oauth2.common.util.OAuth2Utils.SCOPE_PREFIX;

/**
* Authorization endpoint that returns id_token's if requested.
Expand Down Expand Up @@ -369,6 +361,19 @@ public View approveOrDeny(@RequestParam Map<String, String> approvalParameters,
throw new InvalidRequestException("Changes were detected from the original authorization request.");
}

for (String approvalParameter : approvalParameters.keySet()) {
if (approvalParameter.startsWith(SCOPE_PREFIX)) {
String scope = approvalParameters.get(approvalParameter).substring(SCOPE_PREFIX.length());
Set<String> originalScopes = (Set<String>) originalAuthorizationRequest.get("scope");
if (!originalScopes.contains(scope)) {
sessionStatus.setComplete();

return new RedirectView(getUnsuccessfulRedirect(authorizationRequest,
new InvalidScopeException("The requested scopes are invalid. Please use valid scope names in the request."), false), false, true, false);
}
}
}

try {
Set<String> responseTypes = authorizationRequest.getResponseTypes();
String grantType = deriveGrantTypeFromResponseType(responseTypes);
Expand Down
23 changes: 23 additions & 0 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 @@ -19,12 +19,15 @@
import org.springframework.security.oauth2.provider.code.AuthorizationCodeServices;
import org.springframework.web.bind.support.SimpleSessionStatus;
import org.springframework.web.servlet.View;
import org.springframework.web.servlet.view.RedirectView;

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.CoreMatchers.not;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
Expand Down Expand Up @@ -174,6 +177,8 @@ public void approveUnmodifiedRequest() {

View view = uaaAuthorizationEndpoint.approveOrDeny(approvalParameters, model, sessionStatus, principal);
assertThat(view, notNullValue());
assertThat(view, instanceOf(RedirectView.class));
assertThat(((RedirectView)view).getUrl(), not(containsString("error=invalid_scope")));
}

@Test(expected = InvalidRequestException.class)
Expand Down Expand Up @@ -282,6 +287,24 @@ public void testApproveWithModifiedAuthorities() {
uaaAuthorizationEndpoint.approveOrDeny(approvalParameters, model, sessionStatus, principal);
}

@Test
public void testApproveWithModifiedApprovalParameters() {
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));

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

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

assertThat(view, instanceOf(RedirectView.class));
assertThat(((RedirectView)view).getUrl(), containsString("error=invalid_scope"));
}

private AuthorizationRequest getAuthorizationRequest(String clientId, String redirectUri, String state,
String scope, Set<String> responseTypes) {
HashMap<String, String> parameters = new HashMap<>();
Expand Down
46 changes: 38 additions & 8 deletions uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/approvals/ApprovalsMockMvcTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,7 @@
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrlPattern;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;

public class ApprovalsMockMvcTests extends AbstractTokenMockMvcTests {

Expand Down Expand Up @@ -122,7 +119,7 @@ public void test_oauth_authorize_without_csrf() throws Exception {
post("/oauth/authorize")
.session(session)
.param(USER_OAUTH_APPROVAL, "true")
.param("scope.0","test.scope1")
.param("scope.0","scope.test.scope1")
)
.andExpect(status().is4xxClientError());

Expand All @@ -132,7 +129,7 @@ public void test_oauth_authorize_without_csrf() throws Exception {
.with(cookieCsrf().useInvalidToken())
.session(session)
.param(USER_OAUTH_APPROVAL, "true")
.param("scope.0","test.scope1")
.param("scope.0","scope.test.scope1")
)
.andExpect(status().is4xxClientError());

Expand All @@ -145,8 +142,8 @@ public void test_oauth_authorize_without_csrf() throws Exception {
.with(cookieCsrf())
.session(session)
.param(USER_OAUTH_APPROVAL, "true")
.param("scope.0","test.scope1")
.param("scope.1","test.scope2")
.param("scope.0","scope.test.scope1")
.param("scope.1","scope.test.scope2")
)
.andExpect(status().isFound())
.andExpect(redirectedUrlPattern("**/*code=*"));
Expand All @@ -163,6 +160,39 @@ public void test_oauth_authorize_without_csrf() throws Exception {
.andExpect(status().isFound()); //approval page no longer showing up
}

@Test
public void test_oauth_authorize_modified_scope() throws Exception {
String state = generator.generate();

MockHttpSession session = getAuthenticatedSession(user1);
getMockMvc().perform(
get("/oauth/authorize")
.session(session)
.param(RESPONSE_TYPE, "code")
.param(STATE, state)
.param(CLIENT_ID, client1.getClientId()))
.andExpect(status().isOk()); //200 means the approvals page


assertNotNull(session.getAttribute("authorizationRequest"));
assertNotNull(session.getAttribute("org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST"));

getMockMvc().perform(
post("/oauth/authorize")
.with(cookieCsrf())
.session(session)
.param(USER_OAUTH_APPROVAL, "true")
.param("scope.0","scope.different.scope")
.param("scope.1","scope.test.scope2")
)
.andDo(print())
.andExpect(status().is3xxRedirection())
.andExpect(redirectedUrlPattern("http://test.example.org/redirect?error=invalid_scope&error_description=The%20requested%20scopes%20are%20invalid.%20Please%20use%20valid%20scope%20names%20in%20the%20request*"));

assertNull(session.getAttribute("authorizationRequest"));
assertNull(session.getAttribute("org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint.ORIGINAL_AUTHORIZATION_REQUEST"));
}

@Test
public void test_get_approvals() throws Exception {
test_oauth_authorize_without_csrf();
Expand Down

0 comments on commit 95b7d9e

Please sign in to comment.