Merge pull request #3070 from edx/adam/middleware

wiki middleware fix (LMS-2461)

lms/djangoapps/course_wiki/course_nav.py

@@ -4,6 +4,7 @@ from urlparse import urlparse
 from django.http import Http404
 from django.shortcuts import redirect
 from django.conf import settings
+from django.core.urlresolvers import reverse
 from django.core.exceptions import PermissionDenied
 
 from wiki.models import reverse as wiki_reverse
@@ -71,10 +72,16 @@ class Middleware(object):
             # 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:
+                # if a user is not authenticated, redirect them to login
+                if not request.user.is_authenticated():
+                    return redirect(reverse('accounts_login'))
+
                 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()
+                    # if a user is logged in, but not authorized to see a page,
+                    # we'll redirect them to the course about page
+                    return redirect(reverse('about_course', args=[course_id]))
 
             prepend_string = '/courses/' + course_id
             wiki_reverse._transform_url = lambda url: prepend_string + url

lms/djangoapps/course_wiki/tests/tests.py

@@ -4,6 +4,7 @@ from django.test.utils import override_settings
 from courseware.tests.tests import LoginEnrollmentTestCase
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.tests.factories import CourseFactory
 
 from mock import patch
 
@@ -126,8 +127,9 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
 
     @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.
+        """
+        Test that going from a course page to a wiki page when not enrolled
+        redirects a user to the course about page
         """
 
         self.login(self.instructor, self.password)
@@ -138,6 +140,33 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
         course_wiki_page = reverse('wiki:get', kwargs={'path': self.toy.wiki_slug + '/'})
         referer = reverse("courseware", kwargs={'course_id': self.toy.id})
 
+        # When not enrolled, we should get a 302
+        resp = self.client.get(course_wiki_page, follow=False, HTTP_REFERER=referer)
+        self.assertEqual(resp.status_code, 302)
+
+        # and end up at the course about page
         resp = self.client.get(course_wiki_page, follow=True, HTTP_REFERER=referer)
+        target_url, __ = resp.redirect_chain[-1]
+        self.assertTrue(
+            target_url.endswith(reverse('about_course', args=[self.toy.id]))
+        )
+
+    @patch.dict("django.conf.settings.FEATURES", {'ALLOW_WIKI_ROOT_ACCESS': True})
+    def test_redirect_when_not_logged_in(self):
+        """
+        Test that attempting to reach a course wiki page when not logged in
+        redirects the user to the login page
+        """
+        self.logout()
+        course_wiki_page = reverse('wiki:get', kwargs={'path': self.toy.wiki_slug + '/'})
+
+        # When not logged in, we should get a 302
+        resp = self.client.get(course_wiki_page, follow=False)
+        self.assertEqual(resp.status_code, 302)
 
-        self.assertEquals(resp.status_code, 403)
+        # and end up at the login page
+        resp = self.client.get(course_wiki_page, follow=True)
+        target_url, __ = resp.redirect_chain[-1]
+        self.assertTrue(
+            target_url.endswith(reverse('accounts_login'))
+        )