From 3df6ce17b6b13f1fe784a3bb9e5b782fc541eb2a Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 13 Sep 2019 20:34:30 +0200 Subject: [PATCH] Properly handle all those errors >.> ... --- locales/en.json | 1 + src/yunohost/tests/test_permission.py | 3 +-- src/yunohost/tests/test_user-group.py | 17 ++++++++------ src/yunohost/user.py | 33 +++++++++++++++++++++------ 4 files changed, 38 insertions(+), 16 deletions(-) diff --git a/locales/en.json b/locales/en.json index d60a432ab..c48532ed7 100644 --- a/locales/en.json +++ b/locales/en.json @@ -555,6 +555,7 @@ "upnp_disabled": "UPnP has been disabled", "upnp_enabled": "UPnP has been enabled", "upnp_port_open_failed": "Unable to open UPnP ports", + "user_already_exists": "User {user} already exists", "user_created": "The user has been created", "user_creation_failed": "Unable to create user {user}: {error}", "user_deleted": "The user has been deleted", diff --git a/src/yunohost/tests/test_permission.py b/src/yunohost/tests/test_permission.py index 2df6362a9..8db1ae825 100644 --- a/src/yunohost/tests/test_permission.py +++ b/src/yunohost/tests/test_permission.py @@ -1,6 +1,5 @@ import pytest -from moulinette.core import MoulinetteError from yunohost.app import app_install, app_remove, app_change_url, app_list from yunohost.user import user_list, user_info, user_create, user_delete, user_update, \ @@ -211,7 +210,7 @@ def test_permission_create_already_existing(): permission_create("wiki.main") def test_permission_delete_doesnt_existing(): - with pytest.raises(MoulinetteError): + with pytest.raises(YunohostError): permission_delete("doesnt.exist", force=True) res = user_permission_list()['permissions'] diff --git a/src/yunohost/tests/test_user-group.py b/src/yunohost/tests/test_user-group.py index 88644c3e6..30bdeb017 100644 --- a/src/yunohost/tests/test_user-group.py +++ b/src/yunohost/tests/test_user-group.py @@ -1,6 +1,5 @@ import pytest -from moulinette.core import MoulinetteError from yunohost.user import user_list, user_info, user_create, user_delete, user_update, \ user_group_list, user_group_create, user_group_delete, user_group_update, user_group_info from yunohost.domain import _get_maindomain @@ -102,19 +101,23 @@ def test_del_group(): # def test_create_user_with_mail_address_already_taken(): - with pytest.raises(MoulinetteError): + with pytest.raises(YunohostError): user_create("alice2", "Alice", "White", "alice@" + maindomain, "test123Ynh") def test_create_user_with_password_too_simple(): - with pytest.raises(MoulinetteError): + with pytest.raises(YunohostError): user_create("other", "Alice", "White", "other@" + maindomain, "12") def test_create_user_already_exists(): - with pytest.raises(MoulinetteError): + with pytest.raises(YunohostError): user_create("alice", "Alice", "White", "other@" + maindomain, "test123Ynh") +def test_update_user_with_mail_address_already_taken(): + with pytest.raises(YunohostError): + user_update("bob", add_mailalias="alice@" + maindomain) + def test_del_user_that_does_not_exist(): - with pytest.raises(MoulinetteError): + with pytest.raises(YunohostError): user_delete("doesnt_exist") def test_create_group_all_users(): @@ -124,7 +127,7 @@ def test_create_group_all_users(): def test_create_group_already_exists(): # Check groups already exist (regular groups) - with pytest.raises(MoulinetteError): + with pytest.raises(YunohostError): user_group_create("dev") def test_del_group_all_users(): @@ -132,7 +135,7 @@ def test_del_group_all_users(): user_group_delete("all_users") def test_del_group_that_does_not_exist(): - with pytest.raises(MoulinetteError): + with pytest.raises(YunohostError): user_group_delete("doesnt_exist") # diff --git a/src/yunohost/user.py b/src/yunohost/user.py index fbd15018c..c6413d7e1 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -127,12 +127,18 @@ def user_create(operation_logger, username, firstname, lastname, mail, password, ldap = _get_ldap_interface() + if username in user_list()["users"]: + raise YunohostError("user_already_exists", user=username) + # Validate uniqueness of username and mail in LDAP - ldap.validate_uniqueness({ - 'uid': username, - 'mail': mail, - 'cn': username - }) + try: + ldap.validate_uniqueness({ + 'uid': username, + 'mail': mail, + 'cn': username + }) + except Exception as e: + raise YunohostError('user_creation_failed', user=username, error=e) # Validate uniqueness of username in system users all_existing_usernames = {x.pw_name for x in pwd.getpwall()} @@ -249,6 +255,9 @@ def user_delete(operation_logger, username, purge=False): from yunohost.utils.ldap import _get_ldap_interface from yunohost.permission import permission_sync_to_user + if username not in user_list()["users"]: + raise YunohostError('user_unknown', user=username) + operation_logger.start() user_group_update("all_users", remove=username, force=True, sync_perm=False) @@ -340,7 +349,10 @@ def user_update(operation_logger, username, firstname=None, lastname=None, mail= 'webmaster@' + main_domain, 'postmaster@' + main_domain, ] - ldap.validate_uniqueness({'mail': mail}) + try: + ldap.validate_uniqueness({'mail': mail}) + except Exception as e: + raise YunohostError('user_update_failed', user=username, error=e) if mail[mail.find('@') + 1:] not in domains: raise YunohostError('mail_domain_unknown', domain=mail[mail.find('@') + 1:]) if mail in aliases: @@ -353,7 +365,10 @@ def user_update(operation_logger, username, firstname=None, lastname=None, mail= if not isinstance(add_mailalias, list): add_mailalias = [add_mailalias] for mail in add_mailalias: - ldap.validate_uniqueness({'mail': mail}) + try: + ldap.validate_uniqueness({'mail': mail}) + except Exception as e: + raise YunohostError('user_update_failed', user=username, error=e) if mail[mail.find('@') + 1:] not in domains: raise YunohostError('mail_domain_unknown', domain=mail[mail.find('@') + 1:]) user['mail'].append(mail) @@ -611,6 +626,10 @@ def user_group_delete(operation_logger, groupname, force=False, sync_perm=True): from yunohost.permission import permission_sync_to_user from yunohost.utils.ldap import _get_ldap_interface + existing_groups = user_group_list()['groups'].keys() + if groupname not in existing_groups: + raise YunohostError('group_unknown', group=groupname) + # Refuse to delete primary groups of a user (e.g. group 'sam' related to user 'sam') # without the force option... #