-
Notifications
You must be signed in to change notification settings - Fork 152
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.
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
Improve domain qualified username handling when filter users by group with PRIMARY domain #591
Improve domain qualified username handling when filter users by group with PRIMARY domain #591
Conversation
… with PRIMARY domain
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #591 +/- ##
============================================
+ Coverage 44.45% 44.85% +0.39%
+ Complexity 1035 957 -78
============================================
Files 41 41
Lines 8943 8945 +2
Branches 1925 1926 +1
============================================
+ Hits 3976 4012 +36
+ Misses 4092 4054 -38
- Partials 875 879 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
int maxLimit; | ||
if (!SCIMCommonUtils.isConsiderMaxLimitForTotalResultEnabled()) { | ||
maxLimit = Integer.MAX_VALUE; | ||
} else { | ||
maxLimit = getMaxLimit(domainName); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a code refactoring. No need to find the max limit of the user store if it is overridden by Integer max value.
attributeValue = UserCoreUtil.addDomainToName(((ExpressionNode) node).getValue(), domainName); | ||
if (StringUtils.isNotEmpty(domainName) && StringUtils | ||
.containsNone(attributeValue, CarbonConstants.DOMAIN_SEPARATOR)) { | ||
attributeValue = domainName.toUpperCase() + CarbonConstants.DOMAIN_SEPARATOR + attributeValue; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix for the issue. Another similar place referred for this fix
PR builder started |
PR builder completed |
PR builder started |
PR builder completed |
There was a problem hiding this comment.
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/12686626379
The PRIMARY domain has to be appended when filtering the groups by name. If domain is not specified, all the groups from all the user stores will be listed where the filtering condition is matched.
Here is a similar place[1] where same logic is followed, where domain is appended to the group(role) name instead of using the
UserCoreUtil.addDomainToName
method which skip adding "PRIMARY" domain for the groups.[1] https://github.com/wso2-extensions/identity-inbound-provisioning-scim2/blob/master/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java#L2091~L2098
Related Issues