diff --git a/spec/LdapAuth.spec.js b/spec/LdapAuth.spec.js index b577defcd9..7a793ec8f3 100644 --- a/spec/LdapAuth.spec.js +++ b/spec/LdapAuth.spec.js @@ -1,4 +1,5 @@ const ldap = require('../lib/Adapters/Auth/ldap'); +const ldapjs = require('ldapjs'); const mockLdapServer = require('./support/MockLdapServer'); const fs = require('fs'); const port = 12345; @@ -125,6 +126,118 @@ describe('LDAP Injection Prevention', () => { } server.close(done); }); + + // A zero-length credential in a simple bind is the unauthenticated authentication + // mechanism of RFC 4513 section 5.1.2. Directories may answer it with success and map + // the connection to anonymous, so the credential must be refused by Parse Server + // before it is sent. The mock directory used by the tests below accepts such a bind; + // this test is the control that proves it does. + it('mock directory accepts a bind with a zero-length credential', async () => { + const server = await mockLdapServer(port, 'uid=testuser, o=example', false, false, { + allowUnauthenticatedBind: true, + }); + const client = ldapjs.createClient({ url: `ldap://localhost:${port}` }); + try { + await new Promise((resolve, reject) => + client.bind('uid=testuser, o=example', '', err => (err ? reject(err) : resolve())) + ); + expect(server.bindAttempts.length).toBe(1); + expect(server.bindAttempts[0].credentials).toBe(''); + } finally { + client.destroy(); + await new Promise(resolve => server.close(resolve)); + } + }); + + it('should reject empty authData.password', async () => { + const server = await mockLdapServer(port, 'uid=testuser, o=example', false, false, { + allowUnauthenticatedBind: true, + }); + const options = { + suffix: 'o=example', + url: `ldap://localhost:${port}`, + dn: 'uid={{id}}, o=example', + }; + try { + try { + await ldap.validateAuthData({ id: 'testuser', password: '' }, options); + fail('Should have rejected empty password'); + } catch (err) { + expect(err.message).toBe('LDAP: Wrong username or password'); + } + expect(server.bindAttempts.length).toBe(0); + } finally { + await new Promise(resolve => server.close(resolve)); + } + }); + + it('should reject missing authData.password', async () => { + const server = await mockLdapServer(port, 'uid=testuser, o=example', false, false, { + allowUnauthenticatedBind: true, + }); + const options = { + suffix: 'o=example', + url: `ldap://localhost:${port}`, + dn: 'uid={{id}}, o=example', + }; + try { + try { + await ldap.validateAuthData({ id: 'testuser' }, options); + fail('Should have rejected missing password'); + } catch (err) { + expect(err.message).toBe('LDAP: Wrong username or password'); + } + expect(server.bindAttempts.length).toBe(0); + } finally { + await new Promise(resolve => server.close(resolve)); + } + }); + + it('should reject null authData.password', async () => { + const server = await mockLdapServer(port, 'uid=testuser, o=example', false, false, { + allowUnauthenticatedBind: true, + }); + const options = { + suffix: 'o=example', + url: `ldap://localhost:${port}`, + dn: 'uid={{id}}, o=example', + }; + try { + try { + await ldap.validateAuthData({ id: 'testuser', password: null }, options); + fail('Should have rejected null password'); + } catch (err) { + expect(err.message).toBe('LDAP: Wrong username or password'); + } + expect(server.bindAttempts.length).toBe(0); + } finally { + await new Promise(resolve => server.close(resolve)); + } + }); + + it('should reject non-string authData.password', async () => { + const server = await mockLdapServer(port, 'uid=testuser, o=example', false, false, { + allowUnauthenticatedBind: true, + }); + const options = { + suffix: 'o=example', + url: `ldap://localhost:${port}`, + dn: 'uid={{id}}, o=example', + }; + try { + for (const password of [123, {}, [], true]) { + try { + await ldap.validateAuthData({ id: 'testuser', password }, options); + fail(`Should have rejected non-string password: ${JSON.stringify(password)}`); + } catch (err) { + expect(err.message).toBe('LDAP: Wrong username or password'); + } + } + expect(server.bindAttempts.length).toBe(0); + } finally { + await new Promise(resolve => server.close(resolve)); + } + }); }); describe('DN injection prevention', () => { diff --git a/spec/support/MockLdapServer.js b/spec/support/MockLdapServer.js index 935f0703d6..6ae087b06b 100644 --- a/spec/support/MockLdapServer.js +++ b/spec/support/MockLdapServer.js @@ -6,10 +6,23 @@ const tlsOptions = { certificate: fs.readFileSync(__dirname + '/cert/cert.pem'), }; -function newServer(port, dn, provokeSearchError = false, ssl = false) { +function newServer(port, dn, provokeSearchError = false, ssl = false, options = {}) { const server = ssl ? ldapjs.createServer(tlsOptions) : ldapjs.createServer(); + // Records every bind the directory actually receives, so tests can assert that a + // credential is refused before the directory is contacted. + server.bindAttempts = []; + server.bind('o=example', function (req, res, next) { + server.bindAttempts.push({ dn: req.dn.toString(), credentials: req.credentials }); + // Models a directory that honors the unauthenticated authentication mechanism of + // simple bind (RFC 4513 section 5.1.2), which Active Directory permits by default: + // a valid DN with a zero-length credential binds successfully and the connection is + // mapped to anonymous. + if (options.allowUnauthenticatedBind && req.dn.toString() === dn && req.credentials === '') { + res.end(); + return next(); + } if (req.dn.toString() !== dn || req.credentials !== 'secret') { return next(new ldapjs.InvalidCredentialsError()); } res.end(); diff --git a/src/Adapters/Auth/ldap.js b/src/Adapters/Auth/ldap.js index 7312aea67b..f4a1f9e353 100644 --- a/src/Adapters/Auth/ldap.js +++ b/src/Adapters/Auth/ldap.js @@ -40,7 +40,8 @@ * ## Auth Payload * The adapter requires the following `authData` fields: * - `id`: The user's LDAP username. - * - `password`: The user's LDAP password. + * - `password`: The user's LDAP password. Must be a non-empty string; an empty or missing + * password is rejected without contacting the directory. * * ### Example Auth Payload * ```json @@ -122,6 +123,18 @@ function validateAuthData(authData, options) { new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'LDAP: Wrong username or password') ); } + // A missing, empty or non-string password is serialized by ldapjs as a zero-length + // credential, which makes the bind an unauthenticated authentication mechanism of simple + // bind (RFC 4513 section 5.1.2). A directory may answer that with success and map the + // connection to anonymous, which this adapter would otherwise read as a successful + // authentication. RFC 4513 section 5.1.2 states that clients must not use the + // unauthenticated mechanism to authenticate, so the credential is rejected here, before + // the directory is contacted. + if (typeof authData.password !== 'string' || authData.password.length === 0) { + return Promise.reject( + new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'LDAP: Wrong username or password') + ); + } const client = ldapjs.createClient(clientOptions); const escapedId = escapeDN(authData.id); const userCn =