diff --git a/cms/envs/common.py b/cms/envs/common.py index 38913f537a..c79cfe7aa7 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -140,7 +140,6 @@ TEMPLATE_LOADERS = ( ) MIDDLEWARE_CLASSES = ( - 'contentserver.middleware.StaticContentServer', 'request_cache.middleware.RequestCache', 'django.middleware.cache.UpdateCacheMiddleware', 'django.middleware.common.CommonMiddleware', @@ -150,6 +149,7 @@ MIDDLEWARE_CLASSES = ( # Instead of AuthenticationMiddleware, we use a cache-backed version 'cache_toolbox.middleware.CacheBackedAuthenticationMiddleware', + 'contentserver.middleware.StaticContentServer', 'django.contrib.messages.middleware.MessageMiddleware', 'track.middleware.TrackMiddleware', diff --git a/common/djangoapps/contentserver/middleware.py b/common/djangoapps/contentserver/middleware.py index d89e3fdd23..b9c14cd537 100644 --- a/common/djangoapps/contentserver/middleware.py +++ b/common/djangoapps/contentserver/middleware.py @@ -1,4 +1,6 @@ -from django.http import HttpResponse, HttpResponseNotModified +from django.http import (HttpResponse, HttpResponseNotModified, + HttpResponseForbidden) +from student.models import CourseEnrollment from xmodule.contentstore.django import contentstore from xmodule.contentstore.content import StaticContent, XASSET_LOCATION_TAG @@ -41,7 +43,14 @@ class StaticContentServer(object): # NOP here, but we may wish to add a "cache-hit" counter in the future pass - # see if the last-modified at hasn't changed, if not return a 302 (Not Modified) + # Check that user has access to content + if getattr(content, "locked", False): + if not hasattr(request, "user") or not request.user.is_authenticated(): + return HttpResponseForbidden('Unauthorized') + course_partial_id = "/".join([loc.org, loc.course]) + if not request.user.is_staff and not CourseEnrollment.is_enrolled_by_partial( + request.user, course_partial_id): + return HttpResponseForbidden('Unauthorized') # convert over the DB persistent last modified timestamp to a HTTP compatible # timestamp, so we can simply compare the strings diff --git a/common/djangoapps/contentserver/tests/__init__.py b/common/djangoapps/contentserver/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/contentserver/tests/test.py b/common/djangoapps/contentserver/tests/test.py new file mode 100644 index 0000000000..94a614c85a --- /dev/null +++ b/common/djangoapps/contentserver/tests/test.py @@ -0,0 +1,136 @@ +""" +Tests for StaticContentServer +""" +import copy +import logging +from uuid import uuid4 +from path import path +from pymongo import MongoClient + +from django.contrib.auth.models import User +from django.conf import settings +from django.test.client import Client +from django.test.utils import override_settings + +from student.models import CourseEnrollment + +from xmodule.contentstore.django import contentstore, _CONTENTSTORE +from xmodule.modulestore import Location +from xmodule.contentstore.content import StaticContent +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import (studio_store_config, + ModuleStoreTestCase) +from xmodule.modulestore.xml_importer import import_from_xml + +log = logging.getLogger(__name__) + +TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) +TEST_DATA_CONTENTSTORE['OPTIONS']['db'] = 'test_xcontent_%s' % uuid4().hex + +TEST_MODULESTORE = studio_store_config(settings.TEST_ROOT / "data") + + +@override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE, MODULESTORE=TEST_MODULESTORE) +class ContentStoreToyCourseTest(ModuleStoreTestCase): + """ + Tests that use the toy course. + """ + + def setUp(self): + """ + Create user and login. + """ + + settings.MODULESTORE['default']['OPTIONS']['fs_root'] = path('common/test/data') + settings.MODULESTORE['direct']['OPTIONS']['fs_root'] = path('common/test/data') + + self.client = Client() + self.contentstore = contentstore() + + # A locked asset + self.loc_locked = Location('c4x', 'edX', 'toy', 'asset', 'sample_static.txt') + self.url_locked = StaticContent.get_url_path_from_location(self.loc_locked) + + # An unlocked asset + self.loc_unlocked = Location('c4x', 'edX', 'toy', 'asset', 'another_static.txt') + self.url_unlocked = StaticContent.get_url_path_from_location(self.loc_unlocked) + + import_from_xml(modulestore('direct'), 'common/test/data/', ['toy'], + static_content_store=self.contentstore, verbose=True) + + self.contentstore.set_attr(self.loc_locked, 'locked', True) + + # Create user + self.usr = 'testuser' + self.pwd = 'foo' + email = 'test+courses@edx.org' + self.user = User.objects.create_user(self.usr, email, self.pwd) + self.user.is_active = True + self.user.save() + + # Create staff user + self.staff_usr = 'stafftestuser' + self.staff_pwd = 'foo' + staff_email = 'stafftest+courses@edx.org' + self.staff_user = User.objects.create_user(self.staff_usr, staff_email, + self.staff_pwd) + self.staff_user.is_active = True + self.staff_user.is_staff = True + self.staff_user.save() + + def tearDown(self): + + MongoClient().drop_database(TEST_DATA_CONTENTSTORE['OPTIONS']['db']) + _CONTENTSTORE.clear() + + def test_unlocked_asset(self): + """ + Test that unlocked assets are being served. + """ + self.client.logout() + resp = self.client.get(self.url_unlocked) + self.assertEqual(resp.status_code, 200) #pylint: disable=E1103 + + def test_locked_asset_not_logged_in(self): + """ + Test that locked assets behave appropriately in case the user is not + logged in. + """ + self.client.logout() + resp = self.client.get(self.url_locked) + self.assertEqual(resp.status_code, 403) #pylint: disable=E1103 + + def test_locked_asset_not_registered(self): + """ + Test that locked assets behave appropriately in case user is logged in + in but not registered for the course. + """ + self.client.login(username=self.usr, password=self.pwd) + resp = self.client.get(self.url_locked) + self.assertEqual(resp.status_code, 403) #pylint: disable=E1103 + + def test_locked_asset_registered(self): + """ + Test that locked assets behave appropriately in case user is logged in + and registered for the course. + """ + #pylint: disable=E1101 + course_id = "/".join([self.loc_locked.org, self.loc_locked.course, '2012_Fall']) + CourseEnrollment.enroll(self.user, course_id) + self.assertTrue(CourseEnrollment.is_enrolled(self.user, course_id)) + + self.client.login(username=self.usr, password=self.pwd) + resp = self.client.get(self.url_locked) + self.assertEqual(resp.status_code, 200) #pylint: disable=E1103 + + def test_locked_asset_staff(self): + """ + Test that locked assets behave appropriately in case user is staff. + """ + #pylint: disable=E1101 + course_id = "/".join([self.loc_locked.org, self.loc_locked.course, '2012_Fall']) + + self.client.login(username=self.staff_usr, password=self.staff_pwd) + resp = self.client.get(self.url_locked) + self.assertEqual(resp.status_code, 200) #pylint: disable=E1103 + diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 71cff4183b..da16a2fda2 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -843,6 +843,31 @@ class CourseEnrollment(models.Model): except cls.DoesNotExist: return False + @classmethod + def is_enrolled_by_partial(cls, user, course_id_partial): + """ + Returns `True` if the user is enrolled in a course that starts with + `course_id_partial`. Otherwise, returns False. + + Can be used to determine whether a student is enrolled in a course + whose run name is unknown. + + `user` is a Django User object. If it hasn't been saved yet (no `.id` + attribute), this method will automatically save it before + adding an enrollment for it. + + `course_id_partial` is a starting substring for a fully qualified + course_id (e.g. "edX/Test101/"). + """ + try: + return CourseEnrollment.objects.filter( + user=user, + course_id__startswith=course_id_partial, + is_active=1 + ).exists() + except cls.DoesNotExist: + return False + @classmethod def enrollment_mode_for_user(cls, user, course_id): """ diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 397816ec00..a6466ee9f9 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -213,23 +213,34 @@ class EnrollInCourseTest(TestCase): def test_enrollment(self): user = User.objects.create_user("joe", "joe@joe.com", "password") course_id = "edX/Test101/2013" + course_id_partial = "edX/Test101" # Test basic enrollment self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) + self.assertFalse(CourseEnrollment.is_enrolled_by_partial(user, + course_id_partial)) CourseEnrollment.enroll(user, course_id) self.assertTrue(CourseEnrollment.is_enrolled(user, course_id)) + self.assertTrue(CourseEnrollment.is_enrolled_by_partial(user, + course_id_partial)) # Enrolling them again should be harmless CourseEnrollment.enroll(user, course_id) self.assertTrue(CourseEnrollment.is_enrolled(user, course_id)) + self.assertTrue(CourseEnrollment.is_enrolled_by_partial(user, + course_id_partial)) # Now unenroll the user CourseEnrollment.unenroll(user, course_id) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) + self.assertFalse(CourseEnrollment.is_enrolled_by_partial(user, + course_id_partial)) # Unenrolling them again should also be harmless CourseEnrollment.unenroll(user, course_id) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) + self.assertFalse(CourseEnrollment.is_enrolled_by_partial(user, + course_id_partial)) # The enrollment record should still exist, just be inactive enrollment_record = CourseEnrollment.objects.get( diff --git a/common/test/data/toy/static/another_static.txt b/common/test/data/toy/static/another_static.txt new file mode 100644 index 0000000000..83e560736d --- /dev/null +++ b/common/test/data/toy/static/another_static.txt @@ -0,0 +1,17 @@ + + _ + | | + _____ ____ _ _ __ ___ _ __ | | ___ + / _ \ \/ / _` | '_ ` _ \| '_ \| |/ _ \ +| __/> < (_| | | | | | | |_) | | __/ + \___/_/\_\__,_|_| |_| |_| .__/|_|\___| + | | + |_| + _ _ _ + | | | | (_) + ___| |_ __ _| |_ _ ___ + / __| __/ _` | __| |/ __| + \__ \ || (_| | |_| | (__ + |___/\__\__,_|\__|_|\___| + + diff --git a/lms/envs/common.py b/lms/envs/common.py index 81b01179d7..466ec262f9 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -542,7 +542,6 @@ TEMPLATE_LOADERS = ( ) MIDDLEWARE_CLASSES = ( - 'contentserver.middleware.StaticContentServer', 'request_cache.middleware.RequestCache', 'django_comment_client.middleware.AjaxExceptionMiddleware', 'django.middleware.common.CommonMiddleware', @@ -551,6 +550,7 @@ MIDDLEWARE_CLASSES = ( # Instead of AuthenticationMiddleware, we use a cached backed version #'django.contrib.auth.middleware.AuthenticationMiddleware', 'cache_toolbox.middleware.CacheBackedAuthenticationMiddleware', + 'contentserver.middleware.StaticContentServer', 'django.contrib.messages.middleware.MessageMiddleware', 'track.middleware.TrackMiddleware', diff --git a/lms/envs/test.py b/lms/envs/test.py index 94feffdf3e..45e934ec1f 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -97,6 +97,13 @@ MODULESTORE = { } } +CONTENTSTORE = { + 'ENGINE': 'xmodule.contentstore.mongo.MongoContentStore', + 'OPTIONS': { + 'host': 'localhost', + 'db': 'xcontent', + } +} DATABASES = { 'default': {