From 1f12eca334dc2a9c022406d9258623c8a031ab66 Mon Sep 17 00:00:00 2001 From: James Sumners Date: Sat, 16 Nov 2019 10:48:00 -0500 Subject: [PATCH 1/3] Add integration tests (#571) * Initial integration test, um, test * Hack in service * Add integration test for issue #480 * Add fix for issue #418 * Add fix for issue #370 --- .github/workflows/integration.yml | 41 +++++++++++++++++ docker-compose.yml | 8 ++++ lib/client/client.js | 3 +- lib/url.js | 8 +++- package.json | 2 + test-integration/client/connect.test.js | 21 +++++++++ test-integration/client/issues.test.js | 58 +++++++++++++++++++++++++ test/client.test.js | 11 +++++ 8 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/integration.yml create mode 100644 docker-compose.yml create mode 100644 test-integration/client/connect.test.js create mode 100644 test-integration/client/issues.test.js diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml new file mode 100644 index 0000000..6a206e1 --- /dev/null +++ b/.github/workflows/integration.yml @@ -0,0 +1,41 @@ +name: 'Integration Tests' + +# Notes: +# https://github.community/t5/GitHub-Actions/Github-Actions-services-not-reachable/m-p/30739/highlight/true#M538 + +on: + pull_request: + branches: + - master + - next + +jobs: + baseline: + name: Baseline Tests + runs-on: ubuntu-latest + + # services: + # openldap: + # image: docker.pkg.github.com/ldapjs/docker-test-openldap/openldap:1.0 + # ports: + # - 389:389 + # - 636:636 + + steps: + - uses: actions/checkout@v1 + - uses: actions/setup-node@v1 + + # Hack way to start service since GitHub doesn't integrate with its own services + - name: Docker login + run: docker login docker.pkg.github.com -u ${GITHUB_ACTOR} -p ${GITHUB_TOKEN} + env: + GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} + - name: Pull Docker image + run: docker pull "docker.pkg.github.com/ldapjs/docker-test-openldap/openldap:1.0" + - name: Start OpenLDAP service + run: docker run -it -d --name openldap -p 389:389 -p 636:636 docker.pkg.github.com/ldapjs/docker-test-openldap/openldap:1.0 + + - name: Install Packages + run: npm install + - name: Run Tests + run: npm run test:integration diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 0000000..86dbede --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,8 @@ +version: '3' + +services: + openldap: + image: docker.pkg.github.com/ldapjs/docker-test-openldap/openldap:1.0 + ports: + - 389:389 + - 636:636 diff --git a/lib/client/client.js b/lib/client/client.js index d04861b..c60dd9c 100644 --- a/lib/client/client.js +++ b/lib/client/client.js @@ -369,9 +369,8 @@ Client.prototype.exop = function exop (name, value, controls, callback) { if (typeof (value) === 'function') { callback = value controls = [] - value = '' + value = undefined } - if (!(Buffer.isBuffer(value) || typeof (value) === 'string')) { throw new TypeError('value (Buffer || string) required') } if (typeof (controls) === 'function') { callback = controls controls = [] diff --git a/lib/url.js b/lib/url.js index 4e1b061..6a12575 100644 --- a/lib/url.js +++ b/lib/url.js @@ -8,7 +8,13 @@ const filter = require('./filters/') module.exports = { parse: function (urlStr, parseDN) { - const parsedURL = new url.URL(urlStr) + let parsedURL + try { + parsedURL = new url.URL(urlStr) + } catch (error) { + throw new TypeError(urlStr + ' is an invalid LDAP url (scope)') + } + if (!parsedURL.protocol || !(parsedURL.protocol === 'ldap:' || parsedURL.protocol === 'ldaps:')) { throw new TypeError(urlStr + ' is an invalid LDAP url (protocol)') } const u = { diff --git a/package.json b/package.json index 786b84b..8573965 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,8 @@ "test:cov": "tap", "test:cov:html": "tap --coverage-report=html", "test:watch": "tap -n -w --no-coverage-report", + "test:integration": "tap --no-cov 'test-integration/**/*.test.js'", + "test:integration:local": "docker-compose up -d && npm run test:integration && docker-compose down", "lint": "standard | snazzy", "lint:ci": "standard" }, diff --git a/test-integration/client/connect.test.js b/test-integration/client/connect.test.js new file mode 100644 index 0000000..1a82339 --- /dev/null +++ b/test-integration/client/connect.test.js @@ -0,0 +1,21 @@ +'use strict' + +const tap = require('tap') +const ldapjs = require('../../lib') + +const SCHEME = process.env.SCHEME || 'ldap' +const HOST = process.env.HOST || '127.0.0.1' +const PORT = process.env.PORT || 389 + +const baseURL = `${SCHEME}://${HOST}:${PORT}` + +tap.test('connects to a server', t => { + t.plan(2) + + const client = ldapjs.createClient({ url: baseURL }) + client.bind('cn=Philip J. Fry,ou=people,dc=planetexpress,dc=com', 'fry', (err) => { + t.error(err) + t.pass() + client.unbind() + }) +}) diff --git a/test-integration/client/issues.test.js b/test-integration/client/issues.test.js new file mode 100644 index 0000000..0992f65 --- /dev/null +++ b/test-integration/client/issues.test.js @@ -0,0 +1,58 @@ +'use strict' + +const tap = require('tap') +const ldapjs = require('../../lib') + +const SCHEME = process.env.SCHEME || 'ldap' +const HOST = process.env.HOST || '127.0.0.1' +const PORT = process.env.PORT || 389 + +const baseURL = `${SCHEME}://${HOST}:${PORT}` + +tap.test('modifyDN with long name (issue #480)', t => { + const longStr = 'a292979f2c86d513d48bbb9786b564b3c5228146e5ba46f404724e322544a7304a2b1049168803a5485e2d57a544c6a0d860af91330acb77e5907a9e601ad1227e80e0dc50abe963b47a004f2c90f570450d0e920d15436fdc771e3bdac0487a9735473ed3a79361d1778d7e53a7fb0e5f01f97a75ef05837d1d5496fc86968ff47fcb64' + const targetDN = 'cn=Turanga Leela,ou=people,dc=planetexpress,dc=com' + const client = ldapjs.createClient({ url: baseURL }) + client.bind('cn=admin,dc=planetexpress,dc=com', 'GoodNewsEveryone', bindHandler) + + function bindHandler (err) { + t.error(err) + client.modifyDN( + targetDN, + `cn=${longStr},ou=people,dc=planetexpress,dc=com`, + modifyHandler + ) + } + + function modifyHandler (err, res) { + t.error(err) + t.ok(res) + t.equal(res.status, 0) + + client.modifyDN( + `cn=${longStr},ou=people,dc=planetexpress,dc=com`, + targetDN, + (err, res) => { + t.error(err) + client.unbind(t.end) + } + ) + } +}) + +tap.test('whois works correctly (issue #370)', t => { + const client = ldapjs.createClient({ url: baseURL }) + client.bind('cn=Philip J. Fry,ou=people,dc=planetexpress,dc=com', 'fry', (err) => { + t.error(err) + + client.exop('1.3.6.1.4.1.4203.1.11.3', (err, value, res) => { + t.error(err) + t.ok(value) + t.is(value, 'dn:cn=Philip J. Fry,ou=people,dc=planetexpress,dc=com') + t.ok(res) + t.is(res.status, 0) + + client.unbind(t.end) + }) + }) +}) diff --git a/test/client.test.js b/test/client.test.js index 0111b42..b6e5532 100644 --- a/test/client.test.js +++ b/test/client.test.js @@ -368,6 +368,17 @@ tap.test('createClient', t => { ) }) + tap.test('exception from bad createClient parameter (issue #418)', t => { + try { + // This port number is totally invalid. It will cause the URL parser + // to throw an exception that should be caught. + ldap.createClient({ url: 'ldap://127.0.0.1:13891389' }) + } catch (error) { + t.ok(error) + t.end() + } + }) + // TODO: this test is really flaky. It would be better if we could validate // the options _withouth_ having to connect to a server. // t.test('attaches a child function to logger', async t => { From 6d44d409e4e3fe3461bedec113f4cc80a110481b Mon Sep 17 00:00:00 2001 From: James Sumners Date: Sat, 16 Nov 2019 10:49:11 -0500 Subject: [PATCH 2/3] v2.0.0-pre.3 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 8573965..07c7c6e 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "name": "ldapjs", "homepage": "http://ldapjs.org", "description": "LDAP client and server APIs", - "version": "2.0.0-pre.2", + "version": "2.0.0-pre.3", "license": "MIT", "repository": { "type": "git", From 7d397062946c9dcedd497dbfc7acd941af72d2f6 Mon Sep 17 00:00:00 2001 From: James Sumners Date: Mon, 18 Nov 2019 07:17:36 -0500 Subject: [PATCH 3/3] Fix coveralls config --- .github/workflows/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index cd114b9..c9468cb 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -42,10 +42,10 @@ jobs: - name: Coveralls Parallel uses: coverallsapp/github-action@master with: - github-token: ${{ secrets.github_token }} + github-token: ${{ secrets.GITHUB_TOKEN }} parallel: true - name: Coveralls Finished uses: coverallsapp/github-action@master with: - github-token: ${{ secrets.github_token }} + github-token: ${{ secrets.GITHUB_TOKEN }} parallel-finished: true