diff --git a/openedx/core/djangoapps/courseware_api/tests/test_views.py b/openedx/core/djangoapps/courseware_api/tests/test_views.py index a5bf56d01f..56a1213f60 100644 --- a/openedx/core/djangoapps/courseware_api/tests/test_views.py +++ b/openedx/core/djangoapps/courseware_api/tests/test_views.py @@ -4,11 +4,13 @@ Tests for courseware API import unittest from datetime import datetime from urllib.parse import urlencode +from typing import Optional import ddt import mock from completion.test_utils import CompletionWaffleTestMixin, submit_completions_for_testing from django.conf import settings +from django.contrib.auth import get_user_model from django.test.client import RequestFactory from django.urls import reverse # lint-amnesty, pylint: disable=unused-import @@ -29,12 +31,16 @@ from lms.djangoapps.verify_student.services import IDVerificationService from common.djangoapps.student.models import ( CourseEnrollment, CourseEnrollmentCelebration ) +from common.djangoapps.student.roles import CourseInstructorRole from common.djangoapps.student.tests.factories import CourseEnrollmentCelebrationFactory, UserFactory from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import ItemFactory, ToyCourseFactory +User = get_user_model() + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') class BaseCoursewareTests(SharedModuleStoreTestCase): """ @@ -63,6 +69,13 @@ class BaseCoursewareTests(SharedModuleStoreTestCase): password='foo', is_staff=False ) + cls.instructor = UserFactory( + username='instructor', + email=u'instructor@example.com', + password='foo', + is_staff=False + ) + CourseInstructorRole(cls.course.id).add_users(cls.instructor) cls.url = '/api/courseware/course/{}'.format(cls.course.id) @classmethod @@ -79,7 +92,7 @@ class BaseCoursewareTests(SharedModuleStoreTestCase): @override_waffle_flag(REDIRECT_TO_COURSEWARE_MICROFRONTEND, active=True) @override_waffle_flag(COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES, active=True) @override_waffle_flag(COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES_STREAK_CELEBRATION, active=True) -class CourseApiTestViews(BaseCoursewareTests): +class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): """ Tests for the courseware REST API """ @@ -176,6 +189,99 @@ class CourseApiTestViews(BaseCoursewareTests): else: assert not response.data['can_load_courseware']['has_access'] + @ddt.data( + # Who has access to MFE courseware? + { + # Enrolled learners should have access. + "mfe_is_visible": True, + "username": "student", + "enroll_user": True, + "masquerade_role": None, + "expect_can_load_courseware": True, + }, + { + # Un-enrolled learners should NOT have access. + "mfe_is_visible": True, + "username": "student", + "enroll_user": False, + "masquerade_role": None, + "expect_can_load_courseware": False, + }, + { + # Un-enrolled instructors should have access. + "mfe_is_visible": True, + "username": "instructor", + "enroll_user": False, + "masquerade_role": None, + "expect_can_load_courseware": True, + }, + { + # Un-enrolled instructors masquerading as students should have access. + "mfe_is_visible": True, + "username": "instructor", + "enroll_user": False, + "masquerade_role": "student", + "expect_can_load_courseware": True, + }, + { + # If MFE is not visible, enrolled learners shouldn't have access. + "mfe_is_visible": False, + "username": "student", + "enroll_user": True, + "masquerade_role": None, + "expect_can_load_courseware": False, + }, + { + # If MFE is not visible, instructors shouldn't have access. + "mfe_is_visible": False, + "username": "instructor", + "enroll_user": False, + "masquerade_role": None, + "expect_can_load_courseware": False, + }, + { + # If MFE is not visible, masquerading instructors shouldn't have access. + "mfe_is_visible": False, + "username": "instructor", + "enroll_user": False, + "masquerade_role": "student", + "expect_can_load_courseware": False, + }, + + ) + @ddt.unpack + def test_can_load_courseware( + self, + mfe_is_visible: bool, + username: str, + enroll_user: bool, + masquerade_role: Optional[str], + expect_can_load_courseware: bool, + ): + """ + Test that can_load_courseware is calculated correctly based on + access to MFE and access to the course itself. + """ + user = User.objects.get(username=username) + if enroll_user: + CourseEnrollment.enroll(user, self.course.id, 'audit') + + patch_mfe_visible = mock.patch( + 'openedx.core.djangoapps.courseware_api.views.courseware_mfe_is_visible', + return_value=mfe_is_visible, + ) + self.client.login(username=user, password='foo') + if masquerade_role: + self.update_masquerade(role=masquerade_role) + with patch_mfe_visible: + response = self.client.get(self.url) + + assert response.status_code == 200 + if expect_can_load_courseware: + assert response.data['can_load_courseware']['has_access'] + else: + assert not response.data['can_load_courseware']['has_access'] + def test_streak_data_in_response(self): """ Test that metadata endpoint returns data for the streak celebration """ CourseEnrollment.enroll(self.user, self.course.id, 'audit') diff --git a/openedx/core/djangoapps/courseware_api/views.py b/openedx/core/djangoapps/courseware_api/views.py index 767780f24b..bb73f0fc82 100644 --- a/openedx/core/djangoapps/courseware_api/views.py +++ b/openedx/core/djangoapps/courseware_api/views.py @@ -68,6 +68,17 @@ class CoursewareMeta: username or self.request.user.username, course_key, ) + # We must compute course load access *before* setting up masquerading, + # else course staff (who are not enrolled) will not be able view + # their course from the perspective of a learner. + self.load_access = check_course_access( + self.overview, + self.request.user, + 'load', + check_if_enrolled=True, + check_survey_complete=False, + check_if_authenticated=True, + ) self.original_user_is_staff = has_access(self.request.user, 'staff', self.overview).has_access self.original_user_is_global_staff = self.request.user.is_staff self.course_key = course_key @@ -132,21 +143,18 @@ class CoursewareMeta: return course.license @property - def can_load_courseware(self): # lint-amnesty, pylint: disable=missing-function-docstring - access_response = check_course_access( - self.overview, - self.effective_user, - 'load', - check_if_enrolled=True, - check_survey_complete=False, - check_if_authenticated=True, - ).to_json() + def can_load_courseware(self) -> dict: + """ + Can the user load this course in the learning micro-frontend? + + Return a JSON-friendly access response. + """ # Only check whether the MFE is enabled if the user would otherwise be allowed to see it # This means that if the user was denied access, they'll see a meaningful message first if # there is one. - if access_response and not self.is_microfrontend_enabled_for_user(): + if self.load_access and not self.is_microfrontend_enabled_for_user(): return CoursewareMicrofrontendDisabledAccessError().to_json() - return access_response + return self.load_access.to_json() @property def tabs(self):