diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index dee09c0d59..7d3c9d53a3 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/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.") diff --git a/lms/djangoapps/courseware/middleware.py b/lms/djangoapps/courseware/middleware.py new file mode 100644 index 0000000000..7ef0844db5 --- /dev/null +++ b/lms/djangoapps/courseware/middleware.py @@ -0,0 +1,23 @@ +""" +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()] + ) + ) diff --git a/lms/djangoapps/courseware/tests/test_middleware.py b/lms/djangoapps/courseware/tests/test_middleware.py new file mode 100644 index 0000000000..ff92b33e92 --- /dev/null +++ b/lms/djangoapps/courseware/tests/test_middleware.py @@ -0,0 +1,53 @@ +""" +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) diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index 6951318a1f..5cc8150770 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/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()) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 0f450e66ec..73582db039 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/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) diff --git a/lms/envs/common.py b/lms/envs/common.py index 5c045bd0d5..b331e39340 100644 --- a/lms/envs/common.py +++ b/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', )