Only system.admin must be able to list exports #12303 - #12309
Conversation
ExportService.list() enumerated the exports directory for any caller. The only gate was in the HTTP layer, so any app calling exportLib.list() from server-side JS could see every export name. Move the check into ExportServiceImpl.list(), following the same requireAdminRole() pattern as NodeServiceImpl.enumerate(), and throw ForbiddenAccessException when the caller lacks role:system.admin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHqacRZ9V5qnpc35sP8YUF
Codacy's Analysis Summary0 new issue (≤ 0 issue)
|
There was a problem hiding this comment.
Pull request overview
This PR tightens security around node export discovery by enforcing that only callers with the system.admin role can list available exports, aligning the core export service behavior with the intended administrative-only access model.
Changes:
- Added an admin-role authorization gate to
ExportServiceImpl.list()that throwsForbiddenAccessExceptionfor non-admin callers. - Documented the
system.adminrequirement in both the JavaExportServiceAPI and the JavaScriptexport.list()JSDoc. - Updated/extended unit tests to run existing list tests under an admin context and to assert forbidden behavior for unauthenticated and non-admin callers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| modules/lib/lib-export/src/main/resources/lib/xp/export.ts | Documents that export.list() requires the system.admin role. |
| modules/core/core-export/src/test/java/com/enonic/xp/core/impl/export/ExportServiceImplTest.java | Wraps list calls in an admin context and adds authorization-focused test cases. |
| modules/core/core-export/src/main/java/com/enonic/xp/core/impl/export/ExportServiceImpl.java | Enforces system.admin role requirement in list() via requireAdminRole(). |
| modules/core/core-api/src/main/java/com/enonic/xp/export/ExportService.java | Adds JavaDoc describing the authorization requirement and possible exception. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #12309 +/- ##
=========================================
Coverage 87.10% 87.10%
- Complexity 20845 20847 +2
=========================================
Files 2610 2610
Lines 69358 69367 +9
Branches 5749 5750 +1
=========================================
+ Hits 60415 60423 +8
Misses 6270 6270
- Partials 2673 2674 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
This change adds authorization checks to the
ExportService.list()method to ensure only users with the administrator role can list available node exports.Key Changes
requireAdminRole()method that checks if the current user has thesystem.adminrole and throwsForbiddenAccessExceptionif notlist()method requires thesystem.adminrole and can throwForbiddenAccessExceptionADMIN_CONTEXTstatic field to provide an admin-authenticated context for test executionlistAsAdmin()helper method to wrap allexportService.list()calls with admin contextlistAsAdmin()instead of direct callslist_requires_the_administrator_role()- verifies exception when called without authenticationlist_requires_the_administrator_role_for_authenticated_user()- verifies exception when called with non-admin rolesystem.adminrole requirementImplementation Details
The authorization check uses
ContextAccessor.current().getAuthInfo()to retrieve the current authentication context and validates that the user has theADMINrole before allowing the operation. This follows the existing security pattern in the codebase.https://claude.ai/code/session_01LHqacRZ9V5qnpc35sP8YUF