Fix URL encoding for claims parameter in self-registration redirect#10382
Fix URL encoding for claims parameter in self-registration redirect#10382iRAFEEK wants to merge 1 commit into
Conversation
|
|
📝 WalkthroughWalkthroughThe JSP now URL-encodes the ChangesSelf-Registration Callback Validation
Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
identity-apps-core/apps/recovery-portal/src/main/webapp/self-registration-with-verification.jsp (1)
194-199:⚠️ Potential issue | 🟠 MajorFix potential double URL-encoding in
callbackvalidation.
Inself-registration-with-verification.jsp,callbackis already encoded viaIdentityManagementEndpointUtil.encodeURL(request.getParameter("callback"))(line 107) and is later used as-is for the hiddencallbackvalue (line 646) and the sign-in link (line 931). However, validation re-encodes it by callingIdentityManagementEndpointUtil.getURLEncodedCallback(callback)(lines 195-199);getURLEncodedCallbackalso URL-encodes/rebuilds the query parameters, so validation can run against a double-encoded URL that differs from the actualcallbackbeing submitted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@identity-apps-core/apps/recovery-portal/src/main/webapp/self-registration-with-verification.jsp` around lines 194 - 199, Validation re-encodes an already-encoded callback causing possible double-encoding; in self-registration-with-verification.jsp, stop calling IdentityManagementEndpointUtil.getURLEncodedCallback(callback) before validation and instead validate the same form value that will be submitted (the already-encoded callback from IdentityManagementEndpointUtil.encodeURL(request.getParameter("callback"))), i.e., pass callback directly into AuthenticationEndpointUtil.isValidMultiOptionURI(callback) or, if the validator expects an unencoded URL, first decode the callback once and validate the decoded value—update the code paths that call IdentityManagementEndpointUtil.getURLEncodedCallback to use the original encoded callback or a single decode step to ensure validation checks the exact value sent to the form.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@identity-apps-core/apps/recovery-portal/src/main/webapp/self-registration-with-verification.jsp`:
- Around line 194-199: Validation re-encodes an already-encoded callback causing
possible double-encoding; in self-registration-with-verification.jsp, stop
calling IdentityManagementEndpointUtil.getURLEncodedCallback(callback) before
validation and instead validate the same form value that will be submitted (the
already-encoded callback from
IdentityManagementEndpointUtil.encodeURL(request.getParameter("callback"))),
i.e., pass callback directly into
AuthenticationEndpointUtil.isValidMultiOptionURI(callback) or, if the validator
expects an unencoded URL, first decode the callback once and validate the
decoded value—update the code paths that call
IdentityManagementEndpointUtil.getURLEncodedCallback to use the original encoded
callback or a single decode step to ensure validation checks the exact value
sent to the form.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 0ecb30c5-8480-4323-9fb8-f78b00dfc115
📒 Files selected for processing (1)
identity-apps-core/apps/recovery-portal/src/main/webapp/self-registration-with-verification.jsp
iRAFEEK
force-pushed
the
fix-url-encoding-claims
branch
from
ea88714 to
2c7b15d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@identity-apps-core/apps/recovery-portal/src/main/webapp/self-registration-with-verification.jsp`:
- Around line 195-197: The validation uses
AuthenticationEndpointUtil.isValidMultiOptionURI(callback) but the comment
incorrectly assumes callback is URL-encoded while the value at input (rendering
side) was only HTML-attribute encoded via Encode.forHtmlAttribute(...); update
the flow to validate against the raw request parameter properly URL-encoded
(derive a URL-encoded version from the raw request param before calling
AuthenticationEndpointUtil.isValidMultiOptionURI) and keep
Encode.forHtmlAttribute only where the callback is rendered to the page; adjust
usage of the callback variable so the validation step uses the URL-encoded form
while the view rendering continues to use the HTML-attribute-encoded form.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 400fba53-4bd2-456e-a179-6a8cccf8a9ea
📒 Files selected for processing (1)
identity-apps-core/apps/recovery-portal/src/main/webapp/self-registration-with-verification.jsp
iRAFEEK
force-pushed
the
fix-url-encoding-claims
branch
from
2c7b15d to
b19a3fd
Compare
|
Could you add a screen recording of how your fix works in the PR description? and address the coderabbit bot comments too? |
The callback URL containing the claims JSON parameter was not URL-encoded before being rendered in the 'Back to sign in' link, causing a 400 error. This change wraps the callback parameter with encodeURL() to properly percent-encode query parameter values while preserving the URL structure. Fixes wso2/product-is#23288
iRAFEEK
force-pushed
the
fix-url-encoding-claims
branch
from
b19a3fd to
8cd7ff6
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Done 👍 Added a before/after recording (400 → 200) to the PR description, and the CodeRabbit comment is resolved. Ready for another look — thanks! |
|
Can I know how did you test this change? did you test with the latest master branch of identity-apps and product-is or use older versions? I'm asking because this bug seems to be already fixed in the master branch, in which case, we do not need an additional fix at all. Could you please verify? |
PS: @iRAFEEK could you please check on this? |
Thanks for checking! I tested with released IS packs — 7.1.0 (where the issue was filed) and 7.3.0 — not a fresh master + product-is build. On 7.3.0 I found the normal self-registration flow renders self-registration-username-request.jsp, whose "Back to sign in" link is already fixed with encodeURL on master — so the user-facing 400 doesn't reproduce through the normal or org/My-Account flow there (it returns 200). The line this PR changes, in self-registration-with-verification.jsp, is still un-encoded on master, but I could only trigger it by rendering that page directly (via signup.do) — not through a real user flow. So you're likely right that the bug is already effectively fixed on master. If self-registration-with-verification.jsp is no longer reached in any flow, this fix isn't needed and I'm happy to close the PR. Could you confirm whether that page is still used? If it is reachable somewhere, I'm glad to keep the fix; otherwise I'll close it. Thanks for the guidance! |
3 participants
Purpose
Fixes wso2/product-is#23288
The "Back to sign in" link on the self-registration page fails with a 400 error because the
claimsJSON parameter in the callback URL is not URL-encoded.Problem
The
callbackparameter was only HTML-encoded usingEncode.forHtmlAttribute(), which does not handle URL-illegal characters like{,},",:. These characters remained raw in the query string, causing the server to reject the request with 400 Bad Request.Example broken URL:
.../authenticationendpoint/login.do?claims={"userinfo":{"email":{"essential":true}}}
Solution
Wrapped
request.getParameter("callback")withIdentityManagementEndpointUtil.encodeURL()before HTML encoding (line 107). This properly percent-encodes query parameter values while preserving the URL structure. This matches the existing pattern already used inself-registration-username-request.jsp:116.Removed a now-redundant
IdentityManagementEndpointUtil.getURLEncodedCallback()call in the callback validation block (lines 194-199). Since the callback is URL-encoded at the point it is read (step 1), re-encoding it there would double-encode the query parameters (e.g.%7B->%257B). The already-encoded value is validated directly, which is correct becauseAuthenticationEndpointUtil.isValidMultiOptionURI()validates only the scheme/host/path and ignores the query string.Changed File
identity-apps-core/apps/recovery-portal/src/main/webapp/self-registration-with-verification.jsp(callback read at line 107; callback validation at lines 194-199)Verification
Tested on a local WSO2 Identity Server (7.1.0) against
self-registration-with-verification.jsp.Before — the "Back to sign in" link points to
.../login.do?claims={"userinfo":...}with raw braces; the server rejects it with HTTP 400:2026-06-12.02-26-19.mov
After (with this fix) — the link is percent-encoded
.../login.do?claims=%7B%22userinfo%22...and loads the sign-in page with HTTP 200:2026-06-12.02-44-05.mov