From 9e8f3d1a52a2dcd9afb5819a5bee88323d9940da Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 9 Jun 2026 14:40:04 -0500 Subject: [PATCH 1/5] fix: preserve catalog and staff checks for authZ about-page access --- lms/djangoapps/course_api/tests/test_api.py | 28 +++-- lms/djangoapps/courseware/access.py | 91 ++++++++++++-- lms/djangoapps/courseware/tests/test_about.py | 39 ++++++ .../courseware/tests/test_access.py | 111 +++++++++++++++++- 4 files changed, 244 insertions(+), 25 deletions(-) diff --git a/lms/djangoapps/course_api/tests/test_api.py b/lms/djangoapps/course_api/tests/test_api.py index 2c941529f3f4..3d094926ddbe 100644 --- a/lms/djangoapps/course_api/tests/test_api.py +++ b/lms/djangoapps/course_api/tests/test_api.py @@ -19,6 +19,7 @@ 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_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, @@ -147,7 +148,8 @@ def setUpClass(cls): cls.course = cls.create_course() cls.hidden_course = cls.create_course( course='hidden', - visible_to_staff_only=True + visible_to_staff_only=True, + catalog_visibility=CATALOG_VISIBILITY_NONE, ) def test_get_existing_course_as_authorized_user(self): @@ -167,13 +169,13 @@ def test_get_existing_course_as_authorized_user(self): self.verify_course(course) def test_get_existing_course_as_unauthorized_user(self): - """User without role should be denied.""" - with pytest.raises(CourseAccessRedirect): - self._make_api_call( - self.unauthorized_user, - self.unauthorized_user, - self.course.id - ) + """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_nonexistent_course(self): """Nonexistent course should raise 404.""" @@ -212,24 +214,24 @@ def test_hidden_course_for_staff_as_unauthorized_user(self): ) 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.hidden_course.id ) self.add_user_to_role_in_course( self.unauthorized_user, COURSE_EDITOR.external_key, - self.course.id + self.hidden_course.id ) course = self._make_api_call( self.unauthorized_user, self.unauthorized_user, - self.course.id + self.hidden_course.id ) - self.verify_course(course) + self.verify_course(course, course_id='course-v1:edX+hidden+2012_Fall') def test_staff_access_without_authz_role(self): """Staff bypasses AuthZ roles.""" diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index e3b827529629..f1bf4482fa58 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,29 +436,96 @@ 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 + return None + + def _about_page_catalog_visibility_error() -> AccessResponse | CatalogVisibilityError: + """ + 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) + + def legacy_can_see_about_page() -> AccessResponse | CatalogVisibilityError: + """ + Legacy about-page access when AuthZ course authoring is disabled. + + 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). + + Learners, beta testers, and other course-team roles without staff access rely on + catalog visibility only; they are not checked explicitly here. + + 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 - # Return the typed CatalogVisibilityError so downstream handlers - # can provide a meaningful error message instead of a generic 404. - return both_response - @function_trace('can_see_about_page') - def can_see_about_page(): + return _about_page_catalog_visibility_error() + + def authz_can_see_about_page() -> 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. + About-page access when AuthZ course authoring is enabled for the course. + + Applies the same course-level and staff checks as legacy_can_see_about_page, + and additionally grants access to users with COURSES_VIEW_COURSE (including + legacy Studio read access as a fallback during RBAC migration). + + 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_has_course_permission( + user, + COURSES_VIEW_COURSE.identifier, + courselike.id, + LegacyAuthoringPermission.READ, + ): + return ACCESS_GRANTED + + return _about_page_catalog_visibility_error() + + @function_trace("can_see_about_page") + def can_see_about_page() -> AccessResponse | CatalogVisibilityError: + """ + Entry point for about-page visibility checks. + + Routes authenticated users on courses with AuthZ course authoring enabled to + authz_can_see_about_page; all other callers use legacy_can_see_about_page. + + Both paths grant access via catalog_visibility and course staff bypass. The AuthZ + path additionally allows users with COURSES_VIEW_COURSE. """ 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 authz_can_see_about_page() return legacy_can_see_about_page() checkers = { diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index 35cd028621bf..94b727245e2b 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,43 @@ 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) + self.assertTrue( # noqa: PT009 + 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..e711d9701397 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -22,11 +22,12 @@ from enterprise.api.v1.serializers import EnterpriseCustomerSerializer from milestones.tests.utils import MilestonesTestCaseMixin from opaque_keys.edx.locator import CourseLocator +from openedx_authz.constants.roles import COURSE_EDITOR import lms.djangoapps.courseware.access as access import lms.djangoapps.courseware.access_response as access_response from common.djangoapps.student.models import CourseEnrollment -from common.djangoapps.student.roles import CourseCcxCoachRole, CourseStaffRole +from common.djangoapps.student.roles import CourseCcxCoachRole, CourseLimitedStaffRole, CourseStaffRole from common.djangoapps.student.tests.factories import ( AdminFactory, AnonymousUserFactory, @@ -43,6 +44,8 @@ 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 import toggles as core_toggles +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 +1022,109 @@ 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): + """ + see_about_page access when AuthZ course authoring is enabled for the course. + """ + + @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_learner_granted_via_catalog_visibility_both(self): + """Learners without AuthZ roles can view the about page when catalog allows it.""" + response = self._see_about_page_response(self.unauthorized_user, self.course_public) + self.assertTrue(response) # noqa: PT009 + + def test_learner_granted_via_catalog_visibility_about_only(self): + """Learners without AuthZ roles can view about-only courses.""" + response = self._see_about_page_response(self.unauthorized_user, self.course_about_only) + self.assertTrue(response) # noqa: PT009 + + 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) + + self.assertFalse(response) # noqa: PT009 + self.assertIsInstance(response, access_response.CatalogVisibilityError) # noqa: PT009 + + 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) + + self.assertTrue(response) # noqa: PT009 + + def test_course_staff_bypass_when_catalog_hidden(self): + """Course staff can preview the about page when catalog visibility is none.""" + course_staff = StaffFactory.create(course_key=self.course_hidden.id) + + response = self._see_about_page_response(course_staff, self.course_hidden) + + self.assertTrue(response) # noqa: PT009 + + def test_limited_staff_bypass_when_catalog_hidden(self): + """Limited staff inherit staff bypass for about-page access.""" + limited_staff = UserFactory.create() + CourseLimitedStaffRole(self.course_hidden.id).add_users(limited_staff) + + response = self._see_about_page_response(limited_staff, self.course_hidden) + + self.assertTrue(response) # noqa: PT009 + + def test_authz_role_grants_access_when_catalog_hidden(self): + """Users with COURSES_VIEW_COURSE can access hidden about pages.""" + self.add_user_to_role_in_course(self.unauthorized_user, COURSE_EDITOR.external_key, self.course_hidden.id) + + response = self._see_about_page_response(self.unauthorized_user, self.course_hidden) + + self.assertTrue(response) # noqa: PT009 + + def test_anonymous_user_uses_legacy_path(self): + """Anonymous users are routed to the legacy path and follow catalog visibility.""" + anonymous_user = AnonymousUserFactory.create() + + response = self._see_about_page_response(anonymous_user, self.course_public) + + self.assertTrue(response) # noqa: PT009 + + def test_denied_returns_catalog_visibility_error(self): + """AuthZ path returns CatalogVisibilityError when all checks fail.""" + response = self._see_about_page_response(self.unauthorized_user, self.course_hidden) + + self.assertFalse(response) # noqa: PT009 + self.assertIsInstance(response, access_response.CatalogVisibilityError) # noqa: PT009 + self.assertEqual(response.error_code, "not_visible_in_catalog") # noqa: PT009 + + def test_legacy_path_when_authz_disabled(self): + """When AuthZ is off, catalog visibility rules still apply.""" + with patch.object(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, "is_enabled", return_value=False): + response = self._see_about_page_response(self.unauthorized_user, self.course_public) + + self.assertTrue(response) # noqa: PT009 + + hidden_response = self._see_about_page_response(self.unauthorized_user, self.course_hidden) + + self.assertFalse(hidden_response) # noqa: PT009 + self.assertIsInstance(hidden_response, access_response.CatalogVisibilityError) # noqa: PT009 From 98c59e0ff03a0cf1d9858d1ae2a62e71221ee1ef Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Thu, 18 Jun 2026 11:43:55 -0500 Subject: [PATCH 2/5] refactor: use native assert instead unittest-style --- lms/djangoapps/courseware/tests/test_about.py | 4 +-- .../courseware/tests/test_access.py | 30 +++++++++---------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index 94b727245e2b..b34c29a7cb45 100644 --- a/lms/djangoapps/courseware/tests/test_about.py +++ b/lms/djangoapps/courseware/tests/test_about.py @@ -246,9 +246,7 @@ def setUpClass(cls): def setUp(self): super().setUp() self.addCleanup(set_current_request, None) - self.assertTrue( # noqa: PT009 - self.client.login(username=self.unauthorized_user.username, password=self.password) - ) + 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): diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index e711d9701397..746c66d81ed1 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -1052,12 +1052,12 @@ def _see_about_page_response(self, user, course): def test_learner_granted_via_catalog_visibility_both(self): """Learners without AuthZ roles can view the about page when catalog allows it.""" response = self._see_about_page_response(self.unauthorized_user, self.course_public) - self.assertTrue(response) # noqa: PT009 + assert response def test_learner_granted_via_catalog_visibility_about_only(self): """Learners without AuthZ roles can view about-only courses.""" response = self._see_about_page_response(self.unauthorized_user, self.course_about_only) - self.assertTrue(response) # noqa: PT009 + assert response def test_enrolled_learner_denied_when_catalog_hidden(self): """Enrollment alone does not grant about-page access when catalog is hidden.""" @@ -1065,8 +1065,8 @@ def test_enrolled_learner_denied_when_catalog_hidden(self): response = self._see_about_page_response(self.unauthorized_user, self.course_hidden) - self.assertFalse(response) # noqa: PT009 - self.assertIsInstance(response, access_response.CatalogVisibilityError) # noqa: PT009 + 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.""" @@ -1074,7 +1074,7 @@ def test_beta_tester_granted_via_catalog_about(self): response = self._see_about_page_response(beta_tester, self.course_about_only) - self.assertTrue(response) # noqa: PT009 + assert response def test_course_staff_bypass_when_catalog_hidden(self): """Course staff can preview the about page when catalog visibility is none.""" @@ -1082,7 +1082,7 @@ def test_course_staff_bypass_when_catalog_hidden(self): response = self._see_about_page_response(course_staff, self.course_hidden) - self.assertTrue(response) # noqa: PT009 + assert response def test_limited_staff_bypass_when_catalog_hidden(self): """Limited staff inherit staff bypass for about-page access.""" @@ -1091,7 +1091,7 @@ def test_limited_staff_bypass_when_catalog_hidden(self): response = self._see_about_page_response(limited_staff, self.course_hidden) - self.assertTrue(response) # noqa: PT009 + assert response def test_authz_role_grants_access_when_catalog_hidden(self): """Users with COURSES_VIEW_COURSE can access hidden about pages.""" @@ -1099,7 +1099,7 @@ def test_authz_role_grants_access_when_catalog_hidden(self): response = self._see_about_page_response(self.unauthorized_user, self.course_hidden) - self.assertTrue(response) # noqa: PT009 + assert response def test_anonymous_user_uses_legacy_path(self): """Anonymous users are routed to the legacy path and follow catalog visibility.""" @@ -1107,24 +1107,24 @@ def test_anonymous_user_uses_legacy_path(self): response = self._see_about_page_response(anonymous_user, self.course_public) - self.assertTrue(response) # noqa: PT009 + assert response def test_denied_returns_catalog_visibility_error(self): """AuthZ path returns CatalogVisibilityError when all checks fail.""" response = self._see_about_page_response(self.unauthorized_user, self.course_hidden) - self.assertFalse(response) # noqa: PT009 - self.assertIsInstance(response, access_response.CatalogVisibilityError) # noqa: PT009 - self.assertEqual(response.error_code, "not_visible_in_catalog") # noqa: PT009 + assert not response + assert isinstance(response, access_response.CatalogVisibilityError) + assert response.error_code == "not_visible_in_catalog" def test_legacy_path_when_authz_disabled(self): """When AuthZ is off, catalog visibility rules still apply.""" with patch.object(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, "is_enabled", return_value=False): response = self._see_about_page_response(self.unauthorized_user, self.course_public) - self.assertTrue(response) # noqa: PT009 + assert response hidden_response = self._see_about_page_response(self.unauthorized_user, self.course_hidden) - self.assertFalse(hidden_response) # noqa: PT009 - self.assertIsInstance(hidden_response, access_response.CatalogVisibilityError) # noqa: PT009 + assert not hidden_response + assert isinstance(hidden_response, access_response.CatalogVisibilityError) From 28b71be57d64410cbb6b8b043b18e31f0610c979 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Thu, 18 Jun 2026 12:12:22 -0500 Subject: [PATCH 3/5] test: enhance course access tests for various catalog visibility scenarios --- lms/djangoapps/course_api/tests/test_api.py | 113 ++++++++++---------- 1 file changed, 54 insertions(+), 59 deletions(-) diff --git a/lms/djangoapps/course_api/tests/test_api.py b/lms/djangoapps/course_api/tests/test_api.py index 3d094926ddbe..536c2ad00e4b 100644 --- a/lms/djangoapps/course_api/tests/test_api.py +++ b/lms/djangoapps/course_api/tests/test_api.py @@ -16,10 +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_NONE +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, @@ -147,59 +149,69 @@ def setUpClass(cls): cls.course = cls.create_course() cls.hidden_course = cls.create_course( - course='hidden', + 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_as_unauthorized_user(self): + 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 - ) + course = self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.course.id) + self.verify_course(course) + 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.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): """ @@ -207,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 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.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='course-v1:edX+hidden+2012_Fall') + 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) From b7d8b9aeb4579981092d9a2a3ec5a78eef298218 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Thu, 18 Jun 2026 13:20:29 -0500 Subject: [PATCH 4/5] refactor: streamline authz see_about_page access tests by removing redundant cases --- .../courseware/tests/test_access.py | 82 +++++-------------- 1 file changed, 21 insertions(+), 61 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index 746c66d81ed1..6759441f6eae 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -22,12 +22,11 @@ from enterprise.api.v1.serializers import EnterpriseCustomerSerializer from milestones.tests.utils import MilestonesTestCaseMixin from opaque_keys.edx.locator import CourseLocator -from openedx_authz.constants.roles import COURSE_EDITOR import lms.djangoapps.courseware.access as access import lms.djangoapps.courseware.access_response as access_response from common.djangoapps.student.models import CourseEnrollment -from common.djangoapps.student.roles import CourseCcxCoachRole, CourseLimitedStaffRole, CourseStaffRole +from common.djangoapps.student.roles import CourseCcxCoachRole, CourseStaffRole from common.djangoapps.student.tests.factories import ( AdminFactory, AnonymousUserFactory, @@ -44,7 +43,6 @@ 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 import toggles as core_toggles 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 @@ -1026,7 +1024,11 @@ def test_course_catalog_access_num_queries_enterprise(self, user_attr_name, cour class AuthzSeeAboutPageAccessTestCase(CourseAuthoringAuthzTestMixin, SharedModuleStoreTestCase): """ - see_about_page access when AuthZ course authoring is enabled for the course. + 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 @@ -1049,16 +1051,6 @@ 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_learner_granted_via_catalog_visibility_both(self): - """Learners without AuthZ roles can view the about page when catalog allows it.""" - response = self._see_about_page_response(self.unauthorized_user, self.course_public) - assert response - - def test_learner_granted_via_catalog_visibility_about_only(self): - """Learners without AuthZ roles can view about-only courses.""" - response = self._see_about_page_response(self.unauthorized_user, self.course_about_only) - assert response - 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) @@ -1076,55 +1068,23 @@ def test_beta_tester_granted_via_catalog_about(self): assert response - def test_course_staff_bypass_when_catalog_hidden(self): - """Course staff can preview the about page when catalog visibility is none.""" - course_staff = StaffFactory.create(course_key=self.course_hidden.id) - - response = self._see_about_page_response(course_staff, self.course_hidden) - - assert response - - def test_limited_staff_bypass_when_catalog_hidden(self): - """Limited staff inherit staff bypass for about-page access.""" - limited_staff = UserFactory.create() - CourseLimitedStaffRole(self.course_hidden.id).add_users(limited_staff) - - response = self._see_about_page_response(limited_staff, self.course_hidden) - - assert response - - def test_authz_role_grants_access_when_catalog_hidden(self): - """Users with COURSES_VIEW_COURSE can access hidden about pages.""" - self.add_user_to_role_in_course(self.unauthorized_user, COURSE_EDITOR.external_key, self.course_hidden.id) - - response = self._see_about_page_response(self.unauthorized_user, self.course_hidden) - - assert response - def test_anonymous_user_uses_legacy_path(self): - """Anonymous users are routed to the legacy path and follow catalog visibility.""" - anonymous_user = AnonymousUserFactory.create() - - response = self._see_about_page_response(anonymous_user, self.course_public) - - assert response - - def test_denied_returns_catalog_visibility_error(self): - """AuthZ path returns CatalogVisibilityError when all checks fail.""" - response = self._see_about_page_response(self.unauthorized_user, self.course_hidden) - - assert not response - assert isinstance(response, access_response.CatalogVisibilityError) - assert response.error_code == "not_visible_in_catalog" + """ + Anonymous users skip the AuthZ path even when course authoring AuthZ is enabled. - def test_legacy_path_when_authz_disabled(self): - """When AuthZ is off, catalog visibility rules still apply.""" - with patch.object(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, "is_enabled", return_value=False): - response = self._see_about_page_response(self.unauthorized_user, self.course_public) + 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() - assert response + 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) - hidden_response = self._see_about_page_response(self.unauthorized_user, self.course_hidden) + mock_authz_permission.assert_not_called() + assert not hidden_response + assert isinstance(hidden_response, access_response.CatalogVisibilityError) - 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 From 7663bb67a8d1044ecf6df52d19a180107f3bc5d1 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Thu, 18 Jun 2026 14:09:14 -0500 Subject: [PATCH 5/5] refactor: consolidate about-page access logic and enhance clarity of access checks --- lms/djangoapps/courseware/access.py | 56 ++++++++--------------------- 1 file changed, 14 insertions(+), 42 deletions(-) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index f1bf4482fa58..112666c91a73 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -460,36 +460,21 @@ def _about_page_catalog_visibility_error() -> AccessResponse | CatalogVisibility """ return _has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT) - def legacy_can_see_about_page() -> AccessResponse | CatalogVisibilityError: + @function_trace("can_see_about_page") + def can_see_about_page() -> AccessResponse | CatalogVisibilityError: """ - Legacy about-page access when AuthZ course authoring is disabled. + 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). + - 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. - 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 - - return _about_page_catalog_visibility_error() - - def authz_can_see_about_page() -> AccessResponse | CatalogVisibilityError: - """ - About-page access when AuthZ course authoring is enabled for the course. - - Applies the same course-level and staff checks as legacy_can_see_about_page, - and additionally grants access to users with COURSES_VIEW_COURSE (including - legacy Studio read access as a fallback during RBAC migration). - 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. @@ -503,31 +488,18 @@ def authz_can_see_about_page() -> AccessResponse | CatalogVisibilityError: if _has_staff_access_to_block(user, courselike, courselike.id): return ACCESS_GRANTED - if user_has_course_permission( - user, - COURSES_VIEW_COURSE.identifier, - courselike.id, - LegacyAuthoringPermission.READ, + 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() - @function_trace("can_see_about_page") - def can_see_about_page() -> AccessResponse | CatalogVisibilityError: - """ - Entry point for about-page visibility checks. - - Routes authenticated users on courses with AuthZ course authoring enabled to - authz_can_see_about_page; all other callers use legacy_can_see_about_page. - - Both paths grant access via catalog_visibility and course staff bypass. The AuthZ - path additionally allows users with COURSES_VIEW_COURSE. - """ - if user and not user.is_anonymous and core_toggles.enable_authz_course_authoring(courselike.id): - return authz_can_see_about_page() - return legacy_can_see_about_page() - checkers = { 'load': can_load, 'load_mobile': lambda: can_load() and _can_load_course_on_mobile(user, courselike),