CSRF protection for project and recon commands

OpenRefine · Oct 15, 2019 · 3559eeb · 3559eeb
1 parent a340c13
commit 3559eeb
Show file tree

Hide file tree

Showing 31 changed files with 256 additions and 47 deletions.
diff --git a/main/src/com/google/refine/commands/Command.java b/main/src/com/google/refine/commands/Command.java
@@ -240,6 +240,21 @@ protected boolean hasValidCSRFToken(HttpServletRequest request) throws ServletEx
         throw new ServletException("Can't find CSRF token: missing or bad URL parameter");
     }
 
+
+    /**
+     * Checks the validity of a CSRF token, without reading the whole POST body.
+     * Useful when we need to control how the POST body is read (for instance if it
+     * contains files).
+     */
+    protected boolean hasValidCSRFTokenAsGET(HttpServletRequest request) {
+        if (request == null) {
+            throw new IllegalArgumentException("parameter 'request' should not be null");
+        }
+    	Properties options = ParsingUtilities.parseUrlParameters(request);
+        String token = options.getProperty("csrf_token");
+        return token != null && csrfFactory.validToken(token);
+    }
+
     protected static class HistoryEntryResponse {
         @JsonProperty("code")
         protected String getCode() { return "ok"; }

diff --git a/main/src/com/google/refine/commands/importing/ImportingControllerCommand.java b/main/src/com/google/refine/commands/importing/ImportingControllerCommand.java
@@ -56,7 +56,7 @@ public class ImportingControllerCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
-    	if(!checkCSRF(request)) {
+    	if(!hasValidCSRFTokenAsGET(request)) {
     		respondCSRFError(response);
     		return;
     	}
@@ -96,14 +96,4 @@ private ImportingController getController(HttpServletRequest request) {
         }
         return null;
     }
-
-    /**
-     * Checks the validity of a CSRF token, without reading the whole POST body.
-     * See above for details.
-     */
-    private boolean checkCSRF(HttpServletRequest request) {
-    	Properties options = ParsingUtilities.parseUrlParameters(request);
-        String token = options.getProperty("csrf_token");
-        return token != null && csrfFactory.validToken(token);
-    }
 }
diff --git a/main/src/com/google/refine/commands/project/CreateProjectCommand.java b/main/src/com/google/refine/commands/project/CreateProjectCommand.java
@@ -64,6 +64,10 @@ public class CreateProjectCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFTokenAsGET(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         ProjectManager.singleton.setBusy(true);
         try {

diff --git a/main/src/com/google/refine/commands/project/DeleteProjectCommand.java b/main/src/com/google/refine/commands/project/DeleteProjectCommand.java
@@ -49,6 +49,11 @@ public class DeleteProjectCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
+
         response.setHeader("Content-Type", "application/json");
         try {
             long projectID = Long.parseLong(request.getParameter("project"));

diff --git a/main/src/com/google/refine/commands/project/ExportProjectCommand.java b/main/src/com/google/refine/commands/project/ExportProjectCommand.java
@@ -46,6 +46,10 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 import com.google.refine.model.Project;
 
 public class ExportProjectCommand extends Command {
+
+	/**
+	 * This command uses POST but is left CSRF-unprotected as it does not incur a state change.
+	 */
 
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)

diff --git a/main/src/com/google/refine/commands/project/ExportRowsCommand.java b/main/src/com/google/refine/commands/project/ExportRowsCommand.java
@@ -61,6 +61,10 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 
 public class ExportRowsCommand extends Command {
     private  static final Logger logger = LoggerFactory.getLogger("ExportRowsCommand");
+
+	/**
+	 * This command uses POST but is left CSRF-unprotected as it does not incur a state change.
+	 */
 
     @SuppressWarnings("unchecked")
     static public Properties getRequestParameters(HttpServletRequest request) {

diff --git a/main/src/com/google/refine/commands/project/GetModelsCommand.java b/main/src/com/google/refine/commands/project/GetModelsCommand.java
@@ -55,6 +55,11 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 import com.google.refine.model.RecordModel;
 
 public class GetModelsCommand extends Command {
+
+	/**
+	 * This command uses POST but is left CSRF-unprotected as it does not incur a state change.
+	 */
+
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {

diff --git a/main/src/com/google/refine/commands/project/ImportProjectCommand.java b/main/src/com/google/refine/commands/project/ImportProjectCommand.java
@@ -63,6 +63,10 @@ public class ImportProjectCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFTokenAsGET(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         ProjectManager.singleton.setBusy(true);
         try {

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

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

diff --git a/main/src/com/google/refine/commands/project/SetProjectTagsCommand.java b/main/src/com/google/refine/commands/project/SetProjectTagsCommand.java
@@ -43,6 +43,11 @@ public class SetProjectTagsCommand extends Command {
   @Override
   public void doPost(HttpServletRequest request, HttpServletResponse response)
       throws ServletException, IOException {
+  	  if(!hasValidCSRFToken(request)) {
+  		 respondCSRFError(response);
+		 return;
+  	  }
+
 	  response.setHeader("Content-Type", "application/json");
 
 	  Project project;

diff --git a/main/src/com/google/refine/commands/recon/GuessTypesOfColumnCommand.java b/main/src/com/google/refine/commands/recon/GuessTypesOfColumnCommand.java
@@ -93,6 +93,10 @@ protected TypesResponse(
     @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/recon/PreviewExtendDataCommand.java b/main/src/com/google/refine/commands/recon/PreviewExtendDataCommand.java
@@ -79,6 +79,10 @@ public PreviewResponse(List<ColumnInfo> columns2, List<List<Object>> rows2) {
     @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/recon/ReconClearOneCellCommand.java b/main/src/com/google/refine/commands/recon/ReconClearOneCellCommand.java
@@ -75,6 +75,10 @@ protected CellResponse(HistoryEntry historyEntry, Cell newCell, Pool newPool) {
     @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/recon/ReconJudgeOneCellCommand.java b/main/src/com/google/refine/commands/recon/ReconJudgeOneCellCommand.java
@@ -59,6 +59,10 @@ public class ReconJudgeOneCellCommand extends Command {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
+    	if(!hasValidCSRFToken(request)) {
+    		respondCSRFError(response);
+    		return;
+    	}
 
         try {
             request.setCharacterEncoding("UTF-8");

diff --git a/main/tests/server/src/com/google/refine/commands/project/ImportProjectCommandTests.java b/main/tests/server/src/com/google/refine/commands/project/ImportProjectCommandTests.java
@@ -0,0 +1,24 @@
+package com.google.refine.commands.project;
+
+import com.google.refine.commands.CommandTestBase;
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class ImportProjectCommandTests extends CommandTestBase {
+
+	@BeforeMethod
+	public void setUpCommand() {
+		command = new ImportProjectCommand();
+	}
+
+	@Test
+	public void testCSRFProtection() throws ServletException, IOException {
+		command.doPost(request, response);
+		assertCSRFCheckFailed();
+	}
+}
+
diff --git a/main/tests/server/src/com/google/refine/commands/project/RenameProjectCommandTests.java b/main/tests/server/src/com/google/refine/commands/project/RenameProjectCommandTests.java
@@ -0,0 +1,23 @@
+package com.google.refine.commands.project;
+
+import com.google.refine.commands.CommandTestBase;
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class RenameProjectCommandTests extends CommandTestBase {
+
+	@BeforeMethod
+	public void setUpCommand() {
+		command = new RenameProjectCommand();
+	}
+
+	@Test
+	public void testCSRFProtection() throws ServletException, IOException {
+		command.doPost(request, response);
+		assertCSRFCheckFailed();
+	}
+}
diff --git a/main/tests/server/src/com/google/refine/commands/project/SetProjectMetadataCommandTests.java b/main/tests/server/src/com/google/refine/commands/project/SetProjectMetadataCommandTests.java
@@ -58,6 +58,7 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 import com.google.refine.ProjectManager;
 import com.google.refine.ProjectMetadata;
 import com.google.refine.RefineTest;
+import com.google.refine.commands.Command;
 import com.google.refine.commands.project.SetProjectMetadataCommand;
 import com.google.refine.model.Project;
 import com.google.refine.util.ParsingUtilities;
@@ -101,6 +102,7 @@ public void SetUp() throws IOException {
 
         // mock dependencies
         when(request.getParameter("project")).thenReturn(PROJECT_ID);
+        when(request.getParameter("csrf_token")).thenReturn(Command.csrfFactory.getFreshToken());
         when(projMan.getProject(anyLong())).thenReturn(proj);
         when(proj.getMetadata()).thenReturn(metadata);
 

diff --git a/main/tests/server/src/com/google/refine/commands/project/SetProjectTagsCommandTests.java b/main/tests/server/src/com/google/refine/commands/project/SetProjectTagsCommandTests.java
@@ -0,0 +1,23 @@
+package com.google.refine.commands.project;
+
+import com.google.refine.commands.CommandTestBase;
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class SetProjectTagsCommandTests extends CommandTestBase {
+
+	@BeforeMethod
+	public void setUpCommand() {
+		command = new SetProjectTagsCommand();
+	}
+
+	@Test
+	public void testCSRFProtection() throws ServletException, IOException {
+		command.doPost(request, response);
+		assertCSRFCheckFailed();
+	}
+}
diff --git a/main/tests/server/src/com/google/refine/commands/recon/GuessTypesOfColumnCommandTests.java b/main/tests/server/src/com/google/refine/commands/recon/GuessTypesOfColumnCommandTests.java
@@ -0,0 +1,23 @@
+package com.google.refine.commands.recon;
+
+import com.google.refine.commands.CommandTestBase;
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class GuessTypesOfColumnCommandTests extends CommandTestBase {
+
+	@BeforeMethod
+	public void setUpCommand() {
+		command = new GuessTypesOfColumnCommand();
+	}
+
+	@Test
+	public void testCSRFProtection() throws ServletException, IOException {
+		command.doPost(request, response);
+		assertCSRFCheckFailed();
+	}
+}
diff --git a/main/tests/server/src/com/google/refine/commands/recon/PreviewExtendDataCommandTests.java b/main/tests/server/src/com/google/refine/commands/recon/PreviewExtendDataCommandTests.java
@@ -0,0 +1,23 @@
+package com.google.refine.commands.recon;
+
+import com.google.refine.commands.CommandTestBase;
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class PreviewExtendDataCommandTests extends CommandTestBase {
+
+	@BeforeMethod
+	public void setUpCommand() {
+		command = new PreviewExtendDataCommand();
+	}
+
+	@Test
+	public void testCSRFProtection() throws ServletException, IOException {
+		command.doPost(request, response);
+		assertCSRFCheckFailed();
+	}
+}
diff --git a/main/tests/server/src/com/google/refine/commands/recon/ReconClearOneCellCommandTests.java b/main/tests/server/src/com/google/refine/commands/recon/ReconClearOneCellCommandTests.java
@@ -0,0 +1,24 @@
+package com.google.refine.commands.recon;
+
+import com.google.refine.commands.CommandTestBase;
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class ReconClearOneCellCommandTests extends CommandTestBase {
+
+	@BeforeMethod
+	public void setUpCommand() {
+		command = new ReconClearOneCellCommand();
+	}
+
+	@Test
+	public void testCSRFProtection() throws ServletException, IOException {
+		command.doPost(request, response);
+		assertCSRFCheckFailed();
+	}
+}
+
diff --git a/main/tests/server/src/com/google/refine/commands/recon/ReconJudgeOneCellCommandTest.java b/main/tests/server/src/com/google/refine/commands/recon/ReconJudgeOneCellCommandTest.java
@@ -82,6 +82,7 @@ public void setUp() {
         response = mock(HttpServletResponse.class);
 
         when(request.getParameter("project")).thenReturn(String.valueOf(project.id));
+        when(request.getParameter("csrf_token")).thenReturn(Command.csrfFactory.getFreshToken());
 
         writer = mock(PrintWriter.class);
         try {

diff --git a/main/webapp/modules/core/MOD-INF/controller.js b/main/webapp/modules/core/MOD-INF/controller.js
@@ -69,7 +69,7 @@ function registerCommands() {
 
   RS.registerCommand(module, "get-project-metadata", new Packages.com.google.refine.commands.project.GetProjectMetadataCommand());
   RS.registerCommand(module, "get-all-project-metadata", new Packages.com.google.refine.commands.workspace.GetAllProjectMetadataCommand());
-  RS.registerCommand(module, "set-metaData", new Packages.com.google.refine.commands.project.SetProjectMetadataCommand());
+  RS.registerCommand(module, "set-project-metadata", new Packages.com.google.refine.commands.project.SetProjectMetadataCommand());
   RS.registerCommand(module, "get-all-project-tags", new Packages.com.google.refine.commands.workspace.GetAllProjectTagsCommand());
   RS.registerCommand(module, "set-project-tags", new Packages.com.google.refine.commands.project.SetProjectTagsCommand());
 

diff --git a/main/webapp/modules/core/scripts/dialogs/extend-data-preview-dialog.js b/main/webapp/modules/core/scripts/dialogs/extend-data-preview-dialog.js
@@ -185,7 +185,7 @@ ExtendReconciledDataPreviewDialog.prototype._update = function() {
     this._elmts.previewContainer.empty();
   } else {
     // otherwise, refresh the preview
-    $.post(
+    Refine.postCSRF(
         "command/core/preview-extend-data?" + $.param(params),
         {
         rowIndices: JSON.stringify(this._rowIndices),
@@ -194,10 +194,10 @@ ExtendReconciledDataPreviewDialog.prototype._update = function() {
         function(data) {
         self._renderPreview(data);
         },
-        "json"
-    ).fail(function(data) {
-        alert($.i18n('core-views/internal-err'));
-    });
+        "json",
+        function(data) {
+           alert($.i18n('core-views/internal-err'));
+        });
   }
 };