Skip to content

Trim leading spaces from MyAccount access URL #9544

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.

Sign up for GitHub

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

Jump to bottom
Open
KethniiImasha wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Trim leading spaces from MyAccount access URL #9544

KethniiImasha wants to merge 3 commits into from

Conversation

@KethniiImasha
Copy link

@KethniiImasha KethniiImasha commented Jan 10, 2026 •
edited
Loading

Purpose

Trims leading whitespace from the configured MyAccount access URL to prevent invalid redirects after password reset.
This addresses the behavior reported in issue #26498. No UI changes are introduced.

Related Issues

Fixes wso2/product-is#26498

Related PRs

N/A

Checklist

  • e2e cypress tests locally verified. (for internal contributers)
  • Manual test round performed and verified.
  • UX/UI review done on the final implementation.
  • Documentation provided. (Add links if there are any)
  • Relevant backend changes deployed and verified
  • Unit tests provided. (Add links if there are any)
  • Integration tests provided. (Add links if there are any)

Security checks

Developer Checklist (Mandatory)

  • Complete the Developer Checklist in the related product-is issue to track any behavioral change or migration impact.

Summary by CodeRabbit

  • Bug Fixes
    • Improved URL handling on the password reset completion page by trimming stray whitespace from retrieved URLs to ensure reliable navigation and processing. Trimming is applied consistently across the page’s URL retrieval paths to prevent issues caused by leading/trailing spaces and improve link resolution and user redirection.

✏️ Tip: You can customize this high-level summary in your review settings.

@CLAassistant
Copy link

CLAassistant commented Jan 10, 2026 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 10, 2026 •
edited
Loading

Walkthrough

Trimmed leading/trailing whitespace from URLs retrieved in the password reset completion page so application access and user portal URLs are sanitized before further processing.

Changes

Cohort / File(s) Summary
URL Whitespace Sanitization
identity-apps-core/apps/recovery-portal/src/main/webapp/password-reset-complete.jsp		 Wrapped applicationDataRetrieval.getApplicationAccessURL(...) and IdentityManagementEndpointUtil.getUserPortalUrl(...) calls with StringUtils.trim(); also trimmed in the isUserPortalUrl helper when deriving userPortalUrl.

Sequence Diagram(s)

(omitted — change is a small whitespace sanitization and does not introduce new multi-component control flow)

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 I found a space hiding where URLs should start,
I nibbled the edges and gave them a cart.
Trimmed whiskers of whitespace, sent links on their way,
Now apps and portals hop merrily each day. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: trimming leading spaces from MyAccount access URLs to fix redirect issues.
Linked Issues check ✅ Passed The PR successfully addresses issue #26498 by trimming leading/trailing whitespace from configured access URLs to prevent URI parsing errors when retrieving the application list in MyAccount.
Out of Scope Changes check ✅ Passed All changes are scoped to the password reset completion JSP file and focus solely on trimming whitespace from URL strings as required by issue #26498.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description is well-structured and follows the required template with clear purpose, issue references, security checks, and appropriate checklist items marked.

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

✨ Finishing touches
  • 📝 Generate docstrings

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 and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Jan 10, 2026
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
identity-apps-core/apps/recovery-portal/src/main/webapp/password-reset-complete.jsp (1)

268-272: Apply trim to the fallback URL assignment for consistency.

The primary code path (lines 146-147) trims the application access URL, but this fallback path does not. If the user portal URL configuration also contains leading/trailing spaces, the same URI parsing errors could occur.

🔧 Suggested fix 
     if (StringUtils.isBlank(applicationAccessURLWithoutEncoding)) {
-        applicationAccessURLWithoutEncoding = IdentityManagementEndpointUtil.getUserPortalUrl(
+        applicationAccessURLWithoutEncoding = StringUtils.trim(IdentityManagementEndpointUtil.getUserPortalUrl(
                 application.getInitParameter(IdentityManagementEndpointConstants.ConfigConstants.USER_PORTAL_URL),
-                tenantDomain);
+                tenantDomain));
     }
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 341e2a2 and bf5e592.

📒 Files selected for processing (1)
  • identity-apps-core/apps/recovery-portal/src/main/webapp/password-reset-complete.jsp
🔇 Additional comments (1)
identity-apps-core/apps/recovery-portal/src/main/webapp/password-reset-complete.jsp (1)

146-147: Apply trim consistently to both URL retrieval paths.

The trim operation at lines 146-147 correctly prevents URI parsing errors from leading spaces. However, the fallback path at lines 268-272 calls getUserPortalUrl() without trimming, creating an inconsistency where one code path sanitizes URLs but the other doesn't. To ensure robustness, apply StringUtils.trim() to the fallback assignment as well:

