feat: add filesystem-path schema discovery and multi-document YAML loading by carlos-marquez-wgu · Pull Request #534 · openedx/openedx-authz

carlos-marquez-wgu · 2026-10-14T17:36:56Z

Description

Implements ADR 0030: Tutor integration for authorization schema discovery.

Add OPENEDX_AUTHZ_SCHEMA_FILES Django setting for discovering schema files by absolute filesystem path (discovery.py)
Introduce DiscoveredFileResource and SourceFileRecord with sentinel values for non-package sources
Switch YAML parsing from safe_load to safe_load_all so each schema resource is treated as a multi-document stream (loading.py)
Add tests for filesystem discovery, multi-document streams, and filesystem source identity
Add tutor plugin to load Authorization schemas contributed through the openedx-authz-schema patch. Most of it is the output of cookiecutter, leaving it as is for future tutor related changes regarding openedx-authz

Testing instructions

Install tutor-contrib-authz plugin

pip install -e tutor/tutor-contrib-authz

Enable it

tutor plugins enable authz

Create a tutor patch to contribute to the authz schema

touch $(tutor config printroot)-plugins/my_authz_schema.py

Add the schema contents to my_authz_schema.py

from tutor import hooks

hooks.Filters.ENV_PATCHES.add_item(
    (
        "openedx-authz-schema",
        """
schema_version: "1.0"
priority: 100

role_extensions:
- role_id: course_editor
  add_permissions:
    - courses.export_course
  remove_permissions:
    - courses.manage_tags
  display_name: Course author
  description: Creates and exports course content for this site.
        """,
    )
)

Enable your patch

tutor plugins enable my_authz_schema

Run the management command to apply the authz schema

tutor dev run cms ./manage.py cms load_authz_schema

Console should output the newly applied schema:

Authz schema apply: added 1 p row(s), removed 1 p row(s).

Changes to apply:

Casbin policies (2)
  + p, role^course_editor, act^courses.export_course, course-v1^*, allow
  - p, role^course_editor, act^courses.manage_tags, course-v1^*, allow

Definitions (3)
  ~ role: course_editor
  + role-permission: course_editor -> courses.export_course @ course-v1
  - role-permission: course_editor -> courses.manage_tags @ course-v1

Authz schema applied: 1 Casbin policy row(s) added, 1 removed; definition changes: 1 role, 2 role-permission.

To roll back the schema changes: disable the patch and run the management command again

tutor plugins disable my_authz_schema
tutor dev run cms ./manage.py cms load_authz_schema

Merge checklist:
Check off if complete or not applicable:

Documentation updated (not only docstrings)
Fixup commits are squashed away
Unit tests added/updated
Manual testing instructions provided
Noted any: Concerns, dependencies, migration issues, deadlines, tickets

Concerns

Resolved Concern: module as null causing problems

Update: This concern was resolved, veredic was to use the file absolute filepath as module instead of null to avoid further problems.

As per the conversation in this PR, we are taking the approach of setting module as null for file system contributions, what I noted while implementing the changes is that given the table openedx_authz_authzschemasource has this unique constraint:

UNIQUE KEY `authz_source_dist_module_uniq` (`distribution`,`module`)

And for file system contributions, the identifying source is always (<tutor-patch>, NULL), which means all file system contributions collapse into a single source row. MySQL treats NULL != NULL in unique constraints, so the constraint does not prevent duplicate (<tutor-patch>, NULL) rows. The renderer cache handles the single-row case correctly and doesn't break, but if for any reason we get two or more rows with (<tutor-patch>, NULL) (concurrent deployments or something else), renderer.py#L563 will throw a MultipleObjectsReturned error.

Additionally, the single-row constraint means per-file provenance is lost — if OPENEDX_AUTHZ_SCHEMA_FILES lists multiple files, you can't distinguish which file contributed which definition at the database level.

