[FC-0099] docs: add ADR proposing architecture for authz framework#57
Conversation
openedx-webhooks
added
open-source-contribution
|
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. |
mariajgrimaldi
changed the title
mariajgrimaldi
requested review from
BryanttV,
MaferMazu,
bmtcril,
rodmgwgu and
sarina
| - ``id``: A unique identifier for each policy. This depends on the database backend. In our case, MySQL uses an auto-incrementing integer. | ||
| - ``ptype``: The policy type (e.g., ``p`` for policy, ``g, g2`` for grouping). | ||
| - ``v0, v1, v2, v3, v4, v5``: The policy fields that define the subject, action, object, and context. The exact meaning of these fields depends on the policy type and model configuration. | ||
| - Optionally, include additional metadata fields in the policy table to support auditing and versioning. |
There was a problem hiding this comment.
I think these are two different things? We should have as part of this decision what we want to store for auditing and versioning, and it should be non-optional. Whereas metadata fields may be optional?
There was a problem hiding this comment.
We don't have a specific plan for this, but I do agree this should be covered in a separate ADR.
| Maintain Consistent Model and Policy Definitions Across Services | ||
| ---------------------------------------------------------------- | ||
| - Policies should be defined in a way that is consistent across services, using the same naming conventions and structures for subjects, actions, objects, and contexts. For example, if the LMS defines a policy for "viewing a course" the CMS should use the same terminology and structure when defining a similar policy for "viewing content". | ||
| - Each column in the policy table (v0, v1, v2, etc.) should have a consistent meaning across services. For example, for the same type ``v0`` should always represent the subject, ``v1`` the action, ``v2`` the object, and so on. |
There was a problem hiding this comment.
I think it's worth noting that a consequence of this is that if the model.conf changes it will possibly change the meaning of the stored data and that data (at least for dynamic policies) may need to be rebuilt somehow? I'm not even sure if that's possible if the definitions of the dynamic policies assume the default model.conf.
There was a problem hiding this comment.
I think changes in model.conf should be considered breaking. They would also require some form of data migration so the dynamic data can evolve with the model.
For auditing purposes, it may also make sense to version model.conf together with the policies.
|
|
||
| Use the Enforcement API for Authorization Decisions for External Clients | ||
| ------------------------------------------------------------------------ | ||
| - External clients (e.g., MFEs, IDAs, or any service not co-located with the policy store) must use the REST API provided by the Open edX authorization layer to request authorization decisions. |
There was a problem hiding this comment.
Can you be specific about which service is going to host this? I assume it will have to be LMS, but I'd like it to be explicit. Things I've read elsewhere seem to indicate that Studio may have its own store? In which case would external services have to query both? Or would both sets of policies be stored in a single LMS-based store with a single set of REST endpoints?
There was a problem hiding this comment.
I think assuming that the LMS will be the single store is safe enough from both a scalability and security perspective. The majority, if not all, IDAs already communicate with the LMS in some way, so we already have that integration point established. Using the LMS as the store also removes the friction of introducing additional communication paths with other products.
This does not necessarily mean the store could not become a dedicated internal service in the future if that becomes necessary. I would not want to close that direction. But for the time being and the near future, I think it is reasonable to state that the LMS is the service hosting the policy store, and that this is the direction we are currently taking.
|
|
||
| #. **Use of MySQL for Policy Storage**: The choice of MySQL as the backend for the policy store means that all policies will be stored in a relational database, which may have implications for performance and scalability. However, this also allows us to leverage existing database infrastructure and tools for managing and querying policies. If support for other databases is needed in the future, we can explore using other Casbin adapters. | ||
|
|
||
| #. **Roles and Permissions are Stored in the Policy Store**: All roles and permissions will be managed through the policy store, which means that any changes to roles or permissions will need to be made via the Policy Management API or by updating the ``authz.policy`` file. This centralization simplifies management but requires careful handling to avoid conflicts or inconsistencies. |
There was a problem hiding this comment.
I don't suppose there are any built-in safeguards to keep different apps / services from altering the permissions of others?
There was a problem hiding this comment.
We don't currently have any safeguards for services... we could consider using tenancy for filtering though! https://casbin.org/docs/rbac-with-domains/ (not sure how this would look like though)
|
@mariajgrimaldi since this FC is closed, can we close this one? |
|
I think this is still good, and we're hoping to get a little more time to wrap some of these tasks up 🤞 |
Make decisions about: - How the authz framework will interact with the Open edX ecosystem - How the ecosystem will make authorization decisions - How the policies used for access control will be managed and stored at the ecosystem level
* Changes addressing Ty's concerns * Fixed factual mismatches: * Request format: Fixed from 4-field sub, act, obj, scope to actual 3-field sub, act, scope, with explanation of how S-A-O-C maps to it * Policy format: Fixed from p = sub, act, obj, eft to p = sub, act, scope, eft * New section "Authorization Model Configuration": Documents the actual model.conf (request, policy, role definitions, namespace convention, matcher logic) * New section "Scope and Subject Polymorphism": Documents the registry pattern with ContentLibraryScope, CourseScope, UserSubject * API structure: Updated from generic "api.py" to the three actual modules (permissions, roles, users) * Deployment form: Changed from vague "shared library, Django app, or Tutor plugin" to specific "Django app registered as LMS+CMS plugin via entry_points" * PolicyCacheControl: Documented the UUID-based cache invalidation mechanism * Custom matchers: Mentioned is_staff_or_superuser registration * Legacy compatibility: New consequence documenting the legacy_*_role_permissions pattern * Cross-reference added
mariajgrimaldi
force-pushed
the
MJG/authz-architecture-and-data-modeling
branch
from
a42fce0 to
c0cb3c1
Compare
- Correct static policy source: authz.policy file (not constants/) is what gets loaded into the DB via load_policies command - Document dual representation: authz.policy for DB loading, constants/ for programmatic use in migration scripts and views - Fix Casbin adapter URL to officialpycasbin/django-orm-adapter - Note that both representations must be kept in sync (tech debt) Co-Authored-By: Claude Opus 4.6 <[email protected]>
- State LMS as the primary service hosting the policy store, with rationale (existing IDA communication paths) - Fix model.conf note: reference authz.policy for static policies, strengthen language to "breaking" for model changes - Add Casbin domain/tenant filtering as future direction for cross-service policy isolation Co-Authored-By: Claude Opus 4.6 <[email protected]>
github-project-automation
bot
moved this from Waiting on Author
to Done
in Contributions
5 participants
Description
Make decisions about:
Merge checklist:
Check off if complete or not applicable: