Merge branch 'w2p-101573_CSRF-endpoint' into w2p-111801_CSRF-bugfix-7.6

dspace-server-webapp/src/main/java/org/dspace/app/rest/SecurityRestController.java

@@ -0,0 +1,37 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.app.rest;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.dspace.app.rest.security.DSpaceCsrfTokenRepository;
+import org.springframework.http.ResponseEntity;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RestController;
+
+/**
+ * Rest controller handling security related requests
+ */
+@RequestMapping(value = "/api/security")
+@RestController
+public class SecurityRestController {
+    DSpaceCsrfTokenRepository csrfTokenRepository = new DSpaceCsrfTokenRepository();
+
+    /**
+     * The csrf endpoint checks the current csrf cookie and header, resetting it if any of the two are missing, or
+     * they don't match anymore
+     */
+    @RequestMapping(value = "/csrf", method = RequestMethod.POST)
+    public ResponseEntity csrf(HttpServletRequest request, HttpServletResponse response) {
+        csrfTokenRepository.saveNewTokenWhenCookieAndHeaderDontMatch(request, response);
+        return ResponseEntity.ok().build();
+    }
+
+}

dspace-server-webapp/src/main/java/org/dspace/app/rest/StatisticsRestController.java

@@ -9,6 +9,8 @@
 
 import java.util.Arrays;
 import java.util.UUID;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import org.dspace.app.rest.converter.ConverterService;
 import org.dspace.app.rest.exception.RepositoryMethodNotImplementedException;
@@ -20,6 +22,7 @@
 import org.dspace.app.rest.repository.SearchEventRestRepository;
 import org.dspace.app.rest.repository.StatisticsRestRepository;
 import org.dspace.app.rest.repository.ViewEventRestRepository;
