Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add full URLs in exercises to ensure they always load #4589

Merged
merged 4 commits into from
Jan 3, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions kolibri/core/content/test/test_zipcontent.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from django.test import Client
from django.test import TestCase
from le_utils.constants import exercises

from ..models import LocalFile
from ..utils.paths import get_content_storage_file_path
Expand All @@ -22,6 +23,8 @@ class ZipContentTestCase(TestCase):
test_str_1 = "This is a test!"
test_name_2 = "testfile2.txt"
test_str_2 = "And another test..."
test_name_3 = "testfile3.json"
test_str_3 = "A test of image placeholder replacement ${placeholder}".format(placeholder=exercises.IMG_PLACEHOLDER)

def setUp(self):

Expand All @@ -41,6 +44,7 @@ def setUp(self):
with zipfile.ZipFile(self.zip_path, "w") as zf:
zf.writestr(self.test_name_1, self.test_str_1)
zf.writestr(self.test_name_2, self.test_str_2)
zf.writestr(self.test_name_3, self.test_str_3)

self.zip_file_obj = LocalFile(id=self.hash, extension=self.extension, available=True)
self.zip_file_base_url = self.zip_file_obj.get_storage_url()
Expand Down Expand Up @@ -82,6 +86,10 @@ def test_content_security_policy_header(self):
response = self.client.get(self.zip_file_base_url + self.test_name_1)
self.assertEqual(response.get("Content-Security-Policy"), "default-src 'self' 'unsafe-inline' 'unsafe-eval' data: http://testserver")

def test_content_security_policy_header_http_referer(self):
response = self.client.get(self.zip_file_base_url + self.test_name_1, HTTP_REFERER="http://testserver")
rtibbles marked this conversation as resolved.
Show resolved Hide resolved
self.assertEqual(response.get("Content-Security-Policy"), "default-src 'self' 'unsafe-inline' 'unsafe-eval' data: http://testserver")

def test_access_control_allow_origin_header(self):
response = self.client.get(self.zip_file_base_url + self.test_name_1)
self.assertEqual(response.get("Access-Control-Allow-Origin"), "*")
Expand All @@ -98,3 +106,13 @@ def test_access_control_allow_headers(self):
self.assertEqual(response.get("Access-Control-Allow-Headers", ""), headerval)
response = self.client.get(self.zip_file_base_url + self.test_name_1, HTTP_ACCESS_CONTROL_REQUEST_HEADERS=headerval)
self.assertEqual(response.get("Access-Control-Allow-Headers", ""), headerval)

def test_json_image_replacement_http_referer_header(self):
server_name = "http://testserver"
response = self.client.get(self.zip_file_base_url + self.test_name_3, HTTP_REFERER=server_name)
self.assertEqual(response.content, self.test_str_3.replace("$" + exercises.IMG_PLACEHOLDER, (server_name + self.zip_file_base_url).strip("/")))

def test_json_image_replacement_no_http_referer_header(self):
server_name = "http://testserver"
response = self.client.get(self.zip_file_base_url + self.test_name_3)
self.assertEqual(response.content, self.test_str_3.replace("$" + exercises.IMG_PLACEHOLDER, (server_name + self.zip_file_base_url).strip("/")))
40 changes: 37 additions & 3 deletions kolibri/core/content/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@
from django.http import HttpResponse
from django.http.response import FileResponse
from django.http.response import HttpResponseNotModified
from django.urls import reverse
from django.views.decorators.clickjacking import xframe_options_exempt
from django.views.generic.base import View
from le_utils.constants import exercises
from six.moves.urllib.parse import urlparse
from six.moves.urllib.parse import urlunparse

from .api import cache_forever
from .utils.paths import get_content_storage_file_path
Expand All @@ -27,6 +30,35 @@ def _add_access_control_headers(request, response):
response["Access-Control-Allow-Headers"] = requested_headers


def get_referrer_url(request):
if request.META.get('HTTP_REFERER'):
# If available user HTTP_REFERER to infer the host as that will give us more
rtibbles marked this conversation as resolved.
Show resolved Hide resolved
# information if Kolibri is behind a proxy.
return urlparse(request.META.get('HTTP_REFERER'))


def generate_image_prefix_url(zipped_filename, parsed_referrer_url):
# Remove trailing slash
zipcontent = reverse(
'kolibri:core:zipcontent',
kwargs={
"zipped_filename": zipped_filename,
"embedded_filepath": ''
})[:-1]
if parsed_referrer_url:
# Reconstruct the parsed URL using only the scheme(0) and host + port(1)
zipcontent = urlunparse((parsed_referrer_url[0], parsed_referrer_url[1], zipcontent, '', '', ''))
return zipcontent.encode()


def get_host(request, parsed_referrer_url):
if parsed_referrer_url:
host = urlunparse((parsed_referrer_url[0], parsed_referrer_url[1], '', '', '', ''))
else:
host = request.build_absolute_uri(OPTIONS['Deployment']['URL_PATH_PREFIX'])
rtibbles marked this conversation as resolved.
Show resolved Hide resolved
return host.strip("/")


rtibbles marked this conversation as resolved.
Show resolved Hide resolved
class ZipContentView(View):

@xframe_options_exempt
Expand Down Expand Up @@ -59,6 +91,8 @@ def get(self, request, zipped_filename, embedded_filepath):
if request.META.get('HTTP_IF_MODIFIED_SINCE'):
return HttpResponseNotModified()

parsed_referrer_url = get_referrer_url(request)

with zipfile.ZipFile(zipped_path) as zf:

# if no path, or a directory, is being referenced, look for an index.html file
Expand All @@ -79,11 +113,11 @@ def get(self, request, zipped_filename, embedded_filepath):
response = FileResponse(zf.open(info), content_type=content_type)
file_size = info.file_size
else:
image_prefix_url = generate_image_prefix_url(zipped_filename, parsed_referrer_url)
indirectlylit marked this conversation as resolved.
Show resolved Hide resolved
# load the stream from json file into memory, replace the path_place_holder.
content = zf.open(info).read()
str_to_be_replaced = ('$' + exercises.IMG_PLACEHOLDER).encode()
zipcontent = ('/' + request.resolver_match.url_name + "/" + zipped_filename).encode()
content_with_path = content.replace(str_to_be_replaced, zipcontent)
content_with_path = content.replace(str_to_be_replaced, image_prefix_url)
response = HttpResponse(content_with_path, content_type=content_type)
file_size = len(content_with_path)

Expand All @@ -99,7 +133,7 @@ def get(self, request, zipped_filename, embedded_filepath):

# restrict CSP to only allow resources to be loaded from the Kolibri host, to prevent info leakage
# (e.g. via passing user info out as GET parameters to an attacker's server), or inadvertent data usage
host = request.build_absolute_uri(OPTIONS['Deployment']['URL_PATH_PREFIX']).strip("/")
host = get_host(request, parsed_referrer_url)
indirectlylit marked this conversation as resolved.
Show resolved Hide resolved
response["Content-Security-Policy"] = "default-src 'self' 'unsafe-inline' 'unsafe-eval' data: " + host

return response
Expand Down