Skip to content

Commit 32ef4b1

Browse files
committed
fix: preserve catalog and staff checks for authZ about-page access
1 parent 3d0f9c0 commit 32ef4b1

4 files changed

Lines changed: 244 additions & 25 deletions

File tree

lms/djangoapps/course_api/tests/test_api.py

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
2020
from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin
2121
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
22+
from xmodule.course_block import CATALOG_VISIBILITY_NONE
2223
from xmodule.modulestore.exceptions import ItemNotFoundError # pylint: disable=wrong-import-order
2324
from xmodule.modulestore.tests.django_utils import ( # pylint: disable=wrong-import-order
2425
ModuleStoreTestCase,
@@ -147,7 +148,8 @@ def setUpClass(cls):
147148
cls.course = cls.create_course()
148149
cls.hidden_course = cls.create_course(
149150
course='hidden',
150-
visible_to_staff_only=True
151+
visible_to_staff_only=True,
152+
catalog_visibility=CATALOG_VISIBILITY_NONE,
151153
)
152154

153155
def test_get_existing_course_as_authorized_user(self):
@@ -167,13 +169,13 @@ def test_get_existing_course_as_authorized_user(self):
167169
self.verify_course(course)
168170

169171
def test_get_existing_course_as_unauthorized_user(self):
170-
"""User without role should be denied."""
171-
with pytest.raises(CourseAccessRedirect):
172-
self._make_api_call(
173-
self.unauthorized_user,
174-
self.unauthorized_user,
175-
self.course.id
176-
)
172+
"""User without AuthZ role can still access when catalog visibility allows."""
173+
course = self._make_api_call(
174+
self.unauthorized_user,
175+
self.unauthorized_user,
176+
self.course.id
177+
)
178+
self.verify_course(course)
177179

178180
def test_get_nonexistent_course(self):
179181
"""Nonexistent course should raise 404."""
@@ -212,24 +214,24 @@ def test_hidden_course_for_staff_as_unauthorized_user(self):
212214
)
213215

214216
def test_user_gains_access_after_role_assignment(self):
215-
"""User initially denied, then allowed after role assignment."""
217+
"""User denied when catalog is hidden, then allowed after role assignment."""
216218
with pytest.raises(CourseAccessRedirect):
217219
self._make_api_call(
218220
self.unauthorized_user,
219221
self.unauthorized_user,
220-
self.course.id
222+
self.hidden_course.id
221223
)
222224
self.add_user_to_role_in_course(
223225
self.unauthorized_user,
224226
COURSE_EDITOR.external_key,
225-
self.course.id
227+
self.hidden_course.id
226228
)
227229
course = self._make_api_call(
228230
self.unauthorized_user,
229231
self.unauthorized_user,
230-
self.course.id
232+
self.hidden_course.id
231233
)
232-
self.verify_course(course)
234+
self.verify_course(course, course_id='course-v1:edX+hidden+2012_Fall')
233235

234236
def test_staff_access_without_authz_role(self):
235237
"""Staff bypasses AuthZ roles."""

lms/djangoapps/courseware/access.py

Lines changed: 80 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException
4646
from lms.djangoapps.ccx.models import CustomCourseForEdX
4747
from lms.djangoapps.courseware.access_response import (
48+
AccessResponse,
4849
CatalogVisibilityError,
4950
IncorrectPartitionGroupError,
5051
MilestoneAccessError,
@@ -64,6 +65,7 @@
6465
from lms.djangoapps.courseware.toggles import course_is_invitation_only
6566
from lms.djangoapps.mobile_api.models import IgnoreMobileAvailableFlagConfig
6667
from openedx.core import toggles as core_toggles
68+
from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
6769
from openedx.core.djangoapps.authz.decorators import user_has_course_permission
6870
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
6971
from openedx.features.course_duration_limits.access import check_course_expired
@@ -434,29 +436,96 @@ def can_see_in_catalog():
434436
# can provide a meaningful error message instead of a generic 404.
435437
return catalog_response
436438

437-
def legacy_can_see_about_page():
439+
def _about_page_catalog_visibility_access() -> AccessResponse | None:
440+
"""
441+
Shared catalog-visibility gate for about-page access.
442+
443+
Grants access when catalog_visibility is CATALOG_AND_ABOUT or ABOUT only.
444+
445+
Returns ACCESS_GRANTED when either applies, or None when catalog visibility
446+
does not allow the about page (callers fall through to staff/AuthZ checks).
447+
"""
438448
both_response = _has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT)
439449
if both_response:
440450
return ACCESS_GRANTED
441451
about_response = _has_catalog_visibility(courselike, CATALOG_VISIBILITY_ABOUT)
442452
if about_response:
443453
return ACCESS_GRANTED
454+
return None
455+
456+
def _about_page_catalog_visibility_error() -> AccessResponse | CatalogVisibilityError:
457+
"""
458+
Return the typed CatalogVisibilityError so downstream handlers
459+
can provide a meaningful error message instead of a generic 404.
460+
"""
461+
return _has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT)
462+
463+
def legacy_can_see_about_page() -> AccessResponse | CatalogVisibilityError:
464+
"""
465+
Legacy about-page access when AuthZ course authoring is disabled.
466+
467+
Grants access when any of the following is true:
468+
- the course catalog_visibility allows the about page, or
469+
- the user has course staff access (including limited staff via role inheritance).
470+
471+
Learners, beta testers, and other course-team roles without staff access rely on
472+
catalog visibility only; they are not checked explicitly here.
473+
474+
Returns CatalogVisibilityError when all checks fail.
475+
"""
476+
catalog_visibility_access = _about_page_catalog_visibility_access()
477+
if catalog_visibility_access:
478+
return catalog_visibility_access
479+
444480
if _has_staff_access_to_block(user, courselike, courselike.id):
445481
return ACCESS_GRANTED
446-
# Return the typed CatalogVisibilityError so downstream handlers
447-
# can provide a meaningful error message instead of a generic 404.
448-
return both_response
449482

