Add middleware optionally to catch unenrolled students who fail has_access (TNL-286)

lms/djangoapps/courseware/courses.py

@@ -20,6 +20,7 @@ from xmodule.x_module import STUDENT_VIEW
 from courseware.access import has_access
 from courseware.model_data import FieldDataCache
 from courseware.module_render import get_module
+from student.models import CourseEnrollment
 import branding
 
 log = logging.getLogger(__name__)
@@ -72,7 +73,13 @@ def get_course_by_id(course_key, depth=0):
         raise Http404("Course not found.")
 
 
-def get_course_with_access(user, action, course_key, depth=0):
+class UserNotEnrolled(Http404):
+    def __init__(self, course_key):
+        super(UserNotEnrolled, self).__init__()
+        self.course_key = course_key
+
+
+def get_course_with_access(user, action, course_key, depth=0, check_if_enrolled=False):
     """
     Given a course_key, look up the corresponding course descriptor,
     check that the user has the access to perform the specified action
@@ -86,6 +93,11 @@ def get_course_with_access(user, action, course_key, depth=0):
     course = get_course_by_id(course_key, depth=depth)
 
     if not has_access(user, action, course, course_key):
+        if check_if_enrolled and not CourseEnrollment.is_enrolled(user, course_key):
+            # If user is not enrolled, raise UserNotEnrolled exception that will
+            # be caught by middleware
+            raise UserNotEnrolled(course_key)
+
         # Deliberately return a non-specific error message to avoid
         # leaking info about access control settings
         raise Http404("Course not found.")

lms/djangoapps/courseware/middleware.py

+"""
+Middleware for the courseware app
+"""
+
+from django.shortcuts import redirect
+from django.core.urlresolvers import reverse
+
+from courseware.courses import UserNotEnrolled
+
+class RedirectUnenrolledMiddleware(object):
+    """
+    Catch UserNotEnrolled errors thrown by `get_course_with_access` and redirect
+    users to the course about page
+    """
+    def process_exception(self, request, exception):
+        if isinstance(exception, UserNotEnrolled):
+            course_key = exception.course_key
+            return redirect(
+                reverse(
+                    'courseware.views.course_about',
+                    args=[course_key.to_deprecated_string()]
+                )
+            )

lms/djangoapps/courseware/tests/test_middleware.py

+"""
+Tests for courseware middleware
+"""
+
+from django.core.urlresolvers import reverse
+from django.test.utils import override_settings
+from django.test.client import RequestFactory
+from django.http import Http404
+from mock import patch
+
+from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
+import courseware.courses as courses
+from courseware.middleware import RedirectUnenrolledMiddleware
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
+
+
+@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+class CoursewareMiddlewareTestCase(ModuleStoreTestCase):
+    """Tests that courseware middleware is correctly redirected"""
+
+    def setUp(self):
+        self.course = CourseFactory.create()
+
+    def check_user_not_enrolled_redirect(self):
+        """A UserNotEnrolled exception should trigger a redirect"""
+        request = RequestFactory().get("dummy_url")
+        response = RedirectUnenrolledMiddleware().process_exception(
+            request, courses.UserNotEnrolled(self.course.id)
+        )
+        self.assertEqual(response.status_code, 302)
+        # make sure we redirect to the course about page
+        expected_url = reverse(
+            "about_course", args=[self.course.id.to_deprecated_string()]
+        )
+
+        target_url = response._headers['location'][1]
+        self.assertTrue(target_url.endswith(expected_url))
+
+    def test_user_not_enrolled_redirect(self):
+        self.check_user_not_enrolled_redirect()
+
+    @patch.dict("django.conf.settings.FEATURES", {"ENABLE_MKTG_SITE": True})
+    def test_user_not_enrolled_redirect_mktg(self):
+        self.check_user_not_enrolled_redirect()
+
+    def test_process_404(self):
+        """A 404 should not trigger anything"""
+        request = RequestFactory().get("dummy_url")
+        response = RedirectUnenrolledMiddleware().process_exception(
+            request, Http404()
+        )
+        self.assertIsNone(response)

lms/djangoapps/django_comment_client/forum/tests.py

@@ -20,6 +20,7 @@ from django_comment_client.tests.utils import CohortedContentTestCase
 from django_comment_client.forum import views
 
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
+from courseware.courses import UserNotEnrolled
 from nose.tools import assert_true  # pylint: disable=E0611
 from mock import patch, Mock, ANY, call
 
@@ -913,3 +914,26 @@ class FollowedThreadsUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin):
         response_data = json.loads(response.content)
         self.assertEqual(response_data["discussion_data"][0]["title"], text)
         self.assertEqual(response_data["discussion_data"][0]["body"], text)
+
+
+@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+class EnrollmentTestCase(ModuleStoreTestCase):
+    """
+    Tests for the behavior of views depending on if the student is enrolled
+    in the course
+    """
+
+    @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
+    def setUp(self):
+        super(EnrollmentTestCase, self).setUp()
+        self.course = CourseFactory.create()
+        self.student = UserFactory.create()
+
+    @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
+    @patch('lms.lib.comment_client.utils.requests.request')
+    def test_unenrolled(self, mock_request):
+        mock_request.side_effect = make_mock_request_impl('dummy')
+        request = RequestFactory().get('dummy_url')
+        request.user = self.student
+        with self.assertRaises(UserNotEnrolled):
+            views.forum_form_discussion(request, course_id=self.course.id.to_deprecated_string())

lms/djangoapps/django_comment_client/forum/views.py

@@ -163,7 +163,7 @@ def forum_form_discussion(request, course_id):
     course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
     nr_transaction = newrelic.agent.current_transaction()
 
-    course = get_course_with_access(request.user, 'load_forum', course_key)
+    course = get_course_with_access(request.user, 'load_forum', course_key, check_if_enrolled=True)
     course_settings = make_course_settings(course, include_category_map=True)
 
     user = cc.User.from_django_user(request.user)

lms/envs/common.py

@@ -941,6 +941,9 @@ MIDDLEWARE_CLASSES = (
     # use Django built in clickjacking protection
     'django.middleware.clickjacking.XFrameOptionsMiddleware',
 
+    # to redirected unenrolled students to the course info page
+    'courseware.middleware.RedirectUnenrolledMiddleware',
+
     'course_wiki.middleware.WikiAccessMiddleware',
 )