Permalink
Browse files

[cfid-357] Move autoapprove to client registration

The old oauth.client.autoapprove is still used by the
ClientAdminBootstrap but now it just overrides settings
in the client details themselves.  So this is preferred:

  oauth:
    clients:
      foo:
        scope: ...
        authorized-grant-types: ...
        autoapprove: true

or

  oauth:
    clients:
      foo:
        scope: ...
        authorized-grant-types: ...
        autoapprove:
          - openid
          - cloud_controller.read

[Fixes #40345221]

Change-Id: I0460ebd0bfe471d4a61f718d1eb0002ebb2c0cb4
  • Loading branch information...
dsyer committed Nov 29, 2012
1 parent 8bfcd39 commit 2e679ebea83412b1b668c778171bfc75f5d033f0
@@ -16,6 +16,7 @@
package org.cloudfoundry.identity.uaa.oauth;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
@@ -34,14 +35,16 @@
/**
* @author Dave Syer
- *
+ *
*/
public class ClientAdminBootstrap implements InitializingBean {
private static Log logger = LogFactory.getLog(ClientAdminBootstrap.class);
private Map<String, Map<String, Object>> clients = new HashMap<String, Map<String, Object>>();
+ private Collection<String> autoApproveClients = Collections.emptySet();
+
private ClientRegistrationService clientRegistrationService;
private String domain = "cloudfoundry\\.com";
@@ -50,7 +53,7 @@
* 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.
- *
+ *
* @param domain the domain to set
*/
public void setDomain(String domain) {
@@ -65,6 +68,17 @@ public void setClients(Map<String, Map<String, Object>> clients) {
: new HashMap<String, Map<String, Object>>(clients);
}
+ /**
+ * A set of client ids that are unconditionally to be autoapproved (independent of the settings in the client
+ * details map). These clients will have <code>autoapprove=true</code> when they are inserted into the client
+ * details store.
+ *
+ * @param autoApproveClients the auto approve clients
+ */
+ public void setAutoApproveClients(Collection<String> autoApproveClients) {
+ this.autoApproveClients = autoApproveClients;
+ }
+
/**
* @param clientRegistrationService the clientRegistrationService to set
*/
@@ -74,13 +88,34 @@ public void setClientRegistrationService(ClientRegistrationService clientRegistr
@Override
public void afterPropertiesSet() throws Exception {
- deleteLegacyClients();
addHttpsCallbacks();
addNewClients();
+ updateAutoApprovClients();
}
/**
- * Add https callback to clients in cloudfoundry.com if not present
+ * Explicitly override autoapprove in all clients that were provided in the whitelist.
+ */
+ private void updateAutoApprovClients() {
+
+ List<ClientDetails> clients = clientRegistrationService.listClientDetails();
+
+ for (ClientDetails client : clients) {
+ if (!autoApproveClients.contains(client.getClientId())) {
+ continue;
+ }
+ BaseClientDetails base = new BaseClientDetails(client);
+ Map<String,Object> info = new HashMap<String, Object>(client.getAdditionalInformation());
+ info.put("autoapprove", true);
+ base.setAdditionalInformation(info);
+ logger.info("Adding autoapprove flag: " + base);
+ clientRegistrationService.updateClientDetails(base);
+ }
+
+ }
+
+ /**
+ * Make sure all cloudfoundry.com callbacks are https
*/
private void addHttpsCallbacks() {
List<ClientDetails> clients = clientRegistrationService.listClientDetails();
@@ -91,12 +126,15 @@ private void addHttpsCallbacks() {
continue;
}
Set<String> uris = new HashSet<String>(registeredRedirectUri);
+ boolean newItems = false;
for (String uri : registeredRedirectUri) {
if (uri.matches("^http://[^/]*\\." + domain + ".*")) {
+ newItems = true;
+ uris.remove(uri);
uris.add("https" + uri.substring("http".length()));
}
}
- if (uris.size() == registeredRedirectUri.size()) {
+ if (!newItems) {
continue;
}
BaseClientDetails newClient = new BaseClientDetails(client);
@@ -106,24 +144,6 @@ private void addHttpsCallbacks() {
}
}
- /**
- * Delete legacy clients if present.
- */
- private void deleteLegacyClients() {
- List<ClientDetails> clients = clientRegistrationService.listClientDetails();
-
- for (ClientDetails client : clients) {
- if (client.getClientId().startsWith("legacy_")) {
- logger.info("Deleting legacy client " + client.getClientId());
- try {
- clientRegistrationService.removeClientDetails(client.getClientId());
- } catch (Exception e) {
- logger.error("Failed to remove client.", e);
- }
- }
- }
- }
-
private void addNewClients() throws Exception {
for (String clientId : clients.keySet()) {
Map<String, Object> map = clients.get(clientId);
@@ -161,7 +181,7 @@ private void addNewClients() throws Exception {
clientRegistrationService.addClientDetails(client);
}
catch (ClientAlreadyExistsException e) {
- if (override!=null && override) {
+ if (override != null && override) {
logger.info("Overriding client details for " + clientId);
clientRegistrationService.updateClientDetails(client);
clientRegistrationService.updateClientSecret(clientId, client.getClientSecret());
@@ -12,49 +12,37 @@
*/
package org.cloudfoundry.identity.uaa.oauth;
+import java.util.Collection;
+import java.util.Map;
import org.springframework.security.core.Authentication;
import org.springframework.security.oauth2.provider.AuthorizationRequest;
+import org.springframework.security.oauth2.provider.ClientDetails;
+import org.springframework.security.oauth2.provider.ClientDetailsService;
import org.springframework.security.oauth2.provider.approval.TokenServicesUserApprovalHandler;
-import java.util.Collection;
-import java.util.Map;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Arrays;
-import java.util.Collections;
-
/**
* @author Dave Syer
*
*/
public class UaaUserApprovalHandler extends TokenServicesUserApprovalHandler {
- private Collection<String> autoApproveClients = new HashSet<String>();
-
- private Map<String, Collection<String>> autoApproveClientScopes = new HashMap<String, Collection<String>>();
-
private boolean useTokenServices = true;
+ private ClientDetailsService clientDetailsService;
+
/**
- * @param useTokenServices the useTokenServices to set
+ * @param clientDetailsService the clientDetailsService to set
*/
- public void setUseTokenServices(boolean useTokenServices) {
- this.useTokenServices = useTokenServices;
+ public void setClientDetailsService(ClientDetailsService clientDetailsService) {
+ this.clientDetailsService = clientDetailsService;
}
/**
- * @param autoApproveClients the auto approve clients to set
+ * @param useTokenServices the useTokenServices to set
*/
- public void setAutoApproveClients(String[] autoApproveClients) {
- this.autoApproveClients = Arrays.asList(autoApproveClients);
- }
-
- public void setAutoApproveClientScopes(Map<String, Collection<String>> autoApproveClientScopes) {
- this.autoApproveClientScopes = autoApproveClientScopes;
- if (this.autoApproveClientScopes == null) {
- this.autoApproveClientScopes = Collections.emptyMap();
- }
+ public void setUseTokenServices(boolean useTokenServices) {
+ this.useTokenServices = useTokenServices;
}
/**
@@ -72,12 +60,37 @@ public boolean isApproved(AuthorizationRequest authorizationRequest, Authenticat
if (!userAuthentication.isAuthenticated()) {
return false;
}
-// return authorizationRequest.isApproved() || autoApproveClients.contains(authorizationRequest.getClientId());
+ if (authorizationRequest.isApproved()) {
+ return true;
+ }
String clientId = authorizationRequest.getClientId();
- Collection<String> requestedScopes = authorizationRequest.getScope();
- return authorizationRequest.isApproved()
- || autoApproveClients.contains(clientId)
- || (autoApproveClientScopes.containsKey(clientId) && autoApproveClientScopes.get(clientId).containsAll(requestedScopes));
+ boolean approved = false;
+ if (clientDetailsService != null) {
+ ClientDetails client = clientDetailsService.loadClientByClientId(clientId);
+ Collection<String> requestedScopes = authorizationRequest.getScope();
+ if (isAutoApprove(client, requestedScopes)) {
+ approved = true;
+ }
+ }
+ return approved;
+ }
+
+ private boolean isAutoApprove(ClientDetails client, Collection<String> scopes) {
+ Map<String, Object> info = client.getAdditionalInformation();
+ if (info.containsKey("autoapprove")) {
+ Object object = info.get("autoapprove");
+ if (object instanceof Boolean && (Boolean) object || "true".equals(object)) {
+ return true;
+ }
+ if (object instanceof Collection) {
+ @SuppressWarnings("unchecked")
+ Collection<String> autoScopes = (Collection<String>) object;
+ if (autoScopes.containsAll(scopes)) {
+ return true;
+ }
+ }
+ }
+ return false;
}
}
@@ -13,7 +13,12 @@
package org.cloudfoundry.identity.uaa.oauth;
-import static org.mockito.Mockito.*;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
import java.util.Arrays;
import java.util.Collections;
@@ -53,23 +58,45 @@ public void testSimpleAddClient() throws Exception {
map.put("scope", "openid");
map.put("authorized-grant-types", "authorization_code");
map.put("authorities", "uaa.none");
- BaseClientDetails output = new BaseClientDetails("foo", "none", "openid", "authorization_code,refresh_token", "uaa.none");
+ BaseClientDetails output = new BaseClientDetails("foo", "none", "openid", "authorization_code,refresh_token",
+ "uaa.none");
output.setClientSecret("bar");
doSimpleTest(map, output);
}
+ @Test
+ public void testSimpleAddClientWithAutoApprove() throws Exception {
+ Map<String, Object> map = new HashMap<String, Object>();
+ map.put("id", "foo");
+ map.put("secret", "bar");
+ map.put("scope", "openid");
+ map.put("authorized-grant-types", "authorization_code");
+ map.put("authorities", "uaa.none");
+ BaseClientDetails output = new BaseClientDetails("foo", "none", "openid", "authorization_code,refresh_token",
+ "uaa.none");
+ output.setClientSecret("bar");
+ bootstrap.setAutoApproveClients(Arrays.asList("foo"));
+ when(clientRegistrationService.listClientDetails()).thenReturn(Collections.<ClientDetails> emptyList())
+ .thenReturn(Collections.<ClientDetails> singletonList(output));
+ bootstrap.setClients(Collections.singletonMap((String) map.get("id"), map));
+ bootstrap.afterPropertiesSet();
+ verify(clientRegistrationService).addClientDetails(output);
+ BaseClientDetails updated = new BaseClientDetails(output);
+ updated.setAdditionalInformation(Collections.singletonMap("autoapprove", true));
+ verify(clientRegistrationService).updateClientDetails(updated);
+ }
+
@Test
public void testOverrideClient() throws Exception {
Map<String, Object> map = new HashMap<String, Object>();
map.put("secret", "bar");
map.put("override", true);
bootstrap.setClients(Collections.singletonMap("foo", map));
- doThrow(new ClientAlreadyExistsException("Planned")).when(clientRegistrationService)
- .addClientDetails(any(ClientDetails.class));
+ doThrow(new ClientAlreadyExistsException("Planned")).when(clientRegistrationService).addClientDetails(
+ any(ClientDetails.class));
bootstrap.afterPropertiesSet();
verify(clientRegistrationService, times(1)).addClientDetails(any(ClientDetails.class));
- verify(clientRegistrationService, times(1)).updateClientDetails(
- any(ClientDetails.class));
+ verify(clientRegistrationService, times(1)).updateClientDetails(any(ClientDetails.class));
verify(clientRegistrationService, times(1)).updateClientSecret("foo", "bar");
}
@@ -79,24 +106,14 @@ public void testOverrideClientWithYaml() throws Exception {
Map<String, Object> map = new Yaml().loadAs("id: foo\noverride: true\nsecret: bar\n"
+ "access-token-validity: 100", Map.class);
bootstrap.setClients(Collections.singletonMap("foo", map));
- doThrow(new ClientAlreadyExistsException("Planned")).when(clientRegistrationService)
- .addClientDetails(any(ClientDetails.class));
+ doThrow(new ClientAlreadyExistsException("Planned")).when(clientRegistrationService).addClientDetails(
+ any(ClientDetails.class));
bootstrap.afterPropertiesSet();
verify(clientRegistrationService, times(1)).addClientDetails(any(ClientDetails.class));
- verify(clientRegistrationService, times(1)).updateClientDetails(
- any(ClientDetails.class));
+ verify(clientRegistrationService, times(1)).updateClientDetails(any(ClientDetails.class));
verify(clientRegistrationService, times(1)).updateClientSecret("foo", "bar");
}
- @Test
- public void testLegacyClientIsRemoved() throws Exception {
- BaseClientDetails input = new BaseClientDetails("legacy_foo", "password,scim,tokens", "read,write,password",
- "client_credentials", "ROLE_CLIENT,ROLE_ADMIN", null);
- when(clientRegistrationService.listClientDetails()).thenReturn(Arrays.<ClientDetails> asList(input));
- bootstrap.afterPropertiesSet();
- verify(clientRegistrationService, times(1)).removeClientDetails("legacy_foo");
- }
-
@Test
public void testHttpsUrlIsAddedIfNotPresent() throws Exception {
bootstrap.setDomain("bar.com");
@@ -114,7 +131,7 @@ public void testHttpsUrlIsNotAddedIfAlreadyPresent() throws Exception {
"client_credentials", "uaa.none", "http://foo.bar.com,https://foo.bar.com");
when(clientRegistrationService.listClientDetails()).thenReturn(Arrays.<ClientDetails> asList(input));
bootstrap.afterPropertiesSet();
- verify(clientRegistrationService, never()).updateClientDetails(any(ClientDetails.class));
+ verify(clientRegistrationService, times(1)).updateClientDetails(any(ClientDetails.class));
}
private void doSimpleTest(Map<String, Object> map, BaseClientDetails output) throws Exception {
Oops, something went wrong.

0 comments on commit 2e679eb

Please sign in to comment.