CFID-389: add extra validation and default scopes and authorities

Change-Id: I8773398954c847ea0a1fb41dfd25649a2ca40447

common/src/main/java/org/cloudfoundry/identity/uaa/oauth/ClientAdminBootstrap.java

@@ -27,6 +27,7 @@
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.cloudfoundry.identity.uaa.user.UaaAuthority;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.security.oauth2.provider.BaseClientDetails;
@@ -67,7 +68,8 @@ public class ClientAdminBootstrap implements InitializingBean {
 
 	/**
 	 * The domain suffix (default "cloudfoundry.com") used to detect http redirects. If an http callback in this domain
-	 * is found in a client registration and there is no corresponding value with https as well, then the https value will be added.
+	 * is found in a client registration and there is no corresponding value with https as well, then the https value
+	 * will be added.
 	 * 
 	 * @param domain the domain to set
 	 */
@@ -260,6 +262,14 @@ private void addNewClients() throws Exception {
 			if (validity != null) {
 				client.setRefreshTokenValiditySeconds(validity);
 			}
+			// UAA does not use the resource ids in client registrations
+			client.setResourceIds(Collections.singleton("none"));
+			if (client.getScope().isEmpty()) {
+				client.setScope(Collections.singleton("uaa.none"));
+			}
+			if (client.getAuthorities().isEmpty()) {
+				client.setAuthorities(Collections.singleton(UaaAuthority.UAA_NONE));
+			}
 			try {
 				clientRegistrationService.addClientDetails(client);
 			}

common/src/main/java/org/cloudfoundry/identity/uaa/oauth/ClientAdminEndpoints.java

@@ -335,14 +335,18 @@ private ClientDetails validateClient(ClientDetails prototype, boolean create) {
 				}
 			}
 
-			if (client.getAuthorities().isEmpty()) {
-				client.setAuthorities(AuthorityUtils.commaSeparatedStringToAuthorityList("uaa.none"));
-			}
+		}
 
+		if (client.getAuthorities().isEmpty()) {
+			client.setAuthorities(AuthorityUtils.commaSeparatedStringToAuthorityList("uaa.none"));
 		}
 
 		// The UAA does not allow or require resource ids to be registered because they are determined dynamically
-		client.setResourceIds(StringUtils.commaDelimitedListToSet("none"));
+		client.setResourceIds(Collections.singleton("none"));
+
+		if (client.getScope().isEmpty()) {
+			client.setScope(Collections.singleton("uaa.none"));
+		}
 
 		if (requestedGrantTypes.contains("implicit")) {
 			if (StringUtils.hasText(client.getClientSecret())) {

common/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestFactory.java

@@ -207,7 +207,7 @@ private Set<String> checkUserScopes(Set<String> scopes, Collection<? extends Gra
 			}
 		}
 
-		// TODO: maybe move this to the validateParameters method
+		// Check that a token with empty scope is not going to be granted 
 		if (result.isEmpty() && !clientDetails.getScope().isEmpty()) {
 			throw new InvalidScopeException(
 					"Invalid scope (empty) - this user is not allowed any of the requested scopes: " + scopes

common/src/test/java/org/cloudfoundry/identity/uaa/oauth/ClientAdminEndpointsTests.java

@@ -54,6 +54,8 @@ public class ClientAdminEndpointsTests {
 
 	private ClientDetailsService clientDetailsService = Mockito.mock(ClientDetailsService.class);
 
+	private SecurityContextAccessor securityContextAccessor = Mockito.mock(SecurityContextAccessor.class);
+
 	private ClientRegistrationService clientRegistrationService = Mockito.mock(ClientRegistrationService.class);
 
 	@Rule
@@ -63,11 +65,13 @@ public class ClientAdminEndpointsTests {
 	public void setUp() throws Exception {
 		endpoints.setClientDetailsService(clientDetailsService);
 		endpoints.setClientRegistrationService(clientRegistrationService);
+		endpoints.setSecurityContextAccessor(securityContextAccessor);
 		input.setClientId("foo");
 		input.setClientSecret("secret");
 		input.setAuthorizedGrantTypes(Arrays.asList("authorization_code"));
 		details = new BaseClientDetails(input);
 		details.setResourceIds(Arrays.asList("none"));
+		details.setScope(Arrays.asList("uaa.none"));
 		details.setAuthorities(AuthorityUtils.commaSeparatedStringToAuthorityList("uaa.none"));
 		endpoints.afterPropertiesSet();
 	}
@@ -285,7 +289,8 @@ public void testChangeSecretByAdmin() throws Exception {
 	}
 
 	@Test
-	public void testRemoveClientDetails() throws Exception {
+	public void testRemoveClientDetailsAdminCaller() throws Exception {
+		Mockito.when(securityContextAccessor.isAdmin()).thenReturn(true);
 		Mockito.when(clientDetailsService.loadClientByClientId("foo")).thenReturn(details);
 		endpoints.createClientDetails(details);
 		ResponseEntity<Void> result = endpoints.removeClientDetails("foo");
@@ -355,7 +360,7 @@ public String getClientId() {
 
 	@Test
 	public void testAuthorityAllowedByCaller() throws Exception {
-		BaseClientDetails caller = new BaseClientDetails("caller", null, "none", "client_credentials,implicit",
+		BaseClientDetails caller = new BaseClientDetails("caller", null, "uaa.none", "client_credentials,implicit",
 				"uaa.none");
 		when(clientDetailsService.loadClientByClientId("caller")).thenReturn(caller);
 		endpoints.setSecurityContextAccessor(new StubSecurityContextAccessor() {
@@ -421,6 +426,7 @@ public void nonImplicitClientWithEmptySecretIsRejected() throws Exception {
 
 	@Test
 	public void updateNonImplicitClientWithEmptySecretIsOk() throws Exception {
+		Mockito.when(securityContextAccessor.isAdmin()).thenReturn(true);
 		details.setAuthorizedGrantTypes(Arrays.asList("authorization_code"));
 		details.setClientSecret(null);
 		endpoints.updateClientDetails(details, details.getClientId());