Repository navigation
feat: add authz schema compilation - #476
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. |
9c964ac to
2c77167
Compare
2c77167 to
3ed5882
Compare
3ed5882 to
c2f7048
Compare
d0df6c5 to
126c05e
Compare
mariajgrimaldi
left a comment
There was a problem hiding this comment.
LGTM! Thanks so much for addressing all of my comments :)
| permission_changes_for_role["remove"].append((perm, document.priority, document.source)) | ||
| return permission_changes | ||
|
|
||
| def _resolve_contributions( |
There was a problem hiding this comment.
| def _resolve_contributions( | |
| def _resolve_contributions_based_on_priority( |
| if action == "add": | ||
| extension_sources = [ | ||
| RelationshipSource(src, SchemaOriginKind.EXTENSION, max_priority) for src in winning_sources | ||
| ] | ||
| self._apply_added_permission(role_id, perm, current, provenance, extension_sources) | ||
| else: # remove | ||
| self._apply_removed_permission(role_id, perm, current, provenance) |
There was a problem hiding this comment.
nit:
| if action == "add": | |
| extension_sources = [ | |
| RelationshipSource(src, SchemaOriginKind.EXTENSION, max_priority) for src in winning_sources | |
| ] | |
| self._apply_added_permission(role_id, perm, current, provenance, extension_sources) | |
| else: # remove | |
| self._apply_removed_permission(role_id, perm, current, provenance) | |
| match action: | |
| case ACTION.ADD | |
| case ACTION.REMOVE | |
| ... |
I'd also change all references to the hardcoded "add" to ACTION.ADD and so on.
| actions: dict[str, list[tuple[str, int, SourceRecord]]] = {} | ||
| for perm, priority, src in permission_changes["add"]: | ||
| actions.setdefault(perm, []).append(("add", priority, src)) | ||
| for perm, priority, src in permission_changes["remove"]: | ||
| actions.setdefault(perm, []).append(("remove", priority, src)) |
There was a problem hiding this comment.
Took me a while to understand this so thanks for the inline comment!
Problem
Several distributions can contribute to the same authorization schema, and one can extend a role defined by another. The loaded documents need to be resolved into a single set of definitions with per-contribution provenance, and role extensions merged deterministically (ADR 0018 §1, ADR 0023).
Approach
openedx_authz/engine/schema/compilation.py—SchemaCompileropenedx_authz/tests/schema/test_compilation.pyMerge rules follow ADR 0023: metadata replace, add/remove permissions, tri-state
hidden, andpriorityto resolve conflicts. An unresolvable equal-priority conflict raisesSchemaCompileErrorrather than picking a winner. Compilation is pure — no database, no Casbin.Manual testing instructions
Rollback plan
Revert this PR. Nothing consumes the compiler yet, so the revert is inert.
Retro compatibility
No authorization behavior changes. No models, no migration, no automatic code path.
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 (3/8) — #446 split into reviewable pieces. Bases chain bottom-up; merge in order.
rod/authz-schema-loader-only)rod/authz-schema-validator— schema validationrod/authz-schema-models— definition models + migrationrod/authz-schema-renderer— policy rendererrod/authz-schema-applier— schema applierload_authz_schemacommand, version bump and changelogMerge checklist: