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