admins: moar friskies?

locales/en.json

@@ -429,6 +429,7 @@
     "invalid_number_min": "Must be greater than {min}",
     "invalid_password": "Invalid password",
     "invalid_regex": "Invalid regex:'{regex}'",
+    "invalid_credentials": "Invalid password or username",
     "ip6tables_unavailable": "You cannot play with ip6tables here. You are either in a container or your kernel does not support it",
     "iptables_unavailable": "You cannot play with iptables here. You are either in a container or your kernel does not support it",
     "ldap_attribute_already_exists": "LDAP attribute '{attribute}' already exists with value '{value}'",

src/authenticators/ldap_admin.py

@@ -17,7 +17,7 @@ session_secret = random_ascii()
 logger = logging.getLogger("yunohost.authenticators.ldap_admin")
 
 LDAP_URI = "ldap://localhost:389"
-ADMIN_GROUP = "cn=admins,ou=groups,dc=yunohost,dc=org"
+ADMIN_GROUP = "cn=admins,ou=groups"
 AUTH_DN = "uid={uid},ou=users,dc=yunohost,dc=org"
 
 class Authenticator(BaseAuthenticator):
@@ -29,11 +29,27 @@ class Authenticator(BaseAuthenticator):
 
     def _authenticate_credentials(self, credentials=None):
 
-        admins = _get_ldap_interface().search(ADMIN_GROUP, attrs=["memberUid"])[0]["memberUid"]
+        try:
+            admins = _get_ldap_interface().search(ADMIN_GROUP, attrs=["memberUid"])[0].get("memberUid", [])
+        except ldap.SERVER_DOWN:
+            # ldap is down, attempt to restart it before really failing
+            logger.warning(m18n.n("ldap_server_is_down_restart_it"))
+            os.system("systemctl restart slapd")
+            time.sleep(10)  # waits 10 secondes so we are sure that slapd has restarted
 
-        uid, password = credentials.split(":", 1)
+            try:
+                admins = _get_ldap_interface().search(ADMIN_GROUP, attrs=["memberUid"])[0].get("memberUid", [])
+            except ldap.SERVER_DOWN:
+                raise YunohostError("ldap_server_down")
 
-        if uid not in admins:
+        try:
+            uid, password = credentials.split(":", 1)
+        except ValueError:
+            raise YunohostError("invalid_credentials")
+
+        # Here we're explicitly using set() which are handled as hash tables
+        # and should prevent timing attacks to find out the admin usernames?
+        if uid not in set(admins):
             raise YunohostError("invalid_credentials")
 
         dn = AUTH_DN.format(uid=uid)

src/tests/test_ldapauth.py

@@ -3,6 +3,8 @@ import os
 
 from yunohost.authenticators.ldap_admin import Authenticator as LDAPAuth
 from yunohost.tools import tools_rootpw
+from yunohost.user import user_create, user_list, user_update, user_delete
+from yunohost.domain import _get_maindomain
 
 from moulinette import m18n
 from moulinette.core import MoulinetteError
@@ -10,19 +12,52 @@ from moulinette.core import MoulinetteError
 
 def setup_function(function):
 
+    for u in user_list()["users"]:
+        user_delete(u, purge=True)
+
+    maindomain = _get_maindomain()
+
     if os.system("systemctl is-active slapd") != 0:
         os.system("systemctl start slapd && sleep 3")
 
-    tools_rootpw("yunohost", check_strength=False)
+    user_create("alice", "Alice", "White", maindomain, "Yunohost", admin=True)
+    user_create("bob", "Bob", "Snow", maindomain, "test123Ynh")
+
+
+def teardown_function():
+
+    os.system("systemctl is-active slapd || systemctl start slapd && sleep 5")
+
+    for u in user_list()["users"]:
+        user_delete(u, purge=True)
 
 
 def test_authenticate():
-    LDAPAuth().authenticate_credentials(credentials="yunohost")
+    LDAPAuth().authenticate_credentials(credentials="alice:Yunohost")
+
+
+def test_authenticate_with_no_user():
+
+    with pytest.raises(MoulinetteError):
+        LDAPAuth().authenticate_credentials(credentials="Yunohost")
+
+    with pytest.raises(MoulinetteError):
+        LDAPAuth().authenticate_credentials(credentials=":Yunohost")
+
+
+def test_authenticate_with_user_who_is_not_admin():
+
+    with pytest.raises(MoulinetteError) as exception:
+        LDAPAuth().authenticate_credentials(credentials="bob:test123Ynh")
+
+    translation = m18n.n("invalid_password")
+    expected_msg = translation.format()
+    assert expected_msg in str(exception)
 
 
 def test_authenticate_with_wrong_password():
     with pytest.raises(MoulinetteError) as exception:
-        LDAPAuth().authenticate_credentials(credentials="bad_password_lul")
+        LDAPAuth().authenticate_credentials(credentials="alice:bad_password_lul")
 
     translation = m18n.n("invalid_password")
     expected_msg = translation.format()
@@ -30,17 +65,15 @@ def test_authenticate_with_wrong_password():
 
 
 def test_authenticate_server_down(mocker):
-    os.system("systemctl stop slapd && sleep 3")
+    os.system("systemctl stop slapd && sleep 5")
 
     # Now if slapd is down, moulinette tries to restart it
     mocker.patch("os.system")
     mocker.patch("time.sleep")
     with pytest.raises(MoulinetteError) as exception:
-        LDAPAuth().authenticate_credentials(credentials="yunohost")
+        LDAPAuth().authenticate_credentials(credentials="alice:Yunohost")
 
-    translation = m18n.n("ldap_server_down")
-    expected_msg = translation.format()
-    assert expected_msg in str(exception)
+    assert "Unable to reach LDAP server" in str(exception)
 
 
 def test_authenticate_change_password():
@@ -50,10 +83,12 @@ def test_authenticate_change_password():
     tools_rootpw("plopette", check_strength=False)
 
     with pytest.raises(MoulinetteError) as exception:
-        LDAPAuth().authenticate_credentials(credentials="yunohost")
+        LDAPAuth().authenticate_credentials(credentials="alice:Yunohost")
 
     translation = m18n.n("invalid_password")
     expected_msg = translation.format()
     assert expected_msg in str(exception)
 
-    LDAPAuth().authenticate_credentials(credentials="plopette")
+    user_update("alice", password="plopette")
+
+    LDAPAuth().authenticate_credentials(credentials="alice:plopette")

src/utils/ldap.py

@@ -145,6 +145,8 @@ class LDAPInterface:
 
         try:
             result = self.con.search_s(base, ldap.SCOPE_SUBTREE, filter, attrs)
+        except ldap.SERVER_DOWN as e:
+            raise e
         except Exception as e:
             raise MoulinetteError(
                 "error during LDAP search operation with: base='%s', "