isUserMemberOf takes ~30 seconds to complete

[henhal]

I need to authenticate users and also authorize them by checking group membership.

On my network, calling ad.authenticate() takes less than 1 second. However, calling ad.isUserMemberOf() takes anywhere between 15-60 seconds. Is this a known issue? Is it likely to be a problem with the lib or is there any way for me as the client to speed up the request?

var ad = new ActiveDirectory(
        {
            url: 'ldap://<mydomain>.com',
            baseDN: 'dc=<mydomain>,dc=com',
            username: username,
            password: password
        });

logger.info('calling isUserMemberOf');
ad.isUserMemberOf(username, groupName, function(err, isMember) {
        if (err) {
            console.log('group ERROR: ' +JSON.stringify(err));
            return;
        }

        logger.info(username + ' isMemberOf ' + groupName + ': ' + isMember);
    });

2015-05-28T09:01:29.780Z - info: calling isUserMemberOf
2015-05-28T09:01:58.672Z - info: SOME_USER isMemberOf SOME_GROUP: false

[henhal]

Update: I noticed that isUserMemberOf actually pulls down all groups for a given user and then searches the array for the given group. I suppose there isn't any LDAP query to ask for a given group membership, so it has to be all pulled down and searched locally? So I made some tests with ad.getGroupMembershipForUser() and found that this call also takes a long time, and I think I got some strange results, it seems to me that this basically lists all users of all groups in my entire domain, because it goes on forever, listing thousands and thousands of lines of output, whereas my expected output was the ~20 groups in which the given user is a member. The issue seems to be be that it goes into each group, recursively listing users, sub-groups, and users in each sub-group? I am only interested in if user X is a member of group Y, not which other groups he is a member of, and certainly not who else is a member of said group.

Since isUserMemberOf calls getGroupMembershipForUser, it also means it will not stop searching if the given group is found, but waits for the entire recursive call to find ALL groups for the given user is complete before iterating the returned array of groups for the given one.

I tried using

attributes: {
                user: [ 'userPrincipalName'],
                group: [ 'distinguishedName' ]
            }

as well as

{includeMembership: []}

But these options seem to make no difference neither for isUserMemberOf() nor getGroupMembershipForUser() (only valid for findXXX methods?).

Turning it all upside down, I instead checked group membership by calling ad.findGroup() and then iterating the returned members searching for my given user. In my case, the number of members were quite low and there are no nested sub-groups, so this was lightning fast.

To summarize: In my domain, with thousands of users, isUserMemberOf() and getGroupMembershipForUser() are both unusable at this time. Basically what I would need is an option NOT to search for group membership recursively.

Any thoughts on this?

[gheeres]

Please see issue #86. I believe your issue is related.

What happens if you revert back to AD 0.6.4? Does performance improve?

[gheeres]

So I made some tests with ad.getGroupMembershipForUser() and found that this call also takes a long time, and I think I got some strange results, it seems to me that this basically lists all users of all groups in my entire domain, because it goes on forever, listing thousands and thousands of lines of output, whereas my expected output was the ~20 groups in which the given user is a member. The issue seems to be be that it goes into each group, recursively listing users, sub-groups, and users in each sub-group? I am only interested in if user X is a member of group Y, not which other groups he is a member of, and certainly not who else is a member of said group.

The isUserMemberOf() call is actually just a convenience wrapper that uses getGroupMembershipForUser(). Once the membership list is retrieved for the user, it's just simple selection / filtering of the data. Determining if a user is a member of a particular group is actually more complicated that it seems due to groups being able to be members of a group. In ad, only the direct groups assigned to a user are actually set in the memberOf attribute. To get a full memberOf / group listing, you have to enumerate through each group recursively to exhaustively get all of the nested or parent groups.

[pentode]

FWIW, this query will return the list of all groups to which the given user belongs.

// userObj is the result of a user query
var groupQuery = '(member:1.2.840.113556.1.4.1941:=' + userObj.dn + ')'

https://msdn.microsoft.com/en-us/library/aa746475(v=vs.85).aspx

[gheeres]

Yes, I am aware of that, however my experience with that filter has been that it's SLOW... I found that it was actually faster to recursively query the AD to enumerate the information.

