CVE-2017-4973

@@ -1,5 +1,5 @@
 /*******************************************************************************
- *     Cloud Foundry 
+ *     Cloud Foundry
  *     Copyright (c) [2009-2014] Pivotal Software, Inc. All Rights Reserved.
  *
  *     This product is licensed to you under the Apache License, Version 2.0 (the "License").
@@ -12,15 +12,6 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.oauth.approval;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.cloudfoundry.identity.uaa.client.ClientConstants;
@@ -40,12 +31,28 @@
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
 import org.springframework.security.oauth2.provider.ClientDetails;
 import org.springframework.security.oauth2.provider.ClientDetailsService;
+import org.springframework.security.oauth2.provider.NoSuchClientException;
 import org.springframework.stereotype.Controller;
 import org.springframework.util.Assert;
-import org.springframework.web.bind.annotation.*;
+import org.springframework.web.bind.annotation.ExceptionHandler;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.client.RestTemplate;
 import org.springframework.web.servlet.View;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 @Controller
 public class ApprovalsAdminEndpoints implements InitializingBean, ApprovalsControllerService {
 
@@ -170,6 +177,7 @@ public List<Approval> updateApprovals(@RequestBody Approval[] approvals) {
     @ResponseBody
     @Override
     public List<Approval> updateClientApprovals(@PathVariable String clientId, @RequestBody Approval[] approvals) {
+        clientDetailsService.loadClientByClientId(clientId);
         String currentUserId = getCurrentUserId();
         logger.debug("Updating approvals for user: " + currentUserId);
         approvalStore.revokeApprovals(String.format(USER_AND_CLIENT_FILTER_TEMPLATE, currentUserId, clientId));
@@ -203,12 +211,19 @@ private boolean isValidUser(String userId) {
     @ResponseBody
     @Override
     public SimpleMessage revokeApprovals(@RequestParam(required = true) String clientId) {
+        clientDetailsService.loadClientByClientId(clientId);
         String username = getCurrentUserId();
         logger.debug("Revoking all existing approvals for user: " + username + " and client " + clientId);
         approvalStore.revokeApprovals(String.format(USER_AND_CLIENT_FILTER_TEMPLATE, username, clientId));
         return new SimpleMessage("ok", "Approvals of user " + username + " and client " + clientId + " revoked");
     }
 
+    @ExceptionHandler
+    public View handleException(NoSuchClientException nsce) {
+        logger.debug("Client not found:" + nsce.getMessage());
+        return handleException(new UaaException(nsce.getMessage(), 404));
+    }
+
     @ExceptionHandler
     public View handleException(Exception t) {
         UaaException e = t instanceof UaaException ? (UaaException) t : new UaaException("Unexpected error",

@@ -12,6 +12,8 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.login;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.cloudfoundry.identity.uaa.authentication.Origin;
 import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
 import org.cloudfoundry.identity.uaa.client.ClientConstants;
@@ -20,11 +22,15 @@
 import org.springframework.security.core.Authentication;
 import org.springframework.security.oauth2.provider.ClientDetails;
 import org.springframework.security.oauth2.provider.ClientDetailsService;
+import org.springframework.security.oauth2.provider.NoSuchClientException;
 import org.springframework.stereotype.Controller;
 import org.springframework.ui.Model;
+import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.servlet.View;
+import org.springframework.web.servlet.view.RedirectView;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -35,6 +41,7 @@
 @Controller
 public class ProfileController {
 
+    protected static Log logger = LogFactory.getLog(ProfileController.class);
     private final ApprovalsService approvalsService;
     private final ClientDetailsService clientDetailsService;
 
@@ -113,4 +120,10 @@ private boolean isUaaManagedUser(Authentication authentication) {
         }
         return false;
     }
+
+    @ExceptionHandler
+    public View handleException(NoSuchClientException nsce) {
+        logger.debug("Unable to find client for approvals:" + nsce.getMessage());
+        return new RedirectView("profile?error_message_code=request.invalid_parameter", true);
+    }
 }

@@ -33,6 +33,7 @@
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.security.oauth2.provider.ClientDetailsService;
+import org.springframework.security.oauth2.provider.NoSuchClientException;
 import org.springframework.security.oauth2.provider.client.BaseClientDetails;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@@ -108,6 +109,7 @@ public void setUp() throws Exception {
         approvalsByClientId.put("app", Arrays.asList(readApproval, writeApproval));
 
         Mockito.when(approvalsService.getCurrentApprovalsByClientId()).thenReturn(approvalsByClientId);
+        Mockito.doThrow(new NoSuchClientException("invalidId")).when(approvalsService).deleteApprovalsForClient("invalidId");
 
         BaseClientDetails appClient = new BaseClientDetails("app","thing","thing.read,thing.write","authorization_code", "");
         appClient.addAdditionalInformation(ClientConstants.CLIENT_NAME, THE_ULTIMATE_APP);
@@ -205,6 +207,18 @@ public void testUpdateProfile() throws Exception {
         Assert.assertEquals(DENIED, writeApproval.getStatus());
     }
 
+    @Test
+    public void validate_client_id_on_revoke() throws Exception {
+        MockHttpServletRequestBuilder post = post("/profile")
+            .param("checkedScopes", "app-resource.read")
+            .param("delete", "")
+            .param("clientId", "invalidId");
+
+        mockMvc.perform(post)
+            .andExpect(status().isFound())
+            .andExpect(redirectedUrl("profile?error_message_code=request.invalid_parameter"));
+    }
+
     @Test
     public void testRevokeApp() throws Exception {
         MockHttpServletRequestBuilder post = post("/profile")

@@ -53,6 +53,7 @@ login.account_not_verified=Your account is not verified. You can get another ver
 login.account_locked=Your account has been locked because of too many failed attempts to login.
 login.invalid_login_request=Invalid login attempt, request does not meet our security standards, please try again.
 account_activation.invite.email_mismatch=The authenticated email does not match the invited email. Please log in using a different account.
+request.invalid_parameter=The request contains an invalid parameter that can not be processed.
 
 # Passay Properties
 HISTORY_VIOLATION=Password matches one of %1$s previous passwords.

@@ -71,6 +71,7 @@
 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;
 
@@ -188,6 +189,37 @@ public void test_oauth_authorize_without_csrf() throws Exception {
             .andExpect(status().isFound()); //approval page no longer showing up
     }
 
+    @Test
+    public void revoke() throws Exception {
+        test_oauth_authorize_without_csrf();
+        MockHttpSession session = getAuthenticatedSession(user1);
+        getMockMvc().perform(
+            post("/profile")
+                .with(cookieCsrf())
+                .param("delete","true")
+                .param("clientId", client1.getClientId())
+                .session(session)
+        )
+            .andExpect(status().isFound())
+            .andExpect(header().string("Location", "profile"));
+
+    }
+
+    @Test
+    public void revoke_invalid_client() throws Exception {
+        test_oauth_authorize_without_csrf();
+        MockHttpSession session = getAuthenticatedSession(user1);
+        getMockMvc().perform(
+            post("/profile")
+                .with(cookieCsrf())
+                .param("delete","true")
+                .param("clientId", "invalid_id")
+                .session(session)
+        )
+            .andExpect(status().isFound())
+            .andExpect(header().string("Location", "profile?error_message_code=request.invalid_parameter"));
+    }
+
     @Test
     public void test_get_approvals() throws Exception {
         test_oauth_authorize_without_csrf();

@@ -143,6 +143,7 @@
         <intercept-url pattern="/Groups/External/**" access="#oauth2.hasScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="DELETE" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="DELETE" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasAnyScope('scim.write', 'groups.update') or @groupRole.isGroupWriter(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="PUT" />
+        <intercept-url pattern="/Groups/**" access="#oauth2.hasAnyScope('scim.write', 'groups.update') or @groupRole.isGroupWriter(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="POST" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasScope('scim.read') or @groupRole.isGroupReader(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="GET" />
         <intercept-url pattern="/Groups" access="#oauth2.hasScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="POST" />
         <!--<intercept-url pattern="/**" access="ROLE_NONEXISTENT" />-->

@@ -63,6 +63,7 @@
 import static org.cloudfoundry.identity.uaa.mock.util.MockMvcUtils.utils;
 import static org.junit.Assert.assertTrue;
 import static org.springframework.http.MediaType.APPLICATION_JSON;
+import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
@@ -82,6 +83,7 @@ public class ScimGroupEndpointsMockMvcTests extends InjectedMockContextTest {
     private RandomValueStringGenerator generator = new RandomValueStringGenerator();
     private List<String> defaultExternalMembers;
     private List<ScimGroupExternalMember> databaseExternalMembers;
+    private TestClient testClient;
 
     @Before
     public void setUp() throws Exception {
@@ -94,22 +96,35 @@ public void setUp() throws Exception {
         ScimExternalGroupBootstrap bootstrap = getWebApplicationContext().getBean(ScimExternalGroupBootstrap.class);
         bootstrap.afterPropertiesSet();
 
-        TestClient testClient = new TestClient(getMockMvc());
+        testClient = new TestClient(getMockMvc());
         String adminToken = testClient.getClientCredentialsOAuthAccessToken("admin", "adminsecret",
-                "clients.read clients.write clients.secret clients.admin");
+                                                                            "clients.read clients.write clients.secret clients.admin");
         String clientId = generator.generate().toLowerCase();
         String clientSecret = generator.generate().toLowerCase();
         String authorities = "scim.read,scim.write,password.write,oauth.approvals,scim.create";
         utils().createClient(this.getMockMvc(), adminToken, clientId, clientSecret, Collections.singletonList("oauth"), Collections.singletonList("foo,bar"), Collections.singletonList("client_credentials"), authorities);
-        scimReadToken = testClient.getClientCredentialsOAuthAccessToken(clientId, clientSecret,"scim.read password.write");
-        scimWriteToken = testClient.getClientCredentialsOAuthAccessToken(clientId, clientSecret,"scim.write password.write");
+        scimReadToken = testClient.getClientCredentialsOAuthAccessToken(clientId, clientSecret, "scim.read password.write");
+        scimWriteToken = testClient.getClientCredentialsOAuthAccessToken(clientId, clientSecret, "scim.write password.write");
 
         defaultExternalMembers = new LinkedList<>(originalDefaultExternalMembers);
         databaseExternalMembers = new LinkedList<>(originalDatabaseExternalMembers);
 
         scimUser = createUser(scimWriteToken, new HashSet(Arrays.asList("scim.read", "scim.write", "scim.me")));
-        scimReadUserToken = testClient.getUserOAuthAccessToken("cf","", scimUser.getUserName(), "password", "scim.read");
-        identityClientToken = testClient.getClientCredentialsOAuthAccessToken("identity","identitysecret","");
+        scimReadUserToken = testClient.getUserOAuthAccessToken("cf", "", scimUser.getUserName(), "password", "scim.read");
+        identityClientToken = testClient.getClientCredentialsOAuthAccessToken("identity", "identitysecret", "");
+    }
+
+    @Test
+    public void test_member_add() throws Exception {
+        String token = testClient.getUserOAuthAccessToken("cf","", scimUser.getUserName(), "password", "openid");
+        String groupId = getGroupId("scim.read");
+        ScimGroupMember scimGroupMember = new ScimGroupMember(scimUser.getId(), ScimGroupMember.Type.USER, Arrays.asList(ScimGroupMember.Role.MEMBER, ScimGroupMember.Role.READER));
+        MockHttpServletRequestBuilder post = post("/Groups/" + groupId + "/members")
+            .header("Authorization", "Bearer " + token)
+            .header("Content-Type", APPLICATION_JSON_VALUE)
+            .content(JsonUtils.writeValueAsString(scimGroupMember));
+        getMockMvc().perform(post)
+            .andExpect(status().isForbidden());
     }
 
     @Test

@@ -143,6 +143,7 @@
         <intercept-url pattern="/Groups/External/**" access="#oauth2.hasScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="DELETE" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="DELETE" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasAnyScope('scim.write', 'groups.update') or @groupRole.isGroupWriter(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="PUT" />
+        <intercept-url pattern="/Groups/**" access="#oauth2.hasAnyScope('scim.write', 'groups.update') or @groupRole.isGroupWriter(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="POST" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasScope('scim.read') or @groupRole.isGroupReader(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="GET" />
         <intercept-url pattern="/Groups" access="#oauth2.hasScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="POST" />
         <!--<intercept-url pattern="/**" access="ROLE_NONEXISTENT" />-->

@@ -75,8 +75,11 @@
 import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
+import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 import static org.springframework.util.StringUtils.hasText;
 
@@ -124,7 +127,7 @@ public void setUp() throws Exception {
                 "clients.read clients.write clients.secret clients.admin");
         clientId = generator.generate().toLowerCase();
         clientSecret = generator.generate().toLowerCase();
-        String authorities = "scim.read,scim.write,password.write,oauth.approvals,scim.create";
+        String authorities = "scim.read,scim.write,password.write,oauth.approvals,scim.create,other.scope";
         utils().createClient(this.getMockMvc(), adminToken, clientId, clientSecret, Collections.singleton("oauth"), Arrays.asList("foo","bar","scim.read"), Arrays.asList("client_credentials", "password"), authorities);
         scimReadToken = testClient.getClientCredentialsOAuthAccessToken(clientId, clientSecret,"scim.read password.write");
         scimWriteToken = testClient.getClientCredentialsOAuthAccessToken(clientId, clientSecret,"scim.write password.write");
@@ -1118,6 +1121,35 @@ public void delete_nonexistent_user() throws Exception {
             .andExpect(status().isNotFound());
     }
 
+    @Test
+    public void patch_has_one_path() throws Exception {
+        getMockMvc().perform(
+            patch("/Group/groupId/members")
+                .header("Authorization", "Bearer " + scimWriteToken)
+                .header("Content-Type", APPLICATION_JSON_VALUE)
+        )
+            .andDo(print())
+            .andExpect(status().isFound()) //gets caught by the ui filter for unknown URIs
+            .andExpect(redirectedUrl("http://localhost/login"));
+    }
+
+    @Test
+    public void add_member_bad_token() throws Exception {
+        ScimUser user = createUserAndAddToGroups(IdentityZone.getUaa(), Collections.EMPTY_SET);
+        String groupId = getGroupId("scim.read");
+        String anyOldToken = testClient.getClientCredentialsOAuthAccessToken(clientId, clientSecret,"other.scope");
+
+        ScimGroupMember scimGroupMember = new ScimGroupMember(user.getId(), ScimGroupMember.Type.USER, Arrays.asList(ScimGroupMember.Role.MEMBER, ScimGroupMember.Role.READER));
+
+        MockHttpServletRequestBuilder post = post("/Groups/" + groupId + "/members")
+            .header("Authorization", "Bearer " + anyOldToken)
+            .header("Content-Type", APPLICATION_JSON_VALUE)
+            .content(JsonUtils.writeValueAsString(scimGroupMember));
+        getMockMvc().perform(post)
+            .andExpect(status().isForbidden());
+
+    }
+
     @Test
     public void add_member_to_nonexistent_group() throws Exception {
         ScimUser user = createUserAndAddToGroups(IdentityZone.getUaa(), Collections.EMPTY_SET);

@@ -385,8 +385,7 @@ public ScimGroup updateGroup(@RequestBody ScimGroup group, @PathVariable String
         }
     }
 
-    @RequestMapping(value = { "/Group/{groupId}", "/Groups/{groupId}" },
-    method = RequestMethod.PATCH)
+    @RequestMapping(value = { "/Groups/{groupId}" }, method = RequestMethod.PATCH)
     @ResponseBody
     public ScimGroup patchGroup(@RequestBody ScimGroup patch, @PathVariable
                                 String groupId,

@@ -143,6 +143,7 @@
         <intercept-url pattern="/Groups/External/**" access="#oauth2.hasScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="DELETE" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="DELETE" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasAnyScope('scim.write', 'groups.update') or @groupRole.isGroupWriter(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="PUT" />
+        <intercept-url pattern="/Groups/**" access="#oauth2.hasAnyScope('scim.write', 'groups.update') or @groupRole.isGroupWriter(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="POST" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasScope('scim.read') or @groupRole.isGroupReader(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="GET" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasAnyScope('scim.write', 'groups.update') or @groupRole.isGroupWriter(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="PATCH" />
         <intercept-url pattern="/Groups" access="#oauth2.hasScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="POST" />

@@ -75,8 +75,11 @@
 import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
+import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 import static org.springframework.util.StringUtils.hasText;
 
@@ -122,7 +125,7 @@ public void setUp() throws Exception {
                 "clients.read clients.write clients.secret clients.admin");
         clientId = generator.generate().toLowerCase();
         clientSecret = generator.generate().toLowerCase();
-        String authorities = "scim.read,scim.write,password.write,oauth.approvals,scim.create";
+        String authorities = "scim.read,scim.write,password.write,oauth.approvals,scim.create,other.scope";
         utils().createClient(this.getMockMvc(), adminToken, clientId, clientSecret, Collections.singleton("oauth"), Arrays.asList("foo","bar","scim.read"), Arrays.asList("client_credentials", "password"), authorities);
         scimReadToken = testClient.getClientCredentialsOAuthAccessToken(clientId, clientSecret,"scim.read password.write");
         scimWriteToken = testClient.getClientCredentialsOAuthAccessToken(clientId, clientSecret,"scim.write password.write");
@@ -1138,6 +1141,35 @@ public void delete_nonexistent_user() throws Exception {
             .andExpect(status().isNotFound());
     }
 
+    @Test
+    public void patch_has_one_path() throws Exception {
+        getMockMvc().perform(
+            patch("/Group/groupId/members")
+                .header("Authorization", "Bearer " + scimWriteToken)
+                .header("Content-Type", APPLICATION_JSON_VALUE)
+        )
+            .andDo(print())
+            .andExpect(status().isFound()) //gets caught by the ui filter for unknown URIs
+            .andExpect(redirectedUrl("http://localhost/login"));
+    }
+
+    @Test
+    public void add_member_bad_token() throws Exception {
+        ScimUser user = createUserAndAddToGroups(IdentityZone.getUaa(), Collections.EMPTY_SET);
+        String groupId = getGroupId("scim.read");
+        String anyOldToken = testClient.getClientCredentialsOAuthAccessToken(clientId, clientSecret,"other.scope");
+
+        ScimGroupMember scimGroupMember = new ScimGroupMember(user.getId(), ScimGroupMember.Type.USER, Arrays.asList(ScimGroupMember.Role.MEMBER, ScimGroupMember.Role.READER));
+
+        MockHttpServletRequestBuilder post = post("/Groups/" + groupId + "/members")
+            .header("Authorization", "Bearer " + anyOldToken)
+            .header("Content-Type", APPLICATION_JSON_VALUE)
+            .content(JsonUtils.writeValueAsString(scimGroupMember));
+        getMockMvc().perform(post)
+            .andExpect(status().isForbidden());
+
+    }
+
     @Test
     public void add_member_to_nonexistent_group() throws Exception {
         ScimUser user = createUserAndAddToGroups(IdentityZone.getUaa(), Collections.EMPTY_SET);

@@ -385,8 +385,7 @@ public ScimGroup updateGroup(@RequestBody ScimGroup group, @PathVariable String
         }
     }
 
-    @RequestMapping(value = { "/Group/{groupId}", "/Groups/{groupId}" },
-    method = RequestMethod.PATCH)
+    @RequestMapping(value = { "/Groups/{groupId}" }, method = RequestMethod.PATCH)
     @ResponseBody
     public ScimGroup patchGroup(@RequestBody ScimGroup patch, @PathVariable
                                 String groupId,

@@ -144,6 +144,7 @@
         <intercept-url pattern="/Groups/External/**" access="#oauth2.hasScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="DELETE" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="DELETE" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasAnyScope('scim.write', 'groups.update') or @groupRole.isGroupWriter(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="PUT" />
+        <intercept-url pattern="/Groups/**" access="#oauth2.hasAnyScope('scim.write', 'groups.update') or @groupRole.isGroupWriter(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="POST" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasScope('scim.read') or @groupRole.isGroupReader(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="GET" />
         <intercept-url pattern="/Groups/**" access="#oauth2.hasAnyScope('scim.write', 'groups.update') or @groupRole.isGroupWriter(request, 1) or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="PATCH" />
         <intercept-url pattern="/Groups" access="#oauth2.hasScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="POST" />

@@ -75,8 +75,11 @@
 import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
+import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 import static org.springframework.util.StringUtils.hasText;
 
@@ -122,7 +125,7 @@ public void setUp() throws Exception {
                 "clients.read clients.write clients.secret clients.admin");
         clientId = generator.generate().toLowerCase();
         clientSecret = generator.generate().toLowerCase();
-        String authorities = "scim.read,scim.write,password.write,oauth.approvals,scim.create";
+        String authorities = "scim.read,scim.write,password.write,oauth.approvals,scim.create,other.scope";
         utils().createClient(this.getMockMvc(), adminToken, clientId, clientSecret, Collections.singleton("oauth"), Arrays.asList("foo","bar","scim.read"), Arrays.asList("client_credentials", "password"), authorities);
         scimReadToken = testClient.getClientCredentialsOAuthAccessToken(clientId, clientSecret,"scim.read password.write");
         scimWriteToken = testClient.getClientCredentialsOAuthAccessToken(clientId, clientSecret,"scim.write password.write");
@@ -1138,6 +1141,35 @@ public void delete_nonexistent_user() throws Exception {
             .andExpect(status().isNotFound());
     }
 
+    @Test
+    public void patch_has_one_path() throws Exception {
+        getMockMvc().perform(
+            patch("/Group/groupId/members")
+                .header("Authorization", "Bearer " + scimWriteToken)
+                .header("Content-Type", APPLICATION_JSON_VALUE)
+        )
+            .andDo(print())
+            .andExpect(status().isFound()) //gets caught by the ui filter for unknown URIs
+            .andExpect(redirectedUrl("http://localhost/login"));
+    }
+
+    @Test
+    public void add_member_bad_token() throws Exception {
+        ScimUser user = createUserAndAddToGroups(IdentityZone.getUaa(), Collections.EMPTY_SET);
+        String groupId = getGroupId("scim.read");
+        String anyOldToken = testClient.getClientCredentialsOAuthAccessToken(clientId, clientSecret,"other.scope");
+
+        ScimGroupMember scimGroupMember = new ScimGroupMember(user.getId(), ScimGroupMember.Type.USER, Arrays.asList(ScimGroupMember.Role.MEMBER, ScimGroupMember.Role.READER));
+
+        MockHttpServletRequestBuilder post = post("/Groups/" + groupId + "/members")
+            .header("Authorization", "Bearer " + anyOldToken)
+            .header("Content-Type", APPLICATION_JSON_VALUE)
+            .content(JsonUtils.writeValueAsString(scimGroupMember));
+        getMockMvc().perform(post)
+            .andExpect(status().isForbidden());
+
+    }
+
     @Test
     public void add_member_to_nonexistent_group() throws Exception {
         ScimUser user = createUserAndAddToGroups(IdentityZone.getUaa(), Collections.EMPTY_SET);

@@ -12,21 +12,27 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.account;
 
-import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.cloudfoundry.identity.uaa.approval.Approval;
 import org.cloudfoundry.identity.uaa.approval.ApprovalsService;
 import org.cloudfoundry.identity.uaa.approval.DescribedApproval;
-import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
+import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
 import org.cloudfoundry.identity.uaa.constants.OriginKeys;
-import org.cloudfoundry.identity.uaa.approval.Approval;
+import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.oauth2.provider.ClientDetails;
 import org.springframework.security.oauth2.provider.ClientDetailsService;
+import org.springframework.security.oauth2.provider.NoSuchClientException;
 import org.springframework.stereotype.Controller;
 import org.springframework.ui.Model;
+import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.servlet.View;
+import org.springframework.web.servlet.view.RedirectView;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -37,6 +43,8 @@
 @Controller
 public class ProfileController {
 
+    protected static Log logger = LogFactory.getLog(ProfileController.class);
+
     private final ApprovalsService approvalsService;
     private final ClientDetailsService clientDetailsService;
 
@@ -108,6 +116,13 @@ else if (null != delete) {
         return "redirect:profile";
     }
 
+    @ExceptionHandler
+    public View handleException(NoSuchClientException nsce) {
+        logger.debug("Unable to find client for approvals:"+nsce.getMessage());
+        return new RedirectView("profile?error_message_code=request.invalid_parameter", true);
+    }
+
+
     private boolean isUaaManagedUser(Authentication authentication) {
         if (authentication.getPrincipal() instanceof UaaPrincipal) {
             UaaPrincipal principal = (UaaPrincipal) authentication.getPrincipal();

@@ -1,5 +1,5 @@
 /*******************************************************************************
- *     Cloud Foundry 
+ *     Cloud Foundry
  *     Copyright (c) [2009-2016] Pivotal Software, Inc. All Rights Reserved.
  *
  *     This product is licensed to you under the Apache License, Version 2.0 (the "License").
@@ -12,41 +12,45 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.approval;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
-import org.cloudfoundry.identity.uaa.web.ConvertingExceptionView;
-import org.cloudfoundry.identity.uaa.web.ExceptionReport;
 import org.cloudfoundry.identity.uaa.error.UaaException;
 import org.cloudfoundry.identity.uaa.resources.ActionResult;
 import org.cloudfoundry.identity.uaa.security.DefaultSecurityContextAccessor;
 import org.cloudfoundry.identity.uaa.security.SecurityContextAccessor;
 import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
 import org.cloudfoundry.identity.uaa.util.UaaPagingUtils;
+import org.cloudfoundry.identity.uaa.web.ConvertingExceptionView;
+import org.cloudfoundry.identity.uaa.web.ExceptionReport;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
-import org.springframework.security.oauth2.provider.ClientDetails;
 import org.springframework.security.oauth2.provider.ClientDetailsService;
+import org.springframework.security.oauth2.provider.NoSuchClientException;
 import org.springframework.security.oauth2.provider.client.BaseClientDetails;
 import org.springframework.stereotype.Controller;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
-import org.springframework.web.bind.annotation.*;
+import org.springframework.web.bind.annotation.ExceptionHandler;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.client.RestTemplate;
 import org.springframework.web.servlet.View;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 @Controller
 public class ApprovalsAdminEndpoints implements InitializingBean, ApprovalsControllerService {
 
@@ -168,6 +172,7 @@ public List<Approval> updateApprovals(@RequestBody Approval[] approvals) {
     @ResponseBody
     @Override
     public List<Approval> updateClientApprovals(@PathVariable String clientId, @RequestBody Approval[] approvals) {
+        clientDetailsService.loadClientByClientId(clientId);
         String currentUserId = getCurrentUserId();
         logger.debug("Updating approvals for user: " + currentUserId);
         approvalStore.revokeApprovals(String.format(USER_AND_CLIENT_FILTER_TEMPLATE, currentUserId, clientId));
@@ -202,10 +207,17 @@ private boolean isValidUser(String userId) {
     public ActionResult revokeApprovals(@RequestParam(required = true) String clientId) {
         String username = getCurrentUserId();
         logger.debug("Revoking all existing approvals for user: " + username + " and client " + clientId);
+        clientDetailsService.loadClientByClientId(clientId);
         approvalStore.revokeApprovals(String.format(USER_AND_CLIENT_FILTER_TEMPLATE, username, clientId));
         return new ActionResult("ok", "Approvals of user " + username + " and client " + clientId + " revoked");
     }
 
+    @ExceptionHandler
+    public View handleException(NoSuchClientException nsce) {
+        logger.debug("Client not found:"+nsce.getMessage());
+        return handleException(new UaaException(nsce.getMessage(), 404));
+    }
+
     @ExceptionHandler
     public View handleException(Exception t) {
         UaaException e = t instanceof UaaException ? (UaaException) t : new UaaException("Unexpected error",

@@ -1,5 +1,5 @@
 /*******************************************************************************
- *     Cloud Foundry 
+ *     Cloud Foundry
  *     Copyright (c) [2009-2016] Pivotal Software, Inc. All Rights Reserved.
  *
  *     This product is licensed to you under the Apache License, Version 2.0 (the "License").
@@ -12,30 +12,12 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.oauth.approval;
 
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Date;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
-import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.APPROVED;
-import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.DENIED;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
 import com.fasterxml.jackson.core.type.TypeReference;
 import org.cloudfoundry.identity.uaa.approval.Approval;
+import org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus;
 import org.cloudfoundry.identity.uaa.approval.ApprovalsAdminEndpoints;
 import org.cloudfoundry.identity.uaa.approval.JdbcApprovalStore;
-import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
 import org.cloudfoundry.identity.uaa.error.UaaException;
-import org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus;
 import org.cloudfoundry.identity.uaa.resources.jdbc.JdbcPagingListFactory;
 import org.cloudfoundry.identity.uaa.resources.jdbc.SimpleSearchQueryConverter;
 import org.cloudfoundry.identity.uaa.security.SecurityContextAccessor;
@@ -48,14 +30,34 @@
 import org.cloudfoundry.identity.uaa.util.JsonUtils;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.security.oauth2.provider.NoSuchClientException;
 import org.springframework.security.oauth2.provider.client.BaseClientDetails;
 import org.springframework.security.oauth2.provider.client.InMemoryClientDetailsService;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.APPROVED;
+import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.DENIED;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 public class ApprovalsAdminEndpointsTests extends JdbcTestBase {
     private UaaTestAccounts testAccounts = null;
-    
+
     private JdbcApprovalStore dao;
 
     private UaaUserDatabase userDao = null;
@@ -64,6 +66,9 @@ public class ApprovalsAdminEndpointsTests extends JdbcTestBase {
 
     private ApprovalsAdminEndpoints endpoints;
 
+    @Rule
+    public ExpectedException exception = ExpectedException.none();
+
     @Before
     public void initApprovalsAdminEndpointsTests() {
         testAccounts = UaaTestAccounts.standard(null);
@@ -114,6 +119,21 @@ public void cleanupDataSource() throws Exception {
         assertThat(jdbcTemplate.queryForObject("select count(*) from users", Integer.class), is(0));
     }
 
+    @Test
+    public void validate_client_id_on_revoke() throws Exception {
+        exception.expect(NoSuchClientException.class);
+        exception.expectMessage("No client with requested id: invalid_id");
+        endpoints.revokeApprovals("invalid_id");
+    }
+
+    @Test
+    public void validate_client_id_on_update() throws Exception {
+        exception.expect(NoSuchClientException.class);
+        exception.expectMessage("No client with requested id: invalid_id");
+        endpoints.updateClientApprovals("invalid_id", new Approval[0]);
+    }
+
+
     @Test
     public void canGetApprovals() {
         addApproval(marissa.getId(), "c1", "uaa.user", 6000, APPROVED);

@@ -50,6 +50,7 @@
 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;
 
@@ -128,6 +129,37 @@ public void test_oauth_authorize_without_csrf() throws Exception {
             .andExpect(status().isFound()); //approval page no longer showing up
     }
 
+    @Test
+    public void revoke() throws Exception {
+        test_oauth_authorize_without_csrf();
+        MockHttpSession session = getAuthenticatedSession(user1);
+        getMockMvc().perform(
+            post("/profile")
+                .with(cookieCsrf())
+                .param("delete","true")
+                .param("clientId", client1.getClientId())
+                .session(session)
+        )
+            .andExpect(status().isFound())
+            .andExpect(header().string("Location", "profile"));
+
+    }
+
+    @Test
+    public void revoke_invalid_client() throws Exception {
+        test_oauth_authorize_without_csrf();
+        MockHttpSession session = getAuthenticatedSession(user1);
+        getMockMvc().perform(
+            post("/profile")
+                .with(cookieCsrf())
+                .param("delete","true")
+                .param("clientId", "invalid_id")
+                .session(session)
+        )
+            .andExpect(status().isFound())
+            .andExpect(header().string("Location", "profile?error_message_code=request.invalid_parameter"));
+    }
+
     @Test
     public void test_get_approvals() throws Exception {
         test_oauth_authorize_without_csrf();

@@ -12,21 +12,27 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.account;
 
-import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.cloudfoundry.identity.uaa.approval.Approval;
 import org.cloudfoundry.identity.uaa.approval.ApprovalsService;
 import org.cloudfoundry.identity.uaa.approval.DescribedApproval;
-import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
+import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
 import org.cloudfoundry.identity.uaa.constants.OriginKeys;
-import org.cloudfoundry.identity.uaa.approval.Approval;
+import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.oauth2.provider.ClientDetails;
 import org.springframework.security.oauth2.provider.ClientDetailsService;
+import org.springframework.security.oauth2.provider.NoSuchClientException;
 import org.springframework.stereotype.Controller;
 import org.springframework.ui.Model;
+import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.servlet.View;
+import org.springframework.web.servlet.view.RedirectView;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -37,6 +43,8 @@
 @Controller
 public class ProfileController {
 
+    protected static Log logger = LogFactory.getLog(ProfileController.class);
+
     private final ApprovalsService approvalsService;
     private final ClientDetailsService clientDetailsService;
 
@@ -108,6 +116,13 @@ else if (null != delete) {
         return "redirect:profile";
     }
 
+    @ExceptionHandler
+    public View handleException(NoSuchClientException nsce) {
+        logger.debug("Unable to find client for approvals:"+nsce.getMessage());
+        return new RedirectView("profile?error_message_code=request.invalid_parameter", true);
+    }
+
+
     private boolean isUaaManagedUser(Authentication authentication) {
         if (authentication.getPrincipal() instanceof UaaPrincipal) {
             UaaPrincipal principal = (UaaPrincipal) authentication.getPrincipal();

@@ -1,5 +1,5 @@
 /*******************************************************************************
- *     Cloud Foundry 
+ *     Cloud Foundry
  *     Copyright (c) [2009-2016] Pivotal Software, Inc. All Rights Reserved.
  *
  *     This product is licensed to you under the Apache License, Version 2.0 (the "License").
@@ -12,41 +12,45 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.approval;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
-import org.cloudfoundry.identity.uaa.web.ConvertingExceptionView;
-import org.cloudfoundry.identity.uaa.web.ExceptionReport;
 import org.cloudfoundry.identity.uaa.error.UaaException;
 import org.cloudfoundry.identity.uaa.resources.ActionResult;
 import org.cloudfoundry.identity.uaa.security.DefaultSecurityContextAccessor;
 import org.cloudfoundry.identity.uaa.security.SecurityContextAccessor;
 import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
 import org.cloudfoundry.identity.uaa.util.UaaPagingUtils;
+import org.cloudfoundry.identity.uaa.web.ConvertingExceptionView;
+import org.cloudfoundry.identity.uaa.web.ExceptionReport;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
-import org.springframework.security.oauth2.provider.ClientDetails;
 import org.springframework.security.oauth2.provider.ClientDetailsService;
+import org.springframework.security.oauth2.provider.NoSuchClientException;
 import org.springframework.security.oauth2.provider.client.BaseClientDetails;
 import org.springframework.stereotype.Controller;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
-import org.springframework.web.bind.annotation.*;
+import org.springframework.web.bind.annotation.ExceptionHandler;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.client.RestTemplate;
 import org.springframework.web.servlet.View;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 @Controller
 public class ApprovalsAdminEndpoints implements InitializingBean, ApprovalsControllerService {
 
@@ -168,6 +172,7 @@ public List<Approval> updateApprovals(@RequestBody Approval[] approvals) {
     @ResponseBody
     @Override
     public List<Approval> updateClientApprovals(@PathVariable String clientId, @RequestBody Approval[] approvals) {
+        clientDetailsService.loadClientByClientId(clientId);
         String currentUserId = getCurrentUserId();
         logger.debug("Updating approvals for user: " + currentUserId);
         approvalStore.revokeApprovals(String.format(USER_AND_CLIENT_FILTER_TEMPLATE, currentUserId, clientId));
@@ -202,10 +207,17 @@ private boolean isValidUser(String userId) {
     public ActionResult revokeApprovals(@RequestParam(required = true) String clientId) {
         String username = getCurrentUserId();
         logger.debug("Revoking all existing approvals for user: " + username + " and client " + clientId);
+        clientDetailsService.loadClientByClientId(clientId);
         approvalStore.revokeApprovals(String.format(USER_AND_CLIENT_FILTER_TEMPLATE, username, clientId));
         return new ActionResult("ok", "Approvals of user " + username + " and client " + clientId + " revoked");
     }
 
+    @ExceptionHandler
+    public View handleException(NoSuchClientException nsce) {
+        logger.debug("Client not found:"+nsce.getMessage());
+        return handleException(new UaaException(nsce.getMessage(), 404));
+    }
+
     @ExceptionHandler
     public View handleException(Exception t) {
         UaaException e = t instanceof UaaException ? (UaaException) t : new UaaException("Unexpected error",

@@ -1,5 +1,5 @@
 /*******************************************************************************
- *     Cloud Foundry 
+ *     Cloud Foundry
  *     Copyright (c) [2009-2016] Pivotal Software, Inc. All Rights Reserved.
  *
  *     This product is licensed to you under the Apache License, Version 2.0 (the "License").
@@ -12,30 +12,12 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.oauth.approval;
 
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Date;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
-import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.APPROVED;
-import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.DENIED;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
 import com.fasterxml.jackson.core.type.TypeReference;
 import org.cloudfoundry.identity.uaa.approval.Approval;
+import org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus;
 import org.cloudfoundry.identity.uaa.approval.ApprovalsAdminEndpoints;
 import org.cloudfoundry.identity.uaa.approval.JdbcApprovalStore;
-import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
 import org.cloudfoundry.identity.uaa.error.UaaException;
-import org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus;
 import org.cloudfoundry.identity.uaa.resources.jdbc.JdbcPagingListFactory;
 import org.cloudfoundry.identity.uaa.resources.jdbc.SimpleSearchQueryConverter;
 import org.cloudfoundry.identity.uaa.security.SecurityContextAccessor;
@@ -48,14 +30,34 @@
 import org.cloudfoundry.identity.uaa.util.JsonUtils;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.security.oauth2.provider.NoSuchClientException;
 import org.springframework.security.oauth2.provider.client.BaseClientDetails;
 import org.springframework.security.oauth2.provider.client.InMemoryClientDetailsService;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.APPROVED;
+import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.DENIED;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 public class ApprovalsAdminEndpointsTests extends JdbcTestBase {
     private UaaTestAccounts testAccounts = null;
-    
+
     private JdbcApprovalStore dao;
 
     private UaaUserDatabase userDao = null;
@@ -64,6 +66,9 @@ public class ApprovalsAdminEndpointsTests extends JdbcTestBase {
 
     private ApprovalsAdminEndpoints endpoints;
 
+    @Rule
+    public ExpectedException exception = ExpectedException.none();
+
     @Before
     public void initApprovalsAdminEndpointsTests() {
         testAccounts = UaaTestAccounts.standard(null);
@@ -114,6 +119,21 @@ public void cleanupDataSource() throws Exception {
         assertThat(jdbcTemplate.queryForObject("select count(*) from users", Integer.class), is(0));
     }
 
+    @Test
+    public void validate_client_id_on_revoke() throws Exception {
+        exception.expect(NoSuchClientException.class);
+        exception.expectMessage("No client with requested id: invalid_id");
+        endpoints.revokeApprovals("invalid_id");
+    }
+
+    @Test
+    public void validate_client_id_on_update() throws Exception {
+        exception.expect(NoSuchClientException.class);
+        exception.expectMessage("No client with requested id: invalid_id");
+        endpoints.updateClientApprovals("invalid_id", new Approval[0]);
+    }
+
+
     @Test
     public void canGetApprovals() {
         addApproval(marissa.getId(), "c1", "uaa.user", 6000, APPROVED);

@@ -50,6 +50,7 @@
 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;
 
@@ -128,6 +129,37 @@ public void test_oauth_authorize_without_csrf() throws Exception {
             .andExpect(status().isFound()); //approval page no longer showing up
     }
 
+    @Test
+    public void revoke() throws Exception {
+        test_oauth_authorize_without_csrf();
+        MockHttpSession session = getAuthenticatedSession(user1);
+        getMockMvc().perform(
+            post("/profile")
+                .with(cookieCsrf())
+                .param("delete","true")
+                .param("clientId", client1.getClientId())
+                .session(session)
+        )
+            .andExpect(status().isFound())
+            .andExpect(header().string("Location", "profile"));
+
+    }
+
+    @Test
+    public void revoke_invalid_client() throws Exception {
+        test_oauth_authorize_without_csrf();
+        MockHttpSession session = getAuthenticatedSession(user1);
+        getMockMvc().perform(
+            post("/profile")
+                .with(cookieCsrf())
+                .param("delete","true")
+                .param("clientId", "invalid_id")
+                .session(session)
+        )
+            .andExpect(status().isFound())
+            .andExpect(header().string("Location", "profile?error_message_code=request.invalid_parameter"));
+    }
+
     @Test
     public void test_get_approvals() throws Exception {
         test_oauth_authorize_without_csrf();

@@ -12,21 +12,27 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.account;
 
-import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.cloudfoundry.identity.uaa.approval.Approval;
 import org.cloudfoundry.identity.uaa.approval.ApprovalsService;
 import org.cloudfoundry.identity.uaa.approval.DescribedApproval;
-import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
+import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
 import org.cloudfoundry.identity.uaa.constants.OriginKeys;
-import org.cloudfoundry.identity.uaa.approval.Approval;
+import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.oauth2.provider.ClientDetails;
 import org.springframework.security.oauth2.provider.ClientDetailsService;
+import org.springframework.security.oauth2.provider.NoSuchClientException;
 import org.springframework.stereotype.Controller;
 import org.springframework.ui.Model;
+import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.servlet.View;
+import org.springframework.web.servlet.view.RedirectView;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -37,6 +43,8 @@
 @Controller
 public class ProfileController {
 
+    protected static Log logger = LogFactory.getLog(ProfileController.class);
+
     private final ApprovalsService approvalsService;
     private final ClientDetailsService clientDetailsService;
 
@@ -108,6 +116,13 @@ else if (null != delete) {
         return "redirect:profile";
     }
 
+    @ExceptionHandler
+    public View handleException(NoSuchClientException nsce) {
+        logger.debug("Unable to find client for approvals:"+nsce.getMessage());
+        return new RedirectView("profile?error_message_code=request.invalid_parameter", true);
+    }
+
+
     private boolean isUaaManagedUser(Authentication authentication) {
         if (authentication.getPrincipal() instanceof UaaPrincipal) {
             UaaPrincipal principal = (UaaPrincipal) authentication.getPrincipal();

@@ -1,5 +1,5 @@
 /*******************************************************************************
- *     Cloud Foundry 
+ *     Cloud Foundry
  *     Copyright (c) [2009-2016] Pivotal Software, Inc. All Rights Reserved.
  *
  *     This product is licensed to you under the Apache License, Version 2.0 (the "License").
@@ -12,41 +12,45 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.approval;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
-import org.cloudfoundry.identity.uaa.web.ConvertingExceptionView;
-import org.cloudfoundry.identity.uaa.web.ExceptionReport;
 import org.cloudfoundry.identity.uaa.error.UaaException;
 import org.cloudfoundry.identity.uaa.resources.ActionResult;
 import org.cloudfoundry.identity.uaa.security.DefaultSecurityContextAccessor;
 import org.cloudfoundry.identity.uaa.security.SecurityContextAccessor;
 import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
 import org.cloudfoundry.identity.uaa.util.UaaPagingUtils;
+import org.cloudfoundry.identity.uaa.web.ConvertingExceptionView;
+import org.cloudfoundry.identity.uaa.web.ExceptionReport;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
-import org.springframework.security.oauth2.provider.ClientDetails;
 import org.springframework.security.oauth2.provider.ClientDetailsService;
+import org.springframework.security.oauth2.provider.NoSuchClientException;
 import org.springframework.security.oauth2.provider.client.BaseClientDetails;
 import org.springframework.stereotype.Controller;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
-import org.springframework.web.bind.annotation.*;
+import org.springframework.web.bind.annotation.ExceptionHandler;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.client.RestTemplate;
 import org.springframework.web.servlet.View;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 @Controller
 public class ApprovalsAdminEndpoints implements InitializingBean, ApprovalsControllerService {
 
@@ -168,6 +172,7 @@ public List<Approval> updateApprovals(@RequestBody Approval[] approvals) {
     @ResponseBody
     @Override
     public List<Approval> updateClientApprovals(@PathVariable String clientId, @RequestBody Approval[] approvals) {
+        clientDetailsService.loadClientByClientId(clientId);
         String currentUserId = getCurrentUserId();
         logger.debug("Updating approvals for user: " + currentUserId);
         approvalStore.revokeApprovals(String.format(USER_AND_CLIENT_FILTER_TEMPLATE, currentUserId, clientId));
@@ -202,10 +207,17 @@ private boolean isValidUser(String userId) {
     public ActionResult revokeApprovals(@RequestParam(required = true) String clientId) {
         String username = getCurrentUserId();
         logger.debug("Revoking all existing approvals for user: " + username + " and client " + clientId);
+        clientDetailsService.loadClientByClientId(clientId);
         approvalStore.revokeApprovals(String.format(USER_AND_CLIENT_FILTER_TEMPLATE, username, clientId));
         return new ActionResult("ok", "Approvals of user " + username + " and client " + clientId + " revoked");
     }
 
+    @ExceptionHandler
+    public View handleException(NoSuchClientException nsce) {
+        logger.debug("Client not found:"+nsce.getMessage());
+        return handleException(new UaaException(nsce.getMessage(), 404));
+    }
+
     @ExceptionHandler
     public View handleException(Exception t) {
         UaaException e = t instanceof UaaException ? (UaaException) t : new UaaException("Unexpected error",

@@ -1,5 +1,5 @@
 /*******************************************************************************
- *     Cloud Foundry 
+ *     Cloud Foundry
  *     Copyright (c) [2009-2016] Pivotal Software, Inc. All Rights Reserved.
  *
  *     This product is licensed to you under the Apache License, Version 2.0 (the "License").
@@ -12,30 +12,12 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.oauth.approval;
 
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Date;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
-import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.APPROVED;
-import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.DENIED;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
 import com.fasterxml.jackson.core.type.TypeReference;
 import org.cloudfoundry.identity.uaa.approval.Approval;
+import org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus;
 import org.cloudfoundry.identity.uaa.approval.ApprovalsAdminEndpoints;
 import org.cloudfoundry.identity.uaa.approval.JdbcApprovalStore;
-import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
 import org.cloudfoundry.identity.uaa.error.UaaException;
-import org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus;
 import org.cloudfoundry.identity.uaa.resources.jdbc.JdbcPagingListFactory;
 import org.cloudfoundry.identity.uaa.resources.jdbc.SimpleSearchQueryConverter;
 import org.cloudfoundry.identity.uaa.security.SecurityContextAccessor;
@@ -48,14 +30,34 @@
 import org.cloudfoundry.identity.uaa.util.JsonUtils;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.security.oauth2.provider.NoSuchClientException;
 import org.springframework.security.oauth2.provider.client.BaseClientDetails;
 import org.springframework.security.oauth2.provider.client.InMemoryClientDetailsService;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.APPROVED;
+import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.DENIED;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 public class ApprovalsAdminEndpointsTests extends JdbcTestBase {
     private UaaTestAccounts testAccounts = null;
-    
+
     private JdbcApprovalStore dao;
 
     private UaaUserDatabase userDao = null;
@@ -64,6 +66,9 @@ public class ApprovalsAdminEndpointsTests extends JdbcTestBase {
 
     private ApprovalsAdminEndpoints endpoints;
 
+    @Rule
+    public ExpectedException exception = ExpectedException.none();
+
     @Before
     public void initApprovalsAdminEndpointsTests() {
         testAccounts = UaaTestAccounts.standard(null);
@@ -114,6 +119,21 @@ public void cleanupDataSource() throws Exception {
         assertThat(jdbcTemplate.queryForObject("select count(*) from users", Integer.class), is(0));
     }
 
+    @Test
+    public void validate_client_id_on_revoke() throws Exception {
+        exception.expect(NoSuchClientException.class);
+        exception.expectMessage("No client with requested id: invalid_id");
+        endpoints.revokeApprovals("invalid_id");
+    }
+
+    @Test
+    public void validate_client_id_on_update() throws Exception {
+        exception.expect(NoSuchClientException.class);
+        exception.expectMessage("No client with requested id: invalid_id");
+        endpoints.updateClientApprovals("invalid_id", new Approval[0]);
+    }
+
+
     @Test
     public void canGetApprovals() {
         addApproval(marissa.getId(), "c1", "uaa.user", 6000, APPROVED);

@@ -50,6 +50,7 @@
 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;
 
@@ -128,6 +129,37 @@ public void test_oauth_authorize_without_csrf() throws Exception {
             .andExpect(status().isFound()); //approval page no longer showing up
     }
 
+    @Test
+    public void revoke() throws Exception {
+        test_oauth_authorize_without_csrf();
+        MockHttpSession session = getAuthenticatedSession(user1);
+        getMockMvc().perform(
+            post("/profile")
+                .with(cookieCsrf())
+                .param("delete","true")
+                .param("clientId", client1.getClientId())
+                .session(session)
+        )
+            .andExpect(status().isFound())
+            .andExpect(header().string("Location", "profile"));
+
+    }
+
+    @Test
+    public void revoke_invalid_client() throws Exception {
+        test_oauth_authorize_without_csrf();
+        MockHttpSession session = getAuthenticatedSession(user1);
+        getMockMvc().perform(
+            post("/profile")
+                .with(cookieCsrf())
+                .param("delete","true")
+                .param("clientId", "invalid_id")
+                .session(session)
+        )
+            .andExpect(status().isFound())
+            .andExpect(header().string("Location", "profile?error_message_code=request.invalid_parameter"));
+    }
+
     @Test
     public void test_get_approvals() throws Exception {
         test_oauth_authorize_without_csrf();