Modify

Opened 6 years ago

Last modified 5 years ago

#3993 new defect

Trac LDAP plugin makes too many round trips for group membership

Reported by: jason.dusek@… Owned by: eblot
Priority: normal Component: LdapPlugin
Severity: major Keywords:
Cc: Trac Release: 0.11

Description

I have written a group membership routine to allow faster searches to be performed. The Trac plugin need only go to the LDAP server one time to get a user's groups listing, instead of running a separate query for each group on the server.

  • ldapplugin/api.py

     
    146146   
    147147    def _get_user_groups(self, username):
    148148        """Returns a list of all groups a user belongs to"""
    149         ldap_groups = self._ldap.get_groups()
    150         groups = []
    151         for group in ldap_groups:
    152             if self._ldap.is_in_group(self.util.user_attrdn(username), group):
    153                 m = DN_RE.search(group)
    154                 if m:
    155                     groupname = GROUP_PREFIX + m.group('rdn')
    156                     if groupname not in groups:
    157                         groups.append(groupname)
    158         return groups
     149        return [GROUP_PREFIX + g for g in self._ldap.groups_for_user(username)]
    159150
    160151class LdapPermissionStore(Component):
    161152    """
     
    541532        groups = self.get_dn(self.basedn, 'objectclass=' + self.groupname)
    542533        return groups
    543534   
     535    def groups_for_user(self, uid):
     536        """Obtain group listing for a particular uid."""
     537        if self.groupmemberisdn:
     538            dns = get_dn(self.basedn, '(uid=%s)' % uid)
     539            if dns:
     540                uidstr = dns[0]
     541            else:
     542                return False
     543        else:
     544            uidstr = uid
     545        filter = ('(&(objectclass=%s)(%s=%s))' %
     546                  (self.groupname, self.groupmember, uidstr))
     547        results = self._search( self.basedn
     548                              , filter
     549                              , [self.groupattr]
     550                              , ldap.SCOPE_SUBTREE )
     551        self.log.error("Found: %s", str(results))
     552        return [ attrs[self.groupattr][0] for (dn, attrs) in results ]
     553
    544554    def is_in_group(self, userdn, groupdn):
    545555        """Tell whether the uid is member of the group"""
    546556        if self.groupmemberisdn:

Attachments (0)

Change History (4)

comment:1 Changed 6 years ago by anonymous

I have not tested this with dns instead of uids -- I don't have that setup readily available -- but I am using it for uids on a live system and it works fine.

comment:2 follow-ups: Changed 6 years ago by jason.dusek@…

Oh, there is a pointless error logging line in my code.

  • ldapplugin/api.py

     
    146146   
    147147    def _get_user_groups(self, username):
    148148        """Returns a list of all groups a user belongs to"""
    149         ldap_groups = self._ldap.get_groups()
    150         groups = []
    151         for group in ldap_groups:
    152             if self._ldap.is_in_group(self.util.user_attrdn(username), group):
    153                 m = DN_RE.search(group)
    154                 if m:
    155                     groupname = GROUP_PREFIX + m.group('rdn')
    156                     if groupname not in groups:
    157                         groups.append(groupname)
    158         return groups
     149        return [GROUP_PREFIX + g for g in self._ldap.groups_for_user(username)]
    159150
    160151class LdapPermissionStore(Component):
    161152    """
     
    541532        groups = self.get_dn(self.basedn, 'objectclass=' + self.groupname)
    542533        return groups
    543534   
     535    def groups_for_user(self, uid):
     536        """Obtain group listing for a particular uid."""
     537        if self.groupmemberisdn:
     538            dns = get_dn(self.basedn, '(uid=%s)' % uid)
     539            if dns:
     540                uidstr = dns[0]
     541            else:
     542                return False
     543        else:
     544            uidstr = uid
     545        filter = ('(&(objectclass=%s)(%s=%s))' %
     546                  (self.groupname, self.groupmember, uidstr))
     547        results = self._search( self.basedn
     548                              , filter
     549                              , [self.groupattr]
     550                              , ldap.SCOPE_SUBTREE )
     551        return [ attrs[self.groupattr][0] for (dn, attrs) in results ]
     552
    544553    def is_in_group(self, userdn, groupdn):
    545554        """Tell whether the uid is member of the group"""
    546555        if self.groupmemberisdn:

comment:3 in reply to: ↑ 2 Changed 6 years ago by jmeile@…

Oh, there is a pointless error logging line in my code.

Thanks, your patch speed up my setup. With the original setup, I was having some LDAP errors in lots of groups to which a user didn't belonged to. It worked yes, but it was slow.

comment:4 in reply to: ↑ 2 Changed 5 years ago by andrewcooper

Replying to jason.dusek@gmail.com:

Oh, there is a pointless error logging line in my code.

I'm using this in an Active Directory environment where uid is not the attribute for group members. So I changed that to use self.uidattr instead.

I'm not sure why I had to change an escaped ',' but the result from get_dn() needed an extra backslash to work correctly. Our user DNs are of the form "CN=Last\, First,OU=..." and the search string needed 3 backslashes, like "(member=CN=Last\\\, First,OU=...)". In any case, enjoy.

  • api.py

     
    2323import re
    2424import time
    2525import ldap
     26import string
    2627
    2728from trac.core import *
    2829from trac.perm import IPermissionGroupProvider, IPermissionStore
     
    147148   
    148149    def _get_user_groups(self, username):
    149150        """Returns a list of all groups a user belongs to"""
    150         ldap_groups = self._ldap.get_groups()
    151         groups = []
    152         for group in ldap_groups:
    153             if self._ldap.is_in_group(self.util.user_attrdn(username), group):
    154                 m = DN_RE.search(group)
    155                 if m:
    156                     groupname = GROUP_PREFIX + m.group('rdn')
    157                     if groupname not in groups:
    158                         groups.append(groupname)
    159         return groups
     151        return [GROUP_PREFIX + g for g in self._ldap.groups_for_user(username)]
    160152
    161153class LdapPermissionStore(Component):
    162154    """
     
    554546        groups = self.get_dn(self.basedn, 'objectclass=' + self.groupname)
    555547        return groups
    556548   
     549    def groups_for_user(self, uid):
     550        """Obtain group listing for a particular uid."""
     551        if self.groupmemberisdn:
     552            dns = self.get_dn(self.basedn, '(%s=%s)' % (self.uidattr,uid) )
     553            if dns:
     554                uidstr = dns[0]
     555            else:
     556                return False
     557        else:
     558            uidstr = uid
     559        filter = ('(&(objectclass=%s)(%s=%s))' %
     560                  (self.groupname, self.groupmember, string.replace(uidstr,"\\,","\\\,") ) )
     561    self.log.debug(filter)
     562        results = self._search( self.basedn
     563                              , filter
     564                              , [self.groupattr]
     565                              , ldap.SCOPE_SUBTREE )
     566        return [ attrs[self.groupattr][0] for (dn, attrs) in results ]
     567
    557568    def is_in_group(self, userdn, groupdn):
    558569        """Tell whether the uid is member of the group"""
    559570        if self.groupmemberisdn:

Add Comment

Modify Ticket

Action
as new The owner will remain eblot.
Author


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

 
Note: See TracTickets for help on using tickets.