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. |
9f7350f to
d22ebbe
Compare
3a2f951 to
b73023f
Compare
d912d58 to
9ac87a4
Compare
8fa7e9b to
a6abbef
Compare
mariajgrimaldi
left a comment
There was a problem hiding this comment.
Just a few comments for you to review!
| # Namespace prefixes for the internal Casbin form (schema objects never carry them). | ||
| ROLE_PREFIX = "role" | ||
| ACTION_PREFIX = "act" | ||
| SCOPE_WILDCARD = "*" | ||
| ALLOW = "allow" | ||
| POLICY_PTYPE = "p" |
There was a problem hiding this comment.
Don't have this already defined in data.py?
There was a problem hiding this comment.
Not everything was defined in data.py, I refactored it to move missing ones there and have everything in one place. Thanks!
| rows: list[PolicyRow] = field(default_factory=list) | ||
|
|
||
|
|
||
| def policy_row(role_id: str, permission_id: str, scope: str) -> PolicyRow: |
There was a problem hiding this comment.
This looks more than a constructor that should be part of PolicyRow?
There was a problem hiding this comment.
You are right, changed it.
3ef1318 to
11355bb
Compare
dd6858c to
23ec071
Compare
23ec071 to
76bb22a
Compare
mariajgrimaldi
left a comment
There was a problem hiding this comment.
LGTM! Thanks so much :))
| return cls(POLICY_PTYPE, subject, action, scope, effect) | ||
|
|
||
| @classmethod | ||
| def from_grant(cls, role_id: str, permission_id: str, scope: str) -> "PolicyRow": |
There was a problem hiding this comment.
I think we've been calling it assignment
There was a problem hiding this comment.
Assignment would be when we assign a role to a user over a scope. Here we are assigning a permission to a role, I have been using grant to design this since ADR 0025 I think.
These terms are confusing sometimes... What do you think about this idea of using "assignment" for user-role, and "grant" for role-permission?
Problem
A compiled schema carries bare identifiers; the enforcer stores namespaced Casbin
prows. Something has to translate between the two, and it has to be the single place that applies the namespacing — two places would drift, and a later comparison against the stored policy would silently stop matching (ADR 0018 §5).Approach
openedx_authz/engine/renderer.py—PolicyRow,RenderedPolicy,policy_row(),PolicyRendereropenedx_authz/tests/schema/test_renderer.pyrender()emits oneprow per (role, permission, supported scope), applies the internal form (role^,act^,<scope>^*) at this boundary, and is deterministic so the output can be diffed against a stored policy. It emits definition rows only — nevergassignments or legacyg2action inheritance — so data owned by other services is out of reach by construction.The render phase is pure: no Casbin, no Django, no database. The apply phase, which does touch both, is the next PR in the stack.
Manual testing instructions
Rollback plan
Revert this PR. Nothing consumes the renderer yet, so the revert is inert.
Retro compatibility
No authorization behavior changes. Rendering produces rows in memory and writes nothing.
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 (6/8) — #446 split into reviewable pieces. Bases chain bottom-up; merge in order.
rod/authz-schema-models)rod/authz-schema-applier— schema applierload_authz_schemacommand, version bump and changelogMerge checklist: