diff --git a/common/djangoapps/xblock_django/constants.py b/common/djangoapps/xblock_django/constants.py index d93aac40a2..ade2391237 100644 --- a/common/djangoapps/xblock_django/constants.py +++ b/common/djangoapps/xblock_django/constants.py @@ -3,12 +3,26 @@ Constants used by DjangoXBlockUserService """ # Optional attributes stored on the XBlockUser + +# The anonymous user ID for the user in the course. ATTR_KEY_ANONYMOUS_USER_ID = 'edx-platform.anonymous_user_id' +# The global (course-agnostic) anonymous user ID for the user. ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID = 'edx-platform.deprecated_anonymous_user_id' +# The country code determined from the user's request IP address. ATTR_KEY_REQUEST_COUNTRY_CODE = 'edx-platform.request_country_code' +# Whether the user is authenticated or anonymous. ATTR_KEY_IS_AUTHENTICATED = 'edx-platform.is_authenticated' +# The personally identifiable user ID. ATTR_KEY_USER_ID = 'edx-platform.user_id' +# The username. ATTR_KEY_USERNAME = 'edx-platform.username' +# Whether the user is enrolled in the course as a Beta Tester. +ATTR_KEY_USER_IS_BETA_TESTER = 'edx-platform.user_is_beta_tester' +# Whether the user has staff access to the platform. +ATTR_KEY_USER_IS_GLOBAL_STAFF = 'edx-platform.user_is_global_staff' +# Whether the user is a course team member with 'Staff' or 'Admin' access. ATTR_KEY_USER_IS_STAFF = 'edx-platform.user_is_staff' +# A dict containing user's entries from the `UserPreference` model. ATTR_KEY_USER_PREFERENCES = 'edx-platform.user_preferences' +# The user's role in the course ('staff', 'instructor', or 'student'). ATTR_KEY_USER_ROLE = 'edx-platform.user_role' diff --git a/common/djangoapps/xblock_django/tests/test_user_service.py b/common/djangoapps/xblock_django/tests/test_user_service.py index 33cfa9aad4..603069d740 100644 --- a/common/djangoapps/xblock_django/tests/test_user_service.py +++ b/common/djangoapps/xblock_django/tests/test_user_service.py @@ -16,6 +16,8 @@ from common.djangoapps.xblock_django.user_service import ( ATTR_KEY_ANONYMOUS_USER_ID, ATTR_KEY_REQUEST_COUNTRY_CODE, ATTR_KEY_USER_ID, + ATTR_KEY_USER_IS_BETA_TESTER, + ATTR_KEY_USER_IS_GLOBAL_STAFF, ATTR_KEY_USER_IS_STAFF, ATTR_KEY_USER_PREFERENCES, ATTR_KEY_USER_ROLE, @@ -49,11 +51,15 @@ class UserServiceTestCase(TestCase): self.assertListEqual(xb_user.emails, []) def assert_xblock_user_matches_django( - self, xb_user, dj_user, + self, + xb_user, + dj_user, user_is_staff=False, user_role=None, anonymous_user_id=None, request_country_code=None, + user_is_global_staff=False, + user_is_beta_tester=False, ): """ A set of assertions for comparing a XBlockUser to a django User @@ -63,7 +69,9 @@ class UserServiceTestCase(TestCase): assert xb_user.full_name == dj_user.profile.name assert xb_user.opt_attrs[ATTR_KEY_USERNAME] == dj_user.username assert xb_user.opt_attrs[ATTR_KEY_USER_ID] == dj_user.id + assert xb_user.opt_attrs[ATTR_KEY_USER_IS_BETA_TESTER] == user_is_beta_tester assert xb_user.opt_attrs[ATTR_KEY_USER_IS_STAFF] == user_is_staff + assert xb_user.opt_attrs[ATTR_KEY_USER_IS_GLOBAL_STAFF] == user_is_global_staff assert xb_user.opt_attrs[ATTR_KEY_USER_ROLE] == user_role assert xb_user.opt_attrs[ATTR_KEY_ANONYMOUS_USER_ID] == anonymous_user_id assert xb_user.opt_attrs[ATTR_KEY_REQUEST_COUNTRY_CODE] == request_country_code @@ -80,14 +88,24 @@ class UserServiceTestCase(TestCase): self.assert_is_anon_xb_user(xb_user, request_country_code=country_code) @ddt.data( - (False, None, None, None), - (True, 'instructor', None, None), - (True, 'staff', None, None), - (False, 'student', 'abcdef0123', None), - (True, 'student', 'abcdef0123', 'uk'), + (False, None, None, None, False, False), + (True, 'instructor', None, None, False, False), + (True, 'staff', None, None, False, False), + (False, 'student', 'abcdef0123', None, False, False), + (True, 'student', 'abcdef0123', 'uk', False, False), + (False, None, None, None, True, False), + (False, None, None, None, False, True), ) @ddt.unpack - def test_convert_authenticate_user(self, user_is_staff, user_role, anonymous_user_id, request_country_code): + def test_convert_authenticate_user( + self, + user_is_staff, + user_role, + anonymous_user_id, + request_country_code, + user_is_global_staff, + user_is_beta_tester, + ): """ Tests for convert_django_user_to_xblock_user behavior when django user is User. """ @@ -97,15 +115,20 @@ class UserServiceTestCase(TestCase): user_role=user_role, anonymous_user_id=anonymous_user_id, request_country_code=request_country_code, + user_is_global_staff=user_is_global_staff, + user_is_beta_tester=user_is_beta_tester, ) xb_user = django_user_service.get_current_user() assert xb_user.is_current_user self.assert_xblock_user_matches_django( - xb_user, self.user, + xb_user, + self.user, user_is_staff, user_role, anonymous_user_id, request_country_code, + user_is_global_staff, + user_is_beta_tester, ) def test_get_anonymous_user_id_returns_none_for_non_staff_users(self): diff --git a/common/djangoapps/xblock_django/user_service.py b/common/djangoapps/xblock_django/user_service.py index 65fa6d9786..01221ddfa6 100644 --- a/common/djangoapps/xblock_django/user_service.py +++ b/common/djangoapps/xblock_django/user_service.py @@ -18,6 +18,8 @@ from .constants import ( ATTR_KEY_REQUEST_COUNTRY_CODE, ATTR_KEY_USER_ID, ATTR_KEY_USERNAME, + ATTR_KEY_USER_IS_BETA_TESTER, + ATTR_KEY_USER_IS_GLOBAL_STAFF, ATTR_KEY_USER_IS_STAFF, ATTR_KEY_USER_PREFERENCES, ATTR_KEY_USER_ROLE, @@ -36,7 +38,10 @@ class DjangoXBlockUserService(UserService): Constructs a DjangoXBlockUserService object. Args: - user_is_staff(bool): optional - whether the user is staff in the course + django_user(User): optional - the user we are binding to the runtime. Is `None` for an anonymous user. + user_is_beta_tester(bool): optional - whether the user is enrolled in the course as a Beta Tester. + user_is_global_staff(bool): optional - whether the user has staff access to the platform. + user_is_staff(bool): optional - whether the user is a course team member with 'Staff' or 'Admin' access. user_role(str): optional -- user's role in the course ('staff', 'instructor', or 'student') anonymous_user_id(str): optional - anonymous_user_id for the user in the course deprecated_anonymous_user_id(str): optional - There are XBlocks (CAPA and HTML) that use the per-student @@ -46,6 +51,8 @@ class DjangoXBlockUserService(UserService): """ super().__init__(**kwargs) self._django_user = django_user + self._user_is_beta_tester = kwargs.get('user_is_beta_tester', False) + self._user_is_global_staff = kwargs.get('user_is_global_staff', False) self._user_is_staff = kwargs.get('user_is_staff', False) self._user_role = kwargs.get('user_role', 'student') self._anonymous_user_id = kwargs.get('anonymous_user_id', None) @@ -121,6 +128,8 @@ class DjangoXBlockUserService(UserService): xblock_user.opt_attrs[ATTR_KEY_REQUEST_COUNTRY_CODE] = self._request_country_code xblock_user.opt_attrs[ATTR_KEY_USER_ID] = django_user.id xblock_user.opt_attrs[ATTR_KEY_USERNAME] = django_user.username + xblock_user.opt_attrs[ATTR_KEY_USER_IS_BETA_TESTER] = self._user_is_beta_tester + xblock_user.opt_attrs[ATTR_KEY_USER_IS_GLOBAL_STAFF] = self._user_is_global_staff xblock_user.opt_attrs[ATTR_KEY_USER_IS_STAFF] = self._user_is_staff xblock_user.opt_attrs[ATTR_KEY_USER_ROLE] = self._user_role user_preferences = get_user_preferences(django_user) diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index e4ad4c23fa..2bab345b83 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -493,32 +493,6 @@ def prepare_runtime_for_user( will_recheck_access=will_recheck_access, ) - user_is_staff = bool(has_access(user, 'staff', block.location, course_id)) - user_service = DjangoXBlockUserService( - user, - user_is_staff=user_is_staff, - user_role=get_user_role(user, course_id), - anonymous_user_id=anonymous_id_for_user(user, course_id), - # See the docstring of `DjangoXBlockUserService`. - deprecated_anonymous_user_id=anonymous_id_for_user(user, None), - request_country_code=user_location, - ) - - # Rebind module service to deal with noauth modules getting attached to users - rebind_user_service = RebindUserService( - user, - course_id, - prepare_runtime_for_user, - track_function=track_function, - position=position, - wrap_xblock_display=wrap_xblock_display, - grade_bucket_type=grade_bucket_type, - static_asset_path=static_asset_path, - user_location=user_location, - request_token=request_token, - will_recheck_access=will_recheck_access, - ) - # Build a list of wrapping functions that will be applied in order # to the Fragment content coming out of the xblocks that are about to be rendered. block_wrappers = [] @@ -555,6 +529,8 @@ def prepare_runtime_for_user( block_wrappers.append(partial(course_expiration_wrapper, user)) block_wrappers.append(partial(offer_banner_wrapper, user)) + user_is_staff = bool(has_access(user, 'staff', course_id)) + if settings.FEATURES.get('DISPLAY_DEBUG_INFO_TO_STAFF'): if is_masquerading_as_specific_student(user, course_id): # When masquerading as a specific student, we want to show the debug button @@ -568,7 +544,7 @@ def prepare_runtime_for_user( del user.real_user.masquerade_settings user.real_user.masquerade_settings = masquerade_settings else: - staff_access = has_access(user, 'staff', block, course_id) + staff_access = user_is_staff if staff_access: block_wrappers.append(partial(add_staff_markup, user, disable_staff_debug_info)) @@ -581,7 +557,17 @@ def prepare_runtime_for_user( 'fs': FSService(), 'field-data': field_data, 'mako': mako_service, - 'user': user_service, + 'user': DjangoXBlockUserService( + user, + user_is_beta_tester=CourseBetaTesterRole(course_id).has_user(user), + user_is_staff=user_is_staff, + user_is_global_staff=bool(has_access(user, 'staff', 'global')), + user_role=get_user_role(user, course_id), + anonymous_user_id=anonymous_id_for_user(user, course_id), + # See the docstring of `DjangoXBlockUserService`. + deprecated_anonymous_user_id=anonymous_id_for_user(user, None), + request_country_code=user_location, + ), 'verification': XBlockVerificationService(), 'proctoring': ProctoringService(), 'milestones': milestones_helpers.get_service(), @@ -595,10 +581,21 @@ def prepare_runtime_for_user( 'sandbox': SandboxService(contentstore=contentstore, course_id=course_id), 'xqueue': xqueue_service, 'replace_urls': replace_url_service, - 'rebind_user': rebind_user_service, - 'completion': CompletionService(user=user, context_key=course_id) - if user and user.is_authenticated - else None, + # Rebind module service to deal with noauth modules getting attached to users. + 'rebind_user': RebindUserService( + user, + course_id, + prepare_runtime_for_user, + track_function=track_function, + position=position, + wrap_xblock_display=wrap_xblock_display, + grade_bucket_type=grade_bucket_type, + static_asset_path=static_asset_path, + user_location=user_location, + request_token=request_token, + will_recheck_access=will_recheck_access, + ), + 'completion': CompletionService(user=user, context_key=course_id) if user and user.is_authenticated else None, 'i18n': XBlockI18nService, 'library_tools': LibraryToolsService(store, user_id=user.id if user else None), 'partitions': PartitionService(course_id=course_id, cache=DEFAULT_REQUEST_CACHE.data), @@ -629,11 +626,6 @@ def prepare_runtime_for_user( block.runtime.set('position', position) - block.runtime.set('user_is_staff', user_is_staff) - block.runtime.set('user_is_admin', bool(has_access(user, 'staff', 'global'))) - block.runtime.set('user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user)) - block.runtime.set('days_early_for_beta', block.days_early_for_beta) - return field_data diff --git a/lms/djangoapps/courseware/tests/test_block_render.py b/lms/djangoapps/courseware/tests/test_block_render.py index 0504772aa5..dc654a3995 100644 --- a/lms/djangoapps/courseware/tests/test_block_render.py +++ b/lms/djangoapps/courseware/tests/test_block_render.py @@ -61,12 +61,19 @@ from xmodule.video_block import VideoBlock # lint-amnesty, pylint: disable=wron from xmodule.x_module import STUDENT_VIEW, DescriptorSystem # lint-amnesty, pylint: disable=wrong-import-order from common.djangoapps import static_replace from common.djangoapps.course_modes.models import CourseMode # lint-amnesty, pylint: disable=reimported -from common.djangoapps.student.tests.factories import GlobalStaffFactory -from common.djangoapps.student.tests.factories import RequestFactoryNoCsrf -from common.djangoapps.student.tests.factories import UserFactory +from common.djangoapps.student.tests.factories import ( + BetaTesterFactory, + GlobalStaffFactory, + InstructorFactory, + RequestFactoryNoCsrf, + StaffFactory, + UserFactory, +) from common.djangoapps.xblock_django.constants import ( ATTR_KEY_ANONYMOUS_USER_ID, ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID, + ATTR_KEY_USER_IS_BETA_TESTER, + ATTR_KEY_USER_IS_GLOBAL_STAFF, ATTR_KEY_USER_IS_STAFF, ATTR_KEY_USER_ROLE, ) @@ -2346,17 +2353,6 @@ class LMSXBlockServiceBindingTest(LMSXBlockServiceMixin): service = self.block.runtime.service(self.block, expected_service) assert service is not None - def test_beta_tester_fields_added(self): - """ - Tests that the beta tester fields are set on LMS runtime. - """ - self.block.days_early_for_beta = 5 - self._prepare_runtime() - - # pylint: disable=no-member - assert not self.block.runtime.user_is_beta_tester - assert self.block.runtime.days_early_for_beta == 5 - def test_get_set_tag(self): """ Tests the user service interface. @@ -2684,8 +2680,12 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): """ assert getattr(self.block.runtime, attribute) == expected_value - @patch('lms.djangoapps.courseware.block_render.has_access', Mock(return_value=True, autospec=True)) - def test_user_is_staff(self): + @ddt.data((True, 'staff'), (False, 'student')) + @ddt.unpack + def test_user_is_staff(self, is_staff, expected_role): + if is_staff: + self.user = StaffFactory(course_key=self.course.id) + _ = render.prepare_runtime_for_user( self.user, self.student_data, @@ -2696,15 +2696,18 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): course=self.course, ) block_user_info = self.block.runtime.service(self.block, "user").get_current_user() - assert block_user_info.opt_attrs.get(ATTR_KEY_USER_IS_STAFF) - assert block_user_info.opt_attrs.get(ATTR_KEY_USER_ROLE) == 'student' + assert block_user_info.opt_attrs.get(ATTR_KEY_USER_IS_STAFF) == is_staff + assert block_user_info.opt_attrs.get(ATTR_KEY_USER_ROLE) == expected_role with warnings.catch_warnings(): # For now, also test the deprecated accessors for backwards compatibility: warnings.simplefilter("ignore", category=DeprecationWarning) - assert self.block.runtime.user_is_staff - assert self.block.runtime.get_user_role() == 'student' + assert self.block.runtime.user_is_staff == is_staff + assert self.block.runtime.get_user_role() == expected_role + + @ddt.data(True, False) + def test_user_is_admin(self, is_global_staff): + if is_global_staff: + self.user = GlobalStaffFactory.create() - @patch('lms.djangoapps.courseware.block_render.get_user_role', Mock(return_value='instructor', autospec=True)) - def test_get_user_role(self): _ = render.prepare_runtime_for_user( self.user, self.student_data, @@ -2715,10 +2718,51 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): course=self.course, ) block_user_info = self.block.runtime.service(self.block, "user").get_current_user() - assert block_user_info.opt_attrs.get(ATTR_KEY_USER_ROLE) == 'instructor' + assert block_user_info.opt_attrs.get(ATTR_KEY_USER_IS_GLOBAL_STAFF) == is_global_staff + with warnings.catch_warnings(): # For now, also test the deprecated accessors for backwards compatibility: + warnings.simplefilter("ignore", category=DeprecationWarning) + assert self.block.runtime.user_is_admin == is_global_staff + + @ddt.data(True, False) + def test_user_is_beta_tester(self, is_beta_tester): + if is_beta_tester: + self.user = BetaTesterFactory(course_key=self.course.id) + + _ = render.prepare_runtime_for_user( + self.user, + self.student_data, + self.block, + self.course.id, + self.track_function, + self.request_token, + course=self.course, + ) + block_user_info = self.block.runtime.service(self.block, "user").get_current_user() + assert block_user_info.opt_attrs.get(ATTR_KEY_USER_IS_BETA_TESTER) == is_beta_tester + with warnings.catch_warnings(): # For now, also test the deprecated accessors for backwards compatibility: + warnings.simplefilter("ignore", category=DeprecationWarning) + assert self.block.runtime.user_is_beta_tester == is_beta_tester + + @ddt.data((True, 'instructor'), (False, 'student')) + @ddt.unpack + def test_get_user_role(self, is_instructor, expected_role): + if is_instructor: + self.user = InstructorFactory(course_key=self.course.id) + + _ = render.prepare_runtime_for_user( + self.user, + self.student_data, + self.block, + self.course.id, + self.track_function, + self.request_token, + course=self.course, + ) + block_user_info = self.block.runtime.service(self.block, "user").get_current_user() + assert block_user_info.opt_attrs.get(ATTR_KEY_USER_ROLE) == expected_role with warnings.catch_warnings(): # For now, also test the deprecated accessor for backwards compatibility: warnings.simplefilter("ignore", category=DeprecationWarning) - assert self.block.runtime.get_user_role() == 'instructor' + assert self.block.runtime.get_user_role() == expected_role def test_anonymous_student_id(self): expected_anon_id = anonymous_id_for_user(self.user, self.course.id) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index fc9d940391..ccb55bc83a 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -793,7 +793,7 @@ openedx-mongodbproxy==0.2.0 # via -r requirements/edx/base.in optimizely-sdk==4.1.1 # via -r requirements/edx/base.in -ora2==5.0.2 +ora2==5.0.3 # via -r requirements/edx/base.in oscrypto==1.3.0 # via snowflake-connector-python diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index b398f7f31a..af87c93117 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1051,7 +1051,7 @@ openedx-mongodbproxy==0.2.0 # via -r requirements/edx/testing.txt optimizely-sdk==4.1.1 # via -r requirements/edx/testing.txt -ora2==5.0.2 +ora2==5.0.3 # via -r requirements/edx/testing.txt oscrypto==1.3.0 # via diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index a6ca03e397..f9d545f876 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -999,7 +999,7 @@ openedx-mongodbproxy==0.2.0 # via -r requirements/edx/base.txt optimizely-sdk==4.1.1 # via -r requirements/edx/base.txt -ora2==5.0.2 +ora2==5.0.3 # via -r requirements/edx/base.txt oscrypto==1.3.0 # via diff --git a/xmodule/x_module.py b/xmodule/x_module.py index f3e7395fa3..c2e57a0598 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -43,6 +43,8 @@ from common.djangoapps.xblock_django.constants import ( ATTR_KEY_ANONYMOUS_USER_ID, ATTR_KEY_REQUEST_COUNTRY_CODE, ATTR_KEY_USER_ID, + ATTR_KEY_USER_IS_BETA_TESTER, + ATTR_KEY_USER_IS_GLOBAL_STAFF, ATTR_KEY_USER_IS_STAFF, ATTR_KEY_USER_ROLE, ) @@ -1190,6 +1192,36 @@ class ModuleSystemShim: if user_service: return partial(user_service.get_current_user().opt_attrs.get, ATTR_KEY_USER_ROLE) + @property + def user_is_beta_tester(self): + """ + Returns whether the current user is enrolled in the course as a beta tester. + + Deprecated in favor of the user service. + """ + warnings.warn( + 'runtime.user_is_beta_tester is deprecated. Please use the user service instead.', + DeprecationWarning, stacklevel=2, + ) + user_service = self._runtime_services.get('user') or self._services.get('user') + if user_service: + return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_BETA_TESTER) + + @property + def user_is_admin(self): + """ + Returns whether the current user has global staff permissions. + + Deprecated in favor of the user service. + """ + warnings.warn( + 'runtime.user_is_admin is deprecated. Please use the user service instead.', + DeprecationWarning, stacklevel=2, + ) + user_service = self._runtime_services.get('user') or self._services.get('user') + if user_service: + return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_GLOBAL_STAFF) + @property def render_template(self): """