Merge pull request #962 from ekhae/enh-1365-mail-user-creation-ux

[enh] c.f. issue 1365, asking a mail address during user creation is confusing ux, people try enter their existing mail addresses

data/actionsmap/yunohost.yml

@@ -99,13 +99,7 @@ user:
                             - "pattern_lastname"
                 -m:
                     full: --mail
-                    help: Main unique email address
+                    help: (Deprecated, see --domain) Main unique email address
-                    extra:
-                        ask: ask_email
-                        required: True
-                        pattern: &pattern_email
-                            - !!str ^[\w.-]+@([^\W_A-Z]+([-]*[^\W_A-Z]+)*\.)+([^\W\d_]{2,})$
-                            - "pattern_email"
                 -p:
                     full: --password
                     help: User password
@@ -116,6 +110,13 @@ user:
                             - !!str ^.{3,}$
                             - "pattern_password"
                         comment: good_practices_about_user_password
+                -d:
+                    full: --domain
+                    help: Domain for the email address and xmpp account
+                    extra:
+                        pattern: &pattern_domain
+                            - !!str ^([^\W_A-Z]+([-]*[^\W_A-Z]+)*\.)+((xn--)?[^\W_]{2,})$
+                            - "pattern_domain"
                 -q:
                     full: --mailbox-quota
                     help: Mailbox size quota
@@ -157,7 +158,9 @@ user:
                 -m:
                     full: --mail
                     extra:
-                        pattern: *pattern_email
+                        pattern: &pattern_email
+                            - !!str ^[\w.-]+@([^\W_A-Z]+([-]*[^\W_A-Z]+)*\.)+((xn--)?[^\W_]{2,})$
+                            - "pattern_email"
                 -p:
                     full: --change-password
                     help: New password to set
@@ -419,9 +422,7 @@ domain:
                 domain:
                     help: Domain name to add
                     extra:
-                        pattern: &pattern_domain
+                        pattern: *pattern_domain
-                            - !!str ^([^\W_A-Z]+([-]*[^\W_A-Z]+)*\.)+([^\W\d_]{2,})$
-                            - "pattern_domain"
                 -d:
                     full: --dyndns
                     help: Subscribe to the DynDNS service

locales/en.json

@@ -60,6 +60,7 @@
     "apps_catalog_failed_to_download": "Unable to download the {apps_catalog} app catalog: {error}",
     "apps_catalog_obsolete_cache": "The app catalog cache is empty or obsolete.",
     "apps_catalog_update_success": "The application catalog has been updated!",
+    "ask_user_domain": "Domain to use for the user's email address and XMPP account",
     "ask_email": "E-mail address",
     "ask_firstname": "First name",
     "ask_lastname": "Last name",

src/yunohost/tests/test_user-group.py

@@ -26,9 +26,9 @@ def setup_function(function):
     global maindomain
     maindomain = _get_maindomain()
 
-    user_create("alice", "Alice", "White", "alice@" + maindomain, "test123Ynh")
+    user_create("alice", "Alice", "White", maindomain, "test123Ynh")
-    user_create("bob", "Bob", "Snow", "bob@" + maindomain, "test123Ynh")
+    user_create("bob", "Bob", "Snow", maindomain, "test123Ynh")
-    user_create("jack", "Jack", "Black", "jack@" + maindomain, "test123Ynh")
+    user_create("jack", "Jack", "Black", maindomain, "test123Ynh")
 
     user_group_create("dev")
     user_group_create("apps")
@@ -79,7 +79,7 @@ def test_list_groups():
 def test_create_user(mocker):
 
     with message(mocker, "user_created"):
-        user_create("albert", "Albert", "Good", "alber@" + maindomain, "test123Ynh")
+        user_create("albert", "Albert", "Good", maindomain, "test123Ynh")
 
     group_res = user_group_list()['groups']
     assert "albert" in user_list()['users']
@@ -123,25 +123,26 @@ def test_del_group(mocker):
 #
 
 
-def test_create_user_with_mail_address_already_taken(mocker):
-    with raiseYunohostError(mocker, "user_creation_failed"):
-        user_create("alice2", "Alice", "White", "alice@" + maindomain, "test123Ynh")
-
-
 def test_create_user_with_password_too_simple(mocker):
     with raiseYunohostError(mocker, "password_listed"):
-        user_create("other", "Alice", "White", "other@" + maindomain, "12")
+        user_create("other", "Alice", "White", maindomain, "12")
 
 
 def test_create_user_already_exists(mocker):
     with raiseYunohostError(mocker, "user_already_exists"):
-        user_create("alice", "Alice", "White", "other@" + maindomain, "test123Ynh")
+        user_create("alice", "Alice", "White", maindomain, "test123Ynh")
 
+def test_create_user_with_domain_that_doesnt_exists(mocker):
+    with raiseYunohostError(mocker, "domain_unknown"):
+        user_create("alice", "Alice", "White", "doesnt.exists", "test123Ynh")
 
 def test_update_user_with_mail_address_already_taken(mocker):
     with raiseYunohostError(mocker, "user_update_failed"):
         user_update("bob", add_mailalias="alice@" + maindomain)
 
+def test_update_user_with_mail_address_with_unknown_domain(mocker):
+    with raiseYunohostError(mocker, "mail_domain_unknown"):
+        user_update("alice", add_mailalias="alice@doesnt.exists")
 
 def test_del_user_that_does_not_exist(mocker):
     with raiseYunohostError(mocker, "user_unknown"):

src/yunohost/user.py

@@ -34,7 +34,7 @@ import string
 import subprocess
 import copy
 
-from moulinette import m18n
+from moulinette import msignals, msettings, m18n
 from moulinette.utils.log import getActionLogger
 from moulinette.utils.filesystem import read_json, write_to_json, read_yaml, write_to_yaml
 
@@ -46,16 +46,7 @@ logger = getActionLogger('yunohost.user')
 
 
 def user_list(fields=None):
-    """
-    List users
 
-    Keyword argument:
-        filter -- LDAP filter used to search
-        offset -- Starting number for user fetching
-        limit -- Maximum number of user fetched
-        fields -- fields to fetch
-
-    """
     from yunohost.utils.ldap import _get_ldap_interface
 
     user_attrs = {
@@ -105,20 +96,9 @@ def user_list(fields=None):
 
 
 @is_unit_operation([('username', 'user')])
-def user_create(operation_logger, username, firstname, lastname, mail, password,
+def user_create(operation_logger, username, firstname, lastname, domain, password,
-                mailbox_quota="0"):
+                mailbox_quota="0", mail=None):
-    """
-    Create user
 
-    Keyword argument:
-        firstname
-        lastname
-        username -- Must be unique
-        mail -- Main mail address must be unique
-        password
-        mailbox_quota -- Mailbox size quota
-
-    """
     from yunohost.domain import domain_list, _get_maindomain
     from yunohost.hook import hook_callback
     from yunohost.utils.password import assert_password_is_strong_enough
@@ -127,6 +107,30 @@ def user_create(operation_logger, username, firstname, lastname, mail, password,
     # Ensure sufficiently complex password
     assert_password_is_strong_enough("user", password)
 
+    if mail is not None:
+        logger.warning("Packagers ! Using --mail in 'yunohost user create' is deprecated ... please use --domain instead.")
+        domain = mail.split("@")[-1]
+
+    # Validate domain used for email address/xmpp account
+    if domain is None:
+        if msettings.get('interface') == 'api':
+            raise YunohostError('Invalide usage, specify domain argument')
+        else:
+            # On affiche les differents domaines possibles
+            msignals.display(m18n.n('domains_available'))
+            for domain in domain_list()['domains']:
+                msignals.display("- {}".format(domain))
+
+            maindomain = _get_maindomain()
+            domain = msignals.prompt(m18n.n('ask_user_domain') + ' (default: %s)' % maindomain)
+            if not domain:
+                domain = maindomain
+
+    # Check that the domain exists
+    if domain not in domain_list()['domains']:
+        raise YunohostError('domain_unknown', domain)
+
+    mail = username + '@' + domain
     ldap = _get_ldap_interface()
 
     if username in user_list()["users"]:
@@ -158,10 +162,6 @@ def user_create(operation_logger, username, firstname, lastname, mail, password,
     if mail in aliases:
         raise YunohostError('mail_unavailable')
 
-    # Check that the mail domain exists
-    if mail.split("@")[1] not in domain_list()['domains']:
-        raise YunohostError('mail_domain_unknown', domain=mail.split("@")[1])
-
     operation_logger.start()
 
     # Get random UID/GID
@@ -176,6 +176,7 @@ def user_create(operation_logger, username, firstname, lastname, mail, password,
 
     # Adapt values for LDAP
     fullname = '%s %s' % (firstname, lastname)
+
     attr_dict = {
         'objectClass': ['mailAccount', 'inetOrgPerson', 'posixAccount', 'userPermissionYnh'],
         'givenName': [firstname],