450-
@function_trace('can_see_about_page')
451-
def can_see_about_page():
483+
return _about_page_catalog_visibility_error()
484+
485+
def authz_can_see_about_page() -> AccessResponse | CatalogVisibilityError:
452486
"""
453-
Implements the "can see course about page" logic if a course about page should be visible
454-
In this case we use the catalog_visibility property on the course block
455-
but also allow course staff to see this.
487+
About-page access when AuthZ course authoring is enabled for the course.
488+
489+
Applies the same course-level and staff checks as legacy_can_see_about_page,
490+
and additionally grants access to users with COURSES_VIEW_COURSE (including
491+
legacy Studio read access as a fallback during RBAC migration).
492+
493+
AuthZ must not replace catalog visibility or staff bypass; those checks run
494+
first so enrolled learners and beta testers are not blocked by authoring
495+
permissions they do not hold.
496+
497+
Returns CatalogVisibilityError when all checks fail.
498+
"""
499+
catalog_visibility_access = _about_page_catalog_visibility_access()
500+
if catalog_visibility_access:
501+
return catalog_visibility_access
502+
503+
if _has_staff_access_to_block(user, courselike, courselike.id):
504+
return ACCESS_GRANTED
505+
506+
if user_has_course_permission(
507+
user,
508+
COURSES_VIEW_COURSE.identifier,
509+
courselike.id,
510+
LegacyAuthoringPermission.READ,
511+
):
512+
return ACCESS_GRANTED
513+
514+
return _about_page_catalog_visibility_error()
515+
516+
@function_trace("can_see_about_page")
517+
def can_see_about_page() -> AccessResponse | CatalogVisibilityError:
518+
"""
519+
Entry point for about-page visibility checks.
520+
521+
Routes authenticated users on courses with AuthZ course authoring enabled to
522+
authz_can_see_about_page; all other callers use legacy_can_see_about_page.
523+
524+
Both paths grant access via catalog_visibility and course staff bypass. The AuthZ
525+
path additionally allows users with COURSES_VIEW_COURSE.
456526
"""
457527
if user and not user.is_anonymous and core_toggles.enable_authz_course_authoring(courselike.id):
458-
is_authz_allowed = user_has_course_permission(user, COURSES_VIEW_COURSE.identifier, courselike.id)
459-
return ACCESS_GRANTED if is_authz_allowed else CatalogVisibilityError()
528+
return authz_can_see_about_page()
460529
return legacy_can_see_about_page()
461530

462531
checkers = {

lms/djangoapps/courseware/tests/test_about.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import ddt
1111
import pytz
12+
from crum import set_current_request
1213
from django.conf import settings
1314
from django.test.utils import override_settings
1415
from django.urls import reverse
@@ -19,6 +20,7 @@
1920
from common.djangoapps.student.tests.factories import CourseEnrollmentAllowedFactory, UserFactory
2021
from common.djangoapps.track.tests import EventTrackingTestCase
2122
from common.djangoapps.util.milestones_helpers import get_prerequisite_courses_display, set_prerequisite_courses
23+
from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin
2224
from openedx.core.djangoapps.models.course_details import CourseDetails
2325
from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, course_home_url
2426
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):
223225
self.assertContains(resp, "Enroll Now")
224226

