From 3eecd5cfdf8db7a0cdc897ea281d009303e7d036 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Fri, 31 May 2019 11:02:11 +0200 Subject: [PATCH 01/16] fix dockerignore --- .dockerignore | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.dockerignore b/.dockerignore index ee7e14c..45de370 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,3 +1,5 @@ .editorconfig -.build +build node_modules +.git +Dockerfile From 68b1eea86a1b6ec38737522af1a2aa399ca1a1db Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Fri, 31 May 2019 13:36:51 +0200 Subject: [PATCH 02/16] fix membership resolution for ldap objects without any membership --- test/unit/ldap/mapping.test.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/unit/ldap/mapping.test.js b/test/unit/ldap/mapping.test.js index 12cc06b..41ab1e4 100644 --- a/test/unit/ldap/mapping.test.js +++ b/test/unit/ldap/mapping.test.js @@ -30,6 +30,12 @@ const fixtures = { uidNumber: 1, gidNumber: [10, 11], }, + ldapObjectWithoutGroup: { + mail: 'john.doe@example.com', + cn: 'john.doe', + uidNumber: 1, + gidNumber: [10, 11], + }, kubernetesObject: { username: 'john.doe@example.com', uid: 'john.doe', @@ -53,6 +59,15 @@ const fixtures = { 'gidNumber': [10, 11], }, }, + kubernetesObjectWithoutGroup: { + username: 'john.doe@example.com', + uid: 'john.doe', + groups: [], + extra: { + 'uidNumber': 1, + 'gidNumber': [10, 11], + }, + }, }; describe('Mapping.getLdapAttributes()', () => { @@ -95,6 +110,19 @@ describe('Mapping.ldapToKubernetes()', () => { ).toEqual(fixtures.kubernetesObject); }); + test('handles when groups attribute is undefined', () => { + let mapping = new Mapping( + fixtures.mapping.username, + fixtures.mapping.uid, + fixtures.mapping.groups, + fixtures.mapping.extraFields + ); + + expect( + mapping.ldapToKubernetes(fixtures.ldapObjectWithoutGroup) + ).toEqual(fixtures.kubernetesObjectWithoutGroup); + }); + test('handles when groups attribute is a single object', () => { let mapping = new Mapping( fixtures.mapping.username, From 4103cbe6b94a364cf27ffbac9dcb802e5e3c6342 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Fri, 31 May 2019 13:37:29 +0200 Subject: [PATCH 03/16] switch from ldapjs to ldapts --- CHANGELOG.md | 9 +++ __mocks__/ldapjs.js | 85 -------------------- __mocks__/ldapts.js | 48 +++++++++++ package.json | 2 +- src/app.js | 12 +-- src/config.js | 4 - src/ldap/client.js | 103 ++++++++---------------- src/ldap/mapping.js | 2 +- test/integration/app.test.js | 19 +---- test/unit/ldap/client.test.js | 48 +---------- yarn.lock | 145 ++++++++++------------------------ 11 files changed, 138 insertions(+), 339 deletions(-) delete mode 100644 __mocks__/ldapjs.js create mode 100644 __mocks__/ldapts.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 706880f..5acfbfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,15 @@ 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 +- Internal: Use [ldapts](https://github.com/ldapts/ldapts) instead of [ldapjs](https://github.com/joyent/node-ldapjs) as ldap library + +### Fixed +- Fix membership resolution for ldap objects without any membership + +### Removed +- *BREAKING:* LDAP StartTLS is no longer supported +- *BREAKING:* LDAP reconnect logic ## [1.3.0] - 2019-01-07 ### Changed diff --git a/__mocks__/ldapjs.js b/__mocks__/ldapjs.js deleted file mode 100644 index abd1c80..0000000 --- a/__mocks__/ldapjs.js +++ /dev/null @@ -1,85 +0,0 @@ -// @flow -declare var jest: any; -import EventEmitter from 'events'; - -/** Mock of Authenticator class */ -class LdapMock { - starttlsReturnsError: boolean; - bindReturnsError: boolean; - searchReturnsError: boolean; - searchEmitsError: 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() { - this.starttlsReturnsError = false; - this.bindReturnsError = false; - this.searchReturnsError = false; - this.searchEmitsError = false; - this.searchEmitsResult = true; - this.searchEmitsEndStatus = 0; - this.searchResult = {}; - this.emitter = new EventEmitter(); - this.starttls = jest.fn(); - this.starttls.mockImplementation((options, controls, callback) => { - if (this.starttlsReturnsError) { - callback(new Error('error by mock')); - } else { - callback(null); - } - }); - this.bind = jest.fn(); - this.bind.mockImplementation((dn, password, controls, callback) => { - if (this.bindReturnsError) { - callback(new Error('error by mock')); - } else { - callback(null); - } - }); - this.search = jest.fn(); - this.search.mockImplementation((base, options, controls, callback) => { - if (this.searchReturnsError) { - callback(new Error('error by mock')); - } else { - setTimeout(() => { - if (this.searchEmitsError) { - this.emitter.emit('error', new Error('error by mock')); - } else { - 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(); - }); - } -} - -let client = null; -const ldapMock = { - createClient: () => { - if (!client) { - client = new LdapMock(); - } - return client; - }, -}; - -export default ldapMock; diff --git a/__mocks__/ldapts.js b/__mocks__/ldapts.js new file mode 100644 index 0000000..d27a949 --- /dev/null +++ b/__mocks__/ldapts.js @@ -0,0 +1,48 @@ +// @flow +declare var jest: any; + +let staticMock = null; +/** Mock of Authenticator class */ +class LdapMock { + bindReturnsError: boolean; + searchReturnsError: boolean; + searchReturnsResult: boolean; + searchResult: Object; + bind: (string, Array) => Promise; + unbind: () => void; + search: (string, Array) => Promise; + on: (string, (any) => any) => void; + + /** creates the mock */ + constructor() { + if (staticMock) { + return staticMock; + } + staticMock = this; + this.bindReturnsError = false; + this.searchReturnsError = false; + this.searchReturnsResult = true; + this.searchResult = {}; + this.bind = jest.fn(); + this.bind.mockImplementation((dn, password, controls) => { + if (this.bindReturnsError) { + throw new Error('error by mock'); + } else { + return null; + } + }); + this.unbind = jest.fn(); + this.search = jest.fn(); + this.search.mockImplementation((base, options, controls) => { + if (this.searchReturnsError) { + throw new Error('error by mock'); + } else { + return { + searchEntries: this.searchReturnsResult ? [this.searchResult] : [], + }; + } + }); + } +} + +export {LdapMock as Client}; diff --git a/package.json b/package.json index ce552c3..12f1805 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "cors": "^2.8.4", "express": "^4.16.3", "jsonwebtoken": "^8.2.0", - "ldapjs": "^1.0.2", + "ldapts": "^1.7.0", "morgan": "^1.9.0", "winston": "^2.4.1" }, diff --git a/src/app.js b/src/app.js index adaca1a..fe7e330 100644 --- a/src/app.js +++ b/src/app.js @@ -4,8 +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 {Client as Connection} from 'ldapts'; import {config} from './config'; import logger from './logger'; import {Client, Authenticator, Mapping} from './ldap'; @@ -13,21 +12,14 @@ import {Healthz, UserAuthentication, TokenAuthentication} from './api'; // setup basic dependencies let ldapClient = new Client( - ldap.createClient({ + new Connection({ url: config.ldap.uri, timeout: config.ldap.timeout * 1000, connectTimeout: config.ldap.timeout * 1000, - reconnect: { - initialDelay: config.ldap.reconnectInitialDelay, - maxDelay: config.ldap.reconnectMaxDelay, - failAfter: config.ldap.reconnectFailAfter, - }, - log: bunyanWinstonAdapter.createAdapter(logger), }), config.ldap.baseDn, config.ldap.bindDn, 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 295af37..99db7d2 100644 --- a/src/config.js +++ b/src/config.js @@ -31,10 +31,6 @@ const getConfig = () => { baseDn: process.env.LDAP_BASEDN || 'dc=example,dc=com', filter: process.env.LDAP_FILTER || '(uid=%s)', timeout: parseInt(process.env.LDAP_TIMEOUT) || 0, - 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 49449ed..5409a2b 100644 --- a/src/ldap/client.js +++ b/src/ldap/client.js @@ -5,8 +5,6 @@ export default class Client { basedn: string; binddn: string; bindpw: string; - startTls: boolean; - _secure: boolean; /** * Create an LDAP client. @@ -14,22 +12,9 @@ export default class Client { * @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 {boolean} startTls - Whether to use StartTLS on the connection or not. */ - constructor(conn: Object, basedn: string, binddn: string, bindpw: string, startTls: boolean) { + constructor(conn: Object, basedn: string, binddn: string, bindpw: string) { this.client = conn; - this.startTls = startTls; - this._secure = false; - 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; @@ -41,19 +26,17 @@ export default class Client { * @param {string} password - Password to bind. * @return {Promise} */ - bind(dn: string, password: string): Promise { - return new Promise((resolve, reject) => { - if (this.startTls && !this._secure) { - reject(new Error('ldap connection not tls protected')); - } - this.client.bind(dn, password, [], (err, res) => { - if (err) { - resolve(false); - } else { - resolve(true); - } - }); - }); + async bind(dn: string, password: string): Promise { + let authenticated = false; + try { + await this.client.bind(dn, password, []); + authenticated = true; + } catch (error) { + authenticated = false; + } finally { + await this.client.unbind(); + } + return authenticated; } /** @@ -63,7 +46,7 @@ export default class Client { * @param {string} basedn - The base DN to use (optional). * @return {Promise} Promise fulfilled with search result */ - search( + async search( filter: string, attributes: ?Array, basedn: ?string @@ -72,46 +55,24 @@ export default class Client { basedn = this.basedn; } - return new Promise((resolve, reject) => { - if (this.startTls && !this._secure) { - reject(new Error('ldap connection not tls protected')); - } - let that = this; - this.client.bind(this.binddn, this.bindpw, [], (err, res) => { - if (err) { - reject(err); - } 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}]`)); - } - } - }); - } - }); - } - }); - }); + let searchResult = null; + try { + await this.client.bind(this.binddn, this.bindpw, []); + const options = { + filter: filter, + scope: 'sub', + attributes: attributes, + }; + searchResult = await this.client.search(basedn, options, []); + } catch (error) { + throw error; + } finally { + await this.client.unbind(); + } + if (searchResult && searchResult.searchEntries.length > 0) { + return searchResult.searchEntries[0]; + } else { + throw new Error(`no object found with filter [${filter}]`); + } } } diff --git a/src/ldap/mapping.js b/src/ldap/mapping.js index 7474f9d..35f762f 100644 --- a/src/ldap/mapping.js +++ b/src/ldap/mapping.js @@ -66,7 +66,7 @@ class Mapping { if (groups instanceof Array) { return groups; } else { - return [groups]; + return groups ? [groups] : []; } } } diff --git a/test/integration/app.test.js b/test/integration/app.test.js index 2bbe242..10365c7 100644 --- a/test/integration/app.test.js +++ b/test/integration/app.test.js @@ -1,5 +1,4 @@ -import ldap from 'ldapjs'; -jest.mock('ldapjs'); +import {Client as Connection} from 'ldapts'; import request from 'supertest'; import jwt from 'jsonwebtoken'; import {config} from '../../src/config'; @@ -30,16 +29,12 @@ const fixtures = { }, }; -let connection = ldap.createClient(); +let connection = new Connection(); describe('test app routes', () => { beforeEach(() => { - connection.starttlsReturnsError = false; connection.bindReturnsError = false; connection.searchReturnsError = false; - connection.searchEmitsError = false; - connection.searchEmitsEnd = false; - connection.searchEmitsEndStatus = 0; connection.searchResult = { uid: fixtures.ldap.username, memberOf: fixtures.ldap.memberOf, @@ -104,16 +99,6 @@ describe('test app routes', () => { done(error); }); }); - test('GET /auth: 500 Internal Server Error', (done) => { - connection.searchResult.memberOf = null; - request(app) - .get('/auth') - .auth('john.doe', 'secret') - .expect(500) - .end((error) => { - done(error); - }); - }); test('POST /token: 200 OK', (done) => { let tokenReview = fixtures.tokenReviewTemplate; tokenReview.spec.token = jwt.sign(fixtures.tokenPayload, config.jwt.key); diff --git a/test/unit/ldap/client.test.js b/test/unit/ldap/client.test.js index 3e8555b..7e8a0f6 100644 --- a/test/unit/ldap/client.test.js +++ b/test/unit/ldap/client.test.js @@ -1,6 +1,5 @@ import Client from '../../../src/ldap/client'; -import ldap from 'ldapjs'; -jest.mock('ldapjs'); +import {Client as Connection} from 'ldapts'; const fixtures = { basedn: 'dc=example,dc=com', @@ -19,7 +18,7 @@ const fixtures = { ], }; -let connection = ldap.createClient(); +let connection = new Connection(); let client = null; beforeEach(() => { @@ -58,17 +57,6 @@ 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()', () => { @@ -90,17 +78,8 @@ describe('Client.search()', () => { ).rejects.toEqual(new Error('error by mock')); }); - test('Rejects on error event', () => { - connection.searchEmitsError = true; - - expect.hasAssertions(); - return expect( - client.search(fixtures.filter) - ).rejects.toEqual(new Error('error by mock')); - }); - test('Rejects on empty result', () => { - connection.searchEmitsResult = false; + connection.searchReturnsResult = false; expect.hasAssertions(); return expect( @@ -109,25 +88,4 @@ describe('Client.search()', () => { `no object found with filter [${fixtures.filter}]` )); }); - - test('Rejects on search end with status != 0', () => { - connection.searchEmitsEnd = true; - connection.searchEmitsEndStatus = 1; - - expect.hasAssertions(); - return expect( - 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' - )); - }); }); diff --git a/yarn.lock b/yarn.lock index 050be1a..debe682 100644 --- a/yarn.lock +++ b/yarn.lock @@ -252,13 +252,16 @@ arrify@^1.0.0, arrify@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/arrify/-/arrify-1.0.1.tgz#898508da2226f380df904728456849c1501a4b0d" -asn1@0.2.3, asn1@~0.2.3: +asn1@~0.2.3: version "0.2.3" resolved "https://registry.yarnpkg.com/asn1/-/asn1-0.2.3.tgz#dac8787713c9966849fc8180777ebe9c1ddf3b86" -assert-plus@0.1.5: - version "0.1.5" - resolved "https://registry.yarnpkg.com/assert-plus/-/assert-plus-0.1.5.tgz#ee74009413002d84cec7219c6ac811812e723160" +asn1@~0.2.4: + version "0.2.4" + resolved "https://registry.yarnpkg.com/asn1/-/asn1-0.2.4.tgz#8d2475dfab553bb33e77b54e59e880bb8ce23136" + integrity sha512-jxwzQpLQjSmWXgwaCZE9Nz+glAG01yF1QnWgbhGwHI5A6FRIEY6IVqtHhIepHqI7/kyEyQEagBC5mBEFlIYvdg== + dependencies: + safer-buffer "~2.1.0" assert-plus@1.0.0, assert-plus@^1.0.0: version "1.0.0" @@ -883,12 +886,6 @@ babylon@^6.18.0: version "6.18.0" resolved "https://registry.yarnpkg.com/babylon/-/babylon-6.18.0.tgz#af2f3b88fa6f5c1e4c634d1a0f8eac4f55b395e3" -backoff@^2.5.0: - version "2.5.0" - resolved "https://registry.yarnpkg.com/backoff/-/backoff-2.5.0.tgz#f616eda9d3e4b66b8ca7fca79f695722c5f8e26f" - dependencies: - precond "0.2" - balanced-match@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-1.0.0.tgz#89b4d199ab2bee49de164ea02b89ce462d71b767" @@ -1031,14 +1028,10 @@ builtin-modules@^1.0.0: version "1.1.1" resolved "https://registry.yarnpkg.com/builtin-modules/-/builtin-modules-1.1.1.tgz#270f076c5a72c02f5b65a47df94c5fe3a278892f" -bunyan@^1.8.3: - version "1.8.12" - resolved "https://registry.yarnpkg.com/bunyan/-/bunyan-1.8.12.tgz#f150f0f6748abdd72aeae84f04403be2ef113797" - optionalDependencies: - dtrace-provider "~0.8" - moment "^2.10.6" - mv "~2" - safe-json-stringify "~1" +bunyan-winston-adapter@^0.2.0: + version "0.2.0" + resolved "https://registry.yarnpkg.com/bunyan-winston-adapter/-/bunyan-winston-adapter-0.2.0.tgz#950310633405ab5e65fdb7fee76f18fe6a998fcd" + integrity sha1-lQMQYzQFq15l/bf+528Y/mqZj80= bytes@3.0.0: version "3.0.0" @@ -1330,7 +1323,7 @@ cycle@1.0.x: version "1.0.3" resolved "https://registry.yarnpkg.com/cycle/-/cycle-1.0.3.tgz#21e80b2be8580f98b468f379430662b046c34ad2" -dashdash@^1.12.0, dashdash@^1.14.0: +dashdash@^1.12.0: version "1.14.1" resolved "https://registry.yarnpkg.com/dashdash/-/dashdash-1.14.1.tgz#853cfa0f7cbe2fed5de20326b8dd581035f6e2f0" dependencies: @@ -1348,6 +1341,13 @@ debug@^3.1.0: dependencies: ms "2.0.0" +debug@~4.1.1: + version "4.1.1" + resolved "https://registry.yarnpkg.com/debug/-/debug-4.1.1.tgz#3b72260255109c6b589cee050f1d516139664791" + integrity sha512-pYAIzeRo8J6KPEaJ0VWOh5Pzkbw/RetuzehGM7QRRX5he4fPHx2rdKMB256ehJCkX+XRQm16eZLqLNS8RSZXZw== + dependencies: + ms "^2.1.1" + decamelize@^1.0.0, decamelize@^1.1.1: version "1.2.0" resolved "https://registry.yarnpkg.com/decamelize/-/decamelize-1.2.0.tgz#f6534d15148269b20352e7bee26f501f9a191290" @@ -1458,12 +1458,6 @@ domexception@^1.0.0: dependencies: webidl-conversions "^4.0.2" -dtrace-provider@~0.8: - version "0.8.6" - resolved "https://registry.yarnpkg.com/dtrace-provider/-/dtrace-provider-0.8.6.tgz#428a223afe03425d2cd6d6347fdf40c66903563d" - dependencies: - nan "^2.3.3" - ecc-jsbn@~0.1.1: version "0.1.1" resolved "https://registry.yarnpkg.com/ecc-jsbn/-/ecc-jsbn-0.1.1.tgz#0fc73a9ed5f0d53c38193398523ef7e543777505" @@ -1771,10 +1765,6 @@ extglob@^2.0.4: snapdragon "^0.8.1" to-regex "^3.0.1" -extsprintf@1.2.0: - version "1.2.0" - resolved "https://registry.yarnpkg.com/extsprintf/-/extsprintf-1.2.0.tgz#5ad946c22f5b32ba7f8cd7426711c6e8a3fc2529" - extsprintf@1.3.0: version "1.3.0" resolved "https://registry.yarnpkg.com/extsprintf/-/extsprintf-1.3.0.tgz#96918440e3041a7a414f8c52e3c574eb3c3e1e05" @@ -2022,16 +2012,6 @@ glob-parent@^2.0.0: dependencies: is-glob "^2.0.0" -glob@^6.0.1: - version "6.0.4" - resolved "https://registry.yarnpkg.com/glob/-/glob-6.0.4.tgz#0f08860f6a155127b2fadd4f9ce24b1aab6e4d22" - dependencies: - inflight "^1.0.4" - inherits "2" - minimatch "2 || 3" - once "^1.3.0" - path-is-absolute "^1.0.0" - glob@^7.0.3, glob@^7.0.5, glob@^7.1.1, glob@^7.1.2: version "7.1.2" resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.2.tgz#c19c9df9a028702d678612384a6552404c636d15" @@ -3013,27 +2993,14 @@ lcov-parse@^0.0.10: version "0.0.10" resolved "https://registry.yarnpkg.com/lcov-parse/-/lcov-parse-0.0.10.tgz#1b0b8ff9ac9c7889250582b70b71315d9da6d9a3" -ldap-filter@0.2.2: - version "0.2.2" - resolved "https://registry.yarnpkg.com/ldap-filter/-/ldap-filter-0.2.2.tgz#f2b842be0b86da3352798505b31ebcae590d77d0" - dependencies: - assert-plus "0.1.5" - -ldapjs@^1.0.2: - version "1.0.2" - resolved "https://registry.yarnpkg.com/ldapjs/-/ldapjs-1.0.2.tgz#544ff7032b7b83c68f0701328d9297aa694340f9" +ldapts@^1.7.0: + version "1.7.0" + resolved "https://registry.yarnpkg.com/ldapts/-/ldapts-1.7.0.tgz#e036a7d0bac32c82782a4fc4e24ccd347ecee372" + integrity sha512-V1gK0CAWs7DXkqLsIR8YDCCKKCjwiEw4haEd1ls9KCOUGwtojq51mnu4GKwaDlnPIsmekWhLG9krY1jIqerG7A== dependencies: - asn1 "0.2.3" - assert-plus "^1.0.0" - backoff "^2.5.0" - bunyan "^1.8.3" - dashdash "^1.14.0" - ldap-filter "0.2.2" - once "^1.4.0" - vasync "^1.6.4" - verror "^1.8.1" - optionalDependencies: - dtrace-provider "~0.8" + asn1 "~0.2.4" + debug "~4.1.1" + strict-event-emitter-types "~2.0.0" left-pad@^1.2.0: version "1.2.0" @@ -3226,7 +3193,7 @@ mimic-fn@^1.0.0: version "1.2.0" resolved "https://registry.yarnpkg.com/mimic-fn/-/mimic-fn-1.2.0.tgz#820c86a39334640e99516928bd03fca88057d022" -"minimatch@2 || 3", minimatch@^3.0.0, minimatch@^3.0.2, minimatch@^3.0.3, minimatch@^3.0.4: +minimatch@^3.0.0, minimatch@^3.0.2, minimatch@^3.0.3, minimatch@^3.0.4: version "3.0.4" resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-3.0.4.tgz#5166e286457f03306064be5497e8dbb0c3d32083" dependencies: @@ -3251,16 +3218,12 @@ mixin-deep@^1.2.0: for-in "^1.0.2" is-extendable "^1.0.1" -"mkdirp@>=0.5 0", mkdirp@^0.5.1, mkdirp@~0.5.1: +"mkdirp@>=0.5 0", mkdirp@^0.5.1: version "0.5.1" resolved "https://registry.yarnpkg.com/mkdirp/-/mkdirp-0.5.1.tgz#30057438eac6cf7f8c4767f38648d6697d75c903" dependencies: minimist "0.0.8" -moment@^2.10.6: - version "2.21.0" - resolved "https://registry.yarnpkg.com/moment/-/moment-2.21.0.tgz#2a114b51d2a6ec9e6d83cf803f838a878d8a023a" - morgan@^1.9.0: version "1.9.0" resolved "https://registry.yarnpkg.com/morgan/-/morgan-1.9.0.tgz#d01fa6c65859b76fcf31b3cb53a3821a311d8051" @@ -3283,15 +3246,7 @@ mute-stream@0.0.7: version "0.0.7" resolved "https://registry.yarnpkg.com/mute-stream/-/mute-stream-0.0.7.tgz#3075ce93bc21b8fab43e1bc4da7e8115ed1e7bab" -mv@~2: - version "2.1.1" - resolved "https://registry.yarnpkg.com/mv/-/mv-2.1.1.tgz#ae6ce0d6f6d5e0a4f7d893798d03c1ea9559b6a2" - dependencies: - mkdirp "~0.5.1" - ncp "~2.0.0" - rimraf "~2.4.0" - -nan@^2.3.0, nan@^2.3.3: +nan@^2.3.0: version "2.10.0" resolved "https://registry.yarnpkg.com/nan/-/nan-2.10.0.tgz#96d0cd610ebd58d4b4de9cc0c6828cda99c7548f" @@ -3316,10 +3271,6 @@ natural-compare@^1.4.0: version "1.4.0" resolved "https://registry.yarnpkg.com/natural-compare/-/natural-compare-1.4.0.tgz#4abebfeed7541f2c27acfb29bdbbd15c8d5ba4f7" -ncp@~2.0.0: - version "2.0.0" - resolved "https://registry.yarnpkg.com/ncp/-/ncp-2.0.0.tgz#195a21d6c46e361d2fb1281ba38b91e9df7bdbb3" - negotiator@0.6.1: version "0.6.1" resolved "https://registry.yarnpkg.com/negotiator/-/negotiator-0.6.1.tgz#2b327184e8992101177b28563fb5e7102acd0ca9" @@ -3640,10 +3591,6 @@ posix-character-classes@^0.1.0: version "0.1.1" resolved "https://registry.yarnpkg.com/posix-character-classes/-/posix-character-classes-0.1.1.tgz#01eac0fe3b5af71a2a6c02feabb8c1fef7e00eab" -precond@0.2: - version "0.2.3" - resolved "https://registry.yarnpkg.com/precond/-/precond-0.2.3.tgz#aa9591bcaa24923f1e0f4849d240f47efc1075ac" - prelude-ls@~1.1.2: version "1.1.2" resolved "https://registry.yarnpkg.com/prelude-ls/-/prelude-ls-1.1.2.tgz#21932a549f5e52ffd9a827f570e04be62a97da54" @@ -4007,12 +3954,6 @@ rimraf@2, rimraf@^2.2.8, rimraf@^2.5.1, rimraf@^2.5.4, rimraf@^2.6.1: dependencies: glob "^7.0.5" -rimraf@~2.4.0: - version "2.4.5" - resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-2.4.5.tgz#ee710ce5d93a8fdb856fb5ea8ff0e2d75934b2da" - dependencies: - glob "^6.0.1" - run-async@^2.2.0: version "2.3.0" resolved "https://registry.yarnpkg.com/run-async/-/run-async-2.3.0.tgz#0371ab4ae0bdd720d4166d7dfda64ff7a445a6c0" @@ -4033,16 +3974,17 @@ safe-buffer@5.1.1, safe-buffer@^5.0.1, safe-buffer@^5.1.1, safe-buffer@~5.1.0, s version "5.1.1" resolved "https://registry.yarnpkg.com/safe-buffer/-/safe-buffer-5.1.1.tgz#893312af69b2123def71f57889001671eeb2c853" -safe-json-stringify@~1: - version "1.1.0" - resolved "https://registry.yarnpkg.com/safe-json-stringify/-/safe-json-stringify-1.1.0.tgz#bd2b6dad1ebafab3c24672a395527f01804b7e19" - safe-regex@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/safe-regex/-/safe-regex-1.1.0.tgz#40a3669f3b077d1e943d44629e157dd48023bf2e" dependencies: ret "~0.1.10" +safer-buffer@~2.1.0: + version "2.1.2" + resolved "https://registry.yarnpkg.com/safer-buffer/-/safer-buffer-2.1.2.tgz#44fa161b0187b9549dd84bb91802f9bd8385cd6a" + integrity sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg== + sane@^2.0.0: version "2.5.0" resolved "https://registry.yarnpkg.com/sane/-/sane-2.5.0.tgz#6359cd676f5efd9988b264d8ce3b827dd6b27bec" @@ -4302,6 +4244,11 @@ stealthy-require@^1.1.0: version "1.1.1" resolved "https://registry.yarnpkg.com/stealthy-require/-/stealthy-require-1.1.1.tgz#35b09875b4ff49f26a777e509b3090a3226bf24b" +strict-event-emitter-types@~2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/strict-event-emitter-types/-/strict-event-emitter-types-2.0.0.tgz#05e15549cb4da1694478a53543e4e2f4abcf277f" + integrity sha512-Nk/brWYpD85WlOgzw5h173aci0Teyv8YdIAEtV+N88nDB0dLlazZyJMIsN6eo1/AR61l+p6CJTG1JIyFaoNEEA== + string-length@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/string-length/-/string-length-2.0.0.tgz#d40dbb686a3ace960c1cffca562bf2c45f8363ed" @@ -4636,13 +4583,7 @@ vary@^1, vary@~1.1.2: version "1.1.2" resolved "https://registry.yarnpkg.com/vary/-/vary-1.1.2.tgz#2299f02c6ded30d4a5961b0b9f74524a18f634fc" -vasync@^1.6.4: - version "1.6.4" - resolved "https://registry.yarnpkg.com/vasync/-/vasync-1.6.4.tgz#dfe93616ad0e7ae801b332a9d88bfc5cdc8e1d1f" - dependencies: - verror "1.6.0" - -verror@1.10.0, verror@^1.8.1: +verror@1.10.0: version "1.10.0" resolved "https://registry.yarnpkg.com/verror/-/verror-1.10.0.tgz#3a105ca17053af55d6e270c1f8288682e18da400" dependencies: @@ -4650,12 +4591,6 @@ verror@1.10.0, verror@^1.8.1: core-util-is "1.0.2" extsprintf "^1.2.0" -verror@1.6.0: - version "1.6.0" - resolved "https://registry.yarnpkg.com/verror/-/verror-1.6.0.tgz#7d13b27b1facc2e2da90405eb5ea6e5bdd252ea5" - dependencies: - extsprintf "1.2.0" - w3c-hr-time@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/w3c-hr-time/-/w3c-hr-time-1.0.1.tgz#82ac2bff63d950ea9e3189a58a65625fedf19045" From bede70bf0ff3f0e3a9b948714bf7aa703bec3434 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Fri, 31 May 2019 13:38:47 +0200 Subject: [PATCH 04/16] remove ldap starttls and reconnect options --- README.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/README.md b/README.md index 130565b..33304c8 100644 --- a/README.md +++ b/README.md @@ -150,10 +150,6 @@ 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`|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.ldap.startTls`|Whether to use StartTLS for the LDAP connection or not|`LDAP_STARTTLS`|true| |`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| |`config.mapping.groups`|Name of ldap attribute to be used for groups in kubernetes TokenReview|`MAPPING_GROUPS`|memberOf| From 1cf1b8693b413e6bd1704137e8e8feb37c9ca3cc Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Mon, 3 Jun 2019 09:16:57 +0200 Subject: [PATCH 05/16] resolve groups and extra attributes during token authentication (fixes #24) --- src/api/tokenAuthentication.js | 68 +++++++++++----- src/api/userAuthentication.js | 30 +++----- src/app.js | 9 ++- src/ldap/mapping.js | 30 +++++--- test/integration/app.test.js | 18 +++-- test/unit/api/tokenAuthentication.test.js | 94 ++++++++++++++++++----- test/unit/api/userAuthentication.test.js | 57 ++++---------- 7 files changed, 185 insertions(+), 121 deletions(-) diff --git a/src/api/tokenAuthentication.js b/src/api/tokenAuthentication.js index f772223..b0c2bb8 100644 --- a/src/api/tokenAuthentication.js +++ b/src/api/tokenAuthentication.js @@ -1,9 +1,12 @@ // @flow import {Logger} from 'winston'; import jwt from 'jsonwebtoken'; +import {Authenticator, Mapping} from '../ldap'; /** Class for TokenAuthentication API route */ export default class TokenAuthentication { + authenticator: Authenticator; + mapping: Mapping; key: string; logger: Logger; run: (Object, Object) => void @@ -11,10 +14,19 @@ export default class TokenAuthentication { /** * Create API route + * @param {Authenticator} authenticator - Authenticator. + * @param {Mapping} mapping - Attribute mapping (kubernetes<=>ldap). * @param {string} key - Private key. * @param {Logger} logger - Logger to use. */ - constructor(key: string, logger: Logger) { + constructor( + authenticator: Authenticator, + mapping: Mapping, + key: string, + logger: Logger + ) { + this.authenticator = authenticator; + this.mapping = mapping; this.key = key; this.logger = logger; this.run = this.run.bind(this); @@ -26,7 +38,7 @@ export default class TokenAuthentication { * @param {Object} req - Request. * @param {Object} res - Response. */ - run(req: Object, res: Object) { + async run(req: Object, res: Object): Promise { if ( !req.body.apiVersion || !req.body.kind || @@ -40,30 +52,48 @@ export default class TokenAuthentication { ) { res.sendStatus(400); } else { - let responseData = { - apiVersion: 'authentication.k8s.io/v1beta1', - kind: 'TokenReview', - status: { - authenticated: false, - user: {}, - }, - }; - let token =req.body.spec.token; try { - let data = this.extractAndVerifyToken(token); - responseData.status.user = data; - responseData.status.authenticated = true; + let responseData = await this._processToken(token); + res.send(responseData); } catch (error) { - delete responseData.status.user; - responseData.status.authenticated = false; - this.logger.info('Error while verifying token: ' + - `[${error.name}] with message [${error.message}]`); + this.logger.error(error); + res.sendStatus(500); } - res.send(responseData); } } + /** + * Process token + * @param {string} token - The token. + * @return {Object} + */ + async _processToken(token: string): Promise { + let responseData = { + apiVersion: 'authentication.k8s.io/v1beta1', + kind: 'TokenReview', + status: { + authenticated: false, + user: {}, + }, + }; + try { + let tokenData = this.extractAndVerifyToken(token); + let ldapObject = await this.authenticator.getAttributes( + tokenData.username, + this.mapping.getLdapAttributes() + ); + responseData.status.user = this.mapping.ldapToKubernetes(ldapObject); + responseData.status.authenticated = true; + } catch (error) { + delete responseData.status.user; + responseData.status.authenticated = false; + this.logger.info('Error while verifying token: ' + + `[${error.name}] with message [${error.message}]`); + } + return responseData; + } + /** * Validate token * @param {string} token - The token. diff --git a/src/api/userAuthentication.js b/src/api/userAuthentication.js index 070ef37..e258031 100644 --- a/src/api/userAuthentication.js +++ b/src/api/userAuthentication.js @@ -2,14 +2,13 @@ import {Logger} from 'winston'; import atob from 'atob'; import jwt from 'jsonwebtoken'; -import {Authenticator, Mapping} from '../ldap'; +import {Authenticator} from '../ldap'; /** Class for UserAuthentication API route */ export default class UserAuthentication { authenticator: Authenticator; tokenLifetime: number; key: string; - mapping: Mapping; logger: Logger; run: (Object, Object) => void @@ -18,20 +17,17 @@ export default class UserAuthentication { * @param {Authenticator} authenticator - Authenticator. * @param {number} tokenLifetime - Token lifetime. * @param {string} key - Private key for token signing. - * @param {Mapping} mapping - Attribute mapping (kubernetes<=>ldap). * @param {Logger} logger - Logger to use. */ constructor( authenticator: Authenticator, tokenLifetime: number, key: string, - mapping: Mapping, logger: Logger ) { this.authenticator = authenticator; this.tokenLifetime = tokenLifetime; this.key = key; - this.mapping = mapping; this.logger = logger; this.run = this.run.bind(this); } @@ -56,19 +52,19 @@ export default class UserAuthentication { if (!success) { this._sendUnauthorized(res); } else { - return this.getToken(credentials.username).then((token) => { - res.send(token); - }, (error) => { - this.logger.error(error); + try { + res.send(this.getToken(credentials.username)); + } catch (error) { + this.logger.error(error.message); res.sendStatus(500); - }); + } } }, (error) => { - this.logger.info(error); + this.logger.error(error.message); res.sendStatus(500); }); } catch (error) { - this.logger.info(error.message); + this.logger.error(error.message); res.sendStatus(400); } } @@ -88,15 +84,9 @@ export default class UserAuthentication { * @param {string} username - Username. * @return {string} */ - async getToken(username: string): Promise { + getToken(username: string): Promise { try { - let user = await this.authenticator.getAttributes( - username, - this.mapping.getLdapAttributes() - ); - - let data = this.mapping.ldapToKubernetes(user); - let token = jwt.sign(data, this.key, { + let token = jwt.sign({username: username}, this.key, { expiresIn: this.tokenLifetime, }); diff --git a/src/app.js b/src/app.js index fe7e330..bbc42a7 100644 --- a/src/app.js +++ b/src/app.js @@ -29,14 +29,19 @@ let userAuthentication = new UserAuthentication( authenticator, config.jwt.tokenLifetime, config.jwt.key, + logger, +); +let tokenAuthentication = new TokenAuthentication( + authenticator, new Mapping( config.mapping.username, config.mapping.uid, config.mapping.groups, config.mapping.extraFields, ), - logger); -let tokenAuthentication = new TokenAuthentication(config.jwt.key, logger); + config.jwt.key, + logger +); // setup express const app = express(); diff --git a/src/ldap/mapping.js b/src/ldap/mapping.js index 35f762f..c11ea41 100644 --- a/src/ldap/mapping.js +++ b/src/ldap/mapping.js @@ -39,21 +39,29 @@ class Mapping { /** * Convert ldap object to kubernetes * @param {Object} ldapObject - Ldap object to convert - * @return {Ôbject} + * @param {boolean} withGroupAndExtra - Include groups and extra attributes + * @return {Object} */ - ldapToKubernetes(ldapObject: Object): Object { - let object = { + ldapToKubernetes(ldapObject: Object, withGroupAndExtra: boolean = true): Object { + let groupAndExtra = {}; + if (withGroupAndExtra) { + groupAndExtra = { + groups: this.getGroups(ldapObject).map((group) => { + return canonicalizeDn(group); + }), + extra: this.extraFields.reduce((object, field) => { + return { + ...object, + [field]: ldapObject[field], + }; + }, {}), + }; + } + return { username: ldapObject[this.username], uid: ldapObject[this.uid], - groups: this.getGroups(ldapObject).map((group) => { - return canonicalizeDn(group); - }), - extra: {}, + ...groupAndExtra, }; - for (let extraField of this.extraFields) { - object.extra[extraField] = ldapObject[extraField]; - } - return object; } /** diff --git a/test/integration/app.test.js b/test/integration/app.test.js index 10365c7..6534990 100644 --- a/test/integration/app.test.js +++ b/test/integration/app.test.js @@ -11,14 +11,18 @@ const fixtures = { 'cn=test,dc=example,dc=com', ], }, - tokenPayload: { + tokenReviewPayload: { + username: 'john.doe', + iat: Math.floor(Date.now() / 1000), + exp: Math.floor(Date.now() / 1000 + 3600), + }, + tokenReviewResponse: { username: 'john.doe', uid: 'john.doe', groups: [ 'test', ], - iat: Math.floor(Date.now() / 1000), - exp: Math.floor(Date.now() / 1000 + 3600), + extra: {}, }, tokenReviewTemplate: { apiVersion: 'authentication.k8s.io/v1beta1', @@ -64,9 +68,7 @@ describe('test app routes', () => { }) .expect((response) => { let object = jwt.decode(response.text); - expect(object.username).toBe(fixtures.tokenPayload.username); - expect(object.uid).toBe(fixtures.tokenPayload.uid); - expect(object.groups).toEqual(fixtures.tokenPayload.groups); + expect(object.username).toBe(fixtures.tokenReviewPayload.username); }) .end((error) => { done(error); @@ -101,7 +103,7 @@ describe('test app routes', () => { }); test('POST /token: 200 OK', (done) => { let tokenReview = fixtures.tokenReviewTemplate; - tokenReview.spec.token = jwt.sign(fixtures.tokenPayload, config.jwt.key); + tokenReview.spec.token = jwt.sign(fixtures.tokenReviewPayload, config.jwt.key); request(app) .post('/token') .send(tokenReview) @@ -111,7 +113,7 @@ describe('test app routes', () => { expect(object.apiVersion).toBe(fixtures.tokenReviewTemplate.apiVersion); expect(object.kind).toBe(fixtures.tokenReviewTemplate.kind); expect(object.status.authenticated).toBe(true); - expect(object.status.user).toEqual(fixtures.tokenPayload); + expect(object.status.user).toEqual(fixtures.tokenReviewResponse); }) .end((error) => { done(error); diff --git a/test/unit/api/tokenAuthentication.test.js b/test/unit/api/tokenAuthentication.test.js index 7c0f91b..8efa41a 100644 --- a/test/unit/api/tokenAuthentication.test.js +++ b/test/unit/api/tokenAuthentication.test.js @@ -1,10 +1,11 @@ -import winston from 'winston'; + import winston from 'winston'; import {getResponseMock, getRequestMock} from '../mock'; import TokenAuthentication from '../../../src/api/tokenAuthentication'; +import {Authenticator, Mapping} from '../../../src/ldap'; +jest.mock('../../../src/ldap/authenticator'); -const key = 'testsecret'; -const tokenAuthentication = new TokenAuthentication(key, winston); const fixtures = { + key: 'testsecret', validToken: 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.ajB221IVz7aggEfTp3jUPc7UBw5xemmp-LmrmEgFETU', validTokenPayload: { sub: '1234567890', @@ -35,10 +36,38 @@ const fixtures = { user: {}, }, }, + mapping: new Mapping( + 'uid', + 'uid', + 'memberOf', + [ + 'uidNumber', + 'gidNumber', + ] + ), + validTokenResponse: { + username: 'john.doe', + groups: [ + 'test', + ], + extra: { + uidNumber: 1, + gidNumber: 10, + }, + uid: 'john.doe', + }, }; +let authenticator = new Authenticator(); +const tokenAuthentication = new TokenAuthentication( + authenticator, + fixtures.mapping, + fixtures.key, + winston +); + const generateRequestBody = (token) => { - let requestBody = fixtures.requestTemplate; + let requestBody = Object.assign({}, fixtures.requestTemplate); requestBody.spec.token = token; return requestBody; }; @@ -49,6 +78,19 @@ const generateAuthenticatedResponseBody = (payload) => { return responseBody; }; +beforeEach(() => { + authenticator.authenticated = true; + authenticator.attributes = { + uid: fixtures.validTokenResponse.username, + memberOf: fixtures.validTokenResponse.groups.map((group) => { + return 'cn=' + group + ',dc=example,dc=com'; + }), + uidNumber: fixtures.validTokenResponse.extra.uidNumber, + gidNumber: fixtures.validTokenResponse.extra.gidNumber, + }; + authenticator.getAttributesShouldThrowError = false; +}); + describe('TokenAuthentication.extractAndVerifyToken()', () => { test('Returns token content', () => { expect(tokenAuthentication.extractAndVerifyToken(fixtures.validToken)) @@ -75,11 +117,11 @@ describe('TokenAuthentication.run()', () => { ); const responseMock = getResponseMock(); - tokenAuthentication.run(requestMock, responseMock); - - expect(responseMock.send).toHaveBeenCalled(); - expect(responseMock.send.mock.calls[0][0]) - .toEqual(generateAuthenticatedResponseBody(fixtures.validTokenPayload)); + return tokenAuthentication.run(requestMock, responseMock).then(() => { + expect(responseMock.send).toHaveBeenCalled(); + expect(responseMock.send.mock.calls[0][0]) + .toEqual(generateAuthenticatedResponseBody(fixtures.validTokenResponse)); + }); }); test('Sends "not-authenticated" response on invalid token', () => { @@ -88,11 +130,11 @@ describe('TokenAuthentication.run()', () => { ); const responseMock = getResponseMock(); - tokenAuthentication.run(requestMock, responseMock); - - expect(responseMock.send).toHaveBeenCalled(); - expect(responseMock.send.mock.calls[0][0]) - .toEqual(fixtures.notAuthenticatedResponse); + return tokenAuthentication.run(requestMock, responseMock).then(() => { + expect(responseMock.send).toHaveBeenCalled(); + expect(responseMock.send.mock.calls[0][0]) + .toEqual(fixtures.notAuthenticatedResponse); + }); }); test('Sends "not-authenticated" response on invalid token', () => { @@ -101,11 +143,11 @@ describe('TokenAuthentication.run()', () => { ); const responseMock = getResponseMock(); - tokenAuthentication.run(requestMock, responseMock); - - expect(responseMock.send).toHaveBeenCalled(); - expect(responseMock.send.mock.calls[0][0]) - .toEqual(fixtures.notAuthenticatedResponse); + return tokenAuthentication.run(requestMock, responseMock).then(() => { + expect(responseMock.send).toHaveBeenCalled(); + expect(responseMock.send.mock.calls[0][0]) + .toEqual(fixtures.notAuthenticatedResponse); + }); }); test('Sends 400 response on invalid request', () => { @@ -131,4 +173,18 @@ describe('TokenAuthentication.run()', () => { expect(responseMock.sendStatus.mock.calls[0][0]) .toEqual(400); }); + + test('Sends "not-authenticated" on internal server error (e.g. ldap error)', () => { + authenticator.getAttributesShouldThrowError = true; + const requestMock = getRequestMock( + generateRequestBody(fixtures.validToken) + ); + const responseMock = getResponseMock(); + + return tokenAuthentication.run(requestMock, responseMock).then(() => { + expect(responseMock.send).toHaveBeenCalled(); + expect(responseMock.send.mock.calls[0][0]) + .toEqual(fixtures.notAuthenticatedResponse); + }); + }); }); diff --git a/test/unit/api/userAuthentication.test.js b/test/unit/api/userAuthentication.test.js index 5d45bcc..ca3aa8c 100644 --- a/test/unit/api/userAuthentication.test.js +++ b/test/unit/api/userAuthentication.test.js @@ -2,7 +2,7 @@ import winston from 'winston'; import {getResponseMock, getRequestMock} from '../mock'; import jwt from 'jsonwebtoken'; import UserAuthentication from '../../../src/api/userAuthentication'; -import {Authenticator, Mapping} from '../../../src/ldap'; +import {Authenticator} from '../../../src/ldap'; jest.mock('../../../src/ldap/authenticator'); const fixtures = { @@ -11,22 +11,6 @@ const fixtures = { username: 'john.doe', password: 'secret', authHeader: 'Basic am9obi5kb2U6c2VjcmV0', - groups: [ - 'test', - ], - extraFields: { - uidNumber: 1, - gidNumber: 10, - }, - mapping: new Mapping( - 'uid', - 'uid', - 'memberOf', - [ - 'uidNumber', - 'gidNumber', - ] - ), }; let authenticator = new Authenticator(); @@ -34,20 +18,10 @@ const userAuthentication = new UserAuthentication( authenticator, fixtures.lifetime, fixtures.key, - fixtures.mapping, winston); beforeEach(() => { authenticator.authenticated = true; - authenticator.attributes = { - uid: fixtures.username, - memberOf: fixtures.groups.map((group) => { - return 'cn=' + group + ',dc=example,dc=com'; - }), - uidNumber: fixtures.extraFields.uidNumber, - gidNumber: fixtures.extraFields.gidNumber, - }; - authenticator.getAttributesShouldThrowError = false; }); describe('UserAuthentication.run()', () => { @@ -64,8 +38,6 @@ describe('UserAuthentication.run()', () => { let token = jwt.decode(responseMock.send.mock.calls[0][0], {complete: true}); expect(token.header.typ).toBe('JWT'); expect(token.payload.username).toBe(fixtures.username); - expect(token.payload.uid).toBe(fixtures.username); - expect(token.payload.groups).toEqual(fixtures.groups); }); }); @@ -123,15 +95,20 @@ describe('UserAuthentication.run()', () => { .toEqual(400); }); - test('Sends 500 on internal server error (e.g. ldap error)', () => { - authenticator.getAttributesShouldThrowError = true; + test('Sends 500 on invalid key', () => { const requestMock = getRequestMock( '', {'Authorization': fixtures.authHeader} ); const responseMock = getResponseMock(); - return userAuthentication.run(requestMock, responseMock).then(() => { + const failingUserAuthentication = new UserAuthentication( + authenticator, + fixtures.lifetime, + undefined, + winston); + + failingUserAuthentication.run(requestMock, responseMock).then(() => { expect(responseMock.sendStatus).toHaveBeenCalled(); expect(responseMock.sendStatus.mock.calls[0][0]) .toEqual(500); @@ -144,16 +121,12 @@ describe('UserAuthentication.getToken()', () => { let now = Math.floor(new Date() / 1000); expect.hasAssertions(); - return userAuthentication.getToken(fixtures.username).then((result) => { - let token = jwt.decode(result, {complete: true}); - expect(token.header.typ).toBe('JWT'); - expect(token.payload.username).toBe(fixtures.username); - expect(token.payload.uid).toBe(fixtures.username); - expect(token.payload.groups).toEqual(fixtures.groups); - expect(token.payload.extra).toEqual(fixtures.extraFields); - expect(token.payload.exp / 60).toBeCloseTo((now + fixtures.lifetime) / 60); - expect(jwt.verify(result, fixtures.key)); - }); + let result = userAuthentication.getToken(fixtures.username); + let token = jwt.decode(result, {complete: true}); + expect(token.header.typ).toBe('JWT'); + expect(token.payload.username).toBe(fixtures.username); + expect(token.payload.exp / 60).toBeCloseTo((now + fixtures.lifetime) / 60); + expect(jwt.verify(result, fixtures.key)); }); }); From f528eaa8d222dfb5b0abd44feb19a9d2a60a41ad Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Mon, 3 Jun 2019 09:19:47 +0200 Subject: [PATCH 06/16] mention new group resolution strategy --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5acfbfc..4eb97ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Changed +- *BREAKING*: Extra-Attributes and groups are now no longer included in the JWT issued after user authentication. Extra-Attributes and group memberships are now resolved during the token review and are included in the token review response - Internal: Use [ldapts](https://github.com/ldapts/ldapts) instead of [ldapjs](https://github.com/joyent/node-ldapjs) as ldap library ### Fixed From e742b7d1f30d465c70c0a5683dc72fd62a7b3990 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Mon, 3 Jun 2019 10:08:21 +0200 Subject: [PATCH 07/16] add overall error handler --- src/app.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/app.js b/src/app.js index bbc42a7..42ec98a 100644 --- a/src/app.js +++ b/src/app.js @@ -56,5 +56,9 @@ app.use(morgan('combined', { app.get('/healthz', healthz.run); app.get('/auth', userAuthentication.run); app.post('/token', bodyParser.json(), tokenAuthentication.run); +app.use((err, req, res, next) => { + logger.error(err); + res.sendStatus(500); +}); export default app; From a708bd7b1a68a3df1754b6fbb8e26cbf9910d98f Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Wed, 12 Jun 2019 14:33:50 +0200 Subject: [PATCH 08/16] add prometheus exporter --- CHANGELOG.md | 3 +++ README.md | 3 +++ package.json | 3 +++ src/app.js | 24 +++++++++++++++++++++++ src/config.js | 17 +++++++++++++++++ yarn.lock | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 102 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eb97ff..e9806ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ 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] +### Added +- Prometheus exporter on route "/metrics" (basic auth protected) + ### Changed - *BREAKING*: Extra-Attributes and groups are now no longer included in the JWT issued after user authentication. Extra-Attributes and group memberships are now resolved during the token review and are included in the token review response - Internal: Use [ldapts](https://github.com/ldapts/ldapts) instead of [ldapjs](https://github.com/joyent/node-ldapjs) as ldap library diff --git a/README.md b/README.md index 33304c8..138d16d 100644 --- a/README.md +++ b/README.md @@ -157,6 +157,9 @@ List of configurable values: |`config.mapping.username`|Name of Ldap attribute to be used as username in kubernetes TokenReview|`MAPPING_USERNAME`|uid| |`config.jwt.key`|Key for signing the JWT. **DO NOT USE THE DEFAULT VALUE IN PRODUCTION**|`JWT_KEY`|secret| |`config.jwt.tokenLifetime`|Seconds until token a expires|`JWT_TOKEN_LIFETIME`|28800| +|`config.prometheus.username`|Username for prometheus exporter basic auth (use empty string to disable basic auth)|`PROMETHEUS_USERNAME`|prometheus| +|`config.prometheus.password`|Password for prometheus exporter basic auth (use empty string to disable basic auth)|`PROMETHEUS_PASSWORD`|secret| +|`config.prometheus.nodejsProbeInterval`|Probe interval for nodejs metrics in milliseconds|`PROMETHEUS_NODEJS_PROBE_INTERVAL`|10000| ### kubernetes Configure your kubernetes apiserver to use the kube-ldap [webhook for authentication](https://kubernetes.io/docs/admin/authentication/#webhook-token-authentication) using the following configuration file. diff --git a/package.json b/package.json index 12f1805..5933c0c 100644 --- a/package.json +++ b/package.json @@ -12,9 +12,12 @@ "bunyan-winston-adapter": "^0.2.0", "cors": "^2.8.4", "express": "^4.16.3", + "express-basic-auth": "^1.2.0", + "express-prom-bundle": "^5.1.5", "jsonwebtoken": "^8.2.0", "ldapts": "^1.7.0", "morgan": "^1.9.0", + "prom-client": "^11.5.0", "winston": "^2.4.1" }, "devDependencies": { diff --git a/src/app.js b/src/app.js index 42ec98a..798f2df 100644 --- a/src/app.js +++ b/src/app.js @@ -4,6 +4,8 @@ import express from 'express'; import bodyParser from 'body-parser'; import cors from 'cors'; import morgan from 'morgan'; +import basicAuth from 'express-basic-auth'; +import prometheusBundle from 'express-prom-bundle'; import {Client as Connection} from 'ldapts'; import {config} from './config'; import logger from './logger'; @@ -43,6 +45,17 @@ let tokenAuthentication = new TokenAuthentication( logger ); +// setup prometheus exporter +let prometheusExporter = prometheusBundle({ + includeMethod: true, + includePath: true, + promClient: { + collectDefaultMetrics: { + timeout: config.prometheus.nodejsProbeInterval, + }, + }, +}); + // setup express const app = express(); app.use(cors()); @@ -53,6 +66,17 @@ app.use(morgan('combined', { }, }, })); +if ( + Boolean(config.prometheus.username) && + Boolean(config.prometheus.password) +) { + app.use('/metrics', basicAuth({ + users: { + [config.prometheus.username]: config.prometheus.password, + }, + })); +}; +app.use(prometheusExporter); app.get('/healthz', healthz.run); app.get('/auth', userAuthentication.run); app.post('/token', bodyParser.json(), tokenAuthentication.run); diff --git a/src/config.js b/src/config.js index 99db7d2..6cbb5f9 100644 --- a/src/config.js +++ b/src/config.js @@ -21,6 +21,13 @@ const parseArrayFromEnv = ( return defaultValue; }; +const parseNullableFromEnv = ( + env: ?string, + defaultValue: any, +): any => { + return typeof(env) === 'undefined' ? defaultValue : null; +}; + const getConfig = () => { let config = { loglevel: process.env.LOGLEVEL || 'info', @@ -52,6 +59,16 @@ const getConfig = () => { key: process.env.TLS_KEY_PATH || '/etc/ssl/kube-ldap/key.pem', ca: process.env.TLS_CA_PATH || null, }, + prometheus: { + username: parseNullableFromEnv( + process.env.PROMETHEUS_USERNAME, + 'prometheus' + ), + password: parseNullableFromEnv(process.env.PROMETHEUS_PASSWORD, 'secret'), + nodejsProbeInterval: parseInt( + process.env.PROMETHEUS_NODEJS_PROBE_INTERVAL + ) || 10000, + }, port: 0, }; config.port = parseInt(process.env.PORT) || ( diff --git a/yarn.lock b/yarn.lock index debe682..358e417 100644 --- a/yarn.lock +++ b/yarn.lock @@ -906,6 +906,13 @@ base@^0.11.1: mixin-deep "^1.2.0" pascalcase "^0.1.1" +basic-auth@^2.0.1: + version "2.0.1" + resolved "https://registry.yarnpkg.com/basic-auth/-/basic-auth-2.0.1.tgz#b998279bf47ce38344b4f3cf916d4679bbf51e3a" + integrity sha512-NF+epuEdnUYVlGuhaxbbq+dvJttwLnGY+YixlXlME5KpQ5W3CnXA5cVTneY3SPbPDRkcjMbifrwmFYcClgOZeg== + dependencies: + safe-buffer "5.1.2" + basic-auth@~2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/basic-auth/-/basic-auth-2.0.0.tgz#015db3f353e02e56377755f962742e8981e7bbba" @@ -922,6 +929,11 @@ binary-extensions@^1.0.0: version "1.11.0" resolved "https://registry.yarnpkg.com/binary-extensions/-/binary-extensions-1.11.0.tgz#46aa1751fb6a2f93ee5e689bb1087d4b14c6c205" +bintrees@1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/bintrees/-/bintrees-1.0.1.tgz#0e655c9b9c2435eaab68bf4027226d2b55a34524" + integrity sha1-DmVcm5wkNeqraL9AJyJtK1WjRSQ= + block-stream@*: version "0.0.9" resolved "https://registry.yarnpkg.com/block-stream/-/block-stream-0.0.9.tgz#13ebfe778a03205cfe03751481ebb4b3300c126a" @@ -1686,6 +1698,21 @@ expect@^22.4.3: jest-message-util "^22.4.3" jest-regex-util "^22.4.3" +express-basic-auth@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/express-basic-auth/-/express-basic-auth-1.2.0.tgz#a1d40b07721376ba916e73571a60969211224808" + integrity sha512-iJ0h1Gk6fZRrFmO7tP9nIbxwNgCUJASfNj5fb0Hy15lGtbqqsxpt7609+wq+0XlByZjXmC/rslWQtnuSTVRIcg== + dependencies: + basic-auth "^2.0.1" + +express-prom-bundle@^5.1.5: + version "5.1.5" + resolved "https://registry.yarnpkg.com/express-prom-bundle/-/express-prom-bundle-5.1.5.tgz#f298615879299a58cf8ec1350186f4de91d91fa4" + integrity sha512-tUaQUBu0r9zGYcVDpKBI2AeWimuuodaC5BSBkzLPQxRTxaKQShQNnONQSYwjLxbHfPwlCKVZlzfbB9Recnj0Vg== + dependencies: + on-finished "^2.3.0" + url-value-parser "^2.0.0" + express@^4.16.3: version "4.16.3" resolved "https://registry.yarnpkg.com/express/-/express-4.16.3.tgz#6af8a502350db3246ecc4becf6b5a34d22f7ed53" @@ -3395,7 +3422,7 @@ object.pick@^1.3.0: dependencies: isobject "^3.0.1" -on-finished@~2.3.0: +on-finished@^2.3.0, on-finished@~2.3.0: version "2.3.0" resolved "https://registry.yarnpkg.com/on-finished/-/on-finished-2.3.0.tgz#20f1336481b083cd75337992a16971aa2d906947" dependencies: @@ -3618,6 +3645,13 @@ progress@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/progress/-/progress-2.0.0.tgz#8a1be366bf8fc23db2bd23f10c6fe920b4389d1f" +prom-client@^11.5.0: + version "11.5.0" + resolved "https://registry.yarnpkg.com/prom-client/-/prom-client-11.5.0.tgz#b4fd8f621abaa7b1d6f0b01dd5434504a496f40e" + integrity sha512-VxEGhbe/C287yBARXCEWLmbTQy7n97DZmaMuHdqLQ1bB/THLHcelZHcSyxIHigL6UtvKq09ZUEinK6hsH6LwDQ== + dependencies: + tdigest "^0.1.1" + proxy-addr@~2.0.3: version "2.0.3" resolved "https://registry.yarnpkg.com/proxy-addr/-/proxy-addr-2.0.3.tgz#355f262505a621646b3130a728eb647e22055341" @@ -3974,6 +4008,11 @@ safe-buffer@5.1.1, safe-buffer@^5.0.1, safe-buffer@^5.1.1, safe-buffer@~5.1.0, s version "5.1.1" resolved "https://registry.yarnpkg.com/safe-buffer/-/safe-buffer-5.1.1.tgz#893312af69b2123def71f57889001671eeb2c853" +safe-buffer@5.1.2: + version "5.1.2" + resolved "https://registry.yarnpkg.com/safe-buffer/-/safe-buffer-5.1.2.tgz#991ec69d296e0313747d59bdfd2b745c35f8828d" + integrity sha512-Gd2UZBJDkXlY7GbJxfsE8/nvKkUEU1G38c1siN6QP6a9PT9MmHB8GnpscSmMJSoF8LOIrt8ud/wPtojys4G6+g== + safe-regex@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/safe-regex/-/safe-regex-1.1.0.tgz#40a3669f3b077d1e943d44629e157dd48023bf2e" @@ -4391,6 +4430,13 @@ tar@^2.2.1: fstream "^1.0.2" inherits "2" +tdigest@^0.1.1: + version "0.1.1" + resolved "https://registry.yarnpkg.com/tdigest/-/tdigest-0.1.1.tgz#2e3cb2c39ea449e55d1e6cd91117accca4588021" + integrity sha1-Ljyyw56kSeVdHmzZEReszKRYgCE= + dependencies: + bintrees "1.0.1" + test-exclude@^4.2.1: version "4.2.1" resolved "https://registry.yarnpkg.com/test-exclude/-/test-exclude-4.2.1.tgz#dfa222f03480bca69207ca728b37d74b45f724fa" @@ -4537,6 +4583,11 @@ urix@^0.1.0: version "0.1.0" resolved "https://registry.yarnpkg.com/urix/-/urix-0.1.0.tgz#da937f7a62e21fec1fd18d49b35c2935067a6c72" +url-value-parser@^2.0.0: + version "2.0.1" + resolved "https://registry.yarnpkg.com/url-value-parser/-/url-value-parser-2.0.1.tgz#c8179a095ab9ec1f5aa17ca36af5af396b4e95ed" + integrity sha512-bexECeREBIueboLGM3Y1WaAzQkIn+Tca/Xjmjmfd0S/hFHSCEoFkNh0/D0l9G4K74MkEP/lLFRlYnxX3d68Qgw== + use@^3.1.0: version "3.1.0" resolved "https://registry.yarnpkg.com/use/-/use-3.1.0.tgz#14716bf03fdfefd03040aef58d8b4b85f3a7c544" From 2017e98c46c539bb503ff47b465e4d30955a498d Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Wed, 12 Jun 2019 14:36:16 +0200 Subject: [PATCH 09/16] simplify parsing functions --- src/config.js | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/config.js b/src/config.js index 6cbb5f9..8f8c3a1 100644 --- a/src/config.js +++ b/src/config.js @@ -1,13 +1,7 @@ // @flow const parseBooleanFromEnv = (env: ?string, defaultValue: boolean): boolean => { - if (env == 'true') { - return true; - } - if (env == 'false') { - return false; - } - return defaultValue; + return env == 'true' ? true : (env == 'false' ? false : defaultValue); }; const parseArrayFromEnv = ( @@ -15,10 +9,7 @@ const parseArrayFromEnv = ( delimiter: string, defaultValue: Array ): Array => { - if (env) { - return env.split(delimiter); - } - return defaultValue; + return env ? env.split(delimiter) : defaultValue; }; const parseNullableFromEnv = ( From daba70fb946ccc4313f3045415eea74f6e497ead Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Wed, 12 Jun 2019 14:49:20 +0200 Subject: [PATCH 10/16] fix nullable env parser and add tests --- src/config.js | 2 +- test/unit/config.test.js | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/config.js b/src/config.js index 8f8c3a1..1b07a35 100644 --- a/src/config.js +++ b/src/config.js @@ -16,7 +16,7 @@ const parseNullableFromEnv = ( env: ?string, defaultValue: any, ): any => { - return typeof(env) === 'undefined' ? defaultValue : null; + return typeof(env) === 'undefined' ? defaultValue : (env || null); }; const getConfig = () => { diff --git a/test/unit/config.test.js b/test/unit/config.test.js index c28bb5d..b17e7a3 100644 --- a/test/unit/config.test.js +++ b/test/unit/config.test.js @@ -97,6 +97,29 @@ const fixtures = { default: null, testValues: ['/etc/ssl/example.com/ca.pem'], }, + 'prometheus.username': { + env: 'PROMETHEUS_USERNAME', + default: 'prometheus', + testValues: [ + {value: 'john.doe', expected: 'john.doe'}, + {value: '', expected: null}, + ], + }, + 'prometheus.password': { + env: 'PROMETHEUS_PASSWORD', + default: 'secret', + testValues: [ + {value: 'password', expected: 'password'}, + {value: '', expected: null}, + ], + }, + 'prometheus.nodejsProbeInterval': { + env: 'PROMETHEUS_NODEJS_PROBE_INTERVAL', + default: 10000, + testValues: [ + {value: '5000', expected: 5000}, + ], + }, 'port': { env: 'PORT', default: 8081, From e745e79eeba427ad408de7394c88a0feba71bc03 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Wed, 12 Jun 2019 15:11:10 +0200 Subject: [PATCH 11/16] extract prometheus basic auth into function --- src/app.js | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/app.js b/src/app.js index 798f2df..21210e0 100644 --- a/src/app.js +++ b/src/app.js @@ -7,7 +7,7 @@ import morgan from 'morgan'; import basicAuth from 'express-basic-auth'; import prometheusBundle from 'express-prom-bundle'; import {Client as Connection} from 'ldapts'; -import {config} from './config'; +import {config, getConfig} from './config'; import logger from './logger'; import {Client, Authenticator, Mapping} from './ldap'; import {Healthz, UserAuthentication, TokenAuthentication} from './api'; @@ -55,6 +55,21 @@ let prometheusExporter = prometheusBundle({ }, }, }); +let prometheusBasicAuth = (req, res, next) => { + let config = getConfig(); + if ( + Boolean(config.prometheus.username) && + Boolean(config.prometheus.password) + ) { + basicAuth({ + users: { + [config.prometheus.username]: config.prometheus.password, + }, + })(req, res, next); + } else { + next(); + } +}; // setup express const app = express(); @@ -66,16 +81,7 @@ app.use(morgan('combined', { }, }, })); -if ( - Boolean(config.prometheus.username) && - Boolean(config.prometheus.password) -) { - app.use('/metrics', basicAuth({ - users: { - [config.prometheus.username]: config.prometheus.password, - }, - })); -}; +app.use('/metrics', prometheusBasicAuth); app.use(prometheusExporter); app.get('/healthz', healthz.run); app.get('/auth', userAuthentication.run); From 993ae58935d702607f7012d8528ab4cf2b3842eb Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Wed, 12 Jun 2019 15:11:25 +0200 Subject: [PATCH 12/16] add integration tests for prometheus exporter --- test/integration/app.test.js | 47 ++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/integration/app.test.js b/test/integration/app.test.js index 6534990..52513a3 100644 --- a/test/integration/app.test.js +++ b/test/integration/app.test.js @@ -128,4 +128,51 @@ describe('test app routes', () => { done(error); }); }); + test('GET /metrics: 200 OK (enabled basic auth)', (done) => { + request(app) + .post('/metrics') + .auth(config.prometheus.username, config.prometheus.password) + .expect(200) + .end((error) => { + done(error); + }); + }); + test('GET /metrics: 200 OK (with disabled basic auth)', (done) => { + process.env.PROMETHEUS_USERNAME = ''; + process.env.PROMETHEUS_PASSWORD = ''; + request(app) + .post('/metrics') + .expect(200) + .end((error) => { + delete process.env.PROMETHEUS_USERNAME; + delete process.env.PROMETHEUS_PASSWORD; + done(error); + }); + }); + test('GET /metrics: 401 Unauthorized (No authorization header)', (done) => { + request(app) + .post('/metrics') + .expect(401) + .end((error) => { + done(error); + }); + }); + test('GET /metrics: 401 Unauthorized (Wrong username/password)', (done) => { + request(app) + .post('/metrics') + .auth('john.doe', 'secret') + .expect(401) + .end((error) => { + done(error); + }); + }); + test('GET /metrics: 401 Unauthorized (Wrong username/password)', (done) => { + request(app) + .post('/metrics') + .auth('john.doe', 'secret') + .expect(401) + .end((error) => { + done(error); + }); + }); }); From 564d988b2dadb1295b22448807f6bd04ba7a763d Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Wed, 12 Jun 2019 15:45:12 +0200 Subject: [PATCH 13/16] emphasize and clarify breaking changes --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9806ad..94deea6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,15 +9,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Prometheus exporter on route "/metrics" (basic auth protected) ### Changed -- *BREAKING*: Extra-Attributes and groups are now no longer included in the JWT issued after user authentication. Extra-Attributes and group memberships are now resolved during the token review and are included in the token review response +- **BREAKING:** Extra-Attributes and groups are now no longer included in the JWT issued after user authentication. Extra-Attributes and group memberships are now resolved during the token review and are included in the token review response - Internal: Use [ldapts](https://github.com/ldapts/ldapts) instead of [ldapjs](https://github.com/joyent/node-ldapjs) as ldap library ### Fixed - Fix membership resolution for ldap objects without any membership ### Removed -- *BREAKING:* LDAP StartTLS is no longer supported -- *BREAKING:* LDAP reconnect logic +- **BREAKING:** LDAP StartTLS is no longer supported +- **BREAKING:** LDAP reconnect logic (now there's a new connection for every request) ## [1.3.0] - 2019-01-07 ### Changed From 2c6b03440a5b3c37b96b8fafac268a50dcf3c07d Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Wed, 12 Jun 2019 15:48:34 +0200 Subject: [PATCH 14/16] bump nodejs version to latest v8 --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 2181a80..78a1cd7 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM node:8.15.0-alpine +FROM node:8.16.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 61437ef9a02e5879002e1fa6408f11ff5100e030 Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Wed, 12 Jun 2019 15:54:56 +0200 Subject: [PATCH 15/16] fix description of workflow for new group membership resolution order --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 138d16d..84572e7 100644 --- a/README.md +++ b/README.md @@ -9,11 +9,11 @@ A [Webhook Token Authentication](https://kubernetes.io/docs/admin/authentication The kube-ldap webhook token authentication plugin can be used to integrate username/password authentication via LDAP for your kubernetes cluster. It exposes two API endpoints: * /auth - * HTTP basic authenticated requests to this endpoint result in a JSON Web Token, signed by the webhook, including the username, uid and group memberships of the authenticated user. - * The issued token can be used for authenticating to kubernetes. + * HTTP basic authenticated requests to this endpoint result in a JSON Web Token, signed by the webhook, including the username and uid of the authenticated user. + * The issued token can be used for authenticating to kubernetes. * /token * Is called by kubernetes (see [TokenReview](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.9/#tokenreview-v1-authentication)) to verify the token used for authentication. - * Verifies the integrity of the JWT (using the signature) and returns a TokenReview response containing the username, uid and group memberships of the authenticated user. + * Verifies the integrity of the JWT (using the signature) and returns a TokenReview response containing the username, uid, group memberships and extra attributes (if configured) of the authenticated user. ## Deployment The recommended way to deploy kube-ldap is deplyoing kube-ldap in kubernetes itself using the [gyselroth/kube-ldap](https://hub.docker.com/r/gyselroth/kube-ldap/) docker image. From 0c2cede4127253f30bc9ae886e9072f33402813b Mon Sep 17 00:00:00 2001 From: Fabian Jucker Date: Wed, 12 Jun 2019 15:55:33 +0200 Subject: [PATCH 16/16] version bump to 2.0.0 --- CHANGELOG.md | 6 +++++- package.json | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94deea6..9f7cc2e 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] + +## [2.0.0] - 2019-06-12 ### Added - Prometheus exporter on route "/metrics" (basic auth protected) @@ -53,7 +55,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Initial key functionality -[Unreleased]: https://github.com/gyselroth/kube-ldap/compare/v1.3.0...master +[Unreleased]: https://github.com/gyselroth/kube-ldap/compare/v2.0.0...master +[2.0.0]: https://github.com/gyselroth/kube-ldap/compare/v1.3.0...v2.0.0 +[1.3.0]: https://github.com/gyselroth/kube-ldap/compare/v1.2.1...v1.3.0 [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 diff --git a/package.json b/package.json index 5933c0c..248926c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "kube-ldap", - "version": "1.3.0", + "version": "2.0.0", "description": "kubernetes token webhook to check bearer tokens against ldap", "main": "src/index.js", "author": "Fabian Jucker ",