From a340c137d07d76b98ff84aad234160d8fb4e7586 Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Fri, 11 Oct 2019 15:30:54 +0100 Subject: [PATCH] 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); }); };