Granular permission control #252
Open
+608
−266
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| HealthStatus healthStatus = healthChecker.checkStatus(); | ||
| MockStatus mockStatus = mockHttpFilter.checkStatus(); | ||
| State state = new State(spaSettings, healthStatus, mockStatus, instanceInfo.getSettings()); | ||
| Permissions permissions = new Permissions(executor.authorize(Code.consoleMinimal(), request.getResourceResolver())); |
Collaborator
Author
There was a problem hiding this comment.
check flags under /apps/acm/[features/permissions]/[console|scripts|history|snippets] node instead
krystian-panek-vmltech
changed the title
Copilot finished reviewing on behalf of
krystian-panek-vmltech
November 14, 2025 14:34
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements granular permission control for ACM, replacing the single OSGi configuration flag (scriptManagementEnabled) with a fine-grained, repository-based permission system. Users can now control access to individual features through JCR node permissions at /apps/acm/feature/*, enabling more flexible and secure access management.
Key Changes:
- Introduces a new
Permissionsclass that checks feature access based on JCR node existence - Adds permission checks at both UI (frontend) and API (backend) levels for comprehensive authorization
- Updates all servlets and components to enforce feature-based permissions
Reviewed Changes
Copilot reviewed 67 out of 68 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
ui.frontend/vite.config.ts |
Updates build path from /apps/acm/spa/ to /apps/acm/gui/spa/build/ |
ui.frontend/src/types/main.ts |
Adds Permissions type and StateDefault constant, removes scriptManagementEnabled flag |
ui.frontend/src/types/input.ts |
Code style fix: converts double quotes to single quotes for consistency |
ui.frontend/src/router.tsx |
Wraps all routes with permission-protected Route component |
ui.frontend/src/pages/ScriptView.tsx |
Integrates permission check for script execution feature |
ui.frontend/src/pages/ConsolePage.tsx |
Adds permission checks for console execution and script management |
ui.frontend/src/hooks/app.ts |
Adds useFeatureEnabled hook for checking feature permissions |
ui.frontend/src/components/* |
Updates multiple components to use permission-based feature toggles |
ui.frontend/src/Route.tsx |
New component that redirects unauthorized users based on feature permissions |
ui.frontend/src/App.tsx |
Refactors to use StateDefault instead of inline state initialization |
ui.apps/src/main/content/jcr_root/apps/acm/feature/**/.content.xml |
Creates JCR nodes for each feature to enable permission-based access control |
ui.apps/src/main/content/jcr_root/apps/acm/api/.content.xml |
Removes execute-script API node as part of restructuring |
ui.apps/src/main/content/jcr_root/apps/acm/.content.xml |
Adds feature directory and removes spa directory reference |
core/src/main/java/.../util/ServletResult.java |
Adds forbidden() method for HTTP 403 responses |
core/src/main/java/.../state/State.java |
Adds permissions field to state object |
core/src/main/java/.../state/Permissions.java |
New class implementing feature-based permission checking via JCR nodes |
core/src/main/java/.../servlet/StateServlet.java |
Includes permissions in state response |
core/src/main/java/.../servlet/ScriptServlet.java |
Replaces OSGi config check with permission-based authorization |
core/src/main/java/.../servlet/QueueCodeServlet.java |
Adds authorization check before queuing code execution |
core/src/main/java/.../servlet/ExecuteScriptServlet.java |
Removes servlet file (functionality integrated elsewhere) |
core/src/main/java/.../servlet/ExecuteCodeServlet.java |
Adds authorization check before code execution |
core/src/main/java/.../servlet/EventServlet.java |
Enforces maintenance.manage permission for event dispatch |
core/src/main/java/.../gui/SpaSettings.java |
Removes scriptManagementEnabled configuration option |
core/src/main/java/.../gui/Spa.java |
Updates assets path to match new build directory structure |
core/src/main/java/.../code/Executor.java |
Adds authorize() methods to verify both feature permissions and script availability |
core/src/main/java/.../code/Executable.java |
Renames constant from ID_CONSOLE to CONSOLE_ID and adds CONSOLE_SCRIPT_PATH |
core/src/main/java/.../code/CodePrintStream.java |
Refactors method names from printStamped/withLoggerTimestamps to printTimestamped/setLoggerTimestamps |
core/src/main/java/.../AcmConstants.java |
Adds APPS_ROOT constant for feature path resolution |
ui.content/src/main/content/jcr_root/conf/acm/settings/snippet/**/*.yml |
Updates snippets with improved documentation and new condition_changed snippet |
ui.content.example/src/main/content/jcr_root/conf/acm/settings/script/manual/example/*.groovy |
Updates example scripts to use out instead of log for output messages |
README.md |
Adds comprehensive documentation on feature and API permissions configuration |
.gitignore |
Updates ignored build path to match new structure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import dev.vml.es.acm.core.state.Permissions; | ||
|
|
||
| import java.io.IOException; | ||
| import java.security.Permission; |
There was a problem hiding this comment.
Unused import java.security.Permission should be removed. This import was added but is not used anywhere in the code.
Suggested change
| import java.security.Permission; |
Comment on lines
7
to
9
| import dev.vml.es.acm.core.code.Code; | ||
| import dev.vml.es.acm.core.code.ExecutionQueue; | ||
| import dev.vml.es.acm.core.code.Executor; |
There was a problem hiding this comment.
Unused imports should be removed. The imports Code, ExecutionQueue, and Executor are added but not used in the servlet code.
Suggested change
| import dev.vml.es.acm.core.code.Code; | |
| import dev.vml.es.acm.core.code.ExecutionQueue; | |
| import dev.vml.es.acm.core.code.Executor; |
...ent/src/main/content/jcr_root/conf/acm/settings/snippet/available/core/condition/changed.yml
Show resolved
Hide resolved
Comment on lines
+13
to
+14
| if (featureId && !state.permissions.features[featureId]) { | ||
| return <Navigate to="/" replace />; |
There was a problem hiding this comment.
Potential infinite redirect loop if dashboard.view permission is denied. When a user lacks permission for a feature, they're redirected to "/" (dashboard). However, if they also lack dashboard.view permission, this creates a redirect loop. Consider implementing a fallback route or showing an "Access Denied" page when no features are accessible.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #244
scripts/management (instead of osgi config)