fix: keep unmigrated legacy roles working under the authz authoring flag (#38855)#38891
Conversation
|
Thanks for the pull request, @mariajgrimaldi! 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. |
…lag (#38855) Granting and exercising course creator access crashed once authz.enable_course_authoring was on. The courses.create_course permission was implemented in AuthZ, but the course creator role itself was never migrated there. The code deciding whether to use the AuthZ path only checked the flag's state, not whether the specific role had actually been migrated, so unmigrated roles like course creator still used the AuthZ path and crashed. enable_authz_course_authoring now takes an optional role and falls back to the course creator role's own check for any role without a migrated AuthZ equivalent, regardless of the flag. See ADR 0027. Merged to master as 0f57cf2bbf1. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
4b118dc to
7daa33c
Compare
|
Confirmed this is working on the PR sandbox: https://github.com/openedx/openedx-platform/pull/38891/checks?check_run_id=87363304519. You can follow the instructions in the cover letter to test! FYI @farhaanbukhsh 👀 |
|
@farhaanbukhsh: considering we had a successfull build and tests on the PR sandbox - and also an approval (thanks @AhtishamShahid!), I'm going to go ahead and merge this! I'll be on the lookout for any regression. Thanks! |
Description
Granting and exercising course creator access crashed once authz.enable_course_authoring was on. The courses.create_course permission was implemented in AuthZ, but the course creator role itself was never migrated there. The code deciding whether to use the AuthZ path only checked the flag's state, not whether the specific role had actually been migrated, so unmigrated roles like course creator still used the AuthZ path and crashed.
enable_authz_course_authoring now takes an optional role and falls back to the course creator role's own check for any role without a migrated AuthZ equivalent, regardless of the flag. See ADR 0027.
Merged to master as 0f57cf2bbf1.
Backport of: #38855 but dropping dd6c7bd since it only affects the master branch not verawood
Supporting information
Fixes openedx/openedx-authz#353
Fixes openedx/openedx-authz#354
Testing instructions
openedx-authz#353 (500 on granting course creator via Django admin)
authz.enable_course_authoringwaffle flag./admin/course_creators/coursecreator/<id>/change/.InvalidKeyErrortrying to parsecourse_key=None) was raised.openedx-authz#354 (403 on course creation despite a legacy grant)
/admin/course_creators/coursecreator/).POST /course/returns 403 (User does not have the permission to create courses in this organization or course creation is disabled).Deadline
Verawood
Other information
Include anything else that will help reviewers and consumers understand the change.