[FC-0099] feat: add openedx-authz to library apis user_can_create_library and require_permission_for_library_key #37501

MaferMazu · 2025-10-17T22:54:02Z

Description

This PR:

Introduces openedx-authz in two api methods user_can_create_library
and require_permission_for_library_key.
Modify the collections and publish enforcement to use the new permissions and also check the old one.
Add tests based on test_library_permission but using the new authz system.

Implementation:
We implement user_has_permission_across_lib_authz_systems to handle the verification of permissions in both system (the one in content_libraries/permissions.py and the new one implemented in openedx-authz).

Consequences:
By adding openedx-authz enforcement in require_permission_for_library_key, we are extending the verification in those places: https://github.com/search?q=repo%3Aopenedx%2Fedx-platform%20require_permission_for_library_key&type=code

Important note:
With this change we are adding checks over explicit permissions in the openedx-authz system. So we rely on bridgekeeper for the moment to make attribute checking (specific cases: course_creator imply library_creator, and the library having the allow_public_read attribute imply view_library true).

Supporting information

openedx/openedx-authz#88
https://openedx-authz.readthedocs.io/en/latest/concepts/core_roles_and_permissions/content_library_roles.html#roles-and-permissions-summary-table

Testing instructions

Using python test

I didn't modify the current test, so the tested behavior won't change with this addition.
I added test that verify the rest api calls when he data uses the new openedx-authz system.

In stage

Stage information: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/5279023110/AuthZ+Stage+Information

Preparation of the env:

Have an library user, a contributor, an author and an admin.

Credentials: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/5279023110/AuthZ+Stage+Information

You can create your own library, or use one created. For this I'll use lib:OpenedX:LIBTESTDELETE/ as example.

Test:
Library User

Libraries
- GET: Enter in https://studio.singapore.edunext.cloud/api/libraries/v2/lib:OpenedX:LIBTESTDELETE/
  - Expected result: Obtain info.
Library content
- Edit content: Enter in https://apps.singapore.edunext.cloud/authoring/library/lib:OpenedX:LIBTESTDELETE/
  - Expected result: view the library but be unable to add new elements.
- Publish: Inside the library click library info
  - Expected result: No see the publish button.
Collections
- Edit Collections: Inside https://apps.singapore.edunext.cloud/authoring/library/lib:OpenedX:LIBTESTDELETE/, click collections.
  - Expected result: be unable to edit collections.
Delete: You can use this collection for a quick postman test
simple-delete-37501.json
- Expected result: 403

Library Contributor

Libraries
- GET: Enter in https://studio.singapore.edunext.cloud/api/libraries/v2/lib:OpenedX:LIBTESTDELETE/
  - Expected result: Obtain info.
Library content
- Edit content: Enter in https://apps.singapore.edunext.cloud/authoring/library/lib:OpenedX:LIBTESTDELETE/
  - Expected result: view the library and be able to add new elements.
- Publish: Inside the library click library info
  - Expected result: No see the publish button. - This is something we should modify in the front as part of release testing (for now we can view the button but in backend the contributor can not publish that is the expected behavior)
Collections
- Edit Collections: Inside https://apps.singapore.edunext.cloud/authoring/library/lib:OpenedX:LIBTESTDELETE/, click collections.
  - Expected result: be able to edit collections.
Delete: You can use this collection for a quick postman test (changing the credentials)
simple-delete-37501.json
- Expected result: 403

Library Author
The same + publish content.

Library Admin
It should have access to get, edit the content and collections and delete (soft delete a library)

Deadline

Ulmo cut.

Other information

Related to: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/4724490259/PRD+Roles+Permissions

openedx-webhooks · 2025-10-17T22:54:07Z

Thanks for the pull request, @MaferMazu!

This repository is currently maintained by @openedx/wg-maintenance-edx-platform.

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.

openedx/core/djangoapps/content_libraries/rest_api/blocks.py

requirements/edx/github.in

mariajgrimaldi

I know this is still a work in progress, but I think we should add unit tests for the cases in
https://github.com/openedx/edx-platform/blob/8fad71724e6b87e4c3f2c95f69b21c7e7507a5da/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py#L565-L745
to make sure we're still covering the same scenarios we're modifying here, assuming these are the ones affected.