+import org.dspace.app.rest.security.DSpaceCsrfTokenRepository;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.rest.webmvc.ControllerUtils;
@@ -37,6 +40,7 @@
 @RestController
 @RequestMapping("/api/" + RestAddressableModel.STATISTICS)
 public class StatisticsRestController implements InitializingBean {
+    DSpaceCsrfTokenRepository csrfTokenRepository = new DSpaceCsrfTokenRepository();
 
     @Autowired
     private DiscoverableEndpointsService discoverableEndpointsService;
@@ -87,13 +91,21 @@ public PagedModel<SearchEventResource> getSearchEvents() throws Exception {
     }
 
     @RequestMapping(method = RequestMethod.POST, value = "/viewevents")
-    public ResponseEntity<RepresentationModel<?>> postViewEvent() throws Exception {
+    public ResponseEntity<RepresentationModel<?>> postViewEvent(HttpServletRequest request,
+                                                                HttpServletResponse response) throws Exception {
+        // WebSecurityConfiguration disabled CSRF for these endpoints, so they don't fail when the tokens are missing,
+        // but we should still add the proper response headers in these cases
+        csrfTokenRepository.saveNewTokenWhenCookieAndHeaderDontMatch(request, response);
         ViewEventResource result = converter.toResource(viewEventRestRepository.createViewEvent());
         return ControllerUtils.toResponseEntity(HttpStatus.CREATED, new HttpHeaders(), result);
     }
 
     @RequestMapping(method = RequestMethod.POST, value = "/searchevents")
-    public ResponseEntity<RepresentationModel<?>> postSearchEvent() throws Exception {
+    public ResponseEntity<RepresentationModel<?>> postSearchEvent(HttpServletRequest request,
+                                                                  HttpServletResponse response) throws Exception {
+        // WebSecurityConfiguration disabled CSRF for these endpoints, so they don't fail when the tokens are missing,
+        // but we should still add the proper response headers in these cases
+        csrfTokenRepository.saveNewTokenWhenCookieAndHeaderDontMatch(request, response);
         SearchEventResource result = converter.toResource(searchEventRestRepository.createSearchEvent());
         return ControllerUtils.toResponseEntity(HttpStatus.CREATED, new HttpHeaders(), result);
     }

dspace-server-webapp/src/main/java/org/dspace/app/rest/security/DSpaceCsrfTokenRepository.java

@@ -83,6 +83,9 @@ public CsrfToken generateToken(HttpServletRequest request) {
     /**
      * This method has been modified for DSpace.
      * <P>
+     * It filters out GET requests to avoid them refreshing the token when one is already being set by the csrf endpoint
+     * on SecurityRestController
+     * <P>
      * It now uses ResponseCookie to build the cookie, so that the "SameSite" attribute can be applied.
      * <P>
      * It also sends the token (if not empty) in both the cookie and the custom "DSPACE-XSRF-TOKEN" header
@@ -93,6 +96,19 @@ public CsrfToken generateToken(HttpServletRequest request) {
     @Override
     public void saveToken(CsrfToken token, HttpServletRequest request,
                           HttpServletResponse response) {
+        // Custom conditions on which to avoid certain default CSRF strategies to add csrf cookies to the response
+        if (request.getMethod().equals("GET") || request.getRequestURI().contains("/api/statistics/")) {
+            return;
+        }
+        saveTokenWithoutConditions(token, request, response);
+    }
+
+    /**
+     * Save the csrf token to the response (see saveToken method), assuming custom conditional checks have already been
+     * applied
+     */
+    public void saveTokenWithoutConditions(CsrfToken token, HttpServletRequest request,
+                          HttpServletResponse response) {
         String tokenValue = token == null ? "" : token.getToken();
         Cookie cookie = new Cookie(this.cookieName, tokenValue);
         cookie.setSecure(request.isSecure());
@@ -120,9 +136,9 @@ public void saveToken(CsrfToken token, HttpServletRequest request,
             sameSite = "Lax";
         }
         ResponseCookie responseCookie = ResponseCookie.from(cookie.getName(), cookie.getValue())
-                                              .path(cookie.getPath()).maxAge(cookie.getMaxAge())
-                                              .domain(cookie.getDomain()).httpOnly(cookie.isHttpOnly())
-                                              .secure(cookie.getSecure()).sameSite(sameSite).build();
+                .path(cookie.getPath()).maxAge(cookie.getMaxAge())
+                .domain(cookie.getDomain()).httpOnly(cookie.isHttpOnly())
+                .secure(cookie.getSecure()).sameSite(sameSite).build();
 
         // Write the ResponseCookie to the Set-Cookie header
         // This cookie is only used by the backend & not needed by client
@@ -136,6 +152,19 @@ public void saveToken(CsrfToken token, HttpServletRequest request,
         }
     }
 
+    /**
+     * Check the current CSRF token in the request's cookie and header and if any of them are missing, or they don't
+     * match, create a new token and save it on the response
+     */
+    public void saveNewTokenWhenCookieAndHeaderDontMatch(HttpServletRequest request, HttpServletResponse response) {
+        CsrfToken token = loadToken(request);
+        CsrfToken headerToken = loadTokenFromHeader(request);
+        if (token == null || headerToken == null || !token.getToken().equals(headerToken.getToken())) {
+            CsrfToken newToken = generateToken(request);
+            saveTokenWithoutConditions(newToken, request, response);
+        }
+    }
+
     @Override
     public CsrfToken loadToken(HttpServletRequest request) {
         Cookie cookie = WebUtils.getCookie(request, this.cookieName);
@@ -150,6 +179,19 @@ public CsrfToken loadToken(HttpServletRequest request) {
         return new DefaultCsrfToken(this.headerName, this.parameterName, token);
     }
 
+    /**
+     * Retrieve the csrf token from the header of the request, if present, otherwise return null
+     * As opposed to the regular loadToken method, which retrieves the token from the cookies instead
+     */
+    public CsrfToken loadTokenFromHeader(HttpServletRequest request) {
+        String token = request.getHeader(this.headerName);
+        if (!StringUtils.hasLength(token)) {
+            return null;
+        }
+
+        return new DefaultCsrfToken(this.headerName, this.parameterName, token);
+    }
+
 
     /**
      * Sets the name of the HTTP request parameter that should be used to provide a token.

dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityConfiguration.java

@@ -109,6 +109,9 @@ protected void configure(HttpSecurity http) throws Exception {
             // (both are defined below as methods).
             // While we primarily use JWT in headers, CSRF protection is needed because we also support JWT via Cookies
             .csrf()
+                // Allow statistics requests to go through without CSRF tokens
+                // (StatisticsRestController will still deal with setting the token in their responses, if invalid)
+                .ignoringAntMatchers("/api/statistics/**")
                 .csrfTokenRepository(this.csrfTokenRepository())
                 .sessionAuthenticationStrategy(this.sessionAuthenticationStrategy())
             .and()

dspace-server-webapp/src/test/java/org/dspace/app/rest/SecurityRestControllerIT.java

@@ -0,0 +1,86 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.app.rest;
+
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+
+import javax.servlet.http.Cookie;
+import javax.ws.rs.core.HttpHeaders;
+
+import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
+import org.hamcrest.core.StringContains;
+import org.junit.Test;
+
+public class SecurityRestControllerIT extends AbstractControllerIntegrationTest {
+
+    @Test
+    public void testCsrfEndpointWithoutCookieOrHeader() throws Exception {
+        String token = getAuthToken(admin.getEmail(), password);
+
+        getClient(token).perform(post("/api/security/csrf"))
+                .andExpect(status().isOk())
+                .andExpect(header().string(
+                        HttpHeaders.SET_COOKIE,
+                        StringContains.containsString("DSPACE-XSRF-COOKIE")
+                ));
+    }
+
+    @Test
+    public void testCsrfEndpointWithoutHeader() throws Exception {
+        String token = getAuthToken(admin.getEmail(), password);
+
+        getClient(token).perform(post("/api/security/csrf").cookie(
+                new Cookie("DSPACE-XSRF-COOKIE", "5f12f98b-5de3-4ee1-bd5f-95b784bd17cf")))
+                .andExpect(status().isOk())
+                .andExpect(header().string(
+                        HttpHeaders.SET_COOKIE,
+                        StringContains.containsString("DSPACE-XSRF-COOKIE")
+                ));
+    }
+
+    @Test
+    public void testCsrfEndpointWithoutCookie() throws Exception {
+        String token = getAuthToken(admin.getEmail(), password);
+
+        getClient(token).perform(post("/api/security/csrf").header("X-XSRF-TOKEN",
+                        "5f12f98b-5de3-4ee1-bd5f-95b784bd17cf"))
+                .andExpect(status().isOk())
+                .andExpect(header().string(
+                        HttpHeaders.SET_COOKIE,
+                        StringContains.containsString("DSPACE-XSRF-COOKIE")
+                ));
+    }
+
+    @Test
+    public void testCsrfEndpointMismatchingCookieAndHeader() throws Exception {
+        String token = getAuthToken(admin.getEmail(), password);
+
+        getClient(token).perform(post("/api/security/csrf").header("X-XSRF-TOKEN",
+                        "5f12f98b-5de3-4ee1-bd5f-95b784bd17cf").cookie(
+                        new Cookie("DSPACE-XSRF-COOKIE", "1427c36d-6d4c-423a-939d-8ddf0115b5c6")))
+                .andExpect(status().isOk())
+                .andExpect(header().string(
+                        HttpHeaders.SET_COOKIE,
+                        StringContains.containsString("DSPACE-XSRF-COOKIE")
+                ));
+    }
+
+    @Test
+    public void testCsrfEndpointMatchingCookieAndHeader() throws Exception {
+        String token = getAuthToken(admin.getEmail(), password);
+
+        getClient(token).perform(post("/api/security/csrf").header("X-XSRF-TOKEN",
+                        "5f12f98b-5de3-4ee1-bd5f-95b784bd17cf").cookie(
+                        new Cookie("DSPACE-XSRF-COOKIE", "5f12f98b-5de3-4ee1-bd5f-95b784bd17cf")))
+                .andExpect(status().isOk())
+                .andExpect(header().doesNotExist(HttpHeaders.SET_COOKIE));
+    }
+
+}