Error: Callback was already called when using getGroupMembershipForUser

[Tonatiu]

Hi there,
I'm trying to use a single connection to make multiple requests ti my LDAP connection and everything works fine when I call userExists or findUser, however the thing is different when I call the getGroupMembershipForUser function because it crashes my server with the current error when the connection is lost or it's reseted.

~/node_modules/async/dist/async.js:326
if (fn === null) throw new Error("Callback was already called.");
^

Error: Callback was already called.
at /node_modules/async/dist/async.js:326:36
at GroupMembersForDN.callback (/node_modules/activedirectory2/lib/components/getGroupMembershipForDN.js:55:7)
at GroupMembersForDN.asyncCallback (/node_modules/activedirectory2/lib/components/getGroupMembershipForDN.js:78:8)
at wrapper (/node_modules/async/dist/async.js:273:20)
at eachOfArrayLike (/node_modules/async/dist/async.js:492:13)
at eachOf (/node_modules/async/dist/async.js:628:16)
at awaitable (/node_modules/async/dist/async.js:212:32)
at Object.eachLimit$2 (/node_modules/async/dist/async.js:2693:16)
at Object.awaitable (/node_modules/async/dist/async.js:212:32)
at Searcher.callback (/node_modules/activedirectory2/lib/components/getGroupMembershipForDN.js:118:11)

I have tried different strategies to avoid this error even configurations, however I haven't been able to get rid of.

[martinBogner-aparavi]

I ran into the exact same issue,
would it be a sufficient fix @jsumners to just add a check to prevent the second callback (on ECONNRESET error) if we already succeeded with the check on the first callback?

Something along the lines of slightly modifying

https://github.com/jsumners/node-activedirectory/blob/194dda33531df6863c9e7e66bcddf65e9035a602/lib/components/getGroupMembershipForDN.js#L45-60

(additional code marked with // ADDITIONAL CHECK)

// ADDITIONAL CHECK
let successful = false;
GroupMembersForDN.prototype.asyncIterator = function asyncIterator (group, acb) {
  if (this.stack.has(group.cn || group.dn)) {
    // ADDITIONAL CHECK
    successful = true  
    return acb()
  }

  if (utils.isGroupResult(group)) {
    log.trace('Adding group "%s" to %s"', group.dn, this.dn)
    const g = new Group(group)
    this.stack.set(g.cn || g.dn, g)

    const getter = new GroupMembersForDN(this.opts, g.dn, (err, nestedGroups) => {
      // ADDITIONAL CHECK
      if (err && !successful) {
        return acb(err)
      }
      nestedGroups.forEach((ng) => {
        if (!this.stack.has(ng.cn || ng.dn)) {
          this.stack.set(ng.cn || ng.dn, ng)
        }
      })
      // ADDITIONAL CHECK
      successful = true
      acb()
    }, this.stack)
    getter.getMembers()
  } else {
    acb()
  }
}

or would this create more problems than solve?

[jsumners]

I don't know. I haven't looked at this code in years.

[xbittencourt]

This error was originally caused by a compatible version of ldapjs.

I made the fix in this commit

Only the old version of ldapjs is compatible with this module. You can temporarily fix this by updating your app package.json to use my patched version.

"overrides": {
    "activedirectory2": {
      "ldapjs": "github:xbittencourt/node-ldapjs"
    }
  }

Then just run npm install to update the package.

[A1exRS]

@xbittencourt thank you sir!
Also if anyone else is gonna use the fix maybe add the commit too ?

"overrides": {
    "activedirectory2": {
      "ldapjs": "github:xbittencourt/node-ldapjs#3697b2f"
    }
  },