Validate client ID

[#143893217] https://www.pivotaltracker.com/story/show/143893217

common/src/main/java/org/cloudfoundry/identity/uaa/oauth/approval/ApprovalsAdminEndpoints.java

@@ -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",

login/src/main/java/org/cloudfoundry/identity/uaa/login/ProfileController.java

@@ -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);
+    }
 }

login/src/test/java/org/cloudfoundry/identity/uaa/login/ProfileControllerTests.java

@@ -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")

uaa/src/main/resources/messages.properties

@@ -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.

uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/approvals/ApprovalsMockMvcTests.java

@@ -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();