feat: add authz permission for the course authoring list by dwong2708 · Pull Request #38199 · openedx/openedx-platform

dwong2708 · 2026-03-20T23:08:36Z

Refactor course access listing to single-pass hybrid AuthZ + legacy approach

Summary

This PR refactors get_courses_accessible_to_user to improve performance, readability, and maintainability by introducing a single-pass hybrid access evaluation model.

Instead of splitting courses into separate AuthZ and legacy pipelines and merging results afterward, this implementation evaluates access per course in a single pass, then performs a single batched query to retrieve the final course list.

Key Improvements

Single-pass access evaluation

Replaced the dual pipeline (AuthZ vs legacy) with a unified loop.
Each course is evaluated once using:
- user_has_course_permission(...)
Eliminates duplicated logic and merging complexity.

Scoped full scan to staff users only

Staff users still require evaluating all courses (expected behavior).
Non-staff users now operate only on candidate course keys, avoiding unnecessary full table scans.

if is_staff_user:
    candidate_keys = CourseOverview.get_all_courses().values_list("id", flat=True)
else:
    candidate_keys = _get_candidate_course_keys(request)

Deferred object fetching

Only fetch full CourseOverview objects after access filtering
Reduces memory usage and DB overhead

Centralized filtering and ordering

Filters (search, active/archived, ordering) are applied once at the end
Ensures consistent behavior and avoids repeated processing

Improved readability and maintainability

Clear step-based flow:

Determine candidate keys
Evaluate access
Batch fetch
Apply filters

Reduced cognitive complexity and branching

Testing Notes

Existing tests should continue to pass without modification
Recommended additional coverage:
- Mixed AuthZ + legacy access scenarios
- Staff vs non-staff behavior
- Empty access cases

Deadline

Verawood

Other information

Resolves: openedx/openedx-authz#190

openedx-webhooks · 2026-03-20T23:08:40Z

Thanks for the pull request, @dwong2708!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

rodmgwgu

It's looking good so far, just some comments

cms/djangoapps/contentstore/views/course.py

cms/djangoapps/contentstore/tests/test_course_listing.py

cms/djangoapps/contentstore/views/course.py

mariajgrimaldi · 2026-03-26T12:30:41Z

cms/djangoapps/contentstore/views/course.py

+    return filter(filter_course, filtered_courses)
+
+
+def _get_candidate_course_keys(request):


Can you help me understand why we are combining both of the keys? I'm not sure I follow. Thanks!

Because non-staff users typically have access to a smaller subset of courses, this significantly reduces the number of courses we need to check for access in the next step.

I left the same comment in the code to improve clarity. Thanks for pointing that out and let me know if it is clearer now.

I wonder about an edge case here where the user has permissions on a course with authz but not in the legacy system, is that even possible?

cms/djangoapps/contentstore/views/course.py

rodmgwgu

Just some questions and observations

cms/djangoapps/contentstore/views/course.py

rodmgwgu · 2026-03-27T21:38:33Z

cms/djangoapps/contentstore/views/course.py

+        # and defer fetching the full course objects until after filtering by access
+        candidate_keys = CourseOverview.get_all_courses().values_list("id", flat=True)
+        # Compute actions once for staff users since they have access to all courses
+        in_process_actions = get_in_process_course_actions(request)


Are in_process_actions only supposed to be fetched for staff users?

Yes it is, according to the old code:

openedx-platform/cms/djangoapps/contentstore/views/course.py

Line 757 in 6d0d910

def get_courses_accessible_to_user(request):

I see that, when we called _accessible_courses_list_from_groups, it in fact meant that in_process_actions was always empty, however, if we got teh AccessListFallback exception, it does call _accessible_courses_summary_iter for non-staff users, which may bring in_process_actions.

rodmgwgu · 2026-03-27T21:43:08Z

cms/djangoapps/contentstore/views/course.py

