From 1a081ea9a9f35d7697511dc32874fbd67366870a Mon Sep 17 00:00:00 2001 From: Filip Hanik Date: Thu, 1 Sep 2016 09:37:49 -0700 Subject: [PATCH 1/3] Add user_token as a valid grant type [#128319711] https://www.pivotaltracker.com/story/show/128319711 --- .../client/ClientAdminEndpointsValidator.java | 5 +- .../ZoneEndpointsClientDetailsValidator.java | 21 ++-- .../ClientAdminEndpointsValidatorTests.java | 21 +++- ...eEndpointsClientDetailsValidatorTests.java | 46 +++++--- .../mock/token/AbstractTokenMockMvcTests.java | 2 + .../mock/token/ListUserTokenMockMvcTests.java | 101 +++++++++++++----- .../uaa/mock/token/UserTokenMockMvcTests.java | 31 ++++++ 7 files changed, 172 insertions(+), 55 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java index 610ef4eed30..58b86667b10 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidator.java @@ -30,13 +30,16 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; + +import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_USER_TOKEN; + public class ClientAdminEndpointsValidator implements InitializingBean, ClientDetailsValidator { private final Log logger = LogFactory.getLog(getClass()); private static final Set VALID_GRANTS = new HashSet<>(Arrays.asList("implicit", "password", - "client_credentials", "authorization_code", "refresh_token")); + "client_credentials", "authorization_code", "refresh_token", GRANT_TYPE_USER_TOKEN)); private static final Collection NON_ADMIN_INVALID_GRANTS = new HashSet<>(Arrays.asList("password")); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/ZoneEndpointsClientDetailsValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/ZoneEndpointsClientDetailsValidator.java index 7f632d9fffe..e8ddf61dd29 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/ZoneEndpointsClientDetailsValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/ZoneEndpointsClientDetailsValidator.java @@ -1,27 +1,30 @@ package org.cloudfoundry.identity.uaa.zone; -import java.util.Collections; - import org.apache.commons.lang.StringUtils; import org.cloudfoundry.identity.uaa.client.ClientDetailsValidator; import org.cloudfoundry.identity.uaa.client.InvalidClientDetailsException; -import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.constants.OriginKeys; +import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.oauth2.provider.ClientDetails; import org.springframework.security.oauth2.provider.client.BaseClientDetails; +import java.util.Collections; + +import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_REFRESH_TOKEN; +import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_USER_TOKEN; + public class ZoneEndpointsClientDetailsValidator implements ClientDetailsValidator { - + private final String requiredScope; - + public ZoneEndpointsClientDetailsValidator(String requiredScope) { this.requiredScope = requiredScope; } @Override public ClientDetails validate(ClientDetails clientDetails, Mode mode) throws InvalidClientDetailsException { - + if (mode == Mode.CREATE) { if (!Collections.singleton("openid").equals(clientDetails.getScope())) { throw new InvalidClientDetailsException("only openid scope is allowed"); @@ -34,6 +37,8 @@ public ClientDetails validate(ClientDetails clientDetails, Mode mode) throws Inv } if (clientDetails.getAuthorizedGrantTypes().contains("client_credentials") || clientDetails.getAuthorizedGrantTypes().contains("authorization_code") || + clientDetails.getAuthorizedGrantTypes().contains(GRANT_TYPE_USER_TOKEN) || + clientDetails.getAuthorizedGrantTypes().contains(GRANT_TYPE_REFRESH_TOKEN) || clientDetails.getAuthorizedGrantTypes().contains("password")) { if (StringUtils.isBlank(clientDetails.getClientSecret())) { throw new InvalidClientDetailsException("client_secret cannot be blank"); @@ -42,12 +47,12 @@ public ClientDetails validate(ClientDetails clientDetails, Mode mode) throws Inv if (!Collections.singletonList(OriginKeys.UAA).equals(clientDetails.getAdditionalInformation().get(ClientConstants.ALLOWED_PROVIDERS))) { throw new InvalidClientDetailsException("only the internal IdP ('uaa') is allowed"); } - + BaseClientDetails validatedClientDetails = new BaseClientDetails(clientDetails); validatedClientDetails.setAdditionalInformation(clientDetails.getAdditionalInformation()); validatedClientDetails.setResourceIds(Collections.singleton("none")); validatedClientDetails.addAdditionalInformation(ClientConstants.CREATED_WITH, requiredScope); - return validatedClientDetails; + return validatedClientDetails; } else if (mode == Mode.MODIFY) { throw new IllegalStateException("This validator cannot be used for modification requests"); } else if (mode == Mode.DELETE) { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidatorTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidatorTests.java index afdbb133dcf..287d08663f9 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidatorTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpointsValidatorTests.java @@ -24,7 +24,9 @@ import java.util.Arrays; -import static org.junit.Assert.assertTrue; +import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_USER_TOKEN; +import static org.hamcrest.Matchers.containsString; +import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; @@ -42,10 +44,7 @@ public void createClient() throws Exception { client.setClientSecret("secret"); caller = new BaseClientDetails("caller","","","client_credentials","clients.write"); validator = new ClientAdminEndpointsValidator(); - } - @Test - public void testValidate_Should_Allow_Prefix_Names() throws Exception { QueryableResourceManager clientDetailsService = mock(QueryableResourceManager.class); SecurityContextAccessor accessor = mock(SecurityContextAccessor.class); when(accessor.isAdmin()).thenReturn(false); @@ -55,6 +54,18 @@ public void testValidate_Should_Allow_Prefix_Names() throws Exception { validator.setClientDetailsService(clientDetailsService); validator.setSecurityContextAccessor(accessor); + } + + @Test + public void test_validate_user_token_grant_type() throws Exception { + client.setAuthorizedGrantTypes(Arrays.asList(GRANT_TYPE_USER_TOKEN)); + validator.validate(client, true, true); + } + + + @Test + public void testValidate_Should_Allow_Prefix_Names() throws Exception { + client.setAuthorities(Arrays.asList(new SimpleGrantedAuthority("uaa.resource"))); validator.validate(client, true, true); client.setAuthorities(Arrays.asList(new SimpleGrantedAuthority(caller.getClientId()+".some.other.authority"))); @@ -63,7 +74,7 @@ public void testValidate_Should_Allow_Prefix_Names() throws Exception { validator.validate(client, true, true); fail(); } catch (InvalidClientDetailsException x) { - assertTrue(x.getMessage().contains("not an allowed authority")); + assertThat(x.getMessage(), containsString("not an allowed authority")); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/ZoneEndpointsClientDetailsValidatorTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/ZoneEndpointsClientDetailsValidatorTests.java index 05b987b5286..0943f717dc7 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/ZoneEndpointsClientDetailsValidatorTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/ZoneEndpointsClientDetailsValidatorTests.java @@ -1,19 +1,25 @@ package org.cloudfoundry.identity.uaa.oauth; -import static org.junit.Assert.*; - -import java.util.Collections; - +import org.cloudfoundry.identity.uaa.client.ClientDetailsValidator.Mode; import org.cloudfoundry.identity.uaa.client.InvalidClientDetailsException; -import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.constants.OriginKeys; -import org.cloudfoundry.identity.uaa.client.ClientDetailsValidator.Mode; import org.cloudfoundry.identity.uaa.zone.ZoneEndpointsClientDetailsValidator; import org.junit.Before; import org.junit.Test; import org.springframework.security.oauth2.provider.ClientDetails; import org.springframework.security.oauth2.provider.client.BaseClientDetails; +import java.util.Arrays; +import java.util.Collections; + +import static org.cloudfoundry.identity.uaa.oauth.client.ClientConstants.ALLOWED_PROVIDERS; +import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_REFRESH_TOKEN; +import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_USER_TOKEN; +import static org.hamcrest.Matchers.containsString; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + public class ZoneEndpointsClientDetailsValidatorTests { private ZoneEndpointsClientDetailsValidator zoneEndpointsClientDetailsValidator; @@ -27,43 +33,51 @@ public void setUp() throws Exception { public void testCreateLimitedClient() { BaseClientDetails clientDetails = new BaseClientDetails("valid-client", null, "openid", "authorization_code,password", "uaa.resource"); clientDetails.setClientSecret("secret"); - clientDetails.addAdditionalInformation(ClientConstants.ALLOWED_PROVIDERS, Collections.singletonList(OriginKeys.UAA)); + clientDetails.addAdditionalInformation(ALLOWED_PROVIDERS, Collections.singletonList(OriginKeys.UAA)); ClientDetails validatedClientDetails = zoneEndpointsClientDetailsValidator.validate(clientDetails, Mode.CREATE); assertEquals(clientDetails.getClientId(), validatedClientDetails.getClientId()); assertEquals(clientDetails.getScope(), validatedClientDetails.getScope()); assertEquals(clientDetails.getAuthorizedGrantTypes(), validatedClientDetails.getAuthorizedGrantTypes()); assertEquals(clientDetails.getAuthorities(), validatedClientDetails.getAuthorities()); assertEquals(Collections.singleton("none"), validatedClientDetails.getResourceIds()); - assertEquals(Collections.singletonList(OriginKeys.UAA), validatedClientDetails.getAdditionalInformation().get(ClientConstants.ALLOWED_PROVIDERS)); + assertEquals(Collections.singletonList(OriginKeys.UAA), validatedClientDetails.getAdditionalInformation().get(ALLOWED_PROVIDERS)); } - + @Test(expected = InvalidClientDetailsException.class) public void testCreateClientNoNameIsInvalid() { BaseClientDetails clientDetails = new BaseClientDetails("", null, "openid", "authorization_code", "uaa.resource"); clientDetails.setClientSecret("secret"); zoneEndpointsClientDetailsValidator.validate(clientDetails, Mode.CREATE); } - - @Test(expected = InvalidClientDetailsException.class) + + @Test public void testCreateClientNoSecretIsInvalid() { - ClientDetails clientDetails = new BaseClientDetails("client", null, "openid", "authorization_code", "uaa.resource"); - zoneEndpointsClientDetailsValidator.validate(clientDetails, Mode.CREATE); + for (String grantType : Arrays.asList("password", "client_credentials", "authorization_code", GRANT_TYPE_USER_TOKEN, GRANT_TYPE_REFRESH_TOKEN)) { + try { + BaseClientDetails clientDetails = new BaseClientDetails("client", null, "openid", grantType, "uaa.resource"); + clientDetails.addAdditionalInformation(ALLOWED_PROVIDERS, Collections.singletonList(OriginKeys.UAA)); + zoneEndpointsClientDetailsValidator.validate(clientDetails, Mode.CREATE); + fail("Grant type:"+grantType + " must require a secret"); + } catch (InvalidClientDetailsException e) { + assertThat(e.getMessage(), containsString("client_secret cannot be blank")); + } + } } @Test public void testCreateClientNoSecretForImplicitIsValid() { BaseClientDetails clientDetails = new BaseClientDetails("client", null, "openid", "implicit", "uaa.resource"); - clientDetails.addAdditionalInformation(ClientConstants.ALLOWED_PROVIDERS, Collections.singletonList(OriginKeys.UAA)); + clientDetails.addAdditionalInformation(ALLOWED_PROVIDERS, Collections.singletonList(OriginKeys.UAA)); ClientDetails validatedClientDetails = zoneEndpointsClientDetailsValidator.validate(clientDetails, Mode.CREATE); assertEquals(clientDetails.getAuthorizedGrantTypes(), validatedClientDetails.getAuthorizedGrantTypes()); } - + @Test(expected = InvalidClientDetailsException.class) public void testCreateAdminScopeClientIsInvalid() { ClientDetails clientDetails = new BaseClientDetails("admin-client", null, "uaa.admin", "authorization_code", "uaa.resource"); zoneEndpointsClientDetailsValidator.validate(clientDetails, Mode.CREATE); } - + @Test(expected = InvalidClientDetailsException.class) public void testCreateAdminAuthorityClientIsInvalid() { ClientDetails clientDetails = new BaseClientDetails("admin-client", null, "openid", "authorization_code", "uaa.admin"); diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/AbstractTokenMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/AbstractTokenMockMvcTests.java index d10c54ff3ed..3b980e2ef79 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/AbstractTokenMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/AbstractTokenMockMvcTests.java @@ -33,6 +33,7 @@ import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import org.cloudfoundry.identity.uaa.zone.IdentityZoneProvisioning; import org.junit.Before; +import org.springframework.security.oauth2.common.util.RandomValueStringGenerator; import org.springframework.security.oauth2.provider.client.BaseClientDetails; import org.springframework.security.oauth2.provider.client.JdbcClientDetailsService; import org.springframework.util.StringUtils; @@ -67,6 +68,7 @@ public abstract class AbstractTokenMockMvcTests extends InjectedMockContextTest protected IdentityProviderProvisioning identityProviderProvisioning; protected String adminToken; protected RevocableTokenProvisioning tokenProvisioning; + protected RandomValueStringGenerator generator = new RandomValueStringGenerator(); @Before public void setUpContext() throws Exception { diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/ListUserTokenMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/ListUserTokenMockMvcTests.java index 89c29191f68..246e25e7bc0 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/ListUserTokenMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/ListUserTokenMockMvcTests.java @@ -23,6 +23,7 @@ import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import org.springframework.security.oauth2.common.util.RandomValueStringGenerator; import org.springframework.security.oauth2.provider.ClientDetails; @@ -36,6 +37,7 @@ import java.util.List; import java.util.stream.Collectors; +import static org.cloudfoundry.identity.uaa.mock.util.MockMvcUtils.getClientCredentialsOAuthAccessToken; import static org.junit.Assert.assertTrue; import static org.springframework.http.HttpHeaders.AUTHORIZATION; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; @@ -49,6 +51,7 @@ public class ListUserTokenMockMvcTests extends AbstractTokenMockMvcTests { MultiValueMap tokensPerUser = new LinkedMultiValueMap<>(); MultiValueMap tokensPerClient = new LinkedMultiValueMap<>(); private String adminClientToken; + private String tokensListToken; private String clientWithUser1IdAsIdToken; @@ -56,11 +59,11 @@ public class ListUserTokenMockMvcTests extends AbstractTokenMockMvcTests { @Before public void createUsersAndClients() throws Exception { user1 = setUpUser(generator.generate(), "tokens.list,scim.read,scim.write", OriginKeys.UAA, IdentityZone.getUaa().getId()); - user2 = setUpUser(generator.generate(), "tokens.list,scim.read,scim.write", OriginKeys.UAA, IdentityZone.getUaa().getId()); - user3 = setUpUser(generator.generate(), "tokens.list,scim.read,scim.write", OriginKeys.UAA, IdentityZone.getUaa().getId()); + user2 = setUpUser(generator.generate(), "scim.read,scim.write", OriginKeys.UAA, IdentityZone.getUaa().getId()); + user3 = setUpUser(generator.generate(), "scim.read,scim.write", OriginKeys.UAA, IdentityZone.getUaa().getId()); client1 = setUpClients(generator.generate(), "", "tokens.list,scim.read","password,refresh_token", false); - client2 = setUpClients(generator.generate(), "", "tokens.list,scim.read","password,refresh_token", false); - client3 = setUpClients(generator.generate(), "", "tokens.list,scim.read","password,refresh_token", false); + client2 = setUpClients(generator.generate(), "", "scim.read","password,refresh_token", false); + client3 = setUpClients(generator.generate(), "", "scim.read","password,refresh_token", false); setUpClients(user1.getId(), "tokens.list", "tokens.list,scim.read","client_credentials,password,refresh_token", false); for (ScimUser user : Arrays.asList(user1, user2, user3)) { @@ -78,7 +81,7 @@ public void createUsersAndClients() throws Exception { tokensPerClient.add(client.getClientId(), token); } } - adminClientToken = MockMvcUtils.getClientCredentialsOAuthAccessToken( + adminClientToken = getClientCredentialsOAuthAccessToken( getMockMvc(), "admin", "adminsecret", @@ -86,7 +89,7 @@ public void createUsersAndClients() throws Exception { null, true); - clientWithUser1IdAsIdToken = MockMvcUtils.getClientCredentialsOAuthAccessToken( + clientWithUser1IdAsIdToken = getClientCredentialsOAuthAccessToken( getMockMvc(), user1.getId(), SECRET, @@ -94,15 +97,24 @@ public void createUsersAndClients() throws Exception { null, true); + ClientDetails tokenListClient = setUpClients(generator.generate(), + "tokens.list", + null, + "client_credentials", + false); + tokensListToken = getClientCredentialsOAuthAccessToken( + getMockMvc(), + tokenListClient.getClientId(), + SECRET, + null, + null, + true); + } @Test public void listUserTokenAsAdmin() throws Exception { - List tokens = getTokenList("/oauth/token/list/user/" + user1.getId(), - adminClientToken, - status().isOk()); - List tokenIds = getTokenIds(tokens); - validateTokens(tokenIds, tokensPerUser.get(user1.getId())); + listTokens("/oauth/token/list/user/" + user1.getId(), adminClientToken, tokensPerUser.get(user1.getId())); } protected void validateTokens(List actual, List expected) { @@ -117,13 +129,26 @@ protected List getTokenIds(List tokens) { } + @Test + @Ignore + public void listClientToken_with_TokensList_Scope() throws Exception { + for (String clientId : Arrays.asList(client1.getClientId(), client2.getClientId(), client3.getClientId())) { + listTokens("/oauth/token/list/client/" + clientId, tokensListToken, tokensPerClient.get(clientId)); + } + } + @Test public void listClientTokenAsAdmin() throws Exception { - List tokens = getTokenList("/oauth/token/list/client/" + client1.getClientId(), - adminClientToken, - status().isOk()); - List tokenIds = getTokenIds(tokens); - validateTokens(tokenIds, tokensPerClient.get(client1.getClientId())); + for (String clientId : Arrays.asList(client1.getClientId(), client2.getClientId(), client3.getClientId())) { + listTokens("/oauth/token/list/client/" + clientId, adminClientToken, tokensPerClient.get(clientId)); + } + } + + @Test + public void listClientTokenAs_Other_Client() throws Exception { + for (String clientId : Arrays.asList(client1.getClientId(), client2.getClientId(), client3.getClientId())) { + listTokens("/oauth/token/list/client/" + clientId, adminClientToken, tokensPerClient.get(clientId)); + } } @Test @@ -134,10 +159,15 @@ public void listUserTokenAsAnotherUser() throws Exception { } @Test + @Ignore public void listClientTokensAsAnotherClient() throws Exception { getTokenList("/oauth/token/list/client/" + client1.getClientId(), tokensPerClient.getFirst(client3.getClientId()), status().isForbidden()); + + getTokenList("/oauth/token/list/client/" + client1.getClientId(), + tokensListToken, + status().isOk()); } @Test @@ -148,21 +178,42 @@ public void listUserTokensAsAClient() throws Exception { } @Test - public void listUserTokens() throws Exception { - List tokens = getTokenList("/oauth/token/list/user/" + user1.getId(), - tokensPerUser.getFirst(user1.getId()), + @Ignore + public void listUserTokens_for_self() throws Exception { + String userId = user2.getId(); + listTokens("/oauth/token/list/user/" + userId, tokensPerUser.getFirst(userId), tokensPerUser.get(userId)); + } + + @Test + @Ignore + public void listUserTokens_for_someone_else() throws Exception { + + getTokenList("/oauth/token/list/user/" + user2.getId(), + tokensPerUser.getFirst(user1.getId()), + status().isOk()); + + getTokenList("/oauth/token/list/user/" + user1.getId(), + tokensPerUser.getFirst(user2.getId()), + status().isForbidden()); + } + + @Test + public void listUserTokens_using_TokensList_scope() throws Exception { + String userId = user1.getId(); + listTokens("/oauth/token/list/user/" + userId, tokensPerUser.getFirst(userId), tokensPerUser.get(userId)); + } + + protected void listTokens(String urlTemplate, String accessToken, List expectedTokenIds) throws Exception { + List tokens = getTokenList(urlTemplate, + accessToken, status().isOk()); List tokenIds = getTokenIds(tokens); - validateTokens(tokenIds, tokensPerUser.get(user1.getId())); + validateTokens(tokenIds, expectedTokenIds); } @Test public void listClientTokens() throws Exception { - List tokens = getTokenList("/oauth/token/list/client/" + client1.getClientId(), - tokensPerClient.getFirst(client1.getClientId()), - status().isOk()); - List tokenIds = getTokenIds(tokens); - validateTokens(tokenIds, tokensPerClient.get(client1.getClientId())); + listTokens("/oauth/token/list/client/" + client1.getClientId(), tokensPerClient.getFirst(client1.getClientId()), tokensPerClient.get(client1.getClientId())); } protected List getTokenList(String urlTemplate, diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/UserTokenMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/UserTokenMockMvcTests.java index b9ae7234f35..9de6c90d8dc 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/UserTokenMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/UserTokenMockMvcTests.java @@ -18,6 +18,7 @@ import com.fasterxml.jackson.core.type.TypeReference; import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.mock.util.MockMvcUtils; +import org.cloudfoundry.identity.uaa.oauth.token.TokenConstants; import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.junit.Test; @@ -157,4 +158,34 @@ public void test_invalid_grant_type() throws Exception { .andExpect(content().string(containsString("\"Unauthorized grant type: user_token\""))); } + @Test + public void test_create_client_with_user_token_grant() throws Exception { + String adminToken = MockMvcUtils.getClientCredentialsOAuthAccessToken( + getMockMvc(), + "admin", + "adminsecret", + "uaa.admin", + null, + true + ); + + BaseClientDetails client = new BaseClientDetails( + generator.generate(), + null, + "openid,uaa.user,tokens.", + TokenConstants.GRANT_TYPE_USER_TOKEN, + null + ); + client.setClientSecret(SECRET); + getMockMvc().perform( + post("/oauth/clients") + .header(HttpHeaders.AUTHORIZATION, "Bearer "+adminToken) + .accept(MediaType.APPLICATION_JSON) + .contentType(MediaType.APPLICATION_JSON_VALUE) + .content(JsonUtils.writeValueAsString(client)) + ) + .andExpect(status().isCreated()); + + } + } From 51c72abfd14026cf8ab2738c97bb4ca297b450a5 Mon Sep 17 00:00:00 2001 From: Filip Hanik Date: Thu, 1 Sep 2016 11:26:11 -0700 Subject: [PATCH 2/3] Implement token list access control rules [#128319711] https://www.pivotaltracker.com/story/show/128319711 --- .../uaa/oauth/TokenRevocationEndpoint.java | 37 +++++++++--- .../token/JdbcRevocableTokenProvisioning.java | 11 ++++ .../token/RevocableTokenProvisioning.java | 2 + .../JdbcRevocableTokenProvisioningTest.java | 38 ++++++++++++ .../webapp/WEB-INF/spring/oauth-endpoints.xml | 6 +- .../mock/token/ListUserTokenMockMvcTests.java | 60 +++++++++---------- 6 files changed, 112 insertions(+), 42 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/TokenRevocationEndpoint.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/TokenRevocationEndpoint.java index dbabfa48784..cc871e6b86b 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/TokenRevocationEndpoint.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/TokenRevocationEndpoint.java @@ -16,6 +16,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal; import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants; import org.cloudfoundry.identity.uaa.oauth.token.RevocableToken; import org.cloudfoundry.identity.uaa.oauth.token.RevocableTokenProvisioning; @@ -29,9 +30,11 @@ import org.springframework.security.oauth2.common.exceptions.OAuth2Exception; import org.springframework.security.oauth2.common.util.RandomValueStringGenerator; import org.springframework.security.oauth2.provider.NoSuchClientException; +import org.springframework.security.oauth2.provider.OAuth2Authentication; import org.springframework.security.oauth2.provider.client.BaseClientDetails; import org.springframework.security.oauth2.provider.error.DefaultWebResponseExceptionTranslator; import org.springframework.security.oauth2.provider.error.WebResponseExceptionTranslator; +import org.springframework.security.oauth2.provider.expression.OAuth2ExpressionUtils; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.PathVariable; @@ -87,18 +90,36 @@ public ResponseEntity revokeTokenById(@PathVariable String tokenId) { return new ResponseEntity<>(OK); } - @RequestMapping(value = "/oauth/token/list/user/{userId}", method = GET) - public ResponseEntity> listUserTokens(@PathVariable String userId) { - logger.debug("Listing revocable tokens for user:"+userId); - List result = tokenProvisioning.getUserTokens(userId); + @RequestMapping(value = "/oauth/token/list", method = GET) + public ResponseEntity> listUserTokens(OAuth2Authentication authentication) { + UaaPrincipal principal = (UaaPrincipal) authentication.getUserAuthentication().getPrincipal(); + String userId = principal.getId(); + String clientId = authentication.getOAuth2Request().getClientId(); + logger.debug("Listing revocable tokens access token userId:"+ userId +" clientId:"+ clientId); + List result = tokenProvisioning.getUserTokens(userId, clientId); return new ResponseEntity<>(result, OK); } + @RequestMapping(value = "/oauth/token/list/user/{userId}", method = GET) + public ResponseEntity> listUserTokens(@PathVariable String userId, OAuth2Authentication authentication) { + if (OAuth2ExpressionUtils.hasAnyScope(authentication, new String[] {"tokens.list", "uaa.admin"})) { + logger.debug("Listing revocable tokens for user:" + userId); + List result = tokenProvisioning.getUserTokens(userId); + return new ResponseEntity<>(result, OK); + } else { + return listUserTokens(authentication); + } + } + @RequestMapping(value = "/oauth/token/list/client/{clientId}", method = GET) - public ResponseEntity> listClientTokens(@PathVariable String clientId) { - logger.debug("Listing revocable tokens for client:"+clientId); - List result = tokenProvisioning.getClientTokens(clientId); - return new ResponseEntity<>(result, OK); + public ResponseEntity> listClientTokens(@PathVariable String clientId, OAuth2Authentication authentication) { + if (OAuth2ExpressionUtils.hasAnyScope(authentication, new String[] {"tokens.list", "uaa.admin"})) { + logger.debug("Listing revocable tokens for client:" + clientId); + List result = tokenProvisioning.getClientTokens(clientId); + return new ResponseEntity<>(result, OK); + } else { + return listUserTokens(authentication); + } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/JdbcRevocableTokenProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/JdbcRevocableTokenProvisioning.java index 26d4738f2db..43874b62c8e 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/JdbcRevocableTokenProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/JdbcRevocableTokenProvisioning.java @@ -25,6 +25,9 @@ import java.sql.SQLException; import java.util.List; import java.util.concurrent.atomic.AtomicLong; +import java.util.stream.Collectors; + +import static org.springframework.util.StringUtils.isEmpty; public class JdbcRevocableTokenProvisioning implements RevocableTokenProvisioning, SystemDeletable { @@ -139,6 +142,14 @@ public List getUserTokens(String userId) { return template.query(GET_BY_USER_QUERY, rowMapper, userId, IdentityZoneHolder.get().getId()); } + @Override + public List getUserTokens(String userId, String clientId) { + if (isEmpty(clientId)) { + throw new NullPointerException("Client ID can not be null when retrieving tokens."); + } + return getUserTokens(userId).stream().filter(r -> clientId.equals(r.getClientId())).collect(Collectors.toList()); + } + @Override public List getClientTokens(String clientId) { return template.query(GET_BY_CLIENT_QUERY, rowMapper, clientId, IdentityZoneHolder.get().getId()); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/RevocableTokenProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/RevocableTokenProvisioning.java index e73961b4249..79dea553963 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/RevocableTokenProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/RevocableTokenProvisioning.java @@ -20,6 +20,8 @@ public interface RevocableTokenProvisioning extends ResourceManager getUserTokens(String userId); + List getUserTokens(String userId, String clientId); + List getClientTokens(String clientId); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/JdbcRevocableTokenProvisioningTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/JdbcRevocableTokenProvisioningTest.java index 5198fab47e9..d001d9ec40d 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/JdbcRevocableTokenProvisioningTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/JdbcRevocableTokenProvisioningTest.java @@ -57,6 +57,7 @@ public class JdbcRevocableTokenProvisioningTest extends JdbcTestBase { public void createData() { createData("test-token-id", "test-user-id", "test-client-id"); } + public void createData(String tokenId, String userId, String clientId) { value = new char[100*1024]; Arrays.fill(value, 'X'); @@ -148,6 +149,39 @@ public void listUserTokens() throws Exception { listTokens(false); } + @Test(expected = NullPointerException.class) + public void listUserTokens_Null_ClientId() { + dao.getUserTokens("userid", null); + } + + @Test(expected = NullPointerException.class) + public void listUserTokens_Empty_ClientId() { + dao.getUserTokens("userid", ""); + } + + @Test + public void listUserTokenForClient() throws Exception { + String clientId = "test-client-id"; + String userId = "test-user-id"; + List expectedTokens = new ArrayList<>(); + int count = 37; + RandomValueStringGenerator generator = new RandomValueStringGenerator(36); + for (int i=0; i actualTokens = dao.getUserTokens(userId, clientId); + assertThat(actualTokens, containsInAnyOrder(expectedTokens.toArray())); + } + @Test public void listClientTokens() throws Exception { listTokens(true); @@ -170,6 +204,10 @@ public void listTokens(boolean client) throws Exception { expectedTokens.add(this.expected); } + //create a random record that should not show up + createData(generator.generate(), generator.generate(), generator.generate()); + insertToken(); + List actualTokens = client ? dao.getClientTokens(clientId) : dao.getUserTokens(userId); assertThat(actualTokens, containsInAnyOrder(expectedTokens.toArray())); } diff --git a/uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml b/uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml index 2034ec93e60..a06b24d060e 100755 --- a/uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml +++ b/uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml @@ -84,8 +84,10 @@ authentication-manager-ref="emptyAuthenticationManager" entry-point-ref="oauthAuthenticationEntryPoint" xmlns="http://www.springframework.org/schema/security" use-expressions="true"> - - + + + + diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/ListUserTokenMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/ListUserTokenMockMvcTests.java index 246e25e7bc0..84f56e355fd 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/ListUserTokenMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/ListUserTokenMockMvcTests.java @@ -23,7 +23,6 @@ import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.springframework.security.oauth2.common.util.RandomValueStringGenerator; import org.springframework.security.oauth2.provider.ClientDetails; @@ -41,12 +40,13 @@ import static org.junit.Assert.assertTrue; import static org.springframework.http.HttpHeaders.AUTHORIZATION; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; public class ListUserTokenMockMvcTests extends AbstractTokenMockMvcTests { - private ClientDetails client1, client2,client3; - private ScimUser user1, user2, user3; + private ClientDetails client1withTokensListScope, client2,client3; + private ScimUser user1withTokensListScope, user2, user3; private RandomValueStringGenerator generator = new RandomValueStringGenerator(); MultiValueMap tokensPerUser = new LinkedMultiValueMap<>(); MultiValueMap tokensPerClient = new LinkedMultiValueMap<>(); @@ -58,16 +58,16 @@ public class ListUserTokenMockMvcTests extends AbstractTokenMockMvcTests { @Before public void createUsersAndClients() throws Exception { - user1 = setUpUser(generator.generate(), "tokens.list,scim.read,scim.write", OriginKeys.UAA, IdentityZone.getUaa().getId()); + user1withTokensListScope = setUpUser(generator.generate(), "tokens.list,scim.read,scim.write", OriginKeys.UAA, IdentityZone.getUaa().getId()); user2 = setUpUser(generator.generate(), "scim.read,scim.write", OriginKeys.UAA, IdentityZone.getUaa().getId()); user3 = setUpUser(generator.generate(), "scim.read,scim.write", OriginKeys.UAA, IdentityZone.getUaa().getId()); - client1 = setUpClients(generator.generate(), "", "tokens.list,scim.read","password,refresh_token", false); + client1withTokensListScope = setUpClients(generator.generate(), "", "tokens.list,scim.read", "password,refresh_token", false); client2 = setUpClients(generator.generate(), "", "scim.read","password,refresh_token", false); client3 = setUpClients(generator.generate(), "", "scim.read","password,refresh_token", false); - setUpClients(user1.getId(), "tokens.list", "tokens.list,scim.read","client_credentials,password,refresh_token", false); + setUpClients(user1withTokensListScope.getId(), "tokens.list", "tokens.list,scim.read", "client_credentials,password,refresh_token", false); - for (ScimUser user : Arrays.asList(user1, user2, user3)) { - for (ClientDetails client : Arrays.asList(client1, client2, client3)) { + for (ScimUser user : Arrays.asList(user1withTokensListScope, user2, user3)) { + for (ClientDetails client : Arrays.asList(client1withTokensListScope, client2, client3)) { String token = MockMvcUtils.getUserOAuthAccessToken( getMockMvc(), client.getClientId(), @@ -91,7 +91,7 @@ public void createUsersAndClients() throws Exception { clientWithUser1IdAsIdToken = getClientCredentialsOAuthAccessToken( getMockMvc(), - user1.getId(), + user1withTokensListScope.getId(), SECRET, null, null, @@ -114,7 +114,13 @@ public void createUsersAndClients() throws Exception { @Test public void listUserTokenAsAdmin() throws Exception { - listTokens("/oauth/token/list/user/" + user1.getId(), adminClientToken, tokensPerUser.get(user1.getId())); + listTokens("/oauth/token/list/user/" + user1withTokensListScope.getId(), adminClientToken, tokensPerUser.get(user1withTokensListScope.getId())); + } + + @Test + public void listUserTokenAsSelf() throws Exception { + String user2Token = tokensPerUser.getFirst(user2.getId()); + listTokens("/oauth/token/list", user2Token, Arrays.asList(user2Token)); } protected void validateTokens(List actual, List expected) { @@ -130,76 +136,65 @@ protected List getTokenIds(List tokens) { } @Test - @Ignore public void listClientToken_with_TokensList_Scope() throws Exception { - for (String clientId : Arrays.asList(client1.getClientId(), client2.getClientId(), client3.getClientId())) { + for (String clientId : Arrays.asList(client1withTokensListScope.getClientId(), client2.getClientId(), client3.getClientId())) { listTokens("/oauth/token/list/client/" + clientId, tokensListToken, tokensPerClient.get(clientId)); } } @Test public void listClientTokenAsAdmin() throws Exception { - for (String clientId : Arrays.asList(client1.getClientId(), client2.getClientId(), client3.getClientId())) { + for (String clientId : Arrays.asList(client1withTokensListScope.getClientId(), client2.getClientId(), client3.getClientId())) { listTokens("/oauth/token/list/client/" + clientId, adminClientToken, tokensPerClient.get(clientId)); } } @Test public void listClientTokenAs_Other_Client() throws Exception { - for (String clientId : Arrays.asList(client1.getClientId(), client2.getClientId(), client3.getClientId())) { + for (String clientId : Arrays.asList(client1withTokensListScope.getClientId(), client2.getClientId(), client3.getClientId())) { listTokens("/oauth/token/list/client/" + clientId, adminClientToken, tokensPerClient.get(clientId)); } } @Test public void listUserTokenAsAnotherUser() throws Exception { - getTokenList("/oauth/token/list/user/" + user1.getId(), + getTokenList("/oauth/token/list/user/" + user1withTokensListScope.getId(), tokensPerUser.getFirst(user2.getId()), status().isForbidden()); } @Test - @Ignore public void listClientTokensAsAnotherClient() throws Exception { - getTokenList("/oauth/token/list/client/" + client1.getClientId(), + getTokenList("/oauth/token/list/client/" + client1withTokensListScope.getClientId(), tokensPerClient.getFirst(client3.getClientId()), status().isForbidden()); - getTokenList("/oauth/token/list/client/" + client1.getClientId(), + getTokenList("/oauth/token/list/client/" + client1withTokensListScope.getClientId(), tokensListToken, status().isOk()); } @Test - public void listUserTokensAsAClient() throws Exception { - getTokenList("/oauth/token/list/user/" + user1.getId(), - clientWithUser1IdAsIdToken, - status().isForbidden()); - } - - @Test - @Ignore public void listUserTokens_for_self() throws Exception { String userId = user2.getId(); - listTokens("/oauth/token/list/user/" + userId, tokensPerUser.getFirst(userId), tokensPerUser.get(userId)); + listTokens("/oauth/token/list/user/" + userId, tokensPerUser.getFirst(userId), Arrays.asList(tokensPerUser.getFirst(userId))); } @Test - @Ignore public void listUserTokens_for_someone_else() throws Exception { getTokenList("/oauth/token/list/user/" + user2.getId(), - tokensPerUser.getFirst(user1.getId()), + tokensPerUser.getFirst(user1withTokensListScope.getId()), status().isOk()); - getTokenList("/oauth/token/list/user/" + user1.getId(), + getTokenList("/oauth/token/list/user/" + user1withTokensListScope.getId(), tokensPerUser.getFirst(user2.getId()), status().isForbidden()); } @Test public void listUserTokens_using_TokensList_scope() throws Exception { - String userId = user1.getId(); + String userId = user1withTokensListScope.getId(); listTokens("/oauth/token/list/user/" + userId, tokensPerUser.getFirst(userId), tokensPerUser.get(userId)); } @@ -213,7 +208,7 @@ protected void listTokens(String urlTemplate, String accessToken, List e @Test public void listClientTokens() throws Exception { - listTokens("/oauth/token/list/client/" + client1.getClientId(), tokensPerClient.getFirst(client1.getClientId()), tokensPerClient.get(client1.getClientId())); + listTokens("/oauth/token/list/client/" + client1withTokensListScope.getClientId(), tokensPerClient.getFirst(client1withTokensListScope.getClientId()), tokensPerClient.get(client1withTokensListScope.getClientId())); } protected List getTokenList(String urlTemplate, @@ -225,6 +220,7 @@ protected List getTokenList(String urlTemplate, .header(AUTHORIZATION, "Bearer "+ accessToken) ) .andExpect(status) + .andDo(print()) .andReturn(); if (result.getResponse().getStatus() == 200) { String response = result.getResponse().getContentAsString(); From 1d097dd7834b4e231aa73330c7ba38a6599bb37e Mon Sep 17 00:00:00 2001 From: Filip Hanik Date: Thu, 1 Sep 2016 11:37:10 -0700 Subject: [PATCH 3/3] Define the methods and add denyAll clauses --- uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml b/uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml index a06b24d060e..ec267f047f4 100755 --- a/uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml +++ b/uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml @@ -71,7 +71,8 @@ xmlns="http://www.springframework.org/schema/security" use-expressions="true"> - + + @@ -84,9 +85,9 @@ authentication-manager-ref="emptyAuthenticationManager" entry-point-ref="oauthAuthenticationEntryPoint" xmlns="http://www.springframework.org/schema/security" use-expressions="true"> - - - + + +