Add test for csrf token for approvals pages

Implement csrf on approvals pages [#129374221] https://www.pivotaltracker.com/story/show/129374221
cloudfoundry · Aug 30, 2016 · f3d8a9e · f3d8a9e
1 parent 0eb1069
commit f3d8a9e
Show file tree

Hide file tree

Showing 16 changed files with 510 additions and 56 deletions.
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/approval/LoginUaaApprovalsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/approval/LoginUaaApprovalsService.java
@@ -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,11 +12,7 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.approval;
 
-import org.cloudfoundry.identity.uaa.approval.ApprovalsService;
-import org.cloudfoundry.identity.uaa.approval.DescribedApproval;
 import org.cloudfoundry.identity.uaa.constants.OriginKeys;
-import org.cloudfoundry.identity.uaa.approval.Approval;
-import org.cloudfoundry.identity.uaa.approval.ApprovalsControllerService;
 import org.springframework.beans.factory.annotation.Autowired;
 
 import java.util.ArrayList;

diff --git a/.../main/java/org/cloudfoundry/identity/uaa/security/web/CookieBasedCsrfTokenRepository.java b/.../main/java/org/cloudfoundry/identity/uaa/security/web/CookieBasedCsrfTokenRepository.java
@@ -84,6 +84,7 @@ public void saveToken(CsrfToken token, HttpServletRequest request, HttpServletRe
         Cookie csrfCookie = new Cookie(token.getParameterName(), token.getToken());
         csrfCookie.setHttpOnly(true);
         csrfCookie.setSecure(secure || request.getProtocol().equals("https"));
+
         if (expire) {
             csrfCookie.setMaxAge(0);
         } else {

diff --git a/server/src/main/resources/login-ui.xml b/server/src/main/resources/login-ui.xml
@@ -55,7 +55,7 @@
         <!-- TODO: add entry point that can redirect back to client app? -->
         <anonymous enabled="false" />
         <custom-filter ref="autologinAuthenticationFilter" position="FORM_LOGIN_FILTER" />
-        <csrf disabled="true"/>
+        <csrf disabled="false" token-repository-ref="loginCookieCsrfRepository"/>
     </http>
 
     <http name="secFilterCodeLogin" request-matcher-ref="autologinRequestMatcher" entry-point-ref="loginEntryPoint"
@@ -156,8 +156,22 @@
         <access-denied-handler ref="loginEntryPoint"/>
     </http>
 
-    <bean id="uiLoginRequestMatcher" class="org.springframework.security.web.util.matcher.AntPathRequestMatcher">
-        <constructor-arg value="/login.do" />
+    <bean id="uiCookeCsrfRequestMatcher" class="org.springframework.security.web.util.matcher.OrRequestMatcher">
+        <constructor-arg>
+        <list>
+            <bean class="org.springframework.security.web.util.matcher.AntPathRequestMatcher">
+                <constructor-arg value="/login.do" />
+            </bean>
+            <bean class="org.cloudfoundry.identity.uaa.security.web.UaaRequestMatcher">
+                <constructor-arg value="/oauth/authorize" />
+                <property name="method" value="POST"/>
+            </bean>
+            <bean class="org.cloudfoundry.identity.uaa.security.web.UaaRequestMatcher">
+                <constructor-arg value="/profile" />
+                <property name="method" value="POST"/>
+            </bean>
+        </list>
+        </constructor-arg>
     </bean>
 
     <bean id="uiLogoutRequestMatcher" class="org.springframework.security.web.util.matcher.AntPathRequestMatcher">
@@ -186,6 +200,9 @@
                 <bean class="org.springframework.security.web.util.matcher.AntPathRequestMatcher">
                     <constructor-arg value="/logout.do**" />
                 </bean>
+                <bean class="org.springframework.security.web.util.matcher.AntPathRequestMatcher">
+                    <constructor-arg value="/profile" />
+                </bean>
             </list>
         </constructor-arg>
     </bean>
@@ -231,7 +248,7 @@
       <!--<logout logout-url="/logout.do" success-handler-ref="logoutHandler" invalidate-session="true"/>-->
       <custom-filter ref="logoutFilter" after="LOGOUT_FILTER"/>
       <custom-filter ref="samlLogoutFilter" before="LOGOUT_FILTER"/>
-      <csrf disabled="false"  token-repository-ref="loginCookieCsrfRepository" request-matcher-ref="uiLoginRequestMatcher"/>
+      <csrf disabled="false"  token-repository-ref="loginCookieCsrfRepository" request-matcher-ref="uiCookeCsrfRequestMatcher"/>
       <access-denied-handler error-page="/login?error=invalid_login_request"/>
       <request-cache ref="savedRequestCache"/>
     </http>

diff --git a/...st/java/org/cloudfoundry/identity/uaa/integration/CheckTokenEndpointIntegrationTests.java b/...st/java/org/cloudfoundry/identity/uaa/integration/CheckTokenEndpointIntegrationTests.java
@@ -12,19 +12,10 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.integration;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-
-import java.net.URI;
-import java.util.Arrays;
-import java.util.Map;
-
 import org.cloudfoundry.identity.uaa.ServerRunning;
 import org.cloudfoundry.identity.uaa.integration.util.IntegrationTestUtils;
 import org.cloudfoundry.identity.uaa.test.TestAccountSetup;
 import org.cloudfoundry.identity.uaa.test.UaaTestAccounts;
-import org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository;
 import org.junit.Rule;
 import org.junit.Test;
 import org.springframework.http.HttpHeaders;
@@ -41,6 +32,15 @@
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
 
+import java.net.URI;
+import java.util.Arrays;
+import java.util.Map;
+
+import static org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
 /**
  * @author Dave Syer
  */
@@ -95,7 +95,7 @@ public void testDecodeToken() throws Exception {
         MultiValueMap<String, String> formData = new LinkedMultiValueMap<>();
         formData.add("username", testAccounts.getUserName());
         formData.add("password", testAccounts.getPassword());
-        formData.add(CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, csrf);
+        formData.add(DEFAULT_CSRF_COOKIE_NAME, csrf);
 
         // Should be redirected to the original URL, but now authenticated
         result = serverRunning.postForResponse("/login.do", headers, formData);
@@ -115,6 +115,7 @@ public void testDecodeToken() throws Exception {
 
             formData.clear();
             formData.add("user_oauth_approval", "true");
+            formData.add(DEFAULT_CSRF_COOKIE_NAME, IntegrationTestUtils.extractCookieCsrf(response.getBody()));
             result = serverRunning.postForResponse("/oauth/authorize", headers, formData);
             assertEquals(HttpStatus.FOUND, result.getStatusCode());
             location = result.getHeaders().getLocation().toString();

diff --git a/...oundry/identity/uaa/integration/OpenIdTokenAuthorizationWithApprovalIntegrationTests.java b/...oundry/identity/uaa/integration/OpenIdTokenAuthorizationWithApprovalIntegrationTests.java
@@ -56,6 +56,7 @@
 import java.util.Map;
 import java.util.Set;
 
+import static org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
@@ -275,6 +276,7 @@ private String doOpenIdHybridFlowIdTokenAndReturnCode(Set<String> responseTypes,
 
             formData.clear();
             formData.add("user_oauth_approval", "true");
+            formData.add(DEFAULT_CSRF_COOKIE_NAME, IntegrationTestUtils.extractCookieCsrf(response.getBody()));
             result = serverRunning.postForResponse("/oauth/authorize", headers, formData);
             assertEquals(HttpStatus.FOUND, result.getStatusCode());
             location = UriUtils.decode(result.getHeaders().getLocation().toString(), "UTF-8");

diff --git a/...t/java/org/cloudfoundry/identity/uaa/integration/RefreshTokenSupportIntegrationTests.java b/...t/java/org/cloudfoundry/identity/uaa/integration/RefreshTokenSupportIntegrationTests.java
@@ -15,7 +15,6 @@
 import org.cloudfoundry.identity.uaa.ServerRunning;
 import org.cloudfoundry.identity.uaa.integration.util.IntegrationTestUtils;
 import org.cloudfoundry.identity.uaa.oauth.jwt.JwtHelper;
-import org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository;
 import org.cloudfoundry.identity.uaa.test.TestAccountSetup;
 import org.cloudfoundry.identity.uaa.test.UaaTestAccounts;
 import org.junit.Before;
@@ -36,6 +35,7 @@
 import java.util.Arrays;
 import java.util.Map;
 
+import static org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -97,7 +97,7 @@ public void testTokenRefreshedCorrectFlow() throws Exception {
         MultiValueMap<String, String> formData = new LinkedMultiValueMap<String, String>();
         formData.add("username", testAccounts.getUserName());
         formData.add("password", testAccounts.getPassword());
-        formData.add(CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, IntegrationTestUtils.extractCookieCsrf(response.getBody()));
+        formData.add(DEFAULT_CSRF_COOKIE_NAME, IntegrationTestUtils.extractCookieCsrf(response.getBody()));
 
         // Should be redirected to the original URL, but now authenticated
         result = serverRunning.postForResponse("/login.do", headers, formData);
@@ -117,6 +117,7 @@ public void testTokenRefreshedCorrectFlow() throws Exception {
 
             formData.clear();
             formData.add("user_oauth_approval", "true");
+            formData.add(DEFAULT_CSRF_COOKIE_NAME, IntegrationTestUtils.extractCookieCsrf(response.getBody()));
             result = serverRunning.postForResponse("/oauth/authorize", headers, formData);
             assertEquals(HttpStatus.FOUND, result.getStatusCode());
             location = result.getHeaders().getLocation().toString();

diff --git a/...st/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java b/...st/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java
@@ -56,6 +56,7 @@
 import java.util.List;
 import java.util.Map;
 
+import static org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -519,6 +520,7 @@ private OAuth2AccessToken getAccessToken(String clientId, String clientSecret, S
 
             formData.clear();
             formData.add("user_oauth_approval", "true");
+            formData.add(DEFAULT_CSRF_COOKIE_NAME, IntegrationTestUtils.extractCookieCsrf(response.getBody()));
             formData.add("scope.0", "scope." + CFID);
             result = serverRunning.postForResponse("/oauth/authorize", headers, formData);
             assertEquals(HttpStatus.FOUND, result.getStatusCode());

diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/AutologinIT.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/AutologinIT.java
@@ -13,7 +13,6 @@
 package org.cloudfoundry.identity.uaa.integration.feature;
 
 import org.cloudfoundry.identity.uaa.integration.util.IntegrationTestUtils;
-import org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository;
 import org.cloudfoundry.identity.uaa.test.UaaTestAccounts;
 import org.junit.After;
 import org.junit.Assert;
@@ -44,6 +43,7 @@
 import java.util.List;
 import java.util.Map;
 
+import static org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME;
 import static org.junit.Assert.assertEquals;
 
 @RunWith(SpringJUnit4ClassRunner.class)
@@ -125,9 +125,9 @@ public void testSimpleAutologinFlow() throws Exception {
 
         //generate an autologin code with our credentials
         ResponseEntity<Map> autologinResponseEntity = restOperations.exchange(baseUrl + "/autologin",
-            HttpMethod.POST,
-            new HttpEntity<>(requestBody.toSingleValueMap(), headers),
-            Map.class);
+                                                                              HttpMethod.POST,
+                                                                              new HttpEntity<>(requestBody.toSingleValueMap(), headers),
+                                                                              Map.class);
         String autologinCode = (String) autologinResponseEntity.getBody().get("code");
 
         //start the authorization flow - this will issue a login event
@@ -143,28 +143,32 @@ public void testSimpleAutologinFlow() throws Exception {
         //rest template that does NOT follow redirects
         RestTemplate template = new RestTemplate(new DefaultIntegrationTestConfig.HttpClientFactory());
         headers.remove("Authorization");
-        ResponseEntity<Map> authorizeResponse = template.exchange(authorizeUrl,
-            HttpMethod.GET,
-            new HttpEntity<>(new HashMap<String,String>(),headers),
-            Map.class);
+        headers.add(HttpHeaders.ACCEPT, MediaType.TEXT_HTML_VALUE);
+        ResponseEntity<String> authorizeResponse =
+            template.exchange(authorizeUrl,
+                              HttpMethod.GET,
+                              new HttpEntity<>(new HashMap<String, String>(), headers),
+                              String.class);
 
 
         //we are now logged in. retrieve the JSESSIONID
         List<String> cookies = authorizeResponse.getHeaders().get("Set-Cookie");
-        assertEquals(1, cookies.size());
+        assertEquals(2, cookies.size());
         headers = getAppBasicAuthHttpHeaders();
         headers.add("Cookie", cookies.get(0));
+        headers.add("Cookie", cookies.get(1));
 
         //if we receive a 200, then we must approve our scopes
         if (HttpStatus.OK == authorizeResponse.getStatusCode()) {
             authorizeUrl = UriComponentsBuilder.fromHttpUrl(baseUrl)
                 .path("/oauth/authorize")
                 .queryParam("user_oauth_approval", "true")
+                .queryParam(DEFAULT_CSRF_COOKIE_NAME, IntegrationTestUtils.extractCookieCsrf(authorizeResponse.getBody()))
                 .build().toUriString();
             authorizeResponse = template.exchange(authorizeUrl,
-                HttpMethod.POST,
-                new HttpEntity<>(new HashMap<String,String>(),headers),
-                Map.class);
+                                                  HttpMethod.POST,
+                                                  new HttpEntity<>(new HashMap<String,String>(),headers),
+                                                  String.class);
         }
 
         //approval is complete, we receive a token code back
@@ -194,43 +198,49 @@ public void testSimpleAutologinFlow() throws Exception {
 
         headers.set(headers.ACCEPT, MediaType.TEXT_HTML_VALUE);
         ResponseEntity<String> loginResponse = template.exchange(baseUrl + "/login",
-            HttpMethod.GET,
-            new HttpEntity<>(null, headers),
-            String.class);
+                                                                 HttpMethod.GET,
+                                                                 new HttpEntity<>(null, headers),
+                                                                 String.class);
 
         if (loginResponse.getHeaders().containsKey("Set-Cookie")) {
             for (String cookie : loginResponse.getHeaders().get("Set-Cookie")) {
                 headers.add("Cookie", cookie);
             }
         }
         String csrf = IntegrationTestUtils.extractCookieCsrf(loginResponse.getBody());
-        requestBody.add(CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, csrf);
+        requestBody.add(DEFAULT_CSRF_COOKIE_NAME, csrf);
 
         headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED);
         loginResponse = restOperations.exchange(baseUrl + "/login.do",
-            HttpMethod.POST,
-            new HttpEntity<>(requestBody, headers),
-            String.class);
+                                                HttpMethod.POST,
+                                                new HttpEntity<>(requestBody, headers),
+                                                String.class);
         cookies = loginResponse.getHeaders().get("Set-Cookie");
         assertEquals(3, cookies.size());
         headers.clear();
         for (String cookie : loginResponse.getHeaders().get("Set-Cookie")) {
-            headers.add("Cookie", cookie);
+            if (!cookie.contains("1970")) { //deleted cookie
+                headers.add("Cookie", cookie);
+            }
         }
-        restOperations.exchange(baseUrl + "/profile",
-            HttpMethod.GET,
-            new HttpEntity<>(null, headers),Void.class);
+        headers.add(HttpHeaders.ACCEPT, MediaType.TEXT_HTML_VALUE);
+        ResponseEntity<String> profilePage =
+            restOperations.exchange(baseUrl + "/profile",
+                                    HttpMethod.GET,
+                                    new HttpEntity<>(null, headers), String.class);
+
 
         String revokeApprovalsUrl = UriComponentsBuilder.fromHttpUrl(baseUrl)
             .path("/profile")
             .build().toUriString();
         requestBody.clear();
         requestBody.add("clientId","app");
         requestBody.add("delete","");
+        requestBody.add(DEFAULT_CSRF_COOKIE_NAME, IntegrationTestUtils.extractCookieCsrf(profilePage.getBody()));
         ResponseEntity<Void> revokeResponse = template.exchange(revokeApprovalsUrl,
-            HttpMethod.POST,
-            new HttpEntity<>(requestBody, headers),
-            Void.class);
+                                                                HttpMethod.POST,
+                                                                new HttpEntity<>(requestBody, headers),
+                                                                Void.class);
         assertEquals(HttpStatus.FOUND, revokeResponse.getStatusCode());
     }
 

diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OpenIdTokenGrantsIT.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OpenIdTokenGrantsIT.java
@@ -58,6 +58,7 @@
 import java.util.Map;
 import java.util.Set;
 
+import static org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.is;
@@ -348,6 +349,7 @@ private void doOpenIdHybridFlowIdTokenAndCode(Set<String> responseTypes, String
 
             formData.clear();
             formData.add("user_oauth_approval", "true");
+            formData.add(DEFAULT_CSRF_COOKIE_NAME, IntegrationTestUtils.extractCookieCsrf(response.getBody()));
             result = restOperations.exchange(loginUrl + "/oauth/authorize", HttpMethod.POST, new HttpEntity<>(formData, headers), Void.class);
             assertEquals(HttpStatus.FOUND, result.getStatusCode());
             location = UriUtils.decode(result.getHeaders().getLocation().toString(), "UTF-8");

diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java
@@ -84,10 +84,12 @@
 import java.util.regex.Pattern;
 
 import static org.cloudfoundry.identity.uaa.provider.ExternalIdentityProviderDefinition.USER_NAME_ATTRIBUTE_PREFIX;
+import static org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
+
 public class IntegrationTestUtils {
 
     public static final DefaultResponseErrorHandler fiveHundredErrorHandler = new DefaultResponseErrorHandler(){
@@ -1060,6 +1062,7 @@ public static Map<String,String> getAuthorizationCodeTokenMap(ServerRunning serv
 
             formData.clear();
             formData.add("user_oauth_approval", "true");
+            formData.add(DEFAULT_CSRF_COOKIE_NAME, IntegrationTestUtils.extractCookieCsrf(response.getBody()));
             result = serverRunning.postForResponse("/oauth/authorize", headers, formData);
             assertEquals(HttpStatus.FOUND, result.getStatusCode());
             location = result.getHeaders().getLocation().toString();

diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/UaaJunitSuiteRunner.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/UaaJunitSuiteRunner.java
@@ -30,6 +30,7 @@
 import java.lang.reflect.Modifier;
 import java.util.List;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * Suite that runs classes that extend the
@@ -42,7 +43,10 @@ public class UaaJunitSuiteRunner extends Suite {
 
     protected static Class<?>[] allSuiteClasses() {
         Reflections reflections = new Reflections("org.cloudfoundry.identity.uaa");
-        Set<Class<? extends InjectedMockContextTest>> subTypes = reflections.getSubTypesOf(InjectedMockContextTest.class);
+        Set<Class<? extends InjectedMockContextTest>> subTypes =
+            reflections.getSubTypesOf(InjectedMockContextTest.class).stream().filter(
+                c -> !Modifier.isAbstract(c.getModifiers())
+            ).collect(Collectors.toSet());
         return subTypes.toArray(new Class[subTypes.size()]);
     }