Merge pull request #21 from gyselroth/dev

version 1.3.0
gyselroth · Jan 7, 2019 · 01f5ad9 · 01f5ad9
2 parents dff511f + 83179ab
commit 01f5ad9
Show file tree

Hide file tree

Showing 17 changed files with 235 additions and 65 deletions.
diff --git 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" && 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" != "" && 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
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1,48 @@
+# 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]
+
+## [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)
+- 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.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
+[1.0.0]: https://github.com/gyselroth/kube-ldap/tree/v1.0.0
diff --git a/Dockerfile b/Dockerfile
@@ -1,7 +1,7 @@
-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://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

diff --git 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|
@@ -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|

diff --git a/__mocks__/ldapjs.js b/__mocks__/ldapjs.js
@@ -8,21 +8,22 @@ class LdapMock {
   bindReturnsError: boolean;
   searchReturnsError: boolean;
   searchEmitsError: boolean;
-  searchEmitsEnd: boolean;
+  searchEmitsResult: boolean;
   searchEmitsEndStatus: number;
   searchResult: Object;
   emitter: EventEmitter;
   starttls: (string, Array<string>) => Promise<Object>;
   bind: (string, Array<string>) => Promise<Object>;
   search: (string, Array<string>) => Promise<Object>;
+  on: (string, (any) => any) => void;
 
   /** creates the mock */
   constructor() {
     this.starttlsReturnsError = false;
     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/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/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 <[email protected]>",
@@ -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/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/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';
@@ -19,12 +20,14 @@ let ldapClient = new Client(
     reconnect: {
       initialDelay: config.ldap.reconnectInitialDelay,
       maxDelay: config.ldap.reconnectMaxDelay,
-      failAfter: config.ldap.reconnectFailAfter
-    }
+      failAfter: config.ldap.reconnectFailAfter,
+    },
+    log: bunyanWinstonAdapter.createAdapter(logger),
   }),
   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
@@ -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',
@@ -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
@@ -5,24 +5,31 @@ export default class Client {
   basedn: string;
   binddn: string;
   bindpw: string;
+  startTls: boolean;
   _secure: boolean;
 
   /**
   * Create an LDAP 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.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;
@@ -36,14 +43,15 @@ export default class Client {
   */
   bind(dn: string, password: string): Promise<boolean> {
     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) => {
         if (err) {
           resolve(false);
+        } else {
+          resolve(true);
         }
-        resolve(true);
       });
     });
   }
@@ -65,38 +73,44 @@ 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;
       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}]`));
           });
-        });
+        }
       });
     });
   }