Traceback (most recent call last):
  File "/openedx/edx-platform/./manage.py", line 93, in <module>
    execute_from_command_line([sys.argv[0]] + django_args)
  File "/openedx/venv/lib/python3.12/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "/openedx/venv/lib/python3.12/site-packages/django/core/management/__init__.py", line 436, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/openedx/venv/lib/python3.12/site-packages/django/core/management/base.py", line 420, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/openedx/venv/lib/python3.12/site-packages/django/core/management/base.py", line 464, in execute
    output = self.handle(*args, **options)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/mnt/openedx-authz/src/openedx_authz/management/commands/load_authz_schema.py", line 80, in handle
    result = pipeline.apply(force=options.get("force", False))
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/mnt/openedx-authz/src/openedx_authz/engine/schema/pipeline.py", line 110, in apply
    return self._applier.apply(rendered, schema, force=force)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/mnt/openedx-authz/src/openedx_authz/engine/renderer.py", line 283, in apply
    self._store_sources(schema)
  File "/mnt/openedx-authz/src/openedx_authz/engine/renderer.py", line 632, in _store_sources
    role=obj, source=source_obj(record), defaults={"origin_kind": schema_models.OriginKind.BASE}
                     ^^^^^^^^^^^^^^^^^^
  File "/mnt/openedx-authz/src/openedx_authz/engine/renderer.py", line 563, in source_obj
    obj, _ = schema_models.AuthzSchemaSource.objects.update_or_create(
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/openedx/venv/lib/python3.12/site-packages/django/db/models/manager.py", line 87, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/openedx/venv/lib/python3.12/site-packages/django/db/models/query.py", line 990, in update_or_create
    obj, created = self.select_for_update().get_or_create(
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/openedx/venv/lib/python3.12/site-packages/django/db/models/query.py", line 948, in get_or_create
    return self.get(**kwargs), False
           ^^^^^^^^^^^^^^^^^^
  File "/openedx/venv/lib/python3.12/site-packages/django/db/models/query.py", line 638, in get
    raise self.model.MultipleObjectsReturned(
openedx_authz.models.schema.AuthzSchemaSource.MultipleObjectsReturned: get() returned more than one AuthzSchemaSource -- it returned 3!

It is probably unlikely to hit this path, but I wanted to note it as an existing risk, I can add additional code to handle that situation gracefully but wanted to bring this on before I keep going through the rabbit hole (what are we going to do if we hit that situation?)

Alternatively, if it's acceptable, I wanted to bring the other option we discussed in the linked PR comment, and that one being using the file system absolute path as the module, I understand this breaks the pattern of module = python module, but we are already breaking a convention by allowing distribution being just a sentinel value. With the sentinel value approach, this is clearer (just drop the last commit in this PR).

I went ahead and implemented it with module = null in mind, but wanted to bring this up so we can choose to take one trade off or another (more solutions are welcome, thanks!)

AI Usage

Kiro + Claude were used to assist with the creation of the tests and code implementations while thoroughly and carefully guided, everything was reviewed and corrected manually by an actual person and ensured the changes were up to standard and met the closing issue requirements.

Closes:

Dependencies

This PR has a dependency on the changes introduced by #542, this PR needs to be merged after #542 is merged to main, so for the time being this PR points to #542 Branch, and will rebased and updated once the other PR is merged

openedx-webhooks · 2026-10-14T17:37:05Z

Thanks for the pull request, @carlos-marquez-wgu!

This repository is currently maintained by @openedx/committers-openedx-authz.

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 approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To 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:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where 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:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

carlos-marquez-wgu · 2026-10-20T20:43:30Z

Hi @openedx/committers-openedx-authz, this PR is ready for reviews

BryanttV

Hi @carlos-marquez-wgu, thanks for the work here! I tested it locally by adding a tutor plugin with new categories, permissions, and roles, as well as extending the existing roles, and it worked as expected.

Regarding your concern, I think the best approach is to go with your initial proposal and use the filesystem absolute path as a module. What do you think, @rodmgwgu?

Also, regarding the PR instructions, from what I see in the tutor-contrib-authz plugin, there is no init task registered, so tutor dev do init wouldn't have any effect at the moment.

BryanttV · 2026-10-22T17:36:06Z

Hi @carlos-marquez-wgu, I was reviewing this PR: #542, and I think we should base these changes on that branch, otherwise, there will be conflicts. Can we update the PR? cc @rodmgwgu

…ading Implement ADR 0029: Tutor integration for authorization schema discovery. - Add OPENEDX_AUTHZ_SCHEMA_FILES Django setting for discovering schema files by absolute filesystem path (discovery.py) - Introduce DiscoveredFileResource and SourceFileRecord with sentinel values for non-package sources - Switch YAML parsing from safe_load to safe_load_all so each schema resource is treated as a multi-document stream (loading.py) - Add tests for filesystem discovery, multi-document streams, and filesystem source identity - Add tutor plugin

carlos-marquez-wgu · 2026-10-23T18:17:16Z

Hi @carlos-marquez-wgu, I was reviewing this PR: #542, and I think we should base these changes on that branch, otherwise, there will be conflicts. Can we update the PR? cc @rodmgwgu

Thanks @BryanttV , I just rebased my branch on top of PR#542, I updated this PR as well to point to that branch provisionally (so the diff shows only my changes), I'll move it / rebase it again against main once PR#542 is merged

carlos-marquez-wgu · 2026-10-23T18:24:17Z

Hi @carlos-marquez-wgu, thanks for the work here! I tested it locally by adding a tutor plugin with new categories, permissions, and roles, as well as extending the existing roles, and it worked as expected.

Regarding your concern, I think the best approach is to go with your initial proposal and use the filesystem absolute path as a module. What do you think, @rodmgwgu?

Sounds good, @rodmgwgu let me know if you have any comments here, if we are good I can just drop the 2nd commit of this PR to use the initial proposal

Also, regarding the PR instructions, from what I see in the tutor-contrib-authz plugin, there is no init task registered, so tutor dev do init wouldn't have any effect at the moment.

Ah, you are right about the do init command, I forget we need this PR merged first overhangio/tutor#1496, I just removed the step in the testing instructions, is better / more direct to just use the management command, thanks!

rodmgwgu · 2026-10-23T22:20:19Z

Hi @carlos-marquez-wgu, thanks for the work here! I tested it locally by adding a tutor plugin with new categories, permissions, and roles, as well as extending the existing roles, and it worked as expected.
Regarding your concern, I think the best approach is to go with your initial proposal and use the filesystem absolute path as a module. What do you think, @rodmgwgu?

Sounds good, @rodmgwgu let me know if you have any comments here, if we are good I can just drop the 2nd commit of this PR to use the initial proposal

Also, regarding the PR instructions, from what I see in the tutor-contrib-authz plugin, there is no init task registered, so tutor dev do init wouldn't have any effect at the moment.

Ah, you are right about the do init command, I forget we need this PR merged first overhangio/tutor#1496, I just removed the step in the testing instructions, is better / more direct to just use the management command, thanks!

I'm ok with using the filesystem absolute path as a module as long as it is clearly documented. Thanks!

… contributions This reverts commit 0f10788.

…ystem path

carlos-marquez-wgu · 2026-10-26T16:55:58Z

Thank you @BryanttV and @rodmgwgu !

I reverted the changes made for supporting module = null, now uses absolute filepath system, I updated the ADR 0030 as well to cover for that too

The code is rebased as well, so the code is ready for review but it'll sit waiting for PR#542 to get merged

rodmgwgu · 2026-10-26T18:53:18Z

@@ -0,0 +1 @@
+__version__ = "22.0.0"


nit: Should this be "23.0.0" to match the Willow tutor version?

You are right, fixed, thanks!

rodmgwgu · 2026-10-26T18:55:02Z

+
+[project.optional-dependencies]
+dev = [
+    "tutor[dev]>=22.0.0,<23.0.0",


Same here, I think Willow is version 23.0.0

Fixed here too, thanks

rodmgwgu

Looking good, just some nit comments. Thanks!

rodmgwgu · 2026-10-26T19:02:11Z

+        except ImproperlyConfigured:
+            return []
+        if not isinstance(file_paths, list):
+            raise ImproperlyConfigured


raise ImproperlyConfigured raises the bare class with no message, and ImproperlyConfigured is inconsistent with the rest of this method — the unsupported-extension and missing-file cases both raise SchemaDiscoveryError with a descriptive message. A string accidentally passed to the setting should fail the same diagnosable way, e.g.:

Suggested change

raise ImproperlyConfigured

if not isinstance(file_paths, list):

raise SchemaDiscoveryError(

"OPENEDX_AUTHZ_SCHEMA_FILES must be a list of filesystem paths, "

f"got {type(file_paths).__name__!r}."

)

Also there's no test covering the non-list case even though this guard was added for exactly that — worth an @override_settings(OPENEDX_AUTHZ_SCHEMA_FILES="...") test asserting it raises.

Changed, thanks!

rodmgwgu · 2026-10-26T19:02:11Z

-    def parse_yaml(contents: bytes, resource: DiscoveredResource) -> object:
-        """Parse YAML bytes and preserve the resulting root value."""
+    def parse_yamls(contents: bytes, resource: DiscoveredResource) -> list[object]:
+        """Parse YAML bytes as a multi-document stream


nit: docstring summary line is broken mid-sentence.

Suggested change

"""Parse YAML bytes as a multi-document stream

"""Parse YAML bytes as a multi-document stream, skipping empty

documents and preserving each resulting root value.

Fixed, thanks!

…ersion to verawood tutor, proper exception raising

rodmgwgu

Thanks for attending my comments!

BryanttV

Thanks! LGTM, we need to wait for #542 to be merged.

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 14, 2026

openedx-webhooks added this to Contributions Oct 14, 2026

github-project-automation Bot moved this to Needs Triage in Contributions Oct 14, 2026

carlos-marquez-wgu mentioned this pull request Oct 14, 2026

feat: add tutor-contrib-authz plugin openedx/openedx-tutor-plugins#77

Closed

carlos-marquez-wgu force-pushed the carlos-marquez-wgu/schema-discovery-tutor-patch-implementation branch 5 times, most recently from 1c20ec7 to 3953580 Compare October 20, 2026 20:22

carlos-marquez-wgu marked this pull request as ready for review October 20, 2026 20:43

carlos-marquez-wgu requested review from BryanttV, mariajgrimaldi and rodmgwgu October 21, 2026 15:25

BryanttV requested changes Oct 21, 2026

View reviewed changes

Comment thread src/openedx_authz/engine/schema/discovery.py Outdated

Comment thread src/openedx_authz/engine/schema/discovery.py Outdated

Comment thread src/openedx_authz/engine/schema/discovery.py Outdated

mphilbrick211 added the mao-onboarding Reviewing this will help onboard devs from an Axim mission-aligned organization (MAO). label Oct 22, 2026

mphilbrick211 moved this from Needs Triage to In Eng Review in Contributions Oct 22, 2026

carlos-marquez-wgu added 2 commits October 23, 2026 12:06

squash! Added support for module=None for file system file contributions

0f10788

carlos-marquez-wgu force-pushed the carlos-marquez-wgu/schema-discovery-tutor-patch-implementation branch from cd6dc7e to 0f10788 Compare October 23, 2026 18:07

carlos-marquez-wgu changed the base branch from main to MJG/applets-composable-roles October 23, 2026 18:11

carlos-marquez-wgu changed the base branch from MJG/applets-composable-roles to MJG/instructor-dashboard-permissions October 23, 2026 18:12

carlos-marquez-wgu changed the base branch from MJG/instructor-dashboard-permissions to MJG/support-json-schema October 23, 2026 18:14

carlos-marquez-wgu added 2 commits October 26, 2026 09:27

squash! Revert of: Added support for module=None for file system file…

723338d

… contributions This reverts commit 0f10788.

squash! Updated ADR 0030 to change module from null to absolute files…

1806829

…ystem path

carlos-marquez-wgu added 2 commits October 26, 2026 09:59

squash! Changed ADR 0029 -> ADR 0030

1939125

squash! Added additional testing for DiscoveredFileResource.read_bytes()

f40288e

carlos-marquez-wgu requested a review from BryanttV October 26, 2026 16:53

rodmgwgu reviewed Oct 26, 2026

View reviewed changes

squash! Addressing PR Comments: added test, fixed docstring, bumped v…

45f5e82

…ersion to verawood tutor, proper exception raising

carlos-marquez-wgu requested a review from rodmgwgu October 27, 2026 17:19

rodmgwgu approved these changes Oct 27, 2026

View reviewed changes

BryanttV approved these changes Oct 27, 2026

View reviewed changes

mariajgrimaldi deleted the branch openedx:MJG/support-json-schema October 27, 2026 21:07

mariajgrimaldi closed this Oct 27, 2026

github-project-automation Bot moved this from In Eng Review to Done in Contributions Oct 27, 2026

-            raise ImproperlyConfigured
+        if not isinstance(file_paths, list):
+            raise SchemaDiscoveryError(
+                "OPENEDX_AUTHZ_SCHEMA_FILES must be a list of filesystem paths, "
+                f"got {type(file_paths).__name__!r}."
+            )

	"""Parse YAML bytes as a multi-document stream
	"""Parse YAML bytes as a multi-document stream, skipping empty
	documents and preserving each resulting root value.

Conversation

carlos-marquez-wgu commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing instructions

Concerns

AI Usage

Dependencies

Uh oh!

openedx-webhooks commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carlos-marquez-wgu commented Oct 20, 2026

Uh oh!

BryanttV left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BryanttV commented Oct 22, 2026

Uh oh!

carlos-marquez-wgu commented Oct 23, 2026

Uh oh!

carlos-marquez-wgu commented Oct 23, 2026

Uh oh!

rodmgwgu commented Oct 23, 2026

Uh oh!

carlos-marquez-wgu commented Oct 26, 2026

Uh oh!

rodmgwgu Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

carlos-marquez-wgu Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

rodmgwgu Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

carlos-marquez-wgu Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

rodmgwgu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rodmgwgu Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlos-marquez-wgu Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

rodmgwgu Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

carlos-marquez-wgu Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

rodmgwgu left a comment

Choose a reason for hiding this comment

Uh oh!

BryanttV left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

carlos-marquez-wgu commented Oct 14, 2026 •

edited

Loading

openedx-webhooks commented Oct 14, 2026 •

edited

Loading

rodmgwgu left a comment •

edited

Loading

rodmgwgu Oct 26, 2026 •

edited

Loading