diff --git a/lms/djangoapps/course_api/tests/test_api.py b/lms/djangoapps/course_api/tests/test_api.py index 2c941529f3f4..536c2ad00e4b 100644 --- a/lms/djangoapps/course_api/tests/test_api.py +++ b/lms/djangoapps/course_api/tests/test_api.py @@ -16,9 +16,12 @@ from rest_framework.request import Request from rest_framework.test import APIRequestFactory +from common.djangoapps.student.roles import CourseLimitedStaffRole +from common.djangoapps.student.tests.factories import StaffFactory, UserFactory from lms.djangoapps.courseware.exceptions import CourseAccessRedirect from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from xmodule.course_block import CATALOG_VISIBILITY_ABOUT, CATALOG_VISIBILITY_NONE from xmodule.modulestore.exceptions import ItemNotFoundError # pylint: disable=wrong-import-order from xmodule.modulestore.tests.django_utils import ( # pylint: disable=wrong-import-order ModuleStoreTestCase, @@ -146,58 +149,69 @@ def setUpClass(cls): cls.course = cls.create_course() cls.hidden_course = cls.create_course( - course='hidden', - visible_to_staff_only=True + course="hidden", + visible_to_staff_only=True, + catalog_visibility=CATALOG_VISIBILITY_NONE, + ) + cls.about_only_course = cls.create_course( + course="aboutonly", + catalog_visibility=CATALOG_VISIBILITY_ABOUT, ) def test_get_existing_course_as_authorized_user(self): """User with COURSE_EDITOR role can access course.""" - self.add_user_to_role_in_course( - self.authorized_user, - COURSE_EDITOR.external_key, - self.course.id - ) + self.add_user_to_role_in_course(self.authorized_user, COURSE_EDITOR.external_key, self.course.id) - course = self._make_api_call( - self.authorized_user, - self.authorized_user, - self.course.id - ) + course = self._make_api_call(self.authorized_user, self.authorized_user, self.course.id) + + self.verify_course(course) + + def test_get_existing_course_without_authz_role_when_catalog_visible(self): + """User without AuthZ role can still access when catalog visibility allows.""" + course = self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.course.id) self.verify_course(course) - def test_get_existing_course_as_unauthorized_user(self): - """User without role should be denied.""" + def test_about_only_catalog_visibility_without_authz_role(self): + """User without AuthZ role can access when catalog visibility is about-only.""" + course = self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.about_only_course.id) + + self.verify_course(course, course_id=str(self.about_only_course.id)) + + def test_hidden_course_denied_without_authz_role(self): + """User without AuthZ role is denied when catalog visibility is none.""" with pytest.raises(CourseAccessRedirect): - self._make_api_call( - self.unauthorized_user, - self.unauthorized_user, - self.course.id - ) + self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.hidden_course.id) def test_get_nonexistent_course(self): """Nonexistent course should raise 404.""" - course_key = CourseKey.from_string('edX/toy/nope') + course_key = CourseKey.from_string("edX/toy/nope") with pytest.raises(Http404): - self._make_api_call( - self.authorized_user, - self.authorized_user, - course_key - ) + self._make_api_call(self.authorized_user, self.authorized_user, course_key) + + def test_course_staff_bypasses_authz_on_hidden_course(self): + """Course staff can access a hidden course without an AuthZ role.""" + course_staff = StaffFactory.create(course_key=self.hidden_course.id) + + course = self._make_api_call(course_staff, course_staff, self.hidden_course.id) + + self.verify_course(course, course_id=str(self.hidden_course.id)) + + def test_limited_staff_bypasses_authz_on_hidden_course(self): + """Limited course staff can access a hidden course without an AuthZ role.""" + limited_staff = UserFactory(password=self.password) + CourseLimitedStaffRole(self.hidden_course.id).add_users(limited_staff) + + course = self._make_api_call(limited_staff, limited_staff, self.hidden_course.id) + + self.verify_course(course, course_id=str(self.hidden_course.id)) def test_hidden_course_for_staff(self): """Staff can access hidden course.""" - course = self._make_api_call( - self.staff_user, - self.staff_user, - self.hidden_course.id - ) + course = self._make_api_call(self.staff_user, self.staff_user, self.hidden_course.id) - self.verify_course( - course, - course_id='course-v1:edX+hidden+2012_Fall' - ) + self.verify_course(course, course_id=str(self.hidden_course.id)) def test_hidden_course_for_staff_as_unauthorized_user(self): """ @@ -205,39 +219,22 @@ def test_hidden_course_for_staff_as_unauthorized_user(self): should not bypass visibility rules. """ with pytest.raises(CourseAccessRedirect): - self._make_api_call( - self.staff_user, - self.unauthorized_user, - self.hidden_course.id - ) + self._make_api_call(self.staff_user, self.unauthorized_user, self.hidden_course.id) def test_user_gains_access_after_role_assignment(self): - """User initially denied, then allowed after role assignment.""" + """User denied when catalog is hidden, then allowed after role assignment.""" with pytest.raises(CourseAccessRedirect): - self._make_api_call( - self.unauthorized_user, - self.unauthorized_user, - self.course.id - ) - self.add_user_to_role_in_course( - self.unauthorized_user, - COURSE_EDITOR.external_key, - self.course.id - ) - course = self._make_api_call( - self.unauthorized_user, - self.unauthorized_user, - self.course.id - ) - self.verify_course(course) + self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.hidden_course.id) + + self.add_user_to_role_in_course(self.unauthorized_user, COURSE_EDITOR.external_key, self.hidden_course.id) + + course = self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.hidden_course.id) + + self.verify_course(course, course_id=str(self.hidden_course.id)) def test_staff_access_without_authz_role(self): """Staff bypasses AuthZ roles.""" - course = self._make_api_call( - self.staff_user, - self.staff_user, - self.course.id - ) + course = self._make_api_call(self.staff_user, self.staff_user, self.course.id) self.verify_course(course) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index e3b827529629..112666c91a73 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -45,6 +45,7 @@ from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException from lms.djangoapps.ccx.models import CustomCourseForEdX from lms.djangoapps.courseware.access_response import ( + AccessResponse, CatalogVisibilityError, IncorrectPartitionGroupError, MilestoneAccessError, @@ -64,6 +65,7 @@ from lms.djangoapps.courseware.toggles import course_is_invitation_only from lms.djangoapps.mobile_api.models import IgnoreMobileAvailableFlagConfig from openedx.core import toggles as core_toggles +from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission from openedx.core.djangoapps.authz.decorators import user_has_course_permission from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.features.course_duration_limits.access import check_course_expired @@ -434,30 +436,69 @@ def can_see_in_catalog(): # can provide a meaningful error message instead of a generic 404. return catalog_response - def legacy_can_see_about_page(): + def _about_page_catalog_visibility_access() -> AccessResponse | None: + """ + Shared catalog-visibility gate for about-page access. + + Grants access when catalog_visibility is CATALOG_AND_ABOUT or ABOUT only. + + Returns ACCESS_GRANTED when either applies, or None when catalog visibility + does not allow the about page (callers fall through to staff/AuthZ checks). + """ both_response = _has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT) if both_response: return ACCESS_GRANTED about_response = _has_catalog_visibility(courselike, CATALOG_VISIBILITY_ABOUT) if about_response: return ACCESS_GRANTED - if _has_staff_access_to_block(user, courselike, courselike.id): - return ACCESS_GRANTED - # Return the typed CatalogVisibilityError so downstream handlers - # can provide a meaningful error message instead of a generic 404. - return both_response + return None - @function_trace('can_see_about_page') - def can_see_about_page(): + def _about_page_catalog_visibility_error() -> AccessResponse | CatalogVisibilityError: """ - Implements the "can see course about page" logic if a course about page should be visible - In this case we use the catalog_visibility property on the course block - but also allow course staff to see this. + Return the typed CatalogVisibilityError so downstream handlers + can provide a meaningful error message instead of a generic 404. + """ + return _has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT) + + @function_trace("can_see_about_page") + def can_see_about_page() -> AccessResponse | CatalogVisibilityError: """ - if user and not user.is_anonymous and core_toggles.enable_authz_course_authoring(courselike.id): - is_authz_allowed = user_has_course_permission(user, COURSES_VIEW_COURSE.identifier, courselike.id) - return ACCESS_GRANTED if is_authz_allowed else CatalogVisibilityError() - return legacy_can_see_about_page() + Entry point for about-page visibility checks. + + Grants access when any of the following is true: + - the course catalog_visibility allows the about page, or + - the user has course staff access (including limited staff via role inheritance), or + - the user is authenticated, AuthZ course authoring is enabled for the course, + and the user has COURSES_VIEW_COURSE (including legacy Studio read access + as a fallback during RBAC migration). + + Learners, beta testers, and other course-team roles without staff access rely on + catalog visibility only; they are not checked explicitly here. + + AuthZ must not replace catalog visibility or staff bypass; those checks run + first so enrolled learners and beta testers are not blocked by authoring + permissions they do not hold. + + Returns CatalogVisibilityError when all checks fail. + """ + catalog_visibility_access = _about_page_catalog_visibility_access() + if catalog_visibility_access: + return catalog_visibility_access + + if _has_staff_access_to_block(user, courselike, courselike.id): + return ACCESS_GRANTED + + if ( + user + and not user.is_anonymous + and core_toggles.enable_authz_course_authoring(courselike.id) + and user_has_course_permission( + user, COURSES_VIEW_COURSE.identifier, courselike.id, LegacyAuthoringPermission.READ + ) + ): + return ACCESS_GRANTED + + return _about_page_catalog_visibility_error() checkers = { 'load': can_load, diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index 35cd028621bf..b34c29a7cb45 100644 --- a/lms/djangoapps/courseware/tests/test_about.py +++ b/lms/djangoapps/courseware/tests/test_about.py @@ -9,6 +9,7 @@ import ddt import pytz +from crum import set_current_request from django.conf import settings from django.test.utils import override_settings from django.urls import reverse @@ -19,6 +20,7 @@ from common.djangoapps.student.tests.factories import CourseEnrollmentAllowedFactory, UserFactory from common.djangoapps.track.tests import EventTrackingTestCase from common.djangoapps.util.milestones_helpers import get_prerequisite_courses_display, set_prerequisite_courses +from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin from openedx.core.djangoapps.models.course_details import CourseDetails from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, course_home_url from openedx.features.course_experience.waffle import ENABLE_COURSE_ABOUT_SIDEBAR_HTML @@ -223,6 +225,41 @@ def test_about_page_public_view(self, course_visibility): self.assertContains(resp, "Enroll Now") +class AuthzAboutPageTestCase( + CourseAuthoringAuthzTestMixin, + LoginEnrollmentTestCase, + SharedModuleStoreTestCase, + EventTrackingTestCase, +): + """ + About page HTTP access when AuthZ course authoring is enabled. + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course_without_about = CourseFactory.create(catalog_visibility=CATALOG_VISIBILITY_NONE) + cls.course_with_about = CourseFactory.create(catalog_visibility=CATALOG_VISIBILITY_ABOUT) + CourseDetails.update_about_item(cls.course_without_about, "overview", "WITHOUT ABOUT", None) + CourseDetails.update_about_item(cls.course_with_about, "overview", "WITH ABOUT", None) + + def setUp(self): + super().setUp() + self.addCleanup(set_current_request, None) + assert self.client.login(username=self.unauthorized_user.username, password=self.password) + + @override_settings(COURSE_ABOUT_VISIBILITY_PERMISSION="see_about_page") + def test_about_page_honors_catalog_visibility_without_authz_role(self): + """A learner without AuthZ roles can view catalog-visible about pages.""" + url = reverse("about_course", args=[str(self.course_with_about.id)]) + resp = self.client.get(url) + self.assertContains(resp, "WITH ABOUT") + + url = reverse("about_course", args=[str(self.course_without_about.id)]) + resp = self.client.get(url) + self.assertRedirects(resp, reverse("dashboard"), fetch_redirect_response=False) + + class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): """ Tests for the course about page diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index a54f5f232744..6759441f6eae 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -43,6 +43,7 @@ from lms.djangoapps.courseware.masquerade import CourseMasquerade from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase, masquerade_as_group_member from lms.djangoapps.courseware.toggles import course_is_invitation_only +from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES @@ -1019,3 +1020,71 @@ def test_course_catalog_access_num_queries_enterprise(self, user_attr_name, cour course_overview = CourseOverview.get_from_id(course.id) with self.assertNumQueries(num_queries, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): bool(access.has_access(user, 'see_exists', course_overview, course_key=course.id)) + + +class AuthzSeeAboutPageAccessTestCase(CourseAuthoringAuthzTestMixin, SharedModuleStoreTestCase): + """ + AuthZ-specific see_about_page edge cases not covered elsewhere. + + Catalog visibility grants, staff bypass, AuthZ role grants, and learner + denials are tested in test__catalog_visibility*, TestGetCourseDetailAuthz, + and AuthzAboutPageTestCase. + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course_public = CourseFactory.create( + catalog_visibility=CATALOG_VISIBILITY_CATALOG_AND_ABOUT, + course="authzpublic", + ) + cls.course_about_only = CourseFactory.create( + catalog_visibility=CATALOG_VISIBILITY_ABOUT, + course="authzabout", + ) + cls.course_hidden = CourseFactory.create( + catalog_visibility=CATALOG_VISIBILITY_NONE, + course="authzhidden", + ) + + def _see_about_page_response(self, user, course): + course_overview = CourseOverview.get_from_id(course.id) + return access.has_access(user, "see_about_page", course_overview, course_key=course.id) + + def test_enrolled_learner_denied_when_catalog_hidden(self): + """Enrollment alone does not grant about-page access when catalog is hidden.""" + CourseEnrollmentFactory(user=self.unauthorized_user, course_id=self.course_hidden.id) + + response = self._see_about_page_response(self.unauthorized_user, self.course_hidden) + + assert not response + assert isinstance(response, access_response.CatalogVisibilityError) + + def test_beta_tester_granted_via_catalog_about(self): + """Beta testers rely on catalog visibility, not AuthZ authoring permissions.""" + beta_tester = BetaTesterFactory.create(course_key=self.course_about_only.id) + + response = self._see_about_page_response(beta_tester, self.course_about_only) + + assert response + + def test_anonymous_user_uses_legacy_path(self): + """ + Anonymous users skip the AuthZ path even when course authoring AuthZ is enabled. + + user_has_course_permission is only reached on the AuthZ path, so it must not + be called for anonymous users on a catalog-hidden course. + """ + anonymous_user = AnonymousUserFactory.create() + + with patch( + "lms.djangoapps.courseware.access.user_has_course_permission", + ) as mock_authz_permission: + hidden_response = self._see_about_page_response(anonymous_user, self.course_hidden) + + mock_authz_permission.assert_not_called() + assert not hidden_response + assert isinstance(hidden_response, access_response.CatalogVisibilityError) + + public_response = self._see_about_page_response(anonymous_user, self.course_public) + assert public_response