Repository navigation
Enforcement: Cohorts tab (course_groups API) #492
Description
Permissions
courses.view_cohortscourses.manage_cohorts
Endpoints
These endpoints are not part of the instructor v2 API. They live in openedx/core/djangoapps/course_groups:
/api/cohorts/v1/courses/{course_id}/cohorts/…(GET →view_cohorts, write →manage_cohorts)/api/cohorts/v1/courses/{course_id}/cohorts/{id}/users/…- Cohort CSV upload →
manage_cohorts courses/{course_id}/cohorts/settings(course_cohort_settings_handler) → read or write depending on the method
Notes
- Other consumers also use these endpoints (e.g. discussions and the legacy LMS). The change must not remove access from anyone who has it today.
- Estimate this task separately: it's a different app, with different tests and reviewers.
Acceptance criteria
- The common criteria from Backend permission enforcement (LMS) #485 are met.
- The other consumers were identified and verified to still work.
Blocked by: #486
Metadata
Metadata
Labels
Type
Projects
- Status
Activity
efortish commentedon Oct 17, 2026
Looked into this to scope the work ahead of time. #486 (the blocker) is still open with no PR or branch from anyone, so there's nothing to pick up there yet. Leaving the findings here so whoever picks this up next doesn't have to re-derive them.
Endpoints in scope
In
openedx/core/djangoapps/course_groups, all three use the same DRF permission class today,permission_classes = (IsAuthenticated, IsStaffOrAdmin):CohortSettings(views.py, around line 485): GET/PATCH,/api/cohorts/v1/settings/{course_id}CohortHandler(views.py, around line 525): GET should map toview_cohorts, POST/PATCH tomanage_cohorts,/api/cohorts/v1/courses/{course_id}/cohorts/{id}CohortUsers(views.py, around line 675): GET toview_cohorts, POST/DELETE tomanage_cohorts,.../cohorts/{id}/users/{username}Outside that DRF group:
CohortCSV(lms/djangoapps/instructor/views/api.py): should requiremanage_cohortscourse_cohort_settings_handler(course_groups/views.py, around line 124): a legacy function-based view, also used by Studio and discussions. GET should map toview_cohorts, PATCH tomanage_cohortsOut of scope (used by Discussions and the legacy LMS UI, not the Cohorts tab):
cohort_handler,users_in_cohort,add_users_to_cohort,remove_user_from_cohort,debug_cohort_mgmt, all under the non-/api/courses/{course_id}/cohorts/...routes. These should stay untouched per the "must not remove access from anyone who has it today" note above.What #486 already gives us, and what it doesn't yet
openedx/core/djangoapps/authz/decorators.pyalready has the combined check #486 describes:authz_permission_required/user_has_course_permissioncheck AuthZ whenAUTHZ_COURSE_AUTHORING_FLAGis enabled for the course, and otherwise fall back to alegacy_permissioncallable.What's missing is a legacy-permission table that actually fits this tab.
LegacyAuthoringPermission/LEGACY_PERMISSION_HANDLER_MAP(authz/constants.py) only mapREAD/WRITEtohas_studio_read_access/has_studio_write_access, which is Studio authoring logic. The equivalent for this tab is whatIsStaffOrAdmin(course_groups/permissions.py) already does: GET is allowed to forum admin/moderator/community TA in addition to staff/instructor, writes are staff/instructor only. That logic needs an LMS-side home, either as new entries on the existing enum/map or a separate one, as #486 itself leaves open ("ExtendingLegacyAuthoringPermissionandLEGACY_PERMISSION_HANDLER_MAP, or creating an LMS equivalent").CourseMetadataView(lms/djangoapps/instructor/views/api_v2.py) also doesn't expose any AuthZ permission yet, and tab visibility (get_course_tab_list) isn't wired to AuthZ either. That's explicitly #486's responsibility, not something worth solving per-tab.Suggested plan once #486 unblocks this
IsStaffOrAdmin's current logic) wherever Shared infrastructure and exposing permissions to the MFE #486 lands it.IsStaffOrAdminforauthz_permission_required/user_has_course_permissionwithcourses.view_cohorts/courses.manage_cohortsplus that legacy fallback, across the three DRF views,CohortCSV, andcourse_cohort_settings_handler.CourseMetadataViewpermissions/tab-visibility piece to Shared infrastructure and exposing permissions to the MFE #486 rather than duplicating a one-off version of it here.efortish commentedon Oct 20, 2026
@BryanttV Could you take a look of this?
BryanttV commentedon Oct 20, 2026
Hi @efortish, thanks for the analysis!
The PR for #486 is now up, and the explanation on how to migrate a view is in the README. I'd love to get your review as well. If you have any questions, please let me know.
We need to migrate all the views used by the Instructor Dashboard MFE for the Cohorts tab. I've left a list of the views here: Cohorts Tab, but if you spot any other endpoint used by the MFE, please point it out so we can take it into account for the migration.
Views used by the legacy UI, such as
course_cohort_settings_handler, won't be migrated for now.efortish commentedon Oct 20, 2026
Reviewed #39255, left my notes there: openedx/openedx-platform#39255 (review)
Good news for this issue specifically:
legacy.py'shas_cohort_view_access/has_cohort_manage_accessalready reproduceIsStaffOrAdmin's current logic, which is exactly the legacy-equivalent gap I flagged in my earlier comment here. So once #486 merges, wiring the three DRF views (CohortSettings,CohortHandler,CohortUsers),CohortCSV, andcourse_cohort_settings_handlerover toInstructorAuthzPermission/user_has_instructor_permissionshould be mostly mechanical, following the README's own cohorts example.One gap that isn't specific to this issue:
courses.view_cohorts/courses.manage_cohortsaren't defined in the openedx-authz schema yet (same as every Instructor Dashboard permission except course team and grading settings). That'll need to land incourse_permissions.yamlbefore this can be wired for real.BryanttV commentedon Oct 21, 2026
Thanks, @efortish. For now, we can move forward with the PR based on the branch from openedx/openedx-platform#39255, which should already have everything needed to migrate a view. Regarding the permissions, they will be included in this issue: #484. In the meantime, we can work by adding the specific permissions we need directly in Casbin, just for testing purposes.
efortish commentedon Oct 21, 2026
Opened a PR covering this: eduNEXT/edx-platform#328
Targeting bav/instructor-authz-shared-infra directly (same approach as openedx-authz#491) so the diff only shows this issue's changes. The PR description has the full detail: which endpoint maps to which permission, why no legacy_permission_check was needed for the three DRF views (the per-method default checks already registered reproduce IsStaffOrAdmin exactly), the two endpoints whose legacy fallback I deliberately kept narrower than the others (course_cohort_settings_handler and CohortCSV, both flagged with the reasoning), and a shared-infra fix (InstructorAuthzPermission needed a configurable course_id_url_kwarg, since cohorts uses course_key_string). Draft until #39255 merges.