Repository navigation
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!
f5ad8fe to
ce37ce4
Compare
|
Awesome work, thanks, @rodmgwgu ! |
9ffc961 to
cb95be0
Compare
23c2d94 to
66886b4
Compare
44f0866 to
02a2dad
Compare
Add the load_authz_schema management command to the CMS and LMS init scripts, right after load_policies within the existing openedx-authz guard block. load_policies populates the Casbin policy rows that the authorization engine uses for permission checks, but those rows carry no display metadata — no role names, descriptions, permission categories, or source tracking. load_authz_schema fills that gap: it discovers static authz schema YAML files contributed by installed applications, validates and compiles them, and writes the resulting role and permission definitions to dedicated tables. The API then serves those definitions so frontends like the Admin Console can display roles and permissions. The command is idempotent — running it twice with the same schema produces no duplicate rows and preserves dynamic roles and user assignments. It must run after migrations (it needs the DB schema) and before the application serves traffic. See also: - openedx/openedx-authz#446 - https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0021-authorization-definition-api.rst
Add the load_authz_schema management command to the CMS and LMS init scripts, right after load_policies within the existing openedx-authz guard block. load_policies populates the Casbin policy rows that the authorization engine uses for permission checks, but those rows carry no display metadata — no role names, descriptions, permission categories, or source tracking. load_authz_schema fills that gap: it discovers static authz schema YAML files contributed by installed applications, validates and compiles them, and writes the resulting role and permission definitions to dedicated tables. The API then serves those definitions so frontends like the Admin Console can display roles and permissions. The command is idempotent — running it twice with the same schema produces no duplicate rows and preserves dynamic roles and user assignments. It must run after migrations (it needs the DB schema) and before the application serves traffic. See also: - openedx/openedx-authz#446 - https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0021-authorization-definition-api.rst
02a2dad to
3276379
Compare
mariajgrimaldi
left a comment
There was a problem hiding this comment.
I haven't tested this, will do so soon!
| self._gate(self._validator.validate(documents)) | ||
| schema = self._compiler.compile(documents) | ||
| self._gate(self._validator.validate_compiled(schema)) |
There was a problem hiding this comment.
What would happen if two commands with different schemas (let's say one it's out of date) are executed at the same time? I guess the atomic in the previous PR would take care of that?
There was a problem hiding this comment.
Yes, the previous PR ensures that applying the actual change is atomic, and if two commands run at the same time, the last one finishing will win.
However there is nothing that would prevent this from happening, but given how this command is meant to be run (on deployment, usually via tutor), I don't see this happening easily.
What do you think?
There was a problem hiding this comment.
I see it more as two operators running the command from different shells at the same time, rather than necessarily two deployments, since the command can also be executed manually.
I don't think we need to prevent that from happening, but we should at least make it visible when another execution is already in progress.
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? |
| def _report_plan(self, plan, *, applied: bool) -> None: | ||
| """Print the change report (ADR 0018 §6). | ||
|
|
||
| Covers the definition tables (roles, permissions, categories, and | ||
| role-permission grants) as well as the Casbin ``p`` policy rows. The two | ||
| are reported separately because they are distinct layers: apply syncs the | ||
| definition metadata even when no ``p`` row changes, so a metadata-only | ||
| edit is a real change the operator needs to see. ``applied`` only changes | ||
| the verb tense in the section headers (past tense once written). | ||
| """ | ||
| if plan.unchanged: | ||
| self.stdout.write(self.style.SUCCESS("Authz schema unchanged; nothing would be written.")) | ||
| return | ||
|
|
||
| added_label = "added" if applied else "to add" | ||
| removed_label = "removed" if applied else "to remove" | ||
|
|
||
| self.stdout.write(f"Casbin policy rows {added_label} ({len(plan.added_rows)}):") | ||
| for row in plan.added_rows: | ||
| self.stdout.write(f" + {row.as_policy()}") | ||
|
|
||
| self.stdout.write(f"Casbin policy rows {removed_label} ({len(plan.removed_rows)}):") | ||
| for row in plan.removed_rows: | ||
| self.stdout.write(f" - {row.as_policy()}") | ||
|
|
||
| self._report_definitions(plan) | ||
|
|
||
| if plan.blocking_assignments: | ||
| self.stdout.write( | ||
| self.style.WARNING( | ||
| f"{len(plan.blocking_assignments)} role(s) with existing assignments would be " | ||
| "removed; apply requires --force:" | ||
| ) | ||
| ) | ||
| for role, subject in plan.blocking_assignments: | ||
| self.stdout.write(f" ! {role} assigned to {subject}") |
There was a problem hiding this comment.
[non-blocking] Could we format the change report as one compact, colorized diff? The current output separates additions and removals, so related changes are difficult to scan.
Something like this would be easier to review:
Casbin policies (2)
+ p, role^course_editor, act^courses.view_course, course-v1^*, allow
- p, role^old_editor, act^courses.manage_tags, course-v1^*, allow
Definitions (2)
+ permission: courses.view_course
~ role: course_editor
The +, ~, and - markers would still work without color. We could also omit empty sections.
7a03353 to
fdbe286
Compare
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: