Skip to content
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

Paginated User search returns an incorrect totalResults when applying a filter #339

Open
racosta-dev opened this issue Mar 24, 2021 · 0 comments

Comments

@racosta-dev
Copy link

Description:
There is an wrong behaviour with the SCIM2 User paginated search whenever you apply a filter to the search that causes totalResults pagination metadata to return an incorrect value: instead of reporting the actual total results count, it just reports your current result size both in the itemsPerPage and totalResults parameters.

When you call POST /Users/.search or GET /Users, SCIMUserManager.listUsersWithGET() method is internally called.

    @Override
    public List<Object> listUsersWithPost(SearchRequest searchRequest, Map<String, Boolean> requiredAttributes)
            throws CharonException, NotImplementedException, BadRequestException {

        return listUsersWithGET(searchRequest.getFilter(), (Integer)searchRequest.getStartIndex(),
                (Integer)searchRequest.getCount(), searchRequest.getSortBy(), searchRequest.getSortOder(),
                searchRequest.getDomainName(), requiredAttributes);
    }

This method calls listUsers() when there's no informed filter, and filterUsers() when there's one.

    @Override
    public List<Object> listUsersWithGET(Node rootNode, Integer startIndex, Integer count, String sortBy,
                                         String sortOrder, String domainName, Map<String, Boolean> requiredAttributes)
            throws CharonException, NotImplementedException {

        // Validate NULL value for startIndex.
        startIndex = handleStartIndexEqualsNULL(startIndex);
        if (sortBy != null || sortOrder != null) {
            throw new NotImplementedException("Sorting is not supported");
        } else if (count != null && count == 0) {
            return Collections.emptyList();
        } else if (rootNode != null) {
            return filterUsers(rootNode, requiredAttributes, startIndex, count, sortBy, sortOrder, domainName);
        } else {
            return listUsers(requiredAttributes, startIndex, count, sortBy, sortOrder, domainName);
        }
    }

Internally, listUsers() calculates total count and then sets it in response (List[0]):

            //...

            if (canPaginate(offset, limit)) {
                coreUsers = listUsernamesAcrossAllDomains(offset, limit, sortBy, sortOrder);

                String[] userStoreDomainNames = getDomainNames();
                boolean canCountTotalUserCount = canCountTotalUserCount(userStoreDomainNames);
                if (canCountTotalUserCount) {
                    for (String userStoreDomainName : userStoreDomainNames) {
                        totalUsers += getTotalUsers(userStoreDomainName);
                    }
                }
            }

            //...
            List<Object> scimUsers = getUserDetails(coreUsers, requiredAttributes);
            if (totalUsers != 0) {
                users.set(0, Math.toIntExact(totalUsers)); // Set total number of results to 0th index.
            } else {
                users.set(0, scimUsers.size());
            }
            users.addAll(scimUsers); // Set user details from index 1.

However, when filterUsers() is called, whether it calls filterUsersBySingleAttribute() or getMultiAttributeFilteredUsers() it is using the current search result size as a total result.

In the case of filterUsersBySingleAttribute(), a getDetailedUsers() method is called after retrieving users:

    private List<Object> filterUsersBySingleAttribute(ExpressionNode node, Map<String, Boolean> requiredAttributes,
                                                      int offset, int limit, String sortBy, String sortOrder,
                                                      String domainName) throws CharonException {

        Set<org.wso2.carbon.user.core.common.User> users;

        if (log.isDebugEnabled()) {
            log.debug(String.format("Listing users by filter: %s %s %s", node.getAttributeValue(), node.getOperation(),
                    node.getValue()));
        }
        // Check whether the filter operation is supported by the users endpoint.
        if (isFilteringNotSupported(node.getOperation())) {
            String errorMessage =
                    "Filter operation: " + node.getOperation() + " is not supported for filtering in users endpoint.";
            throw new CharonException(errorMessage);
        }
        domainName = resolveDomainName(domainName, node);
        try {
            // Check which APIs should the filter needs to follow.
            if (isUseLegacyAPIs(limit)) {
                users = filterUsersUsingLegacyAPIs(node, limit, offset, domainName);
            } else {
                users = filterUsers(node, offset, limit, sortBy, sortOrder, domainName);
            }
        } catch (NotImplementedException e) {
            String errorMessage = String.format("System does not support filter operator: %s", node.getOperation());
            throw new CharonException(errorMessage, e);
        }

        return getDetailedUsers(users, requiredAttributes);
    }
    private List<Object> getDetailedUsers(Set<org.wso2.carbon.user.core.common.User> coreUsers,
                                          Map<String, Boolean> requiredAttributes)
            throws CharonException {

        List<Object> filteredUsers = new ArrayList<>();
        // 0th index is to store total number of results.
        filteredUsers.add(0);

        // Set total number of filtered results.
        filteredUsers.set(0, coreUsers.size());

        // Get details of the finalized user list.
        filteredUsers.addAll(getFilteredUserDetails(coreUsers, requiredAttributes));
        return filteredUsers;
    }

