Add CSRF protection to cell, history, column and expr commands

OpenRefine · Oct 14, 2019 · 70e37b9 · 70e37b9
1 parent 51ddd27
commit 70e37b9
Show file tree

Hide file tree

Showing 43 changed files with 618 additions and 95 deletions.
diff --git a/main/src/com/google/refine/commands/EngineDependentCommand.java b/main/src/com/google/refine/commands/EngineDependentCommand.java
@@ -70,6 +70,10 @@ abstract public class EngineDependentCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         try {
             Project project = getProject(request);

diff --git a/main/src/com/google/refine/commands/GetAllPreferencesCommand.java b/main/src/com/google/refine/commands/GetAllPreferencesCommand.java
@@ -46,6 +46,10 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 import com.google.refine.preference.PreferenceStore;
 
 public class GetAllPreferencesCommand extends Command {
+	/**
+	 * The command uses POST (not sure why?) but does not actually modify any state
+	 * so it does not require CSRF.
+	 */
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {

diff --git a/main/src/com/google/refine/commands/cell/JoinMultiValueCellsCommand.java b/main/src/com/google/refine/commands/cell/JoinMultiValueCellsCommand.java
@@ -50,6 +50,10 @@ public class JoinMultiValueCellsCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         try {
             Project project = getProject(request);

diff --git a/main/src/com/google/refine/commands/cell/KeyValueColumnizeCommand.java b/main/src/com/google/refine/commands/cell/KeyValueColumnizeCommand.java
@@ -50,6 +50,10 @@ public class KeyValueColumnizeCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         try {
             Project project = getProject(request);

diff --git a/main/src/com/google/refine/commands/cell/SplitMultiValueCellsCommand.java b/main/src/com/google/refine/commands/cell/SplitMultiValueCellsCommand.java
@@ -52,6 +52,10 @@ public class SplitMultiValueCellsCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         try {
             Project project = getProject(request);

diff --git a/main/src/com/google/refine/commands/cell/TransposeColumnsIntoRowsCommand.java b/main/src/com/google/refine/commands/cell/TransposeColumnsIntoRowsCommand.java
@@ -50,6 +50,10 @@ public class TransposeColumnsIntoRowsCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         try {
             Project project = getProject(request);

diff --git a/main/src/com/google/refine/commands/cell/TransposeRowsIntoColumnsCommand.java b/main/src/com/google/refine/commands/cell/TransposeRowsIntoColumnsCommand.java
@@ -50,6 +50,10 @@ public class TransposeRowsIntoColumnsCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         try {
             Project project = getProject(request);

diff --git a/main/src/com/google/refine/commands/column/MoveColumnCommand.java b/main/src/com/google/refine/commands/column/MoveColumnCommand.java
@@ -50,6 +50,10 @@ public class MoveColumnCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         try {
             Project project = getProject(request);

diff --git a/main/src/com/google/refine/commands/column/RemoveColumnCommand.java b/main/src/com/google/refine/commands/column/RemoveColumnCommand.java
@@ -50,6 +50,10 @@ public class RemoveColumnCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         try {
             Project project = getProject(request);

diff --git a/main/src/com/google/refine/commands/column/RenameColumnCommand.java b/main/src/com/google/refine/commands/column/RenameColumnCommand.java
@@ -50,6 +50,10 @@ public class RenameColumnCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         try {
             Project project = getProject(request);

diff --git a/main/src/com/google/refine/commands/expr/LogExpressionCommand.java b/main/src/com/google/refine/commands/expr/LogExpressionCommand.java
@@ -48,7 +48,11 @@ public class LogExpressionCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
-
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
+
         try {
             String expression = request.getParameter("expression");
 

diff --git a/main/src/com/google/refine/commands/expr/PreviewExpressionCommand.java b/main/src/com/google/refine/commands/expr/PreviewExpressionCommand.java
@@ -111,6 +111,10 @@ public PreviewResult(List<ExpressionValue> evaluated) {
             this.results = evaluated;
         }
     }
+    /**
+     * The command uses POST but does not actually modify any state so it does
+     * not require CSRF.
+     */
 
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)

diff --git a/main/src/com/google/refine/commands/expr/ToggleStarredExpressionCommand.java b/main/src/com/google/refine/commands/expr/ToggleStarredExpressionCommand.java
@@ -40,6 +40,11 @@ public class ToggleStarredExpressionCommand extends Command {
 
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
+
         String expression = request.getParameter("expression");
 
         TopList starredExpressions = ((TopList) ProjectManager.singleton.getPreferenceStore().get(

diff --git a/main/src/com/google/refine/commands/history/ApplyOperationsCommand.java b/main/src/com/google/refine/commands/history/ApplyOperationsCommand.java
@@ -54,6 +54,10 @@ public class ApplyOperationsCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         Project project = getProject(request);
         String jsonString = request.getParameter("operations");

diff --git a/main/src/com/google/refine/commands/history/CancelProcessesCommand.java b/main/src/com/google/refine/commands/history/CancelProcessesCommand.java
@@ -53,6 +53,10 @@ public void doPost(HttpServletRequest request, HttpServletResponse response)
         if( response == null ) {
             throw new IllegalArgumentException("parameter 'request' should not be null");
         }
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         try {
             Project project = getProject(request);

diff --git a/main/src/com/google/refine/commands/history/UndoRedoCommand.java b/main/src/com/google/refine/commands/history/UndoRedoCommand.java
@@ -48,6 +48,10 @@ public class UndoRedoCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         Project project = getProject(request);
 

diff --git a/main/src/com/google/refine/commands/importing/CancelImportingJobCommand.java b/main/src/com/google/refine/commands/importing/CancelImportingJobCommand.java
@@ -48,6 +48,10 @@ public class CancelImportingJobCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         long jobID = Long.parseLong(request.getParameter("jobID"));
         ImportingJob job = ImportingManager.getJob(jobID);

diff --git a/main/src/com/google/refine/commands/importing/CreateImportingJobCommand.java b/main/src/com/google/refine/commands/importing/CreateImportingJobCommand.java
@@ -52,6 +52,10 @@ public class CreateImportingJobCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         long id = ImportingManager.createJob().id;
 

diff --git a/main/src/com/google/refine/commands/importing/GetImportingConfigurationCommand.java b/main/src/com/google/refine/commands/importing/GetImportingConfigurationCommand.java
@@ -49,6 +49,10 @@ public static class ConfigurationResponse {
         @JsonProperty("config")
         ImportingConfiguration config = new ImportingConfiguration();
     }
+    /**
+     * This command uses POST but does not actually modify any state so
+     * it is not CSRF-protected.
+     */
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {

diff --git a/main/src/com/google/refine/commands/importing/GetImportingJobStatusCommand.java b/main/src/com/google/refine/commands/importing/GetImportingJobStatusCommand.java
@@ -66,6 +66,10 @@ protected JobStatusResponse(String code, String message, ImportingJob job) {
         }
     }
 
+    /**
+     * This command uses POST but does not actually modify any state so
+     * it is not CSRF-protected.
+     */
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {

diff --git a/main/tests/server/src/com/google/refine/commands/CommandTestBase.java b/main/tests/server/src/com/google/refine/commands/CommandTestBase.java
@@ -0,0 +1,41 @@
+package com.google.refine.commands;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.testng.annotations.BeforeMethod;
+
+import com.google.refine.util.TestUtils;
+
+public class CommandTestBase {
+	protected HttpServletRequest request = null;
+    protected HttpServletResponse response = null;
+    protected Command command = null;
+    protected StringWriter writer = null;
+
+	@BeforeMethod
+	public void setUpRequestResponse() {
+		request = mock(HttpServletRequest.class);
+		response = mock(HttpServletResponse.class);
+		writer = new StringWriter();
+		try {
+            when(response.getWriter()).thenReturn(new PrintWriter(writer));
+        } catch (IOException e) {
+            e.printStackTrace();
+        }
+	}
+
+	/** 
+	 * Convenience method to check that CSRF protection was triggered
+	 */
+	protected void assertCSRFCheckFailed() {
+		TestUtils.assertEqualAsJson("{\"code\":\"error\",\"message\":\"Missing or invalid csrf_token parameter\"}", writer.toString());
+	}
+}
diff --git a/main/tests/server/src/com/google/refine/commands/EngineDependentCommandTests.java b/main/tests/server/src/com/google/refine/commands/EngineDependentCommandTests.java
@@ -0,0 +1,38 @@
+package com.google.refine.commands;
+
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.refine.browsing.EngineConfig;
+import com.google.refine.model.AbstractOperation;
+import com.google.refine.model.Project;
+
+public class EngineDependentCommandTests extends CommandTestBase {
+
+	private static class EngineDependentCommandStub extends EngineDependentCommand {
+
+		@Override
+		protected AbstractOperation createOperation(Project project, HttpServletRequest request,
+				EngineConfig engineConfig) throws Exception {
+			return null;
+		}
+
+	}
+
+	@BeforeMethod
+	public void setUpCommand() {
+		command = new EngineDependentCommandStub();
+	}
+
+	@Test
+	public void testCSRFProtection() throws ServletException, IOException {
+		command.doPost(request, response);
+		assertCSRFCheckFailed();
+	}
+}
+
diff --git a/main/tests/server/src/com/google/refine/commands/cell/JoinMultiValueCellsCommandTests.java b/main/tests/server/src/com/google/refine/commands/cell/JoinMultiValueCellsCommandTests.java
@@ -0,0 +1,25 @@
+package com.google.refine.commands.cell;
+
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.refine.commands.CommandTestBase;
+import com.google.refine.commands.cell.JoinMultiValueCellsCommand;
+
+public class JoinMultiValueCellsCommandTests extends CommandTestBase {
+
+	@BeforeMethod
+	public void setUpCommand() {
+		command = new JoinMultiValueCellsCommand();
+	}
+
+	@Test
+	public void testCSRFProtection() throws ServletException, IOException {
+		command.doPost(request, response);
+		assertCSRFCheckFailed();
+	}
+}
diff --git a/main/tests/server/src/com/google/refine/commands/cell/KeyValueColumnizeCommandTests.java b/main/tests/server/src/com/google/refine/commands/cell/KeyValueColumnizeCommandTests.java
@@ -0,0 +1,24 @@
+package com.google.refine.commands.cell;
+
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.refine.commands.CommandTestBase;
+
+public class KeyValueColumnizeCommandTests extends CommandTestBase {
+
+	@BeforeMethod
+	public void setUpCommand() {
+		command = new KeyValueColumnizeCommand();
+	}
+
+	@Test
+	public void testCSRFProtection() throws ServletException, IOException {
+		command.doPost(request, response);
+		assertCSRFCheckFailed();
+	}
+}
diff --git a/main/tests/server/src/com/google/refine/commands/cell/SplitMultiValueCellsCommandTests.java b/main/tests/server/src/com/google/refine/commands/cell/SplitMultiValueCellsCommandTests.java
@@ -0,0 +1,23 @@
+package com.google.refine.commands.cell;
+
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.refine.commands.CommandTestBase;
+
+public class SplitMultiValueCellsCommandTests extends CommandTestBase {
+	@BeforeMethod
+	public void setUpCommand() {
+		command = new SplitMultiValueCellsCommand();
+	}
+
+	@Test
+	public void testCSRFProtection() throws ServletException, IOException {
+		command.doPost(request, response);
+		assertCSRFCheckFailed();
+	}
+}