Merge pull request #2713 from edx/cdodge/add-auth-to-wiki

add some authorization checks to the Wiki

common/lib/xmodule/xmodule/course_module.py

@@ -399,6 +399,10 @@ class CourseFields(object):
     max_student_enrollments_allowed = Integer(help="Limit the number of students allowed to enroll in this course.",
                                               scope=Scope.settings)
 
+    allow_public_wiki_access = Boolean(help="Whether to allow an unenrolled user to view the Wiki",
+                                       default=False,
+                                       scope=Scope.settings)
+
 class CourseDescriptor(CourseFields, SequenceDescriptor):
     module_class = SequenceModule
 

lms/djangoapps/course_wiki/course_nav.py

@@ -3,14 +3,19 @@ from urlparse import urlparse
 
 from django.http import Http404
 from django.shortcuts import redirect
+from django.conf import settings
+from django.core.exceptions import PermissionDenied
 
 from wiki.models import reverse as wiki_reverse
 from courseware.access import has_access
 from courseware.courses import get_course_with_access
+from student.models import CourseEnrollment
 
 
 IN_COURSE_WIKI_REGEX = r'/courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/wiki/(?P<wiki_path>.*|)$'
 
+IN_COURSE_WIKI_COMPILED_REGEX = re.compile(IN_COURSE_WIKI_REGEX)
+WIKI_ROOT_ACCESS_COMPILED_REGEX = re.compile(r'^/wiki/(?P<wiki_path>.*|)$')
 
 class Middleware(object):
     """
@@ -44,6 +49,11 @@ class Middleware(object):
         referer = request.META.get('HTTP_REFERER')
         destination = request.path
 
+        # Check to see if we don't allow top-level access to the wiki via the /wiki/xxxx/yyy/zzz URLs
+        # this will help prevent people from writing pell-mell to the Wiki in an unstructured way
+        path_match = WIKI_ROOT_ACCESS_COMPILED_REGEX.match(destination)
+        if path_match and not settings.FEATURES.get('ALLOW_WIKI_ROOT_ACCESS', False):
+            raise PermissionDenied()
 
         if request.method == 'GET':
             new_destination = self.get_redirected_url(request.user, referer, destination)
@@ -53,10 +63,20 @@ class Middleware(object):
                 self.redirected = True
                 return redirect(new_destination)
 
-        course_match = re.match(IN_COURSE_WIKI_REGEX, destination)
+        course_match = IN_COURSE_WIKI_COMPILED_REGEX.match(destination)
         if course_match:
             course_id = course_match.group('course_id')
-            prepend_string = '/courses/' + course_match.group('course_id')
+
+            # Authorization Check
+            # Let's see if user is enrolled or the course allows for public access
+            course = get_course_with_access(request.user, course_id, 'load')
+            if not course.allow_public_wiki_access:
+                is_enrolled = CourseEnrollment.is_enrolled(request.user, course.id)
+                is_staff = has_access(request.user, course, 'staff')
+                if not (is_enrolled or is_staff):
+                    raise PermissionDenied()
+
+            prepend_string = '/courses/' + course_id
             wiki_reverse._transform_url = lambda url: prepend_string + url
 
         return None

lms/djangoapps/course_wiki/tests/tests.py

@@ -5,6 +5,8 @@ from courseware.tests.tests import LoginEnrollmentTestCase
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
 from xmodule.modulestore.django import modulestore
 
+from mock import patch
+
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
 class WikiRedirectTestCase(LoginEnrollmentTestCase):
@@ -23,6 +25,7 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
         self.activate_user(self.student)
         self.activate_user(self.instructor)
 
+    @patch.dict("django.conf.settings.FEATURES", {'ALLOW_WIKI_ROOT_ACCESS': True})
     def test_wiki_redirect(self):
         """
         Test that requesting wiki URLs redirect properly to or out of classes.
@@ -60,6 +63,22 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
         self.assertEqual(resp.status_code, 302)
         self.assertEqual(resp['Location'], 'http://testserver' + destination)
 
+    @patch.dict("django.conf.settings.FEATURES", {'ALLOW_WIKI_ROOT_ACCESS': False})
+    def test_wiki_no_root_access(self):
+        """
+        Test to verify that normally Wiki's cannot be browsed from the /wiki/xxxx/yyy/zz URLs
+
+        """
+        self.login(self.student, self.password)
+
+        self.enroll(self.toy)
+
+        referer = reverse("progress", kwargs={'course_id': self.toy.id})
+        destination = reverse("wiki:get", kwargs={'path': 'some/fake/wiki/page/'})
+
+        resp = self.client.get(destination, HTTP_REFERER=referer)
+        self.assertEqual(resp.status_code, 403)
+
     def create_course_page(self, course):
         """
         Test that loading the course wiki page creates the wiki page.
@@ -88,6 +107,7 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
         self.assertContains(resp, "Course Info")
         self.assertContains(resp, "courseware")
 
+    @patch.dict("django.conf.settings.FEATURES", {'ALLOW_WIKI_ROOT_ACCESS': True})
     def test_course_navigator(self):
         """"
         Test that going from a course page to a wiki page contains the course navigator.
@@ -103,3 +123,21 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
         resp = self.client.get(course_wiki_page, follow=True, HTTP_REFERER=referer)
 
         self.has_course_navigator(resp)
+
+    @patch.dict("django.conf.settings.FEATURES", {'ALLOW_WIKI_ROOT_ACCESS': True})
+    def test_wiki_not_accessible_when_not_enrolled(self):
+        """"
+        Test that going from a course page to a wiki page contains the course navigator.
+        """
+
+        self.login(self.instructor, self.password)
+        self.enroll(self.toy)
+        self.create_course_page(self.toy)
+
+        self.login(self.student, self.password)
+        course_wiki_page = reverse('wiki:get', kwargs={'path': self.toy.wiki_slug + '/'})
+        referer = reverse("courseware", kwargs={'course_id': self.toy.id})
+
+        resp = self.client.get(course_wiki_page, follow=True, HTTP_REFERER=referer)
+
+        self.assertEquals(resp.status_code, 403)

lms/envs/common.py

@@ -224,6 +224,11 @@ FEATURES = {
 
     # Toggle embargo functionality
     'EMBARGO': False,
+
+    # Whether the Wiki subsystem should be accessible via the direct /wiki/ paths. Setting this to True means
+    # that people can submit content and modify the Wiki in any arbitrary manner. We're leaving this as True in the
+    # defaults, so that we maintain current behavior
+    'ALLOW_WIKI_ROOT_ACCESS': True,
 }
 
 # Used for A/B testing