From 6e83202426e4c81deb718bf1883ef07b182d0211 Mon Sep 17 00:00:00 2001
From: Friedrich Weber <friedrich.weber@netknights.it>
Date: Mon, 19 Jun 2017 18:16:49 +0200
Subject: [PATCH 01/13] Add simple login premable detection mechanism

Working on #13
---
 pi_ldapproxy/realmmapping.py           | 52 ++++++++++++++++++++++++++
 pi_ldapproxy/test/test_realmmapping.py | 44 ++++++++++++++++++++++
 2 files changed, 96 insertions(+)
 create mode 100644 pi_ldapproxy/realmmapping.py
 create mode 100644 pi_ldapproxy/test/test_realmmapping.py

diff --git a/pi_ldapproxy/realmmapping.py b/pi_ldapproxy/realmmapping.py
new file mode 100644
index 0000000..2b225f8
--- /dev/null
+++ b/pi_ldapproxy/realmmapping.py
@@ -0,0 +1,52 @@
+from ldaptor.protocols.pureldap import LDAPFilter_and, LDAPFilter_or, LDAPFilter_equalityMatch, LDAPSearchRequest, \
+    LDAPSearchResultEntry
+from twisted.logger import Logger
+
+log = Logger()
+
+
+def find_app_marker(filter, attribute='objectclass', value_prefix='App-'):
+    """
+    Given an ldaptor filter, try to extract an app marker, i.e. find
+    a marker such that the filter contains an expression (<attribute>=<value_prefix><marker>),
+    e.g. (objectclass=App-ownCloud).
+    It may be nested in &() and |() expressions.
+    :param filter: ldaptor filter
+    :param attribute: attribute name whose value contains the app marker
+    :param value_prefix: prefix of the app marker
+    :return: None or an app marker (a string)
+    """
+    if isinstance(filter, LDAPFilter_and) or isinstance(filter, LDAPFilter_or):
+        # recursively search and/or expressions
+        for subfilter in filter:
+            app_marker = find_app_marker(subfilter, attribute, value_prefix)
+            if app_marker:
+                return app_marker
+    elif isinstance(filter, LDAPFilter_equalityMatch):
+        # check attribute name and value prefix
+        if filter.attributeDesc.value == attribute:
+            value = filter.assertionValue.value
+            if value.startswith(value_prefix):
+                return value[len(value_prefix):]
+    return None
+
+
+def detect_login_preamble(request, response, attribute='objectclass', value_prefix='App-'):
+    """
+    Determine whether the request/response pair constitutes a login preamble.
+    If it does, return the login DN and the app marker.
+    :param request: LDAP request
+    :param response: LDAP response
+    :param attribute: see ``find_app_marker``
+    :param value_prefix: see ``find_app_marker``
+    :return: A tuple ``(DN, app marker)`` or None
+    """
+    if isinstance(request, LDAPSearchRequest) and request.filter:
+        # TODO: Check base dn?
+        marker = find_app_marker(request.filter, attribute, value_prefix)
+        # TODO: This will be called multiple times for the same search request!
+        # i.e. we do not notice if the response has >1 entries
+        if marker is not None and isinstance(response, LDAPSearchResultEntry):
+            log.info('Detected login preamble: {!r} ({!r})'.format(response.objectName, marker))
+            return (response.objectName, marker)
+    return None
diff --git a/pi_ldapproxy/test/test_realmmapping.py b/pi_ldapproxy/test/test_realmmapping.py
new file mode 100644
index 0000000..9106d82
--- /dev/null
+++ b/pi_ldapproxy/test/test_realmmapping.py
@@ -0,0 +1,44 @@
+import twisted
+from ldaptor.ldapfilter import parseFilter
+from ldaptor.protocols import pureldap
+
+from pi_ldapproxy.realmmapping import find_app_marker, detect_login_preamble
+
+
+class TestRealmMapping(twisted.trial.unittest.TestCase):
+    def test_find_app_marker(self):
+        filter = parseFilter('(&(|(objectclass=person)(objectclass=App-someApp))(cn=user123))')
+        self.assertEqual(find_app_marker(filter), 'someApp')
+
+        filter = parseFilter('(&(|(objectclass=person)(someOtherAttribute=App-someApp))(cn=user123))')
+        self.assertIsNone(find_app_marker(filter))
+        self.assertEqual(find_app_marker(filter, attribute='someOtherAttribute'), 'someApp')
+
+        filter = parseFilter('(&(|(objectclass=person)(someOtherAttribute=Prefix-someApp))(cn=user123))')
+        self.assertEqual(find_app_marker(filter, attribute='someOtherAttribute', value_prefix='Prefix-'), 'someApp')
+
+        filter = parseFilter('(&(|(objectclass=person))(cn=user123))')
+        self.assertIsNone(find_app_marker(filter))
+
+    def test_detect_login_preamble(self):
+        filter = parseFilter('(&(|(objectclass=person)(objectclass=App-someApp))(cn=user123))')
+        request = pureldap.LDAPSearchRequest(baseObject='cn=users,dc=test,dc=local',
+                                  scope=pureldap.LDAP_SCOPE_wholeSubtree, derefAliases=0,
+                                  sizeLimit=0, timeLimit=0, typesOnly=0,
+                                  filter=filter,
+                                  attributes=())
+        dn = 'cn=user123,cn=users,dc=test,dc=local'
+        response = pureldap.LDAPSearchResultEntry(dn, [('cn', ['user123'])])
+        self.assertEqual(detect_login_preamble(request, response), (dn, 'someApp'))
+
+        self.assertIsNone(detect_login_preamble(request, pureldap.LDAPSearchResultDone(0)))
+
+        filter = parseFilter('(&(|(objectclass=person)(someAttribute=Foo-someApp))(cn=user123))')
+        request = pureldap.LDAPSearchRequest(baseObject='cn=users,dc=test,dc=local',
+                                             scope=pureldap.LDAP_SCOPE_wholeSubtree, derefAliases=0,
+                                             sizeLimit=0, timeLimit=0, typesOnly=0,
+                                             filter=filter,
+                                             attributes=())
+        dn = 'cn=user123,cn=users,dc=test,dc=local'
+        response = pureldap.LDAPSearchResultEntry(dn, [('cn', ['user123'])])
+        self.assertEqual(detect_login_preamble(request, response, 'someAttribute', 'Foo-'), (dn, 'someApp'))

From ccec90ce272d3b3ecb8fb314554f5b86d01a3339 Mon Sep 17 00:00:00 2001
From: Friedrich Weber <friedrich.weber@netknights.it>
Date: Mon, 19 Jun 2017 18:41:47 +0200
Subject: [PATCH 02/13] Proxy: Try to detect login preambles

For now, this only writes to the logfile.

Working on #13
---
 pi_ldapproxy/proxy.py        | 19 +++++++++++++++++++
 pi_ldapproxy/realmmapping.py |  1 -
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/pi_ldapproxy/proxy.py b/pi_ldapproxy/proxy.py
index f5abe61..0c3bc51 100644
--- a/pi_ldapproxy/proxy.py
+++ b/pi_ldapproxy/proxy.py
@@ -19,6 +19,7 @@
 
 from pi_ldapproxy.bindcache import BindCache
 from pi_ldapproxy.config import load_config
+from pi_ldapproxy.realmmapping import detect_login_preamble
 from pi_ldapproxy.usermapping import MAPPING_STRATEGIES, UserMappingError
 
 log = Logger()
@@ -135,6 +136,24 @@ def bind_service_account(self):
         log.info('Binding service account ...')
         yield self.client.bind(self.factory.service_account_dn, self.factory.service_account_password)
 
