ldapjs / node-ldapjs

LDAP Client and Server API for node.js
http://ldapjs.org
MIT License
1.61k stars 445 forks source link

The search fails if the DN string of base contains non-ascii characters. #860

Closed dmdragon closed 1 year ago

dmdragon commented 1 year ago

I am trying to use ldapjs to retrieve a username (cn) and employee number (employeeID) from an Active Directory server running in my organization.

My organization's Active Directory uses non-ascii characters (Japanese) for the cn of organizational unit (OU).

When I try to search the ldapjs client API using that OU as the base, I get an error saying "No Such Object".

So, I used parseDN of the DN API, turned the DN string of the OU (string containing non-ascii characters) into a DN object, and specified the toString() function of the DN object, but it still gave the same error, "No Such Object".

How can I perform a search based on a DN that contains non-ascii characters?

When I specified a DN that does not contain non-ASCII characters as the base, the search was successful. And the non-ASCII character part of the OU DN string included in the result was encoded and matched the result of toString() of the DN object.

The code that caused the search to fail is shown below.

const ldap = require('ldapjs');
const parseDN = ldap.parseDN;

const username = process.env.USERNAME
const password = process.env.PASSWORD

const client = ldap.createClient({ url: 'ldap://example.com' });

client.bind(username, password, (err) => {
    if (err) {
        console.error(err);
        return;
    }
}

const opts = {
    filter: '(&(objectCategory=person)(objectClass=user))',
    scope: 'sub',
    paged: true,
    sizeLimit: 100,
    attributes: ['cn', 'employeeID']
};

const baseDN = parseDN('ou=<non-ascii string>,dc=example,dc=com');

client.search(baseDN.toString(), opts, (err, res) => {
    if (err) {
        console.error(err);
    }
    res.on('searchEntry', (entry) => {
        console.log(entry.pojo);
    });
    res.on('error', (err) => {
        console.error(err);
    });
    res.on('end', () => {
        client.unbind();
    });
});

With this code, if the argument of parseDN is 'dc=example,dc=com' (if it does not contain a non-ascii string), the search will succeed.

Note that changing the first argument of search to "baseDN" instead of "baseDN.toString()" resulted in the error "base must be a DN string or DN instance".

dmdragon commented 1 year ago

P.S. With another LDAP client (Paython ldap3 module), I was able to search successfully even if I specified a string containing non-ascii characters in the base, so I do not see any problem on the server side.

jsumners commented 1 year ago

DN strings are defined by https://www.rfc-editor.org/rfc/rfc4514. Specifically https://www.rfc-editor.org/rfc/rfc4514#section-2.3 links to https://www.rfc-editor.org/rfc/rfc4512 to explain how AttributeType and AttributeValue strings can be represented. The relevant section is https://www.rfc-editor.org/rfc/rfc4512#section-2.5. Specifically, this section defines the components as being oids, which are defined in https://www.rfc-editor.org/rfc/rfc4512#section-1.4 as:

oid = descr / numericoid
descr = keystring
keystring = leadkeychar *keychar
leadkeychar = ALPHA
keychar = ALPHA / DIGIT / HYPHEN
ALPHA   = %x41-5A / %x61-7A   ; "A"-"Z" / "a"-"z"
DIGIT   = %x30 / LDIGIT       ; "0"-"9"
HYPHEN  = %x2D ; hyphen ("-")

In short: the LDAP spec only allows for ASCII characters in DN strings. You may need to use a numericoid.

dmdragon commented 1 year ago

Thanks for the reply, I understood that non-ascii characters are not allowed in DN strings. However, I have converted a DN string containing non-ascii characters to a DN string without non-ascii characters using parseDN in the DN API. But, when I specified that converted string as base and searched for it, I got the error.

jsumners commented 1 year ago

What does the result of DN.toString() look like?

dmdragon commented 1 year ago

I do not have access to the server at this time. So, I executed the following code. The string specified after "ou=" will be the same as the actual string.

const parseDN = require('ldapjs').parseDN;

const base = 'ou=テスト,dc=example,dc=com';
// "テスト" is the Japanese word for "test".

const baseDN = parseDN(base);

console.log(base);              // ou=テスト,dc=example,dc=com
console.log(baseDN.toString()); // ou=\e3\83\86\e3\82\b9\e3\83\88,dc=example,dc=com
jsumners commented 1 year ago

I am not seeing the described issue. I created a new integration test in PR https://github.com/ldapjs/node-ldapjs/pull/861 and the tests pass correctly: https://github.com/ldapjs/node-ldapjs/actions/runs/4455697466/jobs/7825663784?pr=861#step:6:9

jsumners commented 1 year ago

@dmdragon are you able to confirm that my added test meets your expectations?

jsumners commented 1 year ago

Due to lack of response I am closing this issue. If you're able to provide a minimal reproduction that shows the issue, please reopen.

ncphins commented 1 year ago

I believe I may be seeing the same issue here when trying to migrate our code to 3.x.

I have a DN that I retrieve from a group (in the member attribute). I need to then search for that DN to get the details.

For example, the DN returned in the member attribute is: "CN=Céline Jones,OU=Users,OU=France,DC=emea,DC=FOO,DC=com"

In v2.x, I could use that in a search as the "base" (scope: "base") and I could find the user. In 3.0, that fails with "No Such Object".

I tried creating a DN object (via parseDN("CN=Céline Jones,OU=Users,OU=France,DC=emea,DC=FOO,DC=com") and then using the "toString()" version of that in a search. That also fails.

In that case, the results of the DN.toString() are: "CN=C\c3\a9line Jones,OU=Users,OU=France,DC=emea,DC=FOO,DC=com" which looks correct.

As it stands, I cannot find a successful way to search for a DN with non-ASCII characters. I will research the document linked above more carefully, but these searches were working in v2.x.

jsumners commented 1 year ago

I really need a reproduction in order to be able to investigate the issue. You should be able to follow the work I did in #861 to create one. Also, please read carefully https://github.com/ldapjs/node-ldapjs/issues/860#issuecomment-1474479422

ncphins commented 1 year ago

Ok. I will see what I can do. We are not using the server implementation here, only the client side to search existing MS AD installations. But I will see if I can write a test that shows the issue.

jsumners commented 1 year ago

The test mentioned above is an "integration" test hitting our Docker instance provided by https://github.com/ldapjs/docker-test-openldap.

ncphins commented 1 year ago

I have not given up on this. I am struggling to get the Docker container running on my Mac (or Windows for that matter) to allow a connection (via ldapsearch or our app). Once that is functioning, I will get back to trying to provide a test case.

jsumners commented 1 year ago

What issue are you having with the container? The README could be updated with better instructions.

ncphins commented 1 year ago

Well, I have it running on My Mac. Looks like it is running fine, but I cannot access not via ldapsearch. I keep getting the same error.

I opened an issue there to see if someone could give me a pointer. But it just seems like I cannot connect to it. I see logs int he container that make me think it is running. The port is in use by Docker.

https://github.com/rroemhild/docker-test-openldap/issues/47

jsumners commented 1 year ago

Please do not bother the upstream project. If there is an issue with our fork, please file it on our repo.

Reading the issue you opened, I don't see what command you used to start the container. Please bring the issue over to https://github.com/ldapjs/docker-test-openldap and include that information.

ncphins commented 1 year ago

I really apologize for that mistake. I have closed the issue. I got the tests running in your repo.

I have a test written that (I believe) simulates my issue here, and the test is passing in the repo. I am searching for the group 'ship_crew' and then subsequently searching for each member via the DN that is returned in the member array. That group has a member (Bender Bending Rodríguez) that should simulate the issue we are seeing perfectly. I am not sure why our search is failing now, but I am going back there to see if I can find the issue in our code.

ncphins commented 1 year ago

Update today is that I do not see an obvious issue in our code. I went back to the integration test which is passing against the container, and I ran the same test (in the ldapjs repo) against our production AD server here for our company, as well as a large test AD server we have for testing our project. The test fails when run against those servers. The production AD server has 520 members and the test fails on the first member lookup that has a non-ascii character in the name, same as our production code.

For completeness, I also ran our app pointed at the container AD and used "ship_crew" as a test in our software. We successfully managed to lookup all the members in our code from that container.

I tried to run the integration test against the v2.x branch, but that looks like it will take me a bit more work.

In short, I believe there is a bug, but I am not sure how to provide a reproducible test with the container.

jsumners commented 1 year ago

You can see in https://github.com/ldapjs/docker-test-openldap/pull/2 that we added an LDAP entry that matches the description provided in the original post in this thread. That is, the OU has non-ascii characters in the name. You are describing a situation wherein the entry name (CN) has non-ascii characters. I'd suggest starting with creating an entry that fully replicates your situation.

ncphins commented 1 year ago

I apologize. Maybe I am not being clear. Or maybe I am not understanding your comment completely.

The integration test I wrote here (not pushed or on a branch you can see) is searching for the DN (indirectly through the group ship_crew): cn=Bender Bending Rodríguez,ou=people,dc=planetexpress,dc=com

That entry already exists in the container. That CN has non-ascii characters. And the search works fine in the integration test. I will see if I can add an identical name to the one we are seeing the issue with to rule that out.

jsumners commented 1 year ago

FYI, I have updated the Docker image project, and pushed new images to the GitHub package repo, in https://github.com/ldapjs/docker-test-openldap/pull/3

jimmyengman commented 1 year ago

As #936 was a duplicate of this issue I write my question here instead.

If I start an OpenLDAP docker container as suggested in https://github.com/ldapjs/docker-test-openldap and then connect to it with ldap.createClient (http://ldapjs.org/client.html) I can search with filter sn=Conrad and get a searchEntry for Hermes Conrad printed but when I search for sn=Rodríguez no entry is found.

It seems to work to have non ascii characters for some attributes (a group objects member attribute and objects entryDN attribute) but for all the rest I have tested it does not work. It is a problem because the swedish letter å,ä,ö are in many users and groups names and after upgrading to ldapjs v.3 we can no longer get a search result for them.

import { default as ldap } from 'ldapjs';

// ************ using this opts searching for user on surname no entry is found *****************
const opts = {
filter: '(&(sn=Rodríguez))',
scope: 'sub',
attributes: ['dn', 'sn', 'cn'],
type: 'user'
};

// ************ if this opts searching for user on surname are used an entry will be printed ************
// const opts = {
// filter: '(&(sn=Conrad))',
// scope: 'sub',
// attributes: ['dn', 'sn', 'cn'],
// type: 'user'
// };

// ************ if this opts are used an entry will be printed ************
// const opts = {
//   filter: '(&(entryDN=cn=Bender Bending Rodríguez,ou=people,dc=planetexpress,dc=com))',
//   scope: 'sub',
//   attributes: ['dn', 'sn', 'cn'],
//   type: 'user'
// };

// ************ if this opts are used an entry with the group ship_crew will be printed ************
// const opts = {
//   filter: '(&(member=cn=Bender Bending Rodríguez,ou=people,dc=planetexpress,dc=com))',
//   scope: 'sub',
//   attributes: ['dn', 'sn', 'cn'],
//   type: 'group'
// };

client.search('dc=planetexpress,dc=com', opts, (err, res) => {

res.on('searchRequest', (searchRequest) => {
console.log('searchRequest: ', searchRequest.messageId);
});
res.on('searchEntry', (entry) => {
console.log('entry: ' + JSON.stringify(entry.pojo));
});
res.on('searchReference', (referral) => {
console.log('referral: ' + referral.uris.join());
});
res.on('error', (err) => {
console.error('error: ' + err.message);
});
res.on('end', (result) => {
console.log('status: ' + result.status);
});
});
jsumners commented 1 year ago

Thank you for providing a reproduction showing the full search options used. A failing test has been added in #938.

ncphins commented 1 year ago

I appreciate the failing case as well since I was unable to generate one with the test server but could with production servers. Thanks @jimmyengman

jsumners commented 1 year ago

The issue is https://github.com/ldapjs/filter/issues/6.

jsumners commented 1 year ago

This issue should be resolved in https://github.com/ldapjs/node-ldapjs/releases/tag/v3.0.5. Please open a new issue if this is not the case.

If this fix has helped you out, please consider contributing to my GitHub sponsorship.