MaferMazu · 2025-10-24T00:54:31Z

@BryanttV @mariajgrimaldi, following the recommendation to add our enforcement in require_permission_for_library_key, we are already adding the enforcement in some of the places I identified.

This PR currently covers the parts marked in here: openedx/openedx-authz#88 (tested)

Right now, I am working on the collection permissions (I know I have some tests failing, but I'll work on that tomorrow morning) to try to have all the enforcement in this PR, since we're far from having all the enforcement points updated.

I added require_authz_lib_permission to handle the permissions that weren't in the old system.

Things this PR needs:

Fix collection tests
Update the remaining enforcement points
Create a test_content_library_permission using data from the new system

mariajgrimaldi

Here's another round of review. My main concern is transparency and readability. It's a bit confusing to have multiple functions checking permissions, so we might need to rethink how to centralize all checks in one place. That way, we avoid maintaining different versions and make it easier for others to follow the authorization logic.

I want to prioritize parity functionality instead of quality and nits, so we can find a good place in between.

openedx/core/djangoapps/content_libraries/api/libraries.py

openedx/core/djangoapps/content_libraries/rest_api/collections.py

mariajgrimaldi · 2025-11-11T20:22:38Z

@MaferMazu: can we report this in the openedx-authz repo?

Expected result: No see the publish button. - This is something we should modify in the front as part of release testing (for now we can view the button but in backend the contributor can not publish that is the expected behavior)

mariajgrimaldi

LGTM!

Thank you so much for the hard work. I left a few nits for you to consider, but everything else looks good to go. Everything I tested also looked good! Thanks :)

MaferMazu · 2025-11-12T04:44:06Z

I am having problems with the test using the last openedx-authz. I'm working on it.

mariajgrimaldi · 2025-11-12T13:55:03Z

@MaferMazu could you check these changes to see if they help with the unit tests? 3cd4176

A few notes:

The atomic blocks aren’t needed since the API we call already handles atomic consistency.
There was an issue when inheriting from classes that both define the same methods (like setUp, setUpClass, etc.) and share the same parent (Django test class), which caused unexpected behavior.
I split the test into smaller, clearer blocks to make debugging and review easier.

Let me know what you think of the changes. Thanks!

bmtcril

I've tested this locally and it looks good to me once @mariajgrimaldi 's comments are sorted!

MaferMazu · 2025-11-12T18:16:40Z

@mariajgrimaldi, thanks a lot for the help with the test. I applied your changes, and they work. Thanks again.

The last change I did was to upgrade the openedx-authz package.

@mariajgrimaldi, can you help me review this?

mariajgrimaldi

LGTM! Thank you :)

feat: add the authz publish check in rest_api blocks and containers feat: add the authz checks in libraries and refactor feat: add collections checks feat: update enforcement in serializer file refactor: refactor the permission check functions fix: fix value error fix: calling the queries twice

refactor: refactor the tests and apply feedback fix: apply feedback Revert "refactor: refactor the tests and apply feedback" This reverts commit aa0bd52. refactor: use constants and avoid mapping test: fix the test to have them in order docs: about we rely on bridgekeeper and the old check for two cases docs: update openedx/core/djangoapps/content_libraries/api/libraries.py Co-authored-by: Maria Grimaldi (Majo) <maria.grimaldi@edunext.co> refactor: use global scope wildcard instead of * refactor: allow receiving PermissionData objects refactor: do not inherit from BaseRolesTestCase to favor CL setup methods If both BaseRolesTestCase and ContentLibrariesRestApiTest define a method with the same name (e.g., setUp()), Python will use the one found first in the MRO, which is the one in BaseRolesTestCase because it is listed first in the class definition leading to unexpected behavior. refactor: remove unnecessary imports and indent