+    # Step 2: Single-pass decision → collect valid keys
+    valid_course_keys = set()
+    for course_key in candidate_keys:
+        if is_staff_user or user_has_course_permission(


Is this check really needed? From what I'm understanding, _get_candidate_course_keys already filtered the courses so we only get the ones the user has permission to see, and the other code path is for staff which should see everything anyway.

Or am I missing something?

This was something I was unsure about. Based on the old code:

openedx-platform/cms/djangoapps/contentstore/views/course.py

Line 767 in 6d0d910

courses, in_process_course_actions = _accessible_courses_summary_iter(request)

The _accessible_courses_summary_iter function is where the logic for staff users is handled. It iterates through each course and applies has_studio_read_access:

openedx-platform/cms/djangoapps/contentstore/views/course.py

Line 410 in 6d0d910

return has_studio_read_access(request.user, course_summary.id)

That’s why this code attempts to preserve the previous behavior. However, I agree with you that it seems redundant. I can remove it if you’re okay with that and verify everything with tests.

I decided to remove this redundant check.

rodmgwgu · 2026-03-30T18:08:26Z

Just tested in my local, and it's working correctly.

mariajgrimaldi

Only a nit and a question left.

Looks good! Interested in testing how these changes perform against only legacy.

mariajgrimaldi · 2026-03-31T11:04:56Z

cms/djangoapps/contentstore/views/course.py

+    authz_keys = {
+        access.course_key
+        for access in authz_scopes
+        if isinstance(access, CourseOverviewData) and access.course_key


Are we expecting something different than CourseOverviewData here or should we trust the get_scopes_for_user_and_permission signature?

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Mar 20, 2026

openedx-webhooks added this to Contributions Mar 20, 2026

github-project-automation bot moved this to Needs Triage in Contributions Mar 20, 2026

dwong2708 force-pushed the dwong2708/authz-new-perms-course-list-endpoint branch from 25142ea to 96b100a Compare March 21, 2026 00:03

dwong2708 changed the title ~~Dwong2708/authz new perms course list endpoint~~ feat: add authz permission for the course authoring list Mar 21, 2026

dwong2708 requested review from bmtcril, mariajgrimaldi, rodmgwgu and wgu-taylor-payne March 21, 2026 22:57

dwong2708 marked this pull request as ready for review March 21, 2026 22:58

mphilbrick211 added the mao-onboarding Reviewing this will help onboard devs from an Axim mission-aligned organization (MAO). label Mar 23, 2026

mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Mar 23, 2026

rodmgwgu reviewed Mar 24, 2026

View reviewed changes

cms/djangoapps/contentstore/views/course.py Show resolved Hide resolved

cms/djangoapps/contentstore/views/course.py Show resolved Hide resolved

cms/djangoapps/contentstore/views/course.py Show resolved Hide resolved

dwong2708 force-pushed the dwong2708/authz-new-perms-course-list-endpoint branch from 78bf785 to 3cba95e Compare March 24, 2026 19:25

mariajgrimaldi requested changes Mar 26, 2026

View reviewed changes

dwong2708 requested review from mariajgrimaldi and rodmgwgu March 26, 2026 17:32

rodmgwgu reviewed Mar 27, 2026

View reviewed changes

dwong2708 added 9 commits March 27, 2026 16:33

feat: add authz permission for the course authoring list

91ec578

fixup! feat: add authz permission for the course authoring list

980f2c2

fixup! feat: add authz permission for the course authoring list

94aed9a

fixup! feat: add authz permission for the course authoring list

01a767d

fixup! feat: add authz permission for the course authoring list

11589d9

fixup! feat: add authz permission for the course authoring list

3a0edc9

fixup! feat: add authz permission for the course authoring list

93bc724

fixup! feat: add authz permission for the course authoring list

a8fe5e5

fixup! feat: add authz permission for the course authoring list

489a90f

dwong2708 requested a review from rodmgwgu March 28, 2026 00:12

fixup! feat: add authz permission for the course authoring list

29b8587

dwong2708 force-pushed the dwong2708/authz-new-perms-course-list-endpoint branch from 234f420 to 29b8587 Compare March 30, 2026 17:12

mariajgrimaldi approved these changes Mar 31, 2026

View reviewed changes

		return filter(filter_course, filtered_courses)


		def _get_candidate_course_keys(request):

Conversation

dwong2708 commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Refactor course access listing to single-pass hybrid AuthZ + legacy approach

Summary

Key Improvements

Testing Notes

Deadline

Other information

Uh oh!

openedx-webhooks commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rodmgwgu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rodmgwgu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rodmgwgu commented Mar 30, 2026

Uh oh!

mariajgrimaldi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dwong2708 commented Mar 20, 2026 •

edited

Loading

openedx-webhooks commented Mar 20, 2026 •

edited

Loading

mariajgrimaldi left a comment •

edited

Loading