-
Notifications
You must be signed in to change notification settings - Fork 9
docs: define the static authorization schema #421
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
Changes from all commits
ff64cae
78a973f
932025f
917b782
342a6c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,161 @@ | ||
| 0017: Define Static Roles and Permissions in an Authorization Schema | ||
| #################################################################### | ||
|
|
||
| Status | ||
| ****** | ||
|
|
||
| **Draft** | ||
|
|
||
| Context | ||
| ******* | ||
|
|
||
| `ADR 0016`_ defines static roles in the authz schema and dynamic roles in the authz model, which the API manages through the Casbin adapter. Today, however, static roles and permissions are spread across Python constants, Python role mappings, ``authz.policy``, and frontend code. A developer who adds a role or permission must therefore update several representations, including a separate copy of its display information in the frontend. | ||
|
|
||
| Casbin remains the authorization engine, and its adapter continues to read and write policy rows. The authz schema adds the Open edX format that Casbin does not provide, allowing an application to declare stable identifiers, supported scopes, role permissions, and display information in one place. During deployment, the schema is compiled into Casbin rows, while the API makes the display information available to the frontend. | ||
|
|
||
| Decision | ||
| ******** | ||
|
|
||
| 1. Schema format and boundary | ||
| ============================= | ||
|
|
||
| The authz schema is a versioned YAML format for static permissions, permission categories, roles, and changes to existing roles. Every file declares ``schema_version`` and ``priority``. Open edX publishes a YAML Schema for this format so that editors, CI, and the compiler all apply the same field and validation rules. | ||
|
|
||
| The existing static role and permission definitions in Python modules and ``authz.policy`` will move into the schema. Once this migration is complete, the schema becomes the source for static definitions, so developers add a new role or permission there without duplicating it in Python constants or policy files. | ||
|
|
||
| 2. Permissions and categories | ||
| ============================= | ||
|
|
||
| A permission contains: | ||
|
|
||
| * ``namespace`` and ``name``, which form the stable identifier used by application checks, such as ``courses.view_course``; | ||
| * the scope namespaces where it can apply; | ||
| * a normalized permission category; and | ||
| * ``display_name``, ``description``, and an optional Paragon icon name. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the Paragon icon is optional, what would be shown by default? Is there currently an easy way to validate if a Paragon icon exists? Since it is mentioned as part of the initial validation.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can make some kind of request to render it (not sure how easy or out-of-the-box). I was thinking we could do a simple check for the name following paragon conventions. What do you think? |
||
|
|
||
| For example: | ||
|
|
||
| .. code-block:: yaml | ||
|
|
||
| permission_categories: | ||
| - id: course_content | ||
| display_name: Course content | ||
| description: Permissions for viewing and editing course content. | ||
| icon: Article | ||
|
|
||
| permissions: | ||
| - namespace: courses | ||
| name: view_course | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we specify a naming convention for namespaces, names and permission_categories id? Like ascii lower snake case only?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. YES! I'm working on a reference file
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here it is, I think we could handle the details there once we settle on this ADR: #431 |
||
| display_name: View course | ||
| description: View course configuration and content. | ||
| category: course_content | ||
| scopes: [course-v1] | ||
| icon: Visibility | ||
| - namespace: courses | ||
| name: delete_course | ||
| display_name: Delete course | ||
| description: Delete a course. | ||
| category: course_content | ||
| scopes: [course-v1] | ||
| icon: Delete | ||
|
|
||
| Here, ``course_content`` groups the two permissions for display. The complete permission IDs are ``courses.view_course`` and ``courses.delete_course``, while ``course-v1`` is the scope namespace where they apply. Application code uses the complete permission ID, so changing ``display_name`` does not change permission checks. | ||
|
|
||
| 3. Roles and role extensions | ||
| ============================ | ||
|
|
||
| A role contains a stable identifier, display name, description, supported scope namespaces, and a list of complete permission identifiers. When an application needs to change an existing role, it uses ``role_extensions``. An extension may add or remove permissions and may replace the role's display name or description. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we clearly define the properties that
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opened a different ADR for this: #430
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Excellent! Could we include a reference to that ADR here? |
||
|
|
||
| Version 1 does not define display order for roles, permissions, or categories. Clients may sort them alphabetically or apply another order that suits their interface. | ||
|
|
||
| .. code-block:: yaml | ||
|
|
||
| schema_version: "1.0" | ||
| priority: 100 | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to explain a bit about how
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I do it in the validation and conflicts section, let me know if it's not clear enough: https://github.com/openedx/openedx-authz/pull/421/changes#diff-323a833929851b16109620a7eee56dfe642c78e8c26d00a06cc6c08f321182ceR115 |
||
| roles: | ||
| - id: course_observer | ||
| display_name: Course observer | ||
| description: Can review a course without changing it. | ||
| scopes: [course-v1] | ||
| permissions: | ||
| - courses.view_course | ||
|
|
||
| role_extensions: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea, I like it. |
||
| - role: course_admin | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can an extension be added to a role that doesn't exist? Or is this included in the initial validation?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be included in the validation
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| add_permissions: | ||
| - courses.delete_course | ||
|
|
||
| In these examples, ``courses.view_course`` appears under ``course_content`` in the UI and belongs to the new ``course_observer`` role. The ``role_extensions`` entry adds ``courses.delete_course`` to the existing ``course_admin`` role. The compiler can render the observer's role-permission relationship as: | ||
|
|
||
| .. code-block:: text | ||
|
|
||
| p, role^course_observer, act^courses.view_course, course-v1^*, allow | ||
|
|
||
| The compiler also creates a row that links ``course_admin`` to ``courses.delete_course``. Priority resolves conflicts between definitions and role extensions; it does not control how roles or permissions appear in the UI. | ||
|
|
||
| The schema contains static definitions, while user assignments and user-defined roles stay in the application database. Casbin's ``model.conf`` and matcher also remain owned by ``openedx-authz``. | ||
|
|
||
| 4. Validation and conflicts | ||
| =========================== | ||
|
|
||
| Validation first checks each file against the published schema. It rejects: | ||
|
|
||
| * invalid YAML, unknown fields, missing required fields, and values with the wrong type; | ||
| * unsupported schema versions; | ||
| * IDs that contain uppercase letters or unsupported punctuation; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we enforce also snake case naming convention?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! |
||
| * icon names that don't follow the available icons from ``@openedx/paragon/icons``; and | ||
| * display fields that exceed the agreed size limits. | ||
|
|
||
| After loading every file, validation checks the combined definitions. It rejects: | ||
|
|
||
| * references to permissions or categories that do not exist; | ||
| * a role extension whose role does not exist in the combined definitions; | ||
| * a role used in a scope where one of its permissions cannot apply; | ||
| * conflicting definitions or role extensions with the same priority; and | ||
| * unsupported combinations of schema versions. | ||
|
|
||
| The validator warns about duplicate definitions, including identical definitions, and about definitions or role extensions that do not take effect because another file has a higher priority. The highest-priority contribution resolves a conflict. If several files add the same permission to the same role, the compiler creates one Casbin row and records each contributing source. | ||
|
|
||
| Unit tests can load schema fixtures into an in-memory Casbin enforcer and check allowed and denied requests. This follows the model-testing approach used by OpenFGA and the schema assertions used by SpiceDB, while keeping the tests in normal application test suites. | ||
|
|
||
| Consequences | ||
| ************ | ||
|
|
||
| * Applications declare static permission identifiers, role-to-permission assignments, and UI fields in the same YAML format. | ||
| * Application checks continue to use stable permission identifiers and do not depend on role names. | ||
| * Application clients can read the compiled definitions from the API and remove its copy. | ||
| * Schema validation needs both per-file checks and checks across all files, including conflicts and role extensions. | ||
| * Role extensions can change the permissions assigned to built-in roles. Deployment must report them clearly, and the operator remains responsible for approving them. | ||
| * Version 1 excludes permission implication and role inheritance. | ||
|
|
||
| Rejected Alternatives | ||
| ********************* | ||
|
|
||
| Raw Casbin policy files | ||
| ======================= | ||
|
|
||
| They express the rows Casbin needs for permission checks but omit the display fields and source information required by users and applications. | ||
|
|
||
| Python constants and role mappings | ||
| ================================== | ||
|
|
||
| Definitions would remain split across backend and frontend code. Applications would also need to change ``openedx-authz`` to add definitions they own. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens to the current role and permission definitions in the Python modules? Will we keep using them? If I want to create a new permission, do I have to add it to the YAML and the Python constants file? Or will we do something different?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'd need to entirely migrate to a configuration file to avoid having definitions scattered across multiple places
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made some changes: 917b782 |
||
|
|
||
| Embedded categories on every permission | ||
| ======================================= | ||
|
|
||
| Repeating category labels makes localization and consistent presentation harder. A normalized category gives permissions one stable grouping reference. | ||
|
|
||
| References | ||
| ********** | ||
|
|
||
| * `ADR 0016`_ | ||
| * `Casbin adapters`_ | ||
| * `ASDF YAML Schema`_ | ||
| * `Paragon icons`_ | ||
|
|
||
| .. _ADR 0016: 0016-static-and-dynamic-roles.rst | ||
| .. _Casbin adapters: https://v3.casbin.org/docs/adapters | ||
| .. _ASDF YAML Schema: https://www.asdf-format.org/projects/asdf-standard/en/1.0.2/schemas/yaml_schema.html | ||
| .. _Paragon icons: https://paragon-openedx.netlify.app/components/icon/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,5 @@ | ||
| References | ||
| ########## | ||
|
|
||
| .. toctree:: | ||
| :maxdepth: 1 |
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.
Should the permissions schema also have a
versionandpriority?Uh oh!
There was an error while loading. Please reload this page.
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'd have to look into this in more detail to weigh between the two options. Here's a brain dump:
What kind of use cases can we solve by adding priority to permissions themselves?