From 21b841a08909d3416916f37d13002656f6a5f7fd Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Fri, 11 Oct 2019 08:33:55 +0100 Subject: [PATCH 01/13] Add CSRF token generation capabilities, for #2164 --- .../refine/commands/CSRFTokenFactory.java | 94 +++++++++++++++++++ .../com/google/refine/commands/Command.java | 2 + .../refine/commands/GetCSRFTokenCommand.java | 18 ++++ .../commands/CSRFTokenFactoryTests.java | 49 ++++++++++ .../commands/GetCSRFTokenCommandTest.java | 49 ++++++++++ 5 files changed, 212 insertions(+) create mode 100644 main/src/com/google/refine/commands/CSRFTokenFactory.java create mode 100644 main/src/com/google/refine/commands/GetCSRFTokenCommand.java create mode 100644 main/tests/server/src/com/google/refine/commands/CSRFTokenFactoryTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/GetCSRFTokenCommandTest.java diff --git a/main/src/com/google/refine/commands/CSRFTokenFactory.java b/main/src/com/google/refine/commands/CSRFTokenFactory.java new file mode 100644 index 000000000..af9c1ac36 --- /dev/null +++ b/main/src/com/google/refine/commands/CSRFTokenFactory.java @@ -0,0 +1,94 @@ +package com.google.refine.commands; + +import java.time.Instant; +import java.util.Map; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; + +import org.apache.commons.lang.RandomStringUtils; + +import java.security.SecureRandom; + +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; + +/** + * Generates CSRF tokens and checks their validity. + * @author Antonin Delpeuch + * + */ +public class CSRFTokenFactory { + + /** + * Maps each token to the time it was generated + */ + protected final LoadingCache tokenCache; + + /** + * Time to live for tokens, in seconds + */ + protected final long timeToLive; + + /** + * Length of the tokens to generate + */ + protected final int tokenLength; + + /** + * Random number generator used to create tokens + */ + protected final SecureRandom rng; + + /** + * Constructs a new CSRF token factory. + * + * @param timeToLive + * Time to live for tokens, in seconds + * @param tokenLength + * Length of the tokens generated + */ + public CSRFTokenFactory(long timeToLive, int tokenLength) { + tokenCache = CacheBuilder.newBuilder() + .expireAfterWrite(timeToLive, TimeUnit.SECONDS) + .build( + new CacheLoader() { + @Override + public Instant load(String key) { + return Instant.now(); + } + + }); + this.timeToLive = timeToLive; + this.rng = new SecureRandom(); + this.tokenLength = tokenLength; + } + + /** + * Generates a fresh CSRF token, which will remain valid for the configured amount of time. + */ + public String getFreshToken() { + // Generate a random token + String token = RandomStringUtils.random(tokenLength, 0, 0, true, true, null, rng); + // Put it in the cache + try { + tokenCache.get(token); + } catch (ExecutionException e) { + // cannot happen + } + return token; + } + + /** + * Checks that a given CSRF token is valid. + * @param token + * the token to verify + * @return + * true if the token is valid + */ + public boolean validToken(String token) { + Map map = tokenCache.asMap(); + Instant cutoff = Instant.now().minusSeconds(timeToLive); + return map.containsKey(token) && map.get(token).isAfter(cutoff); + } +} diff --git a/main/src/com/google/refine/commands/Command.java b/main/src/com/google/refine/commands/Command.java index 5c605f7e5..bcee55a49 100644 --- a/main/src/com/google/refine/commands/Command.java +++ b/main/src/com/google/refine/commands/Command.java @@ -66,6 +66,8 @@ import com.google.refine.util.ParsingUtilities; public abstract class Command { final static protected Logger logger = LoggerFactory.getLogger("command"); + + final static CSRFTokenFactory csrfFactory = new CSRFTokenFactory(3600, 32); protected RefineServlet servlet; diff --git a/main/src/com/google/refine/commands/GetCSRFTokenCommand.java b/main/src/com/google/refine/commands/GetCSRFTokenCommand.java new file mode 100644 index 000000000..f5f668c35 --- /dev/null +++ b/main/src/com/google/refine/commands/GetCSRFTokenCommand.java @@ -0,0 +1,18 @@ +package com.google.refine.commands; + +import java.io.IOException; +import java.util.Collections; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +/** + * Generates a fresh CSRF token. + */ +public class GetCSRFTokenCommand extends Command { + @Override + public void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { + respondJSON(response, Collections.singletonMap("token", csrfFactory.getFreshToken())); + } +} diff --git a/main/tests/server/src/com/google/refine/commands/CSRFTokenFactoryTests.java b/main/tests/server/src/com/google/refine/commands/CSRFTokenFactoryTests.java new file mode 100644 index 000000000..f966e971b --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/CSRFTokenFactoryTests.java @@ -0,0 +1,49 @@ +package com.google.refine.commands; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import java.time.Instant; + +import org.testng.annotations.Test; + +public class CSRFTokenFactoryTests { + + static class CSRFTokenFactoryStub extends CSRFTokenFactory{ + public CSRFTokenFactoryStub(long timeToLive, int tokenLength) { + super(timeToLive, tokenLength); + } + public void tamperWithToken(String token, Instant newGenerationTime) { + tokenCache.asMap().put(token, newGenerationTime); + } + } + + @Test + public void testGenerateValidToken() { + CSRFTokenFactory factory = new CSRFTokenFactory(10, 25); + // Generate a fresh token + String token = factory.getFreshToken(); + // Immediately after, the token is still valid + assertTrue(factory.validToken(token)); + // The token has the right length + assertEquals(25, token.length()); + } + + @Test + public void testInvalidToken() { + CSRFTokenFactory factory = new CSRFTokenFactory(10, 25); + assertFalse(factory.validToken("bogusToken")); + } + + @Test + public void testOldToken() { + CSRFTokenFactoryStub stub = new CSRFTokenFactoryStub(10, 25); + // Generate a fresh token + String token = stub.getFreshToken(); + // Manually change the generation time + stub.tamperWithToken(token, Instant.now().minusSeconds(100)); + // The token should now be invalid + assertFalse(stub.validToken(token)); + } +} diff --git a/main/tests/server/src/com/google/refine/commands/GetCSRFTokenCommandTest.java b/main/tests/server/src/com/google/refine/commands/GetCSRFTokenCommandTest.java new file mode 100644 index 000000000..6133a2e91 --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/GetCSRFTokenCommandTest.java @@ -0,0 +1,49 @@ +package com.google.refine.commands; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertTrue; + +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.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.google.refine.util.ParsingUtilities; + +public class GetCSRFTokenCommandTest { + protected HttpServletRequest request = null; + protected HttpServletResponse response = null; + protected StringWriter writer = null; + protected Command command = null; + + @BeforeMethod + public void setUp() { + request = mock(HttpServletRequest.class); + response = mock(HttpServletResponse.class); + command = new GetCSRFTokenCommand(); + writer = new StringWriter(); + try { + when(response.getWriter()).thenReturn(new PrintWriter(writer)); + } catch (IOException e) { + e.printStackTrace(); + } + } + + @Test + public void testGetToken() throws JsonParseException, JsonMappingException, IOException, ServletException { + command.doGet(request, response); + ObjectNode result = ParsingUtilities.mapper.readValue(writer.toString(), ObjectNode.class); + String token = result.get("token").asText(); + assertTrue(Command.csrfFactory.validToken(token)); + } +} From 51ddd27909a97895a6f7d559f5ff8fb705cc4a71 Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Fri, 11 Oct 2019 09:50:01 +0100 Subject: [PATCH 02/13] Require CSRF token in EditOneCellCommand --- .../com/google/refine/commands/Command.java | 32 +++++++- .../browsing/ComputeClustersCommand.java | 6 +- .../browsing/ComputeFacetsCommand.java | 5 ++ .../commands/cell/EditOneCellCommand.java | 4 + .../cell/EditOneCellCommandTests.java | 78 +++++++++++++++++++ .../webapp/modules/core/MOD-INF/controller.js | 1 + main/webapp/modules/core/scripts/project.js | 19 ++++- 7 files changed, 139 insertions(+), 6 deletions(-) create mode 100644 main/tests/server/src/com/google/refine/commands/cell/EditOneCellCommandTests.java diff --git a/main/src/com/google/refine/commands/Command.java b/main/src/com/google/refine/commands/Command.java index bcee55a49..cc0fca5b3 100644 --- a/main/src/com/google/refine/commands/Command.java +++ b/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 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 { diff --git a/main/src/com/google/refine/commands/browsing/ComputeClustersCommand.java b/main/src/com/google/refine/commands/browsing/ComputeClustersCommand.java index bc86ae763..c499015f7 100644 --- a/main/src/com/google/refine/commands/browsing/ComputeClustersCommand.java +++ b/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 { diff --git a/main/src/com/google/refine/commands/browsing/ComputeFacetsCommand.java b/main/src/com/google/refine/commands/browsing/ComputeFacetsCommand.java index b0d91aef1..b27fbd1bc 100644 --- a/main/src/com/google/refine/commands/browsing/ComputeFacetsCommand.java +++ b/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 { diff --git a/main/src/com/google/refine/commands/cell/EditOneCellCommand.java b/main/src/com/google/refine/commands/cell/EditOneCellCommand.java index e299000ce..55cf39252 100644 --- a/main/src/com/google/refine/commands/cell/EditOneCellCommand.java +++ b/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"); diff --git a/main/tests/server/src/com/google/refine/commands/cell/EditOneCellCommandTests.java b/main/tests/server/src/com/google/refine/commands/cell/EditOneCellCommandTests.java new file mode 100644 index 000000000..d9fcc4c07 --- /dev/null +++ b/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()); + } +} diff --git a/main/webapp/modules/core/MOD-INF/controller.js b/main/webapp/modules/core/MOD-INF/controller.js index cae639734..588395b9b 100644 --- a/main/webapp/modules/core/MOD-INF/controller.js +++ b/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()); diff --git a/main/webapp/modules/core/scripts/project.js b/main/webapp/modules/core/scripts/project.js index 45e04132c..0f1bc8f65 100644 --- a/main/webapp/modules/core/scripts/project.js +++ b/main/webapp/modules/core/scripts/project.js @@ -388,10 +388,21 @@ Refine.postProcess = function(moduleName, command, params, body, updateOptions, Refine.setAjaxInProgress(); - $.post( - "command/" + moduleName + "/" + command + "?" + $.param(params), - body, - onDone, + // Get a CSRF token first + $.get( + "command/core/get-csrf-token", + {}, + 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" ); From 70e37b90853fc50160a8adbb3b1730e126ac421e Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Fri, 11 Oct 2019 12:24:44 +0100 Subject: [PATCH 03/13] Add CSRF protection to cell, history, column and expr commands --- .../commands/EngineDependentCommand.java | 4 + .../commands/GetAllPreferencesCommand.java | 4 + .../cell/JoinMultiValueCellsCommand.java | 4 + .../cell/KeyValueColumnizeCommand.java | 4 + .../cell/SplitMultiValueCellsCommand.java | 4 + .../cell/TransposeColumnsIntoRowsCommand.java | 4 + .../cell/TransposeRowsIntoColumnsCommand.java | 4 + .../commands/column/MoveColumnCommand.java | 4 + .../commands/column/RemoveColumnCommand.java | 4 + .../commands/column/RenameColumnCommand.java | 4 + .../commands/expr/LogExpressionCommand.java | 6 +- .../expr/PreviewExpressionCommand.java | 4 + .../expr/ToggleStarredExpressionCommand.java | 5 + .../history/ApplyOperationsCommand.java | 4 + .../history/CancelProcessesCommand.java | 4 + .../commands/history/UndoRedoCommand.java | 4 + .../importing/CancelImportingJobCommand.java | 4 + .../importing/CreateImportingJobCommand.java | 4 + .../GetImportingConfigurationCommand.java | 4 + .../GetImportingJobStatusCommand.java | 4 + .../refine/commands/CommandTestBase.java | 41 ++++++++ .../commands/EngineDependentCommandTests.java | 38 +++++++ .../cell/JoinMultiValueCellsCommandTests.java | 25 +++++ .../cell/KeyValueColumnizeCommandTests.java | 24 +++++ .../SplitMultiValueCellsCommandTests.java | 23 +++++ .../TransposeColumnsIntoRowsCommandTests.java | 23 +++++ .../TransposeRowsIntoColumnsCommandTests.java | 22 +++++ .../column/MoveColumnCommandTests.java | 24 +++++ .../column/RemoveColumnCommandTests.java | 23 +++++ .../column/RenameColumnCommandTests.java | 23 +++++ .../expr/LogExpressionCommandTests.java | 23 +++++ .../ToggleStarredExpressionCommandTests.java | 9 ++ .../history/ApplyOperationsCommandTests.java | 24 +++++ .../history/CancelProcessesCommandTests.java | 23 +++++ .../history/UndoRedoCommandTests.java | 23 +++++ .../CancelImportingJobCommandTests.java | 22 +++++ .../CreateImportingJobCommandTests.java | 23 +++++ .../util/CancelProcessesCommandTests.java | 4 + .../dialogs/expression-preview-dialog.js | 61 +++++++----- .../core/scripts/index/create-project-ui.js | 5 +- .../controller.js | 98 ++++++++++--------- main/webapp/modules/core/scripts/project.js | 25 +++-- .../core/scripts/project/process-panel.js | 20 ++-- 43 files changed, 616 insertions(+), 93 deletions(-) create mode 100644 main/tests/server/src/com/google/refine/commands/CommandTestBase.java create mode 100644 main/tests/server/src/com/google/refine/commands/EngineDependentCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/cell/JoinMultiValueCellsCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/cell/KeyValueColumnizeCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/cell/SplitMultiValueCellsCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/cell/TransposeColumnsIntoRowsCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/cell/TransposeRowsIntoColumnsCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/column/MoveColumnCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/column/RemoveColumnCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/column/RenameColumnCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/expr/LogExpressionCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/history/ApplyOperationsCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/history/CancelProcessesCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/history/UndoRedoCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/importing/CancelImportingJobCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/importing/CreateImportingJobCommandTests.java diff --git a/main/src/com/google/refine/commands/EngineDependentCommand.java b/main/src/com/google/refine/commands/EngineDependentCommand.java index a13330ae5..bc091803d 100644 --- 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 index cdf641e08..4cc2212a6 100644 --- a/main/src/com/google/refine/commands/GetAllPreferencesCommand.java +++ b/main/src/com/google/refine/commands/GetAllPreferencesCommand.java @@ -46,6 +46,10 @@ import com.google.refine.model.Project; 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 index 29d043d47..f65c1c0ee 100644 --- 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 index f941e5e8a..b926dfc4d 100644 --- 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 index 071614259..0918b04ba 100644 --- 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 index db26bd954..24c16b95f 100644 --- 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 index 7803d4860..8ace53158 100644 --- 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 index d88791763..75a5463ca 100644 --- 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 index 2a8e0915e..362691748 100644 --- 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 index 5ff00f811..725887c8b 100644 --- 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 index 818da044a..71df8ca24 100644 --- 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 index 1ba11ac7e..a31b8c94c 100644 --- 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 class PreviewExpressionCommand extends Command { 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 index 7a06011ed..fce0c7418 100644 --- 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 index df93d964f..c57ac44c3 100644 --- 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 index 8f1192bf6..d416efbde 100644 --- 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 class CancelProcessesCommand extends Command { 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 index aa7d4aa62..0bf9de91e 100644 --- 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 index c501ddcd3..7ec5cdd7e 100644 --- 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 index 27ed84979..3f8cea114 100644 --- 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 index 2899114d6..b31204a39 100644 --- 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 class GetImportingConfigurationCommand extends Command { @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 index 930b22032..fbfe4d0a6 100644 --- a/main/src/com/google/refine/commands/importing/GetImportingJobStatusCommand.java +++ b/main/src/com/google/refine/commands/importing/GetImportingJobStatusCommand.java @@ -66,6 +66,10 @@ public class GetImportingJobStatusCommand extends Command { } } + /** + * 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 new file mode 100644 index 000000000..7e7c84915 --- /dev/null +++ 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 new file mode 100644 index 000000000..923613aa3 --- /dev/null +++ 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 new file mode 100644 index 000000000..e9678d367 --- /dev/null +++ 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 new file mode 100644 index 000000000..efb533700 --- /dev/null +++ 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 new file mode 100644 index 000000000..ed3adbc54 --- /dev/null +++ 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(); + } +} diff --git a/main/tests/server/src/com/google/refine/commands/cell/TransposeColumnsIntoRowsCommandTests.java b/main/tests/server/src/com/google/refine/commands/cell/TransposeColumnsIntoRowsCommandTests.java new file mode 100644 index 000000000..c6562c624 --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/cell/TransposeColumnsIntoRowsCommandTests.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 TransposeColumnsIntoRowsCommandTests extends CommandTestBase { + @BeforeMethod + public void setUpCommand() { + command = new TransposeColumnsIntoRowsCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} diff --git a/main/tests/server/src/com/google/refine/commands/cell/TransposeRowsIntoColumnsCommandTests.java b/main/tests/server/src/com/google/refine/commands/cell/TransposeRowsIntoColumnsCommandTests.java new file mode 100644 index 000000000..105b1be31 --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/cell/TransposeRowsIntoColumnsCommandTests.java @@ -0,0 +1,22 @@ +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 TransposeRowsIntoColumnsCommandTests extends CommandTestBase { + @BeforeMethod + public void setUpCommand() { + command = new TransposeRowsIntoColumnsCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} diff --git a/main/tests/server/src/com/google/refine/commands/column/MoveColumnCommandTests.java b/main/tests/server/src/com/google/refine/commands/column/MoveColumnCommandTests.java new file mode 100644 index 000000000..da7f25b3b --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/column/MoveColumnCommandTests.java @@ -0,0 +1,24 @@ +package com.google.refine.commands.column; + +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 MoveColumnCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new MoveColumnCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} + diff --git a/main/tests/server/src/com/google/refine/commands/column/RemoveColumnCommandTests.java b/main/tests/server/src/com/google/refine/commands/column/RemoveColumnCommandTests.java new file mode 100644 index 000000000..d666f0c3c --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/column/RemoveColumnCommandTests.java @@ -0,0 +1,23 @@ +package com.google.refine.commands.column; + +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 RemoveColumnCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new RemoveColumnCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} diff --git a/main/tests/server/src/com/google/refine/commands/column/RenameColumnCommandTests.java b/main/tests/server/src/com/google/refine/commands/column/RenameColumnCommandTests.java new file mode 100644 index 000000000..a0d8d94bc --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/column/RenameColumnCommandTests.java @@ -0,0 +1,23 @@ +package com.google.refine.commands.column; + +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 RenameColumnCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new RenameColumnCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} diff --git a/main/tests/server/src/com/google/refine/commands/expr/LogExpressionCommandTests.java b/main/tests/server/src/com/google/refine/commands/expr/LogExpressionCommandTests.java new file mode 100644 index 000000000..1283820ef --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/expr/LogExpressionCommandTests.java @@ -0,0 +1,23 @@ +package com.google.refine.commands.expr; + +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 LogExpressionCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new LogExpressionCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} diff --git a/main/tests/server/src/com/google/refine/commands/expr/ToggleStarredExpressionCommandTests.java b/main/tests/server/src/com/google/refine/commands/expr/ToggleStarredExpressionCommandTests.java index 2b8376db4..d4166cd62 100644 --- a/main/tests/server/src/com/google/refine/commands/expr/ToggleStarredExpressionCommandTests.java +++ b/main/tests/server/src/com/google/refine/commands/expr/ToggleStarredExpressionCommandTests.java @@ -35,7 +35,9 @@ import javax.servlet.ServletException; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; +import com.google.refine.commands.Command; import com.google.refine.commands.expr.ToggleStarredExpressionCommand; +import com.google.refine.util.TestUtils; public class ToggleStarredExpressionCommandTests extends ExpressionCommandTestBase { @@ -70,7 +72,14 @@ public class ToggleStarredExpressionCommandTests extends ExpressionCommandTestBa " }"; when(request.getParameter("expression")).thenReturn("grel:facetCount(value, 'value', 'Column 1')"); when(request.getParameter("returnList")).thenReturn("yes"); + when(request.getParameter("csrf_token")).thenReturn(Command.csrfFactory.getFreshToken()); command.doPost(request, response); assertResponseJsonIs(json); } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + TestUtils.assertEqualAsJson("{\"code\":\"error\",\"message\":\"Missing or invalid csrf_token parameter\"}", writer.toString()); + } } diff --git a/main/tests/server/src/com/google/refine/commands/history/ApplyOperationsCommandTests.java b/main/tests/server/src/com/google/refine/commands/history/ApplyOperationsCommandTests.java new file mode 100644 index 000000000..d7422eb18 --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/history/ApplyOperationsCommandTests.java @@ -0,0 +1,24 @@ +package com.google.refine.commands.history; + +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 ApplyOperationsCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new ApplyOperationsCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} + diff --git a/main/tests/server/src/com/google/refine/commands/history/CancelProcessesCommandTests.java b/main/tests/server/src/com/google/refine/commands/history/CancelProcessesCommandTests.java new file mode 100644 index 000000000..dcf3ec07e --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/history/CancelProcessesCommandTests.java @@ -0,0 +1,23 @@ +package com.google.refine.commands.history; + +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 CancelProcessesCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new CancelProcessesCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} diff --git a/main/tests/server/src/com/google/refine/commands/history/UndoRedoCommandTests.java b/main/tests/server/src/com/google/refine/commands/history/UndoRedoCommandTests.java new file mode 100644 index 000000000..dc4ed459e --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/history/UndoRedoCommandTests.java @@ -0,0 +1,23 @@ +package com.google.refine.commands.history; + +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 UndoRedoCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new UndoRedoCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} diff --git a/main/tests/server/src/com/google/refine/commands/importing/CancelImportingJobCommandTests.java b/main/tests/server/src/com/google/refine/commands/importing/CancelImportingJobCommandTests.java new file mode 100644 index 000000000..b7c69e977 --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/importing/CancelImportingJobCommandTests.java @@ -0,0 +1,22 @@ +package com.google.refine.commands.importing; +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 CancelImportingJobCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new CancelImportingJobCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} diff --git a/main/tests/server/src/com/google/refine/commands/importing/CreateImportingJobCommandTests.java b/main/tests/server/src/com/google/refine/commands/importing/CreateImportingJobCommandTests.java new file mode 100644 index 000000000..70f44ff52 --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/importing/CreateImportingJobCommandTests.java @@ -0,0 +1,23 @@ +package com.google.refine.commands.importing; + +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 CreateImportingJobCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new CreateImportingJobCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} diff --git a/main/tests/server/src/com/google/refine/commands/util/CancelProcessesCommandTests.java b/main/tests/server/src/com/google/refine/commands/util/CancelProcessesCommandTests.java index 03cd8c70c..3be08e976 100644 --- a/main/tests/server/src/com/google/refine/commands/util/CancelProcessesCommandTests.java +++ b/main/tests/server/src/com/google/refine/commands/util/CancelProcessesCommandTests.java @@ -56,6 +56,7 @@ import org.testng.annotations.Test; import com.google.refine.ProjectManager; import com.google.refine.RefineTest; +import com.google.refine.commands.Command; import com.google.refine.commands.history.CancelProcessesCommand; import com.google.refine.model.Project; import com.google.refine.process.ProcessManager; @@ -159,6 +160,7 @@ public class CancelProcessesCommandTests extends RefineTest { // 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.getProcessManager()).thenReturn(processMan); try { @@ -197,6 +199,7 @@ public class CancelProcessesCommandTests extends RefineTest { public void doPostThrowsIfCommand_getProjectReturnsNull(){ // mock dependencies when(request.getParameter("project")).thenReturn(PROJECT_ID); + when(request.getParameter("csrf_token")).thenReturn(Command.csrfFactory.getFreshToken()); when(projMan.getProject(anyLong())) .thenReturn(null); try { @@ -225,6 +228,7 @@ public class CancelProcessesCommandTests extends RefineTest { // 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.getProcessManager()).thenReturn(processMan); try { diff --git a/main/webapp/modules/core/scripts/dialogs/expression-preview-dialog.js b/main/webapp/modules/core/scripts/dialogs/expression-preview-dialog.js index ae6b337fd..5554b5ca5 100644 --- a/main/webapp/modules/core/scripts/dialogs/expression-preview-dialog.js +++ b/main/webapp/modules/core/scripts/dialogs/expression-preview-dialog.js @@ -157,13 +157,15 @@ ExpressionPreviewDialog.Widget.prototype.getExpression = function(commit) { s = this._getLanguage() + ":" + s; if (commit) { - $.post( - "command/core/log-expression?" + $.param({ project: theProject.id }), - { expression: s }, - function(data) { - }, - "json" - ); + Refine.wrapCSRF(function(token) { + $.post( + "command/core/log-expression?" + $.param({ project: theProject.id }), + { expression: s, csrf_token: token }, + function(data) { + }, + "json" + ); + }); } return s; @@ -284,16 +286,21 @@ ExpressionPreviewDialog.Widget.prototype._renderExpressionHistory = function(dat .addClass(entry.starred ? "data-table-star-on" : "data-table-star-off") .appendTo(tr.insertCell(0)) .click(function() { - $.post( - "command/core/toggle-starred-expression", - { expression: entry.code }, - function(data) { - entry.starred = !entry.starred; - renderEntry(self,tr,entry); - self._renderStarredExpressionsTab(); - }, - "json" - ); + Refine.wrapCSRF(function(token) { + $.post( + "command/core/toggle-starred-expression", + { + expression: entry.code, + csrf_token: token + }, + function(data) { + entry.starred = !entry.starred; + renderEntry(self,tr,entry); + self._renderStarredExpressionsTab(); + }, + "json" + ); + }); }); $(''+$.i18n('core-dialogs/reuse')+'').appendTo(tr.insertCell(1)).click(function() { @@ -348,15 +355,17 @@ ExpressionPreviewDialog.Widget.prototype._renderStarredExpressions = function(da var o = Scripting.parse(entry.code); $(''+$.i18n('core-dialogs/remove')+'').appendTo(tr.insertCell(0)).click(function() { - $.post( - "command/core/toggle-starred-expression", - { expression: entry.code, returnList: true }, - function(data) { - self._renderStarredExpressions(data); - self._renderExpressionHistoryTab(); - }, - "json" - ); + Refine.wrapCSRF(function(token) { + $.post( + "command/core/toggle-starred-expression", + { expression: entry.code, returnList: true, csrf_token: token }, + function(data) { + self._renderStarredExpressions(data); + self._renderExpressionHistoryTab(); + }, + "json" + ); + }); }); $('Reuse').appendTo(tr.insertCell(1)).click(function() { diff --git a/main/webapp/modules/core/scripts/index/create-project-ui.js b/main/webapp/modules/core/scripts/index/create-project-ui.js index 7e97009a4..4b176830f 100644 --- a/main/webapp/modules/core/scripts/index/create-project-ui.js +++ b/main/webapp/modules/core/scripts/index/create-project-ui.js @@ -271,5 +271,8 @@ Refine.CreateProjectUI.composeErrorMessage = function(job) { }; Refine.CreateProjectUI.cancelImportingJob = function(jobID) { - $.post("command/core/cancel-importing-job?" + $.param({ "jobID": jobID })); + Refine.wrapCSRF(function(token) { + $.post("command/core/cancel-importing-job?" + $.param({ "jobID": jobID }), + {csrf_token: token}); + }); }; diff --git a/main/webapp/modules/core/scripts/index/default-importing-controller/controller.js b/main/webapp/modules/core/scripts/index/default-importing-controller/controller.js index 83e85ef74..031738616 100644 --- a/main/webapp/modules/core/scripts/index/default-importing-controller/controller.js +++ b/main/webapp/modules/core/scripts/index/default-importing-controller/controller.js @@ -78,61 +78,63 @@ Refine.DefaultImportingController.prototype.startImportJob = function(form, prog return this.value === ""; }).attr("disabled", "disabled"); - $.post( - "command/core/create-importing-job", - null, - function(data) { - var jobID = self._jobID = data.jobID; + Refine.wrapCSRF(function(token) { + $.post( + "command/core/create-importing-job", + { csrf_token: token }, + function(data) { + var jobID = self._jobID = data.jobID; - form.attr("method", "post") - .attr("enctype", "multipart/form-data") - .attr("accept-charset", "UTF-8") - .attr("target", "create-project-iframe") - .attr("action", "command/core/importing-controller?" + $.param({ - "controller": "core/default-importing-controller", - "jobID": jobID, - "subCommand": "load-raw-data" - })); - form[0].submit(); + form.attr("method", "post") + .attr("enctype", "multipart/form-data") + .attr("accept-charset", "UTF-8") + .attr("target", "create-project-iframe") + .attr("action", "command/core/importing-controller?" + $.param({ + "controller": "core/default-importing-controller", + "jobID": jobID, + "subCommand": "load-raw-data" + })); + form[0].submit(); - var start = new Date(); - var timerID = window.setInterval( - function() { - self._createProjectUI.pollImportJob( - start, jobID, timerID, - function(job) { - return job.config.hasData; - }, - function(jobID, job) { - self._job = job; - self._onImportJobReady(); - if (callback) { - callback(jobID, job); + var start = new Date(); + var timerID = window.setInterval( + function() { + self._createProjectUI.pollImportJob( + start, jobID, timerID, + function(job) { + return job.config.hasData; + }, + function(jobID, job) { + self._job = job; + self._onImportJobReady(); + if (callback) { + callback(jobID, job); + } + }, + function(job) { + alert(job.config.error + '\n' + job.config.errorDetails); + self._startOver(); } - }, - function(job) { - alert(job.config.error + '\n' + job.config.errorDetails); - self._startOver(); - } + ); + }, + 1000 ); - }, - 1000 - ); - self._createProjectUI.showImportProgressPanel(progressMessage, function() { - // stop the iframe - $('#create-project-iframe')[0].contentWindow.stop(); + self._createProjectUI.showImportProgressPanel(progressMessage, function() { + // stop the iframe + $('#create-project-iframe')[0].contentWindow.stop(); - // stop the timed polling - window.clearInterval(timerID); + // stop the timed polling + window.clearInterval(timerID); - // explicitly cancel the import job - Refine.CreateProjectUI.cancelImportingJob(jobID); + // explicitly cancel the import job + Refine.CreateProjectUI.cancelImportingJob(jobID); - self._createProjectUI.showSourceSelectionPanel(); - }); - }, - "json" - ); + self._createProjectUI.showSourceSelectionPanel(); + }); + }, + "json" + ); + }); }; Refine.DefaultImportingController.prototype._onImportJobReady = function() { diff --git a/main/webapp/modules/core/scripts/project.js b/main/webapp/modules/core/scripts/project.js index 0f1bc8f65..e5ede5e5d 100644 --- a/main/webapp/modules/core/scripts/project.js +++ b/main/webapp/modules/core/scripts/project.js @@ -388,22 +388,18 @@ Refine.postProcess = function(moduleName, command, params, body, updateOptions, Refine.setAjaxInProgress(); - // Get a CSRF token first - $.get( - "command/core/get-csrf-token", - {}, - function(response) { + Refine.wrapCSRF( + function(token) { // Add it to the body and submit it as a POST request - body['csrf_token'] = response['token']; + body['csrf_token'] = token; $.post( "command/" + moduleName + "/" + command + "?" + $.param(params), body, onDone, "json" ); - }, - "json" + } ); window.setTimeout(function() { @@ -413,6 +409,19 @@ Refine.postProcess = function(moduleName, command, params, body, updateOptions, }, 500); }; +// Requests a CSRF token and calls the supplied callback +// with the token +Refine.wrapCSRF = function(onCSRF) { + $.get( + "command/core/get-csrf-token", + {}, + function(response) { + onCSRF(response['token']); + }, + "json" + ); +}; + Refine.setAjaxInProgress = function() { $(document.body).attr("ajax_in_progress", "true"); }; diff --git a/main/webapp/modules/core/scripts/project/process-panel.js b/main/webapp/modules/core/scripts/project/process-panel.js index f0bd55b85..886ea5a28 100644 --- a/main/webapp/modules/core/scripts/project/process-panel.js +++ b/main/webapp/modules/core/scripts/project/process-panel.js @@ -124,15 +124,17 @@ ProcessPanel.prototype.undo = function() { ProcessPanel.prototype._cancelAll = function() { var self = this; - $.post( - "command/core/cancel-processes?" + $.param({ project: theProject.id }), - null, - function(o) { - self._data = null; - self._runOnDones(); - }, - "json" - ); + Refine.wrapCSRF(function(token) { + $.post( + "command/core/cancel-processes?" + $.param({ project: theProject.id }), + { csrf_token: token }, + function(o) { + self._data = null; + self._runOnDones(); + }, + "json" + ); + }); }; ProcessPanel.prototype._render = function(newData) { From 91cead27f85958d4a7340739a0f73358bc6254c3 Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Fri, 11 Oct 2019 13:53:43 +0100 Subject: [PATCH 04/13] CSRF protection for ImportingController --- .../importing/ImportingControllerCommand.java | 14 ++ .../ImportingControllerCommandTests.java | 24 +++ .../dialogs/expression-preview-dialog.js | 63 +++--- .../controller.js | 196 +++++++++--------- .../file-selection-panel.js | 51 ++--- main/webapp/modules/core/scripts/project.js | 28 +-- .../core/scripts/project/process-panel.js | 20 +- 7 files changed, 221 insertions(+), 175 deletions(-) create mode 100644 main/tests/server/src/com/google/refine/commands/importing/ImportingControllerCommandTests.java diff --git a/main/src/com/google/refine/commands/importing/ImportingControllerCommand.java b/main/src/com/google/refine/commands/importing/ImportingControllerCommand.java index 02a279db9..7474b3ff6 100644 --- a/main/src/com/google/refine/commands/importing/ImportingControllerCommand.java +++ b/main/src/com/google/refine/commands/importing/ImportingControllerCommand.java @@ -56,6 +56,10 @@ public class ImportingControllerCommand extends Command { @Override public void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { + if(!checkCSRF(request)) { + respondCSRFError(response); + return; + } ImportingController controller = getController(request); if (controller != null) { @@ -92,4 +96,14 @@ public class ImportingControllerCommand extends Command { } 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/tests/server/src/com/google/refine/commands/importing/ImportingControllerCommandTests.java b/main/tests/server/src/com/google/refine/commands/importing/ImportingControllerCommandTests.java new file mode 100644 index 000000000..7f3a29da6 --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/importing/ImportingControllerCommandTests.java @@ -0,0 +1,24 @@ +package com.google.refine.commands.importing; + +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 ImportingControllerCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new ImportingControllerCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} + diff --git a/main/webapp/modules/core/scripts/dialogs/expression-preview-dialog.js b/main/webapp/modules/core/scripts/dialogs/expression-preview-dialog.js index 5554b5ca5..78f77f284 100644 --- a/main/webapp/modules/core/scripts/dialogs/expression-preview-dialog.js +++ b/main/webapp/modules/core/scripts/dialogs/expression-preview-dialog.js @@ -157,15 +157,13 @@ ExpressionPreviewDialog.Widget.prototype.getExpression = function(commit) { s = this._getLanguage() + ":" + s; if (commit) { - Refine.wrapCSRF(function(token) { - $.post( - "command/core/log-expression?" + $.param({ project: theProject.id }), - { expression: s, csrf_token: token }, - function(data) { - }, - "json" - ); - }); + Refine.postCSRF( + "command/core/log-expression?" + $.param({ project: theProject.id }), + { expression: s }, + function(data) { + }, + "json" + ); } return s; @@ -286,21 +284,18 @@ ExpressionPreviewDialog.Widget.prototype._renderExpressionHistory = function(dat .addClass(entry.starred ? "data-table-star-on" : "data-table-star-off") .appendTo(tr.insertCell(0)) .click(function() { - Refine.wrapCSRF(function(token) { - $.post( - "command/core/toggle-starred-expression", - { - expression: entry.code, - csrf_token: token - }, - function(data) { - entry.starred = !entry.starred; - renderEntry(self,tr,entry); - self._renderStarredExpressionsTab(); - }, - "json" - ); - }); + Refine.postCSRF( + "command/core/toggle-starred-expression", + { + expression: entry.code + }, + function(data) { + entry.starred = !entry.starred; + renderEntry(self,tr,entry); + self._renderStarredExpressionsTab(); + }, + "json" + ); }); $(''+$.i18n('core-dialogs/reuse')+'').appendTo(tr.insertCell(1)).click(function() { @@ -355,17 +350,15 @@ ExpressionPreviewDialog.Widget.prototype._renderStarredExpressions = function(da var o = Scripting.parse(entry.code); $(''+$.i18n('core-dialogs/remove')+'').appendTo(tr.insertCell(0)).click(function() { - Refine.wrapCSRF(function(token) { - $.post( - "command/core/toggle-starred-expression", - { expression: entry.code, returnList: true, csrf_token: token }, - function(data) { - self._renderStarredExpressions(data); - self._renderExpressionHistoryTab(); - }, - "json" - ); - }); + Refine.postCSRF( + "command/core/toggle-starred-expression", + { expression: entry.code, returnList: true }, + function(data) { + self._renderStarredExpressions(data); + self._renderExpressionHistoryTab(); + }, + "json" + ); }); $('Reuse').appendTo(tr.insertCell(1)).click(function() { diff --git a/main/webapp/modules/core/scripts/index/default-importing-controller/controller.js b/main/webapp/modules/core/scripts/index/default-importing-controller/controller.js index 031738616..60d8d1ac4 100644 --- a/main/webapp/modules/core/scripts/index/default-importing-controller/controller.js +++ b/main/webapp/modules/core/scripts/index/default-importing-controller/controller.js @@ -92,7 +92,8 @@ Refine.DefaultImportingController.prototype.startImportJob = function(form, prog .attr("action", "command/core/importing-controller?" + $.param({ "controller": "core/default-importing-controller", "jobID": jobID, - "subCommand": "load-raw-data" + "subCommand": "load-raw-data", + "csrf_token": token })); form[0].submit(); @@ -182,27 +183,30 @@ Refine.DefaultImportingController.prototype._ensureFormatParserUIHasInitializati if (!(format in this._parserOptions)) { var self = this; var dismissBusy = DialogSystem.showBusy($.i18n('core-index-import/inspecting')); - $.post( - "command/core/importing-controller?" + $.param({ - "controller": "core/default-importing-controller", - "jobID": this._jobID, - "subCommand": "initialize-parser-ui", - "format": format - }), - null, - function(data) { - dismissBusy(); + Refine.wrapCSRF(function(token) { + $.post( + "command/core/importing-controller?" + $.param({ + "controller": "core/default-importing-controller", + "jobID": this._jobID, + "subCommand": "initialize-parser-ui", + "format": format, + "csrf_token": token + }), + null, + function(data) { + dismissBusy(); - if (data.options) { - self._parserOptions[format] = data.options; - onDone(); - } - }, - "json" - ) - .fail(function() { - dismissBusy(); - alert($.i18n('core-views/check-format')); + if (data.options) { + self._parserOptions[format] = data.options; + onDone(); + } + }, + "json" + ) + .fail(function() { + dismissBusy(); + alert($.i18n('core-views/check-format')); + }); }); } else { onDone(); @@ -211,32 +215,35 @@ Refine.DefaultImportingController.prototype._ensureFormatParserUIHasInitializati Refine.DefaultImportingController.prototype.updateFormatAndOptions = function(options, callback, finallyCallBack) { var self = this; - $.post( - "command/core/importing-controller?" + $.param({ - "controller": "core/default-importing-controller", - "jobID": this._jobID, - "subCommand": "update-format-and-options" - }), - { - "format" : this._format, - "options" : JSON.stringify(options) - }, - function(o) { - if (o.status == 'error') { - if (o.message) { - alert(o.message); + Refine.wrapCSRF(function(token) { + $.post( + "command/core/importing-controller?" + $.param({ + "controller": "core/default-importing-controller", + "jobID": this._jobID, + "subCommand": "update-format-and-options", + "csrf_token": token + }), + { + "format" : this._format, + "options" : JSON.stringify(options) + }, + function(o) { + if (o.status == 'error') { + if (o.message) { + alert(o.message); + } else { + var messages = []; + $.each(o.errors, function() { messages.push(this.message); }); + alert(messages.join('\n\n')); + } + finallyCallBack(); } else { - var messages = []; - $.each(o.errors, function() { messages.push(this.message); }); - alert(messages.join('\n\n')); + callback(o); } - finallyCallBack(); - } else { - callback(o); - } - }, - "json" - ); + }, + "json" + ); + }); }; Refine.DefaultImportingController.prototype.getPreviewData = function(callback, numRows) { @@ -286,56 +293,59 @@ Refine.DefaultImportingController.prototype._createProject = function() { var options = this._formatParserUI.getOptions(); options.projectName = projectName; options.projectTags = projectTags; - $.post( - "command/core/importing-controller?" + $.param({ - "controller": "core/default-importing-controller", - "jobID": this._jobID, - "subCommand": "create-project" - }), - { - "format" : this._format, - "options" : JSON.stringify(options) - }, - function(o) { - if (o.status == 'error') { - alert(o.message); - return; - } - - var start = new Date(); - var timerID = window.setInterval( - function() { - self._createProjectUI.pollImportJob( - start, - self._jobID, - timerID, - function(job) { - return "projectID" in job.config; - }, - function(jobID, job) { - Refine.CreateProjectUI.cancelImportingJob(jobID); - document.location = "project?project=" + job.config.projectID; - }, - function(job) { - alert($.i18n('core-index-import/errors')+'\n' + Refine.CreateProjectUI.composeErrorMessage(job)); - self._onImportJobReady(); - } + Refine.wrapCSRF(function(token) { + $.post( + "command/core/importing-controller?" + $.param({ + "controller": "core/default-importing-controller", + "jobID": this._jobID, + "subCommand": "create-project", + "csrf_token": token + }), + { + "format" : this._format, + "options" : JSON.stringify(options) + }, + function(o) { + if (o.status == 'error') { + alert(o.message); + return; + } + + var start = new Date(); + var timerID = window.setInterval( + function() { + self._createProjectUI.pollImportJob( + start, + self._jobID, + timerID, + function(job) { + return "projectID" in job.config; + }, + function(jobID, job) { + Refine.CreateProjectUI.cancelImportingJob(jobID); + document.location = "project?project=" + job.config.projectID; + }, + function(job) { + alert($.i18n('core-index-import/errors')+'\n' + Refine.CreateProjectUI.composeErrorMessage(job)); + self._onImportJobReady(); + } + ); + }, + 1000 ); - }, - 1000 + self._createProjectUI.showImportProgressPanel($.i18n('core-index-import/creating-proj'), function() { + // stop the timed polling + window.clearInterval(timerID); + + // explicitly cancel the import job + Refine.CreateProjectUI.cancelImportingJob(self._jobID); + + self._createProjectUI.showSourceSelectionPanel(); + }); + }, + "json" ); - self._createProjectUI.showImportProgressPanel($.i18n('core-index-import/creating-proj'), function() { - // stop the timed polling - window.clearInterval(timerID); - - // explicitly cancel the import job - Refine.CreateProjectUI.cancelImportingJob(self._jobID); - - self._createProjectUI.showSourceSelectionPanel(); - }); - }, - "json" - ); + }); } }; diff --git a/main/webapp/modules/core/scripts/index/default-importing-controller/file-selection-panel.js b/main/webapp/modules/core/scripts/index/default-importing-controller/file-selection-panel.js index 90c63b722..30469ec56 100644 --- a/main/webapp/modules/core/scripts/index/default-importing-controller/file-selection-panel.js +++ b/main/webapp/modules/core/scripts/index/default-importing-controller/file-selection-panel.js @@ -328,30 +328,33 @@ Refine.DefaultImportingController.prototype._commitFileSelection = function() { var self = this; var dismissBusy = DialogSystem.showBusy($.i18n('core-index-import/inspecting-files')); - $.post( - "command/core/importing-controller?" + $.param({ - "controller": "core/default-importing-controller", - "jobID": this._jobID, - "subCommand": "update-file-selection" - }), - { - "fileSelection" : JSON.stringify(this._job.config.fileSelection) - }, - function(data) { - dismissBusy(); + Refine.wrapCSRF(function(token) { + $.post( + "command/core/importing-controller?" + $.param({ + "controller": "core/default-importing-controller", + "jobID": this._jobID, + "subCommand": "update-file-selection", + "csrf_token": token + }), + { + "fileSelection" : JSON.stringify(this._job.config.fileSelection) + }, + function(data) { + dismissBusy(); - if (!(data)) { - self._createProjectUI.showImportJobError($.i18n('core-index-import/unknown-err')); - } else if (data.code == "error" || !("job" in data)) { - self._createProjectUI.showImportJobError((data.message) ? ($.i18n('core-index-import/error')+ ' ' + data.message) : $.i18n('core-index-import/unknown-err')); - } else { - // Different files might be selected. We start over again. - delete this._parserOptions; + if (!(data)) { + self._createProjectUI.showImportJobError($.i18n('core-index-import/unknown-err')); + } else if (data.code == "error" || !("job" in data)) { + self._createProjectUI.showImportJobError((data.message) ? ($.i18n('core-index-import/error')+ ' ' + data.message) : $.i18n('core-index-import/unknown-err')); + } else { + // Different files might be selected. We start over again. + delete this._parserOptions; - self._job = data.job; - self._showParsingPanel(true); - } - }, - "json" - ); + self._job = data.job; + self._showParsingPanel(true); + } + }, + "json" + ); + }); }; diff --git a/main/webapp/modules/core/scripts/project.js b/main/webapp/modules/core/scripts/project.js index e5ede5e5d..9a2f73166 100644 --- a/main/webapp/modules/core/scripts/project.js +++ b/main/webapp/modules/core/scripts/project.js @@ -388,18 +388,11 @@ Refine.postProcess = function(moduleName, command, params, body, updateOptions, Refine.setAjaxInProgress(); - Refine.wrapCSRF( - function(token) { - - // Add it to the body and submit it as a POST request - body['csrf_token'] = token; - $.post( - "command/" + moduleName + "/" + command + "?" + $.param(params), - body, - onDone, - "json" - ); - } + Refine.postCSRF( + "command/" + moduleName + "/" + command + "?" + $.param(params), + body, + onDone, + "json" ); window.setTimeout(function() { @@ -422,6 +415,17 @@ Refine.wrapCSRF = function(onCSRF) { ); }; +// Performs a POST request where an additional CSRF token +// is supplied in the POST data. The arguments match those +// of $.post(). +Refine.postCSRF = function(url, data, success, dataType) { + Refine.wrapCSRF(function(token) { + var fullData = data || {}; + data['csrf_token'] = token; + $.post(url, fulldata, success, dataType); + }); +}; + Refine.setAjaxInProgress = function() { $(document.body).attr("ajax_in_progress", "true"); }; diff --git a/main/webapp/modules/core/scripts/project/process-panel.js b/main/webapp/modules/core/scripts/project/process-panel.js index 886ea5a28..29d38ba35 100644 --- a/main/webapp/modules/core/scripts/project/process-panel.js +++ b/main/webapp/modules/core/scripts/project/process-panel.js @@ -124,17 +124,15 @@ ProcessPanel.prototype.undo = function() { ProcessPanel.prototype._cancelAll = function() { var self = this; - Refine.wrapCSRF(function(token) { - $.post( - "command/core/cancel-processes?" + $.param({ project: theProject.id }), - { csrf_token: token }, - function(o) { - self._data = null; - self._runOnDones(); - }, - "json" - ); - }); + Refine.postCSRF( + "command/core/cancel-processes?" + $.param({ project: theProject.id }), + { }, + function(o) { + self._data = null; + self._runOnDones(); + }, + "json" + ); }; ProcessPanel.prototype._render = function(newData) { From f7177e670d120f6da565aba0be089671ee2e18fa Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Tue, 15 Oct 2019 12:04:17 +0100 Subject: [PATCH 05/13] Adapt frontend for CSRF in import dialog --- main/webapp/modules/core/scripts/index.js | 20 +++++++++++++++++++ .../controller.js | 10 +++++----- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/main/webapp/modules/core/scripts/index.js b/main/webapp/modules/core/scripts/index.js index 1a22bfba2..f4aad9440 100644 --- a/main/webapp/modules/core/scripts/index.js +++ b/main/webapp/modules/core/scripts/index.js @@ -37,6 +37,26 @@ var Refine = { actionAreas: [] }; +Refine.wrapCSRF = function(onCSRF) { + $.get( + "command/core/get-csrf-token", + {}, + function(response) { + onCSRF(response['token']); + }, + "json" + ); +}; + +Refine.postCSRF = function(url, data, success, dataType) { + Refine.wrapCSRF(function(token) { + var fullData = data || {}; + data['csrf_token'] = token; + $.post(url, fulldata, success, dataType); + }); +}; + + var lang = (navigator.language|| navigator.userLanguage).split("-")[0]; var dictionary = ""; $.ajax({ diff --git a/main/webapp/modules/core/scripts/index/default-importing-controller/controller.js b/main/webapp/modules/core/scripts/index/default-importing-controller/controller.js index 60d8d1ac4..cb749bb79 100644 --- a/main/webapp/modules/core/scripts/index/default-importing-controller/controller.js +++ b/main/webapp/modules/core/scripts/index/default-importing-controller/controller.js @@ -187,7 +187,7 @@ Refine.DefaultImportingController.prototype._ensureFormatParserUIHasInitializati $.post( "command/core/importing-controller?" + $.param({ "controller": "core/default-importing-controller", - "jobID": this._jobID, + "jobID": self._jobID, "subCommand": "initialize-parser-ui", "format": format, "csrf_token": token @@ -219,12 +219,12 @@ Refine.DefaultImportingController.prototype.updateFormatAndOptions = function(op $.post( "command/core/importing-controller?" + $.param({ "controller": "core/default-importing-controller", - "jobID": this._jobID, + "jobID": self._jobID, "subCommand": "update-format-and-options", "csrf_token": token }), { - "format" : this._format, + "format" : self._format, "options" : JSON.stringify(options) }, function(o) { @@ -297,12 +297,12 @@ Refine.DefaultImportingController.prototype._createProject = function() { $.post( "command/core/importing-controller?" + $.param({ "controller": "core/default-importing-controller", - "jobID": this._jobID, + "jobID": self._jobID, "subCommand": "create-project", "csrf_token": token }), { - "format" : this._format, + "format" : self._format, "options" : JSON.stringify(options) }, function(o) { From a340c137d07d76b98ff84aad234160d8fb4e7586 Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Fri, 11 Oct 2019 15:30:54 +0100 Subject: [PATCH 06/13] CSRF protection for OpenWorkspaceDirCommand and language loading --- .../commands/OpenWorkspaceDirCommand.java | 4 ++ .../commands/lang/GetLanguagesCommand.java | 6 --- .../commands/lang/LoadLanguageCommand.java | 7 ++- .../OpenWorkspaceDirCommandTests.java | 23 +++++++++ .../lang/LoadLanguageCommandTests.java | 48 +++++++++++++++++++ main/webapp/modules/core/scripts/index.js | 10 ++-- .../core/scripts/index/open-project-ui.js | 14 +++--- main/webapp/modules/core/scripts/project.js | 4 +- 8 files changed, 97 insertions(+), 19 deletions(-) create mode 100644 main/tests/server/src/com/google/refine/commands/OpenWorkspaceDirCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/lang/LoadLanguageCommandTests.java diff --git a/main/src/com/google/refine/commands/OpenWorkspaceDirCommand.java b/main/src/com/google/refine/commands/OpenWorkspaceDirCommand.java index 49296707a..b09ffb0fe 100644 --- a/main/src/com/google/refine/commands/OpenWorkspaceDirCommand.java +++ b/main/src/com/google/refine/commands/OpenWorkspaceDirCommand.java @@ -48,6 +48,10 @@ public class OpenWorkspaceDirCommand extends Command { @Override public void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { + if(!hasValidCSRFToken(request)) { + respondCSRFError(response); + return; + } String serverName = request.getServerName(); diff --git a/main/src/com/google/refine/commands/lang/GetLanguagesCommand.java b/main/src/com/google/refine/commands/lang/GetLanguagesCommand.java index 7af237b3d..a07bf1e98 100644 --- a/main/src/com/google/refine/commands/lang/GetLanguagesCommand.java +++ b/main/src/com/google/refine/commands/lang/GetLanguagesCommand.java @@ -93,12 +93,6 @@ public class GetLanguagesCommand extends Command { @Override public void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - doPost(request, response); - } - - @Override - public void doPost(HttpServletRequest request, HttpServletResponse response) - throws ServletException, IOException { String modname = request.getParameter("module"); if (modname == null) { diff --git a/main/src/com/google/refine/commands/lang/LoadLanguageCommand.java b/main/src/com/google/refine/commands/lang/LoadLanguageCommand.java index 15336ec1d..892dab86d 100644 --- a/main/src/com/google/refine/commands/lang/LoadLanguageCommand.java +++ b/main/src/com/google/refine/commands/lang/LoadLanguageCommand.java @@ -62,11 +62,16 @@ public class LoadLanguageCommand extends Command { throws ServletException, IOException { doPost(request, response); } + + /** + * POST is supported but does not actually change any state so we do + * not add CSRF protection to it. This ensures existing extensions will not + * have to be updated to add a CSRF token to their requests (2019-11-10) + */ @Override public void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - String modname = request.getParameter("module"); if (modname == null) { modname = "core"; diff --git a/main/tests/server/src/com/google/refine/commands/OpenWorkspaceDirCommandTests.java b/main/tests/server/src/com/google/refine/commands/OpenWorkspaceDirCommandTests.java new file mode 100644 index 000000000..770b86a66 --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/OpenWorkspaceDirCommandTests.java @@ -0,0 +1,23 @@ +package com.google.refine.commands; +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 OpenWorkspaceDirCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new OpenWorkspaceDirCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} + diff --git a/main/tests/server/src/com/google/refine/commands/lang/LoadLanguageCommandTests.java b/main/tests/server/src/com/google/refine/commands/lang/LoadLanguageCommandTests.java new file mode 100644 index 000000000..f239673df --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/lang/LoadLanguageCommandTests.java @@ -0,0 +1,48 @@ +package com.google.refine.commands.lang; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertTrue; + +import com.fasterxml.jackson.databind.JsonNode; +import com.google.refine.RefineServlet; +import com.google.refine.commands.CommandTestBase; +import com.google.refine.util.ParsingUtilities; + +import edu.mit.simile.butterfly.ButterflyModule; + +import java.io.File; +import java.io.IOException; + +import javax.servlet.ServletException; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +public class LoadLanguageCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new LoadLanguageCommand(); + ButterflyModule coreModule = mock(ButterflyModule.class); + + when(coreModule.getName()).thenReturn("core"); + when(coreModule.getPath()).thenReturn(new File("webapp/modules/core")); + RefineServlet servlet = mock(RefineServlet.class); + when(servlet.getModule("core")).thenReturn(coreModule); + command.init(servlet); + } + + @Test + public void testLoadLanguages() throws ServletException, IOException { + when(request.getParameter("module")).thenReturn("core"); + when(request.getParameterValues("lang")).thenReturn(new String[] {"en"}); + + command.doPost(request, response); + + JsonNode response = ParsingUtilities.mapper.readValue(writer.toString(), JsonNode.class); + assertTrue(response.has("dictionary")); + assertTrue(response.has("lang")); + } +} + diff --git a/main/webapp/modules/core/scripts/index.js b/main/webapp/modules/core/scripts/index.js index f4aad9440..d1e101563 100644 --- a/main/webapp/modules/core/scripts/index.js +++ b/main/webapp/modules/core/scripts/index.js @@ -37,6 +37,8 @@ var Refine = { actionAreas: [] }; +// Requests a CSRF token and calls the supplied callback +// with the token Refine.wrapCSRF = function(onCSRF) { $.get( "command/core/get-csrf-token", @@ -48,15 +50,17 @@ Refine.wrapCSRF = function(onCSRF) { ); }; +// Performs a POST request where an additional CSRF token +// is supplied in the POST data. The arguments match those +// of $.post(). Refine.postCSRF = function(url, data, success, dataType) { Refine.wrapCSRF(function(token) { var fullData = data || {}; - data['csrf_token'] = token; - $.post(url, fulldata, success, dataType); + fullData['csrf_token'] = token; + $.post(url, fullData, success, dataType); }); }; - var lang = (navigator.language|| navigator.userLanguage).split("-")[0]; var dictionary = ""; $.ajax({ diff --git a/main/webapp/modules/core/scripts/index/open-project-ui.js b/main/webapp/modules/core/scripts/index/open-project-ui.js index c3340f950..498206994 100644 --- a/main/webapp/modules/core/scripts/index/open-project-ui.js +++ b/main/webapp/modules/core/scripts/index/open-project-ui.js @@ -59,16 +59,16 @@ Refine.OpenProjectUI = function(elmt) { $('#projects-workspace-open').text($.i18n('core-index-open/browse')); $('#projects-workspace-open').click(function() { - $.ajax({ - type: "POST", - url: "command/core/open-workspace-dir", - dataType: "json", - success: function (data) { + Refine.postCSRF( + "command/core/open-workspace-dir", + {}, + function (data) { if (data.code != "ok" && "message" in data) { alert(data.message); } - } - }); + }, + "json" + ); }); Refine.TagsManager.allProjectTags = []; this._buildTagsAndFetchProjects(); diff --git a/main/webapp/modules/core/scripts/project.js b/main/webapp/modules/core/scripts/project.js index 9a2f73166..dda3754f9 100644 --- a/main/webapp/modules/core/scripts/project.js +++ b/main/webapp/modules/core/scripts/project.js @@ -421,8 +421,8 @@ Refine.wrapCSRF = function(onCSRF) { Refine.postCSRF = function(url, data, success, dataType) { Refine.wrapCSRF(function(token) { var fullData = data || {}; - data['csrf_token'] = token; - $.post(url, fulldata, success, dataType); + fullData['csrf_token'] = token; + $.post(url, fullData, success, dataType); }); }; From 3559eeb11fa3f538d6057d9183378a427fdcd11d Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Mon, 14 Oct 2019 14:04:38 +0100 Subject: [PATCH 07/13] CSRF protection for project and recon commands --- .../com/google/refine/commands/Command.java | 15 ++++++++++++ .../importing/ImportingControllerCommand.java | 12 +--------- .../project/CreateProjectCommand.java | 4 ++++ .../project/DeleteProjectCommand.java | 5 ++++ .../project/ExportProjectCommand.java | 4 ++++ .../commands/project/ExportRowsCommand.java | 4 ++++ .../commands/project/GetModelsCommand.java | 5 ++++ .../project/ImportProjectCommand.java | 4 ++++ .../project/RenameProjectCommand.java | 6 ++++- .../project/SetProjectMetadataCommand.java | 4 ++++ .../project/SetProjectTagsCommand.java | 5 ++++ .../recon/GuessTypesOfColumnCommand.java | 4 ++++ .../recon/PreviewExtendDataCommand.java | 4 ++++ .../recon/ReconClearOneCellCommand.java | 4 ++++ .../recon/ReconJudgeOneCellCommand.java | 4 ++++ .../project/ImportProjectCommandTests.java | 24 +++++++++++++++++++ .../project/RenameProjectCommandTests.java | 23 ++++++++++++++++++ .../SetProjectMetadataCommandTests.java | 2 ++ .../project/SetProjectTagsCommandTests.java | 23 ++++++++++++++++++ .../recon/GuessTypesOfColumnCommandTests.java | 23 ++++++++++++++++++ .../recon/PreviewExtendDataCommandTests.java | 23 ++++++++++++++++++ .../recon/ReconClearOneCellCommandTests.java | 24 +++++++++++++++++++ .../recon/ReconJudgeOneCellCommandTest.java | 1 + .../webapp/modules/core/MOD-INF/controller.js | 2 +- .../dialogs/extend-data-preview-dialog.js | 10 ++++---- main/webapp/modules/core/scripts/index.js | 9 ++++--- .../scripts/index/edit-metadata-dialog.js | 16 ++++++------- .../core/scripts/index/import-project-ui.js | 4 ++++ .../core/scripts/index/open-project-ui.js | 15 ++++++------ main/webapp/modules/core/scripts/project.js | 15 ++++++------ .../reconciliation/standard-service-panel.js | 5 ++-- 31 files changed, 256 insertions(+), 47 deletions(-) create mode 100644 main/tests/server/src/com/google/refine/commands/project/ImportProjectCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/project/RenameProjectCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/project/SetProjectTagsCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/recon/GuessTypesOfColumnCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/recon/PreviewExtendDataCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/recon/ReconClearOneCellCommandTests.java diff --git a/main/src/com/google/refine/commands/Command.java b/main/src/com/google/refine/commands/Command.java index cc0fca5b3..eb42a23f0 100644 --- a/main/src/com/google/refine/commands/Command.java +++ b/main/src/com/google/refine/commands/Command.java @@ -240,6 +240,21 @@ public abstract class Command { 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 index 7474b3ff6..db7097a24 100644 --- 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 @@ public class ImportingControllerCommand extends Command { } 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 index f5f99dc24..8a0f133d5 100644 --- 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 index 9d146a01e..b04037198 100644 --- 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 index 3252f6a2a..ee129f7c9 100644 --- a/main/src/com/google/refine/commands/project/ExportProjectCommand.java +++ b/main/src/com/google/refine/commands/project/ExportProjectCommand.java @@ -46,6 +46,10 @@ import com.google.refine.io.FileProjectManager; 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 index 44c031da4..dd6444a1f 100644 --- a/main/src/com/google/refine/commands/project/ExportRowsCommand.java +++ b/main/src/com/google/refine/commands/project/ExportRowsCommand.java @@ -61,6 +61,10 @@ import com.google.refine.model.Project; 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 index ebf707d7b..b980154dc 100644 --- a/main/src/com/google/refine/commands/project/GetModelsCommand.java +++ b/main/src/com/google/refine/commands/project/GetModelsCommand.java @@ -55,6 +55,11 @@ import com.google.refine.model.Project; 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 index 740b72c9b..aac3ba489 100644 --- 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 index b8023df4a..b3365edf3 100644 --- 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 index c121c6b10..55297c3f3 100644 --- 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 index 71fbba7f1..b6a928dba 100644 --- 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 index 7e747706f..0d5f38951 100644 --- a/main/src/com/google/refine/commands/recon/GuessTypesOfColumnCommand.java +++ b/main/src/com/google/refine/commands/recon/GuessTypesOfColumnCommand.java @@ -93,6 +93,10 @@ public class GuessTypesOfColumnCommand 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/recon/PreviewExtendDataCommand.java b/main/src/com/google/refine/commands/recon/PreviewExtendDataCommand.java index 57bdffab8..6775f1c28 100644 --- 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 class PreviewExtendDataCommand 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/recon/ReconClearOneCellCommand.java b/main/src/com/google/refine/commands/recon/ReconClearOneCellCommand.java index 6234f872e..30dc67ec6 100644 --- a/main/src/com/google/refine/commands/recon/ReconClearOneCellCommand.java +++ b/main/src/com/google/refine/commands/recon/ReconClearOneCellCommand.java @@ -75,6 +75,10 @@ public class ReconClearOneCellCommand 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/recon/ReconJudgeOneCellCommand.java b/main/src/com/google/refine/commands/recon/ReconJudgeOneCellCommand.java index 778ec2cd4..f37dc1838 100644 --- 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 new file mode 100644 index 000000000..063f526e6 --- /dev/null +++ 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 new file mode 100644 index 000000000..4b60756e0 --- /dev/null +++ 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 index 3771bd05a..008dc08fb 100644 --- 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 @@ import com.fasterxml.jackson.databind.node.ObjectNode; 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 class SetProjectMetadataCommandTests extends RefineTest { // 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 new file mode 100644 index 000000000..3b073fac4 --- /dev/null +++ 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 new file mode 100644 index 000000000..03010589b --- /dev/null +++ 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 new file mode 100644 index 000000000..b4c04eddd --- /dev/null +++ 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 new file mode 100644 index 000000000..72ca77b73 --- /dev/null +++ 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 index 3dc741b6b..b53b11249 100644 --- 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 class ReconJudgeOneCellCommandTest extends RefineTest { 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 index 588395b9b..237c1ac3b 100644 --- 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 index d60d9440c..6acb515dc 100644 --- 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')); + }); } }; diff --git a/main/webapp/modules/core/scripts/index.js b/main/webapp/modules/core/scripts/index.js index d1e101563..682bcf826 100644 --- a/main/webapp/modules/core/scripts/index.js +++ b/main/webapp/modules/core/scripts/index.js @@ -53,11 +53,14 @@ Refine.wrapCSRF = function(onCSRF) { // Performs a POST request where an additional CSRF token // is supplied in the POST data. The arguments match those // of $.post(). -Refine.postCSRF = function(url, data, success, dataType) { - Refine.wrapCSRF(function(token) { +Refine.postCSRF = function(url, data, success, dataType, failCallback) { + return Refine.wrapCSRF(function(token) { var fullData = data || {}; fullData['csrf_token'] = token; - $.post(url, fullData, success, dataType); + var req = $.post(url, fullData, success, dataType); + if (failCallback !== undefined) { + req.fail(failCallback); + } }); }; diff --git a/main/webapp/modules/core/scripts/index/edit-metadata-dialog.js b/main/webapp/modules/core/scripts/index/edit-metadata-dialog.js index 6fb7ea275..b58c1c823 100644 --- a/main/webapp/modules/core/scripts/index/edit-metadata-dialog.js +++ b/main/webapp/modules/core/scripts/index/edit-metadata-dialog.js @@ -31,16 +31,16 @@ function EditMetadataDialog(metaData, targetRowElem) { if (newTags !== null) { $(td1).text(newTags); metaData[key] = newTags; - $.ajax({ - type : "POST", - url : "command/core/set-project-tags", - data : { + Refine.postCSRF( + "command/core/set-project-tags", + { "project" : project, "old" : oldTags, "new" : newTags }, - dataType : "json", - }); + function(data) {}, + "json" + ); } Refine.OpenProjectUI.refreshProject(targetRowElem, metaData, project); @@ -58,8 +58,8 @@ function EditMetadataDialog(metaData, targetRowElem) { if (newValue !== null) { $(td1).text(newValue); metaData[key] = newValue; - $.post( - "command/core/set-metaData", + Refine.postCSRF( + "command/core/set-project-metadata", { project : project, name : key, diff --git a/main/webapp/modules/core/scripts/index/import-project-ui.js b/main/webapp/modules/core/scripts/index/import-project-ui.js index 5229cd69e..3561611d0 100644 --- a/main/webapp/modules/core/scripts/index/import-project-ui.js +++ b/main/webapp/modules/core/scripts/index/import-project-ui.js @@ -33,6 +33,10 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. Refine.ImportProjectUI = function(elmt) { elmt.html(DOM.loadHTML("core", "scripts/index/import-project-ui.html")); + + Refine.wrapCSRF(function(token) { + elem.attr('action', "command/core/import-project?" + $.param({ csrf_token: token}); + }); this._elmt = elmt; this._elmts = DOM.bind(elmt); diff --git a/main/webapp/modules/core/scripts/index/open-project-ui.js b/main/webapp/modules/core/scripts/index/open-project-ui.js index 498206994..142c4f3bf 100644 --- a/main/webapp/modules/core/scripts/index/open-project-ui.js +++ b/main/webapp/modules/core/scripts/index/open-project-ui.js @@ -221,18 +221,17 @@ Refine.OpenProjectUI.prototype._renderProjects = function(data) { .html("") .click(function() { if (window.confirm($.i18n('core-index-open/del-body') + project.name + "\"?")) { - $.ajax({ - type: "POST", - url: "command/core/delete-project", - data: { "project" : project.id }, - dataType: "json", - success: function (data) { + Refine.postCSRF( + "command/core/delete-project", + { "project" : project.id }, + function (data) { if (data && typeof data.code != 'undefined' && data.code == "ok") { Refine.TagsManager.allProjectTags = []; self._buildTagsAndFetchProjects(); } - } - }); + }, + "json" + ); } return false; }).appendTo( diff --git a/main/webapp/modules/core/scripts/project.js b/main/webapp/modules/core/scripts/project.js index dda3754f9..b7d11895c 100644 --- a/main/webapp/modules/core/scripts/project.js +++ b/main/webapp/modules/core/scripts/project.js @@ -216,20 +216,19 @@ Refine._renameProject = function() { return; } - $.ajax({ - type: "POST", - url: "command/core/rename-project", - data: { "project" : theProject.id, "name" : name }, - dataType: "json", - success: function (data) { + Refine.postCSRF( + "command/core/rename-project", + { "project" : theProject.id, "name" : name }, + function (data) { if (data && typeof data.code != "undefined" && data.code == "ok") { theProject.metadata.name = name; Refine.setTitle(); } else { alert($.i18n('core-index/error-rename')+" " + data.message); } - } - }); + }, + "json" + ); }; /* diff --git a/main/webapp/modules/core/scripts/reconciliation/standard-service-panel.js b/main/webapp/modules/core/scripts/reconciliation/standard-service-panel.js index 9bdbdecec..2d2612168 100644 --- a/main/webapp/modules/core/scripts/reconciliation/standard-service-panel.js +++ b/main/webapp/modules/core/scripts/reconciliation/standard-service-panel.js @@ -44,7 +44,7 @@ ReconStandardServicePanel.prototype._guessTypes = function(f) { var self = this; var dismissBusy = DialogSystem.showBusy(); - $.post( + Refine.postCSRF( "command/core/guess-types-of-column?" + $.param({ project: theProject.id, columnName: this._column.name, @@ -74,7 +74,8 @@ ReconStandardServicePanel.prototype._guessTypes = function(f) { dismissBusy(); f(); - } + }, + "json" ); }; From 5dc005749aaef481184f4483919f3811a7e6c0f0 Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Mon, 14 Oct 2019 14:28:00 +0100 Subject: [PATCH 08/13] Add CSRF protection to remaining commands --- .../refine/commands/SetPreferenceCommand.java | 4 +++ .../commands/row/AnnotateOneRowCommand.java | 4 +++ .../commands/row/DenormalizeCommand.java | 4 +++ .../refine/commands/row/GetRowsCommand.java | 4 +++ .../commands/SetPreferenceCommandTests.java | 24 +++++++++++++++ .../row/AnnotateOneRowCommandTests.java | 22 ++++++++++++++ .../commands/row/DenormalizeCommandTests.java | 23 +++++++++++++++ .../modules/core/scripts/facets/list-facet.js | 2 +- .../core/scripts/index/lang-settings-ui.js | 29 ++++++++++--------- .../modules/core/scripts/preferences.js | 6 ++-- .../scripts/reconciliation/recon-manager.js | 27 ++++++++++------- 11 files changed, 121 insertions(+), 28 deletions(-) create mode 100644 main/tests/server/src/com/google/refine/commands/SetPreferenceCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/row/AnnotateOneRowCommandTests.java create mode 100644 main/tests/server/src/com/google/refine/commands/row/DenormalizeCommandTests.java diff --git a/main/src/com/google/refine/commands/SetPreferenceCommand.java b/main/src/com/google/refine/commands/SetPreferenceCommand.java index 43857e11f..f6c1d2c6a 100644 --- a/main/src/com/google/refine/commands/SetPreferenceCommand.java +++ b/main/src/com/google/refine/commands/SetPreferenceCommand.java @@ -49,6 +49,10 @@ public class SetPreferenceCommand 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; PreferenceStore ps = project != null ? diff --git a/main/src/com/google/refine/commands/row/AnnotateOneRowCommand.java b/main/src/com/google/refine/commands/row/AnnotateOneRowCommand.java index 6555f8b47..1d27cb819 100644 --- a/main/src/com/google/refine/commands/row/AnnotateOneRowCommand.java +++ b/main/src/com/google/refine/commands/row/AnnotateOneRowCommand.java @@ -50,6 +50,10 @@ public class AnnotateOneRowCommand extends Command { @Override public void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { + if(!hasValidCSRFToken(request)) { + respondCSRFError(response); + return; + } response.setCharacterEncoding("UTF-8"); response.setHeader("Content-Type", "application/json"); diff --git a/main/src/com/google/refine/commands/row/DenormalizeCommand.java b/main/src/com/google/refine/commands/row/DenormalizeCommand.java index 6ba38d6a1..0a13177d1 100644 --- a/main/src/com/google/refine/commands/row/DenormalizeCommand.java +++ b/main/src/com/google/refine/commands/row/DenormalizeCommand.java @@ -50,6 +50,10 @@ public class DenormalizeCommand 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/row/GetRowsCommand.java b/main/src/com/google/refine/commands/row/GetRowsCommand.java index cfeb21cdc..5467b4246 100644 --- a/main/src/com/google/refine/commands/row/GetRowsCommand.java +++ b/main/src/com/google/refine/commands/row/GetRowsCommand.java @@ -111,6 +111,10 @@ public class GetRowsCommand extends Command { } } + /** + * This command accepts both POST and GET. It is not CSRF-protected as it does not incur any state change. + */ + @Override public void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { diff --git a/main/tests/server/src/com/google/refine/commands/SetPreferenceCommandTests.java b/main/tests/server/src/com/google/refine/commands/SetPreferenceCommandTests.java new file mode 100644 index 000000000..9ad95db7c --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/SetPreferenceCommandTests.java @@ -0,0 +1,24 @@ +package com.google.refine.commands; + +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 SetPreferenceCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new SetPreferenceCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} + diff --git a/main/tests/server/src/com/google/refine/commands/row/AnnotateOneRowCommandTests.java b/main/tests/server/src/com/google/refine/commands/row/AnnotateOneRowCommandTests.java new file mode 100644 index 000000000..1b1e19e91 --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/row/AnnotateOneRowCommandTests.java @@ -0,0 +1,22 @@ +package com.google.refine.commands.row; +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 AnnotateOneRowCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new AnnotateOneRowCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} diff --git a/main/tests/server/src/com/google/refine/commands/row/DenormalizeCommandTests.java b/main/tests/server/src/com/google/refine/commands/row/DenormalizeCommandTests.java new file mode 100644 index 000000000..885806bc5 --- /dev/null +++ b/main/tests/server/src/com/google/refine/commands/row/DenormalizeCommandTests.java @@ -0,0 +1,23 @@ +package com.google.refine.commands.row; +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 DenormalizeCommandTests extends CommandTestBase { + + @BeforeMethod + public void setUpCommand() { + command = new DenormalizeCommand(); + } + + @Test + public void testCSRFProtection() throws ServletException, IOException { + command.doPost(request, response); + assertCSRFCheckFailed(); + } +} + diff --git a/main/webapp/modules/core/scripts/facets/list-facet.js b/main/webapp/modules/core/scripts/facets/list-facet.js index a02c57650..cdd7daccf 100644 --- a/main/webapp/modules/core/scripts/facets/list-facet.js +++ b/main/webapp/modules/core/scripts/facets/list-facet.js @@ -721,7 +721,7 @@ ListFacet.prototype._setChoiceCountLimit = function(choiceCount) { if (!isNaN(n)) { var self = this; - $.post( + Refine.postCSRF( "command/core/set-preference", { name : "ui.browsing.listFacet.limit", diff --git a/main/webapp/modules/core/scripts/index/lang-settings-ui.js b/main/webapp/modules/core/scripts/index/lang-settings-ui.js index b322ba825..ceb1bc8d1 100644 --- a/main/webapp/modules/core/scripts/index/lang-settings-ui.js +++ b/main/webapp/modules/core/scripts/index/lang-settings-ui.js @@ -28,19 +28,22 @@ Refine.SetLanguageUI = function(elmt) { }); this._elmts.set_lan_btn.bind('click', function(e) { - $.ajax({ - url : "command/core/set-preference?", - type : "POST", - async : false, - data : { - name : "userLang", - value : JSON.stringify($("#langDD option:selected").val()) - }, - success : function(data) { - alert($.i18n('core-index-lang/page-reload')); - location.reload(true); - } - }); + Refine.wrapCSRF(function(token) { + $.ajax({ + url : "command/core/set-preference?", + type : "POST", + async : false, + data : { + name : "userLang", + value : JSON.stringify($("#langDD option:selected").val()), + csrf_token: token + }, + success : function(data) { + alert($.i18n('core-index-lang/page-reload')); + location.reload(true); + } + }); + }); }); }; diff --git a/main/webapp/modules/core/scripts/preferences.js b/main/webapp/modules/core/scripts/preferences.js index 19bf905eb..2a799ae8f 100644 --- a/main/webapp/modules/core/scripts/preferences.js +++ b/main/webapp/modules/core/scripts/preferences.js @@ -78,7 +78,7 @@ function PreferenceUI(tr, key, value) { } $(td1).text(newValue); - $.post( + Refine.postCSRF( "command/core/set-preference", { name : key, @@ -96,7 +96,7 @@ function PreferenceUI(tr, key, value) { $('