diff --git a/locales/en.json b/locales/en.json index c7f3d0085..74b62408e 100644 --- a/locales/en.json +++ b/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}'", diff --git a/src/authenticators/ldap_admin.py b/src/authenticators/ldap_admin.py index 31b2a7363..704816460 100644 --- a/src/authenticators/ldap_admin.py +++ b/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) diff --git a/src/tests/test_ldapauth.py b/src/tests/test_ldapauth.py index 7a09fff40..5e741fe0f 100644 --- a/src/tests/test_ldapauth.py +++ b/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") diff --git a/src/utils/ldap.py b/src/utils/ldap.py index 98c0fecf7..28ff8eebe 100644 --- a/src/utils/ldap.py +++ b/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', "