From b5f47f9f61d9f9800ca1bae5afcc7cf5a965441e Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Thu, 26 Jul 2018 11:20:25 +0200 Subject: [PATCH 01/16] add WWW-Authenticate header to 401 response during user authentication --- src/api/userAuthentication.js | 13 +++++++++++-- test/unit/api/userAuthentication.test.js | 7 ++++++- test/unit/mock/index.js | 1 + 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/api/userAuthentication.js b/src/api/userAuthentication.js index c76c3a4..070ef37 100644 --- a/src/api/userAuthentication.js +++ b/src/api/userAuthentication.js @@ -45,7 +45,7 @@ export default class UserAuthentication { run(req: Object, res: Object) { let authHeader = req.get('Authorization'); if (!authHeader) { - res.sendStatus(401); + this._sendUnauthorized(res); } else { try { let credentials = UserAuthentication.parseBasicAuthHeader(authHeader); @@ -54,7 +54,7 @@ export default class UserAuthentication { credentials.password ).then((success) => { if (!success) { - res.sendStatus(401); + this._sendUnauthorized(res); } else { return this.getToken(credentials.username).then((token) => { res.send(token); @@ -74,6 +74,15 @@ export default class UserAuthentication { } } + /** + * Send an HTTP 401 (Unauthorized) response including a WWW-Authenticate header + * @param {Object} res - Response. + */ + _sendUnauthorized(res: Object): void { + res.set('WWW-Authenticate', 'Basic realm="kubernetes"'); + res.sendStatus(401); + } + /** * Get/Generate token for user * @param {string} username - Username. diff --git a/test/unit/api/userAuthentication.test.js b/test/unit/api/userAuthentication.test.js index 51e9cc0..5d45bcc 100644 --- a/test/unit/api/userAuthentication.test.js +++ b/test/unit/api/userAuthentication.test.js @@ -79,6 +79,9 @@ describe('UserAuthentication.run()', () => { expect.hasAssertions(); return userAuthentication.run(requestMock, responseMock).then(() => { + expect(responseMock.set).toHaveBeenCalled(); + expect(responseMock.set.mock.calls[0][0]) + .toEqual('WWW-Authenticate', 'Basic realm="kubernetes"'); expect(responseMock.sendStatus).toHaveBeenCalled(); expect(responseMock.sendStatus.mock.calls[0][0]) .toEqual(401); @@ -90,7 +93,9 @@ describe('UserAuthentication.run()', () => { const responseMock = getResponseMock(); userAuthentication.run(requestMock, responseMock); - + expect(responseMock.set).toHaveBeenCalled(); + expect(responseMock.set.mock.calls[0][0]) + .toEqual('WWW-Authenticate', 'Basic realm="kubernetes"'); expect(responseMock.sendStatus).toHaveBeenCalled(); expect(responseMock.sendStatus.mock.calls[0][0]) .toEqual(401); diff --git a/test/unit/mock/index.js b/test/unit/mock/index.js index cebf3e6..d7f467e 100644 --- a/test/unit/mock/index.js +++ b/test/unit/mock/index.js @@ -2,6 +2,7 @@ const getResponseMock = () => { return { send: jest.fn(), sendStatus: jest.fn(), + set: jest.fn(), }; }; From 934d571eea21af6fb919f152e0a91d56dfa91ff0 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Thu, 26 Jul 2018 11:20:55 +0200 Subject: [PATCH 02/16] add some more tests to config and ldap client --- jest.config.js | 4 ++++ test/unit/config.test.js | 1 + test/unit/ldap/client.test.js | 23 +++++++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/jest.config.js b/jest.config.js index fd811d3..b5975d1 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,4 +1,8 @@ module.exports = { verbose: true, testEnvironment: 'node', + collectCoverageFrom: [ + 'src/**', + '!src/index.js', + ], }; diff --git a/test/unit/config.test.js b/test/unit/config.test.js index 059f8b5..53901d1 100644 --- a/test/unit/config.test.js +++ b/test/unit/config.test.js @@ -78,6 +78,7 @@ const fixtures = { default: true, testValues: [ {value: 'false', expected: false}, + {value: 'true', expected: true}, {value: 'abc', expected: true}, ], }, diff --git a/test/unit/ldap/client.test.js b/test/unit/ldap/client.test.js index 8293759..c6d8740 100644 --- a/test/unit/ldap/client.test.js +++ b/test/unit/ldap/client.test.js @@ -20,6 +20,7 @@ let connection = ldap.createClient(); let client = new Client(connection); beforeEach(() => { + client._secure = true; connection.starttlsReturnsError = false; connection.bindReturnsError = false; connection.searchReturnsError = false; @@ -48,6 +49,17 @@ describe('Client.bind()', () => { client.bind(fixtures.dn, fixtures.password) ).resolves.toBe(false); }); + + test('Rejects on unprotected connection', () => { + client._secure = false; + + expect.hasAssertions(); + return expect( + client.bind(fixtures.dn, fixtures.password) + ).rejects.toEqual(new Error( + 'ldap connection not tls protected' + )); + }); }); describe('Client.search()', () => { @@ -98,4 +110,15 @@ describe('Client.search()', () => { client.search(fixtures.filter) ).rejects.toBe(connection.searchEmitsEndStatus); }); + + test('Rejects on unprotected connection', () => { + client._secure = false; + + expect.hasAssertions(); + return expect( + client.search(fixtures.filter) + ).rejects.toEqual(new Error( + 'ldap connection not tls protected' + )); + }); }); From 510ad9b1a172412af16fbced4758ac5586fb5b20 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Thu, 26 Jul 2018 11:44:55 +0200 Subject: [PATCH 03/16] lint: missing comma --- src/app.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app.js b/src/app.js index 15f7e24..e074181 100644 --- a/src/app.js +++ b/src/app.js @@ -19,8 +19,8 @@ let ldapClient = new Client( reconnect: { initialDelay: config.ldap.reconnectInitialDelay, maxDelay: config.ldap.reconnectMaxDelay, - failAfter: config.ldap.reconnectFailAfter - } + failAfter: config.ldap.reconnectFailAfter, + }, }), config.ldap.baseDn, config.ldap.bindDn, From 50d0a30a5304c2c7831100712436bfccce8ab662 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Thu, 26 Jul 2018 11:56:07 +0200 Subject: [PATCH 04/16] build and publish docker image for dev branch too --- .travis.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 610d772..03c8d3b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,8 +14,8 @@ cache: - "node_modules" yarn: true after_success: - - test "$TRAVIS_TAG" != "" && docker build -t gyselroth/kube-ldap:$TRAVIS_BRANCH . - - test "$TRAVIS_TAG" != "" && docker login -u "$DOCKER_USERNAME" -p "$DOCKER_PASSWORD" + - (test "$TRAVIS_TAG" != "" || test "$TRAVIS_BRANCH" = "dev") && docker build -t gyselroth/kube-ldap:$TRAVIS_BRANCH . + - (test "$TRAVIS_TAG" != "" || test "$TRAVIS_BRANCH" = "dev") && docker login -u "$DOCKER_USERNAME" -p "$DOCKER_PASSWORD" - test "$TRAVIS_TAG" != "" && docker tag gyselroth/kube-ldap:$TRAVIS_BRANCH gyselroth/kube-ldap:latest - - test "$TRAVIS_TAG" != "" && docker push gyselroth/kube-ldap:$TRAVIS_BRANCH + - (test "$TRAVIS_TAG" != "" || test "$TRAVIS_BRANCH" = "dev") && docker push gyselroth/kube-ldap:$TRAVIS_BRANCH - test "$TRAVIS_TAG" != "" && docker push gyselroth/kube-ldap:latest From 588a90b2dd750a0ef1943b076305abd6ad415c08 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Fri, 30 Nov 2018 09:43:08 +0100 Subject: [PATCH 05/16] add logger to ldap connection --- package.json | 1 + src/app.js | 2 ++ 2 files changed, 3 insertions(+) diff --git a/package.json b/package.json index 5dfd3dc..5ce6482 100644 --- a/package.json +++ b/package.json @@ -9,6 +9,7 @@ "atob": "^2.0.3", "babel-polyfill": "^6.26.0", "body-parser": "^1.18.2", + "bunyan-winston-adapter": "^0.2.0", "cors": "^2.8.4", "express": "^4.16.3", "jsonwebtoken": "^8.2.0", diff --git a/src/app.js b/src/app.js index e074181..74af83c 100644 --- a/src/app.js +++ b/src/app.js @@ -4,6 +4,7 @@ import express from 'express'; import bodyParser from 'body-parser'; import cors from 'cors'; import morgan from 'morgan'; +import bunyanWinstonAdapter from 'bunyan-winston-adapter'; import ldap from 'ldapjs'; import {config} from './config'; import logger from './logger'; @@ -21,6 +22,7 @@ let ldapClient = new Client( maxDelay: config.ldap.reconnectMaxDelay, failAfter: config.ldap.reconnectFailAfter, }, + log: bunyanWinstonAdapter.createAdapter(logger), }), config.ldap.baseDn, config.ldap.bindDn, From 01752742e92abfa37dd62d3c2f37bb886685b796 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Fri, 30 Nov 2018 09:49:42 +0100 Subject: [PATCH 06/16] fix alpine glibc installation --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 349aa75..18488cd 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ FROM node:8.10.0-alpine RUN apk --no-cache add ca-certificates wget && \ - wget -q -O /etc/apk/keys/sgerrand.rsa.pub https://raw.githubusercontent.com/sgerrand/alpine-pkg-glibc/master/sgerrand.rsa.pub && \ + wget -q -O /etc/apk/keys/sgerrand.rsa.pub https://alpine-pkgs.sgerrand.com/sgerrand.rsa.pub && \ wget https://github.com/sgerrand/alpine-pkg-glibc/releases/download/2.27-r0/glibc-2.27-r0.apk && \ apk add glibc-2.27-r0.apk && \ apk del wget From 8da4c89e0430d09bdef01ac07a1774a19b0a7a65 Mon Sep 17 00:00:00 2001 From: Christoffer Reijer Date: Wed, 19 Dec 2018 19:38:21 +0100 Subject: [PATCH 07/16] Handle response from AD with groups as single string In our environment with Active Directory on Windows Server 2016 we don't get a response where `memberOf` is a list but instead a string with a single group. Adjust the code to allow for such responses. --- src/ldap/mapping.js | 16 +++++++++++++++- test/unit/ldap/mapping.test.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/ldap/mapping.js b/src/ldap/mapping.js index 2ecf3d7..7474f9d 100644 --- a/src/ldap/mapping.js +++ b/src/ldap/mapping.js @@ -45,7 +45,7 @@ class Mapping { let object = { username: ldapObject[this.username], uid: ldapObject[this.uid], - groups: ldapObject[this.groups].map((group) => { + groups: this.getGroups(ldapObject).map((group) => { return canonicalizeDn(group); }), extra: {}, @@ -55,6 +55,20 @@ class Mapping { } return object; } + + /** + * Get group of LDAP object + * @param {Object} ldapObject - Ldap object to convert + * @return {Array} + */ + getGroups(ldapObject: Object): Array { + let groups = ldapObject[this.groups]; + if (groups instanceof Array) { + return groups; + } else { + return [groups]; + } + } } export default Mapping; diff --git a/test/unit/ldap/mapping.test.js b/test/unit/ldap/mapping.test.js index 4976597..12cc06b 100644 --- a/test/unit/ldap/mapping.test.js +++ b/test/unit/ldap/mapping.test.js @@ -23,6 +23,13 @@ const fixtures = { uidNumber: 1, gidNumber: [10, 11], }, + ldapObjectWithSingleGroup: { + mail: 'john.doe@example.com', + cn: 'john.doe', + memberOf: 'cn=kubernetes,ou=groups,dc=example,dc=com', + uidNumber: 1, + gidNumber: [10, 11], + }, kubernetesObject: { username: 'john.doe@example.com', uid: 'john.doe', @@ -35,6 +42,17 @@ const fixtures = { 'gidNumber': [10, 11], }, }, + kubernetesObjectWithSingleGroup: { + username: 'john.doe@example.com', + uid: 'john.doe', + groups: [ + 'kubernetes', + ], + extra: { + 'uidNumber': 1, + 'gidNumber': [10, 11], + }, + }, }; describe('Mapping.getLdapAttributes()', () => { @@ -76,4 +94,17 @@ describe('Mapping.ldapToKubernetes()', () => { mapping.ldapToKubernetes(fixtures.ldapObject) ).toEqual(fixtures.kubernetesObject); }); + + test('handles when groups attribute is a single object', () => { + let mapping = new Mapping( + fixtures.mapping.username, + fixtures.mapping.uid, + fixtures.mapping.groups, + fixtures.mapping.extraFields + ); + + expect( + mapping.ldapToKubernetes(fixtures.ldapObjectWithSingleGroup) + ).toEqual(fixtures.kubernetesObjectWithSingleGroup); + }); }); From 3a589b82d238d9d39a384bbb3fbff5ed6b7e7a61 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Thu, 20 Dec 2018 08:15:31 +0100 Subject: [PATCH 08/16] don't push pull request builds to dockerhub --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 03c8d3b..4c8b72e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,7 +15,7 @@ cache: yarn: true after_success: - (test "$TRAVIS_TAG" != "" || test "$TRAVIS_BRANCH" = "dev") && docker build -t gyselroth/kube-ldap:$TRAVIS_BRANCH . - - (test "$TRAVIS_TAG" != "" || test "$TRAVIS_BRANCH" = "dev") && docker login -u "$DOCKER_USERNAME" -p "$DOCKER_PASSWORD" + - (test "$TRAVIS_TAG" != "" || (test "$TRAVIS_BRANCH" = "dev" && test "$TRAVIS_PULL_REQUEST" = "false")) && docker login -u "$DOCKER_USERNAME" -p "$DOCKER_PASSWORD" - test "$TRAVIS_TAG" != "" && docker tag gyselroth/kube-ldap:$TRAVIS_BRANCH gyselroth/kube-ldap:latest - - (test "$TRAVIS_TAG" != "" || test "$TRAVIS_BRANCH" = "dev") && docker push gyselroth/kube-ldap:$TRAVIS_BRANCH + - (test "$TRAVIS_TAG" != "" || (test "$TRAVIS_BRANCH" = "dev" && test "$TRAVIS_PULL_REQUEST" = "false")) && docker push gyselroth/kube-ldap:$TRAVIS_BRANCH - test "$TRAVIS_TAG" != "" && docker push gyselroth/kube-ldap:latest From d0e69a84b83523d7e171107d5472710c98d2a34a Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Fri, 4 Jan 2019 11:40:07 +0100 Subject: [PATCH 09/16] correct unit (ms instead of s) for reconnect parameters --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 7d0aa01..d84b88f 100644 --- a/README.md +++ b/README.md @@ -150,8 +150,8 @@ List of configurable values: |`config.ldap.baseDn`|Base DN for LDAP search|`LDAP_BASEDN`|dc=example,dc=com| |`config.ldap.filter`|Filter for LDAP search|`LDAP_FILTER`|(uid=%s)| |`config.ldap.timeout`|Timeout for LDAP connections & operations (in seconds)|`LDAP_TIMEOUT`|0 (infinite for operations, OS default for connections)| -|`config.ldap.reconnectInitialDelay`|Seconds to wait before reconnecting|`LDAP_RECONN_INIT_DELAY`|100| -|`config.ldap.reconnectMaxDelay`|Maximum seconds to wait before reconnecting|`LDAP_RECONN_MAX_DELAY`|1000| +|`config.ldap.reconnectInitialDelay`|Milliseconds to wait before reconnecting|`LDAP_RECONN_INIT_DELAY`|100| +|`config.ldap.reconnectMaxDelay`|Maximum milliseconds to wait before reconnecting|`LDAP_RECONN_MAX_DELAY`|1000| |`config.ldap.reconnectFailAfter`|Fail after number of retries|`LDAP_RECONN_FAIL_AFTER`|10| |`config.mapping.username`|Name of ldap attribute to be used as username in kubernetes TokenReview|`MAPPING_USERNAME`|uid| |`config.mapping.uid`|Name of ldap attribute to be used as uid in kubernetes TokenReview|`MAPPING_UID`|uid| From 96bbae71e21099fc81973eebf3ac1bcfd9425acc Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Fri, 4 Jan 2019 11:40:52 +0100 Subject: [PATCH 10/16] establish tls connection on every connection, prevent code after reject/resolve from running --- src/ldap/client.js | 67 ++++++++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 29 deletions(-) diff --git a/src/ldap/client.js b/src/ldap/client.js index e7fcba1..3f992ad 100644 --- a/src/ldap/client.js +++ b/src/ldap/client.js @@ -17,11 +17,13 @@ export default class Client { constructor(conn: Object, basedn: string, binddn: string, bindpw: string) { this.client = conn; this._secure = false; - this.client.starttls({}, [], (err, res) => { - if (err) { - throw err; - } - this._secure = true; + this.client.on('connect', () => { + this.client.starttls({}, [], (err, res) => { + if (err) { + throw err; + } + this._secure = true; + }); }); this.basedn = basedn; this.binddn = binddn; @@ -42,8 +44,9 @@ export default class Client { this.client.bind(dn, password, [], (err, res) => { if (err) { resolve(false); + } else { + resolve(true); } - resolve(true); }); }); } @@ -72,31 +75,37 @@ export default class Client { this.client.bind(this.binddn, this.bindpw, [], (err, res) => { if (err) { reject(err); - } - - let options = { - filter: filter, - scope: 'sub', - attributes: attributes, - }; - that.client.search(basedn, options, [], (err, res) => { - if (err) { - reject(err); - } - - res.on('searchEntry', function(entry) { - resolve(entry.object); - }); - res.on('error', function(err) { - reject(err); - }); - res.on('end', function(result) { - if (result.status !== 0) { - reject(result.status); + } else { + let options = { + filter: filter, + scope: 'sub', + attributes: attributes, + }; + that.client.search(basedn, options, [], (err, res) => { + let searchResult = null; + if (err) { + reject(err); + } else { + res.on('searchEntry', function(entry) { + searchResult = entry.object; + }); + res.on('error', function(err) { + reject(err); + }); + res.on('end', function(result) { + if (result.status !== 0) { + reject(result.status); + } else { + if (searchResult) { + resolve(searchResult); + } else { + reject(new Error(`no object found with filter [${filter}]`)); + } + } + }); } - reject(new Error(`no object found with filter [${filter}]`)); }); - }); + } }); }); } From 8e8d693643e7d403d90114c1a8ceccd4cf594faa Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Fri, 4 Jan 2019 13:30:01 +0100 Subject: [PATCH 11/16] make the usage of starttls configurable --- src/app.js | 3 ++- src/config.js | 1 + src/ldap/client.js | 27 ++++++++++++++++----------- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/app.js b/src/app.js index 74af83c..adaca1a 100644 --- a/src/app.js +++ b/src/app.js @@ -26,7 +26,8 @@ let ldapClient = new Client( }), config.ldap.baseDn, config.ldap.bindDn, - config.ldap.bindPw + config.ldap.bindPw, + config.ldap.startTls, ); let authenticator = new Authenticator(ldapClient, config.ldap.filter, logger); diff --git a/src/config.js b/src/config.js index c16eea2..af88a3f 100644 --- a/src/config.js +++ b/src/config.js @@ -34,6 +34,7 @@ const getConfig = () => { reconnectInitialDelay: parseInt(process.env.LDAP_RECONN_INIT_DELAY) || 100, reconnectMaxDelay: parseInt(process.env.LDAP_RECONN_MAX_DELAY) || 1000, reconnectFailAfter: parseInt(process.env.LDAP_RECONN_FAIL_AFTER) || 10, + startTls: parseBooleanFromEnv(process.env.LDAP_STARTTLS, true), }, mapping: { username: process.env.MAPPING_USERNAME || 'uid', diff --git a/src/ldap/client.js b/src/ldap/client.js index 3f992ad..49449ed 100644 --- a/src/ldap/client.js +++ b/src/ldap/client.js @@ -5,6 +5,7 @@ export default class Client { basedn: string; binddn: string; bindpw: string; + startTls: boolean; _secure: boolean; /** @@ -12,19 +13,23 @@ export default class Client { * @param {Object} conn - Ldap connection. * @param {string} basedn - The base DN to use. * @param {string} binddn - DN of the bind user to use. - * @param {string} bindpw - Password of the bind user to use . + * @param {string} bindpw - Password of the bind user to use. + * @param {boolean} startTls - Whether to use StartTLS on the connection or not. */ - constructor(conn: Object, basedn: string, binddn: string, bindpw: string) { + constructor(conn: Object, basedn: string, binddn: string, bindpw: string, startTls: boolean) { this.client = conn; + this.startTls = startTls; this._secure = false; - this.client.on('connect', () => { - this.client.starttls({}, [], (err, res) => { - if (err) { - throw err; - } - this._secure = true; + if (this.startTls) { + this.client.on('connect', () => { + this.client.starttls({}, [], (err, res) => { + if (err) { + throw err; + } + this._secure = true; + }); }); - }); + } this.basedn = basedn; this.binddn = binddn; this.bindpw = bindpw; @@ -38,7 +43,7 @@ export default class Client { */ bind(dn: string, password: string): Promise { return new Promise((resolve, reject) => { - if (!this._secure) { + if (this.startTls && !this._secure) { reject(new Error('ldap connection not tls protected')); } this.client.bind(dn, password, [], (err, res) => { @@ -68,7 +73,7 @@ export default class Client { } return new Promise((resolve, reject) => { - if (!this._secure) { + if (this.startTls && !this._secure) { reject(new Error('ldap connection not tls protected')); } let that = this; From f7c4dfe955796ff4c4b64ec53262e4ffc78805a6 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Fri, 4 Jan 2019 14:40:40 +0100 Subject: [PATCH 12/16] fix tests --- __mocks__/ldapjs.js | 24 +++++++++++++++--------- test/unit/ldap/client.test.js | 17 +++++++++++++---- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/__mocks__/ldapjs.js b/__mocks__/ldapjs.js index e4302b7..abd1c80 100644 --- a/__mocks__/ldapjs.js +++ b/__mocks__/ldapjs.js @@ -8,13 +8,14 @@ class LdapMock { bindReturnsError: boolean; searchReturnsError: boolean; searchEmitsError: boolean; - searchEmitsEnd: boolean; + searchEmitsResult: boolean; searchEmitsEndStatus: number; searchResult: Object; emitter: EventEmitter; starttls: (string, Array) => Promise; bind: (string, Array) => Promise; search: (string, Array) => Promise; + on: (string, (any) => any) => void; /** creates the mock */ constructor() { @@ -22,7 +23,7 @@ class LdapMock { this.bindReturnsError = false; this.searchReturnsError = false; this.searchEmitsError = false; - this.searchEmitsEnd = false; + this.searchEmitsResult = true; this.searchEmitsEndStatus = 0; this.searchResult = {}; this.emitter = new EventEmitter(); @@ -48,21 +49,26 @@ class LdapMock { callback(new Error('error by mock')); } else { setTimeout(() => { - if (this.searchEmitsEnd) { - this.emitter.emit('end', { - status: this.searchEmitsEndStatus, - }); - } else if (this.searchEmitsError) { + if (this.searchEmitsError) { this.emitter.emit('error', new Error('error by mock')); } else { - this.emitter.emit('searchEntry', { - object: this.searchResult, + if (this.searchEmitsResult) { + this.emitter.emit('searchEntry', { + object: this.searchResult, + }); + } + this.emitter.emit('end', { + status: this.searchEmitsEndStatus, }); } }, 100); callback(null, this.emitter); } }); + this.on = jest.fn(); + this.on.mockImplementation((event, callback) => { + callback(); + }); } } diff --git a/test/unit/ldap/client.test.js b/test/unit/ldap/client.test.js index c6d8740..3e8555b 100644 --- a/test/unit/ldap/client.test.js +++ b/test/unit/ldap/client.test.js @@ -3,6 +3,9 @@ import ldap from 'ldapjs'; jest.mock('ldapjs'); const fixtures = { + basedn: 'dc=example,dc=com', + binddn: 'uid=bind,dc=example,dc=com', + bindpw: 'secret', username: 'john.doe', dn: 'uid=john.doe,dc=example,dc=com', password: 'secret', @@ -17,15 +20,21 @@ const fixtures = { }; let connection = ldap.createClient(); -let client = new Client(connection); +let client = null; beforeEach(() => { - client._secure = true; + client = new Client( + connection, + fixtures.basedn, + fixtures.binddn, + fixtures.bindpw, + true + ); connection.starttlsReturnsError = false; connection.bindReturnsError = false; connection.searchReturnsError = false; connection.searchEmitsError = false; - connection.searchEmitsEnd = false; + connection.searchEmitsResult = true; connection.searchEmitsEndStatus = 0; connection.searchResult = { uid: fixtures.username, @@ -91,7 +100,7 @@ describe('Client.search()', () => { }); test('Rejects on empty result', () => { - connection.searchEmitsEnd = true; + connection.searchEmitsResult = false; expect.hasAssertions(); return expect( From 677a6903f3fd1dc750ee8d4340ba04febac798fc Mon Sep 17 00:00:00 2001 From: Christoffer Reijer Date: Sat, 5 Jan 2019 11:53:45 +0100 Subject: [PATCH 13/16] Bump base image for container The base image for the container is bumped from Node 8.10 to 8.15 which also bumps Alpine from 3.6 to 3.8. --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 18488cd..2181a80 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM node:8.10.0-alpine +FROM node:8.15.0-alpine RUN apk --no-cache add ca-certificates wget && \ wget -q -O /etc/apk/keys/sgerrand.rsa.pub https://alpine-pkgs.sgerrand.com/sgerrand.rsa.pub && \ From 93e37a3c757d55dd953180ae10e31254551f5fa2 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Mon, 7 Jan 2019 10:28:08 +0100 Subject: [PATCH 14/16] change default loglevel to info --- README.md | 2 +- src/config.js | 2 +- test/unit/config.test.js | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index d84b88f..ec3d2db 100644 --- a/README.md +++ b/README.md @@ -139,7 +139,7 @@ List of configurable values: |Setting|Description|Environment Variable| Default Value| |-------|-----------|--------------------|--------------| |`config.port`|HTTP port to listen|`PORT`|8081 (8080 if TLS is disabled)| -|`config.loglevel`|Loglevel for winston logger|`LOGLEVEL`|debug| +|`config.loglevel`|Loglevel for winston logger. **CAUTION: debug loglevel may log sensitive parameters like user passwords**|`LOGLEVEL`|info| |`config.tls.enabled`|Enable TLS (HTTPS). **DO NOT DISABLE IN PRODUCTION UNLESS YOU HAVE A TLS REVERSE PROXY IN PLACE**|`TLS_ENABLED` ("true" or "false")|true| |`config.tls.cert`|Path to certificate (pem) to use for TLS (HTTPS)|`TLS_CERT_PATH`|/etc/ssl/kube-ldap/cert.pem| |`config.tls.key`|Path to private key (pem) to use for TLS (HTTPS)|`TLS_KEY_PATH`|/etc/ssl/kube-ldap/key.pem| diff --git a/src/config.js b/src/config.js index af88a3f..295af37 100644 --- a/src/config.js +++ b/src/config.js @@ -23,7 +23,7 @@ const parseArrayFromEnv = ( const getConfig = () => { let config = { - loglevel: process.env.LOGLEVEL || 'debug', + loglevel: process.env.LOGLEVEL || 'info', ldap: { uri: process.env.LDAP_URI || 'ldap://ldap.example.com', bindDn: process.env.LDAP_BINDDN || 'uid=bind,dc=example,dc=com', diff --git a/test/unit/config.test.js b/test/unit/config.test.js index 53901d1..c28bb5d 100644 --- a/test/unit/config.test.js +++ b/test/unit/config.test.js @@ -3,8 +3,8 @@ import {getConfig} from '../../src/config'; const fixtures = { 'loglevel': { env: 'LOGLEVEL', - default: 'debug', - testValues: ['info'], + default: 'info', + testValues: ['debug'], }, 'ldap.uri': { env: 'LDAP_URI', From 91856ab60eb854f469d19db806bcbdc0e76fd32e Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Mon, 7 Jan 2019 10:28:45 +0100 Subject: [PATCH 15/16] introduce changelog --- CHANGELOG.md | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..26a9fa8 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,45 @@ +# Changelog +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [Unreleased] +### Changed +- Failed authentication sends a WWW-Authenticate header in the HTTP response +- Default loglevel is now info (was debug) +- Update node to latest 8.x LTS in docker image + +### Added +- LDAP related logging +- Configuration parameter whether to use StartTLS for LDAP or not (enabled by default). + +### Fixed +- Single group memberships are returned as a string (instead of an array) by LDAP in some cases and broke the membership resolution. This is now handled correctly. +- Fixed units in README for LDAP reconnect config parameters. + +## [1.2.1] - 2018-07-19 +### Added +- LDAP reconnect logic (with configurable parameters) + +## [1.2.0] - 2018-04-20 +### Added +- Configuration parameters for LDAP connection and operation timeouts. +- Configurable mapping between LDAP and kubernetes attributes. + +## [1.1.0] - 2018-03-27 +### Security +- TLS (HTTPS) support (enabled by default). + +### Changed +- Log error if a DN is not in a canonicalizable format. + +## [1.0.0] - 2018-03-27 +### Added +- Initial key functionality + +[Unreleased]: https://github.com/gyselroth/kube-ldap/compare/master...dev +[1.2.1]: https://github.com/gyselroth/kube-ldap/compare/v1.2.0...v1.2.1 +[1.2.0]: https://github.com/gyselroth/kube-ldap/compare/v1.1.0...v1.2.0 +[1.1.0]: https://github.com/gyselroth/kube-ldap/compare/v1.0.0...v1.1.0 +[1.0.0]: https://github.com/gyselroth/kube-ldap/tree/v1.0.0 From 83179ab163743d60783ac753499523a277314db0 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Mon, 7 Jan 2019 10:32:33 +0100 Subject: [PATCH 16/16] version bump to 1.3.0 --- CHANGELOG.md | 3 +++ package.json | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26a9fa8..583996d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] + +## [1.3.0] - 2019-01-07 ### Changed - Failed authentication sends a WWW-Authenticate header in the HTTP response - Default loglevel is now info (was debug) @@ -39,6 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Initial key functionality [Unreleased]: https://github.com/gyselroth/kube-ldap/compare/master...dev +[1.3.0]: https://github.com/gyselroth/kube-ldap/compare/v1.2.1...v1.3.0 [1.2.1]: https://github.com/gyselroth/kube-ldap/compare/v1.2.0...v1.2.1 [1.2.0]: https://github.com/gyselroth/kube-ldap/compare/v1.1.0...v1.2.0 [1.1.0]: https://github.com/gyselroth/kube-ldap/compare/v1.0.0...v1.1.0 diff --git a/package.json b/package.json index 5ce6482..ce552c3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "kube-ldap", - "version": "1.2.1", + "version": "1.3.0", "description": "kubernetes token webhook to check bearer tokens against ldap", "main": "src/index.js", "author": "Fabian Jucker ",