if (StringUtils.isBlank(applicationAccessURLWithoutEncoding)) {
    applicationAccessURLWithoutEncoding = StringUtils.trim(IdentityManagementEndpointUtil.getUserPortalUrl(
        application.getInitParameter(IdentityManagementEndpointConstants.ConfigConstants.USER_PORTAL_URL),
        tenantDomain));
}

Additionally, verify that automated tests cover both the primary path and fallback path with URLs containing leading/trailing spaces.

coderabbitai[bot]
coderabbitai bot reviewed Jan 10, 2026
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

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/password-reset-complete.jsp (1)

281-287: Consider trimming the portal URL in the helper method for consistency.

The isUserPortalUrl helper method (lines 283-285) retrieves the user portal URL but doesn't apply trimming. This is called earlier in the flow (lines 113, 127) to determine the application name. If the configured portal URL has leading spaces, the comparison would fail, potentially causing applicationName to be incorrectly set.

While the main redirect URLs are now trimmed at lines 146-147 and 269-273, the comparison logic could still fail due to untrimmed URLs.

🔧 Proposed fix for consistency 
 private boolean isUserPortalUrl(String callback, String tenantDomain, ServletContext application) {
 
-    String userPortalUrl = IdentityManagementEndpointUtil.getUserPortalUrl(
+    String userPortalUrl = StringUtils.trim(IdentityManagementEndpointUtil.getUserPortalUrl(
             application.getInitParameter(IdentityManagementEndpointConstants.ConfigConstants.USER_PORTAL_URL),
-            tenantDomain);
+            tenantDomain));
     return StringUtils.equals(callback, userPortalUrl);
 }
🤖 Fix all issues with AI agents 
In
@identity-apps-core/apps/recovery-portal/src/main/webapp/password-reset-complete.jsp:
- Around line 146-147: The isUserPortalUrl helper currently compares the
callback to the raw value returned by
IdentityManagementEndpointUtil.getUserPortalUrl which can contain
leading/trailing whitespace; update isUserPortalUrl to trim the returned
userPortalUrl (use StringUtils.trim on the result of
IdentityManagementEndpointUtil.getUserPortalUrl) and compare the callback
against this trimmed userPortalUrl (keep the method name isUserPortalUrl and the
comparison using StringUtils.equals).
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf5e592 and e9e278c.

📒 Files selected for processing (1)
  • identity-apps-core/apps/recovery-portal/src/main/webapp/password-reset-complete.jsp
🔇 Additional comments (1)
identity-apps-core/apps/recovery-portal/src/main/webapp/password-reset-complete.jsp (1)

269-273: LGTM! Consistent trimming for the fallback portal URL.

The trimming is correctly applied to the fallback user portal URL, maintaining consistency with the application access URL handling.

@KethniiImasha
Copy link
Author

KethniiImasha commented Jan 10, 2026 via email

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 10, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the current state of the password-reset-complete.jsp file to verify changes
cat ./identity-apps-core/apps/recovery-portal/src/main/webapp/password-reset-complete.jsp

Length of output: 22614

@KethniiImasha, thank you for applying the trim to the fallback URL assignment at lines 268-272! That change looks good and maintains consistency with the primary path. 👍

However, there's one remaining inconsistency: the isUserPortalUrl() helper method (lines 281-287) still retrieves the user portal URL without trimming. This method is called earlier in the flow at lines 113 and 127 to determine the application name.

Why this matters:
If the configured portal URL has leading/trailing whitespace, the comparison in isUserPortalUrl() would fail and return false, even though the same URL would work correctly after being trimmed at lines 268-272. This could cause the wrong application name to be set.

Suggested fix for the helper method:

private boolean isUserPortalUrl(String callback, String tenantDomain, ServletContext application) {

    String userPortalUrl = StringUtils.trim(IdentityManagementEndpointUtil.getUserPortalUrl(
            application.getInitParameter(IdentityManagementEndpointConstants.ConfigConstants.USER_PORTAL_URL),
            tenantDomain));
    return StringUtils.equals(callback, userPortalUrl);
}

This will ensure that whitespace handling is consistent throughout the entire flow—from the initial comparison logic through to the final redirect URL assignment. 🐰

@KethniiImasha
Copy link
Author

KethniiImasha commented Jan 10, 2026 via email

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 10, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the current state of the isUserPortalUrl() helper method in the file
cat ./identity-apps-core/apps/recovery-portal/src/main/webapp/password-reset-complete.jsp | grep -A 10 "private boolean isUserPortalUrl"

