feat: use authz permissions on xblock handler when enabled#38179
Conversation
|
Thanks for the pull request, @wgu-taylor-payne! 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. |
eb5b8b6 to
cfa5592
Compare
rodmgwgu
left a comment
There was a problem hiding this comment.
Tested a couple of use cases in my local and works as expected, looks good, thanks!
bmtcril
left a comment
There was a problem hiding this comment.
Just one optional comment, otherwise looks good to me and works locally 👍
| str: The permission identifier (e.g., 'courses.view_course'), or None if authz should be skipped | ||
| """ | ||
| if usage_key_string: | ||
| # Skip authz for library context |
There was a problem hiding this comment.
Can you add more context about where enforcement is handled in content libraries here?
There was a problem hiding this comment.
Sure, I've added more context.
dwong2708
left a comment
There was a problem hiding this comment.
Looks good! Just one non-blocking suggestion and one question.
When I worked on Instructor Dashboard V2, I noticed that ORA2 (a custom XBlock) internally calls an endpoint via a custom XBlock handler:
http://local.openedx.io:8000/courses/course-v1:WGU+CS002+2025_T1/xblock/block-v1:WGU+CS002+2025_T1+type@openassessment+block@ff4f5fddf42d4b9787e69c1a8cbeb058/handler/get_ora2_responses
This points to cms/djangoapps/contentstore/views/component.py (component_handler).
This made me wonder whether we should also take this endpoint into account.
| elif request.method == "DELETE": | ||
| if block_type == "static_tab": | ||
| return COURSES_MANAGE_PAGES_AND_RESOURCES.action.external_key | ||
| return COURSES_EDIT_COURSE_CONTENT.action.external_key |
There was a problem hiding this comment.
It would be nice to standardize the use of the identifier. As I’ve seen, in other places the <AUTHZ_PERM>.identifier style is more commonly used and shorter
There was a problem hiding this comment.
Thanks, I've changed those.
@dwong2708 I scoped this PR to just the As you have brought up, there are other xblock related endpoints that are called in different course related pages/contexts in the CMS, but I was thinking that those would be implemented as part of the existing M1 tasks. So, as we implement our tickets, we just need to be aware that only this specific xblock view has been covered and the others will need to be addressed as we go along. Does that sound right @rodmgwgu? Or should I look at more xblock endpoints here? |
Co-authored-by: Kiro <noreply@kiro.dev>
cfa5592 to
508e85d
Compare
Makes sense. Whether we decide to address the other XBlock endpoints or not, I think scoping this task to the XBlock handler sounds good to me. Thanks. |
Description
Use openedx-authz permissions for the
/xblock/endpoint, when theAUTHZ_COURSE_AUTHORING_FLAGis enabled for the given course. The mapping between actions in the CMS and the associated permission is found in this document.If the
AUTHZ_COURSE_AUTHORING_FLAGis disabled then the legacy permission checks will occur.Supporting information
Closes #37925.
Testing instructions
Run
pyteston the new test modulecms/djangoapps/contentstore/tests/test_xblock_handler_permissions.py.To test manually, make sure that the
AUTHZ_COURSE_AUTHORING_FLAGis enabled for the course you are testing, and that openedx-platform is using openedx-authz >= 0.22.0. Runmanage.py cms load_policiesto ensure latest casbin policies have been loaded and then assign course related roles to the users you are testing. Go though the actions outlined in the document linked above and verify that the correct permissions are checked and applied for the given user.Deadline
Verawood.