Skip to content

fixes #250: Migrate the admin console page to JSTL - #264

Merged
Fishbowler merged 1 commit into
igniterealtime:mainfrom
guusdk:250_migrate-admin-console-to-jstl
Sep 25, 2026
Merged

Fishbowler merged 1 commit into
igniterealtime:mainfrom
guusdk:250_migrate-admin-console-to-jstl

Conversation

@guusdk

@guusdk guusdk commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Replace the scriptlet-based rendering of rest-api.jsp with JSTL and the Openfire admin taglib (admin:infobox, admin:contentBox), move all translatable content to the i18n files, escape user-provided values, and add CSRF protection to the settings form.

Summary by CodeRabbit

  • New Features
    • Added a localized REST API settings page with controls for service access, authentication, allowed IP addresses, and logging.
    • Added CSRF checks and validation for settings changes, with clear status and error messages.
    • Changes to custom authentication settings now reload the REST API plugin.
  • Documentation
    • Updated the 1.12.1 changelog to include the admin console migration and security updates.

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

Next included review available in 25 minutes.

Check out review usage here.

View limit details

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: f767d329-39c4-41e8-b99a-de82028389ed

📥 Commits

Reviewing files that changed from the base of the PR and between 2eba1a8 and 003f603.

⛔ Files ignored due to path filters (3)
  • src/web/images/error-16x16.gif is excluded by !**/*.gif
  • src/web/images/success-16x16.gif is excluded by !**/*.gif
  • src/web/images/warning-16x16.gif is excluded by !**/*.gif
📒 Files selected for processing (3)
  • src/i18n/restapi_i18n.properties
  • src/i18n/restapi_i18n_nl.properties
  • src/web/rest-api.jsp
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: c1a0b01a-68ea-49f5-a1d8-9ef35f0d22a1

📥 Commits

Reviewing files that changed from the base of the PR and between 6f119ef and 2eba1a8.

⛔ Files ignored due to path filters (3)
  • src/web/images/error-16x16.gif is excluded by !**/*.gif
  • src/web/images/success-16x16.gif is excluded by !**/*.gif
  • src/web/images/warning-16x16.gif is excluded by !**/*.gif
📒 Files selected for processing (4)
  • changelog.html
  • plugin.xml
  • src/i18n/restapi_i18n.properties
  • src/web/rest-api.jsp
Files not reviewed due to moderation or processing errors (4)
  • src/web/rest-api.jsp
  • src/i18n/restapi_i18n.properties
  • plugin.xml
  • changelog.html

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The REST API admin settings page now uses localized JSTL rendering and a POST form with CSRF validation. Valid saves update settings and log the edit. Saves involving custom authentication reload the plugin and redirect to its administration page.

Changes

REST API admin settings

Layer / File(s) Summary
Validate and process settings saves
src/web/rest-api.jsp
Save requests must include a CSRF token matching the cookie. The page validates authentication settings, updates valid settings, logs the edit, and reloads the plugin when custom authentication is selected or currently active.
Render localized settings page
src/i18n/restapi_i18n.properties, plugin.xml, src/web/rest-api.jsp, changelog.html
The admin-console item and settings page use localization keys. The page renders current configuration, status messages, warnings, and errors. The changelog adds an entry for issue #250.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant rest-api.jsp
  participant REST API plugin
  participant WebManager
  Browser->>rest-api.jsp: POST settings and CSRF token
  rest-api.jsp->>rest-api.jsp: Compare request token with CSRF cookie
  rest-api.jsp->>REST API plugin: Validate custom authentication filter
  rest-api.jsp->>WebManager: Log settings edit and plugin reload
  rest-api.jsp->>Browser: Redirect after save
Loading

Suggested reviewers: fishbowler

Merge Risk: ⚪ Minimal · up to 2eba1

No concrete issue is established that would block merging, though the changed settings page still needs normal validation.

Architecture Summary

Architecture risk: 🔵 Low · up to 2eba1

The change affects 3 systems.

Changed systems: src, changelog.html, plugin.xml

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (ui) was modified; 2 changed files map to changed impact.
  • observed — changelog.html (service) was modified; 1 changed file maps to changed impact.
  • observed — plugin.xml (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in changelog.html: Added a 1.12.1 changelog entry for issue #250 describing the admin console page’s migration to JSTL, translatable content, and CSRF protection.
  • observed — Modified behavior in plugin.xml: The REST API admin-console item replaces its hard-coded name and description with the restapi.admin.item.settings.name and restapi.admin.item.settings.description message keys; its ID and URL are unchanged.
  • observed — Modified behavior in src/i18n/restapi_i18n.properties: Added localized REST API admin settings labels and descriptions for service status, basic/secret/custom-filter authentication, allowed IP addresses, and logging, plus documentation, save/error messages, and notices about forwarded client addresses and custom-filter reloads.
  • observed — Modified behavior in src/web/rest-api.jsp: Replaces wildcard and legacy administration setup and request handling with explicit imports, WebManager initialization, and parameter parsing during saves. Saves now require a matching CSRF cookie and request parameter; invalid authentication types use an empty error value, and custom authentication validates its filter through the loaded REST API plugin. The previous code did not check CSRF and stored the authentication-type error as "invalid value".
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: migrating the admin console page to JSTL. It matches the changeset and references issue #250.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Warning

Review coverage is incomplete: 4 files could not be fully reviewed. Findings from completed review steps are included; see review info for details.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Fishbowler Fishbowler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. Live test looks positive.

  • NL i18n?
  • Some description for Additional Logging?

Replace the scriptlet-based rendering of rest-api.jsp with JSTL and the Openfire admin taglib (admin:infobox, admin:contentBox), move all translatable content to the i18n files, escape user-provided values, and add CSRF protection to the settings form.
@guusdk
guusdk force-pushed the 250_migrate-admin-console-to-jstl branch from 2eba1a8 to 003f603 Compare September 25, 2026 15:07
@Fishbowler
Fishbowler merged commit f870853 into igniterealtime:main Sep 25, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants