Get `totalResults` Value by Total User Count in SCIM2 User API #547

dhaura · 2024-03-29T11:08:25Z

Purpose

When the SCIM API is invoked with a filter, the current behavior involves returning a predefined value for the totalResults parameter based on the MaxUserNameListLength, rather than reflecting the actual results that match the filter criteria. This fix addresses the issue by accurately determining the user count that corresponds to the given filter.
Furthermore, when invoking user endpoint with groups filter by passing an role (hybrid or system roles), still the endpoint returns a list of userd which a linked to the passed role. But when role group seperation is enabled, it should return an empty list since the passed value is for a role (not for a group). This is also fixed with this PR.
To enable these fixes, the following config should be added.

[scim2]
enable_group_based_user_filter_improvements = true

Related Issues

SCIM returns faulty totalResults when the filter parameter is given with pagination parameters wso2/product-is#12813

Related PRs

…TotalResultsByUserCount config is enabled, retrieve users only for groups in scim2 user endpoint group filter.

CLAassistant · 2024-03-29T11:08:33Z

All committers have signed the CLA.

...y.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java

…ovements.

mpmadhavig · 2024-04-04T03:39:33Z

@dhaura could you please list out all the flows you covered while testing.

Also, if you haven't done already, test this with count query param added in the request.

count > 100 -> returned count 100 and totalResults should give the actual amount.
count < 100 -> returned count should be the specified amount and totalResults should give the actual amount.

dhaura · 2024-04-04T04:08:32Z

@mpmadhavig I tested this fix with the SCIM2 user endpoint groups filter using API call [1] for following scenarios.

Groups created by organization admin. (ex: Stundet)
- For 10 and 110 users per group.
Hybrid Roles: Administrator, everyone

[1] -

curl --location 'https://<IS_DOMAIN:PORT>/t/<TENANT_DOMAIN>/scim2/Users?filter=groups%2Beq%2BStudent' \
--header 'Authorization: Bearer <TOKEN>'

Also, if you haven't done already, test this with count query param added in the request.

count > 100 -> returned count 100 and totalResults should give the actual amount.

count = 100

count = 105

count < 100 -> returned count should be the specified amount and totalResults should give the actual amount.

count = 10

count = 50

jenkins-is-staging · 2024-04-10T05:32:03Z

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

jenkins-is-staging · 2024-04-10T07:06:38Z

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

jenkins-is-staging

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

ShanChathusanda93 · 2024-06-01T17:10:32Z

...y.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java

+                    // Get the total user count by the filter query.
+                    // This is only implemented for JDBC userstores.


Can we make this a single line comment? Or else format as a multi line comment?

Addressed with #553

ShanChathusanda93 · 2024-06-01T17:11:23Z

...y.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java

@@ -1457,6 +1464,30 @@ private UsersGetResponse filterUsersBySingleAttribute(ExpressionNode node, Map<S
        return getDetailedUsers(filteredUsers, totalResults);
    }

+    /**
+     * method to get user count by filtering parameter.


Suggestion : Get user count by filtering parameter.

Addressed with #553

ShanChathusanda93 · 2024-06-01T17:14:24Z

...y.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java

@@ -1694,6 +1730,54 @@ private Set<org.wso2.carbon.user.core.common.User> filterUsers(Node node, int of
        }
    }

+    /**
+     * Method to get User Count by Group filter


Suggestion : Get user count by group filter

Addressed with #553

ShanChathusanda93 · 2024-06-01T17:16:37Z

...y.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java

+        String attributeValue = ((ExpressionNode) node).getValue();
+
+        /*
+        If there is a domain and if the domain separator is not found in the attribute value, append the domain
+        with the domain separator in front of the new attribute value.
+        */
+        attributeValue = UserCoreUtil.addDomainToName(((ExpressionNode) node).getValue(), domainName);


Do we need the extra assignment in 1749?

Addressed with #553

asha15 and others added 6 commits March 18, 2024 09:24

get totalResults by user count

dc634b7

add return statement

38df20b

bump kernel version

92fc824

Fix checkstyle issues

9ce165e

For JDBC userstores, if both role group separation and EnableRetrieve…

cdabb8f

…TotalResultsByUserCount config is enabled, retrieve users only for groups in scim2 user endpoint group filter.

Remove JDBC check in filterUsersByGroup method.

0e094f3

dhaura and others added 2 commits March 29, 2024 16:39

Merge branch 'master' into DP-enable-totalresults-by-user-count

cf636c7

Remove JDBC check in filterUsersByGroup method.

2fb85ef

dhaura commented Apr 1, 2024

View reviewed changes

...y.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java Outdated Show resolved Hide resolved

dhaura added 3 commits April 2, 2024 12:58

Rename identity.xml scim2 config to EnableGroupBasedUserFilteringImpr…

6971d70

…ovements.

Use new total user count method for groups only for JDBC userstores.

dbd790b

Fix formatting issues.

41d5fb4

asha15 mentioned this pull request Apr 3, 2024

Get totalResults value by user count #533

Closed

Bump kernel version.

efc6e70

jenkins-is-staging approved these changes Apr 10, 2024

View reviewed changes

ashensw approved these changes Apr 10, 2024

View reviewed changes

sumedhe merged commit 906897b into wso2-extensions:master Apr 15, 2024
3 checks passed

dhaura mentioned this pull request Apr 15, 2024

Bump Scim2 and Framework Versions wso2/product-is#20197

Merged

ShanChathusanda93 reviewed Jun 1, 2024

View reviewed changes

dhaura mentioned this pull request Jun 3, 2024

Improve Code Formatting #553

Open

This was referenced Aug 1, 2024

Enable SCIM2 Group based User Filtering Improvements by Default wso2/carbon-identity-framework#5830

Merged

[Migration] Enable SCIM2 Group based User Filtering Improvements by Default wso2/product-is#20820

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get `totalResults` Value by Total User Count in SCIM2 User API #547

Get `totalResults` Value by Total User Count in SCIM2 User API #547

dhaura commented Mar 29, 2024 •

edited

Loading

CLAassistant commented Mar 29, 2024 •

edited

Loading

mpmadhavig commented Apr 4, 2024

dhaura commented Apr 4, 2024

jenkins-is-staging commented Apr 10, 2024

jenkins-is-staging commented Apr 10, 2024

jenkins-is-staging left a comment

ShanChathusanda93 Jun 1, 2024

dhaura Jun 3, 2024

ShanChathusanda93 Jun 1, 2024

dhaura Jun 3, 2024

ShanChathusanda93 Jun 1, 2024

dhaura Jun 3, 2024

ShanChathusanda93 Jun 1, 2024

dhaura Jun 3, 2024

		// Get the total user count by the filter query.
		// This is only implemented for JDBC userstores.

Get totalResults Value by Total User Count in SCIM2 User API #547

Get totalResults Value by Total User Count in SCIM2 User API #547

Conversation

dhaura commented Mar 29, 2024 • edited Loading

Purpose

Related Issues

Related PRs

CLAassistant commented Mar 29, 2024 • edited Loading

mpmadhavig commented Apr 4, 2024

dhaura commented Apr 4, 2024

jenkins-is-staging commented Apr 10, 2024

jenkins-is-staging commented Apr 10, 2024

jenkins-is-staging left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Get `totalResults` Value by Total User Count in SCIM2 User API #547

Get `totalResults` Value by Total User Count in SCIM2 User API #547

dhaura commented Mar 29, 2024 •

edited

Loading

CLAassistant commented Mar 29, 2024 •

edited

Loading