Length of output: 585

@KethniiImasha, excellent work! 🎉 I've verified that the StringUtils.trim() has been correctly applied to the isUserPortalUrl() helper method. The whitespace handling is now consistent across all code paths:

✅ Primary path: Application access URL trimmed (lines ~146-147)
✅ Fallback path: User portal URL trimmed (lines ~268-272)
✅ Helper method: User portal URL trimmed in comparison logic (lines ~283-285)

This ensures that leading/trailing spaces in configured URLs won't cause inconsistencies in application name determination or redirect failures. The fix comprehensively addresses the issue described in #26498. Great job! 🐰

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@KethniiImasha
Copy link
Author

KethniiImasha commented Jan 10, 2026 via email

@KethniiImasha
Copy link
Author

KethniiImasha commented Jan 10, 2026 via email

@KethniiImasha
Copy link
Author

KethniiImasha commented Jan 10, 2026 via email

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 10, 2026

Tip

For best results, initiate chat on the files or code changes.

@KethniiImasha, you're very welcome! 😊 I'm glad we could work together to ensure the whitespace handling is robust and consistent throughout the entire flow. Your thorough implementation addresses the issue comprehensively, and the fix will prevent those frustrating URI-related errors in MyAccount. Great collaboration! 🎉

(ノ^ヮ^)ノ*:・゚✧

@KethniiImasha KethniiImasha reopened this Jan 10, 2026
@KethniiImasha
Copy link
Author

KethniiImasha commented Jan 10, 2026

Hi team,

This PR fixes issue #26498 by trimming leading/trailing whitespace from the MyAccount access URL, ensuring consistent URL handling across the password reset flow. It includes three commits:

Trimmed the primary URL path.

Trimmed the fallback URL assignment.

Applied trim in isUserPortalUrl() helper method for consistency.

All checks have passed, and no UI changes were introduced. Could someone please review and approve this PR?

Thanks!

@pavinduLakshan
Copy link
Member

pavinduLakshan commented Jan 11, 2026

Hi @KethniiImasha, rather than trimming the access URL at the time it is used, we should do it at the time of persistence, i.e, when the application’s access URL is configured.

@KethniiImasha
Copy link
Author

KethniiImasha commented Jan 12, 2026

Hi @pavinduLakshan,

Thank you for the feedback! You are absolutely right trimming at persistence time is the better approach.

I have investigated the codebase and wanted to clarify where to apply the fix:

Option 1: Console UI (apps/console/src/features/applications/)

  • Trim before sending to backend API during app create/update

Option 2: Backend service layer

  • Trim when receiving the request before saving to database

Which layer would you prefer for this fix?

Also, should I remove the current JSP changes once the persistence-layer fix is implemented?

Thank you!

@pavinduLakshan
Copy link
Member

pavinduLakshan commented Jan 12, 2026

Hi @KethniiImasha,

Which layer would you prefer for this fix?

Let's add the fix in the backend because console is just an application that consumes the backend.

Also, should I remove the current JSP changes once the persistence-layer fix is implemented?

Let's keep those intact as already configured access URLs stored in the database will be handled in the JSP level.

@KethniiImasha
Copy link
Author

KethniiImasha commented Jan 12, 2026 via email •
edited
Loading

@pavinduLakshan
Copy link
Member

pavinduLakshan commented Jan 12, 2026

Identity-apps-core trim where setAccessUrl() is called (e.g., in AppPortalUtils.java) before saving.

This is for the system apps only. therefore

Identity-api-server – trim the access URL in the application management service layer before persistence.

For your fix to work for any application, we should do this. ☝️

@KethniiImasha KethniiImasha mentioned this pull request Jan 13, 2026
Open
1 task
@KethniiImasha
Copy link
Author

KethniiImasha commented Jan 13, 2026 •
edited
Loading

Hi @pavinduLakshan,

I have completed the backend fix !
Backend PR (identity-api-server)
[https://github.com/wso2/identity-api-server/pull/1049]

  • Trims access URL in createApplication()
  • Trims access URL in patchApplication()
  • Ensures all user-created applications persist clean access URLs

Frontend PR (identity-apps - this PR)

  • Keeps JSP-level trimming for system apps
  • Handles legacy data already in the database

Changes are implemented and ready for review. Thanks for your guidance on the architectural approach!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Leading spaces in Access URL cause errors when retrieving application list in MyAccount

3 participants

@KethniiImasha @CLAassistant @pavinduLakshan