From 1ee5068f0d86ba838b94fb11e3c75916436e58ca Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Thu, 17 Oct 2019 10:42:05 +0100 Subject: [PATCH] CSRF protection for Wikidata extension --- .../scripts/dialogs/manage-account-dialog.js | 4 ++-- .../scripts/dialogs/perform-edits-dialog.js | 2 +- .../scripts/dialogs/schema-alignment-dialog.js | 2 +- .../wikidata/commands/LoginCommand.java | 5 +++++ .../commands/PreviewWikibaseSchemaCommand.java | 7 +++++++ .../commands/SaveWikibaseSchemaCommand.java | 4 ++++ .../wikidata/commands/LoginCommandTest.java | 12 ++++++++++++ .../commands/SaveWikibaseSchemaCommandTest.java | 16 ++++++++++++++++ .../wikidata/commands/SchemaCommandTest.java | 5 ++++- main/webapp/modules/core/scripts/index.js | 6 +++++- main/webapp/modules/core/scripts/preferences.js | 6 +++++- main/webapp/modules/core/scripts/project.js | 6 +++++- 12 files changed, 67 insertions(+), 8 deletions(-) diff --git a/extensions/wikidata/module/scripts/dialogs/manage-account-dialog.js b/extensions/wikidata/module/scripts/dialogs/manage-account-dialog.js index 22fe0d35e..fb35fd32e 100644 --- a/extensions/wikidata/module/scripts/dialogs/manage-account-dialog.js +++ b/extensions/wikidata/module/scripts/dialogs/manage-account-dialog.js @@ -56,7 +56,7 @@ ManageAccountDialog.display = function(logged_in_username, saved_credentials, ca elmts.loginButton.click(function() { frame.hide(); - $.post( + Refine.postCSRF( "command/wikidata/login", elmts.loginForm.serialize(), function(data) { @@ -71,7 +71,7 @@ ManageAccountDialog.display = function(logged_in_username, saved_credentials, ca }); elmts.logoutButton.click(function() { - $.post( + Refine.postCSRF( "command/wikidata/login", "logout=true", function(data) { diff --git a/extensions/wikidata/module/scripts/dialogs/perform-edits-dialog.js b/extensions/wikidata/module/scripts/dialogs/perform-edits-dialog.js index 089cfedab..fa2bfd718 100644 --- a/extensions/wikidata/module/scripts/dialogs/perform-edits-dialog.js +++ b/extensions/wikidata/module/scripts/dialogs/perform-edits-dialog.js @@ -94,7 +94,7 @@ PerformEditsDialog.checkAndLaunch = function () { ManageAccountDialog.ensureLoggedIn(function(logged_in_username) { if (logged_in_username) { var discardWaiter = DialogSystem.showBusy($.i18n('perform-wikidata-edits/analyzing-edits')); - $.post( + Refine.postCSRF( "command/wikidata/preview-wikibase-schema?" + $.param({ project: theProject.id }), { engine: JSON.stringify(ui.browsingEngine.getJSON()) }, function(data) { diff --git a/extensions/wikidata/module/scripts/dialogs/schema-alignment-dialog.js b/extensions/wikidata/module/scripts/dialogs/schema-alignment-dialog.js index 6c982d6a2..cc6760e17 100644 --- a/extensions/wikidata/module/scripts/dialogs/schema-alignment-dialog.js +++ b/extensions/wikidata/module/scripts/dialogs/schema-alignment-dialog.js @@ -1283,7 +1283,7 @@ SchemaAlignmentDialog.preview = function() { $('.invalid-schema-warning').show(); return; } - $.post( + Refine.postCSRF( "command/wikidata/preview-wikibase-schema?" + $.param({ project: theProject.id }), { schema: JSON.stringify(schema), engine: JSON.stringify(ui.browsingEngine.getJSON()) }, function(data) { diff --git a/extensions/wikidata/src/org/openrefine/wikidata/commands/LoginCommand.java b/extensions/wikidata/src/org/openrefine/wikidata/commands/LoginCommand.java index 83c194582..cc3330934 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/commands/LoginCommand.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/commands/LoginCommand.java @@ -41,6 +41,11 @@ public class LoginCommand extends Command { @Override public void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { + if(!hasValidCSRFToken(request)) { + respondCSRFError(response); + return; + } + String username = request.getParameter("wb-username"); String password = request.getParameter("wb-password"); String remember = request.getParameter("remember-credentials"); diff --git a/extensions/wikidata/src/org/openrefine/wikidata/commands/PreviewWikibaseSchemaCommand.java b/extensions/wikidata/src/org/openrefine/wikidata/commands/PreviewWikibaseSchemaCommand.java index 7c1c25621..1ebdfb1dd 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/commands/PreviewWikibaseSchemaCommand.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/commands/PreviewWikibaseSchemaCommand.java @@ -45,6 +45,13 @@ import com.google.refine.commands.Command; import com.google.refine.model.Project; public class PreviewWikibaseSchemaCommand extends Command { + + /** + * This command uses POST but is left CSRF-unprotected since it does not + * incur a side effect or state change in the backend. + * The reason why it uses POST is to make sure large schemas and engines + * can be passed as parameters. + */ @Override public void doPost(HttpServletRequest request, HttpServletResponse response) diff --git a/extensions/wikidata/src/org/openrefine/wikidata/commands/SaveWikibaseSchemaCommand.java b/extensions/wikidata/src/org/openrefine/wikidata/commands/SaveWikibaseSchemaCommand.java index 0115ff8d0..728eeefd2 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/commands/SaveWikibaseSchemaCommand.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/commands/SaveWikibaseSchemaCommand.java @@ -46,6 +46,10 @@ public class SaveWikibaseSchemaCommand 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/extensions/wikidata/tests/src/org/openrefine/wikidata/commands/LoginCommandTest.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/commands/LoginCommandTest.java index 36ca6c4bd..10221863b 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/commands/LoginCommandTest.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/commands/LoginCommandTest.java @@ -1,6 +1,7 @@ package org.openrefine.wikidata.commands; import static org.testng.Assert.assertEquals; +import static org.mockito.Mockito.when; import java.io.IOException; @@ -9,6 +10,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.util.TestUtils; + public class LoginCommandTest extends CommandTest { @BeforeMethod @@ -18,8 +22,16 @@ public class LoginCommandTest extends CommandTest { @Test public void testNoCredentials() throws ServletException, IOException { + when(request.getParameter("csrf_token")).thenReturn(Command.csrfFactory.getFreshToken()); + command.doPost(request, response); assertEquals("{\"logged_in\":false,\"username\":null}", writer.toString()); } + + @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/extensions/wikidata/tests/src/org/openrefine/wikidata/commands/SaveWikibaseSchemaCommandTest.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/commands/SaveWikibaseSchemaCommandTest.java index cd981382b..26d094c67 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/commands/SaveWikibaseSchemaCommandTest.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/commands/SaveWikibaseSchemaCommandTest.java @@ -34,6 +34,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.util.TestUtils; + public class SaveWikibaseSchemaCommandTest extends SchemaCommandTest { @BeforeMethod @@ -44,6 +47,8 @@ public class SaveWikibaseSchemaCommandTest extends SchemaCommandTest { @Test public void testValidSchema() throws ServletException, IOException { + when(request.getParameter("csrf_token")).thenReturn(Command.csrfFactory.getFreshToken()); + String schemaJson = jsonFromFile("schema/inception.json").toString(); when(request.getParameter("schema")).thenReturn(schemaJson); @@ -54,6 +59,8 @@ public class SaveWikibaseSchemaCommandTest extends SchemaCommandTest { @Test public void testInvalidSchema() throws ServletException, IOException { + when(request.getParameter("csrf_token")).thenReturn(Command.csrfFactory.getFreshToken()); + String schemaJson = "{\"itemDocuments\":[{\"statementGroups\":[{\"statements\":[]}]," +"\"nameDescs\":[]}],\"wikibasePrefix\":\"http://www.wikidata.org/entity/\"}"; @@ -62,4 +69,13 @@ public class SaveWikibaseSchemaCommandTest extends SchemaCommandTest { assertTrue(writer.toString().contains("\"error\"")); } + + @Test + public void testCsrfProtection() throws ServletException, IOException { + String schemaJson = jsonFromFile("schema/inception.json").toString(); + when(request.getParameter("schema")).thenReturn(schemaJson); + + command.doPost(request, response); + TestUtils.assertEqualAsJson("{\"code\":\"error\",\"message\":\"Missing or invalid csrf_token parameter\"}", writer.toString()); + } } diff --git a/extensions/wikidata/tests/src/org/openrefine/wikidata/commands/SchemaCommandTest.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/commands/SchemaCommandTest.java index 8af1528cd..7316f2906 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/commands/SchemaCommandTest.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/commands/SchemaCommandTest.java @@ -32,6 +32,7 @@ import javax.servlet.ServletException; import org.testng.annotations.Test; +import com.google.refine.commands.Command; import com.google.refine.util.ParsingUtilities; public abstract class SchemaCommandTest extends CommandTest { @@ -39,9 +40,11 @@ public abstract class SchemaCommandTest extends CommandTest { @Test public void testNoSchema() throws ServletException, IOException { + when(request.getParameter("csrf_token")).thenReturn(Command.csrfFactory.getFreshToken()); + command.doPost(request, response); - assertEquals("{\"code\":\"error\",\"message\":\"No Wikibase schema provided.\"}", writer.toString()); + assertEquals(writer.toString(), "{\"code\":\"error\",\"message\":\"No Wikibase schema provided.\"}"); } @Test diff --git a/main/webapp/modules/core/scripts/index.js b/main/webapp/modules/core/scripts/index.js index 682bcf826..70158d5b8 100644 --- a/main/webapp/modules/core/scripts/index.js +++ b/main/webapp/modules/core/scripts/index.js @@ -56,7 +56,11 @@ Refine.wrapCSRF = function(onCSRF) { Refine.postCSRF = function(url, data, success, dataType, failCallback) { return Refine.wrapCSRF(function(token) { var fullData = data || {}; - fullData['csrf_token'] = token; + if (typeof fulldata == 'string') { + fullData = fullData + $.param({csrf_token: token}); + } else { + fullData['csrf_token'] = token; + } var req = $.post(url, fullData, success, dataType); if (failCallback !== undefined) { req.fail(failCallback); diff --git a/main/webapp/modules/core/scripts/preferences.js b/main/webapp/modules/core/scripts/preferences.js index a4bd9af52..c2d8939e4 100644 --- a/main/webapp/modules/core/scripts/preferences.js +++ b/main/webapp/modules/core/scripts/preferences.js @@ -55,7 +55,11 @@ Refine.wrapCSRF = function(onCSRF) { Refine.postCSRF = function(url, data, success, dataType, failCallback) { return Refine.wrapCSRF(function(token) { var fullData = data || {}; - fullData['csrf_token'] = token; + if (typeof fulldata == 'string') { + fullData = fullData + $.param({csrf_token: token}); + } else { + fullData['csrf_token'] = token; + } var req = $.post(url, fullData, success, dataType); if (failCallback !== undefined) { req.fail(failCallback); diff --git a/main/webapp/modules/core/scripts/project.js b/main/webapp/modules/core/scripts/project.js index b7d11895c..549838810 100644 --- a/main/webapp/modules/core/scripts/project.js +++ b/main/webapp/modules/core/scripts/project.js @@ -420,7 +420,11 @@ Refine.wrapCSRF = function(onCSRF) { Refine.postCSRF = function(url, data, success, dataType) { Refine.wrapCSRF(function(token) { var fullData = data || {}; - fullData['csrf_token'] = token; + if (typeof fulldata == 'string') { + fullData = fullData + $.param({csrf_token: token}); + } else { + fullData['csrf_token'] = token; + } $.post(url, fullData, success, dataType); }); };