From 10465e3eb087e5e777844ec50a52dad3bc2c62ce Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 24 Feb 2014 23:41:57 -0500 Subject: [PATCH] add some authorization checks to the Wiki --- common/lib/xmodule/xmodule/course_module.py | 4 +++ lms/djangoapps/course_wiki/course_nav.py | 24 +++++++++++-- lms/djangoapps/course_wiki/tests/tests.py | 38 +++++++++++++++++++++ lms/envs/common.py | 5 +++ 4 files changed, 69 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index d1620c1ae4..c56569dd3a 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/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 diff --git a/lms/djangoapps/course_wiki/course_nav.py b/lms/djangoapps/course_wiki/course_nav.py index 434860a0f7..84dd84d499 100644 --- a/lms/djangoapps/course_wiki/course_nav.py +++ b/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[^/]+/[^/]+/[^/]+)/wiki/(?P.*|)$' +IN_COURSE_WIKI_COMPILED_REGEX = re.compile(IN_COURSE_WIKI_REGEX) +WIKI_ROOT_ACCESS_COMPILED_REGEX = re.compile(r'^/wiki/(?P.*|)$') 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 diff --git a/lms/djangoapps/course_wiki/tests/tests.py b/lms/djangoapps/course_wiki/tests/tests.py index 55fc47e9dd..a1a2b58536 100644 --- a/lms/djangoapps/course_wiki/tests/tests.py +++ b/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) diff --git a/lms/envs/common.py b/lms/envs/common.py index 7a31bd69df..a9d2f75726 100644 --- a/lms/envs/common.py +++ b/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