1 files changed, 30 insertions(+), 22 deletions(-)

M metasrht/auth/ldap.py

@@ 9,13 9,6 @@ from metasrht.auth.base import AuthMethod, get_user
 from metasrht.auth_validation import *
 from metasrht.types import User, UserType
 
-# FIXME: LDAP connection management could be improved:
-#   * We keep a connection (self.conn) open for browsing the LDAP tree,
-#     changing passwords and emails: what happen if this connection is closed
-#     (e.g. LDAP server restarts)? How can we detect if the connection is dead?
-#   * We open a new connection each time a valid username tries to login: could
-#     we reuse the same connection?
-#
 # FIXME: we assume user DN is formatted as follow: uid=$username,$user_base.
 # Should we make it configurable?
 


@@ 36,16 29,34 @@ class LDAPAuthMethod(AuthMethod):
         self.user_base = cfg('meta.sr.ht::auth::ldap', 'user-base')
         self.create_users = cfgb('meta.sr.ht::auth::ldap', 'create-users')
 
-        bind_dn = cfg('meta.sr.ht::auth::ldap', 'bind-dn')
-        bind_pw = cfg('meta.sr.ht::auth::ldap', 'bind-password')
+        self.bind_dn = cfg('meta.sr.ht::auth::ldap', 'bind-dn')
+        self.bind_pw = cfg('meta.sr.ht::auth::ldap', 'bind-password')
 
         # Initialize connection to LDAP server.
         try:
-            self.conn = ldap.initialize(self.server)
-            self.conn.protocol_version = ldap.VERSION3
-            self.conn.simple_bind_s(bind_dn, bind_pw)
+            self.conn = False
+            self.conn = self._get_ldap_conn()
         except Exception as e:
-            print("Could not query LDAP server: {}".format(e))
+            print("Could not query LDAP server {}: {}".format(self.server, e))
+
+    def _get_ldap_conn(self, retry=True):
+        if not self.conn:
+            self.conn = self.ldap.initialize(self.server)
+            self.conn.protocol_version = self.ldap.VERSION3
+
+        # Check if the connection is still alive - it would have been cut if
+        # the LDAP server restarted. We try again once, and raise an exception
+        # if it does not work.
+        try:
+            self.conn.simple_bind_s(self.bind_dn, self.bind_pw)
+        except self.ldap.SERVER_DOWN as e:
+            if retry:
+                self.conn = False
+                self._get_ldap_conn(False)
+            else:
+                raise(e)
+
+        return self.conn
 
     def get_dn_for(self, username: str) -> str:
         return "uid={},{}".format(username, self.user_base)


@@ 54,7 65,7 @@ class LDAPAuthMethod(AuthMethod):
         try:
             ldap_search_filter = "uid={}".format(username)
             import_attributes = ['uid', 'mail']
-            ldap_match = self.conn.search_s(
+            ldap_match = self._get_ldap_conn().search_s(
                     self.user_base,
                     self.ldap.SCOPE_SUBTREE,
                     ldap_search_filter,


@@ 109,16 120,13 @@ class LDAPAuthMethod(AuthMethod):
 
         # Query LDAP server.
         try:
-            user_conn = self.ldap.initialize(self.server)
-            user_conn.protocol_version = self.ldap.VERSION3
-            user_conn.simple_bind_s(self.get_dn_for(username), password)
-            user_conn.unbind()
+            self._get_ldap_conn().simple_bind_s(self.get_dn_for(username), password)
         except self.ldap.INVALID_CREDENTIALS:
             valid.error('Username or password incorrect')
             return False
         except Exception as e:
-            valid.error('Something went wrong while querying the'
-                    'authentication backend. Please try again later or contact'
+            valid.error('Something went wrong while querying the '
+                    'authentication backend. Please try again later or contact '
                     'your administrator.')
             return False
 


@@ 155,11 163,11 @@ class LDAPAuthMethod(AuthMethod):
 
     def set_user_email(self, user: User, email: str) -> bool:
         ldif = [(self.ldap.MOD_REPLACE, 'mail', str.encode(email))]
-        self.conn.modify_s(self.get_dn_for(user.username), ldif)
+        self._get_ldap_conn().modify_s(self.get_dn_for(user.username), ldif)
 
         user.email = email
         db.session.commit()
 
     def set_user_password(self, user: User, password: str) -> bool:
         audit_log("password reset", user=user)
-        self.conn.passwd_s(self.get_dn_for(user.username), None, password)
+        self._get_ldap_conn().passwd_s(self.get_dn_for(user.username), None, password)