+    def handleProxiedResponse(self, response, request, controls):
+        """
+        Called by `ProxyBase` to handle the response of an incoming request.
+        :param response:
+        :param request:
+        :param controls:
+        :return:
+        """
+        # Try to detect login preamble
+        if isinstance(request, pureldap.LDAPSearchRequest):
+            # TODO: Read attribute and value prefix from config
+            # TODO: Check that this is connection is bound to the service account?
+            result = detect_login_preamble(request, response)
+            if result is not None:
+                dn, app = result
+                log.info('Detected login preamble: {!r}/{!r}'.format(dn, app))
+        return response
+
     def handleBeforeForwardRequest(self, request, controls, reply):
         """
         Called by `ProxyBase` to handle an incoming request.
diff --git a/pi_ldapproxy/realmmapping.py b/pi_ldapproxy/realmmapping.py
index 2b225f8..ca0e673 100644
--- a/pi_ldapproxy/realmmapping.py
+++ b/pi_ldapproxy/realmmapping.py
@@ -47,6 +47,5 @@ def detect_login_preamble(request, response, attribute='objectclass', value_pref
         # TODO: This will be called multiple times for the same search request!
         # i.e. we do not notice if the response has >1 entries
         if marker is not None and isinstance(response, LDAPSearchResultEntry):
-            log.info('Detected login preamble: {!r} ({!r})'.format(response.objectName, marker))
             return (response.objectName, marker)
     return None

From ad09622c720c026afa6c5678e38617aae5037010 Mon Sep 17 00:00:00 2001
From: Friedrich Weber <friedrich.weber@netknights.it>
Date: Tue, 20 Jun 2017 09:01:23 +0200
Subject: [PATCH 03/13] Add prototype of a preamble cache

Working on #13
---
 example-proxy.ini             |  8 +++++-
 pi_ldapproxy/bindcache.py     |  8 +++---
 pi_ldapproxy/config.py        |  6 +++++
 pi_ldapproxy/preamblecache.py | 50 +++++++++++++++++++++++++++++++++++
 pi_ldapproxy/proxy.py         | 25 +++++++++++++++---
 5 files changed, 88 insertions(+), 9 deletions(-)
 create mode 100644 pi_ldapproxy/preamblecache.py

diff --git a/example-proxy.ini b/example-proxy.ini
index a029ae5..ecf95a4 100644
--- a/example-proxy.ini
+++ b/example-proxy.ini
@@ -65,4 +65,10 @@ attribute = sAMAccountName
 # The timeout is specified in seconds.
 # This feature is EXPERIMENTAL.
 enabled = false
-timeout = 3
\ No newline at end of file
+timeout = 3
+
+[preamble-cache]
+enabled = true
+timeout = 5
+attribute = objectclass
+value-prefix = App-
\ No newline at end of file
diff --git a/pi_ldapproxy/bindcache.py b/pi_ldapproxy/bindcache.py
index 21ef368..b09b17d 100644
--- a/pi_ldapproxy/bindcache.py
+++ b/pi_ldapproxy/bindcache.py
@@ -37,11 +37,11 @@ def add_to_cache(self, dn, password):
         item = (dn, password)
         if item not in self._cache:
             current_time = reactor.seconds()
-            log.info('Adding to cache: dn={dn!r}, time={time!r}', dn=dn, time=current_time)
+            log.info('Adding to bind cache: dn={dn!r}, time={time!r}', dn=dn, time=current_time)
             self._cache[item] = current_time
             self.callLater(self.timeout, self.remove_from_cache, dn, password)
         else:
-            log.info('Already in the cache: dn={dn!r}', dn=dn)
+            log.info('Already in the bind cache: dn={dn!r}', dn=dn)
 
     def remove_from_cache(self, dn, password):
         """
@@ -53,9 +53,9 @@ def remove_from_cache(self, dn, password):
         item = (dn, password)
         if item in self._cache:
             del self._cache[item]
-            log.info('Removed from cache: dn={dn!r} ({remaining!r} remaining)', dn=dn, remaining=len(self._cache))
+            log.info('Removed from bind cache: dn={dn!r} ({remaining!r} remaining)', dn=dn, remaining=len(self._cache))
         else:
-            log.info("Removal failed as dn={dn!r} cached", dn=dn)
+            log.info("Removal from bind cache failed as dn={dn!r} is not cached", dn=dn)
 
     def is_cached(self, dn, password):
         """
diff --git a/pi_ldapproxy/config.py b/pi_ldapproxy/config.py
index a888a30..ec35ec6 100644
--- a/pi_ldapproxy/config.py
+++ b/pi_ldapproxy/config.py
@@ -27,6 +27,12 @@
 [bind-cache]
 enabled = boolean
 timeout = integer(default=3)
+
+[preamble-cache]
+enabled = boolean
+timeout = integer(default=3)
+attribute = string(default='objectclass')
+value-prefix = string(default='App-')
 """
 
 def report_config_errors(config, result):
diff --git a/pi_ldapproxy/preamblecache.py b/pi_ldapproxy/preamblecache.py
new file mode 100644
index 0000000..c4bcb47
--- /dev/null
+++ b/pi_ldapproxy/preamblecache.py
@@ -0,0 +1,50 @@
+from twisted.internet import reactor
+from twisted.logger import Logger
+
+log = Logger()
+
+class PreambleCache(object):
+    # (see http://twistedmatrix.com/documents/current/core/howto/trial.html)
+    callLater = reactor.callLater
+
+    def __init__(self, timeout):
+        self.timeout = timeout
+
+        #: Map of dn to tuples (app marker, insertion timestamp)
+        self._preambles = {}
+
+    def add_to_cache(self, dn, marker):
+        if dn in self._preambles:
+            log.info('Entry {dn!r} already cached (marker={marker!r}), overwriting ...'.format(dn=dn, marker=marker))
+        current_time = reactor.seconds()
+        log.info('Adding to preamble cache: dn={dn!r}, marker={marker!r}, time={time!r}',
+                 dn=dn, time=current_time, marker=marker)
+        self._preambles[dn] = (marker, current_time)
+        self.callLater(self.timeout, self.remove_from_cache, dn, marker)
+
+    def remove_from_cache(self, dn, marker):
+        if dn in self._preambles:
+            stored_marker, stored_timestamp = self._preambles[dn]
+            if stored_marker == marker:
+                del self._preambles[dn]
+                log.info('Removed {!r}/{!r} from preamble cache'.format(dn, marker))
+            else:
+                log.warn('Removal from preamble cache failed: {!r} mapped to {!r}, not {!r}'.format(
+                    dn, stored_marker, marker
+                ))
+        else:
+            log.info('Removal from preamble cache failed, as dn={!r} is not cached'.format(dn))
+
+    def get_cached_marker(self, dn):
+        if dn in self._preambles:
+            marker, timestamp = self._preambles[dn]
+            current_time = reactor.seconds()
+            if current_time - timestamp < self.timeout:
+                return marker
+            else:
+                log.warn('Inconsistent preamble cache: dn={dn!r}, inserted={inserted!r}, current={current!r}'.format(
+                    dn=dn, inserted=timestamp, current=current_time
+                ))
+        else:
+            log.info('No entry in preamble cache for dn={dn!r}'.format(dn=dn))
+        return None
diff --git a/pi_ldapproxy/proxy.py b/pi_ldapproxy/proxy.py
index 0c3bc51..2c86286 100644
--- a/pi_ldapproxy/proxy.py
+++ b/pi_ldapproxy/proxy.py
@@ -19,6 +19,7 @@
 
 from pi_ldapproxy.bindcache import BindCache
 from pi_ldapproxy.config import load_config
+from pi_ldapproxy.preamblecache import PreambleCache
 from pi_ldapproxy.realmmapping import detect_login_preamble
 from pi_ldapproxy.usermapping import MAPPING_STRATEGIES, UserMappingError
 
@@ -148,10 +149,7 @@ def handleProxiedResponse(self, response, request, controls):
         if isinstance(request, pureldap.LDAPSearchRequest):
             # TODO: Read attribute and value prefix from config
             # TODO: Check that this is connection is bound to the service account?
-            result = detect_login_preamble(request, response)
-            if result is not None:
-                dn, app = result
-                log.info('Detected login preamble: {!r}/{!r}'.format(dn, app))
+            self.factory.process_search_response(request, response)
         return response
 
     def handleBeforeForwardRequest(self, request, controls, reply):
@@ -254,6 +252,14 @@ def __init__(self, config):
         else:
             self.bind_cache = None
 
+        enable_preamble_cache = config['preamble-cache']['enabled']
+        if enable_preamble_cache:
+            self.preamble_cache = PreambleCache(config['preamble-cache']['timeout'])
+        else:
+            self.preamble_cache = None
+        self.preamble_cache_attribute = config['preamble-cache']['attribute']
+        self.preamble_cache_value_prefix = config['preamble-cache']['value-prefix']
+
         if config['ldap-backend']['test-connection']:
             self.test_connection()
 
@@ -293,6 +299,17 @@ def finalize_authentication(self, dn, password):
         if self.bind_cache is not None:
             self.bind_cache.add_to_cache(dn, password)
 
