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.

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

Improve domain qualified username handling when filter users by group with PRIMARY domain #591

Merged
merged 3 commits into from
Jan 9, 2025
Merged

Improve domain qualified username handling when filter users by group with PRIMARY domain #591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d403f7
Improve domain qualified username handling when filter users by group…
sadilchamishka Jan 8, 2025
a7cefda
Add unit test for filter users of group
sadilchamishka Jan 8, 2025
96d26d0
Refactor unit tests
sadilchamishka Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions ...cim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017-2024, WSO2 LLC. (http://www.wso2.com).
* Copyright (c) 2017-2025, WSO2 LLC. (http://www.wso2.com).
*
* WSO2 LLC. licenses this file to you under the Apache License,
* Version 2.0 (the "License"); you may not use this file except
Expand Down Expand Up @@ -1481,9 +1481,11 @@ private UsersGetResponse filterUsersBySingleAttribute(ExpressionNode node, Map<S
// Check that total user count matching the client query needs to be calculated.
if (isJDBCUSerStore(domainName) || isAllConfiguredUserStoresJDBC()
|| SCIMCommonUtils.isConsiderTotalRecordsForTotalResultOfLDAPEnabled()) {
int maxLimit = getMaxLimit(domainName);
int maxLimit;
if (!SCIMCommonUtils.isConsiderMaxLimitForTotalResultEnabled()) {
maxLimit = Integer.MAX_VALUE;
} else {
maxLimit = getMaxLimit(domainName);
}
Comment on lines +1484 to 1489
Copy link
Contributor Author

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.

// Get total users based on the filter query without depending on pagination params.
if (SCIMCommonUtils.isGroupBasedUserFilteringImprovementsEnabled() &&
Expand Down Expand Up @@ -1791,7 +1793,10 @@ private int getUserCountByGroup(Node node, String domainName)
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);
if (StringUtils.isNotEmpty(domainName) && StringUtils
.containsNone(attributeValue, CarbonConstants.DOMAIN_SEPARATOR)) {
attributeValue = domainName.toUpperCase() + CarbonConstants.DOMAIN_SEPARATOR + attributeValue;
}
Comment on lines -1794 to +1799
Copy link
Contributor Author

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


try {
List<String> roleNames = getRoleNames(attributeName, filterOperation, attributeValue);
Expand Down
43 changes: 43 additions & 0 deletions ....common/src/test/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.wso2.carbon.identity.scim2.common.internal.SCIMCommonComponentHolder;
import org.wso2.carbon.user.core.UserStoreClientException;
import org.wso2.carbon.user.core.common.PaginatedUserResponse;
import org.wso2.carbon.user.core.model.UniqueIDUserClaimSearchEntry;
import org.wso2.charon3.core.exceptions.NotImplementedException;
import org.wso2.charon3.core.extensions.UserManager;
import org.wso2.charon3.core.objects.plainobjects.UsersGetResponse;
Expand Down Expand Up @@ -94,6 +95,7 @@
import org.wso2.charon3.core.utils.codeutils.SearchRequest;
import org.wso2.carbon.identity.configuration.mgt.core.model.Resource;

import java.io.IOException;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -148,6 +150,8 @@ public class SCIMUserManagerTest {

@Mock
private AbstractUserStoreManager mockedUserStoreManager;
@Mock
private JDBCUserStoreManager mockedJDBCUserStoreManager;

@Mock
private ClaimManager mockedClaimManager;
Expand Down Expand Up @@ -613,6 +617,45 @@ public void testFilteringUsersWithGET(List<org.wso2.carbon.user.core.common.User
assertEquals(result.getUsers().size(), expectedResultCount);
}

@Test
public void testFilteringUsersOfGroupWithGET() throws UserStoreException, IOException, BadRequestException,
NotImplementedException, CharonException {

String domain = "PRIMARY";
SCIMUserManager scimUserManager = new SCIMUserManager(mockedUserStoreManager, mockedClaimManager);
SCIMResourceTypeSchema schema = SCIMResourceSchemaManager.getInstance().getUserResourceSchema();
FilterTreeManager filterTreeManager = new FilterTreeManager("groups eq admin", schema);
Node node = filterTreeManager.buildTree();

org.wso2.carbon.user.core.common.User testUser1 = new org.wso2.carbon.user.core.common.User(UUID.randomUUID()
.toString(), "testUser1", "testUser1");
testUser1.setUserStoreDomain("PRIMARY");
List<org.wso2.carbon.user.core.common.User> filteredUsers = new ArrayList<>();
filteredUsers.add(testUser1);

scimCommonUtils.when(() -> SCIMCommonUtils.convertLocalToSCIMDialect(anyMap(), anyMap())).thenReturn(new HashMap<String, String>() {{
put(SCIMConstants.CommonSchemaConstants.ID_URI, "1f70378a-69bb-49cf-aa51-a0493c09110c");
}});

when(mockedUserStoreManager.getSecondaryUserStoreManager(domain)).thenReturn(mockedJDBCUserStoreManager);
when(mockedJDBCUserStoreManager.isSCIMEnabled()).thenReturn(true);
scimCommonUtils.when(SCIMCommonUtils::isGroupBasedUserFilteringImprovementsEnabled).thenReturn(true);
when(mockedUserStoreManager.getRoleNames(anyString(), anyInt(), anyBoolean(), anyBoolean(), anyBoolean()))
.thenReturn(new String[]{"admin"});
when(mockedUserStoreManager.getUserCountForGroup(anyString())).thenReturn(filteredUsers.size());
when(mockedUserStoreManager.getUserListOfGroupWithID(anyString())).thenReturn(filteredUsers);

UniqueIDUserClaimSearchEntry uniqueIDUserClaimSearchEntry = new UniqueIDUserClaimSearchEntry();
List<UniqueIDUserClaimSearchEntry> uniqueIDUserClaimSearchEntries = new ArrayList<>();
uniqueIDUserClaimSearchEntries.add(uniqueIDUserClaimSearchEntry);
when(mockedUserStoreManager.getUsersClaimValuesWithID(any(), any(), nullable(String.class)))
.thenReturn(uniqueIDUserClaimSearchEntries);

UsersGetResponse result = scimUserManager.listUsersWithGET(node, 1, null, null, null, domain, new HashMap<>());
assertEquals(result.getUsers().size(), filteredUsers.size());

}

@DataProvider(name = "userInfoForFiltering")
public Object[][] userInfoForFiltering() {

Expand Down
Loading