Skip to content

Commit 1efb8da

Browse files
committed
fix: preserve catalog and staff checks for authZ about-page access (#38736)
* fix: preserve catalog and staff checks for authZ about-page access * refactor: remove about page catalog visibility error function and return CatalogVisibilityError directly
1 parent d06d2d3 commit 1efb8da

4 files changed

Lines changed: 215 additions & 78 deletions

File tree

lms/djangoapps/course_api/tests/test_api.py

Lines changed: 60 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,14 @@
1616
from rest_framework.request import Request
1717
from rest_framework.test import APIRequestFactory
1818

19+
from common.djangoapps.student.roles import CourseLimitedStaffRole
20+
from common.djangoapps.student.tests.factories import StaffFactory, UserFactory
1921
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
2022
from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin
2123
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
22-
from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order
23-
from xmodule.modulestore.tests.django_utils import ( # lint-amnesty, pylint: disable=wrong-import-order
24+
from xmodule.course_block import CATALOG_VISIBILITY_ABOUT, CATALOG_VISIBILITY_NONE
25+
from xmodule.modulestore.exceptions import ItemNotFoundError # pylint: disable=wrong-import-order
26+
from xmodule.modulestore.tests.django_utils import ( # pylint: disable=wrong-import-order
2427
ModuleStoreTestCase,
2528
SharedModuleStoreTestCase,
2629
)
@@ -146,98 +149,92 @@ def setUpClass(cls):
146149

147150
cls.course = cls.create_course()
148151
cls.hidden_course = cls.create_course(
149-
course='hidden',
150-
visible_to_staff_only=True
152+
course="hidden",
153+
visible_to_staff_only=True,
154+
catalog_visibility=CATALOG_VISIBILITY_NONE,
155+
)
156+
cls.about_only_course = cls.create_course(
157+
course="aboutonly",
158+
catalog_visibility=CATALOG_VISIBILITY_ABOUT,
151159
)
152160

153161
def test_get_existing_course_as_authorized_user(self):
154162
"""User with COURSE_EDITOR role can access course."""
155-
self.add_user_to_role_in_course(
156-
self.authorized_user,
157-
COURSE_EDITOR.external_key,
158-
self.course.id
159-
)
163+
self.add_user_to_role_in_course(self.authorized_user, COURSE_EDITOR.external_key, self.course.id)
160164

161-
course = self._make_api_call(
162-
self.authorized_user,
163-
self.authorized_user,
164-
self.course.id
165-
)
165+
course = self._make_api_call(self.authorized_user, self.authorized_user, self.course.id)
166+
167+
self.verify_course(course)
168+
169+
def test_get_existing_course_without_authz_role_when_catalog_visible(self):
170+
"""User without AuthZ role can still access when catalog visibility allows."""
171+
course = self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.course.id)
166172

167173
self.verify_course(course)
168174

169-
def test_get_existing_course_as_unauthorized_user(self):
170-
"""User without role should be denied."""
175+
def test_about_only_catalog_visibility_without_authz_role(self):
176+
"""User without AuthZ role can access when catalog visibility is about-only."""
177+
course = self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.about_only_course.id)
178+
179+
self.verify_course(course, course_id=str(self.about_only_course.id))
180+
181+
def test_hidden_course_denied_without_authz_role(self):
182+
"""User without AuthZ role is denied when catalog visibility is none."""
171183
with pytest.raises(CourseAccessRedirect):
172-
self._make_api_call(
173-
self.unauthorized_user,
174-
self.unauthorized_user,
175-
self.course.id
176-
)
184+
self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.hidden_course.id)
177185

178186
def test_get_nonexistent_course(self):
179187
"""Nonexistent course should raise 404."""
180-
course_key = CourseKey.from_string('edX/toy/nope')
188+
course_key = CourseKey.from_string("edX/toy/nope")
181189

182190
with pytest.raises(Http404):
183-
self._make_api_call(
184-
self.authorized_user,
185-
self.authorized_user,
186-
course_key
187-
)
191+
self._make_api_call(self.authorized_user, self.authorized_user, course_key)
192+
193+
def test_course_staff_bypasses_authz_on_hidden_course(self):
194+
"""Course staff can access a hidden course without an AuthZ role."""
195+
course_staff = StaffFactory.create(course_key=self.hidden_course.id)
196+
197+
course = self._make_api_call(course_staff, course_staff, self.hidden_course.id)
198+
199+
self.verify_course(course, course_id=str(self.hidden_course.id))
200+
201+
def test_limited_staff_bypasses_authz_on_hidden_course(self):
202+
"""Limited course staff can access a hidden course without an AuthZ role."""
203+
limited_staff = UserFactory(password=self.password)
204+
CourseLimitedStaffRole(self.hidden_course.id).add_users(limited_staff)
205+
206+
course = self._make_api_call(limited_staff, limited_staff, self.hidden_course.id)
207+
208+
self.verify_course(course, course_id=str(self.hidden_course.id))
188209

189210
def test_hidden_course_for_staff(self):
190211
"""Staff can access hidden course."""
191-
course = self._make_api_call(
192-
self.staff_user,
193-
self.staff_user,
194-
self.hidden_course.id
195-
)
212+
course = self._make_api_call(self.staff_user, self.staff_user, self.hidden_course.id)
196213

