From fb7eee1225710eb1ae7a2902bdef963e584c48af Mon Sep 17 00:00:00 2001
From: ale <ale@incal.net>
Date: Tue, 26 Aug 2014 10:38:55 +0100
Subject: [PATCH] make the LDAP schema configurable

Also adds a test for server.py, to verify that the program will
actually start the HTTP server.
---
 .gitignore                        |  2 +-
 README.md                         | 33 +++++++++++++++++++++++++
 authserv/ldap_model.py            | 22 +++++++++--------
 authserv/server.py                | 35 +++++++++++++++++---------
 authserv/test/__init__.py         | 21 ++++++++--------
 authserv/test/test_auth_ldap.py   |  1 +
 authserv/test/test_integration.py | 10 +-------
 authserv/test/test_server.py      | 41 +++++++++++++++++++++++++++++++
 8 files changed, 124 insertions(+), 41 deletions(-)
 create mode 100644 authserv/test/test_server.py

diff --git a/.gitignore b/.gitignore
index 4677c24..3a84c4a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,4 +6,4 @@ debian/tmp
 debian/*.log
 venv
 .coverage
-
+.tox
diff --git a/README.md b/README.md
index 24cfcb0..84b101f 100644
--- a/README.md
+++ b/README.md
@@ -32,6 +32,8 @@ some module-level variables:
   the user objects, and to search the user subtree.
 * `LDAP_SERVICE_MAP` is a `{service: query}` dictionary that defines
   how to query the LDAP database for user accounts, for each service.
+* `LDAP_SCHEMA` is a dictionary that describes how to retrieve
+  authentication information from the LDAP object attributes.
 
 
 ## Query definition
@@ -76,6 +78,37 @@ or alternatively, if the database structure is simple enough:
     }
 
 
+## Schema definition
+
+In order to retrieve authentication information from the LDAP object,
+the authentication server needs to know which attributes to use. To do
+so, we use a so-called *schema definition* (a map of symbolic names to
+LDAP attributes). The following attribute names are defined:
+
+* `password`: attribute containing the encrypted password. Usually
+  (and by default) this is `userPassword`, a somewhat standard LDAP
+  attribute. Since this attribute is often also used for
+  authentication of the LDAP protocol itself, an eventual `{crypt}`
+  prefix is ignored. Passwords should be encrypted with the system
+  `crypt` method.
+* `otp_secret`: this attribute should contain the hex-encoded TOTP
+  secret.
+* `app_specific_password`: attribute (possibly defined more than once)
+  containing an encoded app-specific password.
+* `shard`: if set, LDAP attribute containing a shard ID for user
+  partitioning.
+
+The LDAP backend module makes some assumptions on the structure of the
+user database: the most important is that app-specific passwords are
+attributes of the user object. App-specific passwords should be
+encoded as colon-separated strings:
+
+    service:encrypted_password:comment
+
+Again, the password should be encrypted with the system `crypt`
+method. The comment is a free-form string set by the user to tell the
+various credentials apart.
+
 
 # Usage
 
diff --git a/authserv/ldap_model.py b/authserv/ldap_model.py
index 4fb9ae2..fa82833 100644
--- a/authserv/ldap_model.py
+++ b/authserv/ldap_model.py
@@ -8,7 +8,7 @@ from authserv import model
 
 
 # Define the LDAP schema attributes that we will use.
-SCHEMA = {
+DEFAULT_SCHEMA = {
     'password': 'userPassword',
     'otp_secret': 'totpSecret',
     'app_specific_password': 'appSpecificPassword',
@@ -29,10 +29,12 @@ def _expandvars(s, vars, quotefn):
 
 class UserDb(model.UserDb):
 
-    ldap_attrs = SCHEMA.values()
-
-    def __init__(self, service_map, ldap_uri, ldap_bind_dn, ldap_bind_pw):
+    def __init__(self, service_map, schema, ldap_uri, ldap_bind_dn, ldap_bind_pw):
         self.service_map = service_map
+        self.schema = DEFAULT_SCHEMA
+        if schema:
+            self.schema.update(schema)
+        self.ldap_attrs = filter(None, self.schema.values())
         self.ldap_uri = ldap_uri
         self.ldap_bind_dn = ldap_bind_dn
         self.ldap_bind_pw = ldap_bind_pw
@@ -89,7 +91,7 @@ class UserDb(model.UserDb):
             if len(result) > 1:
                 raise Error('too many results from LDAP')
 
-            return User(username, result[0][0], result[0][1])
+            return User(username, result[0][0], result[0][1], self.schema)
 
     def get_user(self, username, service, shard):
         try:
@@ -119,21 +121,21 @@ class AppSpecificPassword(object):
 
 class User(model.User):
 
-    def __init__(self, username, dn, data):
+    def __init__(self, username, dn, data, schema):
         self._username = username
         self._dn = dn
         self._otp_enabled = False
         self._asps = []
         self._shard = None
         for key, values in data.iteritems():
-            if key == SCHEMA['password']:
+            if key == schema['password']:
                 self._password = values[0]
                 if self._password.startswith('{crypt}'):
                     self._password = self._password[7:]
-            elif key == SCHEMA['otp_secret']:
+            elif key == schema['otp_secret']:
                 self._otp_enabled = True
                 self._totp_secret = values[0]
-            elif key == SCHEMA['app_specific_password']:
+            elif key == schema['app_specific_password']:
                 # Format is service:password:comment. Ignore the
                 # comment, and avoid dying on malformed input.
                 self._asps = []
@@ -142,7 +144,7 @@ class User(model.User):
                         self._asps.append(AppSpecificPassword(v))
                     except:
                         pass
-            elif key == SCHEMA['shard']:
+            elif key == schema['shard']:
                 self._shard = values[0]
 
     def otp_enabled(self):
diff --git a/authserv/server.py b/authserv/server.py
index 8105073..16b4f05 100644
--- a/authserv/server.py
+++ b/authserv/server.py
@@ -8,6 +8,7 @@ import logging.handlers
 import optparse
 import os
 import signal
+import sys
 from authserv import app_main
 from authserv import app_nginx
 
@@ -19,6 +20,7 @@ def create_app(app, userdb=None, mc=None):
         from authserv import ldap_model
         userdb = ldap_model.UserDb(
             app.config['LDAP_SERVICE_MAP'],
+            app.config.get('LDAP_SCHEMA'),
             app.config.get('LDAP_URI', 'ldap://127.0.0.1:389'),
             app.config['LDAP_BIND_DN'],
             app.config['LDAP_BIND_PW'])
@@ -54,7 +56,9 @@ def run(flask_app, addr, port, ssl_ca, ssl_cert, ssl_key, dh_params):
     WSGIServer((addr, port), flask_app.wsgi_app, **ssl_args).serve_forever()
 
 
-def main():
+def main(sysargs=None):
+    if sysargs is None:
+        sysargs = sys.argv[1:]
     parser = optparse.OptionParser()
     parser.add_option('--config',
                       help='Configuration file')
@@ -79,7 +83,7 @@ def main():
                       help='Diffie-Helmann parameters file')
     parser.add_option('--debug', action='store_true')
 
-    opts, args = parser.parse_args()
+    opts, args = parser.parse_args(sysargs)
     if len(args) != 0:
         parser.error('Too many arguments')
 
@@ -104,20 +108,29 @@ def main():
     def _stopall(signo, frame):
         logging.info('terminating with signal %d', signo)
         os._exit(0)
-    signal.signal(signal.SIGINT, _stopall)
-    signal.signal(signal.SIGTERM, _stopall)
+    gevent.signal(signal.SIGINT, _stopall)
+    gevent.signal(signal.SIGTERM, _stopall)
 
     # Start the applications that were requested: the NGINX
     # mail_http_auth handler (on its own thread), and the main auth
-    # server application.
+    # server application. Try to catch errors with the application
+    # setup and display something nicer than a stack trace.
     if opts.nginx_port > 0:
-        gevent.spawn(run,
-            create_app(app_nginx.app),
-            '127.0.0.1', opts.nginx_port, None, None, None, None)
-    run(create_app(app_main.app),
-        opts.addr, opts.port, opts.ssl_ca,
+        try:
+            app = create_app(app_nginx.app)
+        except Exception as e:
+            logging.fatal("Application setup error: %s", e)
+            return 1
+        gevent.spawn(run, app, '127.0.0.1', opts.nginx_port,
+                     None, None, None, None)
+    try:
+        app = create_app(app_main.app)
+    except Exception as e:
+        logging.fatal("Application setup error: %s", e)
+        return 1
+    run(app, opts.addr, opts.port, opts.ssl_ca,
         opts.ssl_cert, opts.ssl_key, opts.dh_params)
 
 
 if __name__ == '__main__':
-    main()
+    sys.exit(main())
diff --git a/authserv/test/__init__.py b/authserv/test/__init__.py
index 810ffe4..6df5a86 100644
--- a/authserv/test/__init__.py
+++ b/authserv/test/__init__.py
@@ -1,10 +1,10 @@
 import crypt
 import logging
 import os
-import socket
 import time
 import unittest
 from ldap_test import LdapServer
+from gevent import socket
 from authserv import model
 
 
@@ -79,18 +79,10 @@ class LdapTestBase(unittest.TestCase):
 
     LDIFS = []
 
-    @classmethod
-    def _pick_free_port(cls):
-        sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-        sock.bind(('', 0))
-        port = sock.getsockname()[1]
-        sock.close()
-        return port
-
     @classmethod
     def setup_class(cls):
         # Start the local LDAP server.
-        cls.ldap_port = cls._pick_free_port()
+        cls.ldap_port = free_port()
         cls.ldap_password = 'testpass'
         ldifs = [os.path.join(os.path.dirname(__file__), 'fixtures', x)
                  for x in ['base.ldif'] + cls.LDIFS]
@@ -112,3 +104,12 @@ class LdapTestBase(unittest.TestCase):
     def teardown_class(cls):
         cls.server.stop()
 
+
+def free_port():
+    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM, socket.IPPROTO_TCP)
+    s.bind(('', 0))
+    port = s.getsockname()[1]
+    s.close()
+    return port
+
+
diff --git a/authserv/test/test_auth_ldap.py b/authserv/test/test_auth_ldap.py
index 8ad9a63..666960e 100644
--- a/authserv/test/test_auth_ldap.py
+++ b/authserv/test/test_auth_ldap.py
@@ -25,6 +25,7 @@ class LdapAuthTestBase(LdapTestBase):
     def setUp(self):
         self.userdb = ldap_model.UserDb(
             self.SERVICE_MAP,
+            None,
             'ldap://localhost:%d' % self.ldap_port,
             'cn=manager,o=Anarchy',
             self.ldap_password)
diff --git a/authserv/test/test_integration.py b/authserv/test/test_integration.py
index d9bb1e6..5b4290e 100644
--- a/authserv/test/test_integration.py
+++ b/authserv/test/test_integration.py
@@ -20,14 +20,6 @@ def _relpath(x):
     return os.path.join(os.path.dirname(__file__), x)
 
 
-def _free_port():
-    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM, socket.IPPROTO_TCP)
-    s.bind(('127.0.0.1', 0))
-    port = s.getsockname()[1]
-    s.close()
-    return port
-
-
 class HTTPSClientAuthHandler(urllib2.HTTPSHandler):
 
     def __init__(self, cert, key):
@@ -63,7 +55,7 @@ class SSLServerTest(unittest.TestCase):
         }
 
         cls.pid = 0
-        cls.port = _free_port()
+        cls.port = free_port()
         def _runserver():
             app = server.create_app(app_main.app,
                                     userdb=FakeUserDb(cls.users),
diff --git a/authserv/test/test_server.py b/authserv/test/test_server.py
new file mode 100644
index 0000000..70e8e45
--- /dev/null
+++ b/authserv/test/test_server.py
@@ -0,0 +1,41 @@
+import mock
+import os
+import tempfile
+from authserv import server
+from authserv.test import *
+
+
+def unexpected():
+    raise Exception('unexpected call')
+
+
+class ServerTest(unittest.TestCase):
+
+    def setUp(self):
+        fd, self.config_file = tempfile.mkstemp()
+        os.write(fd, '''
+LDAP_SERVICE_MAP = {
+  'mail': {
+  }
+}
+LDAP_BIND_DN = 'cn=manager,o=Anarchy'
+LDAP_BIND_PW = 'testpass'
+MEMCACHE_ADDR = ['127.0.0.1:11211']
+''')
+        os.close(fd)
+
+    def tearDown(self):
+        os.remove(self.config_file)
+
+    def test_run_fails_without_config(self):
+        with mock.patch('authserv.server.run', side_effect=unexpected):
+            server.main(['--port=1234'])
+
+    def test_run_nossl_ok(self):
+        with mock.patch('authserv.server.run') as p:
+            server.main(['--config=%s' % self.config_file,
+                         '--port=1234'])
+            # Don't really look at the SSL params for now.
+            p.assert_called_once_with(
+                mock.ANY, '0.0.0.0', 1234,
+                mock.ANY, mock.ANY, mock.ANY, mock.ANY)
-- 
GitLab