Repository navigation
Conversation
Declare the permission identifiers used by the Instructor Dashboard API v2. Temporary: the permissions not defined by openedx-authz depend on a separate issue that will define them there. They are declared here only for testing. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Add the shared infrastructure to check Instructor Dashboard permissions with openedx-authz, driven by the authz.enable_course_authoring flag. * With the flag enabled for the course only AuthZ is consulted, like in Studio. With the flag disabled the legacy check is used, so the previous behavior is preserved. * Add user_has_instructor_permission, get_instructor_authz_permissions and the InstructorAuthzPermission DRF permission class, logging which path (authz or legacy) resolved each check. * Add the legacy equivalent of each permission. * CourseMetadataView resolves the Course Info, Enrollments, Course Team, Grading, Date Extensions and Cohorts tabs through AuthZ, and returns the effective permissions in the new authz_permissions field. The existing fields do not change. * CourseEnrollmentsView requires courses.view_enrollments. * Add test helpers (InstructorAuthzTestMixin) and tests. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
* Add examples to "Migrating a view": a single permission, a permission per HTTP method, a custom legacy check, direct use of user_has_instructor_permission and the errors returned. * State in the cohort legacy checks that they follow the same rule as IsStaffOrAdmin. * Wrap the long line in has_cohort_view_access and move the "Shared with Authoring" note of COURSES_VIEW_GRADING_SETTINGS to the same line. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
* Add AuthzTestMixin with the code common to the course authoring and Instructor Dashboard test mixins: the flag patch, the policy seeding and cleanup, and assign_authz_role. * Add set_authz_enabled and set_authz_enabled_for to turn the flag off or enable it only for some courses, instead of ad hoc patches in each test. * CourseAuthoringAuthzTestMixin and InstructorAuthzTestMixin extend it and keep only what is specific to them. * Add docstrings to the Instructor Dashboard AuthZ tests. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…sions Make the default legacy check of each permission the rule of the endpoint that the MFE calls, and keep the tab visibility rules explicit. * view_course_info, view_grading_configuration and view_learner_extensions default to the VIEW_DASHBOARD permission. The Course Info and Grading tabs pass has_staff_access, as the Date Extensions tab passes has_instructor_access. * view_gradebook follows the gradebook API (course author access) and view_grading_settings follows the Studio grading settings (Studio read access). * The Grading tab is resolved with view_grading_configuration. * CourseMetadataView uses the default check of view_course_info, and documents that masquerading is resolved with that permission. * Use other permissions in the README example of a permission per HTTP method. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A tab is now shown when the user holds at least one of the permissions validated in it, instead of a single specific permission. * Add TAB_PERMISSIONS with the permissions of each tab. The Enrollments tab includes manage_course_team because it manages the beta testers. * Add user_has_any_instructor_permission. With the flag enabled it consults only AuthZ; with it disabled it uses the explicit legacy check, or the default one of each permission. * CourseInformationSerializerV2.get_tabs resolves the tabs through it. The explicit legacy checks (staff, instructor) are preserved. * Cover the helper and every (tab, permission) pair in the tests. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The Date Extensions tab is only shown when edx-when is enabled for the course, so the tab tests did not see it now that it is part of the migrated tabs. * Patch is_enabled_for_course for the whole CourseMetadataViewAuthzTest. * Legacy staff does not see Date Extensions, which requires instructor access when the flag is disabled. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
CourseEnrollmentsView goes back to the legacy InstructorPermission check, and its AuthZ tests are removed. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Migrate ChangeDueDateView, GradedSubsectionsView and UnitExtensionsView to InstructorAuthzPermission, following the shared infrastructure from openedx-authz#486. GET endpoints require courses.view_learner_extensions, the POST endpoint requires courses.manage_learner_extensions. Both already had a matching default legacy check registered in LEGACY_PERMISSION_CHECKS, so the existing permission_name on each view is reused unchanged as the fallback when the authz.enable_course_authoring flag is disabled for the course. Add DateExtensionsAuthzTest covering Course Admin, Course Staff, a user with no permission, AuthZ denying a legacy instructor, and the legacy path with the flag off, for all three endpoints. Fixes openedx-authz#491.
|
Thanks for the pull request, @efortish! 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. 🔘 Update the status of your PRYour PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate. 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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
|
Closing in favor of a PR against eduNEXT/edx-platform directly, targeting bav/instructor-authz-shared-infra, so the diff only shows this PR's own changes instead of #39255's too. Will reopen against master once that one merges. |
Description
Stacked on top of #39255 (not mergeable until that one lands, since it carries that PR's full diff). Migrates the three endpoints behind the Date Extensions tab to the shared Instructor Dashboard AuthZ infrastructure from openedx-authz#486, following the README's migration guide (
lms/djangoapps/instructor/authz/README.rst):authz_permissionGradedSubsectionsViewcourses.view_learner_extensionspermissions.VIEW_DASHBOARDUnitExtensionsViewcourses.view_learner_extensionspermissions.VIEW_DASHBOARDChangeDueDateViewcourses.manage_learner_extensionspermissions.GIVE_STUDENT_EXTENSIONEach view keeps its existing
permission_name:LEGACY_PERMISSION_CHECKSinlms/djangoapps/instructor/authz/legacy.pyalready maps bothCOURSES_VIEW_LEARNER_EXTENSIONS/COURSES_MANAGE_LEARNER_EXTENSIONStobridgekeeper_check(permissions.VIEW_DASHBOARD)/bridgekeeper_check(permissions.GIVE_STUDENT_EXTENSION)respectively (built as part of #39255), so this is a "Single permission" migration per the README, no new legacy-check code needed. Withauthz.enable_course_authoringdisabled for a course, behavior is byte-for-byte what it was before this PR.The Date Extensions tab's own visibility logic (
can_see("date_extensions", authz_legacy.has_instructor_access)inserializers_v2.py) was already wired up in #39255, so this PR only touches the three endpoint views and their tests, nothing in the tab-visibility/serializer code.Fixes openedx-authz#491.
Testing
Added
DateExtensionsAuthzTesttotest_api_v2_authz.py(next to the existingCourseMetadataViewAuthzTestfrom #39255), covering all three endpoints:is_user_allowed.assert_not_called()).Also ran the existing (pre-this-PR) tests for these three views to confirm nothing regressed for the legacy path:
GradedSubsectionsViewTest,UnitExtensionsViewTest(intest_api_v2.py), andTestChangeDueDateV2/TestDueDateExtensions(intest_api.py). All pass unchanged.Full suite:
lms/djangoapps/instructor/(1045 passed).ruff,pylint,lint-importsclean.Note for local testing: if you hit
ImportErrors unrelated to this diff when running these tests (AccountActivationEmailContextGeneratedfromopenedx_filters, orFullScopePolicyfromedx_rest_framework_extensions), your devstack's LMS venv hasopenedx-filters/edx-drf-extensionsolder than whatrequirements/edx/base.txtpins. That's what I hit too; fixed locally with:(or whatever those requirements currently pin). Unrelated to this PR's code, just a stale local environment.
Deadline
None