+    def process_search_response(self, request, response):
+        if self.preamble_cache is not None:
+            result = detect_login_preamble(request,
+                                           response,
+                                           self.preamble_cache_attribute,
+                                           self.preamble_cache_value_prefix)
+            if result is not None:
+                dn, marker = result
+                log.info('Detected login preamble: dn={dn!r}, marker={marker!r}'.format(dn=dn, marker=marker))
+                self.preamble_cache.add_to_cache(dn, marker)
+
     def is_bind_cached(self, dn, password):
         """
         Check whether the given credentials are found in the bind cache.

From cb7996e4d0d201fa35ab58f4e4eb12e26a4bec15 Mon Sep 17 00:00:00 2001
From: Friedrich Weber <friedrich.weber@netknights.it>
Date: Tue, 20 Jun 2017 09:26:28 +0200
Subject: [PATCH 04/13] Add a realm mapper (with a simple static strategy)

Working on #13
---
 example-proxy.ini            |  9 +++--
 pi_ldapproxy/config.py       |  7 +++-
 pi_ldapproxy/proxy.py        | 73 +++++++++++++++++++++++-------------
 pi_ldapproxy/realmmapping.py | 39 +++++++++++++++++++
 pi_ldapproxy/usermapping.py  |  2 +-
 5 files changed, 98 insertions(+), 32 deletions(-)

diff --git a/example-proxy.ini b/example-proxy.ini
index ecf95a4..76cc244 100644
--- a/example-proxy.ini
+++ b/example-proxy.ini
@@ -1,8 +1,6 @@
 [privacyidea]
 # URL of your privacyIDEA installation
 instance = http://10.0.0.1
-# Realm to use for authentication (may also be left blank)
-realm =
 
 [ldap-backend]
 # Location of the LDAP backend server, specified using the Twisted endpoint string syntax for clients:
@@ -71,4 +69,9 @@ timeout = 3
 enabled = true
 timeout = 5
 attribute = objectclass
-value-prefix = App-
\ No newline at end of file
+value-prefix = App-
+
+[realm-mapping]
+strategy = static
+# Realm to use for authentication (may also be left blank)
+realm =
\ No newline at end of file
diff --git a/pi_ldapproxy/config.py b/pi_ldapproxy/config.py
index ec35ec6..04d0443 100644
--- a/pi_ldapproxy/config.py
+++ b/pi_ldapproxy/config.py
@@ -7,7 +7,6 @@
 CONFIG_SPEC = """
 [privacyidea]
 instance = string
-realm = string(default='')
 
 [ldap-backend]
 endpoint = string
@@ -33,6 +32,12 @@
 timeout = integer(default=3)
 attribute = string(default='objectclass')
 value-prefix = string(default='App-')
+
+[user-mapping]
+strategy = string
+
+[realm-mapping]
+strategy = string
 """
 
 def report_config_errors(config, result):
diff --git a/pi_ldapproxy/proxy.py b/pi_ldapproxy/proxy.py
index 2c86286..a00cc3e 100644
--- a/pi_ldapproxy/proxy.py
+++ b/pi_ldapproxy/proxy.py
@@ -20,8 +20,8 @@
 from pi_ldapproxy.bindcache import BindCache
 from pi_ldapproxy.config import load_config
 from pi_ldapproxy.preamblecache import PreambleCache
-from pi_ldapproxy.realmmapping import detect_login_preamble
-from pi_ldapproxy.usermapping import MAPPING_STRATEGIES, UserMappingError
+from pi_ldapproxy.realmmapping import detect_login_preamble, REALM_MAPPING_STRATEGIES, RealmMappingError
+from pi_ldapproxy.usermapping import USER_MAPPING_STRATEGIES, UserMappingError
 
 log = Logger()
 
@@ -73,31 +73,38 @@ def authenticate_bind_request(self, request):
             user = yield self.factory.resolve_user(request.dn)
         except UserMappingError:
             # User could not be found
-            log.info('Could not resolve {dn!r}', dn=request.dn)
+            log.info('Could not resolve {dn!r} to user', dn=request.dn)
             result = (False, 'Invalid user.')
         else:
-            log.info('Resolved {dn!r} to {user!r}', dn=request.dn, user=user)
-            password = request.auth
-            response = yield self.request_validate(self.factory.validate_url,
-                                                   user,
-                                                   self.factory.validate_realm,
-                                                   password)
-            json_body = yield readBody(response)
-            if response.code == 200:
-                body = json.loads(json_body)
-                if body['result']['status']:
-                    if body['result']['value']:
-                        result = (True, '')
-                        # TODO: Is this the right place to bind the service user?
-                        if self.factory.bind_service_account:
-                            yield self.bind_service_account()
-                            self.bound = True
+            try:
+                realm = yield self.factory.resolve_realm(request.dn)
+            except RealmMappingError:
+                log.info('Could not resolve {dn!r} to realm', dn=request.dn)
+                # TODO: too much information revealed?
+                result = (False, 'Could not determine realm.')
+            else:
+                log.info('Resolved {dn!r} to {user!r}@{realm!r}', dn=request.dn, user=user, realm=realm)
+                password = request.auth
+                response = yield self.request_validate(self.factory.validate_url,
+                                                       user,
+                                                       realm,
+                                                       password)
+                json_body = yield readBody(response)
+                if response.code == 200:
+                    body = json.loads(json_body)
+                    if body['result']['status']:
+                        if body['result']['value']:
+                            result = (True, '')
+                            # TODO: Is this the right place to bind the service user?
+                            if self.factory.bind_service_account:
+                                yield self.bind_service_account()
+                                self.bound = True
+                        else:
+                            result = (False, 'Failed to authenticate.')
                     else:
-                        result = (False, 'Failed to authenticate.')
+                        result = (False, 'Failed to authenticate. privacyIDEA error.')
                 else:
-                    result = (False, 'Failed to authenticate. privacyIDEA error.')
-            else:
-                result = (False, 'Failed to authenticate. Wrong HTTP response ({})'.format(response.code))
+                    result = (False, 'Failed to authenticate. Wrong HTTP response ({})'.format(response.code))
         defer.returnValue(result)
 
     def send_bind_response(self, result, request, reply):
@@ -226,7 +233,6 @@ def __init__(self, config):
         if self.privacyidea_instance[-1] != '/':
             self.privacyidea_instance += '/'
         self.validate_url = VALIDATE_URL_TEMPLATE.format(self.privacyidea_instance)
-        self.validate_realm = config['privacyidea']['realm']
 
         self.service_account_dn = config['service-account']['dn']
         self.service_account_password = config['service-account']['password']
@@ -241,10 +247,15 @@ def __init__(self, config):
         self.allow_search = config['ldap-proxy']['allow-search']
         self.bind_service_account = config['ldap-proxy']['bind-service-account']
 
-        mapping_strategy = MAPPING_STRATEGIES[config['user-mapping']['strategy']]
-        log.info('Using mapping strategy: {strategy!r}', strategy=mapping_strategy)
+        user_mapping_strategy = USER_MAPPING_STRATEGIES[config['user-mapping']['strategy']]
+        log.info('Using user mapping strategy: {strategy!r}', strategy=user_mapping_strategy)
+
+        self.user_mapper = user_mapping_strategy(self, config['user-mapping'])
+
+        realm_mapping_strategy = REALM_MAPPING_STRATEGIES[config['realm-mapping']['strategy']]
+        log.info('Using realm mapping strategy: {strategy!r}', strategy=realm_mapping_strategy)
 
-        self.user_mapper = mapping_strategy(self, config['user-mapping'])
+        self.realm_mapper = realm_mapping_strategy(self, config['realm-mapping'])
 
         enable_bind_cache = config['bind-cache']['enabled']
         if enable_bind_cache:
@@ -289,6 +300,14 @@ def resolve_user(self, dn):
         """
         return self.user_mapper.resolve(dn)
 
