FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: keep unmigrated legacy roles working under the authz authoring f… · openedx/openedx-platform@0f57cf2 · GitHub

/ openedx-platform Public

Commit 0f57cf2

Browse files
mariajgrimaldi
and
claude
authored
fix: keep unmigrated legacy roles working under the authz authoring flag (#38855)
Granting and exercising course creator access crashed once authz.enable_course_authoring was on. The courses.create_course permission was implemented in AuthZ, but the course creator role itself was never migrated there. The code deciding whether to use the AuthZ path only checked the flag's state, not whether the specific role had actually been migrated, so unmigrated roles like course creator still used the AuthZ path and crashed. enable_authz_course_authoring now takes an optional role and falls back to the course creator role's own check for any role without a migrated AuthZ equivalent, regardless of the flag. See ADR 0027. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent d83c598 commit 0f57cf2

15 files changed

Lines changed: 195 additions & 166 deletions

File tree

‎cms/djangoapps/contentstore/asset_storage_handlers.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@
2626
from openedx_filters.content_authoring.filters import LMSPageURLRequested
2727
from pymongo import ASCENDING, DESCENDING
2828

29+
from common.djangoapps.student.roles import enable_authz_course_authoring
2930
from common.djangoapps.util.date_utils import get_default_time_display
3031
from common.djangoapps.util.json_request import JsonResponse
3132
from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
3233
from openedx.core.djangoapps.authz.decorators import user_has_course_permission
3334
from openedx.core.djangoapps.contentserver.caching import del_cached_content
3435
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
3536
from openedx.core.djangoapps.user_api.models import UserPreference
36-
from openedx.core.toggles import enable_authz_course_authoring
3737
from xmodule.contentstore.content import StaticContent # pylint: disable=wrong-import-order
3838
from xmodule.contentstore.django import contentstore # pylint: disable=wrong-import-order
3939
from xmodule.exceptions import NotFoundError # pylint: disable=wrong-import-order

‎cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from rest_framework import serializers
66

77
from cms.djangoapps.contentstore import toggles
8-
from openedx.core import toggles as core_toggles
8+
from common.djangoapps.student.roles import enable_authz_course_authoring
99

1010

1111
class CourseWaffleFlagsSerializer(serializers.Serializer):
@@ -225,4 +225,4 @@ def get_enable_authz_course_authoring(self, obj):
225225
Method to get the authz.enable_course_authoring waffle flag
226226
"""
227227
course_key = self.get_course_key()
228-
return core_toggles.enable_authz_course_authoring(course_key)
228+
return enable_authz_course_authoring(course_key)

‎cms/djangoapps/contentstore/tests/test_course_create_rerun.py‎

Lines changed: 21 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313
from django.test import override_settings
1414
from django.test.client import RequestFactory
1515
from django.urls import reverse
16+
from edx_toggles.toggles.testutils import override_waffle_flag
1617
from opaque_keys.edx.keys import CourseKey
1718
from opaque_keys.edx.locator import CourseLocator
18-
from openedx_authz.constants.roles import COURSE_EDITOR
1919
from organizations.api import add_organization, get_course_organizations, get_organization_by_short_name
2020
from organizations.exceptions import InvalidOrganizationException
2121
from organizations.models import Organization
@@ -34,6 +34,7 @@
3434
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, OrgContentCreatorRole
3535
from common.djangoapps.student.tests.factories import AdminFactory, UserFactory
3636
from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin
37+
from openedx.core.toggles import AUTHZ_COURSE_AUTHORING_FLAG
3738
from xmodule.course_block import CourseFields
3839
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
3940
from xmodule.modulestore.tests.factories import CourseFactory
@@ -386,119 +387,37 @@ def test_default_enable_flexible_peer_openassessments_on_rerun(
386387
)
387388

388389

389-
class TestCourseHandlerAuthz(
390-
CourseAuthoringAuthzTestMixin,
391-
ModuleStoreTestCase,
392-
):
390+
@ddt.ddt
391+
class TestCourseHandlerStaffAccess(ModuleStoreTestCase):
393392
"""
394-
AuthZ integration tests for course_handler using real RBAC (no mocks).
393+
Tests that global staff can create a course through course_handler with no
394+
course-specific role, covering the GlobalStaff bypass in user_has_role. Course
395+
creation doesn't check AuthZ (see ADR 0027), so this doesn't use the AuthZ mixin.
395396
"""
396397

397398
def setUp(self):
398399
super().setUp()
399-
400400
self.url = reverse("course_handler")
401+
self.staff_user = AdminFactory()
402+
self.staff_client = AjaxEnabledTestClient()
403+
self.staff_client.login(username=self.staff_user.username, password=self.TEST_PASSWORD)
401404

402-
# Create a base course to extract org
403-
self.course = CourseFactory.create()
404-
self.course_key = self.course.id
405-
self.org = self.course_key.org
406-
407-
# If your policy expects this format, keep it
408-
self.org_key = f"course-v1:{self.org}+*"
409-
410-
self.authorized_client = AjaxEnabledTestClient()
411-
self.authorized_client.login(
412-
username=self.authorized_user.username,
413-
password=self.password,
414-
)
415-
416-
self.unauthorized_client = AjaxEnabledTestClient()
417-
self.unauthorized_client.login(
418-
username=self.unauthorized_user.username,
419-
password=self.password,
420-
)
421-
self.authorized_staff_client = AjaxEnabledTestClient()
422-
self.authorized_staff_client.login(
423-
username=self.staff_user.username,
424-
password=self.password,
425-
)
426-
427-
# ------------------------------------------------------------
428-
# CREATE COURSE -- Non-staff users and existing Organization
429-
# ------------------------------------------------------------
430-
@override_settings(DISABLE_COURSE_CREATION=False)
431-
def test_create_course_unauthorized(self):
432-
"""
433-
User without role cannot create course.
434-
"""
435-
436-
response = self.unauthorized_client.ajax_post(self.url, {
437-
"org": self.org,
438-
"number": "CS101",
439-
"display_name": "Authz Course",
440-
"run": "2026_T1",
441-
})
442-
443-
assert response.status_code == 403
444-
445-
@override_settings(DISABLE_COURSE_CREATION=False)
446-
def test_create_course_unauthorized_with_role(self):
447-
"""
448-
User with role but without required permission cannot create course.
449-
"""
450-
451-
self.add_user_to_role_in_course(
452-
self.unauthorized_user,
453-
COURSE_EDITOR.external_key,
454-
"course-v1:someotherorg+*",
455-
)
456-
457-
response = self.unauthorized_client.ajax_post(self.url, {
458-
"org": self.org,
459-
"number": "CS101",
460-
"display_name": "Authz Course",
461-
"run": "2026_T1",
462-
})
463-
464-
assert response.status_code == 403
465-
466-
# ------------------------------------------------------------
467-
# CREATE COURSE -- Staff users
468-
# Only staff users can create course, and they can do it
469-
# without an org role.
470-
# ------------------------------------------------------------
471-
def test_create_course_staff(self):
405+
@ddt.data(True, False)
406+
def test_create_course_staff(self, authz_enabled):
472407
"""
473-
Staff user can create course.
408+
Staff user can create a course with no prior course-specific role, whether
409+
the AuthZ course-authoring flag is on or off.
474410
"""
475-
response = self.authorized_staff_client.ajax_post(self.url, {
476-
"org": self.org,
477-
"number": "CS101",
478-
"display_name": "Authz Course",
479-
"run": "2026_T1",
480-
})
411+
with override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=authz_enabled):
412+
response = self.staff_client.ajax_post(self.url, {
413+
"org": "StaffOrg",
414+
"number": "CS101",
415+
"display_name": "Staff Course",
416+
"run": "2026_T1",
417+
})
481418

482419
assert response.status_code == 200
483420

484-
# ------------------------------------------------------------
485-
# FEATURE FLAG
486-
# ------------------------------------------------------------
487-
@override_settings(DISABLE_COURSE_CREATION=True)
488-
def test_create_course_disabled_by_flag(self):
489-
"""
490-
Even authorized users cannot create course if feature flag is off.
491-
"""
492-
493-
response = self.authorized_staff_client.ajax_post(self.url, {
494-
"org": self.org,
495-
"number": "CS101",
496-
"display_name": "Authz Course",
497-
"run": "2026_T1",
498-
})
499-
500-
assert response.status_code == 403
501-
502421

503422
class TestCourseRerunAuthz(
504423
CourseAuthoringAuthzTestMixin,

‎cms/djangoapps/contentstore/views/course.py‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
GlobalStaff,
7575
OrgStaffRole,
7676
UserBasedRole,
77+
enable_authz_course_authoring,
7778
strict_role_checking,
7879
)
7980
from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json
@@ -857,7 +858,7 @@ def _get_course_keys_from_platform_scope() -> set[CourseKey]:
857858
if core_toggles.AUTHZ_COURSE_AUTHORING_FLAG.is_enabled():
858859
return set(course_keys)
859860

860-
return {course_key for course_key in course_keys if core_toggles.enable_authz_course_authoring(course_key)}
861+
return {course_key for course_key in course_keys if enable_authz_course_authoring(course_key)}
861862

862863

863864
def _get_course_keys_from_scopes(authz_scopes: list[ScopeData]) -> set[CourseKey]:
@@ -888,15 +889,15 @@ def _get_course_keys_from_scopes(authz_scopes: list[ScopeData]) -> set[CourseKey
888889

889890
for access in authz_scopes:
890891
if isinstance(access, CourseOverviewData) and access.course_key:
891-
if core_toggles.enable_authz_course_authoring(access.course_key):
892+
if enable_authz_course_authoring(access.course_key):
892893
course_keys.add(access.course_key)
893894
elif isinstance(access, OrgCourseOverviewGlobData) and access.org:
894895
org_keys.add(access.org)
895896

896897
if org_keys:
897898
course_keys.update(
898899
key for key in _get_course_keys_for_org_scope(org_keys)
899-
if core_toggles.enable_authz_course_authoring(key)
900+
if enable_authz_course_authoring(key)
900901
)
901902

902903
return course_keys
@@ -1344,7 +1345,7 @@ def rerun_course(user, source_course_key, org, number, run, fields, background=T
13441345
# is implemented (pre-assigning roles without a CourseOverview). Once resolved,
13451346
# add_instructor can be called unconditionally here and the created_user fallback
13461347
# in get_in_process_course_actions can be removed.
1347-
if not core_toggles.enable_authz_course_authoring(destination_course_key):
1348+
if not enable_authz_course_authoring(destination_course_key):
13481349
add_instructor(destination_course_key, user, user)
13491350

13501351
# Mark the action as initiated

‎common/djangoapps/student/auth.py‎

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
from django.core.exceptions import PermissionDenied
1212
from opaque_keys.edx.locator import LibraryLocator
1313
from openedx_authz import api as authz_api
14-
from openedx_authz.constants.permissions import COURSES_CREATE_COURSE, COURSES_MANAGE_ADVANCED_SETTINGS
14+
from openedx_authz.constants.permissions import COURSES_MANAGE_ADVANCED_SETTINGS
1515

1616
from common.djangoapps.student.roles import (
1717
CourseBetaTesterRole,
@@ -227,9 +227,9 @@ def is_content_creator(user, org):
227227
"""
228228
Determine whether a user is allowed to create course content for a given organization.
229229
230-
This function abstracts the permission check for course creation. Depending on the
231-
state of the AuthZ feature flag, it delegates the evaluation to either the AuthZ-based
232-
RBAC system or the legacy role-based permission system.
230+
Neither CourseCreatorRole nor OrgContentCreatorRole has a migrated AuthZ equivalent yet
231+
(see ADR 0027), so this always checks the legacy role-based permission system. Once
232+
either role gets a migrated equivalent, this should also check AuthZ, gated on that role.
233233
234234
Args:
235235
user (User): The user whose permissions are being evaluated.
@@ -238,38 +238,10 @@ def is_content_creator(user, org):
238238
Returns:
239239
bool: True if the user has permission to create course content in the given
240240
organization, False otherwise.
241-
242-
Notes:
243-
- When AuthZ is enabled, this checks permissions via RBAC policies.
244-
- When AuthZ is disabled, this falls back to legacy Django role checks.
245-
- Course creation may still be blocked by global feature flags (e.g.,
246-
DISABLE_COURSE_CREATION), which are enforced downstream.
247241
"""
248-
if core_toggles.AUTHZ_COURSE_AUTHORING_FLAG.is_enabled():
249-
return _has_content_creator_access(user, org)
250242
return _has_legacy_content_creator_access(user, org)
251243

252244

253-
def _has_content_creator_access(user, org):
254-
"""
255-
Check if the user has content creator access based on AuthZ permissions.
256-
257-
Returns:
258-
bool: True if the user has platform-wide or org-scoped course creation permission.
259-
"""
260-
if getattr(settings, 'DISABLE_COURSE_CREATION', False):
261-
return False
262-
263-
scope_keys = (
264-
authz_api.PlatformCourseOverviewGlobData.build_external_key(),
265-
authz_api.OrgCourseOverviewGlobData.build_external_key(org),
266-
)
267-
return any(
268-
authz_api.is_user_allowed(user.username, COURSES_CREATE_COURSE.identifier, scope_key)
269-
for scope_key in scope_keys
270-
)
271-
272-
273245
def _has_legacy_content_creator_access(user, org):
274246
"""
275247
Check if the user has the role to create content.

‎common/djangoapps/student/roles.py‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
from common.djangoapps.student.models import CourseAccessRole
2222
from common.djangoapps.student.signals.signals import emit_course_access_role_added, emit_course_access_role_removed
2323
from openedx.core.lib.cache_utils import get_cache
24-
from openedx.core.toggles import enable_authz_course_authoring
24+
from openedx.core.toggles import AUTHZ_COURSE_AUTHORING_FLAG
2525

2626
log = logging.getLogger(__name__)
2727

@@ -43,6 +43,24 @@ def get_legacy_role_from_authz_role(authz_role: str) -> str:
4343
return next((k for k, v in authz_roles.LEGACY_COURSE_ROLE_EQUIVALENCES.items() if v == authz_role), None)
4444

4545

46+
def enable_authz_course_authoring(course_key: CourseKey | None = None, role: str | None = None) -> bool:
47+
"""
48+
True only if the authz.enable_course_authoring waffle flag is enabled and, when `role` is
49+
given, that role has a migrated authz equivalent.
50+
51+
The migration only covers a subset of legacy roles (see LEGACY_COURSE_ROLE_EQUIVALENCES
52+
and ADR 0027), so an unmigrated role must keep resolving to the legacy path even with the
53+
flag on. Most callers have no specific role in scope and can omit `role`. Pass it when
54+
acting on a specific legacy role, so a role like course_creator_group falls back to legacy
55+
instead of crashing or being denied by authz (see openedx-authz#353, #354).
56+
"""
57+
if not AUTHZ_COURSE_AUTHORING_FLAG.is_enabled(course_key):
58+
return False
59+
if role is not None and get_authz_role_from_legacy_role(role) is None:
60+
return False
61+
return True
62+
63+
4664
def authz_add_role(user: User, authz_role: str, course_key: str):
4765
"""
4866
Add a user's role in a course if not already added.
@@ -520,7 +538,7 @@ def add_users(self, *users):
520538
"""
521539
Add the supplied django users to this role.
522540
"""
523-
if enable_authz_course_authoring(self.course_key):
541+
if enable_authz_course_authoring(self.course_key, role=self._role_name):
524542
self._authz_add_users(users)
525543
else:
526544
self._legacy_add_users(users)
@@ -561,7 +579,7 @@ def remove_users(self, *users):
561579
"""
562580
Remove the supplied django users from this role.
563581
"""
564-
if enable_authz_course_authoring(self.course_key):
582+
if enable_authz_course_authoring(self.course_key, role=self._role_name):
565583
self._authz_remove_users(users)
566584
else:
567585
self._legacy_remove_users(users)
@@ -599,7 +617,7 @@ def users_with_role(self):
599617
"""
600618
Return a django QuerySet for all of the users with this role
601619
"""
602-
if enable_authz_course_authoring(self.course_key):
620+
if enable_authz_course_authoring(self.course_key, role=self._role_name):
603621
return self._authz_users_with_role()
604622
else:
605623
return self._legacy_users_with_role()
@@ -628,7 +646,7 @@ def get_orgs_for_user(self, user):
628646
"""
629647
Returns a list of org short names for the user with given role.
630648
"""
631-
if enable_authz_course_authoring(self.course_key):
649+
if enable_authz_course_authoring(self.course_key, role=self._role_name):
632650
return self._authz_get_orgs_for_user(user)
633651
else:
634652
return self._legacy_get_orgs_for_user(user)
@@ -642,7 +660,7 @@ def has_org_for_user(self, user: User, org: str | None = None) -> bool:
642660
org: optional org to check against access to role,
643661
if not specified, will return True if the user has access to at least one org
644662
"""
645-
if enable_authz_course_authoring(self.course_key):
663+
if enable_authz_course_authoring(self.course_key, role=self._role_name):
646664
orgs_with_role = self.get_orgs_for_user(user)
647665
if org:
648666
return org in orgs_with_role

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL