From 861f3e9d3791e4acb3db7e4ed25e9fc1930dce22 Mon Sep 17 00:00:00 2001 From: nbaars Date: Sat, 7 Feb 2015 11:37:53 +0100 Subject: [PATCH 1/2] Moved loading to separate object. Added a unit test for loading the properties --- .../webgoat/plugins/GlobalProperties.java | 24 ++++++++++ .../webgoat/plugins/PluginFileUtils.java | 5 +- .../org/owasp/webgoat/session/Course.java | 17 ++----- .../webgoat/plugins/GlobalPropertiesTest.java | 47 +++++++++++++++++++ 4 files changed, 77 insertions(+), 16 deletions(-) create mode 100644 src/main/java/org/owasp/webgoat/plugins/GlobalProperties.java create mode 100644 src/test/java/org/owasp/webgoat/plugins/GlobalPropertiesTest.java diff --git a/src/main/java/org/owasp/webgoat/plugins/GlobalProperties.java b/src/main/java/org/owasp/webgoat/plugins/GlobalProperties.java new file mode 100644 index 000000000..6e478815f --- /dev/null +++ b/src/main/java/org/owasp/webgoat/plugins/GlobalProperties.java @@ -0,0 +1,24 @@ +package org.owasp.webgoat.plugins; + +import java.io.IOException; +import java.nio.file.Path; +import java.util.List; + +public final class GlobalProperties { + + private final Plugin plugin; + + public GlobalProperties(Path pluginDirectory) { + this.plugin = new Plugin(pluginDirectory); + } + + public void loadProperties(Path globalPropertiesPath) { + try { + List filesInDirectory = PluginFileUtils.getFilesInDirectory(globalPropertiesPath); + this.plugin.loadFiles(filesInDirectory, true); + } catch (IOException e) { + throw new IllegalStateException("Unable to load global properties, check your installation for the directory i18n: " + globalPropertiesPath.toString(), e); + } + } + +} diff --git a/src/main/java/org/owasp/webgoat/plugins/PluginFileUtils.java b/src/main/java/org/owasp/webgoat/plugins/PluginFileUtils.java index 758119f0e..a3b26a34b 100644 --- a/src/main/java/org/owasp/webgoat/plugins/PluginFileUtils.java +++ b/src/main/java/org/owasp/webgoat/plugins/PluginFileUtils.java @@ -15,7 +15,7 @@ public class PluginFileUtils { } public static boolean hasParentDirectoryWithName(Path p, String s) { - if (p == null || p.getParent() == null || p.getRoot().equals(p.getParent())) { + if (p == null || p.getParent() == null || p.getParent().equals(p.getRoot())) { return false; } if (p.getParent().getFileName().toString().equals(s)) { @@ -31,8 +31,7 @@ public class PluginFileUtils { return p; } - public static List getFilesInDirectory( Path directory) throws IOException - { + public static List getFilesInDirectory( Path directory) throws IOException { List files = new ArrayList<>(); DirectoryStream dirStream; dirStream = Files.newDirectoryStream(directory); diff --git a/src/main/java/org/owasp/webgoat/session/Course.java b/src/main/java/org/owasp/webgoat/session/Course.java index 24a06a9d2..aac3517b3 100644 --- a/src/main/java/org/owasp/webgoat/session/Course.java +++ b/src/main/java/org/owasp/webgoat/session/Course.java @@ -3,6 +3,7 @@ package org.owasp.webgoat.session; import org.owasp.webgoat.HammerHead; import org.owasp.webgoat.lessons.AbstractLesson; import org.owasp.webgoat.lessons.Category; +import org.owasp.webgoat.plugins.GlobalProperties; import org.owasp.webgoat.plugins.Plugin; import org.owasp.webgoat.plugins.PluginFileUtils; import org.owasp.webgoat.plugins.PluginsLoader; @@ -284,7 +285,6 @@ public class Course { } private void loadLessonFromPlugin(ServletContext context) { - context.getContextPath(); logger.debug("Loading plugins into cache"); String pluginPath = context.getRealPath("plugin_lessons"); String targetPath = context.getRealPath("plugin_extracted"); @@ -292,17 +292,8 @@ public class Course { logger.error("Plugins directory {} not found", pluginPath); return; } - - // Do a one time load of the container properties - String containerPath = context.getRealPath("container//i18n"); - Plugin theContainer = new Plugin(Paths.get(targetPath)); - try { - theContainer.loadFiles(PluginFileUtils.getFilesInDirectory(Paths.get(containerPath)), false); - } catch (IOException io) { - logger.error("Error loading container properties: ", io); - } - - Path pluginDirectory = Paths.get(pluginPath); + new GlobalProperties(Paths.get(targetPath)).loadProperties(Paths.get(context.getRealPath("container//i18n"))); + List plugins = new PluginsLoader(Paths.get(pluginPath), Paths.get(targetPath)).loadPlugins(true); for (Plugin plugin : plugins) { try { @@ -314,7 +305,7 @@ public class Course { lesson.update(properties); - if (lesson.getHidden() == false) { + if (!lesson.getHidden()) { lessons.add(lesson); } for(Map.Entry lessonPlan : plugin.getLessonPlans().entrySet()) { diff --git a/src/test/java/org/owasp/webgoat/plugins/GlobalPropertiesTest.java b/src/test/java/org/owasp/webgoat/plugins/GlobalPropertiesTest.java new file mode 100644 index 000000000..f1dd80a46 --- /dev/null +++ b/src/test/java/org/owasp/webgoat/plugins/GlobalPropertiesTest.java @@ -0,0 +1,47 @@ +package org.owasp.webgoat.plugins; + +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; + +import static org.junit.Assert.assertNotNull; + +public class GlobalPropertiesTest { + + private Path tempDirectory; + + @Before + public void createTmpDir() throws IOException { + tempDirectory = Files.createTempDirectory(GlobalPropertiesTest.class.getSimpleName()); + tempDirectory.toFile().deleteOnExit(); + } + + @Test + public void propertyFilesShouldBeLoaded() throws IOException { + Path pluginDirectory = Files.createDirectory(Paths.get(tempDirectory.toString(), "plugins")); + Path directory = Files.createDirectory(Paths.get(tempDirectory.toString(), "i18n")); + Path globalProperties = Files.createFile(Paths.get(directory.toString(), "global.properties")); + Files.write(globalProperties, Arrays.asList("test=label for test"), StandardCharsets.UTF_8); + new GlobalProperties(pluginDirectory).loadProperties(directory); + + ClassLoader propertyFilesClassLoader = + ResourceBundleClassLoader.createPropertyFilesClassLoader(this.getClass().getClassLoader()); + assertNotNull(propertyFilesClassLoader.getResourceAsStream("global.properties")); + } + + @Test(expected = IllegalStateException.class) + public void propertyFilesDirectoryNotFoundShouldRaiseError() throws IOException { + Path pluginDirectory = Files.createDirectory(Paths.get(tempDirectory.toString(), "plugins")); + Path directory = Files.createDirectory(Paths.get(tempDirectory.toString(), "i18n")); + Files.delete(directory); + + new GlobalProperties(pluginDirectory).loadProperties(directory); + } + +} \ No newline at end of file From bc21a86b6876a92c1c60bbfd2a4311ae2f310eb6 Mon Sep 17 00:00:00 2001 From: nbaars Date: Sun, 8 Feb 2015 14:12:01 +0100 Subject: [PATCH 2/2] Fixed hard coded rewriting of html files was fixed on SqlStringInjection Added testcases for this situation --- .../org/owasp/webgoat/plugins/Plugin.java | 15 ++++--- .../webgoat/plugins/GlobalPropertiesTest.java | 11 +---- .../org/owasp/webgoat/plugins/PluginTest.java | 45 +++++++++++++++++++ .../webgoat/plugins/PluginTestHelper.java | 34 ++++++++++++++ .../org/owasp/webgoat/plugins/TestPlugin.java | 6 +++ .../plugins/lessonSolutions/rewrite_test.html | 11 +++++ 6 files changed, 107 insertions(+), 15 deletions(-) create mode 100644 src/test/java/org/owasp/webgoat/plugins/PluginTest.java create mode 100644 src/test/java/org/owasp/webgoat/plugins/PluginTestHelper.java create mode 100644 src/test/java/org/owasp/webgoat/plugins/TestPlugin.java create mode 100644 src/test/resources/org/owasp/webgoat/plugins/lessonSolutions/rewrite_test.html diff --git a/src/main/java/org/owasp/webgoat/plugins/Plugin.java b/src/main/java/org/owasp/webgoat/plugins/Plugin.java index 77827910f..e96b56108 100644 --- a/src/main/java/org/owasp/webgoat/plugins/Plugin.java +++ b/src/main/java/org/owasp/webgoat/plugins/Plugin.java @@ -11,11 +11,13 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.nio.file.StandardOpenOption; import java.util.HashMap; import java.util.List; import java.util.Map; +import static java.nio.file.StandardOpenOption.APPEND; +import static java.nio.file.StandardOpenOption.CREATE; +import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING; import static org.owasp.webgoat.plugins.PluginFileUtils.fileEndsWith; import static org.owasp.webgoat.plugins.PluginFileUtils.hasParentDirectoryWithName; @@ -96,9 +98,9 @@ public class Plugin { Path propertiesPath = createPropertiesDirectory(); ResourceBundleClassLoader.setPropertiesPath(propertiesPath); if ( reload ) { - Files.write(propertiesPath.resolve(file.getFileName()), bos.toByteArray(), StandardOpenOption.CREATE, StandardOpenOption.APPEND); + Files.write(propertiesPath.resolve(file.getFileName()), bos.toByteArray(), CREATE, APPEND); } else { - Files.write(propertiesPath.resolve(file.getFileName()), bos.toByteArray(), StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); + Files.write(propertiesPath.resolve(file.getFileName()), bos.toByteArray(), CREATE, TRUNCATE_EXISTING); } } catch (IOException io) { throw new PluginLoadingFailure("Property file detected, but unable to copy the properties", io); @@ -118,9 +120,9 @@ public class Plugin { for (Map.Entry html : solutionLanguageFiles.entrySet()) { byte[] htmlFileAsBytes = Files.readAllBytes(Paths.get(html.getValue().toURI())); String htmlFile = new String(htmlFileAsBytes); - htmlFile = htmlFile.replaceAll(this.lesson.getSimpleName() + "_files", pluginTarget.getFileName().toString() + "/lessons/plugin/SqlStringInjection/lessonSolutions/en/" + this.lesson.getSimpleName() + "_files"); - Files.write(Paths.get(html.getValue().toURI()), htmlFile.getBytes(), StandardOpenOption.CREATE, - StandardOpenOption.TRUNCATE_EXISTING); + htmlFile = htmlFile.replaceAll("lesson_solutions/" + this.lesson.getSimpleName() + "_files", pluginTarget.getFileName().toString() + "/lessons/plugin/" + this.lesson.getSimpleName() + "/lessonSolutions/en/" + this.lesson.getSimpleName() + "_files"); + Files.write(Paths.get(html.getValue().toURI()), htmlFile.getBytes(), CREATE, + TRUNCATE_EXISTING); } } catch (IOException e) { throw new PluginLoadingFailure("Unable to rewrite the paths in the solutions", e); @@ -143,4 +145,5 @@ public class Plugin { public Map getLessonPlans() { return this.lessonPlansLanguageFiles; } + } diff --git a/src/test/java/org/owasp/webgoat/plugins/GlobalPropertiesTest.java b/src/test/java/org/owasp/webgoat/plugins/GlobalPropertiesTest.java index f1dd80a46..2b656b663 100644 --- a/src/test/java/org/owasp/webgoat/plugins/GlobalPropertiesTest.java +++ b/src/test/java/org/owasp/webgoat/plugins/GlobalPropertiesTest.java @@ -1,6 +1,5 @@ package org.owasp.webgoat.plugins; -import org.junit.Before; import org.junit.Test; import java.io.IOException; @@ -14,16 +13,9 @@ import static org.junit.Assert.assertNotNull; public class GlobalPropertiesTest { - private Path tempDirectory; - - @Before - public void createTmpDir() throws IOException { - tempDirectory = Files.createTempDirectory(GlobalPropertiesTest.class.getSimpleName()); - tempDirectory.toFile().deleteOnExit(); - } - @Test public void propertyFilesShouldBeLoaded() throws IOException { + Path tempDirectory = PluginTestHelper.createTmpDir(); Path pluginDirectory = Files.createDirectory(Paths.get(tempDirectory.toString(), "plugins")); Path directory = Files.createDirectory(Paths.get(tempDirectory.toString(), "i18n")); Path globalProperties = Files.createFile(Paths.get(directory.toString(), "global.properties")); @@ -37,6 +29,7 @@ public class GlobalPropertiesTest { @Test(expected = IllegalStateException.class) public void propertyFilesDirectoryNotFoundShouldRaiseError() throws IOException { + Path tempDirectory = PluginTestHelper.createTmpDir(); Path pluginDirectory = Files.createDirectory(Paths.get(tempDirectory.toString(), "plugins")); Path directory = Files.createDirectory(Paths.get(tempDirectory.toString(), "i18n")); Files.delete(directory); diff --git a/src/test/java/org/owasp/webgoat/plugins/PluginTest.java b/src/test/java/org/owasp/webgoat/plugins/PluginTest.java new file mode 100644 index 000000000..aac20ba77 --- /dev/null +++ b/src/test/java/org/owasp/webgoat/plugins/PluginTest.java @@ -0,0 +1,45 @@ +package org.owasp.webgoat.plugins; + +import org.junit.Test; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.List; + +import static org.junit.Assert.assertThat; +import static org.junit.matchers.JUnitMatchers.containsString; +import static org.junit.matchers.JUnitMatchers.hasItem; + +public class PluginTest { + + @Test + public void pathShouldBeRewrittenInHtmlFile() throws Exception { + Path tmpDir = PluginTestHelper.createTmpDir(); + Path pluginSourcePath = PluginTestHelper.pathForLoading(); + Plugin plugin = PluginTestHelper.createPluginFor(TestPlugin.class); + Path htmlFile = Paths.get(pluginSourcePath.toString(), "lessonSolutions", "rewrite_test.html"); + plugin.loadFiles(Arrays.asList(htmlFile), true); + plugin.rewritePaths(tmpDir); + List allLines = Files.readAllLines(htmlFile, StandardCharsets.UTF_8); + + assertThat(allLines, + hasItem(containsString("lessons/plugin/TestPlugin/lessonSolutions/en/TestPlugin_files/image001.png"))); + } + + @Test + public void shouldNotRewriteOtherLinksStartingWithLesson_solutions() throws Exception { + Path tmpDir = PluginTestHelper.createTmpDir(); + Path pluginSourcePath = PluginTestHelper.pathForLoading(); + Plugin plugin = PluginTestHelper.createPluginFor(TestPlugin.class); + Path htmlFile = Paths.get(pluginSourcePath.toString(), "lessonSolutions", "rewrite_test.html"); + plugin.loadFiles(Arrays.asList(htmlFile), true); + plugin.rewritePaths(tmpDir); + List allLines = Files.readAllLines(htmlFile, StandardCharsets.UTF_8); + + assertThat(allLines, + hasItem(containsString("lesson_solutions/Unknown_files/image001.png"))); + } +} \ No newline at end of file diff --git a/src/test/java/org/owasp/webgoat/plugins/PluginTestHelper.java b/src/test/java/org/owasp/webgoat/plugins/PluginTestHelper.java new file mode 100644 index 000000000..684197454 --- /dev/null +++ b/src/test/java/org/owasp/webgoat/plugins/PluginTestHelper.java @@ -0,0 +1,34 @@ +package org.owasp.webgoat.plugins; + +import java.io.IOException; +import java.net.URISyntaxException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.HashMap; +import java.util.Map; + +public class PluginTestHelper { + + private static Path tempDirectory; + + public static Path createTmpDir() throws IOException { + tempDirectory = Files.createTempDirectory(PluginTestHelper.class.getSimpleName()); + tempDirectory.toFile().deleteOnExit(); + return tempDirectory; + } + + public static Path pathForLoading() throws IOException, URISyntaxException { + Path path = Paths.get(PluginTestHelper.class.getProtectionDomain().getCodeSource().getLocation().toURI()); + return Paths.get(path.toString(), "org/owasp/webgoat/plugins"); + } + + public static Plugin createPluginFor(Class pluginClass) throws Exception { + Path pluginTargetPath = Files.createDirectory(Paths.get(tempDirectory.toString(), "pluginTargetPath")); + Plugin plugin = new Plugin(pluginTargetPath); + Map classes = new HashMap<>(); + classes.put(pluginClass.getName(), Files.readAllBytes(Paths.get(pathForLoading().toString(), pluginClass.getSimpleName() + ".class"))); + plugin.loadClasses(classes); + return plugin; + } +} diff --git a/src/test/java/org/owasp/webgoat/plugins/TestPlugin.java b/src/test/java/org/owasp/webgoat/plugins/TestPlugin.java new file mode 100644 index 000000000..69695c8a2 --- /dev/null +++ b/src/test/java/org/owasp/webgoat/plugins/TestPlugin.java @@ -0,0 +1,6 @@ +package org.owasp.webgoat.plugins; + +import org.owasp.webgoat.lessons.SequentialLessonAdapter; + +public class TestPlugin extends SequentialLessonAdapter { +} diff --git a/src/test/resources/org/owasp/webgoat/plugins/lessonSolutions/rewrite_test.html b/src/test/resources/org/owasp/webgoat/plugins/lessonSolutions/rewrite_test.html new file mode 100644 index 000000000..aaeb3600b --- /dev/null +++ b/src/test/resources/org/owasp/webgoat/plugins/lessonSolutions/rewrite_test.html @@ -0,0 +1,11 @@ + + + + + + + + + + + \ No newline at end of file