Merge pull request #340 from edx/feature/configurable-resources-root

Add the ability to control the root where resources load from.

xblock/core.py

@@ -50,6 +50,9 @@ class SharedBlockBase(Plugin):
     """
     Behaviors and attrs which all XBlock like things should share
     """
+
+    resources_dir = 'public'
+
     @classmethod
     def open_local_resource(cls, uri):
         """Open a local resource.
@@ -62,20 +65,27 @@ def open_local_resource(cls, uri):
 
         For security reasons, the default implementation will return only a
         very restricted set of file types, which must be located in a folder
-        called "public". XBlock authors who want to override this behavior will
-        need to take care to ensure that the method only serves legitimate
-        public resources. At the least, the URI should be matched against a
-        whitelist regex to ensure that you do not serve an unauthorized
-        resource.
-
+        that defaults to "public".  The location used for public resources can
+        be changed on a per-XBlock basis. XBlock authors who want to override
+        this behavior will need to take care to ensure that the method only
+        serves legitimate public resources. At the least, the URI should be
+        matched against a whitelist regex to ensure that you do not serve an
+        unauthorized resource.
         """
-        # Verify the URI is in whitelisted form before opening for serving.
-        # URI must begin with public/, and no file path component can start
-        # with a dot, which prevents ".." and ".hidden" files.
-        if not uri.startswith("public/"):
-            raise DisallowedFileError("Only files from public/ are allowed: %r" % uri)
+
+        # If no resources_dir is set, then this XBlock cannot serve local resources.
+        if cls.resources_dir is None:
+            raise DisallowedFileError("This XBlock is not configured to serve local resources")
+
+        # Make sure the path starts with whatever resources_dir is set to.
+        if not uri.startswith(cls.resources_dir + '/'):
+            raise DisallowedFileError("Only files from %r/ are allowed: %r" % (cls.resources_dir, uri))
+
+        # Disalow paths that have a '/.' component, as `/./` is a no-op and `/../`
+        # can be used to recurse back past the entry point of this XBlock.
         if "/." in uri:
             raise DisallowedFileError("Only safe file names are allowed: %r" % uri)
+
         return pkg_resources.resource_stream(cls.__module__, uri)
 
 

xblock/test/test_core.py

@@ -917,6 +917,13 @@ class LoadableXBlock(XBlock):
         """Just something to load resources from."""
         pass
 
+    class UnloadableXBlock(XBlock):
+        """Just something to load resources from."""
+
+        resources_dir = None
+
+        pass
+
     def stub_resource_stream(self, module, name):
         """Act like pkg_resources.resource_stream, for testing."""
         assert module == "xblock.test.test_core"
@@ -951,6 +958,29 @@ def test_open_bad_local_resource(self, uri):
             with assert_raises_regexp(DisallowedFileError, msg):
                 loadable.open_local_resource(uri)
 
+    @ddt.data(
+        "public/hey.js",
+        "public/sub/hey.js",
+        "public/js/vendor/jNotify.jQuery.min.js",
+        "public/something.foo",         # Unknown file extension is fine
+        "public/a/long/PATH/no-problem=here$123.ext",
+        "public/ℓιвяαяу.js",
+        "public/foo.js",
+        "public/.git/secret.js",
+        "static/secret.js",
+        "../public/no-no.bad",
+        "image.png",
+        ".git/secret.js",
+        "static/ℓιвяαяу.js",
+    )
+    def test_open_local_resource_with_no_resources_dir(self, uri):
+        unloadable = self.UnloadableXBlock(None, scope_ids=None)
+
+        with patch('pkg_resources.resource_stream', self.stub_resource_stream):
+            msg = "not configured to serve local resources"
+            with assert_raises_regexp(DisallowedFileError, msg):
+                unloadable.open_local_resource(uri)
+
 
 class TestXBlockDeprecation(WarningTestMixin, unittest.TestCase):
     """