Conversation
…ngsView GET and POST on this endpoint only required studio read access, so a user who could view a course in Studio could also read or overwrite its proctored exam settings by calling the API directly, bypassing the UI's staff-only enforcement. Add the same authz check used elsewhere for pages-and-resources management, with a write-level legacy fallback when authz is disabled. Closes openedx-authz#443
|
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. 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. |
Description
ProctoredExamSettingsView(GET and POST) only checked studio read access. That meant a user who could view a course in Studio could also read or overwrite its proctored exam settings by calling the API directly, bypassing the UI's own staff-only restriction. This adds thecourses.manage_pages_and_resourcesauthz check to both methods, with a write-level legacy fallback for courses that don't have authz course authoring enabled, matching the pattern already used for the same permission incms/djangoapps/contentstore/rest_api/v0/views/tabs.py.Closes openedx-authz#443
Testing
Added
test_authz_user_allowedandtest_authz_user_not_allowed(shared by the GET and POST test classes), plus verified the existing 31 tests in the file still pass, including the legacy read/write role checks.To confirm the new tests actually catch the bug and aren't passing by coincidence, I reverted the fix locally and reran them:
test_authz_user_not_allowedfailed withassert 200 == 403(a user denied by authz still got through), andtest_authz_user_allowedfailed because the authz check was never even called. Restoring the fix brings all 35 tests in the file back to green.