fix(jans-auth-server): escaped postLogoutUri in EndSessionUtils by yuriyz · Pull Request #14103 · JanssenProject/jans (original) (raw)

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7458543f-5ff7-48ac-8718-50e625d8e71e

📥 Commits

Reviewing files that changed from the base of the PR and between 84f5206 and 519182a.

📒 Files selected for processing (1)

jans-auth-server/server/src/test/java/io/jans/as/server/session/ws/rs/EndSessionUtilsTest.java

📝 Walkthrough

Walkthrough

Escape front-channel logout URIs for HTML attributes and post-logout URLs for JavaScript context, fix a helper method name, and add tests verifying escaping and appendState behavior.

Changes

End-Session Output Encoding Security Fix

Layer / File(s)	Summary
EndSessionUtils escaping refactoring jans-auth-server/server/src/main/java/io/jans/as/server/session/ws/rs/EndSessionUtils.java	Adds StringEscapeUtils import and applies escapeHtml4() to each logoutUri inserted into iframe src and escapeEcmaScript() to postLogoutUrl in the generated script.
EndSessionRestWebServiceImpl call site correction jans-auth-server/server/src/main/java/io/jans/as/server/session/ws/rs/EndSessionRestWebServiceImpl.java	Updates the httpBased call to use the corrected createFrontChannelHtml() method name.
EndSessionUtilsTest escaping validation jans-auth-server/server/src/test/java/io/jans/as/server/session/ws/rs/EndSessionUtilsTest.java	Adds TestNG tests that validate HTML-escaping of logoutUri, ECMAScript-escaping of postLogoutUrl and state, safe handling of sequences, correct benign rendering, and appendState duplication avoidance.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

JanssenProject/jans#13466: Modifies EndSessionUtils front-channel logout HTML generation in the same utility area.

Suggested reviewers

yurem
yuriyzz 🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: escaping postLogoutUri in EndSessionUtils, which aligns with the core security fix implemented across the utility class and its callers.
Description check	✅ Passed	The description includes the target issue (#14104), confirms no documentation impact, and follows the template structure, though implementation details section is minimal.
Linked Issues check	✅ Passed	The PR addresses a security fix for XSS vulnerability by escaping HTML and JavaScript contexts in logout URIs and post-logout URLs, which aligns with the auto-created issue's intent to track this security improvement.
Out of Scope Changes check	✅ Passed	All changes are in scope: method name correction, security escaping enhancements in EndSessionUtils, related caller updates, and comprehensive test coverage for the escaping functionality.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches 📝 Generate docstrings

Create stacked PR
Commit on current branch 🧪 Generate unit tests (beta)
Create PR with unit tests
Commit unit tests in branch jans-auth-server-issues-02

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