Repository navigation
Role-agnostic API behavior #414
Description
Description
- Audit of existing code: review the listing, validation, assignment, and filtering paths for roles, looking for:
- Comparisons like
if role == "instructor"or hardcoded lists of valid roles. - Conditional branches specific to a role name.
- Validations that assume a fixed, known set of roles at write time.
- Comparisons like
- Refactor: replace those comparisons with lookups against the dynamically loaded definitions.
- Risk to watch: this kind of hardcoding tends to be scattered across several modules (serializers, DRF permissions, form validators) — worth doing an exhaustive grep before estimating this as "done."
- Regression: current built-in roles must keep working exactly the same after the refactor — this task carries high risk of breaking existing functionality if not covered by regression tests.
Concrete deliverable: no AuthZ endpoint has code that depends on knowing a specific role name in advance; everything resolves against the loaded definitions.
Metadata
Metadata
Assignees
Labels
Type
Projects
- Status
Activity
[-]6. Role-agnostic API behavior[/-][+]Role-agnostic API behavior[/+]efortish commentedon Sep 15, 2026
@BryanttV
Did the exhaustive audit this issue asks for before estimating anything. Read through (not just grepped) every non-test module in the listing, validation, assignment, and filtering paths:
rest_api/v1/views.py,serializers.py,permissions.py,filters.py,fields.py,paginators.pyrest_api/utils.py,decorators.py,data.pyapi/roles.py,users.py,permissions.py,utils.py,data.pyengine/enforcer.py,adapter.py,filter.py,matcher.py,utils.pymodels/core.py,scopes.py,subjects.py,authz_migration.py,engine.pymanagement/commands/enforcement.py,load_policies.py,authz_migrate_course_authoring.py,authz_rollback_course_authoring.pyhandlers.py,admin.py,utils.pyFinding: I didn't find any
if role == "instructor"style comparisons, hardcoded lists of valid roles, or role-name-specific branches in this surface. The role field inRoleMixinis a plainCharField, validated by looking it up againstapi.get_role_definitions_in_scope(scope)(seeRoleScopeValidationMixin._validate_scope_and_roleinserializers.py).RoleListViewandRoleUserAPIViewboth iterate over whateverapi.get_role_definitions_in_scope()returns and key offrole.external_key.DynamicScopePermissionand friends inrest_api/v1/permissions.pycheck permission identifiers (e.g.content_libraries.view_library_team), never role names. This part of the codebase looks like it was built role-agnostic from the start, on top of the Casbin-policy-backedRoleDataabstraction.The only literal role-name strings I found outside
constants/roles.py(which is the role registry itself, i.e. the "loaded definitions") are:LEGACY_COURSE_ROLE_EQUIVALENCESinconstants/roles.py, used byengine/utils.py'smigrate_legacy_course_roles_to_authz/migrate_authz_to_legacy_course_roles.access_level_to_roleinengine/utils.py'smigrate_legacy_permissions.Both are one-time translation tables at the boundary where edx-platform's genuinely fixed legacy fields (
CourseAccessRole.role,ContentLibraryPermission.access_level, both plainchoices=fields on the legacy side) get converted into the new AuthZ model during migration. That's inherent to translating a closed legacy enum into the new system, not "AuthZ endpoint code that depends on knowing a role name in advance," so I don't think it's in scope for this issue's deliverable.Given that, there doesn't seem to be a refactor to do here. What I think is still missing, and what the issue's own "Regression" section points at, is proof: nothing in the test suite currently exercises a role outside the five built-in ones, so the role-agnostic behavior above is implicit rather than locked in by a test.
Happy to add a regression test that defines an arbitrary role directly in the Casbin policy layer and confirms
RoleListView,RoleUserAPIView, and role assignment/unassignment handle it the same as a built-in role, no production code changes. Let me know if that's the right next step here, or if I'm missing a part of the codebase this issue is meant to cover.BryanttV commentedon Sep 30, 2026
Thanks for the validation, @efortish! In that case, I would like to see some tests included to validate that it currently works, just as you suggested.
efortish commentedon Sep 30, 2026
Hello @BryanttV
PR submitted here: #507