Skip to content

feat: implement view and edit permission checks for pages and resources and advanced settings#3031

Open
bra-i-am wants to merge 15 commits into
openedx:masterfrom
eduNEXT:bc/implement-pages-and-resourcer-permissions
Open

feat: implement view and edit permission checks for pages and resources and advanced settings#3031
bra-i-am wants to merge 15 commits into
openedx:masterfrom
eduNEXT:bc/implement-pages-and-resourcer-permissions

Conversation

@bra-i-am
Copy link
Copy Markdown
Contributor

@bra-i-am bra-i-am commented Apr 27, 2026

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_SETTINGS permission.

This PR closes #2933.

What this PR does

Pages and Resources (closes #2933)

  • Adds VIEW_PAGES_AND_RESOURCES and MANAGE_PAGES_AND_RESOURCES permission constants to src/authz/constants.ts
  • Creates getPagesAndResourcesPermissions helper in src/authz/permissionHelpers.ts
  • Computes isEditable from resolved permissions: !isAuthzEnabled || !!canManagePagesAndResources
    • When Authz is disabled: isEditable = true — legacy behavior fully preserved
    • When Authz is enabled: isEditable is true only for users with MANAGE permission
  • Propagates isEditable through PagesAndResourcesContext to all child components
  • Applies disabled={!isEditable} to all interactive elements: app cards in AppCard.jsx, the "Next" button in AppListNextButton.jsx, app list in AppList.jsx, page setting buttons in PageSettingButton.jsx, and app settings modal controls in AppSettingsModal.jsx
  • Adds a PermissionDeniedAlert gate for users who lack both VIEW and MANAGE permissions

Progress and Wiki gear icons

  • Adds getAdvancedSettingsPermissions helper in src/authz/permissionHelpers.ts
  • PageSettingButton fetches canManageAdvancedSettings directly using its own courseId prop (no context pollution)
  • Disables the settings gear icon for the Progress and Wiki apps when the user lacks MANAGE_ADVANCED_SETTINGS
  • All other app gears are unaffected

Header menu (hooks refactor)

  • useContentMenuItems and useSettingMenuItems now use useCourseUserPermissions instead of the lower-level useUserPermissions, eliminating manual isAuthzEnabled guards and per-hook fallback logic
  • All inline permission objects in hooks.tsx replaced with helper functions from permissionHelpers.ts
  • COURSE_PERMISSIONS is no longer imported in hooks.tsx

Role impact

Role Pages & Resources Advanced Settings Progress / Wiki gear
course_auditor View only (inputs disabled) Permission denied Disabled
course_editor Full access Permission denied Disabled
course_staff / course_admin Full access Full access Enabled
Any role, Authz disabled Full access — legacy unchanged Full access — legacy unchanged Enabled

Testing instructions

  1. Enable the AUTHZ_COURSE_AUTHORING_FLAG feature flag.
  2. Assign the course_auditor role to a test user for a specific course.
  3. Log in as that user and navigate to Pages and Resources: all toggle switches, "Save" buttons, and app cards must be non-interactive, the "Next" button in the discussions app list must be disabled, and the Progress and Wiki gear icons must be disabled.
  4. Navigate to Advanced Settings: a PermissionDeniedAlert must be shown.
  5. Log in as a course_editor: Pages & Resources must be fully interactive, Advanced Settings must show PermissionDeniedAlert, and Progress and Wiki gear icons must be disabled.
  6. Log in as a course_staff or course_admin and verify full access everywhere.
  7. Disable AUTHZ_COURSE_AUTHORING_FLAG and verify legacy fallback works correctly for all roles.

Best Practices Checklist

  • New files use TypeScript (src/authz/permissionHelpers.test.ts)
  • isEditable propagated via React Context — no prop drilling
  • canManageAdvancedSettings fetched at point of use — no context pollution
  • No new Redux state added
  • No backend dependencies introduced
  • Uses existing permission helper pattern (getXPermissions) consistently across the codebase

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Apr 27, 2026
@openedx-webhooks
Copy link
Copy Markdown

openedx-webhooks commented Apr 27, 2026

Thanks for the pull request, @bra-i-am!

This repository is currently maintained by @bradenmacdonald.

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 approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To 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:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
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:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@github-project-automation github-project-automation Bot moved this to Needs Triage in Contributions Apr 27, 2026
@bra-i-am bra-i-am changed the title Bc/implement pages and resourcer permissions feat(pages-and-resources): add read-only access for course_auditor role Apr 27, 2026
@bra-i-am bra-i-am force-pushed the bc/implement-pages-and-resourcer-permissions branch from 5bf47e2 to bf1f32a Compare April 27, 2026 23:41
@bra-i-am bra-i-am marked this pull request as ready for review April 27, 2026 23:41
@bra-i-am bra-i-am marked this pull request as draft April 27, 2026 23:42
@bra-i-am bra-i-am force-pushed the bc/implement-pages-and-resourcer-permissions branch from 27c140e to 2768600 Compare April 28, 2026 16:37
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.55%. Comparing base (70191bf) to head (72f45c3).
⚠️ Report is 1 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bra-i-am bra-i-am force-pushed the bc/implement-pages-and-resourcer-permissions branch 2 times, most recently from 7bed3bd to c842c4a Compare April 28, 2026 21:38
@mphilbrick211 mphilbrick211 moved this from Needs Triage to Waiting on Author in Contributions Apr 29, 2026
@bra-i-am bra-i-am force-pushed the bc/implement-pages-and-resourcer-permissions branch 2 times, most recently from 8d98c60 to 699af15 Compare April 29, 2026 14:55
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change disables this:

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change disables these inputs:

Image

@bra-i-am bra-i-am changed the title feat(pages-and-resources): add read-only access for course_auditor role feat(page-and-resources): implement view and edit permission checks Apr 30, 2026
@bra-i-am bra-i-am marked this pull request as ready for review April 30, 2026 14:26
Copy link
Copy Markdown
Contributor

@bradenmacdonald bradenmacdonald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/advanced-settings/AdvancedSettings.tsx Outdated
@dcoa
Copy link
Copy Markdown
Contributor

dcoa commented May 9, 2026

As a suggestion remove the page and resources scope in the PR title and just describe implement view and edit permission checks for pages and resources and advanced settings, because what I noticed is this PR also refactor the previous Advanced Settings validation.

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.webm

Beside that all is working as described:

Advanced settings

Screencast.from.2026-05-09.12-44-31.webm

Pages and Resources

Screencast.from.2026-05-09.12-47-22.webm

@bra-i-am bra-i-am changed the title feat(page-and-resources): implement view and edit permission checks feat: implement view and edit permission checks for pages and resources and advanced settings May 13, 2026
@bra-i-am bra-i-am force-pushed the bc/implement-pages-and-resourcer-permissions branch 3 times, most recently from c9af2f3 to cfc7598 Compare May 13, 2026 16:22
@bradenmacdonald
Copy link
Copy Markdown
Contributor

@bra-i-am Is this ready to merge?

bra-i-am added 12 commits May 14, 2026 09:44
- 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
@bra-i-am bra-i-am force-pushed the bc/implement-pages-and-resourcer-permissions branch from ef39044 to 5f4b0f6 Compare May 14, 2026 14:54
@bra-i-am bra-i-am marked this pull request as draft May 14, 2026 16:31
@bra-i-am
Copy link
Copy Markdown
Contributor Author

bra-i-am commented May 14, 2026

@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.

@bra-i-am bra-i-am force-pushed the bc/implement-pages-and-resourcer-permissions branch from 09a559c to 7a59df6 Compare May 14, 2026 20:26
@bra-i-am
Copy link
Copy Markdown
Contributor Author

bra-i-am commented May 14, 2026

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 course_auditor nor course_editor will be able to access Advanced Settings — they both lack MANAGE_ADVANCED_SETTINGS. Because of this, the read-only mode approach for Advanced Settings was dropped, making the following PRs unnecessary:

This PR was modified so that Advanced Settings remains a binary allow/deny gate (based on MANAGE_ADVANCED_SETTINGS), and the Pages and Resources sections that link to Advanced Settings — Progress and Wiki — have their gear icons disabled when the user lacks that permission:

image

cc. @MaferMazu

@bra-i-am bra-i-am marked this pull request as ready for review May 14, 2026 22:21
@bra-i-am bra-i-am requested a review from bradenmacdonald May 14, 2026 22:21
@bra-i-am
Copy link
Copy Markdown
Contributor Author

@bradenmacdonald, @dcoa: could you please help me review this again? 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Status: Waiting on Author

Development

Successfully merging this pull request may close these issues.

Task - RBAC Authz - Implement frontend check for Pages and resources page

5 participants