197-
self.verify_course(
198-
course,
199-
course_id='course-v1:edX+hidden+2012_Fall'
200-
)
214+
self.verify_course(course, course_id=str(self.hidden_course.id))
201215

202216
def test_hidden_course_for_staff_as_unauthorized_user(self):
203217
"""
204218
Staff requesting data for another user without permissions
205219
should not bypass visibility rules.
206220
"""
207221
with pytest.raises(CourseAccessRedirect):
208-
self._make_api_call(
209-
self.staff_user,
210-
self.unauthorized_user,
211-
self.hidden_course.id
212-
)
222+
self._make_api_call(self.staff_user, self.unauthorized_user, self.hidden_course.id)
213223

214224
def test_user_gains_access_after_role_assignment(self):
215-
"""User initially denied, then allowed after role assignment."""
225+
"""User denied when catalog is hidden, then allowed after role assignment."""
216226
with pytest.raises(CourseAccessRedirect):
217-
self._make_api_call(
218-
self.unauthorized_user,
219-
self.unauthorized_user,
220-
self.course.id
221-
)
222-
self.add_user_to_role_in_course(
223-
self.unauthorized_user,
224-
COURSE_EDITOR.external_key,
225-
self.course.id
226-
)
227-
course = self._make_api_call(
228-
self.unauthorized_user,
229-
self.unauthorized_user,
230-
self.course.id
231-
)
232-
self.verify_course(course)
227+
self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.hidden_course.id)
228+
229+
self.add_user_to_role_in_course(self.unauthorized_user, COURSE_EDITOR.external_key, self.hidden_course.id)
230+
231+
course = self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.hidden_course.id)
232+
233+
self.verify_course(course, course_id=str(self.hidden_course.id))
233234

234235
def test_staff_access_without_authz_role(self):
235236
"""Staff bypasses AuthZ roles."""
236-
course = self._make_api_call(
237-
self.staff_user,
238-
self.staff_user,
239-
self.course.id
240-
)
237+
course = self._make_api_call(self.staff_user, self.staff_user, self.course.id)
241238

242239
self.verify_course(course)
243240

lms/djangoapps/courseware/access.py

Lines changed: 49 additions & 15 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,30 +436,62 @@ 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
444-
if _has_staff_access_to_block(user, courselike, courselike.id):
445-
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
454+
return None
449455

450-
@function_trace('can_see_about_page')
451-
def can_see_about_page():
456+
@function_trace("can_see_about_page")
457+
def can_see_about_page() -> AccessResponse | CatalogVisibilityError:
452458
"""
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.
459+
Entry point for about-page visibility checks.
460+
461+
Grants access when any of the following is true:
462+
- the course catalog_visibility allows the about page, or
463+
- the user has course staff access (including limited staff via role inheritance), or
464+
- the user is authenticated, AuthZ course authoring is enabled for the course,
465+
and the user has COURSES_VIEW_COURSE (including legacy Studio read access
466+
as a fallback during RBAC migration).
467+
468+
Learners, beta testers, and other course-team roles without staff access rely on
469+
catalog visibility only; they are not checked explicitly here.
470+
471+
AuthZ must not replace catalog visibility or staff bypass; those checks run
472+
first so enrolled learners and beta testers are not blocked by authoring
473+
permissions they do not hold.
474+
475+
Returns CatalogVisibilityError when all checks fail.
456476
"""
457-
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()
460-
return legacy_can_see_about_page()
477+
catalog_visibility_access = _about_page_catalog_visibility_access()
478+
if catalog_visibility_access:
479+
return catalog_visibility_access
480+
481+
if _has_staff_access_to_block(user, courselike, courselike.id):
482+
return ACCESS_GRANTED
483+
484+
if (
485+
user
486+
and not user.is_anonymous
487+
and core_toggles.enable_authz_course_authoring(courselike.id)
488+
and user_has_course_permission(
489+
user, COURSES_VIEW_COURSE.identifier, courselike.id, LegacyAuthoringPermission.READ
490+
)
491+
):
492+
return ACCESS_GRANTED
493+
494+
return CatalogVisibilityError()
461495

462496
checkers = {
463497
'load': can_load,

lms/djangoapps/courseware/tests/test_about.py

Lines changed: 37 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,41 @@ 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+
assert self.client.login(username=self.unauthorized_user.username, password=self.password)
250+
251+
@override_settings(COURSE_ABOUT_VISIBILITY_PERMISSION="see_about_page")
252+
def test_about_page_honors_catalog_visibility_without_authz_role(self):
253+
"""A learner without AuthZ roles can view catalog-visible about pages."""
254+
url = reverse("about_course", args=[str(self.course_with_about.id)])
255+
resp = self.client.get(url)
256+
self.assertContains(resp, "WITH ABOUT")
257+
258+
url = reverse("about_course", args=[str(self.course_without_about.id)])
259+
resp = self.client.get(url)
260+
self.assertRedirects(resp, reverse("dashboard"), fetch_redirect_response=False)
261+
262+
226263
class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase):
227264
"""
228265
Tests for the course about page

0 commit comments

Comments
 (0)