feat: enforce openedx-authz permissions on object tag and xblock outline endpoints#38292
feat: enforce openedx-authz permissions on object tag and xblock outline endpoints#38292wgu-taylor-payne wants to merge 6 commits intoopenedx:masterfrom
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. |
d1ae95c to
c26f972
Compare
|
@wgu-taylor-payne we discussed this in the Weekly meeting and the decision is that for M1 we will only focus on implementing the courses.manage_tags permission on course-related endpoints. So no taxonomies-related changes for now. |
55ae268 to
51f9f3e
Compare
2be413a to
97a2719
Compare
…ine endpoints
When AUTHZ_COURSE_AUTHORING_FLAG is enabled for a course, legacy
permission checks are fully bypassed in favor of openedx-authz:
- PUT /object_tags/{object_id}/: enforces courses.manage_tags
- GET /object_tags/{object_id}/: patches can_tag_object in response
- GET /xblock/outline/{usage_key}: enforces courses.view_course
For object tag endpoints, the parent ObjectTagView legacy checks
(ObjectTagObjectPermissions, per-taxonomy can_tag_object, and
view_objecttag in get_queryset) are all bypassed via get_permissions,
get_queryset, and _update_tags overrides. Course key is extracted from
any content key type via get_context_key_from_key_string. Library
content is unaffected (falls through to legacy).
When the flag is off, all endpoints use existing legacy behavior.
Co-authored-by: Kiro <kiro-noreply@amazon.com>
aff4d6c to
2e46a22
Compare
| object_id = self.kwargs["object_id"] | ||
| _, authz_active = self._is_authz_active(object_id) | ||
| if authz_active: | ||
| return get_object_tags(object_id) |
There was a problem hiding this comment.
For that reason, I think this implementation may not be maintainable, because if new logic is added to the original view, it could be affected.
My suggestion is to implement the AuthZ permission directly in ObjectTagView. Another option would be to update ObjectTagView so that the permission check is handled in a separate method, which can then be overridden from openedx-platform.
I also think this could be addressed in a separate ticket to avoid blocking this work.
What do you think?
There was a problem hiding this comment.
Good question. I went this route because I don't think openedx-tagging should be concerned with waffle flags defined in openedx-platform. I was hoping to avoid having to touch openedx-tagging, but yes, you point out a valid concern. I think we can refactor openedx-tagging to allow for a cleaner implementation here. I'll look into that.
There was a problem hiding this comment.
Here is a potential implementation on the openedx-core side that could allow a cleaner path for the changes here: openedx/openedx-core#541.
…ck outline endpoints
…ck outline endpoints
…ck outline endpoints
Description
When
AUTHZ_COURSE_AUTHORING_FLAGis enabled for a course, enforces openedx-authz permissions on content tagging and xblock outline endpoints, fully bypassing legacy permission checks.This PR depends on a companion openedx-core PR (openedx/openedx-core#541) that extracts overridable permission methods from
ObjectTagView, allowing edx-platform to plug in authz checks without duplicating parent view logic.Object tag endpoints (
/api/content_tagging/v1/object_tags/{object_id}/):ObjectTagOrgViewoverrides 4 permission methods introduced in openedx-core:check_view_object_tags_permissioncheck_can_tag_object_permissioncourses.manage_tagsviaauthz_api.is_user_allowed, raisesPermissionDeniedif deniedget_can_tag_object_valueauthz_api.is_user_allowed(COURSES_MANAGE_TAGS)— drivescan_tag_objectin responseget_can_delete_objecttag_valueauthz_api.is_user_allowed(COURSES_MANAGE_TAGS)— drivescan_delete_objecttagin responseAdditionally,
get_permissions()swaps the legacyObjectTagObjectPermissionsforIsAuthenticatedwhen authz is active, so DRF dispatch doesn't block requests before the override methods run.All overrides fall through to
super()(legacy behavior) when the flag is off or the object is not course-scoped (e.g., library content).Course key is extracted from any content key type (course, block, unit, collection, container) via
get_context_key_from_key_string.Xblock outline endpoint (
/xblock/outline/{usage_key}):GET— replaceshas_studio_read_accesswithuser_has_course_permissionusingcourses.view_course, with legacyREADfallback when the flag is off. This endpoint is called by Studio when the content tags drawer opens (e.g., in the course outline, course unit editor, etc.).When the flag is off, all endpoints use existing legacy behavior unchanged.
Endpoint → permission mapping:
PUT /object_tags/{object_id}/courses.manage_tagsGET /object_tags/{object_id}/courses.manage_tags(edit fields)GET /xblock/outline/{usage_key}courses.view_courseRoles and permissions (from Casbin policy):
view_coursemanage_tagscourse_admincourse_staffcourse_editorcourse_auditorSupporting information
Testing instructions
Flag OFF (legacy behavior unchanged):
authz.enable_course_authoringwaffle flag is inactiveFlag ON per-course:
authz.enable_course_authoringfor a specific course → Force Oncourse_staff: open course outline → click tag icon on a section → add/remove tags → save. Should succeed.course_auditor: open course outline → click tag icon. Verify unable to change/save tags (403).course_auditor: verify xblock outline endpoint still loads.Deadline
Verawood
Other information
Co-authored with Kiro — AI-assisted implementation and testing.