+    def resolve_realm(self, dn):
+        """
+        Invoke the realm mapper to find the realm of the user identified by the DN *dn*.
+        :param dn: LDAP distinguished name as string
+        :return: a Deferred firing a string (or raising a RealmMappingError)
+        """
+        return self.realm_mapper.resolve(dn)
+
     def finalize_authentication(self, dn, password):
         """
         Called when a user was successfully authenticated by privacyIDEA. If the bind cache is enabled,
diff --git a/pi_ldapproxy/realmmapping.py b/pi_ldapproxy/realmmapping.py
index ca0e673..62e708a 100644
--- a/pi_ldapproxy/realmmapping.py
+++ b/pi_ldapproxy/realmmapping.py
@@ -49,3 +49,42 @@ def detect_login_preamble(request, response, attribute='objectclass', value_pref
         if marker is not None and isinstance(response, LDAPSearchResultEntry):
             return (response.objectName, marker)
     return None
+
+
+class RealmMappingError(Exception):
+    pass
+
+
+class RealmMappingStrategy(object):
+    """
+    Base class for realm mappers, which are used to determine the user's privacyIDEA realm
+    from an incoming LDAP Bind Request's distinguished name.
+    """
+    def __init__(self, factory, config):
+        """
+        :param factory: `ProxyServerFactory` instance
+        :param config: `[realm-mapping]` section of the config file, as a dictionary
+        """
+        self.factory = factory
+        self.config = config
+
+    def resolve(self, dn):
+        """
+        Given the distinguished name, determine the realm name or raise RealmMappingError.
+        :param dn: DN as string
+        :return: A Deferred which fires the realm name (as a string)
+        """
+        raise NotImplementedError()
+
+
+class StaticMappingStrategy(RealmMappingStrategy):
+    def __init__(self, factory, config):
+        RealmMappingStrategy.__init__(self, factory, config)
+        self.realm = config['realm']
+
+    def resolve(self, dn):
+        return self.realm
+
+REALM_MAPPING_STRATEGIES = {
+    'static': StaticMappingStrategy,
+}
\ No newline at end of file
diff --git a/pi_ldapproxy/usermapping.py b/pi_ldapproxy/usermapping.py
index 10e9c27..2015531 100644
--- a/pi_ldapproxy/usermapping.py
+++ b/pi_ldapproxy/usermapping.py
@@ -93,7 +93,7 @@ def resolve(self, dn):
             # TODO: Are there cases in which we can't unbind?
             yield client.unbind()
 
-MAPPING_STRATEGIES = {
+USER_MAPPING_STRATEGIES = {
     'match': MatchMappingStrategy,
     'lookup': LookupMappingStrategy,
 }
\ No newline at end of file

From 9a283637e0d33886a1ce5b25681a78cf058528be Mon Sep 17 00:00:00 2001
From: Friedrich Weber <friedrich.weber@netknights.it>
Date: Tue, 20 Jun 2017 09:39:42 +0200
Subject: [PATCH 05/13] Add preamble-based realm mapping strategy

Working on #13
---
 example-proxy.ini            |  5 ++++-
 pi_ldapproxy/realmmapping.py | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/example-proxy.ini b/example-proxy.ini
index 76cc244..1490553 100644
--- a/example-proxy.ini
+++ b/example-proxy.ini
@@ -74,4 +74,7 @@ value-prefix = App-
 [realm-mapping]
 strategy = static
 # Realm to use for authentication (may also be left blank)
-realm =
\ No newline at end of file
+realm =
+# strategy = preamble
+# [[mappings]]
+# ownCloud = somerealm
\ No newline at end of file
diff --git a/pi_ldapproxy/realmmapping.py b/pi_ldapproxy/realmmapping.py
index 62e708a..aa2052e 100644
--- a/pi_ldapproxy/realmmapping.py
+++ b/pi_ldapproxy/realmmapping.py
@@ -85,6 +85,22 @@ def __init__(self, factory, config):
     def resolve(self, dn):
         return self.realm
 
+
+class PreambleMappingStrategy(RealmMappingStrategy):
+    def __init__(self, factory, config):
+        RealmMappingStrategy.__init__(self, factory, config)
+        self.mappings = config['mappings']
+
+    def resolve(self, dn):
+        marker = self.factory.preamble_cache.get_cached_marker(dn) # TODO: preamble cache might be None
+        if marker is None:
+            raise RealmMappingError('No preamble for dn={dn!r}'.format(dn=dn))
+        realm = self.mappings.get(marker)
+        if realm is None:
+            raise RealmMappingError('No mapping for marker={marker!r}'.format(marker=marker))
+        return realm
+
 REALM_MAPPING_STRATEGIES = {
     'static': StaticMappingStrategy,
+    'preamble': PreambleMappingStrategy,
 }
\ No newline at end of file

From 67bb867c0ccf10572b5e2ed16f87a31b00cb2f36 Mon Sep 17 00:00:00 2001
From: Friedrich Weber <friedrich.weber@netknights.it>
Date: Tue, 20 Jun 2017 09:58:27 +0200
Subject: [PATCH 06/13] Add scenario 5) incorporating an app marker

Working on #13
---
 ...rvice-account-user-multiple-bind-search.py |  2 +-
 scenarios/5-multiple-search-app-marker.py     | 85 +++++++++++++++++++
 scenarios/common.py                           |  5 +-
 scenarios/example-config.ini                  |  4 +-
 4 files changed, 92 insertions(+), 4 deletions(-)
 create mode 100644 scenarios/5-multiple-search-app-marker.py

diff --git a/scenarios/4-service-account-user-multiple-bind-search.py b/scenarios/4-service-account-user-multiple-bind-search.py
index ac14882..45ffb5e 100644
--- a/scenarios/4-service-account-user-multiple-bind-search.py
+++ b/scenarios/4-service-account-user-multiple-bind-search.py
@@ -3,7 +3,7 @@
 requests.
 
 Given a username, the app uses a service account to find the user's DN. For that, it performs an LDAP bind followed
-by an LDAP server. After that, it issues a bind on behalf of the user and uses an LDAP search under the user's
+by an LDAP search. After that, it issues a bind on behalf of the user and uses an LDAP search under the user's
 context to retrieve profile information of the user. This is done twice during a 3-second timeframe.
 
 This will only work if the LDAP proxy caches bind requests.
diff --git a/scenarios/5-multiple-search-app-marker.py b/scenarios/5-multiple-search-app-marker.py
new file mode 100644
index 0000000..15ceb2b
--- /dev/null
+++ b/scenarios/5-multiple-search-app-marker.py
@@ -0,0 +1,85 @@
+"""
+Scenario 5) App uses a service account to look up user's DN. The search request's filter contains a marker which the
+LDAP proxy uses to identify the requesting application.
+Afterwards, the user sends *two* bind and search requests.
+
+Given a username, the app uses a service account to find the user's DN. For that, it performs an LDAP bind followed
+by an LDAP search. After that, it issues a bind on behalf of the user and uses an LDAP search under the user's
+context to retrieve profile information of the user. This is done twice during a 3-second timeframe.
+
+This will only work if the LDAP proxy caches bind requests.
+We cannot determine whether realm mapping is carried out correctly on the client side.
+"""
+from pprint import pprint
+
+import ldap3
+import configobj
+import time
+
+from common import lookup_user
+
+def perform_login_search(dn, password, ldap_server):
+    conn = ldap3.Connection(ldap_server, user=dn, password=password)
+    print 'Bind with password {!r} ...'.format(password),
+    result = conn.bind()
+    if result:
+        print 'Successful bind!'
+        # Fetch user information
+        conn.search(dn, '(objectClass=*)', attributes=ldap3.ALL_ATTRIBUTES)
+        if len(conn.entries) != 1:
+            raise RuntimeError('Expected one entry, found {}!'.format(len(conn.entries)))
+        entry = conn.entries[0]
+        return {
+            'success': True,
+            'displayName': entry.displayName.value,
+        }
+    else:
+        print 'Bind FAILED!'
+        return {
+            'success': False,
+        }
+
+def login(username, password, ldap_server, service_account_dn, service_account_password,
+          base_dn, loginname_attribute, wait_seconds, marker_filter):
+    """
+    Given username, password and a LDAP configuration, attempt a login.
+    :param username: login name of the user
+    :param password: supplied password
+    :param ldap_server: LDAP server IP
+    :param service_account_dn: Distinguished Name of the service account
+    :param service_account_password: Password of the service account
+    :param base_dn: the base DN under which user search should be performed
+    :param loginname_attribute: the attribute which contains the login name
+    :param wait_seconds: Wait a specific number of seconds before issuing the second user bind request.
+    :param marker_filter: something like "objectclass=App-something" to implement an app marker
+    :return: dictionary with boolean key 'success'. In case of success, it also contains user information.
+    """
+    dn = lookup_user(username, ldap_server, service_account_dn, service_account_password,
+                     base_dn, loginname_attribute, '(|({attr}={username})(%s))' % marker_filter)
+    print 'Given username {!r}, looked up dn: {!r}'.format(username, dn)
+    print '[1] Connecting to LDAP server {!r} ...'.format(ldap_server)
+    result1 = perform_login_search(dn, password, ldap_server)
+    if not result1['success']:
+        print 'exiting ...'
+        return result1
+    print 'Waiting for {!r} seconds ...'.format(wait_seconds)
+    time.sleep(wait_seconds)
+    print '[2] Connecting to LDAP server {!r} ...'.format(ldap_server)
+    result2 = perform_login_search(dn, password, ldap_server)
+    return result2
+
+if __name__ == '__main__':
+    with open('config.ini') as f:
+        config = configobj.ConfigObj(f)
+    password = config['password']
+    if not password:
+        password = raw_input('Password? ')
+    pprint(login(config['username'],
+                 password,
+                 config['ldap-server'],
+                 config['service-account-dn'],
+                 config['service-account-password'],
+                 config['base-dn'],
+                 config['loginname-attribute'],
+                 int(config['wait-seconds']),
+                 config['marker-filter']))
\ No newline at end of file
diff --git a/scenarios/common.py b/scenarios/common.py
index dfe90fc..3b01eda 100644
--- a/scenarios/common.py
+++ b/scenarios/common.py
@@ -15,7 +15,8 @@ def construct_dn(username, base_dn, uid_attribute):
     )
 
 
-def lookup_user(username, ldap_server, service_account_dn, service_account_password, base_dn, loginname_attribute):
+def lookup_user(username,ldap_server, service_account_dn, service_account_password,
+                base_dn, loginname_attribute, filter_template='({attr}={username})'):
     """
     Given an user-provided username, lookup the user's DN. If the user couldn't be found, raise a RuntimeError.
     :param username: login name
@@ -31,7 +32,7 @@ def lookup_user(username, ldap_server, service_account_dn, service_account_passw
     if result:
         print '[Service Account] Successful bind!'
         conn.search(base_dn,
-                    '({attr}={username})'.format(attr=loginname_attribute, username=username),
+                    filter_template.format(attr=loginname_attribute, username=username),
                     attributes=['cn'])
         print '[Service Account] Looking for entry that satisfies {attr}={username}'.format(
             attr=loginname_attribute,
diff --git a/scenarios/example-config.ini b/scenarios/example-config.ini
index 39aeae2..a44403c 100644
--- a/scenarios/example-config.ini
+++ b/scenarios/example-config.ini
@@ -12,4 +12,6 @@ loginname-attribute = sAMAccountName
 service-account-dn = "cn=service,cn=users,dc=test,dc=intranet"
 service-account-password = ""
 # Scenario 4: Wait some seconds between bind requests
-wait-seconds = 2
\ No newline at end of file
+wait-seconds = 2
+# Scenario 5: Include an app marker
+marker-filter = "objectclass=App-something"
\ No newline at end of file

From f103c351b841a2b44f0b2fc3718a8da74e46539e Mon Sep 17 00:00:00 2001
From: Friedrich Weber <friedrich.weber@netknights.it>
Date: Tue, 20 Jun 2017 10:19:19 +0200
Subject: [PATCH 07/13] Restructure mapping step

This makes for a nicer diff.
Working on #13
---
 pi_ldapproxy/proxy.py | 53 +++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/pi_ldapproxy/proxy.py b/pi_ldapproxy/proxy.py
index a00cc3e..e938ef5 100644
--- a/pi_ldapproxy/proxy.py
+++ b/pi_ldapproxy/proxy.py
@@ -71,40 +71,39 @@ def authenticate_bind_request(self, request):
         result = (False, '')
         try:
             user = yield self.factory.resolve_user(request.dn)
+            realm = yield self.factory.resolve_realm(request.dn)
         except UserMappingError:
             # User could not be found
             log.info('Could not resolve {dn!r} to user', dn=request.dn)
             result = (False, 'Invalid user.')
+        except RealmMappingError:
+            # Realm could not be mapped
+            log.info('Could not resolve {dn!r} to realm', dn=request.dn)
+            # TODO: too much information revealed?
+            result = (False, 'Could not determine realm.')
         else:
-            try:
-                realm = yield self.factory.resolve_realm(request.dn)
-            except RealmMappingError:
-                log.info('Could not resolve {dn!r} to realm', dn=request.dn)
-                # TODO: too much information revealed?
-                result = (False, 'Could not determine realm.')
-            else:
-                log.info('Resolved {dn!r} to {user!r}@{realm!r}', dn=request.dn, user=user, realm=realm)
-                password = request.auth
-                response = yield self.request_validate(self.factory.validate_url,
-                                                       user,
-                                                       realm,
-                                                       password)
-                json_body = yield readBody(response)
-                if response.code == 200:
-                    body = json.loads(json_body)
-                    if body['result']['status']:
-                        if body['result']['value']:
-                            result = (True, '')
-                            # TODO: Is this the right place to bind the service user?
-                            if self.factory.bind_service_account:
-                                yield self.bind_service_account()
-                                self.bound = True
-                        else:
-                            result = (False, 'Failed to authenticate.')
+            log.info('Resolved {dn!r} to {user!r}@{realm!r}', dn=request.dn, user=user, realm=realm)
+            password = request.auth
+            response = yield self.request_validate(self.factory.validate_url,
+                                                   user,
+                                                   realm,
+                                                   password)
+            json_body = yield readBody(response)
+            if response.code == 200:
+                body = json.loads(json_body)
+                if body['result']['status']:
+                    if body['result']['value']:
+                        result = (True, '')
+                        # TODO: Is this the right place to bind the service user?
+                        if self.factory.bind_service_account:
+                            yield self.bind_service_account()
+                            self.bound = True
                     else:
-                        result = (False, 'Failed to authenticate. privacyIDEA error.')
+                        result = (False, 'Failed to authenticate.')
                 else:
-                    result = (False, 'Failed to authenticate. Wrong HTTP response ({})'.format(response.code))
+                    result = (False, 'Failed to authenticate. privacyIDEA error.')
+            else:
+                result = (False, 'Failed to authenticate. Wrong HTTP response ({})'.format(response.code))
         defer.returnValue(result)
 
     def send_bind_response(self, result, request, reply):

From 26b08b2f5eef4f71e1bc147a5ed6b92b83799742 Mon Sep 17 00:00:00 2001
From: Friedrich Weber <friedrich.weber@netknights.it>
Date: Tue, 20 Jun 2017 14:17:49 +0200
Subject: [PATCH 08/13] Update unit tests to new config file structure

Working on #13
---
 pi_ldapproxy/test/util.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/pi_ldapproxy/test/util.py b/pi_ldapproxy/test/util.py
index 5b06371..ef2ca68 100644
--- a/pi_ldapproxy/test/util.py
+++ b/pi_ldapproxy/test/util.py
@@ -41,7 +41,6 @@
 BASE_CONFIG = """
 [privacyidea]
 instance = http://example.com
-realm = default
 
 [ldap-backend]
 endpoint = tcp:host=example.com:port=1337:timeout=1
@@ -64,6 +63,13 @@
 strategy = match
 pattern = "uid=([^,]+),cn=users,dc=test,dc=local"
 
+[realm-mapping]
+strategy = static
+realm = default
+
+[preamble-cache]
+enabled = false
+
 [bind-cache]
 enabled = false
 """
@@ -72,7 +78,7 @@ def load_test_config():
     config = configobj.ConfigObj(BASE_CONFIG.splitlines(), configspec=CONFIG_SPEC.splitlines())
     validator = validate.Validator()
     result = config.validate(validator, preserve_errors=True)
-    assert result
+    assert result == True, "Invalid test config"
     return config
 
 class ProxyTestCase(twisted.trial.unittest.TestCase):

From 00e8310280dd507473cd76d13c4a239c82600c40 Mon Sep 17 00:00:00 2001
From: Friedrich Weber <friedrich.weber@netknights.it>
Date: Tue, 20 Jun 2017 15:16:32 +0200
Subject: [PATCH 09/13] Add some source-code documentation.

Working on #13
---
 pi_ldapproxy/preamblecache.py | 47 ++++++++++++++++++++++++++++-------
 pi_ldapproxy/proxy.py         | 10 +++++++-
 pi_ldapproxy/realmmapping.py  | 33 ++++++++++++++++++++++--
 3 files changed, 78 insertions(+), 12 deletions(-)

diff --git a/pi_ldapproxy/preamblecache.py b/pi_ldapproxy/preamblecache.py
index c4bcb47..4ec41dc 100644
--- a/pi_ldapproxy/preamblecache.py
+++ b/pi_ldapproxy/preamblecache.py
@@ -4,18 +4,35 @@
 log = Logger()
 
 class PreambleCache(object):
+    """
+    The preamble cache stores the association of a DN with a so-called "app marker" for a specific timeframe.
+    """
     # (see http://twistedmatrix.com/documents/current/core/howto/trial.html)
     callLater = reactor.callLater
 
     def __init__(self, timeout):
+        """
+        :param timeout: The association is kept in the cache for this timeframe
+        """
         self.timeout = timeout
 
         #: Map of dn to tuples (app marker, insertion timestamp)
         self._preambles = {}
 
     def add_to_cache(self, dn, marker):
+        """
+        Add the entry to the preamble cache. It will be automatically removed after ``timeout`` seconds.
+        If an entry for ``dn`` (with any marker) already exists, it will be overwritten.
+        Keep in mind that removal will then provoke a "Removal from preamble
+        cache failed: ... mapped to ... " log message!
+        If an entry for ``dn`` with the same marker exists, the eviction timeout will *not*
+        be extended if it is added again.
+        :param dn: DN
+        :param marker: App marker (a string)
+        """
         if dn in self._preambles:
-            log.info('Entry {dn!r} already cached (marker={marker!r}), overwriting ...'.format(dn=dn, marker=marker))
+            log.info('Entry {dn!r} already cached {marker!r}, overwriting ...',
+                     dn=dn, marker=self._preambles[dn])
         current_time = reactor.seconds()
         log.info('Adding to preamble cache: dn={dn!r}, marker={marker!r}, time={time!r}',
                  dn=dn, time=current_time, marker=marker)
@@ -23,28 +40,40 @@ def add_to_cache(self, dn, marker):
         self.callLater(self.timeout, self.remove_from_cache, dn, marker)
 
     def remove_from_cache(self, dn, marker):
+        """
+        Remove the entry from the preamble cache. If the DN is mapped to a different marker, a warning is emitted
+        and the entry is *not* removed! If the entry does not exist in the preamble cache, a message is
+        written to the log.
+        :param dn: DN
+        :param marker: App marker (a string)
+        """
         if dn in self._preambles:
             stored_marker, stored_timestamp = self._preambles[dn]
             if stored_marker == marker:
                 del self._preambles[dn]
-                log.info('Removed {!r}/{!r} from preamble cache'.format(dn, marker))
+                log.info('Removed {dn!r}/{marker!r} from preamble cache', dn=dn, marker=marker)
             else:
-                log.warn('Removal from preamble cache failed: {!r} mapped to {!r}, not {!r}'.format(
-                    dn, stored_marker, marker
-                ))
+                log.warn('Removal from preamble cache failed: {dn!r} mapped to {stored!r}, not {marker!r}',
+                         dn=dn, stored=stored_marker, marker=marker)
         else:
-            log.info('Removal from preamble cache failed, as dn={!r} is not cached'.format(dn))
+            log.info('Removal from preamble cache failed, as dn={dn!r} is not cached', dn=dn)
 
     def get_cached_marker(self, dn):
+        """
+        Retrieve the cached marker for the distinguished name ``dn``. This actually checks that the stored entry
+        is still valid. If ``dn`` is not found in the cache, ``None`` is returned and a message is written to the log.
+        :param dn: DN
+        :return: string or None
+        """
         if dn in self._preambles:
             marker, timestamp = self._preambles[dn]
             current_time = reactor.seconds()
             if current_time - timestamp < self.timeout:
                 return marker
             else:
-                log.warn('Inconsistent preamble cache: dn={dn!r}, inserted={inserted!r}, current={current!r}'.format(
+                log.warn('Inconsistent preamble cache: dn={dn!r}, inserted={inserted!r}, current={current!r}',
                     dn=dn, inserted=timestamp, current=current_time
-                ))
+                )
         else:
-            log.info('No entry in preamble cache for dn={dn!r}'.format(dn=dn))
+            log.info('No entry in preamble cache for dn={dn!r}', dn=dn)
         return None
diff --git a/pi_ldapproxy/proxy.py b/pi_ldapproxy/proxy.py
index e938ef5..7581c13 100644
--- a/pi_ldapproxy/proxy.py
+++ b/pi_ldapproxy/proxy.py
@@ -318,6 +318,14 @@ def finalize_authentication(self, dn, password):
             self.bind_cache.add_to_cache(dn, password)
 
     def process_search_response(self, request, response):
+        """
+        Called when ``response`` is sent in response to ``request``. If the preamble cache is enabled,
+        ``detect_login_preamble`` is invoked in order to detect a login preamble. If one was detected,
+        it is added to the preamble cache.
+        :param request: LDAPSearchRequest
+        :param response: LDAPSearchResultEntry or LDAPSearchResultDone
+        :return:
+        """
         if self.preamble_cache is not None:
             result = detect_login_preamble(request,
                                            response,
@@ -325,7 +333,7 @@ def process_search_response(self, request, response):
                                            self.preamble_cache_value_prefix)
             if result is not None:
                 dn, marker = result
-                log.info('Detected login preamble: dn={dn!r}, marker={marker!r}'.format(dn=dn, marker=marker))
+                log.info('Detected login preamble: dn={dn!r}, marker={marker!r}', dn=dn, marker=marker)
                 self.preamble_cache.add_to_cache(dn, marker)
 
     def is_bind_cached(self, dn, password):
diff --git a/pi_ldapproxy/realmmapping.py b/pi_ldapproxy/realmmapping.py
index aa2052e..2dec1d1 100644
--- a/pi_ldapproxy/realmmapping.py
+++ b/pi_ldapproxy/realmmapping.py
@@ -1,5 +1,6 @@
 from ldaptor.protocols.pureldap import LDAPFilter_and, LDAPFilter_or, LDAPFilter_equalityMatch, LDAPSearchRequest, \
     LDAPSearchResultEntry
+from twisted.internet import defer
 from twisted.logger import Logger
 
 log = Logger()
@@ -78,27 +79,55 @@ def resolve(self, dn):
 
 
 class StaticMappingStrategy(RealmMappingStrategy):
+    """
+    `static` mapping strategy: Simply assign the same static realm to all authentication request.
+
+    Configuration:
+        `realm` contains the realm name (can also be empty)
+
+    """
     def __init__(self, factory, config):
         RealmMappingStrategy.__init__(self, factory, config)
         self.realm = config['realm']
 
     def resolve(self, dn):
-        return self.realm
+        return defer.succeed(self.realm)
 
 
 class PreambleMappingStrategy(RealmMappingStrategy):
+    """
+    `preamble` mapping strategy: Look up recent login preambles to find the correct realm.
+    If you use this mapping strategy, make sure the preamble cache is enabled
+    (see `[preamble-cache]`).
+
+    Configuration:
+        `mappings` is a subsection which maps app markers (as witnessed in LDAP search requests)
+        to realm names.
+
+        e.g.:
+
+            [realm-mapping]
+            strategy = preamble
+
+            [[mappings]]
+            myapp-marker = myapp_realm
+    """
     def __init__(self, factory, config):
         RealmMappingStrategy.__init__(self, factory, config)
         self.mappings = config['mappings']
 
     def resolve(self, dn):
+        """
+        Look up ``dn`` in the preamble cache, find the associated marker, look up the assocaited
+        realm in the mapping config, return it.
+        """
         marker = self.factory.preamble_cache.get_cached_marker(dn) # TODO: preamble cache might be None
         if marker is None:
             raise RealmMappingError('No preamble for dn={dn!r}'.format(dn=dn))
         realm = self.mappings.get(marker)
         if realm is None:
             raise RealmMappingError('No mapping for marker={marker!r}'.format(marker=marker))
-        return realm
+        return defer.succeed(realm)
 
 REALM_MAPPING_STRATEGIES = {
     'static': StaticMappingStrategy,

From b25df8d64091dd3745ba5d83624984b0409c84ae Mon Sep 17 00:00:00 2001
From: Friedrich Weber <friedrich.weber@netknights.it>
Date: Tue, 20 Jun 2017 20:22:13 +0200
Subject: [PATCH 10/13] Only detect preambles if exactly one result was
 returned

Working on #13
---
 pi_ldapproxy/proxy.py        | 32 +++++++++++++++++++++++++++-----
 pi_ldapproxy/realmmapping.py |  1 -
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/pi_ldapproxy/proxy.py b/pi_ldapproxy/proxy.py
index 7581c13..de2a910 100644
--- a/pi_ldapproxy/proxy.py
+++ b/pi_ldapproxy/proxy.py
@@ -32,8 +32,20 @@ class ProxyError(Exception):
 VALIDATE_URL_TEMPLATE = '{}validate/check'
 
 class TwoFactorAuthenticationProxy(ProxyBase):
-    #: Specifies whether we have sent a bind request to the LDAP backend at some point
-    bound = False
+    def __init__(self):
+        ProxyBase.__init__(self)
+        #: Specifies whether we have sent a bind request to the LDAP backend at some point
+        self.bound = False
+        #: If we are currently processing a search request, this stores the last entry
+        #: sent during its response. Otherwise, it is None.
+        self.last_search_response_entry = None
+        #: If we are currently processing a search request, this stores the total number of
+        #: entries sent during its response.
+        # Why do we have these two attributes here? For preamble detection, we need to make sure
+        # that the search request returns only one entry. To achieve that, we could store all entries
+        # in a list. However, this introduces unnecessary space overhead (e.g. if the app queries
+        # all users). Thus, we only store the last entry and the total entry count.
+        self.search_response_entries = 0
 
     def request_validate(self, url, user, realm, password):
         """
@@ -153,9 +165,19 @@ def handleProxiedResponse(self, response, request, controls):
         """
         # Try to detect login preamble
         if isinstance(request, pureldap.LDAPSearchRequest):
-            # TODO: Read attribute and value prefix from config
-            # TODO: Check that this is connection is bound to the service account?
-            self.factory.process_search_response(request, response)
+            # If we are sending back a search result entry, we just save it for preamble detection
+            # and count the total number of search result entries.
+            if isinstance(response, pureldap.LDAPSearchResultEntry):
+                self.last_search_response_entry = response
+                self.search_response_entries += 1
+            elif isinstance(response, pureldap.LDAPSearchResultDone):
+                # only check for preambles if we returned exactly one search result entry
+                if self.search_response_entries == 1:
+                    # TODO: Check that this is connection is bound to the service account?
+                    self.factory.process_search_response(request, self.last_search_response_entry)
+                # reset counter and storage
+                self.search_response_entries = 0
+                self.last_search_response_entry = None
         return response
 
     def handleBeforeForwardRequest(self, request, controls, reply):
diff --git a/pi_ldapproxy/realmmapping.py b/pi_ldapproxy/realmmapping.py
index 2dec1d1..ec4966d 100644
--- a/pi_ldapproxy/realmmapping.py
+++ b/pi_ldapproxy/realmmapping.py
@@ -45,7 +45,6 @@ def detect_login_preamble(request, response, attribute='objectclass', value_pref
     if isinstance(request, LDAPSearchRequest) and request.filter:
         # TODO: Check base dn?
         marker = find_app_marker(request.filter, attribute, value_prefix)
-        # TODO: This will be called multiple times for the same search request!
         # i.e. we do not notice if the response has >1 entries
         if marker is not None and isinstance(response, LDAPSearchResultEntry):
             return (response.objectName, marker)

From 79edcb6ef000d78f3d4cfdca59bd6fb84c76ff6d Mon Sep 17 00:00:00 2001
From: Friedrich Weber <friedrich.weber@netknights.it>
Date: Tue, 20 Jun 2017 20:58:23 +0200
Subject: [PATCH 11/13] Give more information if realm mapping fails

Working on #13
---
 pi_ldapproxy/proxy.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pi_ldapproxy/proxy.py b/pi_ldapproxy/proxy.py
index de2a910..e5194bf 100644
--- a/pi_ldapproxy/proxy.py
+++ b/pi_ldapproxy/proxy.py
@@ -88,9 +88,9 @@ def authenticate_bind_request(self, request):
             # User could not be found
             log.info('Could not resolve {dn!r} to user', dn=request.dn)
             result = (False, 'Invalid user.')
-        except RealmMappingError:
+        except RealmMappingError, e:
             # Realm could not be mapped
-            log.info('Could not resolve {dn!r} to realm', dn=request.dn)
+            log.info('Could not resolve {dn!r} to realm: {message!r}', dn=request.dn, message=e.message)
             # TODO: too much information revealed?
             result = (False, 'Could not determine realm.')
         else:

From 3bdf0234c78f2cf815aa57258d617204a38c5f1c Mon Sep 17 00:00:00 2001
From: Friedrich Weber <friedrich.weber@netknights.it>
Date: Wed, 21 Jun 2017 17:50:08 +0200
Subject: [PATCH 12/13] Rename preamble cache to app cache

Working on #13
---
 example-proxy.ini                             |  4 +-
 .../{preamblecache.py => appcache.py}         | 42 +++++++++----------
 pi_ldapproxy/config.py                        |  2 +-
 pi_ldapproxy/proxy.py                         | 27 ++++++------
 pi_ldapproxy/realmmapping.py                  | 18 ++++----
 pi_ldapproxy/test/util.py                     |  2 +-
 6 files changed, 47 insertions(+), 48 deletions(-)
 rename pi_ldapproxy/{preamblecache.py => appcache.py} (61%)

diff --git a/example-proxy.ini b/example-proxy.ini
index 1490553..c94539e 100644
--- a/example-proxy.ini
+++ b/example-proxy.ini
@@ -65,7 +65,7 @@ attribute = sAMAccountName
 enabled = false
 timeout = 3
 
-[preamble-cache]
+[app-cache]
 enabled = true
 timeout = 5
 attribute = objectclass
@@ -75,6 +75,6 @@ value-prefix = App-
 strategy = static
 # Realm to use for authentication (may also be left blank)
 realm =
-# strategy = preamble
+# strategy = app-cache
 # [[mappings]]
 # ownCloud = somerealm
\ No newline at end of file
diff --git a/pi_ldapproxy/preamblecache.py b/pi_ldapproxy/appcache.py
similarity index 61%
rename from pi_ldapproxy/preamblecache.py
rename to pi_ldapproxy/appcache.py
index 4ec41dc..a8e1d8d 100644
--- a/pi_ldapproxy/preamblecache.py
+++ b/pi_ldapproxy/appcache.py
@@ -3,9 +3,9 @@
 
 log = Logger()
 
-class PreambleCache(object):
+class AppCache(object):
     """
-    The preamble cache stores the association of a DN with a so-called "app marker" for a specific timeframe.
+    The app cache stores the association of a DN with a so-called "app marker" for a specific timeframe.
     """
     # (see http://twistedmatrix.com/documents/current/core/howto/trial.html)
     callLater = reactor.callLater
@@ -17,46 +17,46 @@ def __init__(self, timeout):
         self.timeout = timeout
 
         #: Map of dn to tuples (app marker, insertion timestamp)
-        self._preambles = {}
+        self._entries = {}
 
     def add_to_cache(self, dn, marker):
         """
-        Add the entry to the preamble cache. It will be automatically removed after ``timeout`` seconds.
+        Add the entry to the app cache. It will be automatically removed after ``timeout`` seconds.
         If an entry for ``dn`` (with any marker) already exists, it will be overwritten.
-        Keep in mind that removal will then provoke a "Removal from preamble
+        Keep in mind that removal will then provoke a "Removal from app
         cache failed: ... mapped to ... " log message!
         If an entry for ``dn`` with the same marker exists, the eviction timeout will *not*
         be extended if it is added again.
         :param dn: DN
         :param marker: App marker (a string)
         """
-        if dn in self._preambles:
+        if dn in self._entries:
             log.info('Entry {dn!r} already cached {marker!r}, overwriting ...',
-                     dn=dn, marker=self._preambles[dn])
+                     dn=dn, marker=self._entries[dn])
         current_time = reactor.seconds()
-        log.info('Adding to preamble cache: dn={dn!r}, marker={marker!r}, time={time!r}',
+        log.info('Adding to app cache: dn={dn!r}, marker={marker!r}, time={time!r}',
                  dn=dn, time=current_time, marker=marker)
-        self._preambles[dn] = (marker, current_time)
+        self._entries[dn] = (marker, current_time)
         self.callLater(self.timeout, self.remove_from_cache, dn, marker)
 
     def remove_from_cache(self, dn, marker):
         """
-        Remove the entry from the preamble cache. If the DN is mapped to a different marker, a warning is emitted
-        and the entry is *not* removed! If the entry does not exist in the preamble cache, a message is
+        Remove the entry from the app cache. If the DN is mapped to a different marker, a warning is emitted
+        and the entry is *not* removed! If the entry does not exist in the app cache, a message is
         written to the log.
         :param dn: DN
         :param marker: App marker (a string)
         """
-        if dn in self._preambles:
-            stored_marker, stored_timestamp = self._preambles[dn]
+        if dn in self._entries:
+            stored_marker, stored_timestamp = self._entries[dn]
             if stored_marker == marker:
-                del self._preambles[dn]
-                log.info('Removed {dn!r}/{marker!r} from preamble cache', dn=dn, marker=marker)
+                del self._entries[dn]
+                log.info('Removed {dn!r}/{marker!r} from app cache', dn=dn, marker=marker)
             else:
-                log.warn('Removal from preamble cache failed: {dn!r} mapped to {stored!r}, not {marker!r}',
+                log.warn('Removal from app cache failed: {dn!r} mapped to {stored!r}, not {marker!r}',
                          dn=dn, stored=stored_marker, marker=marker)
         else:
-            log.info('Removal from preamble cache failed, as dn={dn!r} is not cached', dn=dn)
+            log.info('Removal from app cache failed, as dn={dn!r} is not cached', dn=dn)
 
     def get_cached_marker(self, dn):
         """
@@ -65,15 +65,15 @@ def get_cached_marker(self, dn):
         :param dn: DN
         :return: string or None
         """
-        if dn in self._preambles:
-            marker, timestamp = self._preambles[dn]
+        if dn in self._entries:
+            marker, timestamp = self._entries[dn]
             current_time = reactor.seconds()
             if current_time - timestamp < self.timeout:
                 return marker
             else:
-                log.warn('Inconsistent preamble cache: dn={dn!r}, inserted={inserted!r}, current={current!r}',
+                log.warn('Inconsistent app cache: dn={dn!r}, inserted={inserted!r}, current={current!r}',
                     dn=dn, inserted=timestamp, current=current_time
                 )
         else:
-            log.info('No entry in preamble cache for dn={dn!r}', dn=dn)
+            log.info('No entry in app cache for dn={dn!r}', dn=dn)
         return None
diff --git a/pi_ldapproxy/config.py b/pi_ldapproxy/config.py
index 04d0443..40fed5b 100644
--- a/pi_ldapproxy/config.py
+++ b/pi_ldapproxy/config.py
@@ -27,7 +27,7 @@
 enabled = boolean
 timeout = integer(default=3)
 
-[preamble-cache]
+[app-cache]
 enabled = boolean
 timeout = integer(default=3)
 attribute = string(default='objectclass')
diff --git a/pi_ldapproxy/proxy.py b/pi_ldapproxy/proxy.py
index e5194bf..c927806 100644
--- a/pi_ldapproxy/proxy.py
+++ b/pi_ldapproxy/proxy.py
@@ -19,7 +19,7 @@
 
 from pi_ldapproxy.bindcache import BindCache
 from pi_ldapproxy.config import load_config
-from pi_ldapproxy.preamblecache import PreambleCache
+from pi_ldapproxy.appcache import AppCache
 from pi_ldapproxy.realmmapping import detect_login_preamble, REALM_MAPPING_STRATEGIES, RealmMappingError
 from pi_ldapproxy.usermapping import USER_MAPPING_STRATEGIES, UserMappingError
 
@@ -284,13 +284,13 @@ def __init__(self, config):
         else:
             self.bind_cache = None
 
-        enable_preamble_cache = config['preamble-cache']['enabled']
-        if enable_preamble_cache:
-            self.preamble_cache = PreambleCache(config['preamble-cache']['timeout'])
+        enable_app_cache = config['app-cache']['enabled']
+        if enable_app_cache:
+            self.app_cache = AppCache(config['app-cache']['timeout'])
         else:
-            self.preamble_cache = None
-        self.preamble_cache_attribute = config['preamble-cache']['attribute']
-        self.preamble_cache_value_prefix = config['preamble-cache']['value-prefix']
+            self.app_cache = None
+        self.app_cache_attribute = config['app-cache']['attribute']
+        self.app_cache_value_prefix = config['app-cache']['value-prefix']
 
         if config['ldap-backend']['test-connection']:
             self.test_connection()
@@ -341,22 +341,21 @@ def finalize_authentication(self, dn, password):
 
     def process_search_response(self, request, response):
         """
-        Called when ``response`` is sent in response to ``request``. If the preamble cache is enabled,
+        Called when ``response`` is sent in response to ``request``. If the app cache is enabled,
         ``detect_login_preamble`` is invoked in order to detect a login preamble. If one was detected,
-        it is added to the preamble cache.
+        the corresponding entry is added to the app cache.
         :param request: LDAPSearchRequest
         :param response: LDAPSearchResultEntry or LDAPSearchResultDone
         :return:
         """
-        if self.preamble_cache is not None:
+        if self.app_cache is not None:
             result = detect_login_preamble(request,
                                            response,
-                                           self.preamble_cache_attribute,
-                                           self.preamble_cache_value_prefix)
+                                           self.app_cache_attribute,
+                                           self.app_cache_value_prefix)
             if result is not None:
                 dn, marker = result
-                log.info('Detected login preamble: dn={dn!r}, marker={marker!r}', dn=dn, marker=marker)
-                self.preamble_cache.add_to_cache(dn, marker)
+                self.app_cache.add_to_cache(dn, marker)
 
     def is_bind_cached(self, dn, password):
         """
diff --git a/pi_ldapproxy/realmmapping.py b/pi_ldapproxy/realmmapping.py
index ec4966d..0efefac 100644
--- a/pi_ldapproxy/realmmapping.py
+++ b/pi_ldapproxy/realmmapping.py
@@ -93,11 +93,11 @@ def resolve(self, dn):
         return defer.succeed(self.realm)
 
 
-class PreambleMappingStrategy(RealmMappingStrategy):
+class AppCacheMappingStrategy(RealmMappingStrategy):
     """
-    `preamble` mapping strategy: Look up recent login preambles to find the correct realm.
-    If you use this mapping strategy, make sure the preamble cache is enabled
-    (see `[preamble-cache]`).
+    `app-cache` mapping strategy: Look up the app cache to find the correct realm.
+    If you use this mapping strategy, make sure the app cache is enabled
+    (see `[app-cache]`).
 
     Configuration:
         `mappings` is a subsection which maps app markers (as witnessed in LDAP search requests)
@@ -106,7 +106,7 @@ class PreambleMappingStrategy(RealmMappingStrategy):
         e.g.:
 
             [realm-mapping]
-            strategy = preamble
+            strategy = app-cache
 
             [[mappings]]
             myapp-marker = myapp_realm
@@ -117,12 +117,12 @@ def __init__(self, factory, config):
 
     def resolve(self, dn):
         """
-        Look up ``dn`` in the preamble cache, find the associated marker, look up the assocaited
+        Look up ``dn`` in the app cache, find the associated marker, look up the associated
         realm in the mapping config, return it.
         """
-        marker = self.factory.preamble_cache.get_cached_marker(dn) # TODO: preamble cache might be None
+        marker = self.factory.app_cache.get_cached_marker(dn) # TODO: app cache might be None
         if marker is None:
-            raise RealmMappingError('No preamble for dn={dn!r}'.format(dn=dn))
+            raise RealmMappingError('No entry in app cache for dn={dn!r}'.format(dn=dn))
         realm = self.mappings.get(marker)
         if realm is None:
             raise RealmMappingError('No mapping for marker={marker!r}'.format(marker=marker))
@@ -130,5 +130,5 @@ def resolve(self, dn):
 
 REALM_MAPPING_STRATEGIES = {
     'static': StaticMappingStrategy,
-    'preamble': PreambleMappingStrategy,
+    'app-cache': AppCacheMappingStrategy,
 }
\ No newline at end of file
diff --git a/pi_ldapproxy/test/util.py b/pi_ldapproxy/test/util.py
index ef2ca68..7705ace 100644
--- a/pi_ldapproxy/test/util.py
+++ b/pi_ldapproxy/test/util.py
@@ -67,7 +67,7 @@
 strategy = static
 realm = default
 
-[preamble-cache]
+[app-cache]
 enabled = false
 
 [bind-cache]

From c000ed1ce29a1eaa52b9b6c8946393caad6657aa Mon Sep 17 00:00:00 2001
From: Friedrich Weber <friedrich.weber@netknights.it>
Date: Wed, 21 Jun 2017 18:04:36 +0200
Subject: [PATCH 13/13] Add comments to example-proxy.ini

Working on #13
---
 example-proxy.ini | 45 +++++++++++++++++++++++++++++++++------------
 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/example-proxy.ini b/example-proxy.ini
index c94539e..be56da5 100644
--- a/example-proxy.ini
+++ b/example-proxy.ini
@@ -56,6 +56,23 @@ attribute = sAMAccountName
 #strategy = match
 #pattern = "cn=([^,]+),cn=users,dc=test,dc=local"
 
+[realm-mapping]
+# The following configures how the privacyIDEA realm of incoming authentication requests
+# is determined.
+# The `static` strategy assigns one realm to all authentication requests.
+strategy = static
+# Realm to use for authentication (may also be left blank to make privacyIDEA use its default realm)
+realm =
+# The `app-cache` strategy is more sophisticated: Once a bind request is received, it checks
+# whether the app cache contains a corresponding app marker (so be sure to enable the bind cache)
+# If yes, this app marker is mapped to a privacyIDEA realm according to the mapping defined below.
+# If the app cache contains no app marker or the app marker is not mapped to a realm,
+# the authentication request fails.
+# strategy = app-cache
+# [[mappings]]
+# This maps the app marker "someApp" to the privacyIDEA realm "somerealm".
+# someApp = somerealm
+
 [bind-cache]
 # If this setting is enabled, successful user bind requests are added to a so-called "bind cache" in which
 # they are kept for a specified time. During that time, incoming bind requests using the same credentials
@@ -66,15 +83,19 @@ enabled = false
 timeout = 3
 
 [app-cache]
-enabled = true
-timeout = 5
-attribute = objectclass
-value-prefix = App-
-
-[realm-mapping]
-strategy = static
-# Realm to use for authentication (may also be left blank)
-realm =
-# strategy = app-cache
-# [[mappings]]
-# ownCloud = somerealm
\ No newline at end of file
+# If this setting is enabled, the LDAP proxy maintains a so-called "app cache".
+# On user login, apps typically perform a LDAP search to locate the user. When
+# they have resolved the DN, they perform the actual bind.
+# We use the LDAP search to identify the app: For that, we add an expression like
+# `objectclass=App-someApp` to the LDAP filter (without changing the filter semantics).
+# We call `someApp` an "app marker". If the LDAP proxy witnesses a search request
+# whose filter contains an app marker and whose result has exactly one entry,
+# the DN of the entry (the user) as well as the app marker are stored in the app cache
+# for a specific timeframe. This information may be used by the `app-cache` realm mapping
+# strategy (see below).
+enabled = false
+#timeout = 5
+# Attribute containing the app marker
+#attribute = objectclass
+# Prefix of the app marker
+#value-prefix = App-
\ No newline at end of file