Require CSRF token in EditOneCellCommand

main/src/com/google/refine/commands/Command.java

@@ -37,6 +37,8 @@ import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.io.Writer;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.Properties;
 
 import javax.servlet.ServletException;
@@ -67,7 +69,7 @@ public abstract class Command {
 
     final static protected Logger logger = LoggerFactory.getLogger("command");
     
-    final static CSRFTokenFactory csrfFactory = new CSRFTokenFactory(3600, 32);
+    final static public CSRFTokenFactory csrfFactory = new CSRFTokenFactory(3600, 32);
 
     protected RefineServlet servlet;
     
@@ -217,6 +219,27 @@ public abstract class Command {
         return def;
     }
     
+    /**
+     * Utility method for retrieving the CSRF token stored in the "csrf_token" parameter of the request,
+     * and checking that it is valid.
+     *
+     * @param request
+     * @return
+     * @throws ServletException
+     */
+    protected boolean hasValidCSRFToken(HttpServletRequest request) throws ServletException {
+        if (request == null) {
+            throw new IllegalArgumentException("parameter 'request' should not be null");
+        }
+        try {
+            String token = request.getParameter("csrf_token");
+            return token != null && csrfFactory.validToken(token);
+        } catch (Exception e) {
+            // ignore
+        }
+        throw new ServletException("Can't find CSRF token: missing or bad URL parameter");
+    }
+    
     protected static class HistoryEntryResponse {
         @JsonProperty("code")
         protected String getCode() { return "ok"; }
@@ -299,6 +322,13 @@ public abstract class Command {
         w.flush();
         w.close();
     }
+    
+    static protected void respondCSRFError(HttpServletResponse response) throws IOException {
+    	Map<String, String> responseJSON = new HashMap<>();
+    	responseJSON.put("code", "error");
+    	responseJSON.put("message", "Missing or invalid csrf_token parameter");
+    	respondJSON(response, responseJSON);
+    }
 
     static protected void respondException(HttpServletResponse response, Exception e)
         throws IOException, ServletException {

main/src/com/google/refine/commands/browsing/ComputeClustersCommand.java

@@ -52,7 +52,11 @@ import com.google.refine.util.ParsingUtilities;
 public class ComputeClustersCommand extends Command {
 
     final static Logger logger = LoggerFactory.getLogger("compute-clusters_command");
-
+    
+    /**
+     * This command uses POST (probably to allow for larger parameters) but does not actually modify any state
+     * so we do not add CSRF protection to it.
+     */
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {

main/src/com/google/refine/commands/browsing/ComputeFacetsCommand.java

@@ -44,6 +44,11 @@ import com.google.refine.commands.Command;
 import com.google.refine.model.Project;
 
 public class ComputeFacetsCommand extends Command {
+	
+    /**
+     * This command uses POST (probably to allow for larger parameters) but does not actually modify any state
+     * so we do not add CSRF protection to it.
+     */
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {

main/src/com/google/refine/commands/cell/EditOneCellCommand.java

@@ -84,6 +84,10 @@ public class EditOneCellCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         try {
             request.setCharacterEncoding("UTF-8");

main/tests/server/src/com/google/refine/commands/cell/EditOneCellCommandTests.java

@@ -0,0 +1,78 @@
+package com.google.refine.commands.cell;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.testng.Assert.assertEquals;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.refine.RefineTest;
+import com.google.refine.commands.Command;
+import com.google.refine.model.Project;
+import com.google.refine.util.TestUtils;
+
+public class EditOneCellCommandTests extends RefineTest {
+	
+	protected Project project = null;
+	protected HttpServletRequest request = null;
+    protected HttpServletResponse response = null;
+    protected Command command = null;
+    protected StringWriter writer = null;
+	
+	@BeforeMethod
+	public void setUpProject() {
+		project = createCSVProject(
+				"first_column,second_column\n"
+				+ "a,b\n"
+				+ "c,d\n");
+		command = new EditOneCellCommand();
+		request = mock(HttpServletRequest.class);
+		response = mock(HttpServletResponse.class);
+		writer = new StringWriter();
+		try {
+            when(response.getWriter()).thenReturn(new PrintWriter(writer));
+        } catch (IOException e) {
+            e.printStackTrace();
+        }
+	}
+	
+	@Test
+	public void testEditOneCell() throws ServletException, IOException {
+		when(request.getParameter("project")).thenReturn(Long.toString(project.id));
+		when(request.getParameter("row")).thenReturn("1");
+		when(request.getParameter("cell")).thenReturn("0");
+		when(request.getParameter("type")).thenReturn("string");
+		when(request.getParameter("value")).thenReturn("e");
+		when(request.getParameter("csrf_token")).thenReturn(Command.csrfFactory.getFreshToken());
+		
+		command.doPost(request, response);
+		
+		assertEquals("a", project.rows.get(0).cells.get(0).value);
+		assertEquals("b", project.rows.get(0).cells.get(1).value);
+		assertEquals("e", project.rows.get(1).cells.get(0).value);
+		assertEquals("d", project.rows.get(1).cells.get(1).value);
+	}
+	
+	@Test
+	public void testMissingCSRFToken() throws ServletException, IOException {
+		when(request.getParameter("project")).thenReturn(Long.toString(project.id));
+		when(request.getParameter("row")).thenReturn("1");
+		when(request.getParameter("cell")).thenReturn("0");
+		when(request.getParameter("type")).thenReturn("string");
+		when(request.getParameter("value")).thenReturn("e");
+		
+		command.doPost(request, response);
+		
+		assertEquals("c", project.rows.get(1).cells.get(0).value);
+		TestUtils.assertEqualAsJson("{\"code\":\"error\",\"message\":\"Missing or invalid csrf_token parameter\"}", writer.toString());
+	}
+}

main/webapp/modules/core/MOD-INF/controller.js

@@ -54,6 +54,7 @@ function registerCommands() {
   var RS = Packages.com.google.refine.RefineServlet;
 
   RS.registerCommand(module, "get-version", new Packages.com.google.refine.commands.GetVersionCommand());
+  RS.registerCommand(module, "get-csrf-token", new Packages.com.google.refine.commands.GetCSRFTokenCommand());
 
   RS.registerCommand(module, "get-importing-configuration", new Packages.com.google.refine.commands.importing.GetImportingConfigurationCommand());
   RS.registerCommand(module, "create-importing-job", new Packages.com.google.refine.commands.importing.CreateImportingJobCommand());

main/webapp/modules/core/scripts/project.js

@@ -388,10 +388,21 @@ Refine.postProcess = function(moduleName, command, params, body, updateOptions,
 
   Refine.setAjaxInProgress();
 
-  $.post(
+  // Get a CSRF token first
-    "command/" + moduleName + "/" + command + "?" + $.param(params),
+  $.get(
-    body,
+    "command/core/get-csrf-token",
-    onDone,
+    {},
+    function(response) {
+
+        // Add it to the body and submit it as a POST request
+        body['csrf_token'] = response['token'];
+        $.post(
+            "command/" + moduleName + "/" + command + "?" + $.param(params),
+            body,
+            onDone,
+            "json"
+        );
+    },
     "json"
   );