-
Notifications
You must be signed in to change notification settings - Fork 3
Granular permission control #252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
core/src/main/java/dev/vml/es/acm/core/servlet/StateServlet.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
core/src/main/java/dev/vml/es/acm/core/servlet/EventServlet.java
Outdated
Show resolved
Hide resolved
core/src/main/java/dev/vml/es/acm/core/servlet/StateServlet.java
Outdated
Show resolved
Hide resolved
...ent/src/main/content/jcr_root/conf/acm/settings/snippet/available/core/condition/changed.yml
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 86 out of 88 changed files in this pull request and generated 11 comments.
Files not reviewed (1)
- test/e2e/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ui.frontend/src/types/main.ts
Outdated
| features: { | ||
| 'console.execute': true, | ||
| 'console.view': true, | ||
| 'dashboard.view': true, |
Copilot
AI
Nov 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dashboard.view feature is defined in the FeatureId type and StateDefault permissions (lines 74, 112) but is not implemented in the backend Java Permissions.Feature enum. This inconsistency means the dashboard permission check will always fail on the backend, even though it's defined in the frontend.
Either:
- Add
DASHBOARD_VIEWto the JavaFeatureenum inPermissions.java, or - Remove
dashboard.viewfrom the frontendFeatureIdtype andStateDefaultif dashboard access doesn't need permission control.
fixes #244