feat: implement view and edit permission checks for pages and resources and advanced settings#3031
Conversation
|
Thanks for the pull request, @bra-i-am! 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. |
5bf47e2 to
bf1f32a
Compare
27c140e to
2768600
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3031 +/- ##
==========================================
+ Coverage 95.53% 95.55% +0.01%
==========================================
Files 1393 1393
Lines 33009 32999 -10
Branches 7403 7402 -1
==========================================
- Hits 31535 31532 -3
+ Misses 1419 1413 -6
+ Partials 55 54 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
7bed3bd to
c842c4a
Compare
8d98c60 to
699af15
Compare
bradenmacdonald
left a comment
There was a problem hiding this comment.
Thanks @bra-i-am ! The code looks good to me. ✅
Once someone else has tested it and merged the other PRs, I will approve and merge this for you. I have not tested it myself, just read the code.
|
As a suggestion remove the page and resources scope in the PR title and just describe I think my comment about displaying/hiding option in the navigation header suits better here, so I just reference it #2991 (comment) Specially because the header implantation for Advanced Settings displayed is still validating "can manage" instead of "can view" permission. Screencast.from.2026-05-09.12-27-27.webmBeside that all is working as described: Advanced settings Screencast.from.2026-05-09.12-44-31.webmPages and Resources Screencast.from.2026-05-09.12-47-22.webm |
c9af2f3 to
cfc7598
Compare
|
@bra-i-am Is this ready to merge? |
- Add VIEW_ADVANCED_SETTINGS and PAGE_AND_RESOURCES permissions
- Add getPagesAndResourcesPermissions helper
- Calculate isEditable and isReadOnly from user permissions
- Propagate isEditable via PagesAndResourcesContext
- Add disabled={!isEditable} to all forms, toggles, and buttons
- Update AppCard and AppListNextButton with isEditable logic
- Change default isEditable to false (fail closed)
- Add unified permission gate showing PermissionDeniedAlert
…/resources access
…d refactor related components
… related components
…ove accessibility attributes
…onfigForm and PageCard tests
ef39044 to
5f4b0f6
Compare
|
@dcoa, @bradenmacdonald: thanks a lot for your help with this PR! I turned this PR into a draft while I solved some doubts about openedx/openedx-authz#272 and openedx/openedx-platform#38462, which is required to make some changes work as expected. |
09a559c to
7a59df6
Compare
…proved permission handling
|
Given the roles and permission sets already discussed at https://openedx.atlassian.net/wiki/spaces/OEPM/pages/5638619138/Authoring+Roles+and+Permissions#2.-Roles-and-permission-sets, neither
This PR was modified so that Advanced Settings remains a binary allow/deny gate (based on
cc. @MaferMazu |
|
@bradenmacdonald, @dcoa: could you please help me review this again? 🙏 |



Description
Implements role-based access control (RBAC) for the Pages and Resources section and adds permission-gated gear icons for Progress and Wiki apps, using the existing
MANAGE_ADVANCED_SETTINGSpermission.This PR closes #2933.
What this PR does
Pages and Resources (closes #2933)
VIEW_PAGES_AND_RESOURCESandMANAGE_PAGES_AND_RESOURCESpermission constants tosrc/authz/constants.tsgetPagesAndResourcesPermissionshelper insrc/authz/permissionHelpers.tsisEditablefrom resolved permissions:!isAuthzEnabled || !!canManagePagesAndResourcesisEditable = true— legacy behavior fully preservedisEditableistrueonly for users with MANAGE permissionisEditablethroughPagesAndResourcesContextto all child componentsdisabled={!isEditable}to all interactive elements: app cards inAppCard.jsx, the "Next" button inAppListNextButton.jsx, app list inAppList.jsx, page setting buttons inPageSettingButton.jsx, and app settings modal controls inAppSettingsModal.jsxPermissionDeniedAlertgate for users who lack both VIEW and MANAGE permissionsProgress and Wiki gear icons
getAdvancedSettingsPermissionshelper insrc/authz/permissionHelpers.tsPageSettingButtonfetchescanManageAdvancedSettingsdirectly using its owncourseIdprop (no context pollution)MANAGE_ADVANCED_SETTINGSHeader menu (hooks refactor)
useContentMenuItemsanduseSettingMenuItemsnow useuseCourseUserPermissionsinstead of the lower-leveluseUserPermissions, eliminating manualisAuthzEnabledguards and per-hook fallback logichooks.tsxreplaced with helper functions frompermissionHelpers.tsCOURSE_PERMISSIONSis no longer imported inhooks.tsxRole impact
course_auditorcourse_editorcourse_staff/course_adminTesting instructions
AUTHZ_COURSE_AUTHORING_FLAGfeature flag.course_auditorrole to a test user for a specific course.PermissionDeniedAlertmust be shown.course_editor: Pages & Resources must be fully interactive, Advanced Settings must showPermissionDeniedAlert, and Progress and Wiki gear icons must be disabled.course_stafforcourse_adminand verify full access everywhere.AUTHZ_COURSE_AUTHORING_FLAGand verify legacy fallback works correctly for all roles.Best Practices Checklist
src/authz/permissionHelpers.test.ts)isEditablepropagated via React Context — no prop drillingcanManageAdvancedSettingsfetched at point of use — no context pollutiongetXPermissions) consistently across the codebase