From d78d52e132bdcda1d3d09ba84106cb6f4fc39718 Mon Sep 17 00:00:00 2001 From: noraiz-anwar Date: Tue, 25 Apr 2017 15:30:15 +0500 Subject: [PATCH] Add masquerading feature for preview mode --- lms/djangoapps/courseware/access.py | 86 +++++++------------ .../courseware/tests/test_access.py | 83 ++++++++---------- 2 files changed, 68 insertions(+), 101 deletions(-) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index e72a129a59..c60368fe1d 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -136,8 +136,9 @@ def has_access(user, action, obj, course_key=None): if not user: user = AnonymousUser() - if in_preview_mode(): - if not bool(has_staff_access_to_preview_mode(user=user, obj=obj, course_key=course_key)): + # Preview mode is only accessible by staff. + if in_preview_mode() and course_key: + if not has_staff_access_to_preview_mode(user, course_key): return ACCESS_DENIED # delegate the work to type-specific functions. @@ -173,51 +174,14 @@ def has_access(user, action, obj, course_key=None): .format(type(obj))) -# ================ Implementation helpers ================================ - -def has_staff_access_to_preview_mode(user, obj, course_key=None): +def has_staff_access_to_preview_mode(user, course_key): """ - Returns whether user has staff access to specified modules or not. - - Arguments: - - user: a Django user object. - - obj: The object to check access for. - - course_key: A course_key specifying which course this access is for. - - Returns an AccessResponse object. + Checks if given user can access course in preview mode. + A user can access a course in preview mode only if User has staff access to course. """ - if course_key is None: - if isinstance(obj, CourseDescriptor) or isinstance(obj, CourseOverview): - course_key = obj.id + has_admin_access_to_course = any(administrative_accesses_to_course_for_user(user, course_key)) - elif isinstance(obj, ErrorDescriptor): - course_key = obj.location.course_key - - elif isinstance(obj, XModule): - course_key = obj.descriptor.course_key - - elif isinstance(obj, XBlock): - course_key = obj.location.course_key - - elif isinstance(obj, CCXLocator): - course_key = obj.to_course_locator() - - elif isinstance(obj, CourseKey): - course_key = obj - - elif isinstance(obj, UsageKey): - course_key = obj.course_key - - if course_key is None: - if GlobalStaff().has_user(user): - return ACCESS_GRANTED - else: - return ACCESS_DENIED - - return _has_access_to_course(user, 'staff', course_key=course_key) + return has_admin_access_to_course or is_masquerading_as_student(user, course_key) def _can_access_descriptor_with_start_date(user, descriptor, course_key): # pylint: disable=invalid-name @@ -733,10 +697,12 @@ def _has_access_to_course(user, access_level, course_key): debug("Deny: no user or anon user") return ACCESS_DENIED - if not in_preview_mode() and is_masquerading_as_student(user, course_key): + if is_masquerading_as_student(user, course_key): return ACCESS_DENIED - if GlobalStaff().has_user(user): + global_staff, staff_access, instructor_access = administrative_accesses_to_course_for_user(user, course_key) + + if global_staff: debug("Allow: user.is_staff") return ACCESS_GRANTED @@ -745,19 +711,10 @@ def _has_access_to_course(user, access_level, course_key): debug("Deny: unknown access level") return ACCESS_DENIED - staff_access = ( - CourseStaffRole(course_key).has_user(user) or - OrgStaffRole(course_key.org).has_user(user) - ) if staff_access and access_level == 'staff': debug("Allow: user has course staff access") return ACCESS_GRANTED - instructor_access = ( - CourseInstructorRole(course_key).has_user(user) or - OrgInstructorRole(course_key.org).has_user(user) - ) - if instructor_access and access_level in ('staff', 'instructor'): debug("Allow: user has course instructor access") return ACCESS_GRANTED @@ -766,6 +723,25 @@ def _has_access_to_course(user, access_level, course_key): return ACCESS_DENIED +def administrative_accesses_to_course_for_user(user, course_key): + """ + Returns types of access a user have for given course. + """ + global_staff = GlobalStaff().has_user(user) + + staff_access = ( + CourseStaffRole(course_key).has_user(user) or + OrgStaffRole(course_key.org).has_user(user) + ) + + instructor_access = ( + CourseInstructorRole(course_key).has_user(user) or + OrgInstructorRole(course_key.org).has_user(user) + ) + + return global_staff, staff_access, instructor_access + + def _has_instructor_access_to_descriptor(user, descriptor, course_key): # pylint: disable=invalid-name """Helper method that checks whether the user has staff access to the course of the location. diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index 80c954558f..f509c9f6fe 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -199,44 +199,41 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes def test_has_staff_access_to_preview_mode(self): """ - Tests users have right access to content in preview mode. + Test that preview mode is only accessible by staff users. """ course_key = self.course.id - usage_key = self.course.scope_ids.usage_id - chapter = ItemFactory.create(category="chapter", parent_location=self.course.location) - overview = CourseOverview.get_from_id(course_key) - test_system = get_test_system() - - ccx = CcxFactory(course_id=course_key) - ccx_locator = CCXLocator.from_course_locator(course_key, ccx.id) - - error_descriptor = ErrorDescriptor.from_xml( - u"ABC \N{SNOWMAN}", - test_system, - CourseLocationManager(course_key), - "error msg" - ) - # Enroll student to the course CourseEnrollmentFactory(user=self.student, course_id=self.course.id) - modules = [ - self.course, - overview, - chapter, - ccx_locator, - error_descriptor, - course_key, - usage_key, - ] - # Course key is not None - self.assertTrue( - bool(access.has_staff_access_to_preview_mode(self.global_staff, obj=self.course, course_key=course_key)) - ) - for user in [self.global_staff, self.course_staff, self.course_instructor]: - for obj in modules: - self.assertTrue(bool(access.has_staff_access_to_preview_mode(user, obj=obj))) - self.assertFalse(bool(access.has_staff_access_to_preview_mode(self.student, obj=obj))) + self.assertTrue(access.has_staff_access_to_preview_mode(user, course_key)) + + self.assertFalse(access.has_staff_access_to_preview_mode(self.student, course_key)) + + # we don't want to restrict a staff user, masquerading as student, + # to access preview mode. + + # Note that self.student now have access to preview mode, + # `is_masquerading_as_student == True` means user is staff and is + # masquerading as a student. + with patch('courseware.access.is_masquerading_as_student') as mock_masquerade: + mock_masquerade.return_value = True + for user in [self.global_staff, self.course_staff, self.course_instructor, self.student]: + self.assertTrue(access.has_staff_access_to_preview_mode(user, course_key)) + + def test_administrative_accesses_to_course_for_user(self): + """ + Test types of admin accesses to a course + """ + course_key = self.course.id + + # `administrative_accesses_to_course_for_user` returns accesses in tuple as + # (`global_staff`, `course_staff`, `course_instructor`). + # Order matters here, for example `True` at first index in tuple essentially means + # given user is a global staff. + for count, user in enumerate([self.global_staff, self.course_staff, self.course_instructor]): + self.assertTrue(access.administrative_accesses_to_course_for_user(user, course_key)[count]) + + self.assertFalse(any(access.administrative_accesses_to_course_for_user(self.student, course_key))) def test_student_has_access(self): """ @@ -264,15 +261,6 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes for obj in modules: self.assertFalse(bool(access.has_access(self.student, 'load', obj, course_key=self.course.id))) - def test_string_has_staff_access_to_preview_mode(self): - """ - Tests different users has right access to string content in preview mode. - """ - self.assertTrue(bool(access.has_staff_access_to_preview_mode(self.global_staff, obj='global'))) - self.assertFalse(bool(access.has_staff_access_to_preview_mode(self.course_staff, obj='global'))) - self.assertFalse(bool(access.has_staff_access_to_preview_mode(self.course_instructor, obj='global'))) - self.assertFalse(bool(access.has_staff_access_to_preview_mode(self.student, obj='global'))) - @patch('courseware.access.in_preview_mode', Mock(return_value=True)) def test_has_access_with_preview_mode(self): """ @@ -286,17 +274,17 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes self.assertFalse(bool(access.has_access(self.student, 'staff', self.course, course_key=self.course.id))) self.assertFalse(bool(access.has_access(self.student, 'load', self.course, course_key=self.course.id))) - # User should be able to preview when masquerade. + # When masquerading is true, user should not be able to access staff content with patch('courseware.access.is_masquerading_as_student') as mock_masquerade: mock_masquerade.return_value = True - self.assertTrue( + self.assertFalse( bool(access.has_access(self.global_staff, 'staff', self.course, course_key=self.course.id)) ) self.assertFalse( bool(access.has_access(self.student, 'staff', self.course, course_key=self.course.id)) ) - @patch('courseware.access.in_preview_mode', Mock(return_value=True)) + @patch('courseware.access_utils.in_preview_mode', Mock(return_value=True)) def test_has_access_in_preview_mode_with_group(self): """ Test that a user masquerading as a member of a group sees appropriate content in preview mode. @@ -851,7 +839,10 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase): user = getattr(self, user_attr_name) user = User.objects.get(id=user.id) - if user_attr_name == 'user_staff' and action == 'see_exists' and course_attr_name == 'course_not_started': + if (user_attr_name == 'user_staff' and + action == 'see_exists' and + course_attr_name in + ['course_default', 'course_not_started']): # checks staff role num_queries = 1 elif user_attr_name == 'user_normal' and action == 'see_exists' and course_attr_name != 'course_started':