...and it sets search size as total result.

On the other hand, getMultiAttributeFilteredUsers() sets directly total results (List[0]) as users size:

    private List<Object> getMultiAttributeFilteredUsers(Node node, Map<String, Boolean> requiredAttributes, int offset,
                                                        int limit, String sortBy, String sortOrder, String domainName)
            throws CharonException {

        List<Object> filteredUsers = new ArrayList<>();
        // 0th index is to store total number of results.
        filteredUsers.add(0);
        Set<org.wso2.carbon.user.core.common.User> users;
        // Handle pagination.
        if (limit > 0) {
            users = getFilteredUsersFromMultiAttributeFiltering(node, offset, limit, sortBy, sortOrder, domainName);
            filteredUsers.set(0, users.size());
            filteredUsers.addAll(getFilteredUserDetails(users, requiredAttributes));
        } else {
            int maxLimit = getMaxLimit(domainName);
            if (StringUtils.isNotEmpty(domainName)) {
                users = getFilteredUsersFromMultiAttributeFiltering(node, offset, maxLimit, sortBy,
                        sortOrder, domainName);
                filteredUsers.set(0, users.size());
                filteredUsers.addAll(getFilteredUserDetails(users, requiredAttributes));
            } else {
                int totalUserCount = 0;
                // If pagination and domain name are not given, then perform filtering on all available user stores.
                while (carbonUM != null) {
                    // If carbonUM is not an instance of Abstract User Store Manger we can't get the domain name.
                    if (carbonUM instanceof AbstractUserStoreManager) {
                        domainName = carbonUM.getRealmConfiguration().getUserStoreProperty("DomainName");
                        users = getFilteredUsersFromMultiAttributeFiltering(node, offset, maxLimit,
                                sortBy, sortOrder, domainName);
                        totalUserCount += users.size();
                        filteredUsers.addAll(getFilteredUserDetails(users, requiredAttributes));
                    }
                    carbonUM = (AbstractUserStoreManager) carbonUM.getSecondaryUserStoreManager();
                }
                //set the total results
                filteredUsers.set(0, totalUserCount);
            }
        }
        return filteredUsers;
    }

So, when we make the following request:

{
  "schemas": [
    "urn:ietf:params:scim:api:messages:2.0:SearchRequest"
  ],
  "attributes": [
    "name",
    "userName",
    "emails",
    "dateOfBirth",
    "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User",
    "groups"
  ],
  "filter": "groups eq roleName",
  "domain": "PRIMARY",
  "startIndex": 1,
  "count": 2
}

It gives us a result like this:

{
    "totalResults": 2,
    "startIndex": 1,
    "itemsPerPage": 2,
    //...
}

When the actual total results is 3, as seen in the example below (where a count of 10 is requested):

{
    "totalResults": 3,
    "startIndex": 1,
    "itemsPerPage": 3,
    //...
}

Suggested Labels:

Suggested Assignees:

Affected Product Version:
From 5.10.0 to latest (master)

OS, DB, other environment details and versions:
Windows, H2, WSO2IS-KM-5.10.0

Steps to reproduce:

  • Make sure you have more than 2 registered users that will match search criteria.
  • Make a POST /Users/.search request with the following information (i.e: with a given filter and a count size equal to 2):
{
  "schemas": [
    "urn:ietf:params:scim:api:messages:2.0:SearchRequest"
  ],
  "attributes": [
    "name",
    "userName",
    "emails",
    "dateOfBirth",
    "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User",
    "groups"
  ],
  "filter": "groups eq roleName",
  "domain": "PRIMARY",
  "startIndex": 1,
  "count": 2
}

And it will return a totalResults count with the same value as the itemsPerPage attribute:

{
    "totalResults": 2,
    "startIndex": 1,
    "itemsPerPage": 2,
    //...
}

Related Issues:

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

No branches or pull requests

1 participant