jsumners / node-activedirectory

ActiveDirectory is an Node.js ldapjs client for authN (authentication) and authZ (authorization) for Microsoft Active Directory with range retrieval support for large Active Directory installations. Originally forked from gheeres/node-activedirectory.
MIT License
52 stars 43 forks source link

Callback called twice on incorrect username/password #46

Open preraksola opened 5 years ago

preraksola commented 5 years ago

I have a very precise and small requirement to determine whether the provided group exists in the AD or not.
However, upon providing wrong credentials the callback of groupExists is called twice, hence crashing my node server. Below is the code snippet that I am using.

let config = {
    url      : req.body.url,
    baseDN   : req.body.baseDN,
    bindDN   : req.body.bindDN,
    password : req.body.password
};
let group  = req.body.group;

let ActiveDirectory = require ( 'activedirectory2' );
let ad              = new ActiveDirectory ( config );
console.log ( 1 );

return ad.groupExists ( config, group, ( err, exists ) => {
    console.log ( 2 );
    if ( err ) {
        console.log ( 3 );
        console.error(err);
    res.json ( {
        status  : - 2,
        message : `Internal Server Error.`
    } );            
    console.log ( 4 );
    } else {
    log ( "info", group + ' exists: ' + exists );
    if ( exists ) {
        res.json ( {
        status  : 1,
        message : `Group "${ group }" found.`
        } );            
    } else {
        res.json ( {
        status  : - 1,
        message : `Group "${ group }" not found.`
        } );
    }
    }
} );

In the console, I get the following output:

1
2
3
InvalidCredentialsError: 80090308: LdapErr: DSID-0C09042A, comment:  ... 
4
2
3
InvalidCredentialsError: 80090308: LdapErr: DSID-0C09042A, comment:  ... 
Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client

Is there something that I am missing?

The issue #8 addresses the similar issue, but authenticate method accepts credentials as separate parameters than config. But that is not the case in groupExists.

preraksola commented 5 years ago

If I use promise instead of callback, it works perfectly fine.

jsumners commented 5 years ago

Can you provide redacted trace logs?

rajsolanki73 commented 4 years ago

any update on this ? i m facing same issue :(

shelagh-lewins-ucl commented 4 years ago

I have the same issue with findGroup. Here's my current workaround, but I don't much like it!

let count = 0;
AD.findGroup(opts, groupCN, (err, group) => {
    if (count > 0) {
        return;
    }

    count += 1;
            // do stuff
});
Xenokrates commented 3 years ago

This is still not solved from release 1.3 up to now (2.1.0). --> The callback fires twice. I run into this with the findUser method. Here my short proof of error:

var ActiveDirectory = require('activedirectory2');

var config = { url: 'ldaps://dc.test.com',
               baseDN: 'DC=test,DC=com',
               username: 'user@test.com',
               password: 'xxx' }

var ad = new ActiveDirectory(config);

// Any of the following username types can be searched on
var sAMAccountName = 'username';
var userPrincipalName = 'username@domain.com';
var dn = 'CN=Smith\\, John,OU=Users,DC=domain,DC=com';

// Find user by a sAMAccountName
ad.findUser(sAMAccountName, function(err, user) {
  console.log(1);
  if (err) {
    console.log(2);
    console.log('ERROR: ' +JSON.stringify(err));
    console.log('User: ' + user );
  } else if (! user) {
    console.log(3);
    console.log('ERR:' + err);
    console.log('User: ' + sAMAccountName + ' not found.');
  } else {
    console.log(4);
    console.log('User: ' + JSON.stringify(user));
  }
})

Output:

1
2
ERROR: {"lde_message":"80090308: LdapErr: DSID-0C090447, comment: AcceptSecurityContext error, data 52e, v3839\u0000","lde_dn":null}
User: undefined
1
3
ERR:undefined
User: username not found.

Best regards

jsumners commented 3 years ago

This is still not solved from release 1.3 up to now (2.1.0).

No one has submitted a pull request to fix it.

Xenokrates commented 3 years ago

What a pitty that nobody takes on that quest. I traced the code execution, but code complexity is above my abilities.

jurjendijkstra commented 3 years ago

Does not happen here. Output for an existing user: 1 4 User: {"d ............

output for a not existing user: 1 3 ERR:undefined User: username not found.

Xenokrates commented 3 years ago

@jurjendijkstra - My guess is that your LDAP/AD allows an anonymous bind ?! (Which is a security issue for a real AD, but might be necessary due to some legacy applications.)

In my environment the connection fails in the BIND phase as you can see from the LDAP error message:

... LdapErr: DSID-0C090447, comment: AcceptSecurityContext error, data 52e ...

This is reproducible with a ldapsearch on the commandline, which throws the same error message:

ldapsearch -H ldaps://dc.test.com -x -b "DC=test,DC=com" -D 'CN=Test-User,DC=test,DC=com' -W '(samaccountname=user)'
Enter LDAP Password:
ldap_bind: Invalid credentials (49)
        additional info: 80090308: LdapErr: DSID-0C090447, comment: AcceptSecurityContext error, data 52e, v3839

The error code 52e in the data is explained here -> https://ldapwiki.com/wiki/Common%20Active%20Directory%20Bind%20Errors -> ERROR_LOGON_FAILURE

It seems that that the bug is triggered due to the error in the BIND phase of the connection.