wso2 / carbon-kernel

Apache License 2.0
146 stars 657 forks source link

Skip group contains not existing user cases #4122

Closed sadilchamishka closed 1 week ago

sadilchamishka commented 1 week ago

Purpose

When listing users from a group of LDAP, there can be members as users and groups.

Getting user list of role: roleone with filter: *
Searching role: roleone SearchBase: ou=Groups,dc=wso2,dc=org SearchFilter: (&(objectClass=groupOfNames)(cn=roleone))
Found role: cn=roleone,ou=Groups,dc=WSO2,dc=ORG
Found attribute: member value: uid=testuser172,ou=Users,dc=WSO2,dc=ORG
Found attribute: member value: uid=testuser246,ou=Users,dc=WSO2,dc=ORG
Found attribute: member value: cn=roletwo,ou=Groups,dc=WSO2,dc=ORG

As we are not supporting nested groups, we have to skip the member attributes which includes as groups. As user-id of corresponding members are null, we can skip those instead of trying to resolve the user object.

jenkins-is-staging commented 1 week ago

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

sadilchamishka commented 1 week ago
Getting name attributes of: uid=testuser246,ou=Users,dc=WSO2,dc=ORG
UserName: testuser246
UserID: c8d2145f-7f76-4232-aa7e-b9f45eede041
testuser246 is added to the result list

Getting name attributes of: cn=roletwo,ou=Groups,dc=WSO2,dc=ORG
UserID: null

For the normal cases, username & userid is resolved, but for these nested group cases, userid will be null and username also will be null as it is not resolved. Hence trying to fetch user without userid and username is useless.

ThaminduDilshan commented 1 week ago

what if the group search base and user search base are the same + group id attribute of the group is the same as the user id attribute + group name attribute and username attribute are the same?

I would suggest fixing this by checking if entries are of the type that is configured for the user entry object class and ignoring the rest

IMO this is a valid concern. However wouldn't checking for the object class require performing additional ldap queries for each user? AFAIK we cannot retrieve properties (i.e. objectClass) of members with the group search query.

sadilchamishka commented 1 week ago

Merging the PR while analyze and send a fix for the raised concern through another PR.

jenkins-is-staging commented 1 week ago

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

sadilchamishka commented 1 week ago

The raised concern is valid. But we can't invoke a ldap query in order fetch the object class to check whether the it is user or group. But if there exist a customer with nested groups, and search bases are same as mentioned in the above comments, some of the nested groups will be listed as users. In such cases based on a config with acknowledging the performance we can give that support.

tharakawijekoon commented 1 week ago

But if there exist a customer with nested groups, and search bases are same as mentioned in the above comments, some of the nested groups will be listed as users.

At this point we have the object class info, why don't we omit these results since we already have the info at this point so there will be no confusion? With the current fix plus this we won't need a config.