225227

228+
class AuthzAboutPageTestCase(
229+
CourseAuthoringAuthzTestMixin,
230+
LoginEnrollmentTestCase,
231+
SharedModuleStoreTestCase,
232+
EventTrackingTestCase,
233+
):
234+
"""
235+
About page HTTP access when AuthZ course authoring is enabled.
236+
"""
237+
238+
@classmethod
239+
def setUpClass(cls):
240+
super().setUpClass()
241+
cls.course_without_about = CourseFactory.create(catalog_visibility=CATALOG_VISIBILITY_NONE)
242+
cls.course_with_about = CourseFactory.create(catalog_visibility=CATALOG_VISIBILITY_ABOUT)
243+
CourseDetails.update_about_item(cls.course_without_about, "overview", "WITHOUT ABOUT", None)
244+
CourseDetails.update_about_item(cls.course_with_about, "overview", "WITH ABOUT", None)
245+
246+
def setUp(self):
247+
super().setUp()
248+
self.addCleanup(set_current_request, None)
249+
self.assertTrue( # noqa: PT009
250+
self.client.login(username=self.unauthorized_user.username, password=self.password)
251+
)
252+
253+
@override_settings(COURSE_ABOUT_VISIBILITY_PERMISSION="see_about_page")
254+
def test_about_page_honors_catalog_visibility_without_authz_role(self):
255+
"""A learner without AuthZ roles can view catalog-visible about pages."""
256+
url = reverse("about_course", args=[str(self.course_with_about.id)])
257+
resp = self.client.get(url)
258+
self.assertContains(resp, "WITH ABOUT")
259+
260+
url = reverse("about_course", args=[str(self.course_without_about.id)])
261+
resp = self.client.get(url)
262+
self.assertRedirects(resp, reverse("dashboard"), fetch_redirect_response=False)
263+
264+
226265
class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase):
227266
"""
228267
Tests for the course about page

lms/djangoapps/courseware/tests/test_access.py

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,12 @@
2222
from enterprise.api.v1.serializers import EnterpriseCustomerSerializer
2323
from milestones.tests.utils import MilestonesTestCaseMixin
2424
from opaque_keys.edx.locator import CourseLocator
25+
from openedx_authz.constants.roles import COURSE_EDITOR
2526

2627
import lms.djangoapps.courseware.access as access
2728
import lms.djangoapps.courseware.access_response as access_response
2829
from common.djangoapps.student.models import CourseEnrollment
29-
from common.djangoapps.student.roles import CourseCcxCoachRole, CourseStaffRole
30+
from common.djangoapps.student.roles import CourseCcxCoachRole, CourseLimitedStaffRole, CourseStaffRole
3031
from common.djangoapps.student.tests.factories import (
3132
AdminFactory,
3233
AnonymousUserFactory,
@@ -43,6 +44,8 @@
4344
from lms.djangoapps.courseware.masquerade import CourseMasquerade
4445
from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase, masquerade_as_group_member
4546
from lms.djangoapps.courseware.toggles import course_is_invitation_only
47+
from openedx.core import toggles as core_toggles
48+
from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin
4649
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
4750
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
4851
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
10191022
course_overview = CourseOverview.get_from_id(course.id)
10201023
with self.assertNumQueries(num_queries, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST):
10211024
bool(access.has_access(user, 'see_exists', course_overview, course_key=course.id))
1025+
1026+
1027+
class AuthzSeeAboutPageAccessTestCase(CourseAuthoringAuthzTestMixin, SharedModuleStoreTestCase):
1028+
"""
1029+
see_about_page access when AuthZ course authoring is enabled for the course.
1030+
"""
1031+
1032+
@classmethod
1033+
def setUpClass(cls):
1034+
super().setUpClass()
1035+
cls.course_public = CourseFactory.create(
1036+
catalog_visibility=CATALOG_VISIBILITY_CATALOG_AND_ABOUT,
1037+
course="authzpublic",
1038+
)
1039+
cls.course_about_only = CourseFactory.create(
1040+
catalog_visibility=CATALOG_VISIBILITY_ABOUT,
1041+
course="authzabout",
1042+
)
1043+
cls.course_hidden = CourseFactory.create(
1044+
catalog_visibility=CATALOG_VISIBILITY_NONE,
1045+
course="authzhidden",
1046+
)
1047+
1048+
def _see_about_page_response(self, user, course):
1049+
course_overview = CourseOverview.get_from_id(course.id)
1050+
return access.has_access(user, "see_about_page", course_overview, course_key=course.id)
1051+
1052+
def test_learner_granted_via_catalog_visibility_both(self):
1053+
"""Learners without AuthZ roles can view the about page when catalog allows it."""
1054+
response = self._see_about_page_response(self.unauthorized_user, self.course_public)
1055+
self.assertTrue(response) # noqa: PT009
1056+
1057+
def test_learner_granted_via_catalog_visibility_about_only(self):
1058+
"""Learners without AuthZ roles can view about-only courses."""
1059+
response = self._see_about_page_response(self.unauthorized_user, self.course_about_only)
1060+
self.assertTrue(response) # noqa: PT009
1061+
1062+
def test_enrolled_learner_denied_when_catalog_hidden(self):
1063+
"""Enrollment alone does not grant about-page access when catalog is hidden."""
1064+
CourseEnrollmentFactory(user=self.unauthorized_user, course_id=self.course_hidden.id)
1065+
1066+
response = self._see_about_page_response(self.unauthorized_user, self.course_hidden)
1067+
1068+
self.assertFalse(response) # noqa: PT009
1069+
self.assertIsInstance(response, access_response.CatalogVisibilityError) # noqa: PT009
1070+
1071+
def test_beta_tester_granted_via_catalog_about(self):
1072+
"""Beta testers rely on catalog visibility, not AuthZ authoring permissions."""
1073+
beta_tester = BetaTesterFactory.create(course_key=self.course_about_only.id)
1074+
1075+
response = self._see_about_page_response(beta_tester, self.course_about_only)
1076+
1077+
self.assertTrue(response) # noqa: PT009
1078+
1079+
def test_course_staff_bypass_when_catalog_hidden(self):
1080+
"""Course staff can preview the about page when catalog visibility is none."""
1081+
course_staff = StaffFactory.create(course_key=self.course_hidden.id)
1082+
1083+
response = self._see_about_page_response(course_staff, self.course_hidden)
1084+
1085+
self.assertTrue(response) # noqa: PT009
1086+
1087+
def test_limited_staff_bypass_when_catalog_hidden(self):
1088+
"""Limited staff inherit staff bypass for about-page access."""
1089+
limited_staff = UserFactory.create()
1090+
CourseLimitedStaffRole(self.course_hidden.id).add_users(limited_staff)
1091+
1092+
response = self._see_about_page_response(limited_staff, self.course_hidden)
1093+
1094+
self.assertTrue(response) # noqa: PT009
1095+
1096+
def test_authz_role_grants_access_when_catalog_hidden(self):
1097+
"""Users with COURSES_VIEW_COURSE can access hidden about pages."""
1098+
self.add_user_to_role_in_course(self.unauthorized_user, COURSE_EDITOR.external_key, self.course_hidden.id)
1099+
1100+
response = self._see_about_page_response(self.unauthorized_user, self.course_hidden)
1101+
1102+
self.assertTrue(response) # noqa: PT009
1103+
1104+
def test_anonymous_user_uses_legacy_path(self):
1105+
"""Anonymous users are routed to the legacy path and follow catalog visibility."""
1106+
anonymous_user = AnonymousUserFactory.create()
1107+
1108+
response = self._see_about_page_response(anonymous_user, self.course_public)
1109+
1110+
self.assertTrue(response) # noqa: PT009
1111+
1112+
def test_denied_returns_catalog_visibility_error(self):
1113+
"""AuthZ path returns CatalogVisibilityError when all checks fail."""
1114+
response = self._see_about_page_response(self.unauthorized_user, self.course_hidden)
1115+
1116+
self.assertFalse(response) # noqa: PT009
1117+
self.assertIsInstance(response, access_response.CatalogVisibilityError) # noqa: PT009
1118+
self.assertEqual(response.error_code, "not_visible_in_catalog") # noqa: PT009
1119+
1120+
def test_legacy_path_when_authz_disabled(self):
1121+
"""When AuthZ is off, catalog visibility rules still apply."""
1122+
with patch.object(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, "is_enabled", return_value=False):
1123+
response = self._see_about_page_response(self.unauthorized_user, self.course_public)
1124+
1125+
self.assertTrue(response) # noqa: PT009
1126+
1127+
hidden_response = self._see_about_page_response(self.unauthorized_user, self.course_hidden)
1128+
1129+
self.assertFalse(hidden_response) # noqa: PT009
1130+
self.assertIsInstance(hidden_response, access_response.CatalogVisibilityError) # noqa: PT009

0 commit comments

Comments
 (0)