fix: masquerading as a role in MFE should not require enrollment (#26975)

The Learning MFE had a bug where instructors selecting
'View As: Audit' would find themselves redirected to the
'Enroll Now' page. This was because the Courseware API,
 which powers the MFE, checked courseware access *after*
setting up masquerading--so, instead of checking whether
the instructor had access to view the course, we checked
whether some arbitrary un-enrolled learner would have
access to the course (the answer is generally No).

We fix this by doing what the Legacy courseware index
does: calculate courseware access *before* setting up
masquerading.

TNL-7989

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')

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):