…rary and require_permission_for_library_key (openedx#37501) * feat: add the authz check to the library api function feat: add the authz publish check in rest_api blocks and containers feat: add the authz checks in libraries and refactor feat: add collections checks feat: update enforcement in serializer file refactor: refactor the permission check functions fix: fix value error fix: calling the queries twice * test: add structure for test and apply feedback refactor: refactor the tests and apply feedback fix: apply feedback Revert "refactor: refactor the tests and apply feedback" This reverts commit aa0bd52. refactor: use constants and avoid mapping test: fix the test to have them in order docs: about we rely on bridgekeeper and the old check for two cases docs: update openedx/core/djangoapps/content_libraries/api/libraries.py Co-authored-by: Maria Grimaldi (Majo) <maria.grimaldi@edunext.co> refactor: use global scope wildcard instead of * refactor: allow receiving PermissionData objects refactor: do not inherit from BaseRolesTestCase to favor CL setup methods If both BaseRolesTestCase and ContentLibrariesRestApiTest define a method with the same name (e.g., setUp()), Python will use the one found first in the MRO, which is the one in BaseRolesTestCase because it is listed first in the class definition leading to unexpected behavior. refactor: remove unnecessary imports and indent * chore: bump openedx-authz version (cherry picked from commit f4f14a6)

…sion_for_library_key (openedx#37501) * feat: add the authz check to the library api function feat: add the authz publish check in rest_api blocks and containers feat: add the authz checks in libraries and refactor feat: add collections checks feat: update enforcement in serializer file refactor: refactor the permission check functions fix: fix value error fix: calling the queries twice * test: add structure for test and apply feedback refactor: refactor the tests and apply feedback fix: apply feedback Revert "refactor: refactor the tests and apply feedback" This reverts commit aa0bd52. refactor: use constants and avoid mapping test: fix the test to have them in order docs: about we rely on bridgekeeper and the old check for two cases docs: update openedx/core/djangoapps/content_libraries/api/libraries.py Co-authored-by: Maria Grimaldi (Majo) <maria.grimaldi@edunext.co> refactor: use global scope wildcard instead of * refactor: allow receiving PermissionData objects refactor: do not inherit from BaseRolesTestCase to favor CL setup methods If both BaseRolesTestCase and ContentLibrariesRestApiTest define a method with the same name (e.g., setUp()), Python will use the one found first in the MRO, which is the one in BaseRolesTestCase because it is listed first in the class definition leading to unexpected behavior. refactor: remove unnecessary imports and indent * chore: bump openedx-authz version (cherry picked from commit f4f14a6)

…rary and require_permission_for_library_key (openedx#37501) * feat: add the authz check to the library api function feat: add the authz publish check in rest_api blocks and containers feat: add the authz checks in libraries and refactor feat: add collections checks feat: update enforcement in serializer file refactor: refactor the permission check functions fix: fix value error fix: calling the queries twice * test: add structure for test and apply feedback refactor: refactor the tests and apply feedback fix: apply feedback Revert "refactor: refactor the tests and apply feedback" This reverts commit aa0bd52. refactor: use constants and avoid mapping test: fix the test to have them in order docs: about we rely on bridgekeeper and the old check for two cases docs: update openedx/core/djangoapps/content_libraries/api/libraries.py Co-authored-by: Maria Grimaldi (Majo) <maria.grimaldi@edunext.co> refactor: use global scope wildcard instead of * refactor: allow receiving PermissionData objects refactor: do not inherit from BaseRolesTestCase to favor CL setup methods If both BaseRolesTestCase and ContentLibrariesRestApiTest define a method with the same name (e.g., setUp()), Python will use the one found first in the MRO, which is the one in BaseRolesTestCase because it is listed first in the class definition leading to unexpected behavior. refactor: remove unnecessary imports and indent * chore: bump openedx-authz version

…sion_for_library_key (openedx#37501) * feat: add the authz check to the library api function feat: add the authz publish check in rest_api blocks and containers feat: add the authz checks in libraries and refactor feat: add collections checks feat: update enforcement in serializer file refactor: refactor the permission check functions fix: fix value error fix: calling the queries twice * test: add structure for test and apply feedback refactor: refactor the tests and apply feedback fix: apply feedback Revert "refactor: refactor the tests and apply feedback" This reverts commit aa0bd52. refactor: use constants and avoid mapping test: fix the test to have them in order docs: about we rely on bridgekeeper and the old check for two cases docs: update openedx/core/djangoapps/content_libraries/api/libraries.py Co-authored-by: Maria Grimaldi (Majo) <maria.grimaldi@edunext.co> refactor: use global scope wildcard instead of * refactor: allow receiving PermissionData objects refactor: do not inherit from BaseRolesTestCase to favor CL setup methods If both BaseRolesTestCase and ContentLibrariesRestApiTest define a method with the same name (e.g., setUp()), Python will use the one found first in the MRO, which is the one in BaseRolesTestCase because it is listed first in the class definition leading to unexpected behavior. refactor: remove unnecessary imports and indent * chore: bump openedx-authz version (cherry picked from commit f4f14a6)

…cement points (#37633) * feat: filter libraries based on user-role scopes (#37564) (cherry picked from commit 6c6fc5d) * feat: add openedx-authz to user_can_create_library and require_permission_for_library_key (#37501) * feat: add the authz check to the library api function feat: add the authz publish check in rest_api blocks and containers feat: add the authz checks in libraries and refactor feat: add collections checks feat: update enforcement in serializer file refactor: refactor the permission check functions fix: fix value error fix: calling the queries twice * test: add structure for test and apply feedback refactor: refactor the tests and apply feedback fix: apply feedback Revert "refactor: refactor the tests and apply feedback" This reverts commit aa0bd52. refactor: use constants and avoid mapping test: fix the test to have them in order docs: about we rely on bridgekeeper and the old check for two cases docs: update openedx/core/djangoapps/content_libraries/api/libraries.py Co-authored-by: Maria Grimaldi (Majo) <maria.grimaldi@edunext.co> refactor: use global scope wildcard instead of * refactor: allow receiving PermissionData objects refactor: do not inherit from BaseRolesTestCase to favor CL setup methods If both BaseRolesTestCase and ContentLibrariesRestApiTest define a method with the same name (e.g., setUp()), Python will use the one found first in the MRO, which is the one in BaseRolesTestCase because it is listed first in the class definition leading to unexpected behavior. refactor: remove unnecessary imports and indent * chore: bump openedx-authz version (cherry picked from commit f4f14a6) * feat: Upgrade Python dependency openedx-authz (#37652) * feat: Upgrade Python dependency openedx-authz handle cache invalidation Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` * fix: update the num of queries in tests --------- Co-authored-by: MaferMazu <35668326+MaferMazu@users.noreply.github.com> Co-authored-by: Maria Fernanda Magallanes Zubillaga <maria.magallanes@edunext.co> (cherry picked from commit 122b4e0) * chore: update requirements to fix the inconsistency --------- Co-authored-by: Maria Grimaldi (Majo) <maria.grimaldi@edunext.co> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

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

openedx-webhooks added this to Contributions Oct 17, 2025

github-project-automation bot moved this to Needs Triage in Contributions Oct 17, 2025

This was referenced Oct 17, 2025

Enforcement Updates openedx/openedx-authz#88

Closed

Improve the enforcement calls in the code openedx/openedx-authz#104

Closed

MaferMazu force-pushed the mfmz/authz-base branch from 0e501cd to ac7792f Compare October 20, 2025 14:25

mariajgrimaldi requested changes Oct 21, 2025

View reviewed changes

openedx/core/djangoapps/content_libraries/rest_api/blocks.py Outdated Show resolved Hide resolved

MaferMazu force-pushed the mfmz/authz-base branch 2 times, most recently from 2bd07e4 to d916e7b Compare October 21, 2025 23:38

MaferMazu marked this pull request as ready for review October 21, 2025 23:46

MaferMazu requested review from feanil and kdmccormick as code owners October 21, 2025 23:46

feanil requested a review from bmtcril October 22, 2025 17:36

feanil reviewed Oct 22, 2025

View reviewed changes

requirements/edx/github.in Outdated Show resolved Hide resolved

MaferMazu added this to RBAC AuthZ Board Oct 22, 2025

MaferMazu moved this to In Progress in RBAC AuthZ Board Oct 22, 2025

mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Oct 22, 2025

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

mariajgrimaldi reviewed Oct 23, 2025

View reviewed changes

MaferMazu force-pushed the mfmz/authz-base branch from d916e7b to 1d03f51 Compare October 23, 2025 15:50

jmakowski1123 added the release blocker Blocks the upcoming release (fix needed) label Oct 23, 2025

This was referenced Oct 23, 2025

Grant Library Admin when Creating a Library openedx/openedx-authz#111

Closed

[FC-0099] feat: assign library roles after successful library creation #37532

Merged

mariajgrimaldi requested changes Oct 24, 2025

View reviewed changes

openedx/core/djangoapps/content_libraries/api/libraries.py Outdated Show resolved Hide resolved

openedx/core/djangoapps/content_libraries/api/libraries.py Outdated Show resolved Hide resolved

openedx/core/djangoapps/content_libraries/rest_api/collections.py Outdated Show resolved Hide resolved

MaferMazu force-pushed the mfmz/authz-base branch 3 times, most recently from bc99706 to bb9a1bb Compare October 27, 2025 03:10

MaferMazu changed the title ~~[FC-0099] feat: add openedx-authz as a requirement and add some enforcerment checks~~ [FC-0099] feat: add openedx-authz to library apis user_can_create_library and require_permission_for_library_key Oct 27, 2025

mariajgrimaldi approved these changes Nov 11, 2025

View reviewed changes

mariajgrimaldi self-requested a review November 12, 2025 13:58

bmtcril approved these changes Nov 12, 2025

View reviewed changes

MaferMazu force-pushed the mfmz/authz-base branch from 6578e8f to 9fff436 Compare November 12, 2025 17:55

MaferMazu mentioned this pull request Nov 12, 2025

Bug: Hide publish library button in Studio. openedx/openedx-authz#139

Closed

mariajgrimaldi approved these changes Nov 13, 2025

View reviewed changes

MaferMazu force-pushed the mfmz/authz-base branch from 90e870e to 71a114e Compare November 13, 2025 14:53

MaferMazu enabled auto-merge (squash) November 13, 2025 15:20

MaferMazu disabled auto-merge November 13, 2025 15:22

MaferMazu added 3 commits November 13, 2025 11:11

chore: bump openedx-authz version

5104ea1

MaferMazu force-pushed the mfmz/authz-base branch from 71a114e to 5104ea1 Compare November 13, 2025 16:33

MaferMazu merged commit f4f14a6 into openedx:master Nov 13, 2025
84 of 95 checks passed

github-project-automation bot moved this from Ready for review to Done in RBAC AuthZ Board Nov 13, 2025

github-project-automation bot moved this from In Eng Review to Done in Contributions Nov 13, 2025

github-project-automation bot moved this from In progress to Done in Build-Test-Release Working Group Nov 13, 2025

mariajgrimaldi linked an issue Nov 13, 2025 that may be closed by this pull request

Bug: 403 when opening library dashboard as "guillermotest" team member openedx/openedx-authz#120

Closed

MaferMazu mentioned this pull request Nov 13, 2025

[Backport FC-0099] feat: add openedx-authz and update libraries enforcement points #37633

Merged

ormsbee mentioned this pull request Jan 11, 2026

refactor: convert global modulestore to request-cached #37859

Closed

[FC-0099] feat: add openedx-authz to library apis user_can_create_library and require_permission_for_library_key #37501

[FC-0099] feat: add openedx-authz to library apis user_can_create_library and require_permission_for_library_key #37501

Uh oh!

Conversation

MaferMazu commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supporting information

Testing instructions

Using python test

In stage

Deadline

Other information

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mariajgrimaldi left a comment

Choose a reason for hiding this comment

Uh oh!

MaferMazu commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mariajgrimaldi 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!

Uh oh!

Uh oh!

Uh oh!

mariajgrimaldi commented Nov 11, 2025

Uh oh!

mariajgrimaldi left a comment

Choose a reason for hiding this comment

Uh oh!

MaferMazu commented Nov 12, 2025

Uh oh!

mariajgrimaldi commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bmtcril left a comment

Choose a reason for hiding this comment

Uh oh!

MaferMazu commented Nov 12, 2025

Uh oh!

mariajgrimaldi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

MaferMazu commented Oct 17, 2025 •

edited

Loading

openedx-webhooks commented Oct 17, 2025 •

edited

Loading

MaferMazu commented Oct 24, 2025 •

edited

Loading

mariajgrimaldi left a comment •

edited

Loading

mariajgrimaldi commented Nov 12, 2025 •

edited

Loading