locked
Speed up LDAP query RRS feed

  • Question

  • Hi All,

    I need to query AD to work out what access a user has to my application.
    I have 7 AD groups (FTP1 to FTP 7), these groups can have other groups added so I'm using GetMembers(True) which is recursive.

    The below function works great but it's a little slow, is there any obvious changes I can make to speed it up?

     

     

    CheckBox1.Checked = IsUserInGroup(User, "FTP1")
    CheckBox2.Checked = IsUserInGroup(User, "FTP2")
    CheckBox3.Checked = IsUserInGroup(User, "FTP3")
    CheckBox4.Checked = IsUserInGroup(User, "FTP4")
    CheckBox5.Checked = IsUserInGroup(User, "FTP5")
    CheckBox6.Checked = IsUserInGroup(User, "FTP6")
    CheckBox7.Checked = IsUserInGroup(User, "FTP7")
    

    Here's the function:

     

     

        Public Shared Function IsUserInGroup(ByVal username As String, ByVal groupname As String) As Boolean
            Dim found = False
            Using context = New PrincipalContext(ContextType.Domain, "DOMAIN")
                Using group = GroupPrincipal.FindByIdentity(context, groupname)
                    If group Is Nothing Then
                        Throw New ArgumentException("Group not be found")
                    End If
    
                    For Each member In group.GetMembers(True)
                        If member.SamAccountName.ToLower.Equals(username.ToLower) Then
                            foundUser = True
                            Exit For
                        End If
                    Next
                End Using
            End Using
            Return found
        End Function
    

    Thanks.

     


    • Edited by madlan Monday, October 17, 2011 8:35 AM
    Friday, October 14, 2011 4:29 PM

Answers

  • Well, it would be nice to have some benchmark values to compare against (how long is long to you?).

    At first glance I'd say that the getMembers is probably where most of your time is (you can stopwatch this to verify this assertion).

    You are getting all of the members recursively before even looking at a single one.

    If you manually iterate through the group recursively you can stop as soon as you find your member.

    i.e.:

    Look at the root level, if you find it, you're done.

    Look through the first sub group, if you fine it, stop.

    and so on.

    The performance implications will depend on the AD higherarchy you have.  If you have lots of sub groups, or sub groups with lots of members, not needing to pull back all of them will speed things up for you.  That said, you will likely have more back-and-forths between your program and AD doing this, so for certain situations you could slow yourself down.  I.e, when you have lots of groups/sub groups and you don't have a match, so you need to independently fetch each group.

    • Proposed as answer by Mike Feng Monday, October 17, 2011 2:40 AM
    • Marked as answer by madlan Monday, October 17, 2011 2:54 PM
    Friday, October 14, 2011 4:44 PM
  • Wouldn't it be easier to just query the memberOf property?

            Dim RootDSE As New DirectoryServices.DirectoryEntry("LDAP://RootDSE")
            Dim DomainDN As String = RootDSE.Properties("DefaultNamingContext").Value
            Dim ADEntry As New DirectoryServices.DirectoryEntry("LDAP://" & DomainDN)
            Dim ADSearch As New System.DirectoryServices.DirectorySearcher(ADEntry)
    
            Dim ADSearchResult As System.DirectoryServices.SearchResult
            ADSearch.PropertiesToLoad.Add("memberOf")
            ADSearch.Filter = ("(samAccountName=" & UserID & ")")
            ADSearch.SearchScope = SearchScope.Subtree
            Dim UserFound As SearchResult = ADSearch.FindOne()
            Dim propertyCount As Integer = UserFound.Properties("memberOf").Count
            If Not IsNothing(UserFound) Then
                Console.WriteLine(UserFound.GetDirectoryEntry().Properties.Item("name").Value)
                Dim propertyCounter As Integer
                Dim dn As String
    
                For propertyCounter = 0 To propertyCount - 1
    
                    dn = CType(UserFound.Properties("memberOf")(propertyCounter), String)
                    Console.WriteLine(dn.ToString)
                Next
            End If
    

     


    Paul ~~~~ Microsoft MVP (Visual Basic)
    • Marked as answer by Mike Feng Tuesday, October 18, 2011 2:42 AM
    Monday, October 17, 2011 3:39 PM

All replies

  • Well, it would be nice to have some benchmark values to compare against (how long is long to you?).

    At first glance I'd say that the getMembers is probably where most of your time is (you can stopwatch this to verify this assertion).

    You are getting all of the members recursively before even looking at a single one.

    If you manually iterate through the group recursively you can stop as soon as you find your member.

    i.e.:

    Look at the root level, if you find it, you're done.

    Look through the first sub group, if you fine it, stop.

    and so on.

    The performance implications will depend on the AD higherarchy you have.  If you have lots of sub groups, or sub groups with lots of members, not needing to pull back all of them will speed things up for you.  That said, you will likely have more back-and-forths between your program and AD doing this, so for certain situations you could slow yourself down.  I.e, when you have lots of groups/sub groups and you don't have a match, so you need to independently fetch each group.

    • Proposed as answer by Mike Feng Monday, October 17, 2011 2:40 AM
    • Marked as answer by madlan Monday, October 17, 2011 2:54 PM
    Friday, October 14, 2011 4:44 PM
  • Hi,

    The problem is the user can be in more than one group, so I do need to iterate through each.
    I'm exiting the loop for the given group once found though.

    Thanks

    Monday, October 17, 2011 9:00 AM
  • I'm talking entirely about the "IsUserInGroup" method.  Within the scope of that method, once you find the user in any group or sub-group you are done; you shouldn't need to keep searching the other groups.
    Monday, October 17, 2011 1:37 PM
  • Wouldn't it be easier to just query the memberOf property?

            Dim RootDSE As New DirectoryServices.DirectoryEntry("LDAP://RootDSE")
            Dim DomainDN As String = RootDSE.Properties("DefaultNamingContext").Value
            Dim ADEntry As New DirectoryServices.DirectoryEntry("LDAP://" & DomainDN)
            Dim ADSearch As New System.DirectoryServices.DirectorySearcher(ADEntry)
    
            Dim ADSearchResult As System.DirectoryServices.SearchResult
            ADSearch.PropertiesToLoad.Add("memberOf")
            ADSearch.Filter = ("(samAccountName=" & UserID & ")")
            ADSearch.SearchScope = SearchScope.Subtree
            Dim UserFound As SearchResult = ADSearch.FindOne()
            Dim propertyCount As Integer = UserFound.Properties("memberOf").Count
            If Not IsNothing(UserFound) Then
                Console.WriteLine(UserFound.GetDirectoryEntry().Properties.Item("name").Value)
                Dim propertyCounter As Integer
                Dim dn As String
    
                For propertyCounter = 0 To propertyCount - 1
    
                    dn = CType(UserFound.Properties("memberOf")(propertyCounter), String)
                    Console.WriteLine(dn.ToString)
                Next
            End If
    

     


    Paul ~~~~ Microsoft MVP (Visual Basic)
    • Marked as answer by Mike Feng Tuesday, October 18, 2011 2:42 AM
    Monday, October 17, 2011 3:39 PM
  • Hi Paul,

    Unfortunately the above is not recursive, most of the groups have nested groups to several levels which is why I was using the GetMembers(True) function.

     

    Tuesday, October 18, 2011 8:36 AM
  • Once you find the groups a user is a member of then you can check those groups for sub group membership. Personally, I think it's a bit inefficient to determine the sub group membership every time you search for a user, since a user technically is only member of those groups by way of the first level parent. 


    Paul ~~~~ Microsoft MVP (Visual Basic)
    Tuesday, October 18, 2011 12:53 PM