-
Notifications
You must be signed in to change notification settings - Fork 5
[FC-0099] docs: ADR proposing authorization foundation for openedx ecosystem #31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for the pull request, @mariajgrimaldi! 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. |
41fd708 to
9ad53ab
Compare
|
|
||
| Use proven frameworks with ABAC support and an adapter | ||
| ------------------------------------------------------ | ||
| * Use existing open source frameworks (Casbin, Cerbos, OpenFGA, SpiceDB). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ory Keto is another popular open source Zanzibar clone worth mentioning, especially as it integrates well with the other Ory suite systems like Hydra that are all very nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! Thanks for bringing it up. I added it to the list for completeness. We looked into it after you mentioned it since it looked very promising, but the red flag that was hard to reconcile was the lack of real use cases. Besides a couple of examples, there wasn’t much to justify bringing in a graph-like database and a separate centralized service. The complexity of implementing it in the ecosystem also didn’t feel right, and I think we should have more clear integration between services before committing to these kinds of technologies.
We also looked at OpenFGA, and I think the comparison would have been similar to what we might have seen with Ory Keto https://openedx.atlassian.net/wiki/spaces/OEPM/pages/5179179033/AuthZ+Technologies+Comparison. And just like with SpiceDB, you can see that PostgreSQL is the best supported backend, while MySQL and other options don’t get the same level of attention, probably because of the type of checks these systems perform.
MaferMazu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still need to review some parts, but this is what I got so far.
sarina
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved pending resolution of @MaferMazu 's comments.
1bcf69d to
8b92586
Compare
| * A **scope** defines the boundary within which a role or policy applies (for example: platform-wide, organization-wide, a single course, or a specific library). | ||
| * Treating scopes as **first-class citizens** means they are explicitly modeled in the system, not hidden inside ad-hoc resource definitions. They must be available to policies, queries, and audits in a consistent way. | ||
| * Scopes can be **parameterized** (e.g., ``organization:ORG-A``, ``course:course-v1:OpenedX+DemoX+DemoCourse``, ``site:sandbox.openedx.org``, ``instance``) to support granular checks. | ||
| * **Inheritance across scopes** must be supported (e.g., permissions granted at the organization level can cascade to courses in that organization when intended). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you clarify what kind of inheritance you have in mind here? I'm not sure the current design supports something like Can Use Studio on organization:ORG-A implicitly grants access to course:course-v1:ORG-A+DemoX+DemoCourse. I think it would need to be modeled as a logical OR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What we're aiming for is more of resource grouping, more like course:course-v1:ORG-A+DemoX+DemoCourse belongs to org ORG-A, kind of like a container of. What do you think might the current design might be missing to support this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we're saying the same thing in different ways. Do you think the resource grouping you're talking about would be handled in the OeX AuthZ layer? For instance that layer explicitly knows about the relationship of courses and orgs and automatically handles the logical OR of "Can edit in Studio on Course X OR org A"? I suppose this would also require that the names of the permissions ("Can edit in Studio") be the same for both the course and the org too, or have en even more complicated mapping.
Or would it be the job of the person writing the authz check to remember to include both where applicable? For instance if the goal was to allow course admins for each course course, org admins for orgs that own the course, and site admins to be able to edit courses the person writing the check would have to write those 3 checks separately as something like can(user, course, 'edit-in-studio') OR can(user, org, 'edit-in-studio') OR can(user, instance, 'edit-in-studio').
I think the current plan can easily handle either of these cases, but would like to know where that kind of inheritance is expected to live.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand better what you meant, thanks for the extra context!
At first I was thinking about this as a resource grouping problem, since many authorization technologies handle that out of the box with parent->child relationships. But I think it makes more sense, as you said, to treat this as containment of decisions, rather than trying to model resource groupings. That way the AuthZ layer handles containment explicitly, while Casbin stays agnostic of these groupings.
From there, the question is the one you pointed out: whether containment should be derived implicitly by the AuthZ layer, or kept explicit by the check writer. If we centralize derivation, we reduce the chance of missing a case, but it hides logic and makes checks less explicit. If we leave it to the writer, it's more deliberate and auditable, but also easier to miss. In that case, it’s like flattening out the containment tree of decisions - the writer writes each relationship instead of relying on hidden derivation.
Even with that trade-off, I lean toward explicit checks. It keeps the logic visible and aligns with the principle of being as explicit as possible, rather than managing it implicitly behind the scenes.
And as far of the permissions naming convention, I think we'd have to manage the same permission but at different scope levels which I think will have implications from an auditing point of view so we'd need the entire context to actually understand what's been asked: can(user, org, 'create-library') OR can(user, instance, 'create-library') -> not sure if this makes much sense but consider it only an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I think we're on the same page! I agree that we should keep the checks explicit, with the possible exception of a "safety hatch" for superusers to prevent everyone from being locked out of something if permissions get messed up.
|
@MaferMazu @bmtcril - can we finish up review of this ADR so we can move on to new work? |
Make decisions about: 1. Permission model to use to adopt an universal language 2. Make scope first class citizens 3. Paradigm to use 4. Policy usage 5. Centralize enforcement 6. Integrations 7. Extensibility 8. Security The goal is to build a system that follows these decisions to successfully address our current restrictions.
Co-authored-by: María Fernanda Magallanes <35668326+MaferMazu@users.noreply.github.com>
50784d7 to
b8081aa
Compare
|
@MaferMazu: friendly reminder to reply to my comments. If your review is non-blocking, please let me know! |
MaferMazu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Thanks @mariajgrimaldi
Description
Make decisions about:
The goal is to build a system that follows these decisions to successfully address our current restrictions.