Here are my notes / comments from the code:

  //  Note: Microsoft provides a 'Transitive Filter' for querying nested groups.
  //        i.e. (member:1.2.840.113556.1.4.1941:=<userDistinguishedName>)
  //        However this filter is EXTREMELY slow. Recursively querying ActiveDirectory
  //        is typically 10x faster.
  opts = _.defaults(_.omit(opts || {}, 'filter', 'scope'), {
    filter: '(member='+parseDistinguishedName(dn)+')',
    scope: 'sub',
    attributes: _.union((opts || {}).attributes || defaultAttributes.group, [ 'member' ])
  });
  search.call(self, opts, function(err, results) {

[gheeres]

Can you try the following patch:

index 4fb2df0..601b050 100755
--- a/lib/activedirectory.js
+++ b/lib/activedirectory.js
@@ -33,8 +33,7 @@ var defaultAttributes = {
     'objectCategory',
     'distinguishedName',
     'cn',
-    'description',
-    'member'
+    'description'
   ]
 };
 var defaultReferrals = {
@@ -621,17 +620,25 @@ function parseRangeAttributes(result, opts, callback) {
 /**
  * Gets the required ldap attributes for group related queries in order to
  * do recursive queries, etc.
+ *
+ * @private
+ * @params {Object} [opts] Optional LDAP query string parameters to execute. { scope: '', filter: '', attributes: [ '', '', ... ], sizeLimit: 0, timelimit: 0 }
  */
-function getRequiredLdapAttributesForGroup() {
-  return([ 'objectCategory', 'groupType', 'member', 'distinguishedName', 'cn' ]);
+function getRequiredLdapAttributesForGroup(opts) {
+  return(_.union([ 'objectCategory', 'groupType', 'distinguishedName', 'cn' ],
+                 includeGroupMembershipFor(opts, 'group') ? [ 'member' ] : []));
 }

 /**
  * Gets the required ldap attributes for user related queries in order to
  * do recursive queries, etc.
+ *
+ * @private
+ * @params {Object} [opts] Optional LDAP query string parameters to execute. { scope: '', filter: '', attributes: [ '', '', ... ], sizeLimit: 0, timelimit: 0 }
  */
-function getRequiredLdapAttributesForUser() {
-  return([ 'distinguishedName', 'cn' ]);
+function getRequiredLdapAttributesForUser(opts) {
+  return(_.union([ 'distinguishedName', 'cn' ],
+                 includeGroupMembershipFor(opts, 'user') ? [ 'member' ] : []));
 }

 /**
@@ -676,7 +683,7 @@ function getGroupMembershipForDN(opts, dn, stack, callback) {
   opts = _.defaults(_.omit(opts || {}, 'filter', 'scope', 'attributes'), {
     filter: '(member='+parseDistinguishedName(dn)+')',
     scope: 'sub',
-    attributes: _.union((opts || {}).attributes || defaultAttributes.group, getRequiredLdapAttributesForGroup())
+    attributes: _.union((opts || {}).attributes || defaultAttributes.group, [ 'groupType' ])
   });
   search.call(self, opts, function(err, results) {
     if (err) {
@@ -936,7 +943,7 @@ ActiveDirectory.prototype.getUsersForGroup = function getUsersForGroup(opts, gro
       opts = {
         filter: filter,
         scope: 'sub',
-        attributes: _.union((opts || {}).attributes || defaultAttributes.user || [], getRequiredLdapAttributesForUser(), [ 'groupType' ])
+        attributes: _.union((opts || {}).attributes || defaultAttributes.user || [], getRequiredLdapAttributesForUser(opts), [ 'groupType' ])
       };
       search.call(self, opts, function onSearch(err, members) {
         if (err) {
@@ -1209,7 +1216,8 @@ ActiveDirectory.prototype.find = function find(opts, callback) {
   opts = _.defaults(_.omit(opts || {}, 'attributes'), {
     scope: 'sub',
     attributes: _.union((opts || {}).attributes || [], defaultAttributes.group || [], defaultAttributes.user || [],
-                        getRequiredLdapAttributesForGroup(), getRequiredLdapAttributesForUser(), [ 'objectCategory' ])
+                        getRequiredLdapAttributesForGroup(opts), getRequiredLdapAttributesForUser(opts),
+                        [ 'objectCategory' ])
   });
   search.call(self, opts, function onFind(err, results) {
     if (err) {
@@ -1399,7 +1407,7 @@ ActiveDirectory.prototype.findGroup = function findGroup(opts, groupName, callba
   opts = _.defaults(_.omit(opts || {}, 'attributes'), {
     filter: getGroupQueryFilter.call(self, groupName),
     scope: 'sub',
-    attributes: _.union((opts || {}).attributes || defaultAttributes.group, getRequiredLdapAttributesForGroup())
+    attributes: _.union((opts || {}).attributes || defaultAttributes.group, getRequiredLdapAttributesForGroup(opts))
   });
   search.call(self, opts, function onSearch(err, results) {
     if (err) {
@@ -1463,7 +1471,7 @@ ActiveDirectory.prototype.findGroups = function findGroup(opts, callback) {
   opts = _.defaults(_.omit(opts || {}, 'attributes'), {
     filter: '(&'+defaultGroupFilter+')',
     scope: 'sub',
-    attributes: _.union((opts || {}).attributes || defaultAttributes.group || [], getRequiredLdapAttributesForGroup())
+    attributes: _.union((opts || {}).attributes || defaultAttributes.group || [], getRequiredLdapAttributesForGroup(opts))
   });
   search.call(self, opts, function onSearch(err, results) {
     if (err) {
@@ -1545,7 +1553,7 @@ ActiveDirectory.prototype.findUser = function findUser(opts, username, includeMe
   opts = _.defaults(_.omit(opts || {}, 'attributes'), {
     filter: getUserQueryFilter.call(self, username),
     scope: 'sub',
-    attributes: _.union((opts || {}).attributes || defaultAttributes.user || [], getRequiredLdapAttributesForUser())
+    attributes: _.union((opts || {}).attributes || defaultAttributes.user || [], getRequiredLdapAttributesForUser(opts))
   });
   search.call(self, opts, function onSearch(err, results) {
     if (err) {
@@ -1613,7 +1621,7 @@ ActiveDirectory.prototype.findUsers = function findUsers(opts, includeMembership
   opts = _.defaults(_.omit(opts || {}, 'attributes'), {
     filter: '(&'+defaultUserFilter+')',
     scope: 'sub',
-    attributes: _.union((opts || {}).attributes || defaultAttributes.user || [], getRequiredLdapAttributesForUser())
+    attributes: _.union((opts || {}).attributes || defaultAttributes.user || [], getRequiredLdapAttributesForUser(opts))
   });
   search.call(self, opts, function onSearch(err, results) {
     if (err) {

[gheeres]

I've automated my personal unit tests into integration tests. During that process, I discovered a number of bugs which I've fixed. If you're still having problems with the latest version, please reopen and post additional details... or better year create an integration / unit test for the problem.

[jesben]

Test: Checking membership for two groups (still recursive):

using isUserMemberOf takes 1.5 second.
using memberof:1.2.840.113556.1.4.1941:=... takes 140 ms.

isUserMemberOf(userDn: string, groupName: string) {
        return new Promise(resolve => {
            /* // Slow
            this.ad.isUserMemberOf(userDn, groupName, function (err, isMember) {
                if (err) {
                    console.log('ERROR: ' + JSON.stringify(err));
                    resolve(false);
                }

                //console.log(userDn + ' isMemberOf ' + groupName + ': ' + isMember);

                resolve(isMember);
            }); */

            // Faster 
            this.ad.findGroup(groupName, (err, group) => {
                if (err) {
                    console.log('ERROR: ' + JSON.stringify(err));
                    resolve(false);
                }

                if (!group) {
                    console.log('Group: ' + groupName + ' not found.');
                    resolve(false);
                } else {
                    //console.log('group', group);

                    let opt = {
                        baseDN: userDn,
                        filter: '(memberof:1.2.840.113556.1.4.1941:=' + group.dn + ')',
                        attributes: 'dn'
                    };

                    this.ad.find(opt, (err, results) => {
                        if (err) {
                            console.log('ERROR: ' + JSON.stringify(err));
                            resolve(false);
                        }

                        //console.log(results);
                        resolve(results && results.hasOwnProperty('users') && results.users.length > 0 ? true : false);
                    });
                }
            });
        });
    }