Skip to content

Improve username claim validation to handle domain-prefixed usernames during JIT provisioning - #4578

Open
Zeta201 wants to merge 2 commits into
wso2:4.12.xfrom
Zeta201:improve-add-user
Open

Improve username claim validation to handle domain-prefixed usernames during JIT provisioning#4578
Zeta201 wants to merge 2 commits into
wso2:4.12.xfrom
Zeta201:improve-add-user

Conversation

@Zeta201

@Zeta201 Zeta201 commented Apr 10, 2026

Copy link
Copy Markdown

Problem

Self registration of a user to a secondary userstore fails with:

WorkflowException: Username and the username claim value should be same.

This issue is caused by a flaw in the existing username claim validation logic in AbstractUserStoreManager:

String userNameWithoutDomain = UserCoreUtil.removeDomainFromName(userName);
if (claims != null && claims.containsKey(USERNAME_CLAIM_URI) &&
!claims.get(USERNAME_CLAIM_URI).equals(userNameWithoutDomain)) {
throw new UserStoreException("Username and the username claim value should be same.");
}

Root Cause

The current logic removes the domain prefix from userName (e.g., PRIMARY/user1 → user1) before comparison, but does not normalize the USERNAME_CLAIM_URI claim value.

This leads to incorrect mismatches in scenarios such as:

  • If the claim value has no domain (e.g., user1) and the username includes a domain (e.g., PRIMARY/user1), the comparison may incorrectly fail.

  • If both the username and claim value include a secondary user store domain (e.g., SECONDARY/John), removing the domain only from the username results in:

    John != SECONDARY/John

    causing a false mismatch.


Fix

Normalize both values by applying UserCoreUtil.removeDomainFromName() to the claim value as well:

!UserCoreUtil.removeDomainFromName(claims.get(USERNAME_CLAIM_URI))
.equals(userNameWithoutDomain)

This ensures the comparison is always performed between domain-free usernames.


Behavior Comparison

userName Claim Value Existing Logic Result Fixed Logic Result
PRIMARY/user1 user1 ❌ user1 != user1 (false mismatch) ✅ user1 == user1
SECONDARY/John SECONDARY/John ❌ SECONDARY/John != John ✅ John == John
user1 user1 ✅ user1 == user1 ✅ user1 == user1

Scope of Fix

The fix is applied to both:

  • addUser

  • addUserWithID

methods in AbstractUserStoreManager.

@coderabbitai

coderabbitai Bot commented Apr 10, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Username-claim validation in user creation was changed to normalize both the stored username and the claim value by removing userstore domain prefixes (with a null-check on the claim) before comparison; a related comment was adjusted.

Changes

Cohort / File(s) Summary
Username claim validation normalization
core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/common/AbstractUserStoreManager.java
In addUser and addUserWithID, null-check the username claim and compare UserCoreUtil.removeDomainFromName(usernameClaimValue) to userNameWithoutDomain; updated comment wording to reflect domain-normalization.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically describes the main change: improving username claim validation to handle domain-prefixed usernames during JIT provisioning, which directly matches the fix in AbstractUserStoreManager.
Description check ✅ Passed PR description comprehensively covers the problem, root cause, fix, behavior comparison, and scope, exceeding the template requirements for a technical fix.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

@wso2-engineering wso2-engineering Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Agent Log Improvement Checklist

⚠️ Warning: AI-Generated Review Comments

  • The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
  • Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.

✅ Before merging this pull request:

  • Review all AI-generated comments for accuracy and relevance.
  • Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
Comment Accepted (Y/N) Reason
#### Log Improvement Suggestion No: 1
#### Log Improvement Suggestion No: 2

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/common/AbstractUserStoreManager.java`:
- Around line 5096-5097: The null value for the username claim can cause a
NullPointerException when calling
UserCoreUtil.removeDomainFromName(claims.get(USERNAME_CLAIM_URI))); update the
conditional in AbstractUserStoreManager to first retrieve the claim value into a
local (e.g., String usernameClaim = claims.get(USERNAME_CLAIM_URI)), check for
null, and if null throw a UserStoreException (or handle per existing error flow)
instead of invoking removeDomainFromName; apply the identical null-guard and
exception handling to the duplicated occurrence referenced at the second
location so both spots use the safe local variable + null check before calling
UserCoreUtil.removeDomainFromName.
🪄 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: 7d74f544-6dde-4776-8657-169980454745

📥 Commits

Reviewing files that changed from the base of the PR and between 4d63763 and 67ca166.

📒 Files selected for processing (1)
  • core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/common/AbstractUserStoreManager.java

@jenkins-is-staging

Copy link
Copy Markdown

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/24228862055

@jenkins-is-staging

Copy link
Copy Markdown

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/24228862055
Status: success

@jenkins-is-staging jenkins-is-staging left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/24228862055

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