Repository navigation
feat: add authz schema pipeline and load_authz_schema command - #446
Conversation
|
Thanks for the pull request, @rodmgwgu! This repository is currently maintained by 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 approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo 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:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere 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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
6750d1c to
4a43021
Compare
2b4993d to
49effcb
Compare
4861927 to
3956e81
Compare
|
I followed the manual testing instructions, migrations ran ok, but this command from Step 1 Dry-run tutor dev run cms ./manage.py cms load_authz_schema --dry-runDoesn't return DetailsRows to add (0):
Rows to remove (0):
Definition changes - category (17):
+ course_access_content
+ course_advanced_certificates
+ course_files
+ course_grading
+ course_import_export
+ course_legacy
+ course_library_updates
+ course_other
+ course_pages_resources
+ course_schedule_details
+ course_tags_taxonomies
+ course_team_group
+ course_updates_handouts
+ library
+ library_collection
+ library_content
+ library_team
Definition changes - permission (48):
+ content_libraries.create_library_collection
+ content_libraries.delete_library
+ content_libraries.delete_library_collection
+ content_libraries.edit_library_collection
+ content_libraries.edit_library_content
+ content_libraries.manage_library_tags
+ content_libraries.manage_library_team
+ content_libraries.publish_library_content
+ content_libraries.reuse_library_content
+ content_libraries.view_library
+ content_libraries.view_library_team
+ courses.create_files
+ courses.delete_files
+ courses.edit_course_content
+ courses.edit_details
+ courses.edit_files
+ courses.edit_grading_settings
+ courses.edit_schedule
+ courses.export_course
+ courses.export_tags
+ courses.import_course
+ courses.legacy_beta_tester_permissions
+ courses.legacy_data_researcher_permissions
+ courses.legacy_instructor_role_permissions
+ courses.legacy_limited_staff_role_permissions
+ courses.legacy_staff_role_permissions
+ courses.manage_advanced_settings
+ courses.manage_certificates
+ courses.manage_course_team
+ courses.manage_course_updates
+ courses.manage_group_configurations
+ courses.manage_library_updates
+ courses.manage_pages_and_resources
+ courses.manage_tags
+ courses.manage_taxonomies
+ courses.publish_course_content
+ courses.view_advanced_settings
+ courses.view_certificates
+ courses.view_checklists
+ courses.view_course
+ courses.view_course_team
+ courses.view_course_updates
+ courses.view_files
+ courses.view_grading_settings
+ courses.view_group_configurations
+ courses.view_library_updates
+ courses.view_pages_and_resources
+ courses.view_schedule_and_details
Definition changes - role (11):
+ course_admin
+ course_auditor
+ course_beta_tester
+ course_data_researcher
+ course_editor
+ course_limited_staff
+ course_staff
+ library_admin
+ library_author
+ library_contributor
+ library_user
Definition changes - role-permission (132):
+ course_admin -> courses.create_files @ course-v1
+ course_admin -> courses.delete_files @ course-v1
+ course_admin -> courses.edit_course_content @ course-v1
+ course_admin -> courses.edit_details @ course-v1
+ course_admin -> courses.edit_files @ course-v1
+ course_admin -> courses.edit_grading_settings @ course-v1
+ course_admin -> courses.edit_schedule @ course-v1
+ course_admin -> courses.export_course @ course-v1
+ course_admin -> courses.export_tags @ course-v1
+ course_admin -> courses.import_course @ course-v1
+ course_admin -> courses.legacy_instructor_role_permissions @ course-v1
+ course_admin -> courses.manage_advanced_settings @ course-v1
+ course_admin -> courses.manage_certificates @ course-v1
+ course_admin -> courses.manage_course_team @ course-v1
+ course_admin -> courses.manage_course_updates @ course-v1
+ course_admin -> courses.manage_group_configurations @ course-v1
+ course_admin -> courses.manage_library_updates @ course-v1
+ course_admin -> courses.manage_pages_and_resources @ course-v1
+ course_admin -> courses.manage_tags @ course-v1
+ course_admin -> courses.manage_taxonomies @ course-v1
+ course_admin -> courses.publish_course_content @ course-v1
+ course_admin -> courses.view_advanced_settings @ course-v1
+ course_admin -> courses.view_certificates @ course-v1
+ course_admin -> courses.view_checklists @ course-v1
+ course_admin -> courses.view_course @ course-v1
+ course_admin -> courses.view_course_team @ course-v1
+ course_admin -> courses.view_course_updates @ course-v1
+ course_admin -> courses.view_files @ course-v1
+ course_admin -> courses.view_grading_settings @ course-v1
+ course_admin -> courses.view_group_configurations @ course-v1
+ course_admin -> courses.view_library_updates @ course-v1
+ course_admin -> courses.view_pages_and_resources @ course-v1
+ course_admin -> courses.view_schedule_and_details @ course-v1
+ course_auditor -> courses.view_advanced_settings @ course-v1
+ course_auditor -> courses.view_certificates @ course-v1
+ course_auditor -> courses.view_checklists @ course-v1
+ course_auditor -> courses.view_course @ course-v1
+ course_auditor -> courses.view_course_team @ course-v1
+ course_auditor -> courses.view_course_updates @ course-v1
+ course_auditor -> courses.view_files @ course-v1
+ course_auditor -> courses.view_grading_settings @ course-v1
+ course_auditor -> courses.view_group_configurations @ course-v1
+ course_auditor -> courses.view_library_updates @ course-v1
+ course_auditor -> courses.view_pages_and_resources @ course-v1
+ course_auditor -> courses.view_schedule_and_details @ course-v1
+ course_beta_tester -> courses.legacy_beta_tester_permissions @ course-v1
+ course_data_researcher -> courses.legacy_data_researcher_permissions @ course-v1
+ course_editor -> courses.create_files @ course-v1
+ course_editor -> courses.edit_course_content @ course-v1
+ course_editor -> courses.edit_details @ course-v1
+ course_editor -> courses.edit_files @ course-v1
+ course_editor -> courses.edit_grading_settings @ course-v1
+ course_editor -> courses.manage_course_updates @ course-v1
+ course_editor -> courses.manage_group_configurations @ course-v1
+ course_editor -> courses.manage_library_updates @ course-v1
+ course_editor -> courses.manage_pages_and_resources @ course-v1
+ course_editor -> courses.manage_tags @ course-v1
+ course_editor -> courses.view_advanced_settings @ course-v1
+ course_editor -> courses.view_certificates @ course-v1
+ course_editor -> courses.view_checklists @ course-v1
+ course_editor -> courses.view_course @ course-v1
+ course_editor -> courses.view_course_team @ course-v1
+ course_editor -> courses.view_course_updates @ course-v1
+ course_editor -> courses.view_files @ course-v1
+ course_editor -> courses.view_grading_settings @ course-v1
+ course_editor -> courses.view_group_configurations @ course-v1
+ course_editor -> courses.view_library_updates @ course-v1
+ course_editor -> courses.view_pages_and_resources @ course-v1
+ course_editor -> courses.view_schedule_and_details @ course-v1
+ course_limited_staff -> courses.legacy_limited_staff_role_permissions @ course-v1
+ course_staff -> courses.create_files @ course-v1
+ course_staff -> courses.delete_files @ course-v1
+ course_staff -> courses.edit_course_content @ course-v1
+ course_staff -> courses.edit_details @ course-v1
+ course_staff -> courses.edit_files @ course-v1
+ course_staff -> courses.edit_grading_settings @ course-v1
+ course_staff -> courses.edit_schedule @ course-v1
+ course_staff -> courses.export_course @ course-v1
+ course_staff -> courses.export_tags @ course-v1
+ course_staff -> courses.import_course @ course-v1
+ course_staff -> courses.legacy_staff_role_permissions @ course-v1
+ course_staff -> courses.manage_advanced_settings @ course-v1
+ course_staff -> courses.manage_certificates @ course-v1
+ course_staff -> courses.manage_course_updates @ course-v1
+ course_staff -> courses.manage_group_configurations @ course-v1
+ course_staff -> courses.manage_library_updates @ course-v1
+ course_staff -> courses.manage_pages_and_resources @ course-v1
+ course_staff -> courses.manage_tags @ course-v1
+ course_staff -> courses.publish_course_content @ course-v1
+ course_staff -> courses.view_advanced_settings @ course-v1
+ course_staff -> courses.view_certificates @ course-v1
+ course_staff -> courses.view_checklists @ course-v1
+ course_staff -> courses.view_course @ course-v1
+ course_staff -> courses.view_course_team @ course-v1
+ course_staff -> courses.view_course_updates @ course-v1
+ course_staff -> courses.view_files @ course-v1
+ course_staff -> courses.view_grading_settings @ course-v1
+ course_staff -> courses.view_group_configurations @ course-v1
+ course_staff -> courses.view_library_updates @ course-v1
+ course_staff -> courses.view_pages_and_resources @ course-v1
+ course_staff -> courses.view_schedule_and_details @ course-v1
+ library_admin -> content_libraries.create_library_collection @ lib
+ library_admin -> content_libraries.delete_library @ lib
+ library_admin -> content_libraries.delete_library_collection @ lib
+ library_admin -> content_libraries.edit_library_collection @ lib
+ library_admin -> content_libraries.edit_library_content @ lib
+ library_admin -> content_libraries.manage_library_tags @ lib
+ library_admin -> content_libraries.manage_library_team @ lib
+ library_admin -> content_libraries.publish_library_content @ lib
+ library_admin -> content_libraries.reuse_library_content @ lib
+ library_admin -> content_libraries.view_library @ lib
+ library_admin -> content_libraries.view_library_team @ lib
+ library_author -> content_libraries.create_library_collection @ lib
+ library_author -> content_libraries.delete_library_collection @ lib
+ library_author -> content_libraries.edit_library_collection @ lib
+ library_author -> content_libraries.edit_library_content @ lib
+ library_author -> content_libraries.manage_library_tags @ lib
+ library_author -> content_libraries.publish_library_content @ lib
+ library_author -> content_libraries.reuse_library_content @ lib
+ library_author -> content_libraries.view_library @ lib
+ library_author -> content_libraries.view_library_team @ lib
+ library_contributor -> content_libraries.create_library_collection @ lib
+ library_contributor -> content_libraries.delete_library_collection @ lib
+ library_contributor -> content_libraries.edit_library_collection @ lib
+ library_contributor -> content_libraries.edit_library_content @ lib
+ library_contributor -> content_libraries.manage_library_tags @ lib
+ library_contributor -> content_libraries.reuse_library_content @ lib
+ library_contributor -> content_libraries.view_library @ lib
+ library_contributor -> content_libraries.view_library_team @ lib
+ library_user -> content_libraries.reuse_library_content @ lib
+ library_user -> content_libraries.view_library @ lib
+ library_user -> content_libraries.view_library_team @ libNext command to query if anything changes do indeed return 0, so everything good there I guess the above output is fine as everything else matches, I'll continue with the rest EDIT: First run of 2026-09-21 16:55:22,112 INFO 1 [openedx_authz.engine.enforcer] [user None] [ip None] enforcer.py:183 - Reloaded policy to version e5c4d849-e0b7-479f-8300-3d1b79d23ce8
2026-09-21 16:55:22,413 INFO 1 [openedx_authz.engine.renderer] [user None] [ip None] renderer.py:645 - Authz schema apply: persisted 11 role(s), 48 permission(s), 17 category(ies).
2026-09-21 16:55:22,416 INFO 1 [openedx_authz.engine.renderer] [user None] [ip None] renderer.py:334 - Authz schema apply: policy rows unchanged; definitions synced.
Authz schema applied: 0 row(s) added, 0 removed.Second run of Seems like its only a minor problem with the reported updated rows? |
|
Opinion: I think it'll be nice if the command that applies the changes: tutor dev run cms ./manage.py cms load_authz_schemaAlso prints the changes in the same way as when passing |
3956e81 to
c75eccb
Compare
Thanks for validating, in this case, it says 0 for both rows to add and to remove because it refers to the casbin p rows, which already exist because of the old policy which has the exact same equivalent rows as the ones generated by the new schema. But this does look confusing, I'll change the text to better communicate this. |
| ******************* | ||
|
|
||
| Changed | ||
| ======= |
There was a problem hiding this comment.
Removed by accident?
There was a problem hiding this comment.
yes, fixed, thanks!
3276379 to
7a03353
Compare
|
[non-blocking] Could we reduce or consolidate the distribution ownership warnings? Nearly every command in the Tutor acceptance run printed this several times: The repeated warnings made the actual schema report difficult to find. Could we print this once and include which distribution was selected, or lower the log level when the fallback is safe? |
fdbe286 to
18285dc
Compare
mariajgrimaldi
left a comment
There was a problem hiding this comment.
I don't think I have any more comments. I ran the schema pipeline acceptance tests and left the test report here. Both failed cases and the other observations are covered in my review comments. I'm approving this. Thanks so much :)
d79f026 to
bca4901
Compare
|
@mariajgrimaldi FYI, created an issue to track the decision on the priority for base definitions: #525 |
9e5037c to
dd83db9
Compare
dd83db9 to
1e19c31
Compare
BryanttV
left a comment
There was a problem hiding this comment.
I tested the command locally and it works great! Thank you so much for all your work on this! 🚀
Closes #410
Closes #433
Problem
The schema phases now exist independently but nothing runs them. Deployment needs one non-interactive entry point that drives discover → load → validate → compile → render → apply, reports what would change, and fails before writing anything if the schema is invalid (ADR 0018).
Approach
The last slice of the pipeline, on top of #480.
openedx_authz/engine/schema/pipeline.py—SchemaPipeline, wiring the phases in orderopenedx_authz/management/commands/load_authz_schema.py— the deployment entry point:--dry-runto print the change report without writing,--forceto allow removing roles that still have assignments, repeatable--dirfor CI and local runsopenedx_authz/tests/schema/{test_pipeline,test_load_authz_schema_command}.pyManual testing instructions
Run in a tutor dev environment with this repo mounted. Every command below is shown for
cms;lmsworks identically. Apply the migration first:1. Dry run reports the change without writing
Expect
Casbin policy rows to add (0) Casbin policy rows to remove (0), and definition sections for 17 categories, 48 permissions, 11 roles, and 132 role-permission grants. Confirm nothing was written:Please note: 0 Casbin policy changes is expected — the schema definitions match the policies already defined in the existing
authz.policyfile, which is auto-imported on launch.2. Apply, then confirm it is idempotent
The first run prints the definition changes and
Authz schema applied: 0 Casbin policy row(s) added, 0 removed; definition changes: 17 category, 48 permission, 11 role, 132 role-permission.. The second printsAuthz schema unchanged; no rows written.— no duplicate rows, no churn (ADR 0018 §2).3. A role extension changes an existing role without copying it (ADR 0023)
Expect exactly one row added (
courses.export_course), one removed (courses.manage_tags), and~ course_editorunder role definition changes. Clean up and restore:4. Removing an assigned role is blocked without
--force(ADR 0018 §6)Assign a static role to a test user, comment that role out of
openedx_authz/authz/schema/course_roles.yaml, and apply. Expect aCommandErrornaming the role and the assigned subject, ending with "Re-run with force to remove them together with their assignments", and no database change. Re-run with--forceand the role, itsprows, its assignment and its definition rows are gone, with one audit record per removed assignment.5. An invalid schema stops deployment before any write
Introduce an error in a schema file — point a role at a permission that does not exist, or give a permission a CamelCase namespace — and run the command. It exits with a
CommandError, the log lists every validation error with its source file, and the database is untouched.Automated coverage
openedx_authz/tests/schema/covers the full discover → load → validate → compile → render → plan/apply path.openedx_authz/tests/integration/test_schema_apply.pyexercises the real Casbin enforcer and ORM; it is excluded from the default pytest run and needs an edx-platform environment:tutor dev run cms pytest -p no:randomly --create-db --ds=cms.envs.test \ /mnt/openedx-authz/openedx_authz/tests/integration/test_schema_apply.pyRollback plan
Revert this PR to remove the command and the pipeline wiring; the phases from PRs 1–7 remain but nothing invokes them. The migration in #478 is additive; roll back by reverting and running the reverse migration if applied.
Retro compatibility
No authorization behavior changed. Existing loading of
authz.policyworks as before. The new policy loading is never automatic — it runs only whenload_authz_schemais invoked.Follow up work required
authz.policyfile — remove role and permission definitions already in the new schema, keeping theg2definitions there for now (Deprecate authz.policy file #462)AI Usage
Kiro was used to assist on feature planning and implementation. Implementation was done step by step with human guidance and validation, based on the ADRs.
Stack (8/8) — this PR was split into eight reviewable pieces. Bases chain bottom-up; merge in order.
load_authz_schemacommand (base:rod/authz-schema-applier)Merge checklist: