Modify

Opened 5 years ago

Last modified 5 years ago

#13487 new defect

KeyError: 'uid' - When browsing "Users" section in Account Manager

Reported by: totalcaos@… Owned by: c0redumb
Priority: high Component: LDAPAcctMngrPlugin
Severity: major Keywords: needinfo
Cc: totalcaos Trac Release: 1.2

Description (last modified by Ryan J Ollos)

When browsing to the users section in account manager i see this error:

Trac detected an internal error:
KeyError: 'uid'

Tracelog:

File "build/bdist.linux-x86_64/egg/security/ldapstore.py", line 59, in get_users
Code fragment:
	
        try:
51	            con = self.init_connection()
52	            resp = con.search_s(base, ldap.SCOPE_SUBTREE, filter, ['dn','uid'])
53	        finally:
54	            if con != None:
55	                con.unbind()
56	       
57	        self.log.debug('List users: get %d users' % (len(resp)))
58	        for entry in resp:
59	            if entry[1]['uid'][0]:
60	                yield entry[1]['uid'][0]

The issue is that the user_matchfilter = sAMAccountName=%s not the default uid which i believe is hard referenced in your code (line 52)

Is there a way to make this more generic?

Attachments (1)

t13487-v2.diff (5.2 KB) - added by anonymous 5 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 5 years ago by Ryan J Ollos

Description: modified (diff)

comment:2 Changed 5 years ago by Jun Omae

I think that it should add an option to specify LDAP field to match the username and authenticate rather than a part of LDAP filter.

Also, the username to use in LDAP filter should be escaped using ldap.filter.filter_format().

Untested patch:

  • ldapacctmngrplugin/trunk/ldapacctmngrplugin/security/ldapstore.py

    y/ldapstore.py
    index 2ee361972..6d119eac3 100644
    a b class LDAPStore (Component): 
    1111    bind_passwd = Option('ldap', 'bind_passwd', '', doc='For server not accepting anonymous bind, specify a bind_dn and password.')
    1212    user_searchbase = Option('ldap', 'user_searchbase', 'dc=company,dc=com', doc='Where to look for users. It is usually "dc=your_company_name,dc=com". Please consult the structure of your LDAP tree.')
    1313    user_searchfilter = Option('ldap', 'user_searchfilter', 'objectClass=inetOrgPerson', doc='Filter for listing valid users. The default ("objectClass=inetOrgPerson") should work for most of the cases.')
    14     user_matchfilter = Option('ldap', 'user_matchfilter', 'uid=%s', doc='The LDAP field for matching username when authenticating. The query is almost always "uid=%s".')
     14    user_field = Option('ldap', 'user_field', 'uid',
     15        doc="""The LDAP field to match the username and authenticate.""")
    1516
    1617    implements(IPasswordStore)
    1718
    class LDAPStore (Component): 
    1920        # Authenticate a user by checking password
    2021        con = None
    2122        base = self.user_searchbase
    22         filter = self.user_matchfilter % user
     23        user_field = self.user_field
     24        filter_ = ldap.filter.filter_format('%s=%s', [user_field, user])
    2325
    2426        # nested "try:" for python2.4
    2527        try:
    2628            try:
    2729                con = self.init_connection()
    28                 resp = con.search_s(base, ldap.SCOPE_SUBTREE, filter, ['dn'])
     30                resp = con.search_s(base, ldap.SCOPE_SUBTREE, filter_, ['dn'])
    2931
    3032                # Added to prevent empty password authentication (some server allows that)
    3133                if not len(resp) :
    class LDAPStore (Component): 
    4446        # Get list of users from LDAP server
    4547        con = None
    4648        base = self.user_searchbase
     49        user_field = self.user_field
    4750        filter = self.user_searchfilter
    4851        resp = None
    4952
    5053        try:
    5154            con = self.init_connection()
    52             resp = con.search_s(base, ldap.SCOPE_SUBTREE, filter, ['dn','uid'])
     55            resp = con.search_s(base, ldap.SCOPE_SUBTREE, filter,
     56                                ['dn', user_field])
    5357        finally:
    5458            if con != None:
    5559                con.unbind()
    5660
    5761        self.log.debug('List users: get %d users' % (len(resp)))
    5862        for entry in resp:
    59             if entry[1]['uid'][0]:
    60                 yield entry[1]['uid'][0]
     63            value = entry[1][user_field][0]
     64            if value:
     65                yield value
    6166
    6267    def init_connection(self):
    6368        # Initialize LDAP connection

comment:3 in reply to:  2 ; Changed 5 years ago by totalcaos@…

Thanks for the patch,

I modifed the ldapstore.py based on your patch, and have this config in trac.ini:

[ldap]
bind_anonymous = no
bind_dn = CN=srv_ldap,CN=ServiceAccounts,CN=Users,DC=corp,DC=xxxx,DC=com
bind_passwd = XXXX
bind_server = ldap://xxxx:389
user_matchfilter = sAMAccountName=%s
user_field = sAMAccountName=%s
user_searchbase = CN=Staff,CN=Users,DC=corp,DC=xxx,DC=com
user_searchfilter = objectClass=person

I no longer see the KeyError: 'uid' when going to Admin -> Users, but don't see a list of my LDAP users. What am I doing that not correct?

How do I help you test this patch?

Thanks!

Last edited 5 years ago by Jun Omae (previous) (diff)

comment:4 Changed 5 years ago by totalcaos

Cc: totalcaos added

comment:5 in reply to:  3 Changed 5 years ago by Jun Omae

Replying to totalcaos@…:

I modifed the ldapstore.py based on your patch, and have this config in trac.ini:

[ldap]
bind_anonymous = no
bind_dn = CN=srv_ldap,CN=ServiceAccounts,CN=Users,DC=corp,DC=xxxx,DC=com
bind_passwd = XXXX
bind_server = ldap://xxxx:389
user_matchfilter = sAMAccountName=%s
user_field = sAMAccountName=%s
user_searchbase = CN=Staff,CN=Users,DC=corp,DC=xxx,DC=com
user_searchfilter = objectClass=person

I no longer see the KeyError: 'uid' when going to Admin -> Users, but don't see a list of my LDAP users. What am I doing that not correct?

The user_field option should be sAMAccountName, not sAMAccountName=%s.

user_field = sAMAccountName

Changed 5 years ago by anonymous

Attachment: t13487-v2.diff added

comment:6 Changed 5 years ago by Jun Omae

The patch is revised and tested with Active Directory 2012 R2: t13487-v2.diff

Could you please try the patch?

P.S. I noticed the LDAPStore authenticates wrongly any username with empty password. The patch includes fix for this.

comment:7 Changed 5 years ago by Jun Omae

Here is configuration for the testing.

[ldap]
bind_anonymous = no
bind_dn = administrator@DOMAIN.REALM
bind_passwd = passphrase
bind_server = ldap://DOMAIN.REALM/
user_matchfilter = sAMAccountName=%s
user_field = sAMAccountName
user_searchbase = dc=DOMAIN,dc=REALM
user_searchfilter = &(objectClass=user)(!(objectClass=computer))

comment:8 Changed 5 years ago by Jun Omae

Keywords: needinfo added

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain c0redumb.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.