fixes #244: Prevent REST API plugin from exposing its own configuration - #245
Open
guusdk wants to merge 9 commits into
Open
fixes #244: Prevent REST API plugin from exposing its own configuration#245guusdk wants to merge 9 commits into
guusdk wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
There are a few concrete correctness/quality issues in the updated code and UI (notably misleading 403 error messaging and minor cleanup) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses issue #244 by preventing the REST API plugin’s /system/properties endpoints from exposing or allowing modification of system properties that configure the REST API plugin itself (notably authentication-related settings). It does so by migrating plugin settings to Openfire’s typed SystemProperty API (scoped to plugin "REST API") and by filtering/forbidding access to all properties owned by that plugin.
Changes:
- Migrates REST API plugin configuration (enabled/auth type/secret/allowed IPs/custom auth filter/logging) from raw
JiveGlobalsaccess to typedSystemPropertydefinitions. - Adds enforcement in
SystemControllerto filter plugin-owned properties from listings and reject GET/PUT/POST/DELETE on those keys with HTTP 403. - Updates admin UI usage, OpenAPI annotations, i18n property descriptions, and adds Hurl e2e coverage for forbidden keys.
File summaries
| File | Description |
|---|---|
| test/system.hurl | Adds e2e assertions that plugin-owned system property keys return 403 for GET/PUT/DELETE. |
| src/web/rest-api.jsp | Updates admin UI to use typed SystemProperty values and enum-based auth type handling. |
| src/java/org/jivesoftware/openfire/plugin/rest/service/UserServiceLegacy.java | Switches legacy user service checks to typed SystemProperty reads. |
| src/java/org/jivesoftware/openfire/plugin/rest/service/SystemService.java | Documents 403 responses for prohibited system property create/update/delete operations. |
| src/java/org/jivesoftware/openfire/plugin/rest/service/JerseyWrapper.java | Uses typed SystemProperty reads to decide whether to load the custom auth filter. |
| src/java/org/jivesoftware/openfire/plugin/rest/service/CustomOpenApiResource.java | Updates OpenAPI security scheme selection to use typed auth type. |
| src/java/org/jivesoftware/openfire/plugin/rest/RESTServicePlugin.java | Introduces typed SystemProperty definitions for plugin configuration and removes PropertyEventListener-based state. |
| src/java/org/jivesoftware/openfire/plugin/rest/controller/UserServiceController.java | Switches service logging toggle to typed SystemProperty. |
| src/java/org/jivesoftware/openfire/plugin/rest/controller/SystemController.java | Filters out plugin-owned properties and forbids access/modification (403) via REST endpoints. |
| src/java/org/jivesoftware/openfire/plugin/rest/controller/MUCServiceController.java | Switches service logging toggle to typed SystemProperty. |
| src/java/org/jivesoftware/openfire/plugin/rest/controller/MUCRoomController.java | Switches service logging toggle to typed SystemProperty. |
| src/java/org/jivesoftware/openfire/plugin/rest/controller/ClusteringController.java | Switches service logging toggle to typed SystemProperty. |
| src/java/org/jivesoftware/openfire/plugin/rest/AuthFilter.java | Switches enabled/auth-type/secret/allowed-IPs checks to typed SystemProperty. |
| src/i18n/restapi_i18n.properties | Adds system_property.* descriptions for newly-typed REST API plugin properties. |
| src/i18n/restapi_i18n_nl.properties | Adds Dutch translations for the new system_property.* descriptions. |
| plugin.xml | Adds <priorToServerVersion> metadata to constrain compatibility. |
| changelog.html | Adds a changelog entry for #244. |
Review details
- Files reviewed: 17/17 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…wn configuration The REST API plugin's system properties endpoints (GET/POST/PUT/DELETE /plugins/restapi/v1/system/properties) allowed reading and writing any Openfire system property, including the properties that control the REST API plugin's own behavior. This allowed a caller with REST API access to durably reconfigure the plugin's own authentication, amongst others.
guusdk
force-pushed
the
244_do-not-expose-own-config
branch
from
September 9, 2026 14:31
351ad1b to
952e736
Compare
|
Warning Review limit reachedNext included review available in 24 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (17)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
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.
The REST API plugin's system properties endpoints (GET/POST/PUT/DELETE /plugins/restapi/v1/system/properties) allowed
reading and writing any Openfire system property, including the properties that control the REST API plugin's own behavior This allowed a caller with REST API access to durably reconfigure the plugin's own authentication, amongst others
This PR:
plugin.restapi.*property is covered automatically.