Skip to content

feat: AuthZ for course authoring compatibility layer #38013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rodmgwgu wants to merge 2 commits into
base: master
Choose a base branch
from
+546 −165
Open

feat: AuthZ for course authoring compatibility layer #38013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9477f62
feat: AuthZ for Course Authoring compatibility layer
rodmgwgu Feb 13, 2026
c3b66ad
squash!: Add active user validation on _authz_add_users
rodmgwgu Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions cms/djangoapps/contentstore/tests/test_course_listing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,16 @@
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
from openedx.core.djangolib.testing.utils import AUTHZ_TABLES
from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order

TOTAL_COURSES_COUNT = 10
USER_COURSES_COUNT = 1

QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES + AUTHZ_TABLES


@ddt.ddt
class TestCourseListing(ModuleStoreTestCase):
Expand Down Expand Up @@ -303,10 +306,10 @@ def test_course_listing_performance(self):
courses_list, __ = _accessible_courses_list_from_groups(self.request)
self.assertEqual(len(courses_list), USER_COURSES_COUNT)

with self.assertNumQueries(1, table_ignorelist=WAFFLE_TABLES):
with self.assertNumQueries(2, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST):
_accessible_courses_list_from_groups(self.request)

with self.assertNumQueries(2, table_ignorelist=WAFFLE_TABLES):
with self.assertNumQueries(2, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST):
_accessible_courses_iter_for_tests(self.request)

def test_course_listing_errored_deleted_courses(self):
Expand Down
27 changes: 15 additions & 12 deletions cms/djangoapps/contentstore/views/library.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,21 @@ def _user_can_create_library_for_org(user, org=None):
org_filter_params['org'] = org
is_course_creator = get_course_creator_status(user) == 'granted'
has_org_staff_role = OrgStaffRole().get_orgs_for_user(user).filter(**org_filter_params).exists()
has_course_staff_role = (
UserBasedRole(user=user, role=CourseStaffRole.ROLE)
.courses_with_role()
.filter(**org_filter_params)
.exists()
)
has_course_admin_role = (
UserBasedRole(user=user, role=CourseInstructorRole.ROLE)
.courses_with_role()
.filter(**org_filter_params)
.exists()
)

all_courses_with_staff_role = UserBasedRole(user=user, role=CourseStaffRole.ROLE).courses_with_role()
Copy link
Contributor

@bmtcril bmtcril Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having trouble understanding what this change does. It seems like it's just shifting the org filtering from the database to the Python?

I think we could also save some queries here by returning early as things become true since we're just ORing them all together at the end.

Copy link
Contributor Author

@rodmgwgu rodmgwgu Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change was necessary because previously, the courses_with_role() method returned a CourseAccessRole QuerySet, but now it returns a list of AuthzCompatCourseAccessRole, which is not a QuerySet, this was necessary to allow compatibility with Authz that doesn't manage Django QuerySets as it depends on Casbin.

I'll check for ways of optimizing this

Copy link
Contributor

@bmtcril bmtcril Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks!

courses_with_staff_role_on_org = all_courses_with_staff_role
if org is not None:
courses_with_staff_role_on_org = [course for course in all_courses_with_staff_role if course.org == org]

has_course_staff_role = len(courses_with_staff_role_on_org) > 0

all_courses_with_admin_role = UserBasedRole(user=user, role=CourseInstructorRole.ROLE).courses_with_role()
courses_with_admin_role_on_org = all_courses_with_admin_role
if org is not None:
courses_with_admin_role_on_org = [course for course in all_courses_with_admin_role if course.org == org]

has_course_admin_role = len(courses_with_admin_role_on_org) > 0

return is_course_creator or has_org_staff_role or has_course_staff_role or has_course_admin_role
else:
# EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present.
Expand Down
4 changes: 2 additions & 2 deletions common/djangoapps/student/role_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
)
from openedx.core.lib.cache_utils import request_cached
from common.djangoapps.student.roles import (
CourseAccessRole,
AuthzCompatCourseAccessRole,
CourseBetaTesterRole,
CourseInstructorRole,
CourseStaffRole,
Expand Down Expand Up @@ -66,7 +66,7 @@ def get_role_cache(user: User) -> RoleCache:


@request_cached()
def get_course_roles(user: User) -> list[CourseAccessRole]:
def get_course_roles(user: User) -> list[AuthzCompatCourseAccessRole]:
"""
Returns a list of all course-level roles that this user has.

Expand Down
Loading
Loading