search

sjkingo / django_auth_ldap3

A library for connecting Django's authentication system to an LDAP directory
BSD 2-Clause "Simplified" License
23 stars 13 forks source link

when checking pgt need to check that user cn match #5

Closed gianlo closed 9 years ago

gianlo commented 9 years ago

otherwise if there's any other user (a part from the one we're checking) with that pgt, this user will successfully authenticate.

sjkingo commented 9 years ago

Can you give an example under what conditions this bug hits? I'm trying to test both the old code and your patch but can't seem to see where the bug is.

gianlo commented 9 years ago

Hi, I'm experiencing the bug in the Active Directory (AD) authentication process. My understanding of check_group_membership (and how it's used in authenticate) is that there is a first ldap query to determine the user's primaryGroupToken (line 128). Than the result of this query (which exists in AD) is used in the second query with as an 'OR' (line 140). However how the code is in its current form, if the user does not belong directly to group_dn (the queried group), the query returns the first user in that primary group (ie the first result from search_ldap, line 169). My change just includes in the case of an existing pgt the check that the cn has to match as well.

CASE: I tested this in the following scenario, set up an AD group zzz. set up a user ZZZ that only belonged to that group. Then used a different group that has users as the group_dn. When check_group_membership retrieves the primaryGroupToken from group_dn, the second query (line 140) returns a list of users from the different group (which does not include our ZZZ user). This thus return something (i.e. the first user from the different group) and the user gets authenticated.

I hope this helps.

Regards

Gianlo

sjkingo commented 9 years ago

That's great, thanks for hashing that out. I will release a new version to PyPi shortly.

sjkingo commented 9 years ago

Version 0.9.2 is now on PyPi which includes the fix.

Thanks again!

gianlo commented 9 years ago

Fab!