zakird/pyad

Checking if the user is in group or if group has the user as a member returns inaccurate result

Andrewkha opened this issue · 3 comments

Hi

Getting the user and the group with the following statements:

group = adgroup.ADGroup.from_cn('group_name')
user = aduser.ADUser.from_cn('user_name')

The user is a member of the group. However
group.check_contains_member(user))
or
user.is_member_of(group)
both return False. What am I doing wrong?

It looks like there is an underlying but in ADUser.__eq__(). The following code failed on our end (trimming personal info, obviously):

>>> v = pyad.from_dn("CN=Example CN Test,OU=Managed,OU=Groups,DC=example,DC=org,DC=domain,DC=com")
>>> u = [x for x in v.get_members()] 
>>> j = pyad.from_cn('Doe, John',search_base=dn) 
>>> v.check_contains_member(j) 
False
>>> j
<ADUser 'CN=Doe\, John,OU=Workers,DC=example,DC=org,DC=domain,DC=com'>
>>> u[-1]
<ADUser 'CN=Doe\, John,OU=Workers,DC=example,DC=org,DC=domain,DC=com'>
>>> j == u[-1]
False 
>>> hash(u[-1]) == hash(j)
True

It looks like it's still using __cmp__(). That's no longer checked in python3 (iirc), so that would cause the bug.

It looks like it doesn't process anything except the first name in the list. I modified it to the below. I don't have access to upload to the project, but i'm pasting it here to assist others.

I don't have push access, but i fixed that function:

def check_contains_member(self, check_member, recursive=False):
    """Checks whether a pyAD object is a member of the group.
        check_member expects a pyAD object to be checked.
        recursive expects True/False which determines whether the group membership will be searched recursively."""
    user_list = self.get_members(recursive=recursive, ignoreGroups=False)

    for checked_member in user_list:
        user = 'cn=' + check_member
        if user == checked_member.prefixed_cn:
            is_true = True
            break
        else:
            is_true = False

    if is_true == True